Promote to main: GT-677 — an approved waiver now suppresses on every shipped path - #499
Merged
Conversation
…ed path GT-677. `evaluateDriftGate` implemented complete waiver suppression behind an optional `waivers?: IWaiverStore`, and all three production call sites omitted it, so `evolith waiver approve` wrote a durable, versioned, state-machine-correct record that no evaluation ever read. Measured before, on a temp workspace: `blockingFailures 95 · frozen 0 · waiverRef undefined`, no "Waived findings" section, and `waiver request` run from a foreign cwd wrote the store next to the CALLER instead of next to the workspace. After: `95 -> 94 · frozen 1 · waiverRef W-9`, the section renders `W-9@v1` with its expiry, and a workspace whose blocking findings are all waived exits 0. The fix is larger than the three call sites, and each addition was measured rather than assumed: - `--evidence` never touched the drift gate, so wiring only the gate would have left the board's own reproduction unchanged. `emitEvaluationEvidence` now takes the same store through one shared mechanism, `freezeWaivedViolations`, so the PR comment and the manifest cannot disagree inside one run. - The writer anchored on `process.cwd()` and a reader anchored on the workspace reproduces the exact symptom while looking correct in review. Both now resolve through `resolveWaiverStorePath`, anchored on the workspace. - Nothing printed the fingerprint `waiver request --fingerprint` requires, so the flow was unusable by hand even once wired. Citations now carry `[fp <hash>]` and the blocked comment prints the waive command. - A stale or mistyped fingerprint was a silent no-op -- the very failure this row records. Active waivers matching no finding are now reported as unmatched. - MCP passed no `codeowners` while its own comment claimed owner enrichment; one shared loader now serves both surfaces. A defect found by adversarial verification OF THIS FIX is closed here too: an explicit `--waivers <relative-path>` resolved against the current directory, found nothing, and `FileWaiverStore` read a missing file as an EMPTY store -- `blocking 95 frozen 0`, suppressing nothing while reporting success. Readers now use `openWaiverStoreForRead`, which fails loudly and names both the requested and the resolved path; the DEFAULT path stays exempt, because a workspace with no waivers is the normal state, not a mistake. Writers keep the creating opener. Tests are the negatives, because the green path already passed while the product suppressed nothing: requested-but-unapproved, rejected, expired and fingerprint-mismatch each assert non-suppression, and mutating `isWaiverActive` to `return true` turns them red. Removing `waivers:` from either CLI call site, from the evidence emitter or from the MCP tool turns the round-trip, contract and MCP suites red. The doc-claim test was observed red against an ad hoc `new FileWaiverStore(...)` at the MCP call site and green again once restored. Suites: core-domain 157/1861, infra-providers 19/167, cli 105/1476, cli e2e 18/133, mcp 63/570, contracts 6/113, test:contract 3/47 -- all green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…suppression # Conflicts: # reference/core/control-center/gaps/gap-tracking.es.md # reference/core/control-center/gaps/gap-tracking.md
… after GT-677 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fix(waiver): make an approved waiver actually suppress on every shipped path (GT-677)
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📊 Bilingual Coverage ImpactPR Changes
Repository Coverage
To create skeletons: node .harness/scripts/generate-es-skeleton.mjs <file.md>Generated by GitHub Actions |
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.
Promotion of
develop→main. Carries the GT-677 fix (product code in four packages) and its board closure.What changes in behaviour
evaluateDriftGateimplemented complete waiver suppression behind an optionalwaivers?: IWaiverStoreand all three production call sites omitted it, soevolith waiver approvewrote a durable record no evaluation ever read. After this promotion:evaluate --format driftwith an approved waiverblockingFailures 95 · frozen 0 · waiverRef undefined94 · frozen 1 · waiverRef W-9, "Waived findings" rendersW-9@v1+ expirywaiver requestfrom a foreign cwd<workspace>/.evolith/waivers.jsonTwo contract changes worth knowing before this reaches
main:evolith waiver approve, and every suppression is named in the PR comment with itswaiverRef, version and expiry.--waivers <relative-path>that does not exist is now a hard error (exit 1) instead of silence. Any script that was passing a wrong path was silently getting an empty store; it will now fail loudly and name both the requested and the resolved path.Why the fix is larger than three call sites
--evidencenever touched the drift gate, so wiring only the gate would have left the row's own reproduction unchanged; one shared mechanism now serves both. The writer anchored onprocess.cwd()while a reader anchored on the workspace — the exact symptom, reintroduced, while looking correct in review. Nothing printed the fingerprintwaiver request --fingerprintneeds. A mistyped fingerprint was a silent no-op. MCP passed nocodeownerswhile its comment claimed owner enrichment.A defect in the fix itself, found by adversarially verifying it, is closed here too: a relative
--waiverspath resolved against the current directory and a missing file was read as an empty store —blocking 95 frozen 0, suppressing nothing while reporting success.Evidence
The negatives are the deliverable, because the green path already passed while the product suppressed nothing.
isWaiverActive → return trueturns the four negatives red; removingwaivers:from either CLI call site, the evidence emitter or the MCP tool turns the round-trip, contract and MCP suites red; the doc-claim test was observed red against an ad hoc store and green once restored. Each mutation was reverted.Not claimed: MCP suppression is real but not observable in the tool response (it reaches the Tracker deposit);
--format json|text|sarifexit codes are unchanged; lint is unchanged, not clean.Verification
All 33 checks on #498 passed (3 skipped). Suites: core-domain 157/1861 · infra-providers 19/167 · cli 105/1476 · cli e2e 18/133 · mcp-server 63/570 · contracts 6/113 · test:contract 3/47. Guards:
08(685 gaps, 637 closure records) ·04·01·41·42·43·46at a fixed point ·47·54.Board: GT-677 →
DONE, counters 655 / 685 done · 2 in progress · 18 pending · 10 deferred.🤖 Generated with Claude Code