timothy/ersatztv
1
0
Fork 0
Code Issues 51 Pull Requests 5 Actions Packages Projects Releases 9 Wiki Activity
Files
ff3df39c43b5dbb37198f354fa4dd9f650de5d62
ersatztv/docs/decisions.md
T
timothyandClaude Opus 4.8 ff3df39c43
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 12s
Details
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 12s
Details
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 14s
Details
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 4m59s
Details
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 6m13s
Details
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Details
fix(process): #303 H10 — anchor verdict marker to line-start (close self-reference false-open) [decisions-edit] 
Second adversarial re-review (of the fix commit itself — dogfooding H10) found
a remaining false-open: the `review-verdict:` marker was matched anywhere on a
line, so a comment merely QUOTING the positive template — an instruction
("please post: Review-verdict: MERGEABLE @ <head>"), a blocking comment
explaining how to clear itself, or the gate's OWN suggestion text echoed into a
comment — was classified as a real head verdict and self-approved the merge.

Fix: anchor the marker to line-start (`^[[:space:]]*review-verdict:`). A real
verdict line starts with the marker; quoted/instructional uses have text before
it. Also drops the dead `nosha` var (SC2034).

Finding 2 (a BLOCKED mis-anchored to an OLDER sha doesn't retract a
MERGEABLE@head) is deliberately NOT "fixed": staleness is symmetric — a
pre-fix BLOCKED@oldsha must not block forever after the fix changes the sha and
earns a fresh MERGEABLE@head. To retract, re-review head and post BLOCKED@head.
Documented in decisions.md.

Pipe-tested 21 cases (5 new: instructional-quote, self-reference reason-text,
line-start-in-multiline, leading-whitespace, blocking-quotes-template), all
deny/ask/allow correct. bash -n + shellcheck clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-12 19:11:05 +02:00

117 KiB
Raw Blame History

Decisions — append-only log

Purpose: why the codebase does what it does, so agents don't "fix" an established convention or relitigate a settled call. Append new entries at the bottom in date order (plus a TOC line in the Index); update this doc in the same PR that changes any fact below (or that establishes a new convention worth recording).

