🐛 fix(ci): -count=1 on the cache-blind dogfood gates (COUNT1) - #84
Merged
Conversation
…2e gates `task dogfood-comparison` ran `go test ./examples/comparison/...` with no -count=1, so it could print `ok ... (cached)` and exit 0 on a tree where the same test genuinely fails. Mechanism: the Go test cache keys on the test BINARY's content ID. examples/comparison/validate_test.go is an EXTERNAL test package that references only exported constants from internal/compare (GateZeroMissedDestructive, ExitCodeForGate), so the comparison logic is unreachable from that binary and the linker strips it — the content ID does not move when internal/compare changes. The logic under test actually runs in a subprocess the test builds at runtime (`go build -o ... ./cmd/assent`), entirely outside the cache key. Corpus DATA edits do invalidate the cache (copyDir os.ReadFile's them, so testlog records them); production code changes do not. Measured before the fix: deleting `|| e == aggregate.EffectChallenge` from isStricterInterventionEffect left `task dogfood-comparison` green-and-cached (exit 0) while `go test -count=1` on the identical tree failed (exit status 6). After the fix the same mutated tree reddens the stage. `task e2e` gets the same treatment: its subject is a PREBUILT bin/assent driven against a live GitLab endpoint — neither the binary bytes nor the forge state is in the cache key. It is not part of `task check`, so this costs nothing. Left alone deliberately: `test` (whole-tree, in-process subjects, honest cache and a real time saver), `coverage` (in-process subjects; -coverprofile results do cache but the profile is reproduced faithfully) and `determinism` (-count=2 is uncacheable by construction). A -count=1 someone deletes later is the D-124 "gate invoked by nothing" species, so it is pinned in hack/examples/dogfood_wiring_test.sh — an existing `task check` stage that already checks Taskfile wiring, which keeps CHECK_STAGES at 19. The pin matches the `go test` COMMAND LINE, never a comment, and carries three controls: a comment-only fixture that must NOT satisfy it, a mutation that strips the flag from the command lines while leaving the explanatory comments intact, and the existing wiring mutation for `- task: dogfood-comparison`. Known residual, out of this lane's write paths: .github/workflows/verify.yaml line 102 runs the same uncounted `go test ./examples/comparison/...`, and actions/setup-go@v7 restores GOCACHE by default, so CI can serve a cached PASS across commits too.
The previous commit closed the LOCAL half of the stale-cache hole. This closes
the half that matters more: .github/workflows/verify.yaml ran its own
`go test ./examples/comparison/...` with no -count=1, and
actions/setup-go@v7.0.0 restores GOCACHE (where test results live) across
commits with no `cache: false` anywhere in the repo — so CI could serve
`ok ... (cached)` and turn a genuinely red tree into a green PR. Same
mechanism as the Taskfile half: the external test package references only
exported constants from internal/compare, the linker strips the compare logic
out of the test binary the cache keys on, and the real subject is a
`go build -o ... ./cmd/assent` subprocess outside the cache key entirely.
Fixed per invocation, deliberately. Do NOT "fix" this by setting
`cache: false` on setup-go: that disables the Go build cache for every step of
every job in every workflow — a broad, permanent speed regression to close one
blind invocation. -count=1 on the affected command is the targeted fix.
Audited and left alone in the workflows, with reasons:
* verify.yaml `go test -race ./...` — whole-tree, in-process subjects; the
cache key is honest and it is a real time saver. Its separate (-race)
cache entry for examples/comparison is covered because the step below now
re-runs that package with -count=1 in the same job.
* verify.yaml determinism block — `-count=2` is uncacheable by construction.
* schemas.yml `go test -race ./schemas/...` — in-process compile+validate;
fixtures are read with os.ReadFile, which testlog records, so edits do
invalidate. No subprocess, no external binary.
The pin now covers both halves. hack/examples/dogfood_wiring_test.sh section 6
asserts that EVERY `go test ... ./examples/comparison...` command in
verify.yaml carries -count=1, matching the command line and never a comment,
and distinguishes "flag missing" (rc=1) from "command absent" (rc=2) so the
assertion cannot pass by going vacuous. Three controls, all observed red for
their own reason: a comment-only fixture; the flag stripped from the real
command line with the explanatory comment left intact; and the command deleted
outright. No new `task check` stage — CHECK_STAGES stays 19.
Evidence this closed a real hole rather than adding a defensive flag:
openspec/specs/p5-aud2-audit-remediation/spec.md:477 reads
`Verify: delete the EffectChallenge term; task dogfood-comparison reddens`.
That Verify step was FALSE before this lane (measured: the stage stayed green
and cached) and is TRUE after it. The spec file is Integrator-owned and left
untouched.
Review F1 — the lane's own thesis did not apply to the lane. The wiring pin reached CI only through `task check` -> release-exitgate, which carries `if: github.event_name != 'pull_request'`. A PR that stripped -count=1 from verify.yaml would have merged green and reddened main afterwards: RELSE-08, the blind spot verify.yaml already names as what let AUD-S18's stale CHECK_STAGES pin merge green four times. Fixed the way AUD2-S05 fixed it — the gate now runs as a step in the pull-request-visible `verify` job, beside workflow_pins_test.sh and before setup-go, since it is pure text over Taskfile.yml and this workflow and needs no toolchain. That step is itself pinned (section 7), mirroring AUD2-S05's check_pr_wiring, with a distinct return code per failure mode so a mutation control proves the branch it claims. Six controls, each observed red for its own code: step deleted (5); invocation COMMENTED OUT while the step comments still name the script (5); invoked with an argument (7); made advisory with continue-on-error (8); the verify JOB given release-exitgate's push-only guard (4); the workflow's `on:` losing pull_request entirely (2). The commented-out control caught a real bug while it was being written: a fixed-string presence check was satisfied by verify.yaml's own "Pinned by hack/examples/dogfood_wiring_test.sh" comment, so the check is now anchored on a `run: bash <script>` COMMAND. Review F2 — workflow_count1_pinned greped the whole line, so `run: go test ./examples/comparison/... # -count=1` satisfied the pin with a genuinely uncounted command in CI. Now anchored the same way count1_pinned already was (`go test[^#]*[[:space:]]-count=1`). The reviewer's two-step exploit (one properly flagged step, one flag-in-a-trailing-comment step) is replayed and rejected. Two new controls: the trailing-comment exploit must be refused, and a genuinely counted command carrying an unrelated trailing comment must still pass — comment-blind, not comment-hostile. Review F3 — narrowed the shipped claim about `test`. It said "in-process subjects, honest cache", which is over-broad by this lane's own criterion: internal/schemadrift compares local schemas/ against `git show origin/main:...` in a SUBPROCESS (invisible to testlog) and internal/provider/isolation_test.go `go build`s a fixture at runtime. The verdict is unchanged (leaving `test` alone is still right); only the reason is now true. Those two are logged as a follow-up, not fixed here. Correction to the previous commit's framing, because accuracy matters more than a good story: hack/compare/exitgate_test.sh runs in the PR-visible verify job, drives `assent compare --suite` as a built binary through bash with no Go test cache in the path, and DOES redden on the same mutation. So the corpus OUTCOME was already defended on PRs. What the uncounted stage silently stopped contributing was validate_test.go's JSON-schema validation of the suite docs, the corpusGateCoverage gate-ID map, and TestCompareCorpusGateFailurePath. The accurate claim is "this stage contributed nothing", not "this mutation would have shipped". The verify.yaml step comment now says exactly that. No new `task check` stage: CHECK_STAGES stays 19 and hack/audit/exitgate_test.sh is untouched — that array pins task check stages, not workflow steps.
Re-review R2-01 — the "Deliberately NOT listed" block carried two false claims. (a) The `coverage` bullet still said "in-process subjects" one line below the `test` bullet that names internal/schemadrift and internal/provider as NOT in-process — and `task coverage` runs `./internal/...`, which contains both. The criterion was applied to one bullet and not the one beside it, in the same commit. The verdict is unchanged (leaving `coverage` alone is still right); the REASON is now the true one: what makes the gated number honest is that a cached `-coverprofile` run REGENERATES the profile faithfully, so the evidence this gate reads is never a stale number even when a cached package result is stale. (b) "Those are logged as a follow-up" pointed at nothing a reader could find — the residual was logged in a gitignored file. Fixed by making the claim true rather than by rewording it: openspec/specs/backlog.md now carries COUNT1-F01 in the "Found in flight, NOT an AUD2 story" block, in AUD2-F01's shape and beside it, with a sizing paragraph that separates the two cases (schemadrift is the real one — the `git show origin/main:…` baseline is a subprocess testlog cannot see, so a cached PASS goes stale exactly when origin/main moves; provider is narrower — the production subject IS linked, only fixture edits are invisible) and two traps, the first being that the obvious fix (-count=1 on test/coverage) is the wrong one. The status line now names both F-rows as open. The script comment points at the row ID. R2-02 — 7b now states its bound instead of implying completeness: rc=3 (verify job block unextractable) has no control; rc=6's other route (a region that is not a step) needs a workflow actionlint cannot parse. R2-03 — deleting only this step's `- name:` merges it into its neighbour, and pr_step_pinned returned 0 on that malformed workflow (measured). Bounded, and rc=6 now has a control. A LINE cap of the workflow_pins_test.sh:433-440 shape is the wrong instrument here: this step's honest size is 14 lines and its merged size 15, so any cap admitting the first admits the second. The invariant used instead is the one actionlint itself enforces — a step has exactly ONE `run:` key — which holds however long the comment block grows. Gates: task check 201 (changelog-verify only, CHANGELOG.md Integrator-owned); every stage after it run individually green except release-changelog-gate-test, same one cause; aud2/AUD-S18(--text-only)/EX/compare exit gates 0; actionlint 0 over all workflows with shellcheck present; `shellcheck -x` 0 on the gate script. CHECK_STAGES stays 19.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
COUNT1 —
-count=1on the cache-blind gatesFixes a real P1:
task dogfood-comparisonand itsverify.yamltwin could printok … (cached)and exit 0 on a genuinely failing tree. Also moves the wiring pininto the PR-visible
verifyjob — previously it ran only in the push-onlyrelease-exitgatejob (RELSE-08 blind spot).Changes
Taskfile.yml—-count=1on the cache-blinddogfood-comparisonand e2e gates.github/workflows/verify.yaml—-count=1on the comparison-corpus dogfood; thedogfood_wiring_test.shpin now runs in the PR-visibleverifyjobhack/examples/dogfood_wiring_test.sh— asserts the-count=1wiring in bothTaskfile.ymlandverify.yaml, plus a §7b mutation control:pr_step_pinnednowreturns rc=6 via a new
n_run == 1invariant where it previously returned 0 on aworkflow whose step had been merged into its neighbour
openspec/specs/backlog.md— tracksCOUNT1-F01(two residualgo testcache blindspots deliberately left alone) and corrects a false coverage claim
CHANGELOG.md— regenerated post-rebase (Integrator-owned)Notes
.gofiles.CHECK_STAGESstays 19 — this adds a workflow step, not atask checkstage;hack/audit/exitgate_test.shis byte-unchanged.backlog.mdagainst AUD2-F01's completion onmain;resolved by hand keeping main's
AUD2-F01 | **DONE**row and paragraph, adding theCOUNT1-F01row beside it, and restating the closing status so onlyCOUNT1-F01is open.
Verification on the rebased head
task check→ 0 (all 19 stages)bash hack/examples/dogfood_wiring_test.sh→ 0actionlint .github/workflows/*.yaml→ 0bash hack/audit/exitgate_test.sh→ 0bash hack/release/exitgate_test.sh→ 0 (run locally becauserelease-exitgateisskipped on PRs — RELSE-08 — and it is the job that greps
backlog.md)This PR is the first to exercise the new
dogfood_wiring_test.shstep inverify.