timothy/ersatztv
1
0
Fork 0
Code Issues 54 Pull Requests 6 Actions Packages Projects Releases 9 Wiki Activity

Derive the CI toolchain population from workflow-owned metadata, not TOOLCHAIN_JOBS #789

New Issue
Closed
opened 2026-08-13 22:21:30 +02:00 by timothy · 3 comments
timothy commented 2026-08-13 22:21:30 +02:00
Owner
Copy Link
Copy Source

Raised by the Codex cold review of #774. scripts/tests/test_ci_image_pin_population.py anchors on a hand-written TOOLCHAIN_JOBS registry, cross-checked both ways against the parsed workflow. That was chosen because set equality between two DERIVED sets is blind to a job losing its container: block — both sides shrink together — so the population needs an anchor that does not move.

The reviewer's objection stands anyway: TOOLCHAIN_JOBS enumerates individual jobs by hand, and calling it scope rather than population does not change that. The partition test proves every job is classified; it cannot prove the classification is correct.

Concrete failure it does not catch: move a .NET-dependent step into build or scan while leaving that job in BARE_RUNNER_JOBS. Every test stays green while the job now needs the toolchain image and does not have it.

Proposed: an explicit machine-readable execution-class marker on each job in the workflow itself, with both sets derived from the parsed YAML, unknown/missing markers rejected, and the marker-derived toolchain set compared against actual container.image values.

Sequencing hazard: editing docker-build.yml re-points ci-image-pin's expected at the editing commit, so this needs the publish-then-pin two-step (docs/ci-cd.md → CI toolchain image). Same trap that kept #744 out of a bundle.

Done-when

  • Execution class declared per job in the workflow, machine-readable
  • Both sets derived; missing or unknown marker is a hard failure
  • TOOLCHAIN_JOBS/BARE_RUNNER_JOBS removed
  • Adversarial review passed
Raised by the Codex cold review of #774. `scripts/tests/test_ci_image_pin_population.py` anchors on a hand-written `TOOLCHAIN_JOBS` registry, cross-checked both ways against the parsed workflow. That was chosen because set equality between two DERIVED sets is blind to a job losing its `container:` block — both sides shrink together — so the population needs an anchor that does not move. The reviewer's objection stands anyway: `TOOLCHAIN_JOBS` enumerates individual jobs by hand, and calling it *scope* rather than *population* does not change that. The partition test proves every job is classified; it cannot prove the classification is **correct**. Concrete failure it does not catch: move a .NET-dependent step into `build` or `scan` while leaving that job in `BARE_RUNNER_JOBS`. Every test stays green while the job now needs the toolchain image and does not have it. Proposed: an explicit machine-readable execution-class marker on each job in the workflow itself, with both sets derived from the parsed YAML, unknown/missing markers rejected, and the marker-derived toolchain set compared against actual `container.image` values. **Sequencing hazard:** editing `docker-build.yml` re-points `ci-image-pin`'s `expected` at the editing commit, so this needs the publish-then-pin two-step (`docs/ci-cd.md` → CI toolchain image). Same trap that kept #744 out of a bundle. ## Done-when - [x] Execution class declared per job in the workflow, machine-readable - [x] Both sets derived; missing or unknown marker is a hard failure - [x] `TOOLCHAIN_JOBS`/`BARE_RUNNER_JOBS` removed - [x] Adversarial review passed
timothy added the ci-cdpriority: medium labels 2026-08-13 22:21:30 +02:00
timothy added the in-progress label 2026-08-28 19:10:54 +02:00
timothy commented 2026-08-28 19:11:17 +02:00
Author
Owner
Copy Link
Copy Source

Claiming for this session (Claude Code / Opus 5, orchestrator).

Bundling #786 + #789 — they are one mechanism, and working #786 alone would build the artifact #789 removes:

  • #786 asks for inline workflow-job guards to be enumerated with "the same reviewed-registry-cross-checked-both-ways shape TOOLCHAIN_JOBS uses".
  • #789 says TOOLCHAIN_JOBS is itself the defect and must be deleted in favour of a machine-readable per-job marker declared in the workflow, with both sets derived from the parsed YAML.

So the bundle resolves the tension in one direction: one per-job execution-class declaration owned by the workflow, both populations derived from it, unknown/missing marker a hard failure. Registry-shaped hand lists go away rather than multiply.

Progress and any scope cuts will be recorded here.

Claiming for this session (Claude Code / Opus 5, orchestrator). Bundling **#786 + #789** — they are one mechanism, and working #786 alone would build the artifact #789 removes: - #786 asks for inline workflow-job guards to be enumerated with "the same reviewed-registry-cross-checked-both-ways shape `TOOLCHAIN_JOBS` uses". - #789 says `TOOLCHAIN_JOBS` is itself the defect and must be **deleted** in favour of a machine-readable per-job marker declared in the workflow, with both sets derived from the parsed YAML. So the bundle resolves the tension in one direction: one per-job execution-class declaration owned by the workflow, both populations derived from it, unknown/missing marker a hard failure. Registry-shaped hand lists go away rather than multiply. Progress and any scope cuts will be recorded here.
timothy commented 2026-08-28 22:29:42 +02:00
Author
Owner
Copy Link
Copy Source

PR #872 is open, covering this and its sibling — the two were bundled because working either alone
would have built the artifact the other removes.

Review: four cold adversarial rounds, alternating model families (Claude / Codex gpt-5.6 /
both in parallel / Claude), each from a review-only brief in an isolated worktree. Severity
converged BLOCKER/HIGH → MEDIUM → LOW/NIT; the final confirmation pass verified the substance clean
(detector-extraction equivalence over 4000 randomized cases per detector, zero mismatches) and its
one blocking finding had already been fixed a commit before it reported.

The recurring defect class was prose drifting from code — over half of all findings, including a
mechanism claim in the decision record that execution refuted. Every factual claim in the changed
docs and comments has since been re-verified by execution.

Two corrections to the issues themselves, both measured rather than assumed, are recorded in the
closing records.

Awaiting CI on d3f470f, then the H10 verdict and the merge gate.

PR #872 is open, covering this and its sibling — the two were bundled because working either alone would have built the artifact the other removes. **Review:** four cold adversarial rounds, alternating model families (Claude / Codex gpt-5.6 / both in parallel / Claude), each from a review-only brief in an isolated worktree. Severity converged BLOCKER/HIGH → MEDIUM → LOW/NIT; the final confirmation pass verified the substance clean (detector-extraction equivalence over 4000 randomized cases per detector, zero mismatches) and its one blocking finding had already been fixed a commit before it reported. The recurring defect class was **prose drifting from code** — over half of all findings, including a mechanism claim in the decision record that execution refuted. Every factual claim in the changed docs and comments has since been re-verified by execution. Two corrections to the issues themselves, both measured rather than assumed, are recorded in the closing records. Awaiting CI on `d3f470f`, then the H10 verdict and the merge gate.
timothy commented 2026-08-28 22:55:15 +02:00
Author
Owner
Copy Link
Copy Source

Closing record

Outcome: Closed by PR #872. TOOLCHAIN_JOBS and BARE_RUNNER_JOBS are deleted from
scripts/tests/test_ci_image_pin_population.py. Each docker-build.yml job declares
env.CI_EXECUTION_CLASS (toolchain/bare-runner); both sets derive from it, and a missing or
unrecognised marker is a hard failure.

Root cause: the literal was not carelessness — set equality 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. The population needed an anchor that does not move with the block, and in 2026-08 a
reviewed list was the only one available. The workflow can hold that anchor itself.

