fix(ca): the coverage gate could not run, in any tree (#507)#513
Merged
Conversation
`plugins/ca/tools` was the last tree on vitest 3. ca-pi pins 4.1.9 exactly and ca-sandbox resolves to it, so one sibling ran a different test-runner major than the other two - divergent behavior, divergent docs, and a migration debt that only grows. It also blocked #507: `@vitest/coverage-v8` peer-pins vitest exactly, so coverage tooling could never reach this tree, and the v3-compatible fallback carries 5 HIGH advisories that fail the dev-inclusive audit gate. Pinned exact rather than caret, matching ca-pi, because the coverage-v8 peer is an exact pin and a drifting specifier ERESOLVEs on any single-side bump. The lock now carries rolldown, lightningcss, @emnapi and @napi-rs/wasm-runtime, which vitest 4 uses for its test transform. None of it reaches the artifact: `farm.js` is built by esbuild invoked directly, not by vitest's pipeline. That is verified rather than argued - the built file is byte-identical, and the absence check required by security-controls.md:266-268 finds no rolldown, lightningcss, emnapi, napi-rs, `.node`, `.wasm`, or vite reference in it, with zero non-`node:` requires. Licenses need no new decision. security-controls.md already extended MPL-2.0 and 0BSD to development-only tooling under `plugins/*/tools` on 2026-07-14, naming the ca-pi Vitest 4.1.9 lock gate as the case it resolved. This puts ca/tools into that same approved posture: its lock is now a superset of the already-approved ca-pi lock by exactly one package, `tsx`, which it already declared. No test changed. 323 passed, 1 skipped before and after. CHANGELOG: none - development tooling only, no shipped payload change. Ref: #507 Claude-Session: https://claude.ai/code/session_01JgdLb6n8mUdkFiKDTA37ML
`tdd` Phase 5 and `refactor` Phase 2/6 instruct "run the coverage command from `tech-stack.md`", and both forbid guessing one. No coverage command existed - no provider installed in any of the three TypeScript trees, no script, and the word "coverage" appeared in tech-stack.md zero times. Every run reached the phase, found nothing to run, and passed through on a gap. A BLOCK gate that cannot execute is worse than an absent one: it reads as satisfied in every lane. Same defect class as #501's five suites that ran on nothing and #506's assertion that restated the bug. `refactor` was the worse casualty. Phase 2 is "Behavioral parity coverage proof" - the gate that justifies the entire lane by showing the pre-existing tests can detect a behavior change BEFORE production code is touched - and it rested wholly on a command that did not exist. The lane was running on Phase 5's "zero pre-existing tests modified" alone. The threshold table never named a metric, and that omission was load-bearing rather than cosmetic: a report gives four numbers that disagree, so "at least 70%" with no column named is not something anyone can be held to. ca-pi measured 85.37% lines against 78.73% branches - compliant at maturity 3, or not, depending purely on which column the reader picked. It is now lines AND branches, both binding. Lines catches code no test reaches at all, which is what #504 turned out to be: a `catch` with zero executions inside a 750-test suite. Branches catches the untaken half of a condition a test does reach. The threshold is deliberately NOT encoded in the three vitest configs. The skills apply it from CONTEXT.md `stage:` against maturity-coverage.md; copying the number into three package configs would fork the source of truth and the copies would drift the first time the stage moves. No CI job enforces coverage, also deliberately. `plugins/ca/tools` is below the stage-2 floor (67.22% lines, 59.46% branches), so wiring it into required CI would block every merge in the repo on an unrelated backfill. That backfill is tracked as its own issue rather than absorbed here, which would have turned #507 into "write several hundred statements of tests". A contract test now asserts that any command a gate reads from tech-stack.md is actually defined there. Verified against the pre-fix file rather than assumed: it fires. CHANGELOG: tdd Phase 5 and refactor Phase 2/6 can run their coverage gate; the maturity threshold now specifies lines and branches, both binding. Closes #507 Refs #511 Claude-Session: https://claude.ai/code/session_01JgdLb6n8mUdkFiKDTA37ML
# Conflicts: # .codearbiter/gate-events.log # plugins/ca/tools/package-lock.json
Collaborator
Author
|
Replaces #512, which GitHub auto-closed when Merge resolution, for the record:
Re-verified after the merge, not just before it:
#509's |
…esolve Adversarial review of this PR returned BLOCK, and both blockers were #507's own defect class reproduced inside the fix for #507. That is the worst possible outcome for this change, so they are fixed here rather than deferred. BLOCK-1 - the guard never ran for the files it guards. GateCommandTest lives in test_ci_impact.py, which CI invokes only in the path-scoped `ci-impact` job. Neither `.codearbiter/tech-stack.md` nor `plugins/ca/skills/**` was in the `impact` filter, and tech-stack.md was in no push trigger either. So a PR whose only change was deleting the coverage section - precisely the regression the guard exists to stop - skipped the one job that runs it. It was green on its own introducing PR only because that PR also edited test_ci_impact.py, which is self-referential and proves nothing about any later change. This is the repo's own established rule, violated: #384 for the parity fixture, #403/#404 for gitleaks and docs.yml, #416 for the decisions directory. Both subjects are now registered, with a reachability test mirroring test_secret_scan_config_edits_reach_the_guard_that_constrains_them. BLOCK-2 - the guard checked the prose, not the command. It asserted a regex matched inside a markdown file and stopped there. Deleting the `coverage` script from two of the three package.json files left tech-stack.md naming a command that no longer existed, with every assertion still green. The guard now resolves `npm --prefix <dir> run <script>` and asserts the script exists in that manifest. The DEFINITIONS map is also checked in BOTH directions now. The first cut carried a `typecheck` entry matching no prose - asserting nothing while looking like protection - and omitted `test`, which is genuinely demanded. Running the bidirectional check immediately surfaced a third live demander neither I nor the map knew about: dependency-reviewer reads an `audit` command. A one-way map had no way to notice. Scope also widened from skills to skills AND agents. Also fixed, from the same review: - coverage-auditor carried a hardcoded copy of the threshold table. The copies diverged the moment the canonical one gained the lines-and-branches rule, leaving the agent the gate DISPATCHES applying a one-number test the gate no longer used. Handed ca-pi's real report at stage 3, the skill says BLOCK and the auditor's own instructions permitted PASS. The duplicate is deleted; it reads maturity-coverage.md. - refactor Phase 6 lacked the no-tooling clause Phase 2 gained, so a Python surface cleared Phase 2 under the documented exemption and then hit an unclearable Phase 6. Same skill, two answers about the same surface. - the documented ca/tools baseline did not reproduce. tech-stack.md said 67.22% lines; a clean checkout gives 67.31%. The gap is `plugins/ca/tools/.farm/`, dev-tree state absent from a fresh clone. Corrected to the clean-checkout figure, which is what CI and every other contributor measure. #511 updated. CHANGELOG: the coverage-gate guard now runs for the files it guards and verifies the command resolves; coverage-auditor reads the shared threshold table. Refs #507 Claude-Session: https://claude.ai/code/session_01JgdLb6n8mUdkFiKDTA37ML
Re-review returned PASS WITH CONCERNS. Both blockers were confirmed fixed by
mutation, but two MEDIUMs remained - and both were the same fail-open shape the
fix had just corrected, sitting one layer further down. Leaving those in the fix
for a fail-open gate is not a trade worth making, so they are closed here.
MEDIUM-9 - the resolution check scanned, then verified whatever it happened to
parse. Zero matches produced an empty finding list and a green test. Its regex
matched only `npm --prefix X run Y`, so two working invocations evaded it
entirely:
npm run coverage --prefix plugins/ca/tools
npm --prefix=plugins/ca/tools run coverage
Both were confirmed green by the reviewer with the `coverage` script deleted
from all three manifests. Fixed three ways: the parser now reads prefix and
script independently of argument order and accepts `--prefix=`; the scan asserts
it is non-vacuous; and a new completeness check derives the expected set from
the `plugins/*/tools` manifests rather than from the prose, so a tree cannot be
added - or documented in a phrasing the parser misses - without being caught.
MEDIUM-10 - the `test` entry proved only that a `## Test` heading was followed
by a non-empty fence. Replacing the body with a bare `:` deleted every real
invocation and stayed green.
The first attempt at that fix did not work, and my own mutation harness is what
caught it: `.*?` under DOTALL scans PAST the section, so the pattern matched a
later ```sh fence from `## Coverage` and reported the gutted section as fine. It
is now tempered with `(?:(?!^## )[\s\S])*?` so it cannot leave the section.
This is worth recording because the untempered version LOOKED strictly tighter
than what it replaced and was not - a green assertion agreeing with the defect,
which is the failure mode this whole issue is about.
The DEFINITIONS comment now records honestly that `coverage` is the only entry
backed by resolution and that `test` and `audit` are text patterns, rather than
claiming uniform strength the map does not have.
Mutation-verified, six cases plus a control:
baseline (unmutated) PASS
`npm run coverage --prefix X` + scripts deleted FIRES
`npm --prefix=X run coverage` + scripts deleted FIRES
no npm invocation at all (vacuity) FIRES
a tree's coverage script undocumented FIRES
`## Test` body gutted to a bare `:` FIRES
coverage section deleted entirely FIRES
The harness itself needed fixing first: its control was red because the temp
tree omitted `tools/ci-impact.py`, which test_ci_impact.py exec_module()s at
import. Every case reported a uniform meaningless red, which proves nothing -
a mutation test whose control fails is worthless.
LOW-11 - root CHANGELOG updated; the entry described only the original text
check and understated what the guard now does.
CHANGELOG: the gate-command guard resolves commands in any npm argument order,
refuses to pass on an empty scan, and derives its expected set from the
manifests.
Refs #507
Claude-Session: https://claude.ai/code/session_01JgdLb6n8mUdkFiKDTA37ML
Third adversarial pass. Two more MEDIUMs, both leaving a tree where the gate cannot run - #507's own definition - so both are closed rather than filed. MEDIUM-12 - the completeness check derived its obligation from the thing it was enforcing. It skipped any tree without a `coverage` script, so deleting the script deleted the requirement to document it. Two coordinated edits - drop the script, drop the doc line - and a tree leaves the coverage gate with every assertion green and no audit entry. The incentive gradient made that worse than the arithmetic suggests. `plugins/ca/tools` is the one tree below the floor, so a change there hits an unclearable BLOCK today. The ways out are backfill (#511), `/ca:override` (logged), or quietly drop the coverage command - and the guard closed the alternatives while leaving the silent door open, for the tree under the most pressure to use it. It is now keyed off `test`, which states the actual rule - a tree that runs tests owes a coverage command - and cannot be satisfied by deleting the subject. MEDIUM-13 - `site/` is a fourth tested TypeScript tree: real vitest suites under `site/test/`, a `test` script, and no coverage command. The completeness check globbed `plugins/*/tools`, so it structurally could not see it. My own commit message claimed "complete by construction ... a new tree cannot be added undocumented", which was true within `plugins/*/tools` and not true of the repo. The glob is now repo-wide. `site/` is held in an explicit `COVERAGE_EXEMPT` set citing #514, with a companion test asserting every exemption still names a real tested tree - a stale exemption fails rather than silently widening. The gap is visible and reviewed instead of structural. It is not closed; #514 owns that, along with `site/` still being on vitest 3. LOW-14 - the non-vacuity assert was global, satisfied by the unrelated `build` and `typecheck` invocations in the same file. Scoped to `coverage`, so it means what its message says. LOW-15 and the over-built question - the reviewer was asked whether any of this should be deleted rather than kept, and answered yes, in one place: the `test` entry had grown a 90-character regex with three false-failure modes (`python3`, a backtick inside a comment, a ```bash fence) to protect the one token that needs it least, since a missing test command is loud. All three would have fired on a legitimate edit, and a guard that cries wolf teaches people to weaken it. Replaced with a short structural check in Python, and the asymmetry is now a named design decision: `coverage` is resolution-backed, `test` and `audit` are structural. Mutation-verified, twelve cases including three that must NOT fire: baseline (unmutated) green drop ca/tools coverage script AND its doc line FIRES a tested tree undocumented and unexempt FIRES stale exemption (site/ removed) FIRES all coverage lines rewritten to `-C` form FIRES `npm run coverage --prefix X` + scripts deleted FIRES `## Test` gutted to a bare `:` FIRES `## Test` reduced to comments only FIRES coverage section deleted entirely FIRES python3 in the Test fence green a backtick inside a Test comment green fence tagged ```bash green CHANGELOG: the coverage completeness check is keyed off tested trees repo-wide, with site/ held as a named, asserted exemption rather than an invisible gap. Refs #507, #514 Claude-Session: https://claude.ai/code/session_01JgdLb6n8mUdkFiKDTA37ML
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.
Closes #507. Step 3 of the chain; stacked on #510 (base is
chore/vitest-4-ca-tools, so the diff stays clean — GitHub retargets tomainwhen #510 merges).The defect
tddPhase 5 andrefactorPhase 2/6 instruct "run the coverage command fromtech-stack.md", and both carry a hard rule forbidding guessing one. No coverage command existed — no provider in any of the three TypeScript trees, no script, andtech-stack.mdcontained the word "coverage" zero times.So every run reached the phase, found nothing to run, and passed through on a gap. A BLOCK gate that cannot execute is worse than an absent one — it reads as satisfied in every lane. Same defect class as #501 (five suites running on nothing) and #506 (an assertion that restated the bug).
refactorwas the worse casualty. Phase 2 is "Behavioral parity coverage proof" — the gate that justifies the entire lane by proving pre-existing tests can detect a behaviour change before production code is touched. It rested wholly on a command that did not exist, leaving the lane running on Phase 5's "zero pre-existing tests modified" alone.What lands
Tooling —
@vitest/coverage-v8@4.1.9in all three trees,npm run coveragein each, scope/provider/reporters in eachvitest.config.tsso the invocation is argument-free and platform-identical.ca/toolsgets its firstvitest.config.ts; coverage output is gitignored.The metric, named. The threshold table said only "minimum coverage". That omission was load-bearing, not cosmetic — a report gives four numbers that disagree:
plugins/ca/toolsplugins/ca-pi/toolsplugins/ca-sandbox/toolsca-pi is compliant at maturity 3 on lines and non-compliant on branches. "≥ 85%" with no column named is not a threshold anyone can be held to.
Now lines and branches, both binding. Lines catches code no test reaches — which is exactly what #504 turned out to be, a
catchwith zero executions inside a 750-test suite. Branches catches the untaken half of a condition a test does reach, which lines alone reports as covered. Statements duplicates lines; functions is noisiest at small counts.A guard against recurrence.
GateCommandTestasserts that any command a gate reads fromtech-stack.mdis actually defined there. Verified in both directions rather than assumed — run against the pre-fix file it fires, and it correctly identifies both demanders (tdd,refactor).Deliberate non-choices
CONTEXT.mdstage:againstmaturity-coverage.md. Copying the number into three package configs forks the source of truth, and the copies drift the first time the stage moves.ca/toolsis below the stage-2 floor, so wiring it into required CI would block every merge in the repo on an unrelated backfill. It is an orchestrator gate, which is what the skills already assume.stage:stays at 2. Raising to 3 today blocks all three trees — ca-pi fails branches at 78.73, ca-sandbox at 79.96. A gate nothing can pass gets overridden, and an overridden gate teaches readers it is noise.Versioning
The surface renders into three hosts, so this is a payload change for ca, ca-codex and ca-pi — not just a
tools/edit.Pi payload, version, changelog, and root metadata advanced together: 0.1.34 -> 0.1.35.v2.9.1/ca-codex-v0.3.0tag), and their gates permit a payload change on an unpublished version. Entry added under root## [Unreleased].Verification
npm run coveragegreen in all three trees; ca/tools still discovers all 324 tests after gaining a configunittest discoveroverplugins/ca/hooks/tests+check-plugin-refs.pysync-core.py --check,build-surface.py --check,build-host-packages.py --check— all in synctest_pi_package.py's exact workspace contract updated for the new script and dependencytsc --noEmitclean in all three treesbuild-surface --checkcaught me editing rendered files instead ofcore/surface/templates; the edits were moved to source and re-rendered.https://claude.ai/code/session_01JgdLb6n8mUdkFiKDTA37ML