feat(235): async-op API contract normalization + playout build observability + F9 scan endpoints #267
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: timothy/ersatztv#267
Reference in New Issue
Block a user
Closes #235 (reviewer#20 F7 + F8 + F9). Built in 4 parallel disjoint-file slices, merged back, one OpenAPI regen.
F8 — normalized async-op contract
Queue-triggering endpoints onto: 202 queued · 404 ProblemDetails (missing) · 409 ProblemDetails (lock held) · 422 ProblemDetails (domain precondition). (Trakt was the reference; #232/#215 already normalized library-scan + playout reset — this finishes the outliers.)
MaintenanceController.EmptyTrash(error)MaintenanceController.CleanArtworkLibrariesController.ScanShow{error}→ 202/404/409/422 (newQueueShowScanResultenum, incl. honestScanFailed→422)ChannelController.ResetPlayoutPlayoutController.ResetAllResetAllPlayoutsResponseModel(queued / skippedLocked / skippedUnsupported)TroubleshootController.TroubleshootPlaybackNotFound()→ 404/422 ProblemDetailsTroubleshoot: status codes the SPA HLS player depends on were preserved — verified
HlsPlayer.tsxnever branches on this endpoint's status (playback state comes from the separate/api/troubleshoot/playback/statuspoll); only the error body was enriched.Playout build observability
Added
isLockedto the single-playoutGET /api/playouts/{id}(PlayoutResponseModel) — the list endpoint already carriedisLocked+BuildStatus(#215). No dedicated push channel (the flag on the existing GETs is the HTTP-observable substitute, matchingGET /api/trakt/status).F9 — parity endpoints (the
Libraries.razordeletion gate; #202 did NOT close these)POST /api/libraries/{id}/scan?deep=false, threadedQueueLibraryScanByLibraryId(…, DeepScan=false)→ForceSynchronize{Plex,Jellyfin,Emby}LibraryById(id, deep)(was hardcodedfalse). Non-breaking.POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep=falseon the three #202 controllers →Synchronize{X}Collections(id, ForceScan:true, deep). Pre-checks existence (404), acquires the per-source collections lock (held = 409), enqueues → 202, compensating-unlock incatch(§3b);ScannerServicereleases infinally.scanLibrary(id, deep),scanCollections). The SPA deep-scan / collections buttons are the removal-PR's remaining parity work (parity doc §5) — API gate itself is now closed.F7 — Trakt batch-lock leak
The global Trakt lock released only on the terminal batch message; a
WorkerServiceshutdown before it leaked the lock permanently (same class as #231/#233/#234). Fix:WorkerServicereleases the Trakt lock in afinallyon read-loop exit if still held. Non-vacuous regression test (inverted-condition control).Docs / generated
docs/decisions.md: #235 entry incl. the accepted-by-design note (unbounded channels + no shutdown drain — in-memory locks die with the process; F7 covers the within-process leak).docs/api-conventions.md§3a/§3b,docs/blazor-route-parity.md§5 (F9 gate closed).v1.json+v1.d.ts+endpoint-index.md(158 endpoints / 243 ops);check:apiclean.Merged current
main(incl. #253 PR1 #263) in; regenerated artifacts post-merge.Verification
Full backend suite green (ErsatzTV.Tests 1324, Core 493, Scanner 1467, Infra 53, Arch 5, FFmpeg 20 — 0 failed); web 666 tests + typecheck + lint + build clean; no API drift. Live-E2E of the changed endpoints to follow.
🤖 Generated with Claude Code
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>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>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>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>Review round (two independent passes + Fable reconciliation + fix re-review)
Cold Claude fork: MERGEABLE. Codex: one High — the new
scan-collectionsendpoints acquire a per-provider collections lock and rely onScannerServiceto release it, butSchedulerServiceenqueued periodic collection scans unlocked while the scanner'sfinallyreleased the lock whenever held → a scheduled scan could cross-release an API scan's lock (#250 class; spurious 202 instead of 409; single-reader channel means no data corruption). The fork missed this — its "only the acquirer enqueues" premise didn't enumerate the scheduler path.Reconciliation (fork vs Codex disagreed → escalated to Fable per protocol): Codex correct. Fable also caught that the naive per-enqueue lock-or-skip would deterministically starve the 2nd+ media source, and prescribed the §C7 batch pattern.
Fix (
787058d1):Unlockflag (default true) on the threeSynchronize{X}Collectionsrecords;ScannerServicereleases onlyif (request.Unlock && …Locked());SchedulerServicelocks once per provider, enqueues every source withUnlock: isLast,catch-releases on enqueue-throw, skips the whole loop if already locked — mirrors the shipped library-scanUnlockprecedent. 8 new tests, non-vacuous (reverting only Plex failed exactly the 3 Plex tests). Plus (589c35d3) the Codex Low:QueueShowScanResultswitch made total (explicitNotFound+UnreachableExceptionfallback).Codex re-review of the fix commit (the #242 discipline — the fix is new code): FIX CORRECT, no actionable findings; confirmed no leak when a non-last scan throws (per-message catch keeps the loop alive → last message still releases), all-source batching, exactly one releaser, and controller/Blazor paths unaffected. Noted coverage nits (scheduler batch test Plex-only on symmetric code) — accepted.
Full backend suite green (ErsatzTV.Tests 1332), web 666, no API drift. CI arming on
589c35d3.