fix(waiver): make an approved waiver actually suppress on every shipped path (GT-677) - #498
Merged
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>
|
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
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 GT-677.
evaluateDriftGateimplemented complete waiver suppression behind an optionalwaivers?: IWaiverStore, and all three production call sites omitted it, soevolith waiver approvewrote a durable, versioned, state-machine-correct record that no evaluation ever read.GT-518readsDONEfor the store, the CLI and the gate — both halves were built and never connected.Measured, before and after
evaluate --format drifton a temp workspaceblockingFailures 95 · frozen 0 · waiverRef undefined, no "Waived findings"94 · frozen 1 · waiverRef W-9, section rendersW-9@v1+ expiryblockingFailures 0 · frozen 95 · status passwaiver requestrun from a foreign cwd<workspace>/.evolith/waivers.jsonThe waiver was created through the shipped
waiver request/waiver approvecommands, run from/tmp, never by seeding a store file.The fix is larger than three call sites, and each addition was measured
--evidencenever touched the drift gate. Wiring only the gate would have left this row's own reproduction unchanged. One shared mechanism (freezeWaivedViolations) now serves the gate and the manifest, so they cannot disagree inside a run.process.cwd(); a reader anchored on the workspace reproduces the exact symptom while looking correct in review. Both now useresolveWaiverStorePathon the workspace.waiver request --fingerprintrequires, so the flow was unusable by hand even once wired. Citations carry[fp <hash>]; the blocked comment prints the waive command.codeownerswhile its comment claimed owner enrichment; one shared loader serves both surfaces.A defect in this fix, found by adversarially verifying it
--waivers <relative-path>resolved against the current directory, found nothing, and a missing file was read as an empty store:cd /private/tmp && evaluate --workspace $W --waivers .evolith/waivers.json→blocking 95 frozen 0— suppressing nothing while reporting success, GT-677's own silent no-op one level down. Readers now useopenWaiverStoreForRead; the same command exits 1 naming both the requested and the resolved path. The default path stays exempt: a workspace with no waivers is the normal state.The negatives are the deliverable
The green path already passed while the product suppressed nothing, so a green-path suite proves nothing here. Requested-but-unapproved, rejected, expired and fingerprint-mismatch each assert non-suppression, and each was proven load-bearing:
isWaiverActive → return trueturns the four negatives red; reverted.waivers:from either CLI call site, the evidence emitter, or the MCP tool turns the round-trip, contract and MCP suites red; each reverted.new FileWaiverStore(...)at the MCP call site (2 failed, 1474 passed) and green again once restored.What is NOT claimed
tools/call evolith-evaluatereturns the same payload with and without an approved waiver. Verified with two full stdio runs. The parity test measures the deposit, which is where the effect exists.--format json|text|sarifexit codes are unchanged; only--format driftmoves its exit code.waiverRefon the manifest is stamped only when exactly one waiver applied;frozenis the per-violation truth.rejected/requestedversion does not revoke an earlier approved one, andrejectedis not a state any shipped surface can write.Suites and guards
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 — all green.
08-validate-tracking(685 gaps, 637 closure records) ·04·01(1505 files) ·41·42(78 guards) ·43(54 observed failing) ·46current and at a fixed point ·47·54.🤖 Generated with Claude Code