fix(importer): confirm deletes against the filesystem before they cascade - #825
Merged
Conversation
A tool that swaps a file by ``rm`` + ``mv``, and any watcher backend that reports an atomic replace as a delete plus an add, leaves both events in one batch. The recreated file carries a new inode, so move detection can never pair them, and dedup let the delete win: the row died, cascading its bookmarks and read progress, while a file sat at that very path. The comic then reappeared on the next scan as a new, unread one. It is the same path with new content, which is a modification. The poller already reached that conclusion by diffing snapshots; this makes the watcher agree. Custom covers are treated the same way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deleting a comic row cascades its bookmarks and read progress away, and nothing brings them back — the next scan re-imports the file as a fresh, unread comic. Yet both scanners *infer* deletes, and every inference has failure modes that name a path still sitting on disk: a watch batch that carries a delete whose paired add lands in the next batch, a directory expansion that overmatched, an inode pair the compatibility checks refused. So the delete phase now confirms each path against the filesystem and leaves anything still there for the next scan to reconcile. A stale row costs a re-read; a wrongly deleted one costs the user their place in the book. Comics, folders and custom covers all check. This cannot save a library whose whole mount vanished, where every path reads as missing, so a delete large enough to look like that logs where to go looking instead. Also fixes a browser staleness bug in the same phase: comics under a deleted folder die by cascade rather than by path, so they never reached the collection capture, and the series or publisher a folder delete emptied was never re-stamped — browsers kept listing comics that were gone. They were also counted as folders rather than comics. The move-guard test that asserted an unrelated comic still deletes left its file on disk, so its fixture now removes it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second of the fixes from the rename/move pipeline audit. This one is structural: it removes the consequence that made every other pairing bug in that audit permanent.
Prerequisite for the rename-first redesign that follows — that change deletes the tag-write guard registry, and this is what keeps its failure modes from costing bookmarks.
The pattern this fixes
Deleting a comic row cascades its bookmarks and read progress away, and nothing brings them back: the next scan re-imports the file as a fresh, unread comic. But both scanners infer deletes rather than observing them, and every inference has failure modes that name a path still sitting on disk:
Each was a separate bug with a separate fix. The common ending was the same: a hard delete by path with no check that the file was actually gone.
Changes
Deletes are confirmed against the filesystem. Comics, folders and custom covers all check before deleting, and anything still on disk is left for the next scan. A stale row costs a re-read; a wrongly deleted one costs the user their place in the book.
A file replaced in place is re-read, not deleted. A path reported both deleted and added in one batch is reclassified as modified. The recreated file has a new inode so move detection can never pair it, and dedup previously let the delete win. The poller already reached this conclusion by diffing snapshots — this makes the watcher agree.
A mass delete is flagged. The backstop cannot save a library whose whole mount vanished, since every path then reads as missing. A delete large enough to look like that (≥50% of the library, ≥50 comics) logs where to go looking. Nothing is blocked.
Folder deletes re-stamp what they emptied. Comics under a deleted folder die by the
parent_foldercascade rather than by path, so they never reached the collection capture: the series or publisher a folder delete emptied was never re-stamped and browsers kept listing comics that were gone. They were also counted as folders rather than comics.Notes for review
test_unregistered_paths_are_untouchedasserted an unrelated comic still deletes while leaving its file on disk — which is now precisely the case that is spared. It removes the file, so it tests guard scoping rather than passing for the wrong reason.Path.exists()per deleted path (paths are already batched), and onecount()per import that deletes comics. A path that is genuinely gone answers immediately.make lint,make ty, full pytest and vitest all pass.