Append-only, enforced (ersatztv#303 H9). A commit or PR that deletes or modifies an existing line is blocked by the Husky commit-msg hook and the CI decisions-guard job; insertions anywhere are always allowed. The block is lifted only by the [decisions-edit] token in the commit message, for the two legitimate reasons to touch history:

  • Fix a factual error in a past entry.
  • Supersede a reversed decision — add the new dated entry, then prepend a > **Superseded YYYY-MM by <new entry title>.** banner to the old entry and tag its Index line (superseded). Keep the old entry — why we changed our mind is the point; never silently rewrite it.

Consolidation (prune/merge superseded entries, refresh the Index) is primarily a step in the release checklist (docs/ci-cd.md → Versioning & releases), done at each release/milestone with [decisions-edit] — not ad hoc mid-arc. A size floor backstops it between releases: the decisions-guard CI job warns (non-blocking) once this file exceeds 1800 lines — the point where it no longer fits one default 2000-line agent Read — so append-only can't grow past what an agent can read in a pass. The metric is the file's read cost (line count), not the entry count. Together these keep append-only from accreting stale, contradictory, or unreadably-large history.

Index

2026-06 — REST API wraps existing MediatR handlers 1:1, no service layer

The REST API (#2, docs/rest-api.md) is thin controllers over the existing MediatR Create/Update/Delete handlers — no new service/business-logic layer was introduced, since nearly every handler already returns Either<BaseError, T>, which maps cleanly to HTTP status codes. Latent handler bugs (missing existence checks, KeyNotFoundException risk, etc.) are fixed at the handler, converting what would have 500'd into a proper 404/422 — not papered over in the controller. Established across the #2a–#2e gap-issue PRs. Deep FK ids nested inside item-list request bodies (e.g. a schedule item's CollectionId) are deliberately not existence-checked at that depth, to avoid N+1 validation queries — precedent set by the schedules endpoints (#172); see docs/api-conventions.md §3 for the up-to-date statement of this rule.

2026-06 — UI rebuild is a React SPA (ChicoryTV) on the REST API, not a Blazor reskin

#59 committed to a full SPA rebuild rather than reskinning Blazor Server pages. Blazor removal is split into two phases under #91: (a) root-flip (SPA becomes /) + legacy-route redirects — DONE, merged via PR #148 (ErsatzTV/LegacyUiRedirects.cs, feat/91-cutover → main). (b) full Blazor removal — gated on every route having an SPA equivalent; tracked route-by-route in docs/blazor-route-parity.md.

2026-07 — Response DTOs live in ErsatzTV.Core/Api, file-scoped #nullable enable

New REST response DTOs go in ErsatzTV.Core/Api/<Domain>/*ResponseModel.cs and mirror the shape of the corresponding Application-layer ViewModel — controllers never expose VM types directly. Because ErsatzTV.Core.csproj sets <Nullable>disable</Nullable> project-wide, any response-model file with an optional member needs its own #nullable enable pragma at the top (most already have one). ErsatzTV.Application has no nullable context at all — do not add ? annotations to types living there; that's a Core/Api-layer-only convention. Full detail: docs/api-conventions.md §2.

2026-07 — PUT-replace list endpoints derive Index from array order; alternate-schedules last row = catch-all default

For "replace the whole list" endpoints (PUT over a collection — schedule items, template items, etc.), the item's Index is derived from its position in the request array, not from a client-supplied index/order field — established by ReplaceScheduleItemsRequest.ToCommand (Items.Select((item, index) => item.ToReplaceCommand(index))). Separately, ProgramScheduleAlternate and PlayoutTemplate rows (both IAlternateScheduleItem) are evaluated in Index order, first-match-wins; the convention is to place the least-conditional (or unconditional) row last so it acts as the catch-all default. Established by the alternate-schedules work (PR #179, AlternateScheduleSelector.cs).

2026-07 — Templates editor in the SPA is a table, not Blazor's drag-calendar

The legacy Blazor TemplateEditor.razor used a drag-and-drop day-grid calendar UI. The SPA equivalent (/app/templates/{id}, PR #173) renders the same day/block assignment as a table instead. This is an accepted, deliberate parity deviation — don't "fix" it to match Blazor's interaction model without discussing it first.

2026-07-07 — API artwork contract: rooted URLs produced server-side

API response DTOs return artwork as rooted, directly-usable URLs (/artwork/posters/..., /artwork/thumbnails/..., /artwork/fanart/...), plus passthrough for absolute http(s):// URLs and Jellyfin/Emby proxy variants. Established by PR #181 (ErsatzTV.Application/LibraryBrowse/Queries/GetLibraryBrowseItemsHandler.cs, private Artwork(...) helper — comment: "Returns a rooted, directly-usable artwork URL for the SPA's <img src>... the SPA [needs it pre-rooted]"), then generalized into the reusable ApiArtwork helper (ErsatzTV.Core/Api/ApiArtwork.cs, PR #183). Root cause: the SPA has no <base href>, unlike Blazor, so relative artwork paths that worked for Blazor pages 404 in the SPA. Do not reuse the Application-layer Mappers used by Blazor (e.g. MediaCards/Television mappers) for new API DTOs — those still return old Blazor-convention relative paths; map from the domain/VM directly and root the path via ApiArtwork.

2026-07-07 — Decode-style endpoints take a row id and look up server-side

Endpoints that decode/expand opaque stored state accept a database row id and resolve server-side, rather than accepting client-supplied serialized state to decode. Established by GET /api/playouts/history/{id} (PlayoutController.GetHistoryDetails, PR #182) — the row's raw JSON (Key/Details) is decoded server-side into PlayoutHistoryDetailsResponseModel, the client never round-trips the raw payload itself.

2026-07-07 — Season/episode/music-video drill-in via parentId, not new child-listing endpoints

Rather than adding dedicated child-listing endpoints per media kind (e.g. "list episodes of a season"), the library-browse endpoint takes an optional parentId query param and the SPA drills in by re-querying with it. Established across PRs #181/#183 (library-picker season drill-in, then media-detail's season/episode/artist/music-video browsing). Avoids a combinatorial explosion of per-kind child endpoints.

2026-07-07 — Convention docs read at session start, updated in-PR

docs/api-conventions.md, docs/spa-conventions.md, docs/e2e-local.md, docs/blazor-route-parity.md, docs/domain-model.md, docs/decisions.md, and docs/README.md are the standing reference set every ChicoryTV session should read before starting work, and each one carries an explicit "update this doc in the same PR" rule rather than deferring doc updates to a follow-up. These docs replace per-session recon — an agent reads the index (docs/README.md) and the relevant convention doc instead of re-deriving conventions from the code each time it starts API/SPA/E2E/parity work. A testing map and a generated-endpoint index are tracked as still-to-come under #185. Drafting this doc set also surfaced a drift in ApiControllerSecurityTests.cs's hardcoded controller registry (several controllers under ErsatzTV/Controllers/Api/ are missing from it — see docs/api-conventions.md §6) — tracked as a follow-up under #184 rather than fixed inline, since it's a pre-existing gap, not something this doc-drafting pass caused.

2026-07-09 — Playback-troubleshooting completion feedback: poll status, no push channel

The SPA playback-troubleshooting screen (PlaybackTroubleshootingScreen.tsx, #145) reports FFmpeg completion by polling GET /api/troubleshoot/playback/status every ~2s while a session is running (plus one poll on mount so a session started elsewhere still gates Play), rather than a server push. The status endpoint returns { state, exitCode, speed, logs }; the screen captures the running→completed/failed transition in local component state and surfaces a completion notice (success on exit 0, warning otherwise) — the SPA equivalent of the Blazor page's MediatR ICourier/ISnackbar PlaybackTroubleshootingCompletedNotification. Chosen over SignalR/SSE because the SPA has no push channel and troubleshooting sessions are short and user-initiated, so a lightweight poll (started on Play, stopped on settle/unmount) is simpler than standing up a new real-time transport. Speed thresholds and the "(Speed: Nx)" badge colors are copied verbatim from the Blazor GetSpeedClass (red <0.9, green >1.1, amber otherwise).

2026-07-09 — datetime-local instead of Chronic natural-language start parsing

The channel-mode "Date and Time" input in the SPA playback-troubleshooting screen uses a native <input type="datetime-local">, a deliberate deviation from the Blazor page, which parsed a free-text field with Chronic.Core.Parser (natural language like "yesterday at 8pm"). The SPA has no Chronic dependency and a picker is unambiguous; the selected local datetime is sent to playback.m3u8 as an ISO-8601 start param via new Date(value).toISOString(), which the controller binds to DateTimeOffset? exactly as the Blazor round-trip ("o") format did.

2026-07-09 — SPA gates Download Media Sample while a session is active

Minor intentional deviation: the SPA playback-troubleshooting screen disables Download Media Sample (alongside Download Results) while a troubleshooting session is starting/running; Blazor only gated Download Results. Both downloads compete with the live transcode for I/O and the sample archiver reads the same media file, so gating both during a session is strictly safer and costs nothing (sessions are short).

2026-07-09 — OpenAPI spec mirrors the runtime Newtonsoft serializer (#198)

The generated OpenAPI document is made to follow the runtime JSON contract, not the reverse. Runtime /api/* responses are serialized by Newtonsoft via CustomContractResolver/CustomNamingStrategy (camelCase + a FFmpegProfileId→ffmpegProfileId special case + [JsonProperty] overrides such as ChannelResponseModel.FFmpegProfile→ffmpegProfile), while Microsoft.AspNetCore.OpenApi generates the spec from System.Text.Json metadata, whose camelCase drifted (fFmpegProfileId, fFmpegProfile). That drift fed the SPA the wrong key. Rather than hand-patch the spec or change the wire format (breaking clients), we added NewtonsoftSchemaNamingTransformer — an OpenAPI schema transformer registered on all three documents that renames each schema property through the same Newtonsoft contract resolver the runtime uses, so the spec matches the wire format by construction. A contract test (OpenApiSerializerContractTests) serializes representative DTOs through the real runtime settings and pins the spec property sets to them. Decision: the wire format is the source of truth; the spec follows it via the real contract resolver. This also fixed a latent SPA bug (the channel-list "FFmpeg profile" column read fFmpegProfile and always showed "Unassigned"). Issue #198.

2026-07-09 — YAML playout validator: paste-textarea instead of a server file path

The legacy Blazor YAML playout validator took a server-side file path (read directly off the container's filesystem). The SPA's YamlValidatorScreen.tsx instead uses a paste <textarea> — a deliberate deviation, not an oversight. The SPA runs entirely client-side against /api/* and has no access to the server's filesystem, so a file-path field would either need a new filesystem-browsing endpoint or silently fail; pasting the YAML directly is simpler and matches how every other SPA editor already round-trips content through the API instead of the disk.

2026-07-09 — Channel numbers: prompt-driven sequential renumber instead of drag-to-reorder

The legacy Blazor channel list let you drag-and-drop rows to reorder channel numbers. The SPA (App.tsx) instead offers a "Renumber" action that walks the list and asks for each channel's new number via a sequence of native prompt() calls. Deliberate deviation: drag-to-reorder needs a dedicated drag library and a bespoke reorder-persistence endpoint; a sequential prompt reuses the existing per-channel update call and needs no new UI dependency. Revisit only if channel counts grow large enough that prompt-per-channel becomes tedious.

2026-07-09 — "Table, not calendar" convention also covers the deco-templates editor

Extends the 2026-07 "Templates editor in the SPA is a table, not Blazor's drag-calendar" entry above (not editing that entry — this generalizes it): the deco-templates editor (DecoTemplatesScreen.tsx) follows the same convention, rendering its day/deco assignment as a table rather than reproducing Blazor's drag-and-drop calendar grid. Same rationale, same accepted-deviation status — don't "fix" either editor to match Blazor's interaction model without discussing it first.

2026-07-09 — Trash "see all" is capped at 100 items per kind; true paging deferred

TrashScreen.tsx requests GET /api/search?query=state:FileNotFound&pageSize=100 — 100 is SearchController's MaxPageSize, and the endpoint has no page-number parameter, so a client can't page past the first 100 matches of a given media kind. The SPA shows the first 100 per kind (movies, shows, episodes, etc. are separate groups, so in practice most trash lists fit well within that per-kind cap); this mirrors the legacy Blazor Trash page, which had the same underlying search behavior and cap. True paging is deferred until the search API grows a page param — not attempted here, since it would mean adding a paging contract server-side, out of scope for this pass.

Superseded 2026-07-11 (#213): the cap is lifted via a per-kind "See all N …" button, without adding any new API surface. GET /api/library/browse (LibraryBrowseController / GetLibraryBrowseItems) already accepts mediaType + pageNum + pageSize and runs the same underlying query as GetSearchResults (which itself fans out to GetLibraryBrowseItems per kind, just always at pageNum=0) — so TrashScreen.tsx pages pageNum=1, 2, … through /api/library/browse?query=state:FileNotFound&mediaType={kind}&pageSize=100 for a kind once the user asks to see past the first 100, and appends the results client-side. The 100/kind first page still comes from /api/search (unchanged, cheapest for the common case where a kind has few matches); only kinds that exceed the cap ever issue the follow-up /api/library/browse calls.

2026-07-11 — Logs page-size is a client-local preference, not a server ConfigElement

The legacy Blazor Logs page persisted the user's chosen rows-per-page via ConfigElementKey.LogsPageSize (SaveConfigElementByKey/GetConfigElementByKey), a per-server-instance setting stored in the DB. LogsScreen.tsx instead persists it to window.localStorage under ctv-logs-page-size (same wrapped-Storage pattern as designSystem.ts's theme preference: try/catch getter, validated against the known option set, falls back to a default) and restores it on mount. Deliberate deviation: this is a per-browser UI preference, not server/business state — no other client should see or be affected by it, so there is no reason to round-trip it through the API and grow a new /api/* surface (or reuse the generic config-element endpoints) just to store a page-size number. Follows the existing SPA localStorage convention (designSystem.ts theme, auth.ts token) rather than introducing a new persistence mechanism.

2026-07-11 — Logs column sorting: allow-listed sortField/sortDirection on GET /api/logs

Parity for Logs.razor's MudTableSortLabel columns (Timestamp, Level — Message was never sortable in Blazor either). LogsController.GetLogs adds sortField (timestamp | level, default timestamp) and sortDirection (asc | desc, default desc) query params, normalized server-side the same way pageNum/pageSize are clamped rather than rejected with a 422: an unrecognized sortField silently falls back to timestamp, an unrecognized sortDirection falls back to desc — the pre-existing default behavior (newest-first) is unreachable to break via a bad query string. LogsScreen.tsx renders the two sortable headers as buttons with a chevron indicating the active field/direction; clicking the active column toggles direction, clicking the other column switches to it ascending.

2026-07-09 — Per-playout "Schedule reset" button dropped; Reset uses the server-default build mode

Blazor's playouts page had both a per-playout Reset and a separate Schedule Reset control (setting the daily rebuild time). The SPA keeps Reset — POST /api/channels/{channelNumber}/playout/reset with no mode param, so the server picks the same default Blazor used (Classic → Refresh, everything else → Reset) — but drops the dedicated "Schedule reset" button: the daily rebuild time is already editable through the playout's Edit-details flow, so a second entry point would duplicate an existing capability. Deliberate deviation, not a lost capability. Issue #210.

2026-07-09 — Collection custom order: move up/down buttons, any-kind collections

Blazor reordered collection items with SortableJS drag-and-drop and only enabled custom ordering for movies-only collections. The SPA (CollectionsScreen.tsx) uses per-row Move up / Move down buttons in an explicit reorder mode instead of drag (no new drag dependency; the mode loads ALL items first because PUT /api/collections/{id}/custom-order replaces the whole order from array position — submitting a partial page would scramble the rest), and does not replicate the movies-only gate: the API and the playback-side CustomOrderCollectionEnumerator sort by CustomIndex regardless of item kind, so the SPA offers reorder for any manual collection with custom order enabled. Issue #211.

2026-07-10 — Shared "Add to…" layer lives in web/src/media/addTo/; select-mode is an explicit toggle

The media mutation surface (#208/#209) is built on one reusable component group, web/src/media/addTo/ (media-domain components, like MediaPosterCard — not generic components/): AddToCollectionDialog (existing-collection Select + inline "(New collection)" create, Blazor AddToCollectionDialog.razor parity), AddToPlaylistDialog (group → playlist Selects, no inline create), AddToScheduleDialog (schedule Select; payload replicates Blazor's AddProgramScheduleItem.ForMediaItem defaults — see addTo/scheduleItem.ts), SaveAsSmartCollectionDialog, and AddToMenu (the drop-in popover for cards/detail pages via MediaPosterCard's actions slot). New screens wanting add-to affordances use this layer — don't build screen-local pickers. Two deliberate deviations from Blazor, applied consistently on the search and browse screens: (1) multi-select is an explicit screen-level "Select" toggle (off = cards open, on = cards select) rather than Blazor's always-on corner-select, because MediaPosterCard's select handler takes over the card's single click gesture; (2) the per-card menu offers collection/playlist for a single item of any kind, plus schedule only for shows/seasons/artists — collection/playlist is a superset of Blazor's per-card collection-only menu, while the schedule target is gated to exactly the kinds Blazor's AddProgramScheduleItem.ForMediaItem call sites offer, because the server validator (ProgramScheduleItemCommandBase.CollectionTypeMustBeValid) accepts only the TelevisionShow/TelevisionSeason/Artist per-media-item CollectionTypes and 422s the rest. "Add All" (query-wide) mirrors Blazor's two-step: materialize ids via GET /api/search/all-items, then reuse the id-list add endpoints — no query-based add command exists server-side. Issues #208/#209.

2026-07-10 — Schedule-item GET returns a flat, non-polymorphic DTO (ScheduleItemResponseModel)

GET/POST/PUT /api/schedules/{id}/items return ScheduleItemResponseModel / ScheduleItemsResponseModel (ErsatzTV.Core/Api/Scheduling/), not the Application-layer ProgramScheduleItemViewModel hierarchy (One/Flood/Multiple/Duration subtypes). The polymorphic VM only described its base shape in OpenAPI, so the SPA couldn't see the subtype fields (issue #126). The flat DTO promotes every subtype field to a nullable top-level member — multipleMode, multipleCount (renamed from the VM's Count), playoutDuration, tailMode, discardToFillAttempts — mapped by pattern-matching the concrete VM in ScheduleItemResponseMapper (ErsatzTV.Application/ProgramSchedules/). Its mutation fields are named 1:1 with ScheduleItemRequest so a GET maps losslessly back to a PUT/POST (ScheduleItemResponseRoundTripTests is the release gate proving the fixed point). It also carries picker-hydration fields the editor needs: collectionName/smartCollectionName/…/playlistName, playlistGroupId (to preselect the playlist's group), per-filler names, watermarks / graphicsElements as NamedIdResponseModel lists, the computed name, and durationEstimate. GetProgramScheduleItemsHandler.EnforceProperties still rewrites StartType→Dynamic, Flood→One and Playlist/Rerun→PlaybackOrder None when ShuffleScheduleItems is on — that lossy normalization is deliberate and lives on the read side (documented + tested). New shared NamedIdResponseModel (ErsatzTV.Core/Api/) is the generic {id, name} embed for API responses. Issues #126/#207/#212.

2026-07-10 — Playout API mutations return 409 while the build lock is held (#215)

Blazor disabled per-playout Reset/Erase/Delete/Edit while a BuildPlayout was in flight (EntityLocker.IsPlayoutLocked, Playouts.razor + per-kind editors); 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. Adversarial-reviewer#18 promoted this to a #91-phase-(b) removal gate: after Blazor is deleted the invariant would vanish entirely.

Decision: enforce the invariant server-side on the API rather than re-implementing a live push channel. PlayoutController and ChannelController inject IEntityLocker; every id-keyed mutation — PUT /api/playouts/{id}, .../deco, .../alternate-schedules, .../templates, POST .../erase-items, .../erase-items-and-history, DELETE /api/playouts/{id}, and POST /api/channels/{channelNumber}/playout/reset — checks IsPlayoutLocked(id) first and returns 409 Conflict (ApiResults.ConflictProblem, new shared helper mirroring NotFoundProblem) while the build lock is held. The PUTs are gated too (not just the destructive ops): the target invariant is "no mutation during a build", matching Blazor's edit-disable.

  • The guard is advisory check-then-act, not mutual exclusion — same posture as Blazor's disabled buttons. A BuildPlayout already sitting in the worker queue can take the lock a few milliseconds after the check passes, so the original race is narrowed, not eliminated; consequences remain self-healing (the next rebuild corrects a half-mutated playout). True prevention — having each mutation acquire the playout lock for its duration — was deliberately not taken: LockPlayout publishes PlayoutUpdatedNotification (UI churn per mutation) and would make mutations block builds, a semantics change out of scope for restoring Blazor parity.

  • reset-all is deliberately NOT gated — it stays 202. ResetAllPlayoutsHandler already silently skips locked playouts, which matches Blazor and the handler semantics; a fire-and-forget bulk enqueue always accepts.

  • SPA mirrors the lock via data, not a push channel — PlayoutListItemResponseModel gains an IsLocked bool (set from IsPlayoutLocked in the controller's list projection). The playouts screen disables Reset/Erase/Erase-and-history/Delete for a locked row and shows a "Building…" Badge; on a 409 from any mutation it surfaces the error and calls query.refresh() so the row picks up the flag. No new polling was added (the existing 30s channel-state poll is unchanged).

Precedent for the 409 shape: TraktController (left as-is with its own private ConflictProblem() to keep the diff small). Convention recorded in api-conventions.md §3a.

2026-07-11 — Schedules SPA editor: draft/explicit-Save over instant-persist; Copy includes multi/smart/rerun; shuffled-GET normalization preserved

The ChicoryTV schedules editor (web/src/screens/SchedulesScreen.tsx + web/src/schedules/, issue #207) rebuilds the schedule-item lineup to full mutation parity with the legacy Blazor editor. Three deliberate decisions:

(a) Draft model with one explicit Save, replacing instant-persist. All add/edit/copy/remove/ reorder mutate a local draft list only; a single Save issues one PUT /api/schedules/{id}/items (the replace endpoint). This is intentional because that PUT is destructive server-side — it deletes and recreates every item row (new ids) and triggers playout rebuilds — so batching edits into one flush (vs. the old per-action POST/DELETE/PUT) minimizes churn and gives the user a Discard/dirty affordance. On 422/network error the draft is kept and the error surfaced; on success the draft is replaced with the server response. A Discard action and a dirty guard (native confirm() on schedule-switch + in-app nav via navigationGuard.ts, plus a beforeunload listener) protect the draft. See docs/spa-conventions.md §8. This retires the old inline ScheduleScreen in App.tsx and its instant-persist add/delete/reorder tests.

(b) Copy item deep-copies ALL source references, including multi/smart/rerun collections. Blazor's CopyItem omitted the multi-collection / smart-collection / rerun-collection references when duplicating an item (copying only the plain collection/media-item/playlist refs) — a latent bug. The SPA's copyDraftItem (web/src/schedules/itemRules.ts) copies every source field + display name, so copying a MultiCollection/SmartCollection/Rerun item preserves its source. Deliberate deviation fixing the Blazor omission.

(c) The shuffled-schedule GET normalization (EnforceProperties) is preserved lossiness, matching Blazor. When a schedule has ShuffleScheduleItems, GET .../items still rewrites startType→Dynamic, Flood→One, and Playlist/Rerun playbackOrder→None (and zeroes discardToFillAttempts for non-random Duration items). The SPA does not fight this — it hides the Fixed start type and Flood playout mode from the option lists (and disables the reorder arrows) for shuffled schedules, mirroring Blazor, so a GET→edit→PUT round-trip stays consistent with the server's read-side normalization. Issue #207.

2026-07-11 — Channel editor: bare-create entry point + external-logo mutual exclusion (#212)

Two Blazor-parity decisions closing the channel-editor gaps (ChannelEditor.razor + ChannelEditViewModel):

  1. Bare-channel create lives on the channels list, not a form-first route. Blazor's /channels (no Id) is a full add form; the SPA instead adds a "New blank channel" action next to "Add Channel" on ChannelsScreen (web/src/App.tsx) that POSTs CreateChannelRequest with Blazor's computed add-mode defaults ((max existing int-parsed channel number) + 1, name: "New Channel", group: "ErsatzTV", ffmpegProfileId = GET /api/settings/ffmpeg's defaultFFmpegProfileId, streamingMode: "TransportStreamHybrid", isEnabled/showInEpg: true, every other field at its C# default(T) — see ChannelEditor.razor's else branch for the source of truth) directly, then navigates to /app/edit-channel/{id} for the rest of the fields. This is a deliberate equivalent, not a parity gap: it reuses the existing full editor instead of duplicating its ~20 fields into a second form. "Add Channel" (/app/new-channel, the library-to-lineup ChannelBuilder flow) is unrelated and untouched.
  2. External logo URL wins over an uploaded logo, mirroring ChannelEditViewModel.ToUpdate/ToCreate. The channel's logo is ArtworkContentTypeModel { path, contentType, isExternalUrl }; on hydration, isExternalUrl: true populates a separate "External logo URL" field and the uploaded-logo draft state is treated as empty (EMPTY_LOGO = { path: '', contentType: '' } in ChannelEditScreen.tsx) so the two never disagree. On submit, a non-blank URL always wins: logo: { path: url, contentType: '' }, exactly matching ExternalLogoUrl's precedence in the C# view model. Uploading a file clears the URL field (the last-set field wins, Blazor parity via UploadLogo's _model.ExternalLogoUrl = null). The URL is validated as http(s) client-side before save is enabled (Blazor has no equivalent validation; added because the field is free text with no server-side format check surfaced to the SPA).

Also landed with #212: preferredAudioLanguageCode/preferredSubtitleLanguageCode, musicVideoCreditsTemplate, and streamSelector moved from free-text Inputs to Selects fed by GET /api/languages / /api/channels/music-video-credits-templates / /api/channels/stream-selectors. Each keeps the channel's currently-stored value selectable even if it's absent from the reference list (optionsKeepingCurrent in ChannelEditScreen.tsx) so loading an existing channel never silently changes the value out from under an unmanaged language code or a template/selector file removed from disk since save.## 2026-07-11 — Queue state lives in the pinned Gitea tracker (#237), not in the handoff file

With multiple sessions/agents working the repo in parallel, the old protocol — every session wholesale-rewrites docs/handoffs/chicorytv-issue-queue.md on main (session state + queue + next-session prompt) — became a last-writer-wins race. New protocol: volatile queue state moved to Gitea, which is concurrency-safe by construction. Pinned tracker issue #237 holds the goal + ordered arc in its body (edited rarely, only on arc changes, re-read before edit) and an append-only session-comment log (fixed template: Closed / Filed / Triage / Arc change / Recommended next). Milestone Blazor removal (#91 phase b) + the review and in-progress labels are the machine-queryable view. Sessions claim an issue before working it (in-progress label + claim comment; the tiny read→claim race window is accepted, later claimant backs off; stale claims — no commits/comments ~48h — may be taken over with a comment). Every new issue gets an explicit end-of-session triage verdict — gate-blocker (milestone + arc slot) or backlog (label only) — so review findings adjust the queue only through that step and the arc doesn't drift. The handoff file keeps only the static kickoff prompt and the append-only Lessons lore (per-session prompts are gone; task context lives in issue bodies).

2026-07-11 — EntityLocker: atomic flags + single-owner release discipline, no owner tokens (#231)

EntityLocker (process-wide singleton, ErsatzTV.Infrastructure/Locking/EntityLocker.cs) is the advisory "operation on entity X is in progress" mutex layer. Adversarial review (#20/F5, →#231) found three defects: six plain-bool flags with a non-atomic check-then-set (two threads could both acquire and both return true), tokenless Unlock* letting any caller release another owner's lock (e.g. BuildPlayoutHandler ignored LockPlayout's return then unconditionally unlocked in finally), and one batch taking a single LockLibrary released after the first of two enqueued scan units (so the second ran unlocked).

Model chosen: tokenless atomic flag + documented single-owner release discipline. The six bools became ints guarded by Interlocked.CompareExchange (0/1), so Lock*'s bool return is now a reliable "this call won the transition" signal and the change event fires exactly once per transition. The contract (XML-doc'd on IEntityLocker): a true from Lock* confers ownership of exactly one release — performed either in the acquiring scope (finally, gated on the captured bool) or by the single designated releaser the acquirer hands off to (the consumer of the message enqueued while holding the lock, with a compensating unlock if the enqueue throws — the established TraktController.EnqueueWithTraktLock / scheduler unlock: last tail pattern). Callers must never release a lock they did not acquire. Unlock* on an already-unlocked slot returns false, fires no event, and logs a Warning — the loud tripwire for double-release bugs; it does not throw or Debug.Assert, because an advisory flag must stay safe to release in finally paths. The three ConcurrentDictionary-backed kinds (Library/Playout/RemoteMediaSource) were already atomic (TryAdd/TryRemove) and kept their semantics (the redundant ContainsKey pre-checks were dropped as tidy-up); the interface signature is unchanged across its ~40 call sites.

Rejected: owner tokens/leases — the acquirer and releaser for Library/Trakt/Plex/Collections locks are different code correlated only by entity id across in-memory Channel<T> queues, so a token would have to travel inside ~8 background-request message types for a defect that discipline plus the now-trustworthy atomic return value already prevents. Counted/reentrant locks — wrong semantics: these are exclusive in-progress flags; two holders is the failure mode, not a feature. Call-site fixes this model prescribes land separately: #232 (scan lifecycle — enqueue only after a successful lock, compensating unlock on enqueue failure, one release per acquisition in batches) and #234 (BuildPlayout/subtitles gate their finally unlock on the captured acquire result).

2026-07-11 — Channels screen extraction (#244): single-file screen, no sibling helper dir (epic #243 phase 1)

First bounded extraction under the App.tsx modularization epic (#243): the Channels domain moved verbatim out of web/src/App.tsx into web/src/screens/ChannelsScreen.tsx (zero-prop, self-sufficient, mirroring the SchedulesScreen extraction), with its behavior tests moved to a colocated ChannelsScreen.test.tsx that owns its own scoped fetch mock (spa-conventions §6). Pure structural move: no API/route/CSS/visual change; App.tsx retains only the import + the <ChannelsScreen /> dispatch clause.

Decision — no web/src/channels/ sibling helper directory (unlike Schedules' web/src/schedules/). Channels' pure logic (stateByChannelId, sortedChannels, groupedChannels, progressFromChannelState, formatChannelNumber) totals ~30 lines with no independent business-rule layer comparable to Schedules' itemRules.ts (~470 lines, separately unit-tested). Keeping it inside the single screen file matches the Blocks/Decos/Templates precedent. Revisit only if a later #243 phase adds substantial pure Channels logic worth isolating.

One cross-domain helper inlined, not shared: the Dashboard-owned progressFromNowPlaying (still in App.tsx, used by OnAirCard) was structurally reused by the Channels progressFromChannelState. Rather than export it from App or create a shared module, its ~5-line start/finish/now percentage math was inlined into the moved progressFromChannelState (behavior-identical — ChannelState.nowPlaying carries the same startUtc/finishUtc shape), so ChannelsScreen.tsx has no import back into App.tsx.

#238 (TopBar primaryAction dead button) left untouched — the channels route's inert ctv:primary-action dispatch is #238's owned bug and out of scope for a behavior-preserving extraction; the shell/action redesign is deferred to #247 (epic phase 4).

2026-07-11 — Media-source management REST write API + SPA (#202)

Replaced the Blazor /media/sources/{local,plex,jellyfin,emby}/... pages (14 routes) with SPA screens under /app/libraries/* over new write controllers (LocalLibrariesController, Plex|Jellyfin|EmbyMediaSourcesController), wrapping existing MediatR commands 1:1 (no new commands, no DB migration). Full design + adversarial-review reconciliation: docs/handoffs/ session record and issue #202. The design surfaced and fixed several pre-existing Application/Infrastructure bugs newly reachable from a programmatic client; each is recorded here because it changes documented behavior, not just adds a route.

Secure apiKey contract (Jellyfin/Emby connection). The connection GET (RemoteConnectionResponseModel) returns { address, hasApiKey } — the stored key never leaves the server, closing a leak where the old design would have served the raw key from an unauthenticated GET under any-origin CORS. On the connection PUT, a blank/omitted apiKey means retain the existing key; a non-blank value sets a new one; the key is required on first connect (no existing secret) → 422. Rationale: GETs aren't behind X-Api-Key (ApiKeyAuthorizationFilter only guards mutating verbs), so a secret-bearing GET is a real exposure regardless of how obscure the route is. Stated explicitly as an input to #197 (the planned read-side-auth review for secret-bearing GETs) — #197 should treat "does any GET return a credential" as one of its checks, not just this one instance.

Three list-replace identity contracts, not one uniform one. An earlier draft assumed a single "Id<1=add / missing=delete / id-preserved" contract across all three PUT-replace families; source inspection proved that false for two of them:

  • Remote library sync preferences (PUT .../{id}/libraries) — the command carries no source id and the handler toggles only the ids present in the body; a row absent from the request is left untouched, not deleted (libraries are sync-discovered, never created via this PUT, so there are no Id=0 adds either). The controller validates the submitted id set against Get{Family}LibrariesBySourceId(id) (422 on any id not owned by the route's source — closes a cross-source hole). Identity is not stable across a disable: Disable{Family}LibrarySync removes and re-adds the row with a fresh id, so the SPA keys its draft to (name, mediaKind), never to Id, and refetches after every save (the PUT returns the reloaded list).
  • Path replacements (PUT .../{id}/path-replacements) — id-based (existing Id=update, Id<1=add, absent=delete) as documented, but the repo UPDATE SQL had no source-id predicate (WHERE Id = @id, no AND {Family}MediaSourceId = @id), so a PUT to source A could silently overwrite source B's row with the same numeric id. Fixed with a handler-level ownership guard (reject any incoming positive id not in this source's current set → 422, no partial mutation) and the repo SQL predicate itself (defense-in-depth for any other caller of that repo method).
  • Local library paths (PUT /api/libraries/local/{id}) — identity is the normalized path string (full path, trailing-separator/case-insensitive), not Id; Id in the request is advisory. Renaming a path is delete-old+add-new under the hood (its LibraryPath.Id changes). Kept as-is (matches the entrenched, tested Blazor behavior and how the SPA edits by value); not rewritten to id-based identity, which would be a bigger, riskier change out of #202's scope.

Plex pin-flow as REST: poll until the lock releases, exception-safe non-handoff unlock. The SPA polls GET /api/media-sources/plex rather than a per-pin status resource (no pin-addressable server state exists to expose; SSE/push was already rejected, 2026-07-09). Polling contract is isLocked && !isAuthorized = waiting on the user; isLocked && isAuthorized = finalizing (discovering servers — do not stop here, the server list is still empty); !isLocked && isAuthorized = success; !isLocked && !isAuthorized = timed out/abandoned. This required fixing a latent lock-leak bug: TryCompletePlexPinFlowHandler threw OperationCanceledException on its 2-minute timeout instead of returning false, and nothing unlocked on that path — an abandoned sign-in wedged the Plex lock until restart or manual sign-out. Fix releases UnlockPlex() on the timeout-throw, a poll-exception, and an enqueue-exception — but deliberately not in an unconditional finally: on success the lock is handed off to SynchronizePlexMediaSources, the sole releaser after server discovery; a blanket finally would double-release and release before discovery completes, re-opening the same race the fix closes. This is the same non-owner-token discipline as the #231 EntityLocker model (2026-07-11 entry above), applied to the pin-flow's handoff-vs-terminal distinction specifically.

404 comes from the controller pre-check, not the handler. Apply/ToEitherAsync both .Join() errors, which flattens any NotFoundError inside a joined Validation down to a plain 422. So every id-taking endpoint's real 404 is a controller-side pre-check (Get...ById(id)-is-None → ApiResults.NotFoundProblem, the TemplateController.DeleteGroup pattern), not a handler-level conversion — converting the joined validators to NotFoundError would be dead code, since the join discards the distinction anyway. This is check-then-act (a delete racing between the pre-check and the command falls through to the handler's own 422, not a 404); accepted and tested against the actual runtime error type rather than a hoped-for handler 404.

"Scan All" dropped, not implemented. The disabled SPA header button on the libraries hub was speculative UI with no Blazor equivalent (Libraries.razor only ever supported per-library scan). Removed rather than backed with a new bulk-scan endpoint; per-library scan (#232) and the new per-source refresh-libraries endpoints (P8/J9/E9) cover the real capability set.

App-owned popstate for guarded sub-path routes. LocalLibraryEditScreen and the other /app/libraries/* editors are the first screens to both register a dirty-navigation guard and track their own sub-path pathname — the combination spa-conventions.md §8 had flagged as unvalidated. React commits child passive effects before parent ones, so a sub-path wrapper that self-registers popstate would fire (and switch sub-screen) before App's guard-restore listener could veto. Resolution: App owns popstate centrally for the libraries route and only pushes an approved sub-path down to the wrapper (which no longer self-registers popstate); on a vetoed pop App re-pushes the pre-pop URL and the wrapper never sees the rejected path. This is scoped to the libraries route only (gated on activeRoute === 'libraries') so unguarded sub-path routes (Playouts, Media) stay byte-identical. See spa-conventions.md §2/§8 for the updated exemplar list and the resolved caveat text.

2026-07-11 — Legacy→SPA redirect matcher: exact map + ordered segment-template patterns (#204)

LegacyUiRedirects.TryGetRedirect grew from a single exact-path dictionary to a two-tier matcher behind the unchanged (PathString, out string) signature. Tier 1 is the existing OrdinalIgnoreCase Map (now 52 entries — the parameterless (A)/(B) routes plus the (E-base) browse roots whose targets carry ?kind=…). Tier 2 is an ordered IReadOnlyList<PatternRule> of 36 segment-template rules ((C)/(C2)/(D)/(E-page)), consulted only on a Tier-1 miss; declaration order is match order (first-match-wins).

Template tokens are minimal: {id} matches a strict positive integer (int.TryParse(seg, NumberStyles.None, InvariantCulture, out id) && id > 0 — rejects signs, whitespace, separators, 0, negatives, and overflow like 999999999999; the raw segment text, e.g. 007, is substituted, not re-formatted); {any} matches any non-empty segment and is dropped (only /playouts/add/{any}); everything else is a literal compared OrdinalIgnoreCase. The request path is split with StringSplitOptions.None and empty segments are rejected (load-bearing: so /channels//5 cannot match /channels/{id}); templates themselves use RemoveEmptyEntries. The existing single-trailing-slash normalization runs before both tiers, so /channels/5/ matches.

The set is collision-free by construction — exact-before-pattern plus strict numeric {id} means no two tiers/rules can match the same path. Guard invariant (comment + Map-keys meta-test): no Tier-1 key or Tier-2 template may begin with /api, /artwork, /docs, /openapi, /iptv, /app, or /media/sources; rules are always full, specific templates — never prefix wildcards (a bare /media/{any} rule is forbidden). The blazor branch does not prefix-guard /api|/artwork|/docs| /openapi, so the matcher's specificity is part of their protection.

Query-string merge: the incoming request query is now merged into the target via a new public AppendQueryString(target, QueryString) helper (one-line Startup change: context.Request.PathBase + LegacyUiRedirects.AppendQueryString(target, context.Request.QueryString)). A target that already carries ? (the ?kind=… browse roots) is &-joined instead of producing a malformed double ?; plain targets keep verbatim-append behavior byte-for-byte. A duplicated key after a merge (?kind=movies + incoming ?kind=shows) is first-wins in the SPA (URLSearchParams.get returns the first value) — acceptable. Extracting the merge into LegacyUiRedirects keeps it unit-testable without a TestServer while preserving the PathBase re-application invariant the Startup source-text test protects.

Rejected: regex pairs (harder to audit for the /api//artwork greediness invariant, noisier tests, no benefit — every parameterized route here is "fixed segments + one variable segment"); ASP.NET TemplateMatcher/RouteMatcher (pulls routing machinery into a static helper for 36 rules); a single unified rule list (loses the O(1) dictionary hit for the ~52 exact routes that dominate real traffic). No /api change, no OpenAPI regen, no SPA change.

2026-07-11 — Blazor removal auth posture: no new exposure beyond phase (a); real auth deferred to #197 (#206)

Sign-off for the #91 phase (b) removal-gate item #206 ("deleting the last challenged Blazor page leaves only the open SPA"). The actual authorization wiring in ErsatzTV/Startup.cs + ErsatzTV/Pages was enumerated in code (not assumed) before clearing the gate.

What is gated today

  • OIDC (OidcHelper.IsEnabled — active only when Authority/ClientId/ClientSecret are configured): AddAuthentication (cookie default, oidc challenge) + AddAuthorization DefaultPolicy = RequireAuthenticatedUser + AddRazorPages(… AuthorizeFolder("/")) (Startup.cs:379-385) + blazor.UseAuthentication()/UseAuthorization() inside the Blazor MapWhen branch (Startup.cs:764-770). AuthorizeFolder("/") gates Razor Pages only, and the sole user-facing Razor Page is Pages/_Host.cshtml — the Blazor Server host (the other .cshtml, Shared/_Favicons.cshtml, is a cosmetic partial). So the OIDC challenge protects exactly the Blazor UI and nothing else.
  • /app (SPA) is served by its own MapWhen(path=/app) static-file branch (Startup.cs:701-714) with no authentication/authorization middleware — open since phase (a) (/→/app, PR #148).
  • /api/* controllers carry no [Authorize] (verified: zero attributes in Controllers/); the Razor-Pages AuthorizeFolder/DefaultPolicy never reach them. Their only optional gate is the per-endpoint ApiKeyAuthorizationFilter (API-key on mutating JSON endpoints), independent of OIDC/Blazor.
  • /iptv/* is gated by ConditionalIptvAuthorizeFilter (JWT JwtOnlyScheme, active only when JwtHelper.IsEnabled) in its own MapWhen branch (Startup.cs:797-803) — independent of Blazor.

Posture after Blazor removal. Removing Pages/_Host.cshtml, AddRazorPages/AuthorizeFolder("/"), blazor.UseAuthentication/UseAuthorization, MapBlazorHub, and MapFallbackToPage("/_Host") deletes the OIDC challenge's only attachment point — no user-facing surface remains challenged. No capability is lost: every Blazor-served capability already has an open SPA equivalent (the #91 parity effort), and the SPA was already the unauthenticated path since phase (a), so removal exposes nothing a user could not already reach via /app.

The one honest caveat (not a regression introduced by removal): an OIDC-configured operator's Blazor admin UI sits behind a login today; after removal there is no login-gated admin UI at all (the SPA admin UI is open). That exposure delta already happened at phase (a) (the open SPA became the default admin surface); removal only deletes the now-redundant challenged duplicate. Designing real SPA/API authentication is deliberately deferred to #197 (cold API security review — a HARD GATE before any remote exposure).

Removal-PR must-not-break (independent gates that survive): ConditionalIptvAuthorizeFilter (/iptv/* JWT), ApiKeyAuthorizationFilter (mutating /api/*), and JwtHelper access_token query support. Leave the OIDC service registrations in place (conditional on config, inert once no Razor Page consumes them) — ripping OIDC out is a #197 decision, not a removal-PR one. The removal PR removes only the Blazor-attached pieces above; MapControllers() + /docs (Scalar), currently co-hosted in the Blazor MapWhen branch, must survive the surgical reduction.

2026-07-11 — Pre-removal Blazor rollback tag blazor-final (#205)

Removal-gate item #205: the removal PR deletes both the Blazor reference implementation and the /system/health escape hatch, so a post-deletion parity gap would otherwise be an archaeology exercise (guessing which release tag still matches main minus Blazor). Decision + procedure, to run as the first action of the Step 2 deletion PR merge (not before — main moves until then):

  1. On the main commit immediately preceding the removal merge (the last commit that still contains ErsatzTV/Pages/**), cut an annotated tag and push it: git tag -a blazor-final -m "Last commit with the legacy Blazor Server UI (pre-#91-phase-b removal)" then git push origin blazor-final. The tag name is blazor-final (not v*) so it does not trigger the v* prod-release build in .gitea/workflows/docker-build.yml.
  2. Restore path (if a gap surfaces post-removal): git checkout blazor-final → docker build -f docker/Dockerfile -t ersatztv:blazor-final . → pin the test container to that image while the gap is fixed forward on main. Alternatively git revert the single deletion merge commit (keep the deletion as one squash/merge commit specifically to make this a one-liner).
  3. Document the tag + restore path in the removal PR body; update this entry with the tag's commit sha when cut.

Not cut this session — main still carries Blazor and will advance before the removal PR.

2026-07-11 — Async-op API contract normalization + playout build observability + F9 scan endpoints (#235)

Reviewer#20 F7/F8/F9. Normalizes the queue-triggering /api/* endpoints onto one contract, closes the two F9 Libraries.razor parity gaps, and hardens the Trakt batch-lock lifecycle. Much of the F8 surface was already normalized by #232 (library scan → QueueLibraryScanResult 202/404/409/422) and #215 (per-id playout mutations + reset → 409 lock guard) — this issue finished the remaining outliers.

Normalized async-op contract (queue-triggering endpoints): 202 Accepted = work queued; 404 ProblemDetails = entity missing (controller pre-check); 409 ProblemDetails = lock held (the running job, or a mutation racing it — §3a/§3b); 422 ProblemDetails = domain precondition (sync disabled / unsupported / start failed). Trakt was the reference implementation. Changes made:

  • MaintenanceController.EmptyTrash — error path 500 text/plain → 404/422 ProblemDetails (ToErrorResult).
  • MaintenanceController.CleanArtwork — silent 200 → 202 (fire-and-forget enqueue). No SPA consumer.
  • LibrariesController.ScanShow — conflated 400 {error} → 202/404/409/422 via a new QueueShowScanResult enum (6 outcomes incl. an honest ScanFailed→422, distinct from Unsupported).
  • ChannelController.ResetPlayout — 200 → 202 (queue-triggering; 404/409 unchanged).
  • PlayoutController.ResetAll — 202 (no body) → 202 + ResetAllPlayoutsResponseModel reporting queuedPlayoutIds / skippedLocked / skippedUnsupported (replaces the silent skip; still 202, still skips locked/ExternalJson by design per §3a — now it reports what it skipped).
  • TroubleshootController.TroubleshootPlayback — bare body-less NotFound() → 404/422 ProblemDetails with distinguishing detail. Status codes the SPA HLS player depends on were preserved — verified HlsPlayer.tsx never branches on this endpoint's status (playback state comes from the separate /api/troubleshoot/playback/status poll); only the error body was enriched.

Playout build observability: the list endpoint (GET /api/playouts) already stamped isLocked + BuildStatus on PlayoutListItemResponseModel (#215); this issue adds isLocked to the single-playout GET /api/playouts/{id} (PlayoutResponseModel), so the detail poll surface carries the §3a lock flag too. No dedicated GET /api/playouts/{id}/status push channel was added — the flag on the existing GETs is the HTTP-observable substitute for Blazor's live lock event, matching the GET /api/trakt/status precedent.

F9 parity endpoints (the Libraries.razor deletion gate — #202 did NOT close these):

  • Deep scan: POST /api/libraries/{id}/scan gains ?deep=false, threaded through QueueLibraryScanByLibraryId(LibraryId, DeepScan=false) into ForceSynchronize{Plex,Jellyfin,Emby}LibraryById(id, deep) (was hardcoded false). Non-breaking: existing callers omit it.
  • External-collections scan: new POST /api/media-sources/{plex|jellyfin|emby}/{id}/scan-collections?deep=false on the three #202 media-source controllers, dispatching Synchronize{X}Collections(id, ForceScan:true, deep). Each pre-checks source existence (404), acquires the per-source collections lock (Lock{X}Collections() — the lock is the running scan, so a false = 409), then enqueues and returns 202; the controller compensating-unlocks in a catch if the enqueue throws (§3b), and ScannerService releases in its finally. Thin SPA clients shipped (scanLibrary(id, deep), scanCollections); the SPA deep-scan / collections buttons are the removal PR's remaining parity work (parity doc §5).

F7 Trakt batch-lock leak fix: the global Trakt lock was released only when the terminal batch message (Unlock: true) was processed; a WorkerService shutdown/cancellation before that message leaked the lock permanently (subsequent Trakt ops 409 until restart — same class as #231/#233/#234). Fix: WorkerService now releases the Trakt lock in a finally on read-loop exit if still held. Non-vacuous regression test proven against an inverted-condition control.

Accepted-by-design (per the issue's decision-record ask): the worker's channels are unbounded and there is no shutdown drain — messages still queued at process exit are dropped. This is acceptable because the entity locks are in-memory singletons that die with the process, so a dropped message can't strand a lock across restarts (the F7 finally covers the within-process shutdown-break leak, which is the only way a lock outlives its batch while the process keeps running). Adding a bounded-channel backpressure / graceful drain is out of scope and would not fix a correctness bug.

2026-07-11 — Optimistic-concurrency contract for replace-all PUTs (#253 PR1: infra + Block reference)

Replace-all aggregate PUTs had no optimistic concurrency — a stale second tab silently overwrote a fresher edit (200, no signal) across ~10 aggregate surfaces. PR1 lands the shared contract on the Block reference aggregate; PRs 2–4 fan it out. The full ratified design + independent-review hardening is #253#issuecomment-8472; the mechanics live in api-conventions.md §7a. Decisions frozen here:

  • Token = uniform plain int Version on each root implementing IVersionedAggregate, EF-mapped .IsConcurrencyToken(), one dual-provider migration (AddAggregateVersions, defaultValue: 0). Not a reused DateUpdated (tick-collision, SQLite TEXT precision, couples UI cosmetics to correctness) and not a MySQL-native rowversion (portability over provider-native).
  • 412 Precondition Failed, not 409 — 409 stays the §3a EntityLocker "build in progress" guard; distinct codes → distinct SPA UX. New PreconditionFailedError : BaseError → 412 in ApiResults.ToErrorResult.
  • Pre-check AND EF token both required. The handler pre-check (a standalone Either introduced AFTER the validation pipeline — never via Apply, which Join()-flattens the subtype to 422) gives a clean 412; the unconditional root.Version++ + IsConcurrencyToken UPDATE-guard + a SaveChangesWithConcurrencyGuard backstop closes the residual load→save TOCTOU (DbUpdateConcurrencyException → 412).
  • Unconditional bump (not "only when a child changed"): EF writes the root row only when a scalar differs, so a no-op PUT-back must still bump to fire the token and rotate every other client's ETag.
  • Config-only aggregate boundary: every mutating handler of a root's editor-visible config state bumps Version (incl. bulk ExecuteUpdate/Delete writers via .SetProperty); regenerated build output (playout items/history) is outside the token — neither bumped nor guarded.
  • Header-only ETag, strong tag of the decimal Version; parsed/emitted by ConcurrencyHeaders. The successful PUT returns the new ETag (else a same-tab second save 412s against its own write).
  • Phasing: Phase 1 (this arc) = a missing If-Match force-writes (zero breakage) while the SPA starts echoing; Phase 2 (a later PR) flips missing → 428 after every editor echoes and one release soaks. If-Match: * stays the scripted force-write escape hatch.
  • Child stable-identity is OUT of #253 (the "moved fill-group item inherits the wrong slot's state" concern on the positional reconcile) — root-anchored versioning is orthogonal to it; split to #259.
  • If-Match status semantics (non-canonical/weak/list → 400) are fail-safe; the stricter RFC 7232 "valid-but-non-matching → 412" refinement is deferred to #197 (#265).

2026-07-11 — Post-commit side effects run on CancellationToken.None (generalized from #251 to #254)

Audit #22 (adversarial-reviewer) found ~20 command handlers threading the request cancellationToken into work that runs after SaveChangesAsync commits — the post-commit WriteAsync enqueue that rebuilds/refreshes the affected entity, mediator.Publish, search-index reindex, cache refresh. A late HTTP-client disconnect cancels that token, so the already-committed mutation throws on the way out and silently drops its side effect (the playout rebuild is never queued → the persisted edit never takes visible effect until a manual Reset). #251 fixed this for the deco handlers; #254 generalizes the policy across the codebase.

Decision. Once a mutation has committed, the entire compensating side effect — enqueues, publishes, reindexes, cache refreshes, and any post-commit lookup that gates one of those enqueues — runs on CancellationToken.None. The commit is the point of no return; past it the side effect must not be half-abortable. Full convention + the two boundaries in docs/api-conventions.md §7b.

Scope of the #254 sweep (this PR). Swept the single-SaveChanges handlers under MediaCollections/, ProgramSchedules/, Playouts/, Channels/ (20 handlers). Deliberately excluded:

  • BuildPlayoutHandler — a background/worker handler; its token is the worker shutdown token, not a client-disconnect token, so its downstream enqueues correctly honor cancellation.
  • UpdateFFmpegSettingsHandler + the two Configuration/ settings handlers — they commit via several sequential IConfigElementRepository.Upsert calls with an interleaved enqueue; "when is it committed" is a partial-commit-under-cancellation question broader than the clean single-SaveChanges F4 pattern. Left for a separate follow-up.
  • Response-projection reloads (ReplaceProgramScheduleItemsHandler / AddProgramScheduleItemHandler post-commit graph reload that builds the returned view model) keep the request token — a cancelled response after a durable commit + None-enqueue loses nothing.
  • Handlers a no-token WriteAsync() already makes behaviorally correct (default == None) were left alone (explicit-None there is cosmetic).

Also folded in the two other #254 items on the same handlers: the channel-guide {number}.xml delete in DeleteChannelHandler/DeletePlayoutHandler now routes through IFileSystem.File.Delete (observable under MockFileSystem) before the commit (a post-commit delete orphans the xml on a crash; the guide xml is regenerable on demand, so a pre-commit delete is the safe ordering), and ReplacePlayoutAlternateScheduleItemsHandler now rejects an empty item list in the handler (not only at the controller pre-guard) so a direct caller can't trip the Max()-on-empty crash.

Coordination note for #253 PR2–PR4. Those PRs add Version++ (pre-commit) to the mutating handlers of the versioned aggregates — several of which this sweep also touched (post-commit token, a different line region). Low git-conflict risk, but merge main in and expect to see the CancellationToken.None convention already present on the post-commit enqueues.

2026-07-11 — #253 PR3: Diff + Scalar concurrency fan-out (Collection / Playout×2 / MultiCollection / RerunCollection)

Context. PR3 of the #253 optimistic-concurrency arc fans the frozen Block recipe (api-conventions §7a) across the five Diff/Scalar aggregates. Three judgment calls beyond the mechanical copy:

H1 — Playout catch(Exception)→422. The two Playout replace handlers wrap SaveChangesAsync in a catch(Exception) that maps any exception to a bare BaseError (→ 422). Rather than let the guard's concurrency failure be reshaped into a 422, the guarded save (SaveChangesWithConcurrencyGuard) returns a PreconditionFailedError Left as a value and the handler returns it before the post-commit block — so it never reaches the catch. Proven by the pre-check-subtype tests (a .Apply flatten would fail ShouldBeOfType<PreconditionFailedError>) plus a non-vacuous Playout racing-save test.

M2 — the SaveChangesAsync() > 0 gates. RerunCollection and Collection-custom-order run their playout-refresh unconditionally on a successful save (the unconditional Version++ makes the old gate always-true; the "nothing changed" branch is dead). MultiCollection is the exception: it saved the name first specifically so a name-only change wouldn't rebuild playouts, so we bump Version on that first save and leave the second (items) save's > 0 gate intact — a name-only edit still bumps + rotates the ETag but does not rebuild. Enumerating every behavior the gate provided before reworking it (the #232 lesson).

Sibling-writer scope (deferred). §7a's config-only boundary says every writer of an aggregate's editor-visible config bumps Version. PR3 ships the five primary endpoints' full contract + the one design-named bulk writer (UpdateDefaultDecoHandler, safe via .SetProperty). It defers the other same-root non-bulk config writers (UpdateCollectionHandler, RemoveItemsFromCollectionHandler, UpdatePlayoutHandler, the ScheduleFile handlers) and the repository-mediated Add*ToCollection family. Rationale: the primary endpoints' own bump+guard fully cover the two-tab lost-update the issue targets; the deferred writers only affect cross-editor ETag rotation, and adding an unconditional bump to a handler that uses plain SaveChangesAsync (not the guard) converts a latent lost-update into a new 500 (DbUpdateConcurrencyException) — doing it safely needs a uniform guard+bump+412 pass of its own, better done with the #197 contract work. Tracked as a follow-up issue.

VMs. Playout.Version surfaces via PlayoutNameViewModel (required arg); the three collection VMs (MediaCollectionViewModel, MultiCollectionViewModel, RerunCollectionViewModel) carry int Version = 0 (defaulted — 0 for the selection-placeholder constructions, real value from the Mapper projection). Header-only via ETag, never echoed in a response body (the Block precedent).

Post-merge addendum (PR3 review, #269). Activating the Version token means EF guards every root UPDATE, so non-participating root-scalar writers that use plain SaveChangesAsync (playout settings / schedule-file / on-demand-checkpoint, collection name) would 500 on a concurrent bump. The realistic UPDATE writers were fixed in-PR with ConcurrencyExtensions.SaveChangesForcingVersion (Phase-1 force-write on conflict: adopt the stored token, retry, never revert the concurrent bump). The deferral above is re-scoped to the DELETE handlers + repository Add* writers only (→ #269).

2026-07-11 — External Collections in the SPA derives from getMediaSources(); collections scans are optimistically bounded (#91b Libraries.razor parity)

Context. The last SPA pre-work before deleting Blazor Libraries.razor was wiring the shipped scanLibrary(id, deep) + scanCollections(family, id, deep) clients (F9 API, #235) into LibrariesScreen, matching the page's four scan actions: quick-library (already had it), deep-library, quick-collections, deep-collections. Landed as its own pure-SPA PR ahead of the removal PR so the deletion diff stays surgical.

Decision 1 — derive the External Collections rows client-side, no new endpoint. Blazor's second table comes from the GetExternalCollections MediatR query (one row per Plex/Jellyfin/Emby source with Libraries.Any(ShouldSyncItems)), which has no REST wrapper. Rather than add one, the SPA derives the rows from the existing GET /api/media-sources: GetAllMediaSourcesForApiHandler already filters each source's libraries to sync-enabled entries (ShouldIncludeLibrary = ShouldSyncItems for remote families), so a remote source with a non-empty libraries list is exactly the Libraries.Any(ShouldSyncItems) filter. No extra endpoint, no extra fetch, single source of truth. (If MediaSourceLibraryResponseModel ever stops filtering to sync-enabled libraries, this equivalence breaks — a test asserting the row set guards it.)

Decision 2 — collections-scan buttons use optimistic, timeout-bounded pending (no poll reconcile). Library scans reconcile their optimistic "pending" flag against GET /api/libraries/scan-status. Collections scans have no such surface — that endpoint is library-keyed, and Blazor only ever observed collections locks via in-process IEntityLocker Are{X}CollectionsLocked events with no HTTP mirror. So the useCollectionsScan hook disables a button optimistically on click and clears it after a fixed COLLECTIONS_PENDING_TIMEOUT_MS (a 409 "already scanning" is benign and keeps it disabled until the timeout; a 404/network clears it and surfaces the error). This is the honest ceiling of what the current API exposes. Follow-up (backlog): a collections scan-status endpoint would let the SPA reconcile collections pending against a live active set the way library scans do; filed as #271 so the timeout isn't mistaken for the intended end state.

2026-07-12 — External-collections scans get an authoritative status surface (#271); the SPA timeout is retired

Context. The 2026-07-11 entry above shipped collections-scan buttons with an optimistic, COLLECTIONS_PENDING_TIMEOUT_MS-bounded pending flag because collections locks had no HTTP mirror (unlike library scans, which reconcile against GET /api/libraries/scan-status). That was the honest ceiling of the API at the time and was filed as #271 so the timeout wasn't mistaken for the end state. This entry closes that follow-up.

Decision 1 — one family-global status endpoint, reading IEntityLocker. New GET /api/media-sources/collections-scan-status (MediaSourcesController → GetCollectionsScanStatus handler) returns one CollectionsScanStatusResponseModel { family } entry per media-source family (plex/jellyfin/emby) whose collections lock is currently held, and only for active ones — the direct counterpart to GET /api/libraries/scan-status. It reads IEntityLocker.Are{X}CollectionsLocked() (the lock is the running scan — the scan-collections controllers acquire it before enqueueing and the scanner releases it on completion), analogous to how the library endpoint reads IScannerProxyService.GetActiveScans(). Two deliberate shape differences from libraries: (a) family-global, not per-source — the collections lock takes no source id (LockPlexCollections()), so an entry means every source of that family is scanning, matching Blazor's all-rows-disabled behavior (per the PR #272 review note); (b) no percent — collections scans expose only a boolean lock, not progress.

Decision 2 — the SPA reconciles authoritatively; the fixed timeout is removed. useCollectionsScan now polls the new endpoint (seeding on mount, so a scan already running when the screen opens disables the buttons immediately — the old timeout couldn't) and reconciles optimistic pending against the active-family set using the same pruneGraceExpiredPending grace-tick helper the library hook uses (now generic over the pending key type). A row shows "Scanning" when its family is in the active set or it has a still-in-grace optimistic pending key. The grace window is kept (not the old wholesale timeout) to absorb the click→observed-active lag and the fast-scan-between-polls race — the same bounded-pending discipline #232/#230 established for library scans. COLLECTIONS_PENDING_TIMEOUT_MS is gone.

2026-07-11 — Blazor Server UI removed (#91 phase b)

The #91 phase (b) removal PR deletes the legacy Blazor Server UI now that the ChicoryTV SPA has parity (all gates cleared: #145, #151/#152/#153/#155, #202, #207, #212/#213, #235 F9). The SPA is the only UI.

Deleted. ErsatzTV/Pages/** (all .razor, incl. _Host.cshtml, FragmentNavigationBase.cs, MultiSelectBase.cs), ErsatzTV/Shared/** (all .razor + _Favicons.cshtml), ErsatzTV/ViewModels/** (39 Blazor edit-form VMs), ErsatzTV/Validators/** (10 Blazor edit-VM FluentValidation validators), App.razor, _Imports.razor, ErsatzTV/Locals/Shared/** + ErsatzTV/Locals/Pages/** (Blazor localization resx — ErsatzTV/Locals/Resources.* is KEPT), ErsatzTV/wwwroot/css/** (site.css), ErsatzTV/wwwroot/lib/** (jquery, jqueryui, sortablejs, hls, media-chrome, roboto), libman.json, and ErsatzTV.Tests/Pages/MultiSelectBaseTests.cs.

9 packages pruned (from both Directory.Packages.props and ErsatzTV/ErsatzTV.csproj; each verified to have zero remaining consumers after the Blazor deletion): MudBlazor, Heron.MudCalendar, Blazored.FluentValidation, BlazorSortable — unambiguous Blazor UI; MediatR.Courier.DependencyInjection — ICourier was consumed only by the deleted pages, and the app's mediator.Publish notification path is plain MediatR (unaffected by the AddCourier removal); Markdig, HtmlSanitizer, Chronic.Core, NaturalSort.Extension — verified zero non-Blazor consumers post-deletion.

Startup surgical reduction. Removed AddRazorPages (+AuthorizeFolder("/")), AddServerSideBlazor, AddMudServices, AddSortable, AddCourier, the Blazor-attached UseAuthentication/UseAuthorization, MapBlazorHub, and MapFallbackToPage("/_Host"). The former "blazor" MapWhen branch (lambda param renamed blazor→legacy) is KEPT — it still co-hosts MapControllers(), /docs (Scalar), dev MapOpenApi(), and the LegacyUiRedirects middleware. MapFallbackToPage("/_Host") is REPLACED by a catch-all endpoints.MapFallback(...) that 302-redirects any unmatched path to PathBase + "/app" EXCEPT paths under /api, /artwork, /docs, /openapi (those get a genuine 404, per #204's design). KEPT (not removed): the OIDC/JWT/API-key SERVICE registrations (inert unless configured; real auth is #197), ConditionalIptvAuthorizeFilter (/iptv/*), and ApiKeyAuthorizationFilter (mutating /api/*) — per the #206 auth-posture sign-off (deleting the Blazor page challenged nothing beyond phase (a)).

LegacyUiRedirects. Added redirects for all 14 /media/sources/* routes → their /app/libraries/* SPA screens (7 Tier-1 exact + 7 Tier-2 {id} patterns; the last Section-2 rows in blazor-route-parity.md), and LIFTED the #204-era /media/sources forbidden-prefix guard (its Blazor pages were replaced by #202's SPA screens). The forbidden-prefix guard now covers only /api, /artwork, /docs, /openapi, /iptv, /app.

Also removed the now-dead ersatztv#25 razor-Sonar <NoWarn>S6966;S3267;…</NoWarn> line from ErsatzTV.csproj — those Sonar rules only needed suppression in .razor @code; on .cs they run at suggestion via .editorconfig, so removal is safe (closes part of #25's burn-down).

Rollback. The tag blazor-final was cut on the pre-removal main commit as the first step (see the 2026-07-11 "Pre-removal Blazor rollback tag blazor-final (#205)" entry above for the exact command + restore path). Not a v* tag → no prod release build.

2026-07-11 — Stable child identity for schedule-item replace (#259, split from #252/#253)

PUT /api/schedules/{id}/items now reconciles by an optional round-tripped child id, not by array position, so an item's persisted fill-group/shuffle state (PlayoutScheduleItemFillGroupIndex, FK OnDelete(Cascade)) follows the logical item across reorders/inserts instead of being inherited by whatever previously held its new slot. Contract + rules in api-conventions §7c. Key decisions:

  • ScheduleItemRequest.Id (int?): null/absent/0 ⇒ new item (controller normalizes 0→null so the handler is two-state). Any id present ⇒ id-based reconcile; a fully id-less payload keeps the verbatim positional fallback (legacy; retires with the §7a Phase-2 If-Match→428 flip).
  • Unknown or duplicate id ⇒ 422, nothing persisted; the guards live in the handler after §7a CheckVersion, so 412 precedes 422 — a client that is both version-stale and id-stale gets the reload signal, not a payload-bug signal. Rationale for reject-not-insert on an unknown id: under Phase-1 force-write a stale id is a live lost-update signal, so silently inserting-as-new would duplicate the item and return a different id than the client sent (the exact class §7a exists to surface). This is also the correct #197 posture — never honor an unrecognized identifier.
  • Scope = schedule items only. Blocks/templates/deco-templates/playlists stay positional: their children are stateless config rows (no FK'd state to misattribute; #3/#4 have no GET child id). Child ids are added only where a child row anchors server-side state; the contract can be retrofitted per-endpoint later (field stays optional) — so this is not #197 ossification pressure.
  • TPT subtype change at a matched id stays delete+insert (EF can't retype in place); state resets and a new id is returned, so the SPA must re-seed item state from the PUT response (a stale id on a second save now 422s).

2026-07-11 — Baseline security response headers + Phase-0 API hardening (#197, PR #279)

Phase-0 of the #197 remediation — the posture-independent safe subset, shipped ahead of the fail-closed/CORS/versioning posture work tracked in #280–#289.

  • Baseline security headers on every response. New ErsatzTV/Middleware/SecurityHeadersMiddleware, registered first in the pipeline (before the /iptv MapWhen branch and UseCors), so it covers /api, /iptv, /artwork, static, the SPA fallback, and filter-produced 4xx alike — which is why it's middleware, not an MVC filter. It sets X-Content-Type-Options: nosniff, X-Frame-Options: DENY, and Referrer-Policy: strict-origin-when-cross-origin. nosniff is the standing backstop for the artwork content-type MIME-sniffing risk (#283). CSP and HSTS are deliberately NOT included here: CSP must be validated against the ChicoryTV SPA's inline assets, and HSTS is a proxy/TLS-termination decision — both belong to the #197 posture design (#284/roadmap), not this baseline. Headers are set eagerly (not via Response.OnStarting); safe today because the pipeline has no UseExceptionHandler/UseStatusCodePages that would Response.Clear() — switch to OnStarting if one is ever added.
  • Constant-time API-key comparison. ApiKeyAuthorizationFilter compares X-Api-Key with CryptographicOperations.FixedTimeEquals (over UTF-8 bytes) instead of ordinal string.Equals, removing the response-timing oracle on the write key. Accept/reject behavior is otherwise identical.
  • Playout pagination clamped. GET /api/playouts and GET /api/playouts/{id}/items now clamp Math.Clamp(pageSize, 1, 100) + Math.Max(0, pageNum) before the query — applying the api-conventions §1 clamp convention the other paged endpoints already follow (these two were passing the raw client value straight to EF Take()).

The larger #197 posture (fail-closed writes, sensitive-read auth tier, CORS lockdown, /api/v1 versioning, the OpenAPI security scheme) is decomposed into #280–#289 with the phased roadmap on #197; those PRs will append their own decisions here as they land.

2026-07-12 — Artwork content-type is sniffed, never reflected (#283, S4/S9 stored XSS)

The artwork upload/serve path trusted client-supplied content types at both ends, giving a stored-XSS chain on unauthenticated GET sinks: upload <script> bytes declared image/png → GET /iptv/logos/{hash}?contentType=text/html served them as HTML in the ErsatzTV origin. The #279 nosniff header is not a fix here — the server was explicitly declaring text/html, which the browser honors regardless of nosniff. The trust was the bug; the fix removes it at both ends.

  • Upload derives the content type from the bytes, never the declared value. UploadArtworkHandler buffers the (size-bounded) upload and calls ErsatzTV.Core/Images/ImageContentTypes.DetectContentType, which uses SkiaSharp's SKCodec to identify the format from the image header only — pixels are not decoded, so this can't be turned into a decompression-bomb vector. A payload that isn't one of the accepted raster formats (png/jpeg/gif/webp) is rejected 422; the declared Content-Type is no longer read at all (the field was dropped from the UploadArtwork command).
  • Serve sniffs the stored file; the ?contentType= reflection is gone. GetCachedImagePath no longer carries a ContentType, and GetImage (/iptv/logos) / GetWatermark (/artwork/watermarks) dropped their [FromQuery] contentType binding. GetCachedImagePathHandler always derives the MIME type from the file (MimeTypes.GetMimeTypeFromFile) and clamps it to the image allow-list (ImageContentTypes.IsAccepted), serving application/octet-stream for anything else — so a file whose bytes are not an accepted image (a legacy cache entry poisoned before the upload sniff landed, or a hypothetical polyglot) is a non-renderable download, never HTML/script. The removal is structural — there is no longer any request path that lets a client choose the served Content-Type. ArtworkContentTypeModel.UrlWithContentType now returns the bare path, and the SPA watermark/logo previews no longer append the query.
  • Defense-in-depth on the persisted JSON DTOs. The {path, contentType} bodies (channel logo, watermark) run their content type through ArtworkContentTypeModel.Sanitized(), which blanks anything outside the image allow-list before it is stored — so a stale/hostile value can't be reflected by any future code path even though the serve route already ignores it.
  • S9 upload-size DoS. Kestrel Limits.MaxRequestBodySize is now set from ETV_MAXIMUM_UPLOAD_MB, so an oversized body is rejected as it is read rather than only after the controller's post-binding file.Length check (kept as the friendly-error backstop). This is a global bound; the app has no other large inbound body (streaming is outbound GET).

ImageContentTypes is the single source of truth for the accepted image types (the allow-list previously duplicated in UploadArtworkHandler). Both serve sinks are [ApiExplorerSettings(IgnoreApi = true)], so none of this changes the OpenAPI document.

2026-07-12 — Fail-closed API auth + sensitive-read tier + CORS/ForwardedHeaders lockdown (#197 Bundle A, PR #292)

Phase-1 of the #197 remediation — the auth posture that must land before any remote exposure. Owner decisions (confirmed this session): single API key (no read/write split), and Api:RequireKeyForReads defaults true (the whole /api surface requires the key). This does not affect Jellyfin/streaming: /iptv/* (playlist/guide/streams/logos) and /artwork/* are outside the filter's /api scope and keep their own optional access-token; only the management API the SPA talks to is gated.

  • Fail-closed writes (#280, S1). The empty-key "open" branch is deleted; there is no open mode. New IApiKeyProvider (ErsatzTV/Services/ApiKeyProvider.cs, singleton, resolved once at startup) yields a never-empty key: Api:WriteKey if set, else a key persisted at FileSystemLayout.ApiKeyPath (/config/api.key, 0600, path logged not value), else a generated 256-bit hex key. Every mutating /api request now requires X-Api-Key.
  • Sensitive-read tier (#282, S3/S5). Reads are gated by Api:RequireKeyForReads (default true) OR a new [RequiresApiKey] marker (mirror of [SkipApiKeyAuthorization]) applied to Troubleshoot/Logs/Settings/Maintenance, so that tier stays gated even if an operator opts reads open. OPTIONS preflight is exempt (CORS middleware owns it). ApiControllerSecurityTests asserts the tier reflectively.
  • Delete dead non-/api mutation surfaces (#281, S2). SortController (POST media/collections/{id}/items, dead Blazor SortableJS residue — the SPA uses PUT /api/collections/{id}/custom-order) and AccountController (POST account/logout, dead OIDC) bypassed the key because they sat outside /api. Removed rather than guarded.
  • CORS opt-in (#284, S6). AllowAnyOrigin/Method/Header is replaced by the ApiCors policy: an exact-origin allowlist from Api:CorsAllowedOrigins (semicolon list) that permits X-Api-Key/If-Match and exposes ETag; with no origins configured there is no cross-origin access (the SPA is same-origin).
  • ForwardedHeaders trust + scanner loopback (#285, S7/S10). GET /api/maintenance/gc → POST (crawler-triggerable GC; spec regenerated). ForwardedHeaders trust is configurable via ForwardedHeaders:KnownProxies/KnownNetworks — unconfigured preserves the current trust-all behavior but logs a warning (flipping the default to loopback-only would break reverse-proxy scheme/host detection and thus M3U/XMLTV absolute URLs — the operator must name their proxy network). ScannerController gains [LocalhostOnly] (the scanner always calls back over http://localhost:{UiPort}/api/scan/...), which is only spoof-resistant once ForwardedHeaders trust is restricted — the two interlock. search/all-items DoS-paging is deferred (it feeds the SPA "add all" flow and needs coordinated pagination; the unauth exposure is already closed by read-gating).
  • SPA (web/). The client sends the stored key (ctv-api-key) on every method (not just mutations); a new keyless API Key screen (/app/api-key) lets the user paste the generated key, and a shell-level banner points there on any 401. See spa-conventions §5e. First-run/upgrade UX: with reads gated by default, the SPA shows no data until the key (from /config/api.key) is entered — an intended consequence of the strict default.

Phase-2 (contract freeze) — the declarative OpenAPI security scheme, global 401 docs, and /api/v1 versioning — remains #286/#287/#288. Phase-3 follow-ups: #265, #269, #172 remainder, search/all-items paging, per-key rate limiting.

2026-07-12 (#197 Bundle C — contract-freeze honesty)

#287 — OpenAPI contract honesty by construction. The "v1" document now emits the ApiKey security scheme plus per-operation security/401 derived from the same ApiKeyAuthorizationFilter.EndpointRequiresKey predicate the runtime filter enforces, so declared auth can never drift from enforced auth. Every operation also gets a synthesized stable operationId (the framework only assigned one when Name= was set — ~90 were missing), and body/param-binding operations get the documented 400 ValidationProblemDetails they actually return. DayOfWeek is now a string enum in the schema (added to Startup.UseStringEnumSchemas), removing the SPA's WithDayNames wart. Pinned by in-process document generation in tests (OpenApiContractHonestyTests) rather than the committed v1.json.

#288 — Wrap the last raw ViewModels; reverse the §7a "intentional version leak." Minted MediaCollectionResponseModel, ProgramScheduleResponseModel, and ChannelDetailResponseModel (all #nullable enable) and routed CollectionController / ScheduleController / SmartCollectionController / ResolutionController.GetResolutionByName / the channel detail GET+writes through ResponseModels, so no /api/* action returns an Application VM. This reverses the earlier §7a judgment that a ResponseModel "purely to hide one field was disproportionate": Version is now header-only (ETag) on every aggregate body — confirmed safe by grepping web/src (the SPA reads version from the ETag header, never the response body). ChannelDetailResponseModel is the full editable field set the channel editor needs (distinct from the lean list ChannelResponseModel; drops only the derived webEncodedName). Also flipped #nullable enable onto the remaining 24 lagging ErsatzTV.Core/Api/ files for schema honesty, and added pageNum paging to GET /api/search.

Channel REST resources are keyed by database Id, never by Number. Channel.Number is user-mutable (editable on update, bulk-renumbered via /api/channels/bulk/renumber, transiently invalid mid-renumber), so the immutable int PK is the canonical key for all /api/channels/* single-item routes, sub-resources (including playout/reset, re-keyed from {channelNumber} to {id:int} in Bundle C), and Location headers. Number remains the identity on broadcast surfaces only (IPTV/M3U/XMLTV), a separate contract. A number-based lookup endpoint may be added additively later; UniqueId (Guid) stays out of the REST contract absent a federation requirement.

2026-07-12 (#269 — non-If-Match root writers force-write past a concurrent Version bump)

Routing the aggregate delete handlers + UpdateProgramScheduleHandler through SaveChangesForcingVersion. Once #253 made each replace-all root's Version an IsConcurrencyToken, EF started guarding every UPDATE and DELETE of that row with WHERE Version=@orig — so any writer that is not part of the If-Match contract but still saves via plain SaveChangesAsync throws an unhandled DbUpdateConcurrencyException→500 if a replace-all editor bumps the row in its narrow load→save window. PR3 already force-wrote the exposed UPDATE siblings (Playout settings/ScheduleFile/checkpoint, UpdateCollectionHandler); a completeness sweep for #269 found the gap was wider than reported — 18 writers in total, all on plain SaveChangesAsync. The correct exposure filter is "any handler that leaves a versioned root Modified or Deleted", NOT just Version-bumpers + deletes — an early sweep used the narrower filter and a review of PR #302 caught what it missed (ErasePlayoutHistory below):

  • the nine versioned-root delete handlers (DeletePlayout/DeleteCollection/DeleteMultiCollection/ DeleteRerunCollection/DeletePlaylist/DeleteBlock/DeleteTemplate/DeleteDecoTemplate/ DeleteProgramSchedule) — a DELETE is now token-guarded too;
  • UpdateProgramScheduleHandler (bumps Version then saved plainly — the ProgramSchedule case PR3 only suspected);
  • the seven item add/remove bumpers that PR2 wired to bump their root but left on plain save — AddProgramScheduleItem/DeleteProgramScheduleItem and the five Add{Items,Movie,Show,Season,Episode}ToPlaylist handlers;
  • ErasePlayoutHistoryHandler — modifies Playout root scalars (Seed/Anchor/OnDemandCheckpoint) without bumping Version, inside an explicit transaction with no try/catch → the one the bumper-only filter missed; reachable via POST /api/playouts/{id}/erase-items-and-history.

All now save through ConcurrencyExtensions.SaveChangesForcingVersion.

Two deliberate boundaries (documented, not gaps): (1) the background build/time-shift Playout-scalar writers (BuildPlayoutHandler via PlayoutBuilder's Anchor/Seed; PlayoutTimeShifter's OnDemandCheckpoint) are token-guarded too but intentionally left on plain save — they never surface a request-path 500 (BuildPlayoutHandler catches → a build-failure BaseError; PlayoutTimeShifter runs only via the background worker), and force-writing would be wrong: a concurrent config edit that bumped Version also enqueues a rebuild, so failing the in-flight build and letting the rebuild redo it with fresh config is correct (force-writing would persist output built from stale config). (2) Item-add force-write can leave a duplicate/gap Index (accepted Phase-1 effect): the handler computes the new index from its stale child list, so if a concurrent replace-all grew the list the item lands at a now-colliding index (no unique constraint on PlaylistItem.Index/ProgramScheduleItem.Index) — non- corrupting, self-correcting on the next edit, still strictly better than the pre-#269 500; a reload-and-recompute-on-conflict refinement is a candidate for #197. Decision: force-write, not 412 — these endpoints take no If-Match (an unconditional DELETE/settings-edit should win over a concurrent editor), matching the Phase-1 force-write posture. A delete has no ETag to rotate, so it needs only the force-write, not a Version bump. A genuine row-deletion race (two concurrent deletes) still surfaces as a DbUpdateConcurrencyException — accepted (rare, non-corrupting, the resource is already gone). Still deferred to #197: cross-editor ETag rotation for the non-bumping config siblings and the scanner-shared Add*ToCollection family (they don't 500 — they insert children / ExecuteDelete, neither of which is token-guarded — they just don't rotate an open editor's ETag). Non-vacuously tested by racing a bump through the handler via a pre-tracked context (RootWriterForceVersionTests), plus an explicit negative control proving the plain-save path throws.

2026-07-12 — Browser SPA session auth: /api accepts session OR machine key (#295 PR1, server-only)

Implements the ratified #295 design (Fable [PLAN-MODE] pass, issue comment 9548). Supersedes the #206 "OIDC wiring stays inert until #197" note: the retained OIDC service registration is now revived, and a cookie session becomes a first-class /api credential alongside the machine X-Api-Key. PR1 is server-only and backward compatible — the SPA keeps sending its stored key; the SPA login flow, the ApiKeyScreen→machine-key repurpose, and spa-conventions §5e land in PR2.

One gate, evolved (not [Authorize]-per-controller). ApiKeyAuthorizationFilter → ApiAuthorizationFilter, same fail-closed-by-omission logic (a forgotten [Authorize] fails open — the #280 failure mode — so the global filter stays the gate). It now accepts a request when a valid X-Api-Key matches OR the principal is an authenticated session; the "does this endpoint need auth?" decision is still the single shared EndpointRequiresKey(...) predicate (also drives OpenAPI, so the spec can't drift). Attributes renamed to match the widened meaning: [RequiresApiKey]→[RequiresAuthentication], [SkipApiKeyAuthorization]→ [SkipApiAuthorization]. IApiKeyProvider, the X-Api-Key header, and Api:WriteKey/Api:RequireKeyForReads are unchanged — machine/key behavior is byte-identical (verified: no OpenAPI drift, existing filter tests still green).

CSRF (session only). The machine key is CSRF-immune (a browser can't set a custom header cross-origin without a credentialed CORS grant we never issue). A cookie session is not: a session-authenticated mutation must carry the X-CSRF header (presence-only — a custom header forces a CORS preflight a cross-site page can't satisfy) or is rejected 403. Reinforced by SameSite=Lax + CORS without AllowCredentials (cross-origin cookie auth is impossible by design). No antiforgery-token machinery.

Cookie ctv-session. Always registered (local login works with no IdP); OIDC handler added only when OIDC:* is configured. HttpOnly, SameSite=Lax, SecurePolicy=SameAsRequest (so a plain-HTTP LAN isn't bricked), 14-day sliding. /api XHR gets 401/403, not a redirect (OnRedirectToLogin/AccessDenied). The UseAuthentication/UseAuthorization middleware — deleted with Blazor in #91b — is revived in the legacy MapWhen branch only (hosts /api + OIDC /callback + /docs; /iptv and /app untouched).

Local store = ConfigElement rows, single admin, NO migration (owner ruling F2): AuthLocalAdminUsername, AuthLocalAdminPasswordHash (ASP.NET PasswordHasher, PBKDF2, via Microsoft.Extensions.Identity.Core), AuthSecurityStamp. A password change rotates the stamp; the cookie OnValidatePrincipal (CookieSecurityStampValidator) compares the claim to the stored stamp and rejects a stale session (revocation). OIDC sessions carry an etv:auth_method=oidc claim and skip the stamp check (governed by the IdP).

Fail-closed out of the box + recovery. An unconfigured instance keeps /api gated (the key still works); first-run is a setup-claim (POST /api/auth/setup, first-claim-wins, only valid while unconfigured — owner ruling F1). Recovery without the browser: Auth:LocalAdmin:Password env seed (LocalAdminSeedService, overwrites + rotates the stamp on startup) or the machine key. Login hardening: per-IP rate limit ([EnableRateLimiting("auth")], 10 / 5 min) on login/setup/password, dummy-hash verify on unknown/unconfigured user (no enumeration).

Authelia = app-owned OIDC session; never trust proxy identity headers (owner ruling F3): the container is LAN-reachable bypassing the proxy, so Remote-User/Remote-Email header trust is spoofable. OIDC→Authelia gives SSO without a double login. ForwardedHeaders behaviour is kept unchanged from #285 (trust any peer with a warning; restrict via KnownProxies/:KnownNetworks). A stricter "ignore X-Forwarded-* unless a proxy is configured" default was implemented and then reverted after review (cold fork M1): the forwarded scheme/host feed /iptv M3U/XMLTV/HLS absolute-URL generation (Request.Scheme in GetChannelGuideHandler/ IptvController), so ignoring them would regress stream URLs to http/internal-host for a proxied deployment that hasn't set KnownProxies. Deployment coordination: operators behind a proxy should set ForwardedHeaders:KnownProxies/:KnownNetworks — it gives the login rate limiter an unspoofable client IP and marks the session cookie Secure behind TLS. The residual (a direct LAN peer can spoof X-Forwarded-For to evade the per-IP login limit when unrestricted) is accepted defense-in-depth loss, mitigated by PBKDF2 + no-enumeration.

Review hardening (fork + independent Codex pass, folded into PR1). Codex caught concurrency defects the fork missed — folded in: (a) atomic first-claim-wins — setup writes the three credential rows in one transaction guarded by the unique ConfigElement.Key index (a lost race → DbUpdateException → 409), so a concurrent claim can't produce a mixed-state credential; (b) consistent login snapshot — login reads the hash + stamp in one query and no longer rehashes-on-verify, so a login racing a password change can't capture a newer stamp than the hash it verified (a concurrent change either fails the old password or leaves the issued cookie carrying the pre-change stamp → revoked next request); (c) env-seed waits on SystemStartup.WaitForDatabase (the migrator is a BackgroundService, so registration order alone didn't guarantee the schema existed) — moved to Services/RunOnce/. Also: logout + password require X-CSRF (the [SkipApiAuthorization] auth surface isn't covered by the filter's CSRF check → forced-logout CSRF), and input length caps on username/password. Logout rotates the security stamp when called from a local session (E2E-caught: SignOutAsync alone only clears the client cookie, leaving the stateless encrypted ticket replayable server-side) — so signing out actually ends the session server-side; for the single admin this revokes all local sessions ("log out everywhere"). Gated on an authenticated session so an unauthenticated caller can't force-revoke the admin. Deferred with a tracked gate: side-effecting [RequiresAuthentication] GETs (troubleshoot playback/archive) aren't CSRF-covered — #301, gating PR2 (latent in PR1: the SPA still uses the machine key). OIDC-session revocation lever (no local stamp) noted for PR3 operator docs.

A fix-commit re-review (Codex, #242 discipline) then confirmed the above resolved and caught a second round: (a) 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's credential persists, defeating the env recovery path). Fixed structurally: the setup-claim endpoint is closed whenever Auth:LocalAdmin:Password is configured — the env seed owns the credential, so there is no claim to race (this also strengthens the setup-claim TOFU posture: an operator on an untrusted network sets the env password and browser setup is disabled). (b) LOW: a concurrent setup race-loser now returns 409 (not 422), and ClaimLocalAdmin's DbUpdateException catch re-checks existence and rethrows genuine/transient DB errors rather than masking them as "already configured". (c) MEDIUM — accepted: two simultaneous authenticated password changes are a non-serializable lost-update (last-write-wins; the loser's cookie may be immediately revoked). Accepted for a single-admin system: it needs two concurrent authenticated sessions both submitting the correct current password at the same instant, and the outcome is self-healing (re-login). Adding EF optimistic concurrency to the credential rows is disproportionate here.

OpenAPI = ApiKey-only; /api/auth/* excluded (owner ruling F4): the spec's audience is machine/MCP clients, and a browser-interactive cookie login isn't something a generated client drives, so the cookie path is an additional accepted credential the doc needn't express. AuthController is [ApiExplorerSettings(IgnoreApi = true)]. Verified: no v1.json/v1.d.ts/endpoint-index drift from this PR.

Phasing. PR1 = this (server only, no migration). PR2 = SPA (drop the key header for browser calls + add X-CSRF, AuthContext + boot gate, login/setup screens, ApiKeyScreen→machine-key management, E2E, spa-conventions §5e). PR3 = key rotation + operator docs (Authelia client + env reference). Rollout: PR1→PR2 same release, then a manual Authelia round-trip checklist before the prod pin bump.

2026-07-12 — Merge-consent derived from state via a ## Done-when issue checklist (#303 H6)

An issue's ## Done-when checklist (in the issue body) is the machine-readable source of truth for whether its PR may merge; consent is derived, not asserted. Rationale: DONE/OPEN status used to live in append-only prose that lags live Gitea state (the queue-drift #303 fixes) — so the completion gate moves out of memory and into a checklist two hooks read. Convention: the issue body carries a ## Done-when section (always an "adversarial review passed" box, plus per-issue criteria); a merge is allowed only when the PR's CI is green and every box on the linked issue (fixes #N) is ticked.

Enforcement (both fail safe, never a silent pass):

  • pretooluse-merge-consent.sh — Claude PreToolUse on mcp__gitea__pull_request_write merge: deny on an unticked box or non-green CI; allow when both satisfied; ask (human prompt) when state isn't derivable (no linked issue, no ## Done-when, no creds, Gitea unreachable). Docs-only PRs exempt.
  • .husky/pre-push → prepush-donewhen.sh — backstop for a direct git push origin main; fail-open (a git hook has no "ask"), blocks only on a positively-proven unticked box.

Both authenticate to Gitea from env only (ETV_GITEA_BASICAUTH / ETV_GITEA_TOKEN, ETV_GITEA_URL) — no creds committed; without them the gate degrades to today's manual confirmation. Rollout is non-breaking: until issues adopt ## Done-when, the merge hook simply asks rather than auto-allowing. See CLAUDE.md → Task Completion Protocol. (H6 lives with H1/H2/H8 in .claude/settings.json; H7 worktree-owner guard is its sibling Wave-2 hook.)

2026-07-12 — Blocking CI gate for API-contract artifacts (#303 H4/H5)

A PR whose diff touches ErsatzTV/Controllers/Api/** or ErsatzTV.Core/Api/** must ship the regenerated OpenAPI artifacts in that same diff, enforced by a blocking api-docs CI job. It rebuilds ErsatzTV/wwwroot/openapi/v1.json, web/src/api/generated/v1.d.ts, and docs/endpoint-index.md from source and fails on any drift. This mechanizes the previously prose-only "docs-update in the same PR" rule for the API contract (the docs-reminder job stays a non-blocking nudge for the route-parity doc). Path-gated inside the job (not via a top-level if:) so it always reports a status on every PR — API-free PRs skip the expensive regen and pass trivially, so it is safe as a required check. Rationale: generation is deterministic from a fresh build (verified — a clean checkout reproduces the committed spec exactly, including the 244 auth security/401 blocks), so CI can trust regenerate-and-diff. The one caveat is local-only: update-openapi.sh runs dotnet-getdocument against the already-built assembly, so a stale bin/ silently yields a stale spec — always dotnet build first (see api-conventions.md §5). CI is immune (no bin/ on a fresh checkout).

2026-07-12 — decisions.md is append-only, enforced; root-screenshot guard (#303 H9/H3)

This log is append-only by construction, not just by convention. A commit or PR that deletes or modifies an existing line of docs/decisions.md is blocked — by the Husky commit-msg hook (.claude/hooks/decisions-guard.sh staged) locally and the blocking decisions-guard CI job (same script, range mode) on PRs. Shared detection, deliberately different granularity: the Husky hook gates each commit (its own message must carry the token); CI gates the PR-wide net diff (token in any commit of the range suffices), so the local hook is the stricter primary gate and CI the push/bypass backstop. Insertions anywhere are always allowed, so a normal new entry (TOC line near the top + a block appended at the bottom, both pure insertions) passes untouched. Detection is git diff --numstat deleted-count > 0, which is robust to markdown - list markers (a byte-level - prefix would false-match). The block is lifted only by the literal [decisions-edit] token in the commit message, reserved for two cases: fixing a factual error, and superseding a reversed decision (add the new entry, prepend a > **Superseded …** banner to the old one, tag its Index line (superseded) — keep the old rationale, never silently rewrite). Consolidation of superseded entries is a release-checklist step (docs/ci-cd.md → Versioning & releases), backstopped by a non-blocking 1800-line size floor in the decisions-guard job (the read-cost point past which the log no longer fits one default agent Read), so append-only doesn't accrete contradictory or unreadably-large history between releases (Timothy's call, 2026-07-12: mark-and-keep on reversal, consolidate at each milestone, size-floor backstop).

Companion guard H3: the Husky pre-commit hook refuses a staged root-level *.png (a review/debug screenshot dropped at the repo root) — belt-and-suspenders with the .gitignore rule, so a forced git add -f still can't land one. Nested *.png (real assets) are unaffected. Rationale for both: the methodology review (#303) — make the process rules derivations/hooks, not prose to remember.

2026-07-12 — Review-verdict merge-gate: latest commit must be reviewed (#303 H10)

A PR may not merge until a Review-verdict: comment on it references the PR's CURRENT head sha — so the latest commit is proven-reviewed, not a stale earlier diff. This mechanizes the ersatztv#242 lesson ("re-review the fix commit, not just the initial PR diff": a review of an earlier revision does not license merging a head that carries un-reviewed follow-up commits). It folds into the existing H6 pretooluse-merge-consent.sh as condition (c), reusing its PR fetch, docs-only exemption, and Gitea-auth-from-env (no second hook → no detection drift, per the #303 methodology review).

Convention: after reviewing a PR (or its latest fix commit), post a PR comment (issue-style, not a Gitea formal-review body — the gate reads issues/{pr}/comments) whose line starts with the marker: Review-verdict: <MERGEABLE|APPROVED|BLOCKED|NOT-MERGEABLE> @ <head-sha> (short ≥7-char or full sha). The gate counts a line as a verdict only when the marker is at line-start (after optional indent) — a comment that merely quotes the template mid-sentence (an instruction "please post: Review-verdict: MERGEABLE @ …", or the gate's own suggestion text echoed back) does not self-approve the merge (adversarial re-review false-open, folded pre-merge). It then classifies each verdict line by the sha in its @ <sha> field, matched to the head by git short-sha prefix semantics (head begins with the token, token ≥7 chars) — NOT a loose substring test, so an older sha that merely contains the head prefix, or the head prefix appearing in an unrelated URL on the line, does not count:

  • a MERGEABLE/APPROVED/LGTM verdict whose @ <sha> is the current head → allow;
  • a negative verdict (BLOCKED/NOT-MERGEABLE) on the head → deny, and it wins over a positive one on the same head (a later BLOCKED retracts an earlier MERGEABLE; to retract, re-review head and post BLOCKED @ head). Staleness is symmetric on purpose: a negative for an older commit is stale exactly like a positive for an older commit, and does NOT override a fresh head-positive — otherwise a pre-fix BLOCKED @ oldsha would block forever even after the fix changes the sha and earns a fresh MERGEABLE @ head (the normal flow). So a genuine block must reference head, per the convention;
  • verdict comment(s) exist but reference only older commits → deny — the stale-review case #242 targets;
  • a Review-verdict: marker with no @ <sha> at all → ask (a lazy/quoted marker; not mislabelled as stale);
  • no Review-verdict: comment at all → ask (graceful adoption, mirrors H6's "no Done-when → ask": surface, don't hard-block a PR that hasn't adopted the convention yet);
  • comments unfetchable / head sha unresolvable → ask.

Scope: the Claude PreToolUse gate on the Gitea merge tool only. A direct git push origin main has no PR comments to check, so the .husky/pre-push backstop is not extended for H10 (the merge tool is the real merge path; docs-only PRs remain exempt via H6's file-set exemption). Rationale, as with the whole Wave-1/2/3 hook set: make the process rule a derivation/hook, not prose to remember (#303).

Reference in New Issue View Git Blame Copy Permalink