fix(janitor): custom cover cleanup fails closed on an unreadable path

[ajslater]

Implements D9 of tasks/followups-implementation-plan.md §3, which the maintainer chose to fold into P1 rather than file separately. Kept as its own PR so it stays reviewable — it is a different model and a different bug from the pending-delete mechanism.

Stacked on #883 → #882 → #881 → #880 → #879. Retarget as each merges.

The bug

The nightly custom-cover cleanup had the same disease the comic delete path was cured of, on a different model — and permanently. BrowserCollectionModel.custom_cover is ForeignKey(CustomCover, on_delete=SET_DEFAULT, null=True, default=None), so deleting the row silently nulls the cover of every collection pointing at it, and nothing restores it. The admin has to find and re-upload the image.

Two ways "I could not look" became "it is not there":

1. ENOENT on the parent condemned the whole group. _scan_parent_for_present_names returned frozenset() for a FileNotFoundError on the parent, so every cover claiming a path under it was marked orphan in one step, with no probe of its own. A share mid-reconnect answers exactly that way.

2. Any other OSError fell back to Path(parent, basename).exists(), which swallows EACCES and returns False.

Confirmed empirically on this Python, and worth knowing for future tests — Path.exists() does not go through Path.stat:

Path.stat patched -> exists() = True
os.stat patched   -> exists() = False

So a permission blip on one folder returned False for files sitting right there, and every collection cover under it was permanently nulled.

Neither GONE_ERRORS, probe_paths, confirm_deleted nor revived_paths was used, though all four existed.

The fix

A directory that would not list now proves nothing — ENOENT included — and each cover path is probed individually by the same probe_paths the importer uses, which fails closed: only ENOENT / ENOTDIR reach the gone pile, and a permission error leaves the row alone.

Candidates then take the second look before confirm_deleted, matching DeletedImporter.delete's order rather than inventing a new one: probe, wait, re-probe only what came back missing, so a filesystem that lies for a moment during a reconnect is caught before anything is condemned.

_group_covers_by_parent drops its now-unused pk half and groups basenames; the delete matches on path__in instead.

The scandir-per-parent optimisation is untouched — one readdir still replaces N stat round-trips on NFS/SMB, which is why it was written that way.

Tests

tests/test_janitor_cleanup_custom_covers.py, 5 cases. All three failure modes reproduce against the pre-fix code (verified by restoring the old semantics and removing the probe layer — the first attempt only reverted the scandir half, and the new probe layer masked it, which is worth knowing):

Case	Pre-fix
an unreadable directory keeps its covers	❌ deleted
a vanished directory does not condemn its group	❌ deleted
a cover that answers on the second look is kept	❌ deleted
a present cover is kept	✅
a really deleted cover is removed	✅

The last one matters: the job still does its job.

Both fakes capture the real os.scandir / os.stat before patching — cleanup imports the shared os module, so a fake that called os.scandir would recurse into itself. CI runs as root, so unreadability is simulated by patching, never chmod.

make fix && make lint && make ty clean. Full make test green: 1309 pytest (+5), 587 vitest.

NEWS, under Fixes: "A briefly unreadable folder no longer loses its collections' custom covers."

Still open, deliberately

_cleanup_orphan_covers purges every custom-cover thumbnail on every nightly run — purge.py builds db_cover_paths with a hardcoded custom=False while being called with cover_root=self.CUSTOM_COVERS_ROOT, so no path under that root can ever be in the set and the walk deletes all of them. That is a separate bug in a separate module (thumbnails, not rows, so it is recoverable) and deserves its own change.

🤖 Generated with Claude Code