#586 — E2E cleanup is scoped by PID, never a pattern-wide pkill
New decision record testing.e2e-cleanup-scope-by-pid (+ regenerated catalog).
docs/e2e-local.md states the constraint in the teardown step, aimed explicitly at brief-writers — the #586 root cause was a delegation gap, not agent error.
docs/handoffs/chicorytv-issue-queue.md gets the one-line lore bullet, since that is what a live session actually reads at startup.
On the trap question (Done-when box 3): reviewed, and deliberately NOT adopted — recorded why.e2e-local.sh's contract is the inverse of e2e-ui.sh's: it hands a running instance back to its caller. Both callers use OUT="$(scripts/e2e-local.sh …)", so an EXIT trap would fire when the command substitution returns and kill the server before either could use it. e2e-ui.sh is the lifecycle owner and traps; this is the launcher and does not.
Instead the script gained the two things that actually prevent the incident:
An lsof pre-flight that names a foreign listener's PID rather than letting the run fail as process N exited before becoming ready + log tail — a framing that reads like a broken build. It reports, never reaps.
ETV_STREAMING_PORT now defaults to the given port.
The Blocker review caught — the escape hatch didn't work
Program.cs:167-172 binds two listeners, and SystemEnvironment.cs:23-27 defaults StreamingPort to 8409 independently of ETV_UI_PORT. So the obvious remedy ETV_UI_PORT=8420still bound 8409, died against the foreign holder, and printed a log naming 8409 — dropping an agent right back into the confusion that produced the pkill. Documenting that remedy would have made #586more likely to recur. Now the streaming port follows the UI port (explicit value still wins) and the pre-flight probes both.
A false premise from the issue body, corrected
#586 states "the CI step uses 8410 while local runs use 8409". On main, docker-build.yml:492 sets ETV_UI_PORT=8409 for functional-e2e — the same port as local. 8410 is the ersatztv-test container on jazz; only the unmerged #445 branch adds an 8410 step. The corrected fact is the stronger one: local and CI collide on the same port, so port choice was never separation.
Length is an invariant separate from correctness: the resolve/staleness checks compare resolved shas, so an 8-char pin of the right commit passes green while matching no registry tag — and all five container: jobs then die at image-pull with manifest unknown, which reads like a registry outage.
Guard fails at the gate and prints the exact tag to use.
Literal 7, not a derived --short=7: in a full clone git may widen an ambiguous abbreviation, demanding a pin ci-image.yml can never publish — the same clone-depth asymmetry the existing comment warns about. Escape hatch documented inline.
Also fixes a pre-existing misdiagnosis: zero pins reported "MORE THAN ONE".
docs/ci-cd.md documents the 7-char rule, git rev-parse --short=7 HEAD, and the caveat that 7 is currently empirical (see #597).
A bare core matched any path component named core; case-insensitively on macOS that included every */Core/source directory, so new untracked files were dropped by git add -A while tracked ones stayed fine — a clean local build and a CI checkout that fails to compile. Now /core + /core.[0-9]*, both anchored (an unanchored core.[0-9]* would re-introduce the very class this fixes). Subdirectory-dump coverage is a written-down accepted tradeoff.
Verification
What
How
.gitignore
Full ignored-file-set diff before/after: identical — nothing lost its ignore. The three real dirs (ErsatzTV.Scanner/Core, ErsatzTV.Scanner.Tests/Core, ErsatzTV.Tests/Core) are trackable without -f.
ci-image-pin
Step body extracted from the YAML by real parse (not hand-copied), run against doctored pins: 7 green; 6 / 8 / 10 red, each naming the correct tag. docker-build.yml restored clean.
Pre-flight
Decoy listeners: UI port busy → red naming it; 8409 busy + ETV_UI_PORT=8420 → now passes (previously a dead end); ETV_STREAMING_PORT=8409 pinned busy → red naming 8409.
Port guard
84o9 / abc → red; empty → correctly falls back to the 8409 default; CI shape (ETV_UI_PORT=8409) unaffected.
Gates
bash -n clean · decisions_validate.py OK · catalog regenerates byte-identically · kickoff-guard OK · pr-checks.yml parses.
Live-E2E: not required and not run (release.live-e2e-required scopes it to API write-path handlers; there is no C# change). The executable diff to e2e-local.sh is the pre-flight plus the env default, both tested directly above; every other line in that script is untouched.
Independent review: done, two rounds — cold-context reviewer, no implementation role. Round 1 returned BLOCKED and found the ETV_STREAMING_PORT Blocker plus the 8410 factual error. Round 2 re-reviewed the fix commit and returned MERGEABLE; its remaining Medium/Low/Nit items are all folded in.
Known, pre-existing
decisions_validate.py emits active-corpus 5682 lines (budget 5600). main is already over budget; this record widens it. Non-blocking ::warning:: ("schedule a consolidation"), not a gate.
Follow-ups filed
#596 — testing.playwright-mcp-download-and-recovery prescribes pkill -f ms-playwright-mcp, the same shared-host reap this PR forbids. Deferred rather than line-edited, because reversing a record is a supersede-and-relocate lifecycle operation. Flags the honest open question: a safe PID-scoped MCP recovery may not exist, in which case the rule is "restart via the client, never the shell."
#597 — make ci-image.yml tag with --short=7 so the 7-char length is enforced rather than observed. Needs its own publish-then-pin two-step, since editing ci-image.yml re-points ci-image-pin's expected at that commit.
Session bundle of three independent CI/repo-hygiene fixes with disjoint file sets.
fixes #586
fixes #594
fixes #485
---
## #586 — E2E cleanup is scoped by PID, never a pattern-wide `pkill`
- New decision record `testing.e2e-cleanup-scope-by-pid` (+ regenerated catalog).
- `docs/e2e-local.md` states the constraint in the teardown step, aimed explicitly at **brief-writers** — the #586 root cause was a delegation gap, not agent error.
- `docs/handoffs/chicorytv-issue-queue.md` gets the one-line lore bullet, since that is what a live session actually reads at startup.
**On the trap question (Done-when box 3): reviewed, and deliberately NOT adopted — recorded why.** `e2e-local.sh`'s contract is the inverse of `e2e-ui.sh`'s: it hands a *running* instance back to its caller. Both callers use `OUT="$(scripts/e2e-local.sh …)"`, so an `EXIT` trap would fire when the command substitution returns and kill the server before either could use it. `e2e-ui.sh` is the lifecycle owner and traps; this is the launcher and does not.
Instead the script gained the two things that actually prevent the incident:
- An `lsof` pre-flight that **names a foreign listener's PID** rather than letting the run fail as `process N exited before becoming ready` + log tail — a framing that reads like a broken build. It reports, never reaps.
- `ETV_STREAMING_PORT` now defaults to the given port.
### The Blocker review caught — the escape hatch didn't work
`Program.cs:167-172` binds **two** listeners, and `SystemEnvironment.cs:23-27` defaults `StreamingPort` to **8409 independently of `ETV_UI_PORT`**. So the obvious remedy `ETV_UI_PORT=8420` *still bound 8409*, died against the foreign holder, and printed a log naming 8409 — dropping an agent right back into the confusion that produced the `pkill`. Documenting that remedy would have made #586 **more** likely to recur. Now the streaming port follows the UI port (explicit value still wins) and the pre-flight probes both.
### A false premise from the issue body, corrected
#586 states "the CI step uses 8410 while local runs use 8409". On `main`, `docker-build.yml:492` sets `ETV_UI_PORT=8409` for `functional-e2e` — the **same** port as local. 8410 is the `ersatztv-test` container on jazz; only the unmerged #445 branch adds an 8410 step. The corrected fact is the stronger one: local and CI collide on the same port, so port choice was never separation.
## #594 — `ci-image-pin` accepts any hex length
Length is an invariant **separate from correctness**: the resolve/staleness checks compare resolved shas, so an 8-char pin of the right commit passes green while matching no registry tag — and all five `container:` jobs then die at image-pull with `manifest unknown`, which reads like a registry outage.
- Guard fails at the gate and prints the exact tag to use.
- Literal `7`, not a derived `--short=7`: in a full clone git may widen an ambiguous abbreviation, demanding a pin `ci-image.yml` can never publish — the same clone-depth asymmetry the existing comment warns about. Escape hatch documented inline.
- Also fixes a **pre-existing** misdiagnosis: zero pins reported "MORE THAN ONE".
- `docs/ci-cd.md` documents the 7-char rule, `git rev-parse --short=7 HEAD`, and the caveat that 7 is currently empirical (see #597).
## #485 — `.gitignore` `core` swallowed `*/Core/` source files
A bare `core` matched any path component named `core`; case-insensitively on macOS that included every `*/Core/` **source** directory, so new untracked files were dropped by `git add -A` while tracked ones stayed fine — a clean local build and a CI checkout that fails to compile. Now `/core` + `/core.[0-9]*`, **both anchored** (an unanchored `core.[0-9]*` would re-introduce the very class this fixes). Subdirectory-dump coverage is a written-down accepted tradeoff.
---
## Verification
| What | How |
|---|---|
| `.gitignore` | Full **ignored-file-set diff** before/after: identical — nothing lost its ignore. The three real dirs (`ErsatzTV.Scanner/Core`, `ErsatzTV.Scanner.Tests/Core`, `ErsatzTV.Tests/Core`) are trackable without `-f`. |
| `ci-image-pin` | Step body extracted from the YAML by real parse (not hand-copied), run against doctored pins: **7 green; 6 / 8 / 10 red**, each naming the correct tag. `docker-build.yml` restored clean. |
| Pre-flight | Decoy listeners: UI port busy → red naming it; **8409 busy + `ETV_UI_PORT=8420` → now passes** (previously a dead end); `ETV_STREAMING_PORT=8409` pinned busy → red naming 8409. |
| Port guard | `84o9` / `abc` → red; empty → correctly falls back to the 8409 default; CI shape (`ETV_UI_PORT=8409`) unaffected. |
| Gates | `bash -n` clean · `decisions_validate.py` OK · catalog regenerates byte-identically · `kickoff-guard` OK · `pr-checks.yml` parses. |
**Live-E2E: not required and not run** (`release.live-e2e-required` scopes it to API write-path handlers; there is no C# change). The executable diff to `e2e-local.sh` is the pre-flight plus the env default, both tested directly above; every other line in that script is untouched.
**Independent review: done, two rounds** — cold-context reviewer, no implementation role. Round 1 returned BLOCKED and found the `ETV_STREAMING_PORT` Blocker plus the 8410 factual error. Round 2 re-reviewed the fix commit and returned MERGEABLE; its remaining Medium/Low/Nit items are all folded in.
## Known, pre-existing
`decisions_validate.py` emits `active-corpus 5682 lines (budget 5600)`. **`main` is already over budget**; this record widens it. Non-blocking `::warning::` ("schedule a consolidation"), not a gate.
## Follow-ups filed
- **#596** — `testing.playwright-mcp-download-and-recovery` prescribes `pkill -f ms-playwright-mcp`, the same shared-host reap this PR forbids. Deferred rather than line-edited, because reversing a record is a supersede-and-relocate lifecycle operation. Flags the honest open question: a safe PID-scoped MCP recovery may not exist, in which case the rule is "restart via the client, never the shell."
- **#597** — make `ci-image.yml` tag with `--short=7` so the 7-char length is enforced rather than observed. Needs its own publish-then-pin two-step, since editing `ci-image.yml` re-points `ci-image-pin`'s `expected` at that commit.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Three independent CI/repo-hygiene fixes swept together; disjoint file sets.
fixes#586 — E2E cleanup is scoped by PID, never a pattern-wide pkill
- New decision record `testing.e2e-cleanup-scope-by-pid`.
- docs/e2e-local.md states the constraint where a BRIEF-WRITER sees it (the
#586 root cause was a delegation gap, not agent error).
- scripts/e2e-local.sh: reviewed against e2e-ui.sh's trap lifecycle and
deliberately does NOT adopt it — its contract is to hand a running instance
back to its caller, so an EXIT trap would kill the server the instant the
launcher returned (both callers use `OUT="$(e2e-local.sh ...)"`). Recorded.
- Instead it gains what actually prevents the incident: an lsof pre-flight
that NAMES a foreign listener's PID rather than letting Kestrel fail its
bind and surface as "process N exited before becoming ready".
- Pre-flight probes BOTH bound ports, and ETV_STREAMING_PORT now defaults to
ETV_UI_PORT. Program.cs binds a second listener whose port defaults to 8409
independently of ETV_UI_PORT, so `ETV_UI_PORT=8420` alone still bound 8409
and died against a foreign holder — i.e. the documented escape hatch was a
dead end that led straight back to the confusion behind the pattern kill.
fixes#594 — ci-image-pin accepts any hex length
- Length is a separate invariant from correctness: the resolve/staleness
checks compare resolved shas, so an 8-char pin of the right commit passes
green while matching NO registry tag, and all five container: jobs then die
at image-pull with `manifest unknown` (reads like a registry outage).
- Guard fails at the gate and prints the exact tag to use. Verified against
doctored pins: 7 green; 6/8/10 red.
- Uses a literal 7 rather than a derived `--short=7`: in a full clone git may
widen an ambiguous abbreviation, demanding a pin ci-image.yml can never
publish. Escape hatch documented inline.
- Also fixes a pre-existing misdiagnosis: zero pins reported "MORE THAN ONE".
- docs/ci-cd.md documents the 7-char rule and `git rev-parse --short=7 HEAD`.
fixes#485 — .gitignore `core` silently ignored `*/Core/` files
- A bare `core` matched any path component named `core`; case-insensitively
on macOS that swallowed every `*/Core/` SOURCE dir, so new untracked files
were dropped by `git add -A` while tracked ones stayed fine — a clean local
build and a CI checkout that fails to compile.
- Now `/core` + `/core.[0-9]*`, both anchored (an unanchored `core.[0-9]*`
would re-introduce the same silent-exclusion class this fixes).
- Verified by diffing the full ignored-file set before/after: identical, and
the three real Core/ dirs are trackable without -f.
Docs updated in-PR: docs/e2e-local.md, docs/ci-cd.md, docs/decisions/
workflow-process.md (+ regenerated catalog), docs/handoffs/chicorytv-issue-queue.md.
Follow-ups filed: #596 (the same shared-host reap in the Playwright-MCP
recovery record) and the ci-image.yml `--short=7` publisher-side fix, which
cannot ride this PR — editing ci-image.yml re-points ci-image-pin's `expected`
at this commit and reds the gate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Independent adversarial review, two rounds, cold-context reviewer with no implementation role (process.independent-review-rubric).
Round 1 — BLOCKED. Found a genuine Blocker plus two High findings:
Blocker: the documented escape hatch ETV_UI_PORT=<other> did not work. Program.cs binds a second listener and SystemEnvironment.cs defaults StreamingPort to 8409 independently of ETV_UI_PORT, so the remedy still bound 8409 and died against the foreign holder — making #586 more likely to recur, not less.
High: the pre-flight probed only the UI port, so the one case a caller reaches it was a false negative.
High: the "CI uses 8410, local uses 8409" claim (taken from #586's body) is false on main — functional-e2e uses 8409.
Also flagged that the branch base was 2 commits behind origin/main, showing a spurious PR #588 revert in the diff.
Round 2 — re-reviewed the fix commit (release.review-verdict-gate: the fix gets reviewed, not just the original diff). Verified the streaming-port default is a literal no-op for every existing caller (CI docker-build.yml, e2e-ui.sh on the #445 branch, e2e-functional.sh), that no streaming-URL construction regresses, and that the dedup loop is safe under set -euo pipefail. Remaining Medium/Low/Nit items — a self-contradiction in the record about the 8420 run, an overstated deferral reason for #597, a missing caveat in docs/ci-cd.md, a non-numeric-port hole, and two prose nits — are all folded into the head commit.
One reviewer suggestion was declined with reason: changing ci-image.yml to --short=7. ci-image-pin derives expected from the last commit touching ci-image.yml itself, so that edit re-points the gate at its own commit and needs a publish-then-pin cycle. Split to #597 as a scope call, not a blocker — and #597's body was corrected after round 2 pointed out my first justification overstated it as impossible.
Independent adversarial review, two rounds, cold-context reviewer with no implementation role (`process.independent-review-rubric`).
**Round 1 — BLOCKED.** Found a genuine Blocker plus two High findings:
- **Blocker**: the documented escape hatch `ETV_UI_PORT=<other>` did not work. `Program.cs` binds a second listener and `SystemEnvironment.cs` defaults `StreamingPort` to 8409 *independently* of `ETV_UI_PORT`, so the remedy still bound 8409 and died against the foreign holder — making #586 more likely to recur, not less.
- **High**: the pre-flight probed only the UI port, so the one case a caller reaches it was a false negative.
- **High**: the "CI uses 8410, local uses 8409" claim (taken from #586's body) is false on `main` — `functional-e2e` uses 8409.
Also flagged that the branch base was 2 commits behind `origin/main`, showing a spurious PR #588 revert in the diff.
**Round 2 — re-reviewed the fix commit** (`release.review-verdict-gate`: the fix gets reviewed, not just the original diff). Verified the streaming-port default is a literal no-op for every existing caller (CI `docker-build.yml`, `e2e-ui.sh` on the #445 branch, `e2e-functional.sh`), that no streaming-URL construction regresses, and that the dedup loop is safe under `set -euo pipefail`. Remaining Medium/Low/Nit items — a self-contradiction in the record about the 8420 run, an overstated deferral reason for #597, a missing caveat in `docs/ci-cd.md`, a non-numeric-port hole, and two prose nits — are all folded into the head commit.
One reviewer suggestion was **declined with reason**: changing `ci-image.yml` to `--short=7`. `ci-image-pin` derives `expected` from the last commit touching `ci-image.yml` itself, so that edit re-points the gate at its own commit and needs a publish-then-pin cycle. Split to #597 as a scope call, not a blocker — and #597's body was corrected after round 2 pointed out my first justification overstated it as impossible.
Review-verdict: MERGEABLE @ c8e79f49
Live boot verification — closing the gap the advisory job left
The Functional E2E job (advisory) failed on infrastructure, before any of this PR's code ran, so it did not exercise the scripts/e2e-local.sh change. Root cause is inside actions/cache's bundled restore script:
ReferenceError: __dirname is not defined in ES module scope
❌ Failure - Main Cache NuGet packages
Step-outcome list confirms Cache NuGet packages was the only step to report; the three e2e-local strings in that log are filenames in a changed-files listing, not execution — no "Copying wwwroot", no "Launching dotnet". Not a known flake either: runs 1345/1344/1347 all had this job green with zero __dirname errors. Per ci.infra-shaped-red-under-load I'm not filing a CI bug off one sample under load, and per ci.batch-pushes-no-cancel-route/dont-repush-to-clear-a-killed-ci-job I'm not forcing a fresh run to paper over it.
Since that left my own change unexercised end-to-end, I built and booted the app locally instead. Results:
Launch
Listeners bound
Outcome
Direct dotnet ErsatzTV.dll, ETV_UI_PORT=8420, no ETV_STREAMING_PORT — the old documented remedy
*:8420and *:8409
← the Blocker, reproduced
Via fixed e2e-local.sh, ETV_UI_PORT=8420
*:8420 only
escape hatch now actually works
Via fixed e2e-local.sh, CI shape ETV_UI_PORT=8409
*:8409 only
ready line reached, GET / → 302, kill "$PID" stopped it cleanly, port freed
The first row is the important one: it is a negative control proving the defect was real rather than inferred from reading Program.cs. An agent following the previously-documented ETV_UI_PORT=<other> advice would have bound 8409 anyway and died against the foreign holder — which is why this PR fixes the escape hatch instead of merely documenting it.
The third row is the regression check that matters for CI: the ETV_STREAMING_PORT line I added to the launch command is a literal no-op at ETV_UI_PORT=8409, exactly as claimed, so functional-e2e is unaffected once the runner recovers.
All test processes were torn down by PID (never pkill -f) — dogfooding the rule this PR adds. No strays; 8409/8420 free.
## Live boot verification — closing the gap the advisory job left
The `Functional E2E` job (advisory) failed on infrastructure, **before** any of this PR's code ran, so it did not exercise the `scripts/e2e-local.sh` change. Root cause is inside `actions/cache`'s bundled restore script:
```
ReferenceError: __dirname is not defined in ES module scope
❌ Failure - Main Cache NuGet packages
```
Step-outcome list confirms `Cache NuGet packages` was the only step to report; the three `e2e-local` strings in that log are **filenames in a changed-files listing**, not execution — no "Copying wwwroot", no "Launching dotnet". Not a known flake either: runs 1345/1344/1347 all had this job green with zero `__dirname` errors. Per `ci.infra-shaped-red-under-load` I'm not filing a CI bug off one sample under load, and per `ci.batch-pushes-no-cancel-route`/`dont-repush-to-clear-a-killed-ci-job` I'm not forcing a fresh run to paper over it.
Since that left my own change unexercised end-to-end, I built and booted the app locally instead. Results:
| Launch | Listeners bound | Outcome |
|---|---|---|
| Direct `dotnet ErsatzTV.dll`, `ETV_UI_PORT=8420`, no `ETV_STREAMING_PORT` — **the old documented remedy** | `*:8420` **and `*:8409`** | ← the Blocker, reproduced |
| Via fixed `e2e-local.sh`, `ETV_UI_PORT=8420` | `*:8420` only | escape hatch now actually works |
| Via fixed `e2e-local.sh`, CI shape `ETV_UI_PORT=8409` | `*:8409` only | ready line reached, `GET /` → 302, `kill "$PID"` stopped it cleanly, port freed |
The first row is the important one: it is a **negative control** proving the defect was real rather than inferred from reading `Program.cs`. An agent following the previously-documented `ETV_UI_PORT=<other>` advice would have bound 8409 anyway and died against the foreign holder — which is why this PR fixes the escape hatch instead of merely documenting it.
The third row is the regression check that matters for CI: the `ETV_STREAMING_PORT` line I added to the launch command is a literal no-op at `ETV_UI_PORT=8409`, exactly as claimed, so `functional-e2e` is unaffected once the runner recovers.
All test processes were torn down **by PID** (never `pkill -f`) — dogfooding the rule this PR adds. No strays; 8409/8420 free.
timothy
merged commit 67e53b1304 into main2026-07-25 13:51:57 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Session bundle of three independent CI/repo-hygiene fixes with disjoint file sets.
fixes #586
fixes #594
fixes #485
#586 — E2E cleanup is scoped by PID, never a pattern-wide
pkilltesting.e2e-cleanup-scope-by-pid(+ regenerated catalog).docs/e2e-local.mdstates the constraint in the teardown step, aimed explicitly at brief-writers — the #586 root cause was a delegation gap, not agent error.docs/handoffs/chicorytv-issue-queue.mdgets the one-line lore bullet, since that is what a live session actually reads at startup.On the trap question (Done-when box 3): reviewed, and deliberately NOT adopted — recorded why.
e2e-local.sh's contract is the inverse ofe2e-ui.sh's: it hands a running instance back to its caller. Both callers useOUT="$(scripts/e2e-local.sh …)", so anEXITtrap would fire when the command substitution returns and kill the server before either could use it.e2e-ui.shis the lifecycle owner and traps; this is the launcher and does not.Instead the script gained the two things that actually prevent the incident:
lsofpre-flight that names a foreign listener's PID rather than letting the run fail asprocess N exited before becoming ready+ log tail — a framing that reads like a broken build. It reports, never reaps.ETV_STREAMING_PORTnow defaults to the given port.The Blocker review caught — the escape hatch didn't work
Program.cs:167-172binds two listeners, andSystemEnvironment.cs:23-27defaultsStreamingPortto 8409 independently ofETV_UI_PORT. So the obvious remedyETV_UI_PORT=8420still bound 8409, died against the foreign holder, and printed a log naming 8409 — dropping an agent right back into the confusion that produced thepkill. Documenting that remedy would have made #586 more likely to recur. Now the streaming port follows the UI port (explicit value still wins) and the pre-flight probes both.A false premise from the issue body, corrected
#586 states "the CI step uses 8410 while local runs use 8409". On
main,docker-build.yml:492setsETV_UI_PORT=8409forfunctional-e2e— the same port as local. 8410 is theersatztv-testcontainer on jazz; only the unmerged #445 branch adds an 8410 step. The corrected fact is the stronger one: local and CI collide on the same port, so port choice was never separation.#594 —
ci-image-pinaccepts any hex lengthLength is an invariant separate from correctness: the resolve/staleness checks compare resolved shas, so an 8-char pin of the right commit passes green while matching no registry tag — and all five
container:jobs then die at image-pull withmanifest unknown, which reads like a registry outage.7, not a derived--short=7: in a full clone git may widen an ambiguous abbreviation, demanding a pinci-image.ymlcan never publish — the same clone-depth asymmetry the existing comment warns about. Escape hatch documented inline.docs/ci-cd.mddocuments the 7-char rule,git rev-parse --short=7 HEAD, and the caveat that 7 is currently empirical (see #597).#485 —
.gitignorecoreswallowed*/Core/source filesA bare
corematched any path component namedcore; case-insensitively on macOS that included every*/Core/source directory, so new untracked files were dropped bygit add -Awhile tracked ones stayed fine — a clean local build and a CI checkout that fails to compile. Now/core+/core.[0-9]*, both anchored (an unanchoredcore.[0-9]*would re-introduce the very class this fixes). Subdirectory-dump coverage is a written-down accepted tradeoff.Verification
.gitignoreErsatzTV.Scanner/Core,ErsatzTV.Scanner.Tests/Core,ErsatzTV.Tests/Core) are trackable without-f.ci-image-pindocker-build.ymlrestored clean.ETV_UI_PORT=8420→ now passes (previously a dead end);ETV_STREAMING_PORT=8409pinned busy → red naming 8409.84o9/abc→ red; empty → correctly falls back to the 8409 default; CI shape (ETV_UI_PORT=8409) unaffected.bash -nclean ·decisions_validate.pyOK · catalog regenerates byte-identically ·kickoff-guardOK ·pr-checks.ymlparses.Live-E2E: not required and not run (
release.live-e2e-requiredscopes it to API write-path handlers; there is no C# change). The executable diff toe2e-local.shis the pre-flight plus the env default, both tested directly above; every other line in that script is untouched.Independent review: done, two rounds — cold-context reviewer, no implementation role. Round 1 returned BLOCKED and found the
ETV_STREAMING_PORTBlocker plus the 8410 factual error. Round 2 re-reviewed the fix commit and returned MERGEABLE; its remaining Medium/Low/Nit items are all folded in.Known, pre-existing
decisions_validate.pyemitsactive-corpus 5682 lines (budget 5600).mainis already over budget; this record widens it. Non-blocking::warning::("schedule a consolidation"), not a gate.Follow-ups filed
testing.playwright-mcp-download-and-recoveryprescribespkill -f ms-playwright-mcp, the same shared-host reap this PR forbids. Deferred rather than line-edited, because reversing a record is a supersede-and-relocate lifecycle operation. Flags the honest open question: a safe PID-scoped MCP recovery may not exist, in which case the rule is "restart via the client, never the shell."ci-image.ymltag with--short=7so the 7-char length is enforced rather than observed. Needs its own publish-then-pin two-step, since editingci-image.ymlre-pointsci-image-pin'sexpectedat that commit.🤖 Generated with Claude Code
Independent adversarial review, two rounds, cold-context reviewer with no implementation role (
process.independent-review-rubric).Round 1 — BLOCKED. Found a genuine Blocker plus two High findings:
ETV_UI_PORT=<other>did not work.Program.csbinds a second listener andSystemEnvironment.csdefaultsStreamingPortto 8409 independently ofETV_UI_PORT, so the remedy still bound 8409 and died against the foreign holder — making #586 more likely to recur, not less.main—functional-e2euses 8409.Also flagged that the branch base was 2 commits behind
origin/main, showing a spurious PR #588 revert in the diff.Round 2 — re-reviewed the fix commit (
release.review-verdict-gate: the fix gets reviewed, not just the original diff). Verified the streaming-port default is a literal no-op for every existing caller (CIdocker-build.yml,e2e-ui.shon the #445 branch,e2e-functional.sh), that no streaming-URL construction regresses, and that the dedup loop is safe underset -euo pipefail. Remaining Medium/Low/Nit items — a self-contradiction in the record about the 8420 run, an overstated deferral reason for #597, a missing caveat indocs/ci-cd.md, a non-numeric-port hole, and two prose nits — are all folded into the head commit.One reviewer suggestion was declined with reason: changing
ci-image.ymlto--short=7.ci-image-pinderivesexpectedfrom the last commit touchingci-image.ymlitself, so that edit re-points the gate at its own commit and needs a publish-then-pin cycle. Split to #597 as a scope call, not a blocker — and #597's body was corrected after round 2 pointed out my first justification overstated it as impossible.Review-verdict: MERGEABLE @
c8e79f49Live boot verification — closing the gap the advisory job left
The
Functional E2Ejob (advisory) failed on infrastructure, before any of this PR's code ran, so it did not exercise thescripts/e2e-local.shchange. Root cause is insideactions/cache's bundled restore script:Step-outcome list confirms
Cache NuGet packageswas the only step to report; the threee2e-localstrings in that log are filenames in a changed-files listing, not execution — no "Copying wwwroot", no "Launching dotnet". Not a known flake either: runs 1345/1344/1347 all had this job green with zero__dirnameerrors. Perci.infra-shaped-red-under-loadI'm not filing a CI bug off one sample under load, and perci.batch-pushes-no-cancel-route/dont-repush-to-clear-a-killed-ci-jobI'm not forcing a fresh run to paper over it.Since that left my own change unexercised end-to-end, I built and booted the app locally instead. Results:
dotnet ErsatzTV.dll,ETV_UI_PORT=8420, noETV_STREAMING_PORT— the old documented remedy*:8420and*:8409e2e-local.sh,ETV_UI_PORT=8420*:8420onlye2e-local.sh, CI shapeETV_UI_PORT=8409*:8409onlyGET /→ 302,kill "$PID"stopped it cleanly, port freedThe first row is the important one: it is a negative control proving the defect was real rather than inferred from reading
Program.cs. An agent following the previously-documentedETV_UI_PORT=<other>advice would have bound 8409 anyway and died against the foreign holder — which is why this PR fixes the escape hatch instead of merely documenting it.The third row is the regression check that matters for CI: the
ETV_STREAMING_PORTline I added to the launch command is a literal no-op atETV_UI_PORT=8409, exactly as claimed, sofunctional-e2eis unaffected once the runner recovers.All test processes were torn down by PID (never
pkill -f) — dogfooding the rule this PR adds. No strays; 8409/8420 free.