docs(563): supersede the scripted-golden deferral with the in-process coverage rule

The deferral record described ScriptedScheduleController as a "1:1 pass-through" to SchedulingEngine. It is not: an unparseable playback order is a 400, an unparseable filler kind SILENTLY degrades to FillerKind.None, an unknown build id is a 404, and the engine's no-progress InvalidOperationException is translated to a 400. Carrying that wording forward would have shipped a false statement, so the successor states a thin adapter with named mappings, each pinned by a test. - new record testing.scripted-engine-in-process-net (active, since 2026-09-05) - predecessor testing.scripted-playout-golden-deferred git mv'd to docs/decisions/archive/testing/ with frontmatter retargeted only; body prose byte-identical, so no Decisions-Edit trailer - docs/decisions.md Index line retargeted to the archive path plus a new dated line for the successor - catalog regenerated with scripts/build_decisions_catalog.py - docs/testing.md: the Golden-file nets paragraph now points at the new coverage instead of "tracked in ersatztv#563"; a new "Scripted playout coverage" section states what is covered where and what is deliberately not covered (Cli.Wrap launch, Kestrel + Startup middleware, ApiAuthorizationFilter), dated 2026-09-05; Timezone independence records the per-call TZ audit that decided which engine instructions the fixtures may use. Refs #563 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV
2026-09-05 09:25:11 +02:00
parent b3bc3ed115
commit 0d2cd89782
5 changed files with 100 additions and 14 deletions
@@ -202,7 +202,7 @@ another doc or an old issue comment should land here and then follow the link.
 - 2026-07-21 — `WorkAheadSlots.Release()` clamps before decrementing and reports unbalance in-band (#539) — [`ffmpeg.work-ahead-slot-release-never-negative`](decisions/records/ffmpeg/work-ahead-slot-release-never-negative.md)
 - 2026-07-21 — `from-lineup` advanced overrides express "clear to none" via a typed `clear` enum list (#135) — [`api.from-lineup-clear-to-none`](decisions/records/api/from-lineup-clear-to-none.md)
 - 2026-07-22 — Channel-level graphics-element attachment + seeded On Now/Next text element (#74) — [`graphics.channel-level-attachment`](decisions/records/graphics/channel-level-attachment.md)
- 2026-07-22 — Sequential (YAML) playout gets a golden; Scripted is excluded from the golden net by construction (#381) — [`testing.scripted-playout-golden-deferred`](decisions/records/testing/scripted-playout-golden-deferred.md)
+- 2026-07-22 — Sequential (YAML) playout gets a golden; Scripted is excluded from the golden net by construction (#381) — [`testing.scripted-playout-golden-deferred`](decisions/archive/testing/scripted-playout-golden-deferred.md) (superseded by `testing.scripted-engine-in-process-net`)
 - 2026-07-22 — per-schedule clock-boundary padding is a synthetic content-less Pad over the existing per-episode machinery (#392) — [`sched.clock-padding-schedule-toggle`](decisions/records/sched/clock-padding-schedule-toggle.md)
 - 2026-07-23 — Channel health = a server-derived `health` object on the channel DTOs, built-timeline detection (#415) — [`api.channel-health-object`](decisions/records/api/channel-health-object.md)
 - 2026-07-23 — Channel origin is immutable creation-provenance, stamped at insert, not a health signal (#414) — [`channel.origin-marker`](decisions/records/channel/origin-marker.md)
@@ -216,3 +216,4 @@ another doc or an old issue comment should land here and then follow the link.
 - 2026-07-25 — The rationale-edit marker is a git trailer, not a substring anywhere in the commit range (#609) — [`ci.decisions-edit-trailer`](decisions/records/ci/decisions-edit-trailer.md)
 - 2026-07-25 — UI-E2E: headless Playwright flows in the existing `functional-e2e` job, browser baked into the CI image (#445) — [`ci.ui-e2e-harness`](decisions/records/ci/ui-e2e-harness.md)
 - 2026-07-26 — Facet-value typeahead restated: every artist source covered; the JSON-column source is paged by row position with no residual SQL predicate (#578) — [`api.search-field-values-sources`](decisions/records/api/search-field-values-sources.md)
+- 2026-09-05 — Scripted playout is covered in-process, engine plus controller adapter; the Cli.Wrap process and the HTTP transport are permanently out of the automated suite (#563) — [`testing.scripted-engine-in-process-net`](decisions/records/testing/scripted-engine-in-process-net.md)
@@ -1,13 +1,13 @@
 ---
 key: testing.scripted-playout-golden-deferred
 title: 2026-07-22 — Sequential (YAML) playout gets a golden; Scripted is excluded from the golden net by construction (#381)
-status: active
+status: superseded
 since: '2026-07-22'
 supersedes: none
-superseded-by: none
-rule: 'The `PlayoutBuildGoldenTests` in-memory golden net covers Sequential (YAML) as of #381. Scripted''s *end-to-end pipeline* is excluded — `ScriptedPlayoutBuilder` runs a user-authored external program that drives the engine over HTTP loopback, which the in-memory harness can''t pin — so that full-pipeline (integration) harness is deferred to #563. But the scheduling *behavior* those scripts drive lives entirely in the in-process `SchedulingEngine` (the `ScriptedScheduleController` is a 1:1 pass-through to it), which IS directly unit/golden-testable; the earlier "Scripted is un-golden-able by construction" framing overstated the constraint by conflating transport with engine. #395 extracts that shared switch to `ContentEnumeratorBuilder` and adds a direct regression net (`ContentEnumeratorBuilderTests`) over it.'
+superseded-by: testing.scripted-engine-in-process-net@2026-09-05
+rule: '(superseded) The `PlayoutBuildGoldenTests` in-memory golden net covers Sequential (YAML) as of #381. Scripted''s *end-to-end pipeline* is excluded — `ScriptedPlayoutBuilder` runs a user-authored external program that drives the engine over HTTP loopback, which the in-memory harness can''t pin — so that full-pipeline (integration) harness is deferred to #563. But the scheduling *behavior* those scripts drive lives entirely in the in-process `SchedulingEngine` (the `ScriptedScheduleController` is a 1:1 pass-through to it), which IS directly unit/golden-testable; the earlier "Scripted is un-golden-able by construction" framing overstated the constraint by conflating transport with engine. #395 extracts that shared switch to `ContentEnumeratorBuilder` and adds a direct regression net (`ContentEnumeratorBuilderTests`) over it.'
 signals: 'why is there no scripted golden; scripted transport vs engine; SchedulingEngine is in-process testable; Cli.Wrap external process is transport only; ScriptedScheduleController 1:1 pass-through; engine-level scripted regression net; EnumeratorForContent · paths: `ErsatzTV.Core.Tests/Scheduling/Goldens/PlayoutBuildGoldenTests.cs`, `ErsatzTV.Core.Tests/Scheduling/Engine/SchedulingEngineTests.cs`, `ErsatzTV.Core/Scheduling/Engine/SchedulingEngine.cs`, `ErsatzTV/Controllers/Api/ScriptedScheduleController.cs`, `ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilder.cs` · issues: #381, #163, #395, #563'
-mechanics: docs/testing.md → Golden-file nets
+mechanics: superseded by `testing.scripted-engine-in-process-net` (ersatztv#563), which stands up the deferred coverage in-process and corrects the "1:1 pass-through" description of `ScriptedScheduleController`
 ---

 **Sequential (YAML) is golden-able and TZ-independent.** `SequentialPlayoutBuilder` reads a YAML schedule
@@ -0,0 +1,54 @@
+---
+key: testing.scripted-engine-in-process-net
+title: '2026-09-05 — Scripted playout is covered in-process, engine plus controller adapter; the Cli.Wrap process and the HTTP transport are permanently out of the automated suite (#563)'
+status: active
+since: '2026-09-05'
+supersedes: testing.scripted-playout-golden-deferred@2026-07-22
+superseded-by: none
+rule: 'Scripted playout is characterized at two in-process levels and no further. (1) `SchedulingEngineTests` drives the engine build API directly with substituted repositories — the same setup order `ScriptedPlayoutBuilder` uses (`WithPlayoutId`/`WithMode`/`WithSeed`/`BuildBetween`/`WithReferenceData`/`RestoreOrReset`, and `WithReferenceData` MUST precede the last two or `PlayoutReferenceData.PlayoutHistory` NREs). (2) `ScriptedScheduleControllerTests` replays a committed script fixture (`ErsatzTV.Tests/Controllers/Fixtures/scripted-build.json`) through the REAL `ScriptedScheduleController` + `ScriptedPlayoutBuilderService.MockSession` + `SchedulingEngine` and pins the resulting `PlayoutItem`s as a snapshot in the golden line format — so exactly ONE action-to-engine mapping exists in the repo, the production one. The `Cli.Wrap` launch of the user-authored program (exit code, timeout, stdout capture) and the Kestrel/middleware/`ApiAuthorizationFilter` transport it calls back over are NOT tested and are not scheduled to be; that residue is named below rather than argued away. The controller is a thin adapter, NOT a 1:1 pass-through: an unparseable playback order is a 400, an unparseable filler kind SILENTLY degrades to `FillerKind.None`, an unknown build id is a 404, and the engine no-progress `InvalidOperationException` is translated to a 400 — each pinned by a test, because a prose claim of equivalence decays.'
+signals: 'why is there no scripted golden; scripted integration harness; scripted transport vs engine; SchedulingEngine is in-process testable; Cli.Wrap external process is transport only; shell-out scoped out; MockSession seam; in-process stand-in for a scripted script; engine build API characterization; committed script fixture replay; ScriptedScheduleController is an adapter not a pass-through; scripted filler kind silently falls back to None; scripted playout no-progress halt maps to 400; scripted build 401 blind spot; EnumeratorForContent · paths: `ErsatzTV.Core.Tests/Scheduling/Engine/SchedulingEngineTests.cs`, `ErsatzTV.Tests/Controllers/ScriptedScheduleControllerTests.cs`, `ErsatzTV.Tests/Controllers/Fixtures/scripted-build.json`, `ErsatzTV.Core/Scheduling/Engine/SchedulingEngine.cs`, `ErsatzTV/Controllers/Api/ScriptedScheduleController.cs`, `ErsatzTV.Core/Scheduling/ScriptedScheduling/ScriptedPlayoutBuilder.cs`, `ErsatzTV.Core.Tests/Scheduling/Goldens/PlayoutBuildGoldenTests.cs` · issues: #563, #381, #395, #163'
+mechanics: docs/testing.md → "Scripted playout coverage" and "Golden-file nets"
+---
+
+**What carries over from the predecessor.** Sequential (YAML) is golden-able and TZ-independent, and
+`PlayoutBuildGoldenTests` covers Classic / Block / Sequential. Scripted's *end-to-end pipeline* stays out
+of that net: `ScriptedPlayoutBuilder` builds nothing itself, it `Cli.Wrap`-executes a user-authored
+program, hands it `http://localhost:{Settings.UiPort}` plus a build id, and reads the result straight off
+the in-process engine afterwards. #395's `ContentEnumeratorBuilderTests` remains the direct net over the
+enumerator-construction switch Scripted and Sequential share.
+
+**What changed.** The predecessor deferred the whole harness to #563 and described the controller as a
+"1:1 pass-through". Both are now settled: the coverage exists in-process, and the pass-through wording was
+wrong — the four adapter mappings above are real behaviour that only a test can hold.
+
+**Why not the full Kestrel + `Cli.Wrap` harness.** Four measured obstacles, not a preference:
+
+1. *The production transport is not "a controller behind Kestrel".* `Startup` 404s any request whose Host
+   is not `localhost` and whose `Connection.LocalPort != Settings.UiPort` (a mutable process-wide static),
+   and `ApiAuthorizationFilter` fail-closes every mutating verb. A hand-rolled minimal host would test a
+   transport the product does not have; the real `Startup` drags in the whole DI graph (TvContext, ffmpeg
+   workers, search index, background services), which is what #381 deferred in the first place.
+2. *The external program is user-authored by definition.* `Playout.ScheduleFile` is an arbitrary command
+   line, so any script we commit is a stand-in we wrote either way. The fork's own driver,
+   `scripts/scripted-schedules/entrypoint.py`, imports `etv_client`, a generated package that exists only
+   in the deploy environment, so it cannot run in the .NET test job without adding a client-generation step.
+3. *It would put an interpreter and process spawning inside the pure in-memory .NET suite*, with the 30s
+   `Cli.Wrap` timeout nested inside CI's `--blame-hang-timeout 2m` — a new flake class on a shared runner.
+4. *The stand-in loses nothing the harness would have added at the engine level.* Replaying the fixture
+   through the real controller and the real service keeps a single action-to-engine mapping under test;
+   only the two transport hops are absent.
+
+**The residue, named.** Uncovered: process launch, exit code and timeout handling; Kestrel plus the
+forwarded-headers / compression / SPA-hosting middleware; and `ApiAuthorizationFilter`. That last one is
+not hypothetical — `ScriptedScheduleController` carries no `[SkipApiAuthorization]` while
+`ScriptedPlayoutBuilder` passes the script only host, build id and mode, with no `X-Api-Key`, and
+`entrypoint.py` builds its client with no credential. Whether a scripted build therefore 401s on its first
+POST in this fork is unverified against a live instance and is tracked separately; it is recorded here as
+the concrete shape of what an engine-level net cannot see, so the scope-out is not read as a claim of
+equivalence.
+
+**Determinism constraints on the fixtures.** Both use Chronological order and only instant-preserving
+instructions (`AddCount`, `AddAll`, `AddDuration`, `PadUntilExact`). `WaitUntil(TimeOnly)` and
+`PadUntil(string)` read the LOCAL day and time-of-day, and `PadToNext` becomes offset-sensitive once a
+call has localized `CurrentTime`, so introducing any of them — or a `shuffle` order — reopens the
+timezone and seed question the fixtures were built to close.
@@ -35,14 +35,10 @@ Three golden-file suites guard the highest-value, most-subtle output:
  produces over a pinned build window. Covers the **Classic** (`PlaybackOrder.Chronological`),
  **Block**, and **Sequential (YAML)** builders. The **Scripted** *end-to-end pipeline* is excluded from this
  net — `ScriptedPlayoutBuilder` runs a user-authored external process that drives the engine over HTTP, which
-  the in-memory harness can't pin; that integration harness is tracked in ersatztv#563. The scheduling
-  *behavior* those scripts drive, though, lives in the in-process `SchedulingEngine` (the HTTP controller is a
-  1:1 pass-through) and IS directly testable — `SchedulingEngineTests` news it up with substitutes, and
-  `ContentEnumeratorBuilderTests` (ersatztv#395) is the direct regression net over the enumerator-construction
-  helper the Scripted and Sequential/YAML engines now share (decision:
-  `testing.scripted-playout-golden-deferred`). The **Sequential** case
-  (`Sequential_yaml`, ersatztv#381)
-  builds from a committed YAML fixture (`Goldens/Fixtures/sequential-schedule.yml`) instead of a
+  the in-memory harness can't pin. Scripted is instead covered in-process at two levels, described under
+  "Scripted playout coverage" below (decision: `testing.scripted-engine-in-process-net`, ersatztv#563).
+  The **Sequential** case (`Sequential_yaml`, ersatztv#381) builds from a committed YAML fixture
+  (`Goldens/Fixtures/sequential-schedule.yml`) instead of a
  `ProgramSchedule`; it is TZ-independent (the `count`/`all`/`duration` handlers do UTC-only arithmetic —
  it passes, not skips, under a non-UTC `TZ`) so needs no `Assume` guard. A third case,
  `Classic_clock_padded` (ersatztv#77), locks clock-boundary padding: a `FillerMode.Pad` +
@@ -67,6 +63,30 @@ diff during normal test runs means the code changed the output — regenerating
 away hides the change instead of surfacing it. Only a human who has confirmed the change is
 intentional should regenerate.

+## Scripted playout coverage
+
+Scripted playout is characterized in-process at two levels, and the transport is deliberately not covered
+at all (ersatztv#563, decision `testing.scripted-engine-in-process-net`).
+
+| Where | Covers |
+| --- | --- |
+| `ErsatzTV.Core.Tests/Scheduling/Engine/SchedulingEngineTests.cs` | The engine build API a script drives: `AddCollection`, `AddCount`, `AddAll`, `AddDuration` (stop-before-end and trim), `PadUntilExact`, EPG guide-group locking, per-item `PlayoutHistory`, the `IsDone` no-progress halt and its reset, and the `GetAnchor`/`RestoreOrReset` round-trip a Continue build resumes from. Substituted repositories, no database. |
+| `ErsatzTV.Tests/Controllers/ScriptedScheduleControllerTests.cs` | A committed script fixture (`Controllers/Fixtures/scripted-build.json`) replayed through the real `ScriptedScheduleController` + `ScriptedPlayoutBuilderService.MockSession` + `SchedulingEngine`, with a pinned 13-item snapshot in the same line format the playout goldens use, plus the four adapter mappings: 404 on an unknown build id, 400 on an unparseable playback order, a silent fall back to `FillerKind.None` on an unparseable filler kind, and the engine's no-progress `InvalidOperationException` translated to a 400. |
+| `ErsatzTV.Core.Tests/Scheduling/ContentEnumeratorBuilderTests.cs` | The enumerator-construction helper the Scripted and Sequential/YAML engines share (ersatztv#395). |
+
+The expected snapshot is a string constant in the test rather than a fourth golden file: the golden
+harness lives in `ErsatzTV.Core.Tests`, which cannot reference a controller, and duplicating it would
+create a second action-to-engine mapping — the thing the single-mapping design exists to avoid.
+
+**Not covered, and not scheduled to be** (measured 2026-09-05): the `Cli.Wrap` launch of the user's own
+program — exit code, the `PlayoutScriptedScheduleTimeoutSeconds` timeout, stdout capture; Kestrel and the
+`Startup` middleware, including the host/`Settings.UiPort` check that 404s a foreign request; and
+`ApiAuthorizationFilter`, which fail-closes every mutating verb for an endpoint without
+`[SkipApiAuthorization]`. Hosting the real `Startup` would drag in the whole DI graph, a hand-rolled
+minimal host would test a transport the product does not have, and the script is user-authored by
+definition — so any committed script is a stand-in either way. The decision record names the concrete
+blind spot this leaves.
+
 ## Timezone independence

 The suite is timezone-independent (ersatztv#24). When constructing test `PlayoutItem`s, always
@@ -74,6 +94,17 @@ set a real `Start` (e.g. `startState.CurrentTime.UtcDateTime`) — never rely on
 `DateTime.MinValue`, which underflows `DateTimeOffset.MinValue` once a non-UTC local offset is
 applied (`StartOffset` calls `ToLocalTime()`). CI runs in UTC; local runs may not.

+**Which `SchedulingEngine` calls are safe to put in a fixture** (audited 2026-09-05). `AddCount`,
+`AddAll`, `AddDuration` and `PadUntilExact` preserve the instant — items are always written as
+`_state.CurrentTime.UtcDateTime`, and `PadUntilExact`'s `ToLocalTime()` changes the offset the state
+carries, not the moment. `WaitUntil(TimeOnly)` and `PadUntil(string)` are **not** safe: both read the
+LOCAL day and time-of-day off `CurrentTime` and rebuild a target from them. `PadToNext` is safe only
+until something localizes `CurrentTime` — it reads `.Year`/`.Month`/`.Day`/`.Hour`/`.Minute` in whatever
+offset that value happens to carry, so a fixture that calls it after `PadUntilExact`, `WaitUntilExact` or
+a Continue anchor becomes TZ-sensitive by ordering rather than by call. The scripted fixtures therefore
+use Chronological order and only the instant-preserving instructions; they pass, rather than skip, under
+`TZ=UTC`, `America/New_York`, `Australia/Lord_Howe` and `Asia/Kathmandu`.
+
 ## Running tests

 Full .NET gate: