Commit Graph
3 Commits
Author SHA1 Message Date
timothy 0cf355e494 fix(772,792): address both cold reviews — an unverified pin is no longer a green job
Two independent reviewers (one cross-family) converged on the same defect, and it was the
important one: the preflight WARNED and exited 0 on every answer that was not 200 or 404,
so a missing `curl`, a moved registry or a DNS change would have left it green forever —
"the check could not run" presenting as "the pin is fine", in a script whose own header
disclaimed exactly that. Unknown answers are now retried (3x, 5s) and then FAIL, with
wording kept distinct from the deleted case because the two send an operator to different
places.

Also from the reviews:

  * An absent secret does not arrive as an unset variable. `${{ secrets.X }}:${{ secrets.Y }}`
    interpolates to ":", a perfectly non-empty and perfectly useless credential, and the
    tests covered only the unset shape. Both halves are now required, and the parametrised
    test drives the production shape.
  * HTTP 200 is not a manifest. A proxy or a login page answers 200 too, so the body is
    fetched and matched for `schemaVersion` (a shell `case`, so no jq dependency and no
    pipeline that can inject).
  * The curl stub ignored `-u` and answered 200 regardless, so deleting the real `-u` would
    have left the suite green while the live registry rejected every request. It now 401s an
    unauthenticated read, as the registry does.
  * The mutation's declared diagnostic changed with the script: now that unknown fails too,
    the exit code no longer separates "deleted" from "could not check", so the proof turns on
    the message and `expect` says so.
  * docs/ci-cd.md: `scan` is no longer the only `docker-build.yml` job on the small lane, so
    the tag-push exclusivity claim and the lane membership were both false. Fixed.
  * "Immutable" was overstated: `ci-image.yml` tags `rev-parse --short HEAD`, so a dispatch or
    a weekly no-cache run at the same HEAD republishes that tag from a rebuilt image. Stated,
    along with what the rebuild recovery does NOT restore (mutable bases and apt, so equivalent
    rather than bit-identical).
  * The recovery recipe left you in a worktree checked out at the pin commit — where the
    verify script does not exist, and where the workflow carries the pre-bump pin. It now
    keeps `$repo`, returns, and removes the worktree. It also needed BuildKit's `http = true`
    caveat: the container driver does not inherit the daemon's insecure-registries.
  * The root cause carries its evidentiary limit and its reproduction commands, and says what
    to conclude if a pin vanishes after server-management#842 lands (refuted, not re-applied).
  * The `ci.required-job-step-execution-markers` carve-out named one container-free job; there
    are two now, and the membership is what rots.
  * The decision record's `''` YAML escapes leaked into rendered prose; "status, no comment ->
    ask" is qualified (a prior positive verdict for the SAME head still satisfies condition
    (c)); "exits 1" is "exits non-zero" (usage exits 2, jq its own status, signals 128+n).

refs #772
refs #792
Decisions-Edit: yes
2026-08-22 23:25:26 +02:00
timothyandtimothy cfdab63a3a fix(767): gate the release path on the delimiter ban with a prerequisite job (#770)
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 19s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 9m13s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m40s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 6m30s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m27s
The delimiter ban protecting `build`'s `Smoke + IPTV E2E` was enforced only by a pytest in `script-tests` — `on: pull_request`, not a required context — so nothing re-checked it on a `v*` tag push, which is exactly when the candidate image is published. A `scan` job now runs the ban test and `build` lists it in `needs:`, so a red `scan` skips `build` and no image is built.

Measured both directions without cutting a release: run 1928 (poisoned Smoke) → scan failed, `Build & push` skipped; run 1929 (control) → scan green, build ran.

The gate rests on three different KINDS of check, because each single kind was defeated in review: the ban test; an execution probe against a poisoned copy with all three `env:` tiers layered; and `scripts/ci-prove-ban-detects.sh`, which is not a test — it poisons the real checkout and vouches only for the ban test's `build` parametrisation failing. Eight review rounds; rounds 1-5 each found a real defect in the previous fix.

Refs: #767
Decisions-Edit: yes
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
2026-08-13 18:04:52 +00:00
timothyandtimothy 884ac8a7e9 fix(756): extend the dropped-step guard to docker-build.yml's required jobs, where a drop is fail-OPEN (#768)
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 9m0s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m24s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Failing after 6m8s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m42s
A `run:` body the runner declines to interpolate is dropped, and the job still
concludes `success` (#751). #751 fixed that in review-verdict.yml, where the
failure is fail-CLOSED. This closes the two places where it is fail-OPEN:
`Build & test (.NET)` and `EF migration integrity (SQLite + MySql)` are the
other two required contexts on `main`, so a dropped step there sends a required
check green having done no work.

Per-STEP markers, not per-job as proposed: a marker on the first step only
proves the job began, while the drop that costs something is `Test`, `Build` or
a migration replay. The trailing guard carries no `if:` — with a dozen steps,
`always()` would announce a false "these steps never executed" on every ordinary
red build; the default `success()` is correct because guard-skipped implies
job-red. Plus a ban on the raw `${{` opener in `test`, `migrations` and `build`,
which makes the class unreachable rather than merely caught. `build` is included
because its Smoke step runs AFTER the image is pushed.

Measured live on the build lane in both directions: probe #765 (drop caught,
sole failure in the job) and #766 (a failing continue-on-error step does not
skip the guard). 510 tests, 30 mutations killed across two harnesses, five cold
review rounds across two model families.

Residual tracked as #767: the `build` ban is review-time only, not fail-closed
on the release path.

fixes #756

Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
2026-08-10 23:35:08 +00:00