Codex-review Low: NotFound previously reached the catch-all by coincidence; a
future enum value would silently 404. Explicit arm + UnreachableException fallback
so an unmapped outcome fails loudly rather than mis-mapping to 404.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The new POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections
endpoints acquire a per-provider collections lock (409 if held) and hand the
single release to the ScannerService finally. But SchedulerService's periodic
collection scans were enqueued WITHOUT the lock, and ScannerService's finally
released the collections lock whenever held with no ownership check. A
scheduler-queued scan running while an API request held the lock cross-released
the API's lock (#250 bug class), letting a second API request get a spurious
202 instead of 409.
Fix (mirrors the SynchronizePlexLibraryByIdIfNeeded(Unlock: !networksFollow)
library-scan precedent):
- Add `bool Unlock = true` (4th positional param) to the three
Synchronize{Plex,Jellyfin,Emby}Collections records; default keeps the
controller + Libraries.razor call sites compiling and releasing on run.
- ScannerService: the three collection finallys now honor `request.Unlock`
(the concrete typed request is in scope in each method) so a batch member
with Unlock:false never releases a lock it doesn't own.
- SchedulerService: replace the unlocked per-source enqueue with a lock-once
per-provider batch — LockX Collections() once, enqueue each source with
Unlock:isLast (last message owns the release), compensating unlock in catch,
and SKIP the whole provider loop if the lock is already held. A naive
"lock-per-source, skip if held" would deterministically starve the 2nd+
source; lock-once-batch does not.
Tests (ErsatzTV.Tests/Services/): ScannerServiceCollectionLockTests drives the
real ScannerService read loop + real EntityLocker and asserts Unlock:false
leaves a held lock intact while Unlock:true releases (all three providers);
SchedulerServiceCollectionLockTests reflect-invokes ScanPlexMediaSources and
asserts it locks once + skips the enqueue when held, and hands the release to
the last message when acquired. Proven non-vacuous: reverting the Plex fix
fails exactly the three Plex tests.
No OpenAPI/v1.json change (internal channel-message record, not a DTO).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Closes the two F9 Libraries.razor parity gaps and normalizes scan-show error
mapping to ProblemDetails.
TASK 1 — library-wide deep scan:
- QueueLibraryScanByLibraryId gains optional `bool DeepScan = false`; handler
threads it into ForceSynchronize{Plex,Jellyfin,Emby}LibraryById.
- POST /api/libraries/{id}/scan?deep=false binds it via [FromQuery].
TASK 2 — external-collections scan (new endpoints):
- POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep=false
acquires the per-source collections lock (§3b: lock IS the running scan → 409),
enqueues Synchronize{X}Collections(id, ForceScan:true, deep) to the scanner
channel, returns 202; compensating-unlock on enqueue throw.
TASK 3 — scan-show normalization:
- New QueueShowScanResult enum; handler returns it instead of bool.
- POST /api/libraries/{id}/scan-show now maps 202/404/409/422 (all errors
ProblemDetails) instead of 200/404/400-anonymous-object.
- Updated the lone Blazor caller (TelevisionSeasonList.razor).
Tests: LibrariesController (scan deep=true, scan-show enum→status), the three
media-source controllers (scan-collections route/404/409/202/compensating-unlock),
and handler tests for both changed handlers (deep threading + show-scan outcomes).
Docs: api-conventions §3b exemplar + blazor-route-parity §5 F9 gate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Slice C of the async-op contract normalization:
- channel reset (POST /api/channels/{channelNumber}/playout/reset) now
returns 202 Accepted (was 200 Ok) — it only queues a background rebuild
- reset-all (POST /api/playouts/reset-all) still 202 but now returns a
ResetAllPlayoutsResponseModel body reporting QueuedPlayoutIds /
SkippedLocked / SkippedUnsupported instead of silently swallowing skips;
handler returns a new ResetAllPlayoutsResult record
- single-playout GET (GET /api/playouts/{id}) now exposes IsLocked on
PlayoutResponseModel, set from IEntityLocker.IsPlayoutLocked mirroring
the list projection — gives a polling client the lock flag
Tests: channel reset asserts 202; reset-all asserts 202 + skipped-body
shape; single GET asserts IsLocked; new ResetAllPlayoutsHandlerTests
(in-memory SQLite) asserts locked/ExternalJson/None land in skipped lists
and eligible playouts in queued. docs/api-conventions.md §3a updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The global Trakt lock is acquired by SchedulerService.RefreshTraktLists /
MatchTraktLists (and TraktController) and released only when the *terminal*
message of a batch — the one carrying Unlock: true (list == traktLists.Last())
— is processed by WorkerService, whose handler (AddTraktListHandler /
MatchTraktListItemsHandler) calls IEntityLocker.UnlockTrakt() in a finally.
WorkerService.ExecuteAsync breaks out of the read loop on
stoppingToken.IsCancellationRequested (and exits on channel completion /
reader cancellation) BEFORE processing the next message. If shutdown lands
after a batch is enqueued but before its terminal Unlock: true message is
handled, UnlockTrakt() never runs and the in-memory Trakt lock leaks for the
rest of the process lifetime (subsequent Trakt operations 409 forever).
Fix (option a): make the batch-release loss-tolerant with a compensating
release in a finally around the read loop — if the Trakt lock is still held
when the worker stops, release it. Chosen over tracking pending ownership
(b) because the lock is a global singleton and WorkerService is its sole
batch-release site, so "held at shutdown" unambiguously means "the terminal
release was lost"; covers all three exit paths (break / channel completion /
cancellation) in one place. Same lock-lifecycle class as #231/#233/#234.
Regression test: WorkerServiceTests gates the first (non-terminal) batch
message on the stopping token, then StopAsync-cancels so the worker breaks
before the terminal Unlock: true message — asserts the lock is released and
the terminal message was never processed. Proven non-vacuous: inverting the
finally condition fails the test.
Backend-only; no controller/DTO/SPA/OpenAPI impact.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Slice A of the async-op API contract normalization.
MaintenanceController:
- EmptyTrash error path: was 500 text/plain (error.ToString()); now maps the
BaseError Left through ApiResults.ToErrorResult() -> 404 (NotFoundError) / 422
ProblemDetails. Success stays 200 OkResult. Added ProducesResponseType 200 + 422.
- CleanArtwork: fire-and-forget enqueue of DeleteOrphanedArtwork was a silent 200;
now returns 202 Accepted (AcceptedResult) since it queues background work.
Added ProducesResponseType 202. (Controller does not derive from ControllerBase,
so results are built directly as before.)
TroubleshootController.TroubleshootPlayback (GET|HEAD /api/troubleshoot/playback.m3u8):
- Two bare body-less NotFound() call sites conflated "not found" with "prepare/
playback failure". Both now return a ProblemDetails body:
* prepare-failure (result.IsLeft): mapped through error.ToErrorResult() -> 404 for
NotFoundError (unknown media item/channel) else 422 for a validation BaseError.
* terminal fall-through (prepare ok but no playable output): kept 404 with a
distinguishing ApiResults.NotFoundProblem(...) detail.
- Added ProducesResponseType 404 + 422 (409 already present).
Consumer check: the SPA (PlaybackTroubleshootingScreen) feeds the playback.m3u8 URL
straight to hls.js via HlsPlayer, which never inspects the HTTP status code — playback
state is surfaced via the separate /api/troubleshoot/playback/status poll. So the
404->422 split for the validation subcase is safe; no player code branches on the
status code.
Tests: MaintenanceControllerTests (200/422/202 + enqueue assertion),
TroubleshootControllerTests (prepare 404 NotFoundError, 422 validation). All green;
Api error-metadata/contract/security scans still pass.
Note: OpenAPI artifacts (v1.json / v1.d.ts) intentionally NOT regenerated here — the
orchestrator regenerates once after all #235 slices merge.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
timothy
