timothy/ersatztv
1
0
Fork 0
Code Issues 54 Pull Requests 6 Actions Packages Projects Releases 9 Wiki Activity

Scanner has no anti-nuke guard: a successful-but-empty media-server fetch flags an entire library FileNotFound #477

New Issue
Closed
opened 2026-07-19 20:59:32 +02:00 by timothy · 3 comments
timothy commented 2026-07-19 20:59:32 +02:00
Owner
Copy Link
Copy Source

Found while working #473 (2026-07-19). Latent, but data-loss-adjacent — filing so it is not rediscovered the hard way.

The gap

All three media-server scanners reconcile with existing.Except(incomingItemIds) → FlagFileNotFound...:

  • ErsatzTV.Scanner/Core/Metadata/MediaServerTelevisionLibraryScanner.cs:173-182, 369-371, 536-538
  • .../MediaServerMovieLibraryScanner.cs:205-207
  • .../MediaServerOtherVideoLibraryScanner.cs:212-214

If the remote fetch returns successfully but empty, incomingItemIds is [], so existing.Except([]) is every item in the library and all of them are flagged FileNotFound.

There is no threshold/ratio check, no "abort if incoming count is 0", and no comparison against the server's reported total anywhere in the three scanners. The only guard present is the trivially-inverted one — if (showItemIds.Count == 0) return []; (JellyfinTelevisionRepository.cs:365-368, 397-400, 429-432) — which short-circuits when nothing is missing, not when everything is.

Why a successful-but-empty fetch is reachable

  • JellyfinApiClient.GetPagedLibraryItems (ErsatzTV.Infrastructure/Jellyfin/JellyfinApiClient.cs:369-404) computes pages from TotalRecordCount; a TotalRecordCount of 0 yields pages = 0 and a clean empty enumeration — no exception, so the sweep proceeds normally. A media server that is up but mid-restore / mid-library-rebuild / returning an emptied library is exactly this case.
  • Protection against a mid-pagination failure is incidental rather than defensive: there is no try/catch in that method, so the exception unwinds past the flag step. MediaServerTelevisionLibraryScanner.cs:67-70 catches only TaskCanceledException/OperationCanceledException. It works, but by accident of control flow.
  • ProjectToShow/ProjectToEpisode failures are silently dropped by .Somes() (JellyfinApiClient.cs:400-403), so an item the server did return but ErsatzTV failed to project is indistinguishable from "deleted upstream" at the reconciliation step. A projection regression could therefore mass-flag healthy items.

Blast radius

FileNotFound is not itself destructive, but it feeds two consumers that are:

  • EmptyTrashHandler (ErsatzTV.Application/Maintenance/Commands/) searches state:FileNotFound (cap 10,000) and calls _mediaItemRepository.DeleteItems(ids) — a user clicking Empty Trash after a bad scan permanently deletes the library rows.
  • PlayoutBuilder's PlayoutSkipMissingItems skip would empty every affected collection.

Suggested fix

A sanity guard before each sweep, e.g. refuse to flag when incomingItemIds.Count == 0 while existing.Count > 0, or when the missing fraction exceeds some threshold — log loudly and skip the sweep instead. Make the guard explicit rather than relying on an exception escaping. Worth also making a projection failure distinguishable from a deletion (count them, and skip the sweep if any occurred).

Done-when

  • A successful-but-empty fetch no longer flags a non-empty library
  • Guard is explicit and logged (not dependent on an exception unwinding)
  • Test in ErsatzTV.Scanner.Tests covering: empty incoming + non-empty existing → no flag writes; normal partial-deletion case still flags
  • Decided (and recorded) whether projection failures should also suppress the sweep
  • Applied to all three scanners (Television / Movie / OtherVideo)
  • Adversarial review passed
