Skip to content

fix(importer): confirm deletes against the filesystem before they cascade - #825

Merged
ajslater merged 3 commits into
developfrom
fix/delete-existence-backstop
Aug 26, 2026
Merged

fix(importer): confirm deletes against the filesystem before they cascade#825
ajslater merged 3 commits into
developfrom
fix/delete-existence-backstop

Conversation

@ajslater

Copy link
Copy Markdown
Owner

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:

  • a watch batch carrying a delete whose paired add lands in the next batch (the 60s force-yield during bulk external activity)
  • a directory expansion that overmatched
  • an inode pair the size/type compatibility checks refused
  • a file replaced in place, where the delete and the add arrive together

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_folder cascade 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

  • One existing test's fixture changed. test_unregistered_paths_are_untouched asserted 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.
  • Cost: one Path.exists() per deleted path (paths are already batched), and one count() per import that deletes comics. A path that is genuinely gone answers immediately.
  • Not covered: an unmount is still destructive if the poller runs against a missing mount; the poller has its own guards for that and the watcher does not. That gap is tracked separately, and the new log line is the breadcrumb for it.
  • Every new test was verified to fail against the old code. make lint, make ty, full pytest and vitest all pass.

ajslater and others added 3 commits August 25, 2026 17:21
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>
@ajslater
ajslater merged commit 88ed4dc into develop Aug 26, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant