#904: guard standalone Deep Scan against relocating a desynced library
Standalone _run_soulsync_deep_scan did a path-only diff (untracked = transfer files not in the soulsync DB) and shutil.move'd EVERY untracked file to Staging — no guard. When the DB is empty/out of sync with disk (volume swap, DB reset, external Picard tag edits) but Transfer holds the real library, that flags the whole library as untracked and relocates all of it; Phase 5 then deletes the rows, and with Staging cleanup on the files are gone for good. Reporter lost ~1,500 tracks into Staging. The stale_guard the orphan detector + media-server deep scan already use (#828, #908) was never wired into this path. Fix: - core/library/standalone_scan.py (pure, tested): plan_standalone_deep_scan() diffs untracked (separator-normalized) and decides whether the move is safe. Blocks when the untracked share is implausibly large (>20 files AND >50% of Transfer — the desync signature, via is_implausible_orphan_flood) or when the user marked Transfer permanent. A normal batch of new arrivals still moves. - web_server: consult the planner before Phase 4; on block, move NOTHING, leave files in place, and surface a loud warning + activity item. Guard Phase 5 deletes too (skip on desync-block or implausible stale share). - 'Transfer is my permanent library — never move files out' toggle (import.transfer_is_permanent) in Settings. - tests/library/test_standalone_scan.py: seam coverage + the #904 regression (empty DB + 1,500 files -> blocked, nothing moved). No behavior change for in-sync libraries; the guard only trips on the desync pattern.
This commit is contained in:
parent
df6815c2cc
commit
01c51a3c0e
5 changed files with 289 additions and 9 deletions
104
core/library/standalone_scan.py
Normal file
104
core/library/standalone_scan.py
Normal file
|
|
@ -0,0 +1,104 @@
|
|||
"""Decision logic for the SoulSync standalone Deep Scan's untracked → Staging move.
|
||||
|
||||
The standalone deep scan (``_run_soulsync_deep_scan`` in web_server) walks the
|
||||
Transfer folder, diffs it against the ``soulsync`` rows in the DB, and relocates
|
||||
every file it can't find a DB record for into Staging for auto-import. That's fine
|
||||
when Transfer is a scratch/landing area — files arrive, get moved, imported, and
|
||||
recorded, so a later scan only ever sees a few genuinely-new arrivals.
|
||||
|
||||
It is a DATA-LOSS trap when the DB is empty or out of sync with disk (a volume
|
||||
swap, a DB reset, external tag edits) while Transfer holds the user's real library:
|
||||
a path-only diff then flags the *entire* library as "untracked" and the scan
|
||||
relocates all of it (issue #904). The same failure mode the orphan detector and the
|
||||
media-server deep scan already guard against (``core.library.stale_guard``) — this
|
||||
path just never used the guard.
|
||||
|
||||
This module is the pure, testable decision: given the Transfer file set, the DB's
|
||||
known paths, and the user's "Transfer is my permanent library" preference, decide
|
||||
WHICH files are untracked and WHETHER it's safe to relocate them. The web layer does
|
||||
only the I/O (walk/move/delete) based on the returned plan.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Iterable, Set
|
||||
|
||||
from core.library.stale_guard import (
|
||||
DEFAULT_MAX_ORPHAN_FRACTION,
|
||||
DEFAULT_MIN_ORPHANS,
|
||||
is_implausible_orphan_flood,
|
||||
)
|
||||
|
||||
# Block reason codes (web layer turns these into a user-facing warning).
|
||||
BLOCK_NONE = ""
|
||||
BLOCK_TRANSFER_PERMANENT = "transfer_permanent"
|
||||
BLOCK_DESYNC = "desync"
|
||||
|
||||
|
||||
def _norm(path: str) -> str:
|
||||
"""Normalize a path for cross-platform comparison (Windows vs Unix separators)."""
|
||||
return str(path).replace("\\", "/")
|
||||
|
||||
|
||||
def diff_untracked(transfer_files: Iterable[str], db_paths: Iterable[str]) -> Set[str]:
|
||||
"""Files present in ``transfer_files`` but with no matching ``db_paths`` record.
|
||||
|
||||
Comparison is separator-normalized, so a DB path stored with one separator style
|
||||
still matches the on-disk path. Pure — no I/O. Returns the original (un-normalized)
|
||||
transfer paths so the caller can act on the real filesystem entries.
|
||||
"""
|
||||
db_norm = {_norm(p) for p in db_paths if p}
|
||||
return {f for f in transfer_files if _norm(f) not in db_norm}
|
||||
|
||||
|
||||
def plan_standalone_deep_scan(
|
||||
transfer_files: Iterable[str],
|
||||
db_paths: Iterable[str],
|
||||
*,
|
||||
never_move: bool = False,
|
||||
min_untracked: int = DEFAULT_MIN_ORPHANS,
|
||||
max_fraction: float = DEFAULT_MAX_ORPHAN_FRACTION,
|
||||
) -> dict:
|
||||
"""Plan the untracked → Staging move for a standalone deep scan. Pure — no I/O.
|
||||
|
||||
Returns a dict:
|
||||
* ``untracked`` (set[str]) — Transfer files with no DB record.
|
||||
* ``move_blocked`` (bool) — True when the untracked files must NOT be relocated.
|
||||
* ``block_reason`` (str) — ``BLOCK_TRANSFER_PERMANENT`` / ``BLOCK_DESYNC`` / "".
|
||||
|
||||
The move is blocked when either:
|
||||
* ``never_move`` is set (the user marked Transfer as their permanent library), or
|
||||
* the untracked share is implausibly large (> ``min_untracked`` files AND
|
||||
> ``max_fraction`` of the folder) — the empty/desynced-DB signature, where a
|
||||
path-only diff would relocate the whole library. Below that floor a normal
|
||||
batch of new arrivals still moves as before.
|
||||
|
||||
``move_blocked`` is only ever True when there ARE untracked files; an empty scan
|
||||
or a clean library returns ``move_blocked=False`` with no reason.
|
||||
"""
|
||||
transfer_set = set(transfer_files) # concrete (handles generators) + dedups
|
||||
untracked = diff_untracked(transfer_set, db_paths)
|
||||
total = len(transfer_set)
|
||||
n_untracked = len(untracked)
|
||||
|
||||
if n_untracked == 0:
|
||||
return {"untracked": untracked, "move_blocked": False, "block_reason": BLOCK_NONE}
|
||||
|
||||
if never_move:
|
||||
return {"untracked": untracked, "move_blocked": True, "block_reason": BLOCK_TRANSFER_PERMANENT}
|
||||
|
||||
if is_implausible_orphan_flood(
|
||||
n_untracked, total, min_orphans=min_untracked, max_fraction=max_fraction
|
||||
):
|
||||
return {"untracked": untracked, "move_blocked": True, "block_reason": BLOCK_DESYNC}
|
||||
|
||||
return {"untracked": untracked, "move_blocked": False, "block_reason": BLOCK_NONE}
|
||||
|
||||
|
||||
__all__ = [
|
||||
"diff_untracked",
|
||||
"plan_standalone_deep_scan",
|
||||
"BLOCK_NONE",
|
||||
"BLOCK_TRANSFER_PERMANENT",
|
||||
"BLOCK_DESYNC",
|
||||
]
|
||||
115
tests/library/test_standalone_scan.py
Normal file
115
tests/library/test_standalone_scan.py
Normal file
|
|
@ -0,0 +1,115 @@
|
|||
"""Standalone Deep Scan planner — the #904 data-loss guard.
|
||||
|
||||
The scan relocates Transfer files the DB doesn't know about into Staging. With a
|
||||
path-only diff, an empty/desynced DB makes the WHOLE library look untracked and the
|
||||
scan moved all of it (reporter lost ~1,500 tracks into Staging). These pin the guard:
|
||||
a normal batch of new arrivals still moves; an implausibly large untracked share (the
|
||||
desync signature) or a 'permanent library' opt-out blocks the move instead.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from core.library.standalone_scan import (
|
||||
BLOCK_DESYNC,
|
||||
BLOCK_NONE,
|
||||
BLOCK_TRANSFER_PERMANENT,
|
||||
diff_untracked,
|
||||
plan_standalone_deep_scan,
|
||||
)
|
||||
|
||||
|
||||
def _files(prefix, n, start=0):
|
||||
return {f"{prefix}/track{i}.flac" for i in range(start, start + n)}
|
||||
|
||||
|
||||
# ── diff_untracked (pure path diff) ──────────────────────────────────────────
|
||||
|
||||
def test_diff_basic():
|
||||
transfer = {"/m/a.flac", "/m/b.flac", "/m/c.flac"}
|
||||
db = {"/m/a.flac", "/m/b.flac"}
|
||||
assert diff_untracked(transfer, db) == {"/m/c.flac"}
|
||||
|
||||
|
||||
def test_diff_is_separator_normalized():
|
||||
# DB stored a Windows-style path; the on-disk path uses forward slashes → still a match
|
||||
transfer = {"/m/Artist/x.flac"}
|
||||
db = {"\\m\\Artist\\x.flac"}
|
||||
assert diff_untracked(transfer, db) == set()
|
||||
|
||||
|
||||
def test_diff_all_untracked_when_db_empty():
|
||||
transfer = _files("/lib", 5)
|
||||
assert diff_untracked(transfer, set()) == transfer
|
||||
|
||||
|
||||
# ── plan: normal (move allowed) ──────────────────────────────────────────────
|
||||
|
||||
def test_clean_library_not_blocked():
|
||||
transfer = _files("/lib", 1000)
|
||||
plan = plan_standalone_deep_scan(transfer, transfer) # all known
|
||||
assert plan["untracked"] == set()
|
||||
assert plan["move_blocked"] is False
|
||||
assert plan["block_reason"] == BLOCK_NONE
|
||||
|
||||
|
||||
def test_normal_new_arrivals_move():
|
||||
# DB knows 990; 10 new files dropped in → small share, moves as before
|
||||
known = _files("/lib", 990)
|
||||
transfer = known | _files("/lib", 10, start=990)
|
||||
plan = plan_standalone_deep_scan(transfer, known)
|
||||
assert len(plan["untracked"]) == 10
|
||||
assert plan["move_blocked"] is False
|
||||
|
||||
|
||||
def test_small_fresh_import_under_floor_moves():
|
||||
# A tiny brand-new folder (under the absolute floor) isn't second-guessed
|
||||
transfer = _files("/lib", 5)
|
||||
plan = plan_standalone_deep_scan(transfer, set())
|
||||
assert len(plan["untracked"]) == 5
|
||||
assert plan["move_blocked"] is False
|
||||
|
||||
|
||||
# ── plan: the #904 guard (move blocked) ──────────────────────────────────────
|
||||
|
||||
def test_regression_904_empty_db_full_library_blocks():
|
||||
# Empty DB + a real 1,500-track library → 100% untracked → BLOCKED, nothing moved
|
||||
transfer = _files("/library", 1500)
|
||||
plan = plan_standalone_deep_scan(transfer, set())
|
||||
assert len(plan["untracked"]) == 1500
|
||||
assert plan["move_blocked"] is True
|
||||
assert plan["block_reason"] == BLOCK_DESYNC
|
||||
|
||||
|
||||
def test_majority_untracked_blocks():
|
||||
# 600 of 1000 unknown (60%, over the 50% line) → desync, blocked
|
||||
known = _files("/lib", 400)
|
||||
transfer = known | _files("/lib", 600, start=400)
|
||||
plan = plan_standalone_deep_scan(transfer, known)
|
||||
assert plan["move_blocked"] is True
|
||||
assert plan["block_reason"] == BLOCK_DESYNC
|
||||
|
||||
|
||||
def test_minority_untracked_just_under_threshold_moves():
|
||||
# 400 of 1000 unknown (40%, under 50%) → still treated as a batch, not a desync
|
||||
known = _files("/lib", 600)
|
||||
transfer = known | _files("/lib", 400, start=600)
|
||||
plan = plan_standalone_deep_scan(transfer, known)
|
||||
assert plan["move_blocked"] is False
|
||||
|
||||
|
||||
def test_never_move_blocks_even_small_sets():
|
||||
# Permanent-library opt-out: block regardless of fraction
|
||||
known = _files("/lib", 990)
|
||||
transfer = known | _files("/lib", 10, start=990)
|
||||
plan = plan_standalone_deep_scan(transfer, known, never_move=True)
|
||||
assert len(plan["untracked"]) == 10
|
||||
assert plan["move_blocked"] is True
|
||||
assert plan["block_reason"] == BLOCK_TRANSFER_PERMANENT
|
||||
|
||||
|
||||
def test_never_move_with_nothing_untracked_is_not_blocked():
|
||||
# Nothing to move → not a "blocked" outcome even with the toggle on
|
||||
transfer = _files("/lib", 100)
|
||||
plan = plan_standalone_deep_scan(transfer, transfer, never_move=True)
|
||||
assert plan["untracked"] == set()
|
||||
assert plan["move_blocked"] is False
|
||||
|
|
@ -16374,16 +16374,40 @@ def _run_soulsync_deep_scan():
|
|||
|
||||
logger.info(f"[SoulSync Deep Scan] {len(db_paths)} tracks in soulsync DB")
|
||||
|
||||
# Phase 3: Find untracked files (in Transfer but not in DB)
|
||||
untracked = transfer_files - db_paths
|
||||
# Also check with normalized paths (Windows vs Unix separators)
|
||||
if untracked:
|
||||
db_paths_normalized = {p.replace('\\', '/') for p in db_paths}
|
||||
untracked = {f for f in untracked if f.replace('\\', '/') not in db_paths_normalized}
|
||||
# Phase 3: Plan the untracked → Staging move, with the data-loss guard (#904).
|
||||
# A path-only diff treats EVERY file the DB doesn't know about as "a new arrival
|
||||
# to relocate". When the DB is empty/out of sync with disk (volume swap, DB reset,
|
||||
# external tag edits) but Transfer holds the real library, that flags the whole
|
||||
# library as untracked and relocates all of it. The planner refuses the move when
|
||||
# the untracked share is implausibly large (the desync signature) or when the user
|
||||
# marked Transfer as their permanent library — leaving files in place and warning.
|
||||
from core.library.standalone_scan import (
|
||||
plan_standalone_deep_scan, BLOCK_TRANSFER_PERMANENT, BLOCK_DESYNC,
|
||||
)
|
||||
never_move = bool(config_manager.get('import.transfer_is_permanent', False))
|
||||
plan = plan_standalone_deep_scan(transfer_files, db_paths, never_move=never_move)
|
||||
untracked = plan['untracked']
|
||||
move_blocked = plan['move_blocked']
|
||||
block_reason = plan['block_reason']
|
||||
|
||||
# Phase 4: Move untracked files to Staging for auto-import
|
||||
# Phase 4: Move untracked files to Staging for auto-import — unless guarded.
|
||||
moved_count = 0
|
||||
if untracked and os.path.isdir(staging_path):
|
||||
blocked_count = 0
|
||||
if untracked and move_blocked:
|
||||
blocked_count = len(untracked)
|
||||
if block_reason == BLOCK_TRANSFER_PERMANENT:
|
||||
warn = (f"Deep scan: {blocked_count} file(s) in Transfer aren't in the database, "
|
||||
f"but Transfer is marked your permanent library — nothing was moved.")
|
||||
else: # BLOCK_DESYNC
|
||||
pct = round(100 * blocked_count / max(1, len(transfer_files)))
|
||||
warn = (f"Deep scan STOPPED to protect your library: {blocked_count} of "
|
||||
f"{len(transfer_files)} files in Transfer ({pct}%) aren't in the database. "
|
||||
f"That usually means the database is out of sync with disk, not that you "
|
||||
f"have {blocked_count} new files — so NOTHING was moved. Re-sync/import "
|
||||
f"before scanning, or enable 'Transfer is my permanent library'.")
|
||||
logger.warning(f"[SoulSync Deep Scan] {warn}")
|
||||
add_activity_item("", "SoulSync Deep Scan — move blocked", warn, "Now")
|
||||
elif untracked and os.path.isdir(staging_path):
|
||||
_db_update_phase_callback('moving_untracked')
|
||||
for file_path in untracked:
|
||||
try:
|
||||
|
|
@ -16415,6 +16439,23 @@ def _run_soulsync_deep_scan():
|
|||
stale_track_ids.append(db_path)
|
||||
stale_count += 1
|
||||
|
||||
# Guard the deletes the same way as the move (#904): if a desync blocked the
|
||||
# move, the DB<->disk mapping is unreliable, so os.path.exists may be lying for
|
||||
# every file — don't delete rows. Also independently skip when the stale share
|
||||
# is implausibly large (storage unreachable / remount), mirroring the orphan guard.
|
||||
from core.library.stale_guard import is_implausible_stale_removal
|
||||
if move_blocked and block_reason == BLOCK_DESYNC:
|
||||
if stale_track_ids:
|
||||
logger.warning(f"[SoulSync Deep Scan] Skipping removal of {stale_count} 'stale' "
|
||||
f"records — move was blocked for desync, mapping is unreliable.")
|
||||
stale_track_ids = []
|
||||
stale_count = 0
|
||||
elif is_implausible_stale_removal(stale_count, len(db_paths)):
|
||||
logger.warning(f"[SoulSync Deep Scan] Skipping removal of {stale_count}/{len(db_paths)} "
|
||||
f"'stale' records — implausibly large share, storage likely unreachable.")
|
||||
stale_track_ids = []
|
||||
stale_count = 0
|
||||
|
||||
# Remove stale records
|
||||
if stale_track_ids:
|
||||
try:
|
||||
|
|
@ -16447,9 +16488,11 @@ def _run_soulsync_deep_scan():
|
|||
summary = f"Deep scan complete: {len(transfer_files)} files scanned"
|
||||
if moved_count > 0:
|
||||
summary += f", {moved_count} untracked files moved to Staging"
|
||||
if blocked_count > 0:
|
||||
summary += f", {blocked_count} untracked files LEFT IN PLACE (move blocked — see warning)"
|
||||
if stale_count > 0:
|
||||
summary += f", {stale_count} stale records removed"
|
||||
if moved_count == 0 and stale_count == 0:
|
||||
if moved_count == 0 and blocked_count == 0 and stale_count == 0:
|
||||
summary += " — library is clean"
|
||||
|
||||
logger.info(f"[SoulSync Deep Scan] {summary}")
|
||||
|
|
|
|||
|
|
@ -6456,6 +6456,21 @@
|
|||
that folder's name (the cause of the "soulsync" mass-mislabel). With it off, the
|
||||
metadata-identified artist is always kept.
|
||||
</div>
|
||||
|
||||
<div class="form-group">
|
||||
<label class="checkbox-label">
|
||||
<input type="checkbox" id="import-transfer-permanent">
|
||||
Transfer is my permanent library — never move files out
|
||||
</label>
|
||||
</div>
|
||||
<div class="help-text">
|
||||
Off by default. Turn this <strong>on</strong> if your Transfer folder is your
|
||||
real, permanent library (served by Navidrome/Plex/etc.) rather than a scratch
|
||||
drop area. A Deep Scan then never relocates files it doesn't recognize out to
|
||||
Staging — it leaves them in place and just reports them. Even with this off, a
|
||||
Deep Scan now refuses to move files when the database looks badly out of sync
|
||||
with disk, as a safety guard against wiping a library (issue #904).
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -1341,6 +1341,8 @@ async function loadSettingsData() {
|
|||
document.getElementById('import-replace-lower-quality').checked = settings.import?.replace_lower_quality === true;
|
||||
const _folderArtistEl = document.getElementById('import-folder-artist-override');
|
||||
if (_folderArtistEl) _folderArtistEl.checked = settings.import?.folder_artist_override === true;
|
||||
const _transferPermEl = document.getElementById('import-transfer-permanent');
|
||||
if (_transferPermEl) _transferPermEl.checked = settings.import?.transfer_is_permanent === true;
|
||||
|
||||
// Populate M3U Export settings
|
||||
document.getElementById('m3u-export-enabled').checked = settings.m3u_export?.enabled === true;
|
||||
|
|
@ -3216,6 +3218,7 @@ async function saveSettings(quiet = false) {
|
|||
import: {
|
||||
replace_lower_quality: document.getElementById('import-replace-lower-quality').checked,
|
||||
folder_artist_override: document.getElementById('import-folder-artist-override')?.checked === true,
|
||||
transfer_is_permanent: document.getElementById('import-transfer-permanent')?.checked === true,
|
||||
staging_path: document.getElementById('staging-path').value || './Staging'
|
||||
},
|
||||
playlists: {
|
||||
|
|
|
|||
Loading…
Reference in a new issue