From c8e79f49f4d7cedc0e25c6bc6e8941a49ee8ce31 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 25 Jul 2026 12:40:53 +0200 Subject: [PATCH] chore(586,594,485): PID-scoped E2E cleanup, ci-image-pin length guard, .gitignore core fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .gitea/workflows/pr-checks.yml | 32 ++++++++++++ .gitignore | 14 +++++- docs/ci-cd.md | 24 ++++++++- docs/decisions/README.md | 1 + docs/decisions/workflow-process.md | 63 ++++++++++++++++++++++++ docs/e2e-local.md | 68 ++++++++++++++++++++++++-- docs/handoffs/chicorytv-issue-queue.md | 8 ++- scripts/e2e-local.sh | 65 ++++++++++++++++++++++-- 8 files changed, 263 insertions(+), 12 deletions(-) diff --git a/.gitea/workflows/pr-checks.yml b/.gitea/workflows/pr-checks.yml index fe0574a9e..32490b298 100644 --- a/.gitea/workflows/pr-checks.yml +++ b/.gitea/workflows/pr-checks.yml @@ -78,10 +78,42 @@ jobs: mapfile -t pins < <(grep -oE 'ersatztv-ci:[0-9a-f]+' .gitea/workflows/docker-build.yml | cut -d: -f2 | sort -u) echo "Image sources last changed in: ${expected}" echo "Pins found in docker-build.yml: ${pins[*]} (${#pins[@]} distinct)" + if [ "${#pins[@]}" -eq 0 ]; then + echo "::error::No ersatztv-ci pin found in docker-build.yml at all. Every container: job must pin ersatztv-ci:<7-char-sha>; if the grep pattern stopped matching, fix it here too (docs/ci-cd.md -> 'CI toolchain image')." + exit 1 + fi if [ "${#pins[@]}" -ne 1 ]; then echo "::error::docker-build.yml pins MORE THAN ONE ersatztv-ci tag (${pins[*]}). All jobs must pin the same image — bump them together." exit 1 fi + # LENGTH is a separate invariant from CORRECTNESS, and only this check covers it + # (ersatztv#594). The resolve + staleness checks below compare RESOLVED shas, so a + # 8/9/10-char abbreviation of the right commit sails through them green — while + # matching NO tag in the registry, because ci-image.yml tags with + # `git rev-parse --short HEAD` under `fetch-depth: 1`, which always yields exactly 7. + # The failure would otherwise surface far downstream as all five `container:` jobs + # dying at image-pull with `manifest unknown`, which reads like a registry outage. + # This is an easy mistake to make: the natural local command prints 8 chars. + # + # Deliberately a literal 7, not a derived `git rev-parse --short=7`: in this full + # clone git may widen an ambiguous abbreviation past 7, which would demand a pin + # ci-image.yml can never publish — the exact clone-depth asymmetry noted above. + # `${expected:0:7}` is plain string truncation, so it is safe to suggest. + # + # ESCAPE HATCH, if you are ever stuck: this makes 7 mandatory, so if `${expected:0:7}` ever + # became an AMBIGUOUS prefix (two objects sharing it), the resolve check below would fail + # and a longer pin — previously the workaround — is now rejected here first. There is no + # in-repo remedy in that state: relax this length check in the same PR and say why. Note + # that ci-image.yml still tags with a plain `--short` (auto-scaled), so "always 7" is an + # empirical property of today's shallow clone, not an enforced invariant. Making the + # publisher emit `--short=7` is tracked as ersatztv#597. It is not blocked, just out of + # scope here: editing ci-image.yml re-points `expected` (above) at that commit, so it needs + # the branch's own publish-then-pin two-step (docs/ci-cd.md -> 'CI toolchain image') — + # ci-image.yml's push trigger has no branches: filter, so a feature branch does publish. + if [ "${#pins[0]}" -ne 7 ]; then + echo "::error::CI toolchain image pin ersatztv-ci:${pins[0]} is ${#pins[0]} chars, but ci-image.yml publishes 7-char tags (it tags with 'git rev-parse --short HEAD' from a fetch-depth:1 clone). A differently-sized abbreviation still resolves to the right commit, so this would pass every other check here — but NO such tag exists in the registry, and all five container: jobs would fail at image-pull time with 'manifest unknown'. Pin exactly: ersatztv-ci:${expected:0:7} (locally: git rev-parse --short=7 HEAD). See docs/ci-cd.md -> 'CI toolchain image'." + exit 1 + fi pin_full="$(git rev-parse --verify --quiet "${pins[0]}^{commit}" || true)" if [ -z "$pin_full" ]; then echo "::error::The pinned CI image tag ersatztv-ci:${pins[0]} does not resolve to a commit in this repo, so it cannot correspond to an image ci-image.yml built from these sources. Rebuild the image and pin the sha it prints." diff --git a/.gitignore b/.gitignore index 55bc3d56c..fd6ce72a2 100644 --- a/.gitignore +++ b/.gitignore @@ -46,7 +46,19 @@ msbuild.wrn .vs/ *.sqlite3* -core +# Core dumps. MUST stay anchored/qualified (ersatztv#485): a bare `core` matches any path +# component named `core`, and on a case-insensitive filesystem (macOS default) that includes +# every `*/Core/` source directory — silently excluding NEW files under e.g. +# ErsatzTV.Scanner/Core/ from `git add -A`. Tracked files are unaffected, so the symptom is a +# clean local build and a CI checkout that fails to compile. +# +# Both patterns are anchored to the repo root ON PURPOSE — an unanchored `core.[0-9]*` would +# re-introduce exactly the silent-exclusion class this fixes. Tradeoff, accepted: a dump written +# into a SUBdirectory is no longer ignored (the old bare `core` did catch those). In practice the +# processes that could drop one, run from the repo root or from `bin/` — and `[Bb]in/` already covers +# the latter. An un-ignored dump is visible noise; a wrongly-ignored source file is not. +/core +/core.[0-9]* scripts/generate-api-sdk/swagger.json scripts/download-test-content.sh diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 6270d99f5..317ee684b 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -632,6 +632,27 @@ ersatztv#299 seeded-media/scanner E2E follow-ups will need. CI is what proves the new image works. The pin is repeated per job because `jobs..container.image` cannot read the workflow `env` context. +**The tag is exactly 7 hex chars — get the length right, not just the commit** (ersatztv#594). +`ci-image.yml` tags with `git rev-parse --short HEAD` under `fetch-depth: 1`, and that shallow clone +holds few enough objects that git always abbreviates to **7**. A full local clone abbreviates to **8**, +so the natural command prints one character too many: + +```bash +git rev-parse --short HEAD # 8 chars in a full clone — WRONG, no such registry tag +git rev-parse --short=7 HEAD # 7 chars — what ci-image.yml publishes. Use this. +``` + +An 8-char pin names the *right commit* but *no existing image*: it satisfies a resolve-and-compare +check, then every `container:` job dies at image-pull with `manifest unknown`, which reads like a +registry outage rather than a one-character pin error. `ci-image-pin` therefore checks the pin's +**length** as an invariant separate from its correctness, and prints the exact tag to use. + +> **Caveat worth knowing before you trust the 7:** `ci-image.yml` still tags with a plain `--short`, +> whose length git *auto-scales* to the object count. 7 is therefore an empirical property of today's +> shallow clone, not an enforced invariant — if that count ever crosses git's threshold, the publisher +> emits 8, the correct pin becomes 8, and the gate's hardcoded 7 goes permanently red demanding a tag +> with no image behind it. Making the publisher emit `--short=7` is tracked as ersatztv#597. + `ci-image.yml` triggers on pushes touching `docker/ci/**`, `workflow_dispatch`, and a weekly Monday 05:00 UTC cron (base-image security updates; Gitea registers `schedule` only from `main`). It runs on `ubuntu-latest` — it was on `small` until server-management#639, where "docker-only" was found to be @@ -658,7 +679,8 @@ registry. Renovate tracks the Dockerfile's image pins (`dockerfile` manager, see **Bumping the pin is enforced, not remembered.** The `ci-image-pin` job (blocking, PR-only; defined in `pr-checks.yml`, but it greps `docker-build.yml` where the pins live) fails if `docker-build.yml`'s pin isn't the short sha of the last commit to touch `docker/ci/**` or -`ci-image.yml`, or if the five jobs ever pin different tags. This exists because **Renovate manages +`ci-image.yml`, if that pin isn't exactly 7 chars long (see above), or if the five jobs ever pin +different tags. This exists because **Renovate manages `docker/ci/Dockerfile`'s base pins but cannot bump an opaque `:`** in `container.image` — so a Renovate base bump would otherwise publish a new image, test the *old* one, and merge with the Dockerfile disagreeing with the pin. A red `ci-image-pin` means: let `ci-image.yml` publish the new diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 41ca79c8d..bf1d0a345 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -161,6 +161,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `spa.topbar-primary-action` | The TopBar's primary-action "+" button renders only when the active route declares a non-empty `primaryAction`, is wired (via a shared `usePrimaryAction` hook) only on single-unambiguous-create-flow list screens, and is dropped everywhere else rather than left as a dead/no-op button. | 2026-07-12 | [link](../decisions.md#2026-07-12--topbar-primary-action-button-wire-creates-drop-the-rest-238) | | `spa.yaml-validator-textarea` | The YAML playout validator takes pasted YAML via a `