Found while working #473 (2026-07-19). Latent, but data-loss-adjacent — filing so it is not rediscovered the hard way. ## The gap All three media-server scanners reconcile with `existing.Except(incomingItemIds)` → `FlagFileNotFound...`: - `ErsatzTV.Scanner/Core/Metadata/MediaServerTelevisionLibraryScanner.cs:173-182, 369-371, 536-538` - `.../MediaServerMovieLibraryScanner.cs:205-207` - `.../MediaServerOtherVideoLibraryScanner.cs:212-214` If the remote fetch returns **successfully but empty**, `incomingItemIds` is `[]`, so `existing.Except([])` is *every item in the library* and all of them are flagged `FileNotFound`. There is **no** threshold/ratio check, no "abort if incoming count is 0", and no comparison against the server's reported total anywhere in the three scanners. The only guard present is the trivially-inverted one — `if (showItemIds.Count == 0) return [];` (`JellyfinTelevisionRepository.cs:365-368, 397-400, 429-432`) — which short-circuits when *nothing is missing*, not when *everything* is. ## Why a successful-but-empty fetch is reachable - `JellyfinApiClient.GetPagedLibraryItems` (`ErsatzTV.Infrastructure/Jellyfin/JellyfinApiClient.cs:369-404`) computes `pages` from `TotalRecordCount`; a `TotalRecordCount` of 0 yields `pages = 0` and a clean empty enumeration — no exception, so the sweep proceeds normally. A media server that is up but mid-restore / mid-library-rebuild / returning an emptied library is exactly this case. - Protection against a *mid-pagination* failure is incidental rather than defensive: there is no try/catch in that method, so the exception unwinds past the flag step. `MediaServerTelevisionLibraryScanner.cs:67-70` catches only `TaskCanceledException`/`OperationCanceledException`. It works, but by accident of control flow. - `ProjectToShow`/`ProjectToEpisode` failures are silently dropped by `.Somes()` (`JellyfinApiClient.cs:400-403`), so an item the server *did* return but ErsatzTV failed to project is indistinguishable from "deleted upstream" at the reconciliation step. A projection regression could therefore mass-flag healthy items. ## Blast radius `FileNotFound` is not itself destructive, but it feeds two consumers that are: - `EmptyTrashHandler` (`ErsatzTV.Application/Maintenance/Commands/`) searches `state:FileNotFound` (cap 10,000) and calls `_mediaItemRepository.DeleteItems(ids)` — a user clicking Empty Trash after a bad scan permanently deletes the library rows. - `PlayoutBuilder`'s `PlayoutSkipMissingItems` skip would empty every affected collection. ## Suggested fix A sanity guard before each sweep, e.g. refuse to flag when `incomingItemIds.Count == 0` while `existing.Count > 0`, or when the missing fraction exceeds some threshold — log loudly and skip the sweep instead. Make the guard explicit rather than relying on an exception escaping. Worth also making a projection failure distinguishable from a deletion (count them, and skip the sweep if any occurred). ## Done-when - [x] A successful-but-empty fetch no longer flags a non-empty library - [x] Guard is explicit and logged (not dependent on an exception unwinding) - [x] Test in `ErsatzTV.Scanner.Tests` covering: empty incoming + non-empty existing → no flag writes; normal partial-deletion case still flags - [x] Decided (and recorded) whether projection failures should also suppress the sweep - [x] Applied to all three scanners (Television / Movie / OtherVideo) - [x] Adversarial review passed
timothy added the bugpriority: medium labels 2026-07-19 20:59:32 +02:00
timothy added the in-progress label 2026-07-19 23:07:54 +02:00
timothy commented 2026-07-19 23:07:55 +02:00
Author
Owner
Copy Link
Copy Source

Claiming (Claude Code orchestrator session, 2026-07-19). Deterministic selector (scripts/select-queue.sh) ranked this behind #350 (skipped — its successor #472 is in-progress) and #474 (primary fix is a prod config/content change, deferred to operator/media-management scope). #477 is the lowest-numbered genuine, self-contained ErsatzTV code bug: data-loss-adjacent scanner reconciliation gap. Working it under the full ## Done-when (explicit+logged guard, tests for empty-incoming vs partial-deletion, all three media-server scanners, projection-failure decision recorded).

Claiming (Claude Code orchestrator session, 2026-07-19). Deterministic selector (`scripts/select-queue.sh`) ranked this behind #350 (skipped — its successor #472 is `in-progress`) and #474 (primary fix is a prod config/content change, deferred to operator/media-management scope). #477 is the lowest-numbered genuine, self-contained ErsatzTV **code** bug: data-loss-adjacent scanner reconciliation gap. Working it under the full `## Done-when` (explicit+logged guard, tests for empty-incoming vs partial-deletion, all three media-server scanners, projection-failure decision recorded).
timothy commented 2026-07-19 23:31:49 +02:00
Author
Owner
Copy Link
Copy Source

