fix(786,789): workflows declare their own per-job metadata; guard populations derive from it #872
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: timothy/ersatztv#872
Reference in New Issue
Block a user
Closes two issues that are one mechanism pulling in opposite directions.
#786 asked for inline workflow-job guards to be enumerated using "the same
reviewed-registry-cross-checked-both-ways shape
TOOLCHAIN_JOBSuses". #789 says that exactshape is the defect and must be deleted. Working #786 as written would have built the artifact #789
removes — so they are fixed together, in #789's direction.
The mechanism
Every job now declares metadata about itself, and both populations derive from it:
env.CI_EXECUTION_CLASStoolchain/bare-runnerdocker-build.ymlenv.CI_JOB_ROLEguard/advisory/noneMissing or unknown values are hard failures in both checkers.
#789 — the literals are gone
TOOLCHAIN_JOBS/BARE_RUNNER_JOBSdeleted. The literal had a real argument behind it: setequality between two derived sets is blind to a member leaving both at once, so a job losing its
container:block leaves the declared and pinned sets balanced. That needs an anchor that does notmove with the block. The marker is that anchor, better placed — claim and reality in one diff hunk.
Its cost is adjacency, and it is paid rather than argued away. A marker four lines from the
container:block can be deleted with it, where a distant literal could not. So the file derives therequirement a third time — from each job's own step bodies — independent of both marker and
block. That is also the only check that sees the failure #789 actually filed: a .NET step moved
into a bare-runner job, where no set changes at all. It is a necessary condition only
(
functional-e2ereachesdotnetviascripts/e2e-local.sh) and says so.#786 — guard JOBS join a population
New
scripts/tests/test_workflow_job_guards.pyasserts set equality both ways betweenguard-declaring jobs and a new Workflow-job guards table in
docs/guard-inventory.md. The scopelimit that said this class was uncovered is updated, with its two residuals stated (classification
correctness is not proven; an inline assertion carries
Proof: NONEhonestly).The four jobs #786 named as lacking a dropped-step guard each get a recorded decision. None is a
required context, so a drop cannot send a required check green;
ci-step-ran.shis deliberately notwidened.
Two corrections to the issues, measured not assumed
expected="$(git log -1 --format=%H -- docker/ci)"— thepathspec is
docker/cionly. Verified:docker-build.ymlwas last touched bymain's HEAD andexpectedstill resolves to32747a0, matching the pin. No publish-then-pin two-step was needed.Classification
guard= the job's output is a verdict.none= its output is an artifact and a red meansthe build did not work, not that an invariant was violated. That leaves exactly three
nonejobs —the two image publishers and
renovate— anddocs-reminderas the soleadvisory(all three ofits check steps are
continue-on-error).A rejected alternative is recorded in the doc so it is not re-adopted: "a guard enforces an invariant
about the repository, so a job exercising the product is
none" reads as more principled and iswrong — it puts
testandmigrationsoutside the table, and those are the two required statuscontexts on
main, i.e. exactly where a failure to fire is fail-open against branch protection.Review
Three cold review rounds, alternating model families (Claude / Codex gpt-5.6 / Claude), each from a
cold review-only brief in an isolated worktree. Severity converged
BLOCKER/HIGH → MEDIUM → LOW/NIT. Every finding is fixed and demonstrated, not asserted.
Round 1 — the
guard/noneline excluded both required status contexts (rule replaced withverdict-vs-artifact; three markers flipped); two positive controls this branch added were vacuous
(
2N == 2N, true at N=0) while the pre-existing sibling they were copied from was not; a mechanismclaim in the decision record was refuted by execution; the adjacency residual was understated.
Round 2 (cross-family) — the retracted rule still shipped as active in three further sites
including the decision record and the generated catalog, each naming
testasnonewhen it shipsas
guard; duplicate table rows collapsed silently into a dict; thetoolchain-preflight"Blocks"cell was false (nothing
needs:it).Round 3 (both families, independently) — the two new
MUTATION-named proofs passed with thecheck they named fully disarmed: they rebuilt the comparison themselves instead of calling the
shipped code. And
test_a_commented_out_marker_does_NOT_countclaimed to guard a text-scanregression that a reviewer demonstrated goes entirely undetected. Both are the branch's own defect
class reproduced by its own fixes.
duplicate_row_faults/env_shadow_faultsare now nameddetectors that the live test and its proof both call, and
declared_rolestakes an injectablepopulation so the fixture test drives the real reader — each verified by reproducing the
reviewers' own demonstrations against the fix.
Round 3 also renamed the marker value
advisory→report-only:docs/ci-cd.mdalready callsfunctional-e2ean advisory job in a different sense, for the one job whose marker isguard.The rename immediately tripped the row regex (
REPORT-ONLYhas a hyphen, the Kind group was[A-Z]+) — surfacing as a loud red naming the job, which is the fail-closed property documented onecommit earlier, working.
The recurring defect class throughout was prose drifting from code — over half the findings.
Every factual claim in the changed docs and comments has since been re-verified by execution:
tool list vs
docker/ci/Dockerfile, required-context strings vsrequired-status-contexts.json,marker-to-
container:distances (6, 58, 6, 7, 7 — so "same diff hunk" was wrong and is now statedas independence rather than proximity),
continue-on-errorcounts, every row's Assertion cellagainst the workflow, and every cited script against its own
Kind.Verification
scripts/tests: 1175 passed, 2 skipped on the rebased tree (baseline onmain: 1083).docker-build.ymlwitnessed red: container block removed, marker removed,dotnetstep planted in a bare-runner job.(
test_MUTATION_the_declared_clause_reddens_the_named_proof[scripts/tests/test_workflow_job_guards.py]).ruff check/ruff format --checkclean on ruff 0.12.11 (CI's pin);decisions-validate: OK;check-doc-narrative --diff origin/main→ 0 warnings.Docs
docs/ci-cd.md→ "Per-job declarations";docs/guard-inventory.md(new section + scope limit +counts);
docs/README.mdtask-signal row; new decision recordtesting.workflow-declares-its-own-job-metadata; catalog regenerated.fixes #786
fixes #789
Self-review findings on the previous commit, all measured before fixing. THE PREDICATE HAD BOTH FAILURE DIRECTIONS. Measured against constructed inputs: full-line comment -> ['dotnet'] false POSITIVE echo string / heredoc -> ['dotnet'] false POSITIVE dotnet-ef migrations -> [] false NEGATIVE playwright install -> [] false NEGATIVE The false positive is the dangerous one: a bare-runner job whose COMMENT merely mentions dotnet would redden the suite on a correct tree, and a correct guard going red on a correct tree is the fastest route to that guard being deleted. Full-line comments are now stripped before matching. `dotnet-ef` and `playwright` join the tool list, and the alternation is sorted longest-first so the match does not depend on backtracking out of the `dotnet` prefix. The residue is stated in the code rather than papered over: a TRAILING `# ...` comment, an `echo "run dotnet build"` and a heredoc body all still match. Parsing those means parsing shell, which `testing.fixing-a-parser-bug-introduces-the-next-one` prices at three or more rounds. It is tolerable only because of its DIRECTION — a loud red naming the job and the token, with an obvious remedy — never a silent pass. Verified the real workflow's classification is unchanged and exact after the change: needs-toolchain derives to {test, migrations, functional-e2e, api-docs, format}, equal to the declared set, zero violations. THE GUARD/NONE RULE WAS UNDER-SPECIFIED. "Purpose, not whether a step can fail" does not decide `docker-build.yml::test`, which blocks the merge as a required context. Sharpened everywhere it is stated: a `guard` job enforces an invariant about the REPOSITORY or its ARTIFACTS; a job that builds or exercises the PRODUCT is `none` even when it is a required check. No marker value changed — the rule now explains the classifications that were already there. Also: a row for test_workflow_job_guards.py in the file-populations table, and the scope-limit preamble de-narrated (it recounted an earlier draft; the operative warning — an unlisted class reads as covered — is kept, the autobiography dropped). Refs #786 Refs #789Round three, reviewed twice in parallel (Codex gpt-5.6 and a cold Claude brief). Both converged independently on the same two MEDIUM findings, which are the branch's own defect class reproduced by its most recent commits: a test asserting a guarantee it does not provide. 1. THE PROOFS DID NOT EXERCISE THE CODE THEY NAMED. `test_MUTATION_a_DUPLICATE_row_is_reported` rebuilt the comparison with its own `len(set(...))`, and `test_MUTATION_a_SHADOWING_env_key_is_ reported` built a synthetic dict and intersected it by hand. Both stayed GREEN with the live check disarmed — a behavioural test wearing a MUTATION name, which is exactly what `testing.guard-ships-with-mutation-proof` excludes. `duplicate_row_faults(rows)` and `env_shadow_faults(docs)` are now named detectors; the live test and its proof both call them. Verified by reproducing the reviewers' own demonstrations against the fix: duplicate detector neutered -> REDDENED "the DETECTOR did not report a second row for ..." shadow detector neutered -> REDDENED "the DETECTOR did not report a shadowing job-level key" 2. `test_a_commented_out_marker_does_NOT_count` CLAIMED TO GUARD A TEXT-SCAN REGRESSION AND DID NOT. It re-implemented the read inline and never called `declared_roles`. One reviewer built a comment-blind grep reader reproducing all 17 current values byte-identically and ran every non-parametrised test in the file: nothing went red. `declared_roles(paths=None)` now takes an injectable population, so the test drives the REAL reader over a fixture, asserts the fault function reports it, and pins that a live marker in the same shape is still read (so the assertion cannot be satisfied by a reader returning None for everything). Verified: installing the grep reader now reddens it. RENAMED `advisory` -> `report-only`. docs/ci-cd.md already calls `functional-e2e` an advisory job in a DIFFERENT sense — not a required context, though it can certainly fail — while this branch declares that job `guard`. A reader would infer the wrong marker for exactly the job whose marker is `guard`. The value is now unambiguous, its Kind is `REPORT-ONLY`, and the manifest's declared `expect` string moved in lockstep with the message it matches. The rename also caught its own adjacent defect: `REPORT-ONLY` contains a hyphen, which the row regex's `[A-Z]+` Kind group did not match, so the docs-reminder row stopped parsing. That surfaced as a loud red naming the job — the fail-closed property documented one commit earlier, working. Widened to `[A-Z-]+`, matching the `BEHAVIOUR-ONLY` convention in the sibling table. Nine further accuracy findings, each measured before fixing: * "would therefore flag every job" — false. git/jq/python3/curl flag 4 of 8. And the REASON was wrong too: `small` is git-only and provisions Python via actions/setup-python, while `build` runs on stock ubuntu-latest, so "the bare runner has them" conflated two lanes. They are excluded because they do not DISCRIMINATE. * "four lines from the container: block" / "in one diff hunk" — false. Measured 6, 58, 6, 7, 7; `migrations` has the whole `services:` block in between. The property the guard rests on is INDEPENDENCE, not proximity, and the docs now say so. * "the merge semantics never have to be relied upon" — too strong. The check proves no NAME COLLISION; it does not prove the runner merges. That evidence is in-repo and live and is now cited: `build`'s Smoke step declares its own step-level `env:` and consumes workflow-level `${IMAGE}` on the same line, on every push to main. * "every malformed row fails closed" — over-broad. A malformed DUPLICATE is invisible: its job is already satisfied by the well-formed row. Stated. * The `scan` Blocks cell said "a marked job's run: body"; the ban's scope is DELIMITER_BAN_JOBS (the required contexts PLUS `build`), which is wider, and `scan` carries no `if:` so it runs on PRs too. * The functional-e2e row credited `e2e-local.sh` (TOOLING, only boots the instance) for curl assertions that live in `e2e-functional.sh` (GUARD). * `report-only` was defined nowhere; now defined as a verdict the job cannot enforce, with the limit stated (nothing detects a `guard` job whose checks are all continue-on-error). * "inline rows carry Proof: NONE" was stated universally while two rows cite a pytest; hedged, and each such row names which PART it covers. * "Exactly three jobs are `none`" was an unchecked prose enumeration in three sites, in a branch that deleted one for that reason. Removed; the markers are the source. * Plus: the ci-cd tool list was short by `dotnet-ef` and `playwright`; docs/README said only a `guard` job needs a row; "all three checks fail together" read as the opposite of "go blind"; and #774's unrealised consequence is now subjunctive rather than past tense. Suite: 1178 passed, 2 skipped, 1 known flake (test_hook_fire_log's production-log guard, which compares mtimes under the SHARED ~/.cache/ersatztv/hook-fire/ — four session logs were written during the window; 78 passed in isolation. ersatztv#809/#822, evidence posted there). Refs #786 Refs #789`test_the_marker_is_read_from_the_PARSED_yaml_not_a_text_search` claimed to guard the parser against regressing to a grep. It asserted only that every tracked job appears as a KEY, so a reader returning the right keys with wrong VALUES satisfied it — the claim was stronger than the code. Cold review found it; the same round found the sibling test making the same over-claim, and that one is now fixed by driving the real reader. Renamed to what it proves (`test_every_tracked_job_APPEARS_in_the_parse`), with the grep-regression claim moved to `test_a_commented_out_marker_does_NOT_count`, which actually drives the reader. Added the property a completeness check structurally cannot express: ATTRIBUTION. A marker belongs to the job whose `env:` block contains it. A line-oriented reader that attached each `CI_JOB_ROLE:` to the nearest preceding job header would smear one job's marker onto a neighbour declaring none, and both jobs still have keys, so nothing phrased over keys can see it. The fixture is two jobs, only the second marked. Witnessed, and the control matters: the smearing reader used to demonstrate it is byte-identical to the real `declared_roles()` on the current tree — faithful on the real tree? True REDDENED: the reader attributed a marker to the wrong job — `first` declares none and must read as None, `second` declares one and must read as its value — so it satisfies every population and completeness assertion in the file, and only the new fixture separates them. Refs #786Round four returned MERGEABLE with seven LOW/NIT items. All seven are fixed rather than shipped, because three of them are the same defect class this branch has been closing all along. THREE MORE PROOFS RE-IMPLEMENTED THE COMPARISON THEY NAMED. The previous commit extracted `duplicate_row_faults` and `env_shadow_faults` for exactly this reason and left the set-equality and Kind comparisons inline, so `test_MUTATION_a_guard_job_with_its_row_DELETED_is_reported`, `…_a_PHANTOM_row_is_reported` and `…_a_row_whose_KIND_disagrees_is_reported` still rebuilt `declared - listed` and the Kind check themselves. Review demonstrated it by flattening the live assertion and watching the file stay green at 50 passed. `coverage_faults(declared, listed) -> (missing, phantom)` and `kind_faults(roles, listed)` are now named detectors that the live checks and all three proofs call, and each proof gained a negative control. Witnessed after the change: coverage_faults disarmed -> deleted-row proof REDDENED, phantom-row proof REDDENED kind_faults disarmed -> kind-disagreement proof REDDENED Worth recording that the system was never fail-open here: the manifest harness already caught the same disarm ("the named test still PASSED with the clause mutated"). The in-file proofs were weaker than their names, not absent. THE ENV-MERGE EVIDENCE WAS STEP-SCOPE USED FOR A JOB-SCOPE CLAIM. `build`'s Smoke step shows the runner LAYERS step-level `env:` over workflow-level, and what this branch newly relies on is job-level layering. Measured: on origin/main no job anywhere declared both a job-level `env:` and a consumed workflow-level var, so the job scope had no in-repo witness. The gap is now named, with the exposure bounded — the two publish jobs are `if: github.event_name != 'pull_request'`, so PR CI cannot exercise it and a non-layering runner would fail loudly on the first push to `main` trying to publish `":<sha>"`, not silently. Four wording fixes, each measured: * the record still enumerated "exactly three jobs are `none`" while three other sites say a list in prose is a second copy that rots — the fourth site now agrees with them. * "the remaining `docker-build.yml` jobs" over-scoped: six remain after `test`/`migrations`, and the table has four, because `toolchain-preflight` and `scan` carry the mark/assert protocol voluntarily without being in the enforced scope. Said so. * "on every push to `main`" ignored the Smoke step's own `docs_only` gate. * `docs/ci-cd.md`'s excluded-token list omitted `tar`, which the source comment includes; and the `scan` Proof cell disclosed the two pytest files' `NONE` but not the cited script row's, two cells from a sibling that discloses it — so that row now states it claims no proof at all. Suite: 1178 passed, 2 skipped (the production-log guard deselected; it is the shared-$HOME flake, green in isolation — #809/#822). ruff 0.12.11 clean over all 47 tracked files; decisions-validate OK; doc-narrative 0 warnings. Refs #786 Refs #789if:-gated job 64d80e5d28Review-verdict: MERGEABLE @
d3f470fFour cold adversarial rounds, alternating model families (Claude, Codex gpt-5.6, both in parallel, then a scoped confirmation pass), each from a review-only brief in an isolated worktree. Rounds 1-3 returned BLOCKED; every finding is fixed and each fix demonstrated by reproducing the reviewer's own test. Round 4 returned MERGEABLE and the confirmation pass verified the detector extraction clean over 4000 randomized equivalence cases per detector with zero mismatches; its one blocking finding had already been fixed a commit before it reported, and its NIT is closed. All five mutation proofs redden when their shipped detector is disarmed. Local suite 1185 passed 2 skipped on the final rebased tree; CI green on
d3f470fwith Build and test at 514s and Script lint and tests at 444s, so neither is a fast-path phantom.