Decisions/conventions changed: testing.workflow-declares-its-own-job-metadata (shared with
#786).

Reusable knowledge:

  • A marker beside the thing it describes is a better anchor than a distant literal, but the cost is
    ADJACENCY and must be paid, not argued away.     A literal three directories away survives a careless
    workflow edit; a marker four lines from the container: block does not. So the change ships a
    THIRD derivation — the job's own step bodies scanned for tools present only in the CI image —
    independent of both marker and block, and it is the only one of the three that can see this issue's
    concrete complaint (a .NET step MOVED into a bare-runner job, where no set changes at all).
  • The residual is real and is disclosed rather than closed: drop the container block, flip the
    marker AND move the invocation behind a script, and all three checks go blind. What bounds it is
    the failure MODE — the job then dies on a missing binary, loudly — where #774's original defect
    sent a REQUIRED check green on the bare runner. A silent pass traded for a noisy crash.
  • This issue's stated sequencing hazard is FALSE, measured. It says editing docker-build.yml
    re-points ci-image-pin's expected, requiring a publish-then-pin two-step. The line is
    expected="$(git log -1 --format=%H -- docker/ci)" — the pathspec is docker/ci only. Verified:
    docker-build.yml was last touched by main's HEAD and expected still resolved to 32747a0,
    matching the pin. No two-step was needed. (Since #744 the same is true of ci-image.yml.)
  • A string predicate over shell needs measuring in BOTH directions before it ships. The first cut
    matched inside comments, echo strings and heredocs (false positives — a bare-runner job whose
    COMMENT mentions dotnet would redden a correct tree) and missed dotnet-ef and playwright (false
    negatives). Full-line comments are now stripped and the alternation is sorted longest-first; the
    remaining residue is stated in the code, and is tolerable only because its direction is a loud red
    with an obvious remedy, never a silent pass.

Verification: scripts/tests 1185 passed / 2 skipped on the final rebased tree. Three real mutations of docker-build.yml
witnessed red (container block removed, marker removed, dotnet step planted in a bare-runner job);
the declared clause in mutation_manifest.py retargeted to the surviving comparison and re-run by
the harness. Two cold adversarial reviews; first returned BLOCKED with four findings, all fixed.

Deferred: none.

Docs updated: docs/ci-cd.md ("Per-job declarations"), docs/guard-inventory.md,
docs/README.md, new decision record + regenerated catalog.

## Closing record **Outcome:** Closed by PR #872. `TOOLCHAIN_JOBS` and `BARE_RUNNER_JOBS` are deleted from `scripts/tests/test_ci_image_pin_population.py`. Each `docker-build.yml` job declares `env.CI_EXECUTION_CLASS` (`toolchain`/`bare-runner`); both sets derive from it, and a missing or unrecognised marker is a hard failure. **Root cause:** the literal was not carelessness — set equality 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. The population needed an anchor that does not move with the block, and in 2026-08 a reviewed list was the only one available. The workflow can hold that anchor itself. **Decisions/conventions changed:** `testing.workflow-declares-its-own-job-metadata` (shared with #786). **Reusable knowledge:** - **A marker beside the thing it describes is a better anchor than a distant literal, but the cost is ADJACENCY and must be paid, not argued away.** A literal three directories away survives a careless workflow edit; a marker four lines from the `container:` block does not. So the change ships a THIRD derivation — the job's own step bodies scanned for tools present only in the CI image — independent of both marker and block, and it is the only one of the three that can see this issue's concrete complaint (a .NET step MOVED into a bare-runner job, where no set changes at all). - **The residual is real and is disclosed rather than closed:** drop the container block, flip the marker AND move the invocation behind a script, and all three checks go blind. What bounds it is the failure MODE — the job then dies on a missing binary, loudly — where #774's original defect sent a REQUIRED check green on the bare runner. A silent pass traded for a noisy crash. - **This issue's stated sequencing hazard is FALSE, measured.** It says editing `docker-build.yml` re-points `ci-image-pin`'s `expected`, requiring a publish-then-pin two-step. The line is `expected="$(git log -1 --format=%H -- docker/ci)"` — the pathspec is `docker/ci` only. Verified: `docker-build.yml` was last touched by `main`'s HEAD and `expected` still resolved to `32747a0`, matching the pin. No two-step was needed. (Since #744 the same is true of `ci-image.yml`.) - **A string predicate over shell needs measuring in BOTH directions before it ships.** The first cut matched inside comments, `echo` strings and heredocs (false positives — a bare-runner job whose COMMENT mentions dotnet would redden a correct tree) and missed `dotnet-ef` and `playwright` (false negatives). Full-line comments are now stripped and the alternation is sorted longest-first; the remaining residue is stated in the code, and is tolerable only because its direction is a loud red with an obvious remedy, never a silent pass. **Verification:** `scripts/tests` 1185 passed / 2 skipped on the final rebased tree. Three real mutations of `docker-build.yml` witnessed red (container block removed, marker removed, `dotnet` step planted in a bare-runner job); the declared clause in `mutation_manifest.py` retargeted to the surviving comparison and re-run by the harness. Two cold adversarial reviews; first returned BLOCKED with four findings, all fixed. **Deferred:** none. **Docs updated:** `docs/ci-cd.md` ("Per-job declarations"), `docs/guard-inventory.md`, `docs/README.md`, new decision record + regenerated catalog.
timothy removed the in-progress label 2026-08-28 22:55:31 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ad-hoc
One-off / ad-hoc work not tracked by a dedicated issue
 api
REST API / HTTP endpoints
 bug
Something isn't working
 ci-cd
Build, test, deploy pipeline
 content
Channel content / schedules / playlists
 dependencies
Dependency updates (Renovate)
 enhancement
New feature or improvement
 frontend
ChicoryTV React SPA frontend
 in-progress
Claimed by an active session — do not pick up
 jellyfin
Jellyfin tuner / IPTV integration
 parked
Excluded from automatic queue pickup; work only when explicitly selected
 priority: high
priority: low
priority: medium
review
Adversarial review finding
 security
Security / vulnerability fix
No labels ci-cd priority: medium
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
renovate timothy (Timothy)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#789