PR #483 open — implementation summary

Fix: shared MediaServerReconciliationGuard.ShouldFlagMissing skips (and logs a Warning) the file-not-found sweep when a successful fetch returns zero items while items exist locally. Wired into the three library-level sweeps (Television shows / Movie / OtherVideo).

Key subtlety — interaction with #476: an empty incoming set is indistinguishable at scan time from a mid-restore error (both report a zero total), so this deliberately overrides #476's degenerate "last item removed => empty incoming => flag" case. #476's cascade still fires for partial deletions (survivors present); its characterization test was moved from an empty incoming to a survivor+removed partial-deletion case so it still exercises the cascade.

Scope decisions (Done-when):

  • Applied to all three scanners (library-level sweeps). Nested TV season/episode sweeps left unguarded — bounded blast radius, legitimate per-parent-empty, and #476's descendant cascade already handles a removed parent.
  • Projection-failure suppression: decided to DEFER (recorded in docs/decisions.md). Distinguishing a silently-dropped projection failure from a real deletion needs a dropped-count threaded out of JellyfinApiClient — larger cross-layer change. Filed as follow-up #484 (with ratio-threshold). The deterministic zero-count guard has no false positives and covers the reported catastrophic case.

Tests: policy table + per-scanner integration (empty incoming flags/reindexes nothing); proven non-vacuous by neutralizing the guard. Full ErsatzTV.Scanner.Tests: 1481 passed; full solution build: 0 errors.

Incidental: filed #485 — .gitignore core silently ignores new files under any Core/ dir on macOS (bit this PR; force-added).

Awaiting CI + independent adversarial review before requesting merge.

## PR #483 open — implementation summary **Fix:** shared `MediaServerReconciliationGuard.ShouldFlagMissing` skips (and logs a Warning) the file-not-found sweep when a successful fetch returns **zero** items while items exist locally. Wired into the three library-level sweeps (Television shows / Movie / OtherVideo). **Key subtlety — interaction with #476:** an empty incoming set is indistinguishable at scan time from a mid-restore error (both report a zero total), so this deliberately overrides #476's degenerate "last item removed => empty incoming => flag" case. #476's cascade still fires for partial deletions (survivors present); its characterization test was moved from an empty incoming to a survivor+removed partial-deletion case so it still exercises the cascade. **Scope decisions (Done-when):** - Applied to all three scanners (library-level sweeps). Nested TV season/episode sweeps left unguarded — bounded blast radius, legitimate per-parent-empty, and #476's descendant cascade already handles a removed parent. - **Projection-failure suppression: decided to DEFER** (recorded in docs/decisions.md). Distinguishing a silently-dropped projection failure from a real deletion needs a dropped-count threaded out of `JellyfinApiClient` — larger cross-layer change. Filed as follow-up **#484** (with ratio-threshold). The deterministic zero-count guard has no false positives and covers the reported catastrophic case. **Tests:** policy table + per-scanner integration (empty incoming flags/reindexes nothing); proven non-vacuous by neutralizing the guard. Full `ErsatzTV.Scanner.Tests`: 1481 passed; full solution build: 0 errors. **Incidental:** filed **#485** — `.gitignore` `core` silently ignores new files under any `Core/` dir on macOS (bit this PR; force-added). Awaiting CI + independent adversarial review before requesting merge.
timothy removed the in-progress label 2026-07-19 23:53:54 +02:00
timothy commented 2026-07-19 23:54:19 +02:00
Author
Owner
Copy Link
Copy Source

Closed — merged to main via PR #483 (55be64bd)

What was done: added a shared MediaServerReconciliationGuard.ShouldFlagMissing that skips (and logs a Warning) the file-not-found sweep when a successful media-server fetch returns zero items while items exist locally. Wired into the three library-level sweeps (MediaServer{Television,Movie,OtherVideo}LibraryScanner).

