Clears the still-live findings from #172 (verified against main; #2/#4/#7 and the
auth/search/Trakt tail were already deliberate-documented or fixed since 2026-07-07).
- Null/empty Name → 500 (10 create/replace handlers). Block/Template/DecoTemplate/Deco
Create+Replace/Update + UpdateFFmpegProfile did `request.Name.Length > 50` on a
client-nullable string → unhandled NullReferenceException → HTTP 500 (no global
exception filter). Now `string.IsNullOrWhiteSpace(request.Name) || .Length > 50` →
422; also rejects empty/whitespace names, matching the group-create handlers'
NotEmpty behavior. CreatePlaylist coalesces null→"" at the DTO so it was an
empty-name persist, not a 500; guarded the same way.
- ReplaceTemplateItems overlap validation iterated with an `item == otherItem`
record value-equality skip, so two exact-duplicate items were value-equal and
bypassed the intersection check (both persisted). Now index-based (i != j) so
duplicates register as a self-intersection and are rejected 422.
- Trimmed the unreachable 404 ProducesResponseType from POST /api/blocks/groups and
POST /api/templates/groups (a create has no parent lookup that can 404); v1.json
regenerated.
- Regression tests: all 10 name-guard paths + the duplicate-items path (19 cases).
- Docs: decisions.md entry + api-conventions.md §3b null-safe-validation bullet.
fixes#172
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Version every /api route to /api/v1 (251 controller routes + ~24 Location
headers + the scanner callback URL + the Startup request-log literal),
uniform across the machine API, auth, scanner and scripted-build surfaces.
Add ApiVersionRewriteMiddleware: a legacy unversioned /api/* request is
rewritten (NOT redirected) to /api/v1/* in-pipeline — method, body, auth
headers and query survive — carrying RFC 8594 Deprecation/Sunset headers,
so curl / the future MCP server / bookmarks keep working. An already-
versioned path passes through; a future /api/v2 is never forced to v1.
Standardize the route convention (leading-slash absolute route per method,
no class-[Route] — except the two Scanner/Scripted controllers whose ~all
actions share a parametrized {id} prefix), enforced by ApiRouteVersioningTests
(^/api/v\d+/ over the whole Controllers.Api surface; browser-nav
/auth/oidc/login is out of scope).
Regenerate v1.json (160 paths, all /api/v1)/endpoint-index/v1.d.ts; sweep 945
SPA request literals + the test mocks (regex + positional URL parsers). /api/v1
is additive-only after freeze; the legacy-rewrite shim sunsets in ~2 releases
(owner decision) with removal tracked as a Phase-3 follow-up.
Docs: decisions.md 2026-07-13, api-conventions §1/§9, rest-api/spa-conventions/
blazor-route-parity/e2e-local/domain-model.
fixes#286
refs #197
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The shared optimistic-concurrency parser (ConcurrencyHeaders.ParseIfMatch) classified any
non-canonical/weak/list If-Match value as Malformed → 400. Per RFC 7232 §3.1 a syntactically
-valid entity-tag that simply doesn't strong-match must be 412; 400 is only for a genuine
grammar violation.
- Rewrite ParseIfMatch as a real RFC 7232 entity-tag/list parser: walks the comma-separated
1#entity-tag list, validates each [W/]DQUOTE *etagc DQUOTE member, and collects the strong
members whose opaque text is our canonical decimal. Weak / empty / non-canonical /
out-of-range tags are valid but contribute no version (→ empty set → 412); genuine grammar
violations (unquoted, SP-in-tag, unterminated, garbage) → 400.
- Reshape IfMatchCondition.ExpectedVersion : Option<int> → ExpectedVersions : Option<Seq<int>>
and VersionedAggregateExtensions.CheckVersion → set membership (any strong match proceeds;
empty set always 412). Threads through 10 replace/update commands + handlers + request
mappers + 9 controllers.
- No wire-contract change (400 + 412 already declared on every PUT; the field is header-derived
and internal — no DTO/route/response-type/OpenAPI change).
- Tests: ConcurrencyHeadersTests rewritten for the new classification (lists, weak, empty,
non-canonical → Version/empty-set; grammar violations → Malformed) + new
VersionedAggregateExtensionsTests for CheckVersion membership/empty-set/force-write.
- Docs: api-conventions.md §7a rewritten; decisions.md entry appended.
Refs #253#197fixes#265
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- GET /api/graphics-elements no longer side-effects; refresh moved to
POST /api/graphics-elements/refresh (204), closing a CSRF vector on a GET.
- PrepareTroubleshootingPlaybackHandler now returns a typed LockedError from
both atomic lock-acquire failures; ApiResults.ToErrorResult maps it to 409
instead of falling through to 422, so a lock lost in the race between the
controller's pre-check and the handler's atomic acquire still reports 409.
- AuthController.MachineKey sets Cache-Control: no-store + Pragma: no-cache
on the 200 response carrying the master API key.
- Reworded the stale "subtitleId query parameter" endpoint description now
that playback/start takes a JSON body.
- Regenerated openapi/v1.json + docs/endpoint-index.md; docs/api-conventions.md
updated with the LockedError pattern (§3a) and the ToErrorResult table row.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Independent Codex review (reconciled by Fable against a MERGEABLE fork verdict) found
SaveChangesForcingVersion silently DROPPED a pending Version++ under a concurrent
versioned-write race: on DbUpdateConcurrencyException it adopted the DB's current
Version verbatim (original = current = dbVersion), so a bumping sibling committed at
dbVersion instead of dbVersion+1. Net: an editor holding the concurrent writer's ETag
was never invalidated by the sibling's change — the exact lost-update the #253/#269
contract exists to close, lost under the very condition the helper handles.
F1 fix (shared helper, corrects all 25 bumpers incl. the pre-existing Add*ToPlaylist /
schedule-item writers): rebase the pending delta on top of the stored token —
pendingDelta = current - original; original = dbVersion; current = dbVersion + pendingDelta
Bumpers (delta 1) advance to dbVersion+1; non-bumpers/deletes (delta 0, e.g.
ErasePlayoutHistory) still adopt the stored token unchanged, so RootWriterForceVersionTests
is unaffected. Idempotent across the bounded retry loop.
F3: the force-race tests now assert Version==3 (rebase), not just membership survival;
added the missing Playout force-race+rotate test. Negative-controlled: with the helper
fix reverted, both strengthened tests go red.
F2 (Medium, deferred → #308): two concurrent same-item Add*ToCollection can both pass the
membership check and the loser 500s on the composite-PK violation (DbUpdateException, which
the helper doesn't catch). Pre-existing and narrow (no corruption); doc claims softened to
name it. Filed #308.
Docs: api-conventions §7a + decisions.md prose corrected from "adopt the stored token" to
the rebase semantics.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Complete the #253 optimistic-concurrency contract's cross-editor ETag
rotation tail. The non-If-Match config siblings mutated editor-visible
state without bumping Version, so a concurrent editor of the same root
never invalidated. Now the Collection Add*/Remove handlers bump
Collection.Version, and UpdateCollection / UpdatePlayout / the three
ScheduleFile writers (which already force-wrote past a concurrent bump)
now bump too — all via SaveChangesForcingVersion (no If-Match → force
write, never 412/500).
No-op idempotence (Fable-caught trap): these gate reindex/BuildPlayout
fan-out on SaveChanges()>0, so an unconditional bump would fire spurious
rebuilds on an idempotent re-add / same-value re-submit. Each now
short-circuits a genuine no-op before the bump — Add handlers by an
explicit membership check (also fixing a latent duplicate-CollectionItem
insert), scalar writers by ChangeTracker.HasChanges().
Corrects #269's framing: the Add*ToCollection family is not
repository-mediated (IMediaCollectionRepository is read-only); each
handler writes via its own dbContext, so the scanner's separate
membership path is unaffected (a background scan does not rotate the
editor ETag).
Tests: CollectionEtagRotationTests + PlayoutScheduleFileEtagRotationTests
(rotation, no-op-without-bump-or-rebuild, force-write-past-concurrent-bump),
no-op guard proven non-vacuous by inverting the membership check.
Docs: api-conventions §7a + decisions.md. No new status codes / no
OpenAPI change (these endpoints take no If-Match, never 412).
The #265 RFC-7232 If-Match parser refinement is a separate PR.
fixes#269
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a blocking `api-docs` CI job: when a PR diff touches the API surface
(ErsatzTV/Controllers/Api/** or ErsatzTV.Core/Api/**) it rebuilds the
generated artifacts from source — v1.json, v1.d.ts, endpoint-index.md —
and fails if any is stale in the diff. Mechanizes the "docs-update in the
same PR" rule for the API contract (docs-reminder stays a non-blocking
route-parity nudge).
Path-gated INSIDE the job (per-step `if:` on a detect output), not via a
top-level `if:`, so the check always reports a status on every PR and is
safe as a required check: API-free PRs skip the dotnet/node setup + regen
and pass trivially.
Verified the gate reproduces the committed baseline: a fresh build
regenerates v1.json byte-identical to HEAD (incl. all 244 auth
security/401 blocks). The only footgun is local — update-openapi.sh runs
dotnet-getdocument against the already-built assembly, so a stale bin/
emits a stale spec; api-conventions.md §5 now flags "build first". CI is
immune (fresh checkout has no bin/).
Docs: api-conventions.md §5 (two-place CI enforcement + stale-assembly
note), decisions.md (new entry). Refs #303.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fix-commit re-review (cold fork) caught that e383c253 tracked a machine-specific
absolute-path symlink `web/node_modules -> /Users/.../web/node_modules` (created
for the eslint pre-push hook). It slipped past `.gitignore` because the
`web/node_modules/` trailing-slash pattern matches only a directory, not a
symlink; a real node_modules dir (a copy) would have been ignored. Untrack it and
tighten the ignore to `web/node_modules` (matches symlink or dir) so it can't
recur.
Also tightens the §7a / decisions.md "they already catch" phrasing (LOW review
nit): only BuildPlayoutHandler catches; PlayoutTimeShifter is insulated by running
solely on the background worker, never the request path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Independent adversarial review (cold fork + Codex) of the first cut converged
on one real miss and two boundaries to document.
- **ErasePlayoutHistoryHandler** (HIGH, both reviewers): modifies Playout ROOT
scalars (Seed/Anchor/OnDemandCheckpoint) *without* bumping Version, inside an
explicit transaction with no try/catch, so it 500s on a concurrent bump —
reachable via POST /api/playouts/{id}/erase-items-and-history. My first sweep
filtered on "Version-bumpers + deletes"; the true exposure surface is "any
handler leaving a versioned root Modified/Deleted", so this slipped through.
Now routes through SaveChangesForcingVersion (+ a non-vacuous through-handler
test that exercises the explicit-transaction path). Re-swept with the correct
filter: ErasePlayoutItems (AsNoTracking + ExecuteDelete children only) and
ResetAllPlayouts (read-only + enqueue) are NOT exposed.
- **Background build/time-shift Playout-scalar writers** (BuildPlayout via
PlayoutBuilder, PlayoutTimeShifter): token-guarded too, but intentionally left
on plain save — they already catch (build-failure, not 500), and force-writing
would persist output built from stale config (the concurrent config bump already
enqueues a rebuild). Documented as a deliberate boundary, not a gap.
- **Item-add index collision** under force-write: documented as an accepted
Phase-1 effect (non-corrupting, self-correcting; reload-recompute refinement
is a #197 candidate).
Also corrects the docs' "every Version bumper" framing to the true filter and the
test docstring's over-broad non-vacuity claim. Full ErsatzTV.Tests green (1483).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fix-commit re-review confirmed the 1st-round fixes resolved and caught a 2nd round:
- HIGH — env-seed vs. setup race: an attacker could claim admin in the startup
window before LocalAdminSeedService runs, and the seed's insert would then be
swallowed (attacker credential persists, defeating env recovery). Fixed
structurally: the setup-claim endpoint is CLOSED (409) whenever
Auth:LocalAdmin:Password is configured — the env seed owns the credential, so
there's no claim to race (also strengthens the setup-claim TOFU posture).
Config.setupRequired reflects it.
- LOW — a concurrent setup race-loser now returns 409 (not 422); ClaimLocalAdmin's
DbUpdateException catch re-checks existence and rethrows genuine/transient DB
errors instead of masking them as "already configured".
- MEDIUM (accepted, documented) — two simultaneous authenticated password changes
are a non-serializable lost-update; accepted for a single-admin system
(self-healing via re-login, implausible timing).
+3 AuthController tests (env-seed closes setup / setupRequired gating). Full
ErsatzTV.Tests green (1506); no generated drift. Docs updated.
Refs #295
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Independent review (cold fork = MERGEABLE-WITH-NITS; Codex = BLOCKED, caught
concurrency defects the fork missed). All actionable findings folded in:
- HIGH (Codex) atomic first-claim-wins: ClaimLocalAdmin now writes the three
credential rows in ONE transaction guarded by the unique ConfigElement.Key
index (lost race -> DbUpdateException -> 409), so concurrent claims can't
produce a mixed-state credential.
- HIGH (Codex) consistent login snapshot: VerifyLocalAdminLogin reads hash+stamp
in one query and drops rehash-on-verify, so a login racing a password change
can't capture a stamp newer than the hash it verified (concurrent change ->
old password fails, or the issued cookie carries the pre-change stamp ->
revoked next request).
- MEDIUM (Codex) env-seed migration race: LocalAdminSeedService is now a RunOnce
BackgroundService that awaits SystemStartup.WaitForDatabase (the migrator is a
BackgroundService; registration order didn't guarantee the schema) + try/catch.
- MEDIUM (fork M1) ForwardedHeaders: reverted the strict-opt-in flip — it would
regress /iptv M3U/XMLTV/HLS absolute-URL generation (Request.Scheme) behind a
proxy without KnownProxies. Kept #285 behavior; KnownProxies still recommended.
- LOW (Codex/fork) require X-CSRF on /api/auth/logout + /password (the
[SkipApiAuthorization] surface isn't covered by the filter's CSRF check;
closes forced-logout CSRF).
- ChangeLocalAdminPassword also writes hash+stamp atomically. Input length caps
on username/password.
Deferred with a tracked gate: MEDIUM (Codex) side-effecting [RequiresAuthentication]
GETs (troubleshoot playback/archive) aren't CSRF-covered -> #301, gates PR2
(latent in PR1: the SPA still uses the machine key).
Verify: full ErsatzTV.Tests green (1501); no OpenAPI/generated drift. Docs updated
(api-conventions §9, decisions.md).
Refs #295#301
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Activating #253's `Version` as an `IsConcurrencyToken` made EF guard every
UPDATE *and DELETE* of a versioned root with `WHERE Version=@orig`, so any
writer outside the If-Match contract that saves via plain `SaveChangesAsync`
throws an unhandled `DbUpdateConcurrencyException`->500 when a replace-all
editor bumps the row in its narrow load->save window (ordinary two-tab UI).
A completeness sweep (grep every `Version` bumper + every root delete, not
just the handlers PR3's close note named) found 17 exposed writers, all now
routed through `ConcurrencyExtensions.SaveChangesForcingVersion` (Phase-1
force-write: adopt the stored token and retry; rethrow only on genuine
row-deletion):
- 9 versioned-root delete handlers (a delete has no ETag to rotate -> force
only, no bump)
- UpdateProgramScheduleHandler (bumps then saved plainly - the ProgramSchedule
case PR3 only suspected; its post-commit query/enqueue also moved to
CancellationToken.None per section 7b)
- 7 item add/remove bumpers PR2 left on plain save:
Add/DeleteProgramScheduleItem + Add{Items,Movie,Show,Season,Episode}ToPlaylist
Force-write (not 412) is correct: these endpoints take no If-Match, so an
unconditional delete/edit should win. No API contract change (no new response
codes) -> no OpenAPI regen.
Still deferred to #197 (cross-editor ETag rotation only, not a 500): the
non-bumping config siblings + the scanner-shared Add*ToCollection family.
Tests: RootWriterForceVersionTests races a bump *through the handler* via a
pre-tracked context (non-vacuous - reverting a handler to plain save fails the
test, verified) for the Option-delete / Either-delete / bump+update shapes,
plus the genuine-conflict rethrow branch and an explicit negative control
proving the plain-save path throws. Full ErsatzTV.Tests green (1482).
Also strips a pre-existing UTF-8 BOM from the touched handlers to satisfy the
.editorconfig `charset=utf-8` rule the pre-commit format hook enforces.
Docs: api-conventions section 7a (fan-out completeness) + decisions.md entry.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Implements the ratified #295 design (PR1, server-only, backward compatible). The
/api surface now accepts a valid X-Api-Key (machine) OR an authenticated session
(browser cookie, local login or OIDC), gated by the evolved ApiAuthorizationFilter
(renamed from ApiKeyAuthorizationFilter; same fail-closed EndpointRequiresKey
predicate). Machine/key behavior is byte-identical and the SPA keeps working via
its stored key — the SPA login flow lands in PR2.
- ApiAuthorizationFilter: key-first (CSRF-immune) then session; session-authed
mutations require the X-CSRF header (403 otherwise). Attributes renamed
[RequiresApiKey]->[RequiresAuthentication], [SkipApiKeyAuthorization]->[SkipApiAuthorization].
- Cookie scheme ctv-session always registered (Lax/SameAsRequest/14d sliding, 401 not
redirect for /api); OIDC handler revived when configured (profile scope, userinfo,
auth-method claim); UseAuthentication/UseAuthorization/UseRateLimiter revived in the
legacy MapWhen branch.
- Local admin = single credential in ConfigElement rows (username / PBKDF2 hash via
Microsoft.Extensions.Identity.Core / rotating security stamp) — NO DB migration.
Password change rotates the stamp; CookieSecurityStampValidator revokes stale local
sessions. Env-seed recovery (Auth:LocalAdmin:*) via LocalAdminSeedService.
- AuthController /api/auth/{config,session,setup,login,logout,password} + browser-nav
GET /auth/oidc/login; excluded from OpenAPI (machine-audience spec). Per-IP rate limit
on login/setup/password; dummy-hash verify (no user enumeration).
- ForwardedHeaders now strict opt-in: X-Forwarded-* ignored unless KnownProxies/Networks
configured (rate-limiter IP + cookie-Secure integrity). Deployment: operators behind a
proxy must set ForwardedHeaders:KnownProxies.
- Tests: session/CSRF filter cases + 17 Application/Auth handler tests; full ErsatzTV.Tests
green (1499). No OpenAPI/generated-artifact drift.
- Docs: api-conventions section 9 rewritten; decisions.md entry (supersedes #206 inert-OIDC note).
Refs #295#197#206#58
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add GET /api/media-sources/collections-scan-status (MediaSourcesController →
GetCollectionsScanStatus handler) reporting which media-source families
(plex/jellyfin/emby) currently hold their external-collections scan lock,
reading IEntityLocker.Are{X}CollectionsLocked(). The lock is family-global
(no source id) and boolean (no percent), so the DTO carries just {family} and
returns only active families — the counterpart to GET /api/libraries/scan-status.
SPA: useCollectionsScan now polls this endpoint and reconciles optimistic
pending against the active-family set (seeding on mount so an in-progress scan
disables buttons immediately), using the same grace-tick helper as library
scans (now generic over the pending key type). Drops COLLECTIONS_PENDING_TIMEOUT_MS
— a long deep scan no longer re-enables the button early, and a fast scan no
longer wedges it disabled for the full timeout. A row shows Scanning when its
family is active or it has an in-grace optimistic pending key.
Tests: handler (3), controller route+delegation (2), SPA api fn + hook reconcile
(mount-seed / 202-promote / 409-keeps-disabled / 404-error). OpenAPI + TS types
regenerated. Docs: api-conventions §3b, blazor-route-parity §5, decisions.md.
Unblocks #91b (arc item 4): Libraries.razor's collections-scan affordance now
has full authoritative parity.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
S4 stored-XSS + S9 upload-size DoS from the #197 cold API review.
The artwork path trusted client-supplied content types at both ends: upload
validated only the declared multipart Content-Type (never decoded the bytes),
and serving reflected a client `?contentType=` straight into the response
Content-Type on unauthenticated GET sinks (/iptv/logos, /artwork/watermarks).
Chain: upload <script> bytes as image/png -> GET ...?contentType=text/html
serves them as HTML in-origin. nosniff (#279) does not help because the server
explicitly declares text/html.
- Upload: derive the content type from the bytes via SkiaSharp SKCodec
(header-only, no decode -> no decompression-bomb path); reject non-images 422.
New ErsatzTV.Core/Images/ImageContentTypes as the single allow-list source.
Dropped the untrusted declared Content-Type from the UploadArtwork command.
- Serve: removed the ?contentType= reflection structurally -- dropped ContentType
from GetCachedImagePath and the [FromQuery] binding on GetImage/GetWatermark;
the handler always sniffs the file, defaulting application/octet-stream.
ArtworkContentTypeModel.UrlWithContentType is now the bare path; SPA previews
no longer append the query.
- Defense-in-depth: channel-logo / watermark {path, contentType} DTOs run through
ArtworkContentTypeModel.Sanitized(), blanking non-allow-listed types on write.
- S9: Kestrel MaxRequestBodySize from ETV_MAXIMUM_UPLOAD_MB rejects oversized
bodies during read (controller file.Length check kept as friendly-error backstop).
Both serve sinks are IgnoreApi, so no OpenAPI change. Tests: byte-sniff accept/
reject, Sanitized() allow-list, Location no longer carries ?contentType=.
Docs: api-conventions §4a + decisions.md 2026-07-12.
Refs #283#197#66
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the id-based reconcile tests to ReplaceProgramScheduleItemsReconcileTests:
reorder moves state with the logical item (the non-vacuous core — proven to fail
under forced-positional), insert-in-middle, delete-unreferenced, unknown-id→422,
duplicate-id→422, and stale-version+unknown-id→412 (412 precedes 422, §7c). The
GET→map→PUT lossless round-trip now round-trips r.Id so it exercises id-mode.
Threads the new int? Id through all command/wire construction sites in tests.
Docs: api-conventions §7c (stable child identity + the deliberate #2-#5 positional
asymmetry) and a decisions.md entry.
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>
Backend mutation-hardening cluster from the 2026-07-11 mutation-safety audit
sweep (adversarial-reviewer #22/#23), the parallel-safe backend-isolated slice.
audit#22 F4 — standardize post-commit enqueues on CancellationToken.None:
20 command handlers under MediaCollections/, ProgramSchedules/, Playouts/,
Channels/ threaded the request cancellationToken into work that runs AFTER
SaveChangesAsync commits (WriteAsync rebuild/refresh enqueues, mediator.Publish,
reindex, cache Refresh, and post-commit lookups that gate an enqueue). A late
client-disconnect then turns an already-durable commit into a thrown request AND
drops the side effect. Generalizes the #251 deco-handler fix. Excludes
BuildPlayoutHandler (worker/background token, not a client-disconnect token),
the config/FFmpeg multi-upsert handlers (partial-commit case, separate
follow-up), and response-projection reloads (correctly keep the request token).
audit#22 F2 — DeleteChannelHandler/DeletePlayoutHandler now delete the channel
guide {number}.xml through IFileSystem.File.Delete (observable under
MockFileSystem) and BEFORE the commit (a post-commit delete orphans the xml on a
crash; the xml is regenerable on demand, so pre-commit delete is the safe order).
audit#23 F4 — ReplacePlayoutAlternateScheduleItemsHandler rejects an empty item
list in the handler (not only the controller pre-guard) so a direct caller can't
trip the Max()-on-empty crash.
Docs: api-conventions.md §7a (post-commit token convention + boundaries),
decisions.md entry (rationale, sweep scope, #253 PR2-4 coordination note).
Tests: guide-cache-delete-through-FS for both delete handlers, empty-list guard.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codex re-review of the fix commit confirmed both prior findings resolved and raised one
new Medium: RFC 7232 would 412 (not 400) a syntactically-valid but non-matching If-Match
(non-canonical "03", weak W/"3", tag lists, empty, overflow). Deferred to #197 (cold
contract pass) as #265 — fail-safe today (the mutation is rejected, never applied) and no
first-party client is affected. Records the deferral where the #253 fan-out will copy the
parser: a code comment in ConcurrencyHeaders + a note in api-conventions §7a.
Refs #253#265
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the shared optimistic-concurrency contract so a stale second tab can no longer
silently overwrite a fresher edit. PR1 lands the infra + the Block reference aggregate;
PRs 2–4 fan the same recipe across the other 8 roots (design: #253#issuecomment-8472).
Contract
- `IVersionedAggregate` (`int Version`) on all 9 replace-all roots (ProgramSchedule,
Block, Template, DecoTemplate, Playlist, Collection, Playout, MultiCollection,
RerunCollection), EF-mapped `.IsConcurrencyToken()`; one dual-provider migration
`AddAggregateVersions` (nullable:false, default 0).
- Strong `ETag` of `Version` on the aggregate GET; `If-Match` on the PUT; mismatch →
412 (distinct from the §3a 409 build-lock guard). Successful PUT returns the new ETag.
- `PreconditionFailedError : BaseError` → 412 in `ApiResults.ToErrorResult`;
`ConcurrencyHeaders.ParseIfMatch/SetETag`; malformed If-Match → 400; `*`/absent =
Phase-1 force-write.
Block reference wiring
- Handler: standalone `Either` via `CheckVersion` AFTER validation (never through
`Apply`, which Join()-flattens the subtype to 422), unconditional `Version++`,
`SaveChangesWithConcurrencyGuard` backstop (DbUpdateConcurrencyException → 412).
- `BlockViewModel.Version` (header-only, not echoed in the body); controller sets the
ETag on GET items and on the successful PUT.
- SPA: `client.requestWithMeta` seam; `blocks.getBlockItemsWithMeta` + `replaceBlock`
If-Match/ETag round-trip; `BlockEditor` holds the ETag, sends If-Match, and on 412
opens a blocking "changed elsewhere — reload" dialog.
Tests
- Handler contract tests: stale-If-Match → 412 (no mutation), matching/absent → success
+ bump, no-op save still bumps, and a two-context racing save → 412; proven
non-vacuous (drop `.IsConcurrencyToken()` → the race test fails).
- Controller tests: malformed If-Match → 400, If-Match threaded to the command, ETag on
GET/PUT, 412 passthrough. SPA: requestWithMeta ETag, replaceBlock If-Match, 412 dialog.
Docs: api-conventions §7a, spa-conventions §4a, domain-model glossary, decisions log.
Refs #253
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Scan queue handler now returns a QueueLibraryScanResult enum
(Queued|NotFound|SyncDisabled|AlreadyScanning) instead of a lying bool;
LibrariesController.ScanLibrary maps them to 202/404/422/409 with ProblemDetails.
Guard the lock->enqueue with the EnqueueWithTraktLock compensating-unlock pattern.
ScannerService now releases every library/collection lock in a finally so a handler
exception can't leak the lock. Plex "Shows" scheduler batch (one lock, two messages)
now has only the trailing SynchronizePlexNetworks carry the single release
(Unlock flag), mirroring the scheduler Trakt tail-token precedent.
Guard the other lock->enqueue producers (Create/UpdateLocalLibrary, UpdateTraktList)
with compensating unlock. SPA drops the PENDING_GRACE_TICKS heuristic now that the
POST reports 202/409/404/422 directly: 202 -> pending+poll, 409 -> reconcile (no
error toast), 404/422 -> surface error.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bug 1 (500 on watermark/graphics save): Replace/Add handlers projected the
freshly-built entity graph, whose ProgramScheduleItemWatermark / -GraphicsElement
join rows carry only foreign-key ids — the Watermark/GraphicsElement navs are null,
and Mapper.ProjectToViewModel dereferences them unguarded, throwing an NRE that the
controller surfaced as a 500 on PUT/POST. Both handlers now reload the persisted
item(s) through the read-side include chain before projecting. Extracted that chain
into ProgramScheduleItemQueryExtensions.IncludeScheduleItemDetails() so GET, Replace
and Add share one source of truth.
Masking: PersistItems returned a lazy LanguageExt Map, and the existing round-trip
test only checked .IsRight — never enumerating it, so the deferred NRE never fired.
The new ScheduleItemWriteProjectionTests force enumeration (as the controller's
.ToList()/serialization does) and seed watermark/graphics via a separate context so
the handler's fresh factory context has nothing pre-tracked.
Bug 2 (server side): GetProgramScheduleItemsHandler now .OrderBy(i => i.Index) —
it previously returned id order, which is not index order.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Blazor parity for the remaining #213 conveniences:
- GET /api/logs gains sortField (timestamp|level) and sortDirection
(asc|desc) query params, allow-listed and normalized (unrecognized
values fall back to the pre-existing timestamp-desc default) rather
than rejected with a 422. LogsScreen.tsx renders clickable, sortable
column headers with a chevron direction indicator.
- LogsScreen.tsx now persists the chosen page size to localStorage
(ctv-logs-page-size) and restores it on mount, following the
existing designSystem.ts localStorage-preference pattern. This is a
client-local UI preference, not the Blazor ConfigElement-backed
server setting — see docs/decisions.md.
- TrashScreen.tsx adds a per-kind "See all N ..." affordance that
pages past the 100/kind /api/search cap using the already-paginated
GET /api/library/browse (mediaType + pageNum), appending results
client-side. No new API surface was needed since that endpoint
already supports the paging the trash screen needed.
docs/decisions.md, docs/blazor-route-parity.md, docs/spa-conventions.md
and docs/api-conventions.md updated in this same commit. OpenAPI spec
regenerated (v1.d.ts unchanged: query params aren't part of the
generated components/schemas surface).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adversarial review finding 1 on PR #225: decisions.md and api-conventions §3a
read as if the race were eliminated; the guard only narrows it (a queued build
can acquire the lock after the check passes). Also records why true lock
acquisition per mutation was not taken.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Blazor disabled per-playout Reset/Erase/Delete/Edit while a BuildPlayout was
in flight (EntityLocker.IsPlayoutLocked); the REST API had no equivalent, so a
client could race an in-flight build with a destructive ExecuteDelete and leave
a half-built playout. After Blazor removal this safety invariant would vanish
entirely (adversarial-reviewer#18 removal gate).
Server:
- Add public ApiResults.ConflictProblem(title, detail) (409, mirrors NotFoundProblem).
- Inject IEntityLocker into PlayoutController; guard every id-keyed mutation
(PUT {id}, PUT .../deco, PUT .../alternate-schedules, PUT .../templates,
POST .../erase-items, POST .../erase-items-and-history, DELETE {id}) → 409
when IsPlayoutLocked(id); add [ProducesResponseType(...409)] to each.
- Guard ChannelController.ResetPlayout the same way after resolving the id.
- reset-all stays 202 (ResetAllPlayoutsHandler already skips locked playouts).
- Stamp IsLocked onto PlayoutListItemResponseModel from IsPlayoutLocked.
SPA:
- Disable Reset/Erase/Erase-and-history/Delete for a locked row + show a
"Building…" Badge; on a 409 surface the error and refresh the list.
Tests: controller-level 409 guard tests (delete/erase/PUT/deco/channel-reset)
+ IsLocked projection test; new OpenAPI contract + metadata 409 rows.
Docs: api-conventions §3a, blazor-route-parity playouts verdict, decisions.md.
Regenerated v1.json + web types.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- api-conventions.md §5a: runtime Newtonsoft casing vs generated spec, the
schema transformer that mirrors it, and the contract test guarding it.
- decisions.md: append the "wire format is source of truth; spec follows via the
real contract resolver" decision.
- spa-conventions.md §4: trust the generated key casing; note the removed
troubleshooting escape hatch and runtime-cased test mocks.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
timothyandOpenAI Codex