Root cause: the sweeps reconcile deletions as existing.Except(incomingItemIds). A successful-but-empty fetch (server mid-restore / library emptied upstream) makes incomingItemIds empty, so existing.Except([]) is every item → the whole library flagged FileNotFound in one scan, which EmptyTrashHandler can then permanently delete and PlayoutSkipMissingItems empties. The only prior protection was an exception incidentally unwinding past the flag step — protection by accident of control flow, not design.

Files changed: MediaServerReconciliationGuard.cs (new) + guard call in the 3 scanners; MediaServerReconciliationGuardTests.cs, MediaServerMovieLibraryScannerTests.cs, MediaServerOtherVideoLibraryScannerTests.cs (new); MediaServerTelevisionLibraryScannerTests.cs (#476 test rewritten to a survivor+removed partial deletion); docs/decisions.md.

Key decision — #476 interaction: an empty incoming set is indistinguishable at scan time from a mid-restore error, so this deliberately overrides #476's degenerate "last item removed => empty incoming => flag" case. #476's cascade still fires for partial deletions (survivors present). Documented in decisions.md.

Deferred: nested TV season/episode sweeps left unguarded (bounded blast radius); ratio-threshold + projection-failure detection → follow-up #484.

Incidental: #485 filed for the .gitignore core entry that silently ignores new files under any Core/ dir on macOS (bit this PR; force-added).

Verification: 1481 scanner tests green, full solution 0 errors, tests proven non-vacuous via negative control, independent cold-context adversarial review returned MERGEABLE, CI green.

## Closed — merged to main via PR #483 (55be64bd) **What was done:** added a shared `MediaServerReconciliationGuard.ShouldFlagMissing` that skips (and logs a Warning) the file-not-found sweep when a *successful* media-server fetch returns zero items while items exist locally. Wired into the three library-level sweeps (`MediaServer{Television,Movie,OtherVideo}LibraryScanner`). **Root cause:** the sweeps reconcile deletions as `existing.Except(incomingItemIds)`. A successful-but-empty fetch (server mid-restore / library emptied upstream) makes `incomingItemIds` empty, so `existing.Except([])` is *every* item → the whole library flagged `FileNotFound` in one scan, which `EmptyTrashHandler` can then permanently delete and `PlayoutSkipMissingItems` empties. The only prior protection was an exception incidentally unwinding past the flag step — protection by accident of control flow, not design. **Files changed:** `MediaServerReconciliationGuard.cs` (new) + guard call in the 3 scanners; `MediaServerReconciliationGuardTests.cs`, `MediaServerMovieLibraryScannerTests.cs`, `MediaServerOtherVideoLibraryScannerTests.cs` (new); `MediaServerTelevisionLibraryScannerTests.cs` (#476 test rewritten to a survivor+removed partial deletion); `docs/decisions.md`. **Key decision — #476 interaction:** an empty incoming set is indistinguishable at scan time from a mid-restore error, so this deliberately overrides #476's degenerate "last item removed => empty incoming => flag" case. #476's cascade still fires for partial deletions (survivors present). Documented in decisions.md. **Deferred:** nested TV season/episode sweeps left unguarded (bounded blast radius); ratio-threshold + projection-failure detection → follow-up **#484**. **Incidental:** **#485** filed for the `.gitignore` `core` entry that silently ignores new files under any `Core/` dir on macOS (bit this PR; force-added). **Verification:** 1481 scanner tests green, full solution 0 errors, tests proven non-vacuous via negative control, independent cold-context adversarial review returned MERGEABLE, CI green.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ad-hoc
One-off / ad-hoc work not tracked by a dedicated issue
 api
REST API / HTTP endpoints
 bug
Something isn't working
 ci-cd
Build, test, deploy pipeline
 content
Channel content / schedules / playlists
 dependencies
Dependency updates (Renovate)
 enhancement
New feature or improvement
 frontend
ChicoryTV React SPA frontend
 in-progress
Claimed by an active session — do not pick up
 jellyfin
Jellyfin tuner / IPTV integration
 parked
Excluded from automatic queue pickup; work only when explicitly selected
 priority: high
priority: low
priority: medium
review
Adversarial review finding
 security
Security / vulnerability fix
No labels bug priority: medium
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate timothy (Timothy)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#477