Skip to content

fix: distinguish a mis-anchored finding from a fabricated one - #72

Merged
Codevena merged 14 commits into
masterfrom
fix/true-positive-hole
Aug 6, 2026
Merged

fix: distinguish a mis-anchored finding from a fabricated one#72
Codevena merged 14 commits into
masterfrom
fix/true-positive-hole

Conversation

@Codevena

@Codevena Codevena commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Closes the true-positive hole pilot-02 exposed. Design of record:
docs/superpowers/specs/2026-08-05-true-positive-hole-design.md.

The defect

pilot-02 turn 2 seeded a path traversal. The panel detected it twice; both detections ended at
INFO and the turn scored a miss. The one that mattered was killed before the critic ever ran:

F-002  path-traversal  line 67  conf 0.90  ->  fact_invalid: true
  [reviewgate fact-check] cited location src/store.ts:67 does not exist in the working tree
  (file has 27 lines) - almost certainly hallucinated; demoted to advisory.

src/store.ts is a brand-new 27-line file. But the finding carried the reviewer's own quote:

evidence_line: "  return readFileSync(`./templates/${name}`, 'utf8')"

which is verbatim line 26. The reviewer read real code and mis-numbered the line. It was
mis-anchored, not fabricated - and the gate told the agent the opposite about a real path
traversal. That one bad anchor caused all three failures: the fact_invalid demote, the missed
merge (25 vs 67 is past both merge windows), and hence singleton consensus, which left the critic
free to demote the second detection too.

What changes

Slice A - distinguish mis-anchored from fabricated. validateFindingFacts now consults the
reviewer's own evidence_line before demoting. A quote that matches a real line of the cited file
and carries an identifier-like token re-anchors the finding to that line instead of demoting it.
No quote, or a quote matching nothing, demotes exactly as before - the empty-file case the pass was
built for is untouched. Runs pre-aggregation, so the repaired line feeds clustering: turn 2's two
findings end up one line apart, merge, reach majority, and corroboration bars the critic. Costs
zero extra I/O (reuses the file the pass already read) and needs no orchestrator change.

Slice B - critic severity floor. The critic's security/correctness exemption was keyed to
CRITICAL, while the sibling delta-scope pass exempts the same categories at any severity. It now
covers WARN too - the one demote that crosses the blocking boundary. Already-INFO stays droppable,
so the critic keeps filtering where reviewers are noisiest.

Both always-on, no new config key. anchor_repaired is marker-observable so pilot-03 can count it.

Review

Five tasks, each implemented by a fresh agent and reviewed independently; then a whole-branch review.

The whole-branch review found a real security hole the per-task gates missed: a punctuation-only
quote
(}) was a valid repair key, turning a fabricated 0.97 CRITICAL from PASS into a hard FAIL.
Fixed by requiring an identifier token. It also rejected a premise in the spec ("quoting real source
proves the finding is mis-anchored" - } is real source); the risk table now says so honestly.

The guard width was then contested: this repo's own gate asked to lower the bound so a genuine line
whose longest token is 2 chars (if (a || b) {) is not wrongly demoted, while the reviewer argued
against. Measured over all 35 distinct evidence_line values recorded across both pilots, both
bounds reject 0
- and the reviewer's stated objection was factually wrong. Ruled for the gate, then
re-ran the full adversarial battery against the shipped code: 0 attack leaks, 0 true positives
lost
, empty-file protection intact.

Verification

  • bunx tsc --noEmit clean, bun run lint clean, full bun test 3184 pass / 12 skip / 0 fail.
  • Every new guard test seen red first via mutation in a throwaway copy.
  • The acceptance test reconstructs the real recorded incident; its 27-line fixture is byte-identical
    to the added lines of rig/results/pilot-02/turns/2/diff.patch.
  • Revert matrix: reverting Slice A's re-anchor, or the marker propagation, or the badge each turns the
    acceptance test red. Reverting Slice B does not - because corroboration alone bars the critic in
    that scenario. The test comment says so rather than claiming more than it proves; Slice B's own case
    is covered by a dedicated singleton test.

Not included

  • No bun run build - the rebuild re-pins the binary and belongs to the pilot-03 sequencing.
  • M5 critic-cost attribution stays out (provider-contract change).
  • Signature reordering stays out (would invalidate every persisted signature and cache entry);
    recorded as deliberate in the spec's out-of-scope section.

Codevena added 14 commits August 5, 2026 22:13
A punctuation-only evidence_line ("}", "  }", fullwidth "}") normalizes to
a string that matches many lines, so it could re-anchor and rescue a wholly
fabricated CRITICAL that the pre-branch code correctly demoted. Require the
quote to carry at least one identifier-like token before it counts as a
repair key; anything else falls through to the existing demote.

Also: correct the now-backwards evidence_line trust-status comments in the
finding schema and the review-output schema shown to the model, fix an
off-by-one in the acceptance test's simulated hunk range, reword the
tie-break doc comment on reanchorByEvidence (unreachable given the
range-checked call site — last-match and nearest-match are the same
occurrence), rename a label collision in report-writer.ts, and correct two
overclaims plus record one deliberate seam in the design doc.
…e quote

The identifier-token guard required a 3-character minimum ({2,}), which
rejected genuine 2-character quotes ("if", "do", "in", "of", "as" and other
2-char identifiers) as evidence for an anchor repair. Lower the bound to
{1,} (2-char minimum): punctuation-only quotes ("}", "});", fullwidth "}")
stay rejected at any bound since they contain no identifier character at
all, so this only fixes the false-negative on real code without reopening
the punctuation hole closed in the previous commit.

Also label f.evidence_line as UNTRUSTED reviewer-supplied input at both
reanchorByEvidence's doc comment and its call site in validateFindingFacts,
now that it decides blocking vs advisory rather than being purely
render-only.
@Codevena
Codevena merged commit 9019e1e into master Aug 6, 2026
3 checks passed
@Codevena
Codevena deleted the fix/true-positive-hole branch August 6, 2026 06:41
Codevena added a commit that referenced this pull request Aug 6, 2026
pilot-03 scores task (b) (PR #72, merged 9019e1e). Exactly ONE thing changes
against pilot-02: the compiled gate binary, sha256:7f92445b -> sha256:fc9b8c18.

Frozen BEFORE the run, as pilot-02 was. What this registration fixes about
pilot-02's:

- Every floor is a RATE. pilot-02's M3 floor was written as an absolute count
  carried over from pilot-01 while the denominator had become variable, which
  demanded 3/3 and was never the intent.
- The sharp claim is the -critic ablation delta (expected 0/3), paired with a
  registered DISCRIMINATOR: a 0 delta is evidence only if the critic actually
  proposed a demote a protection blocked. Otherwise it is trivially 0, and that
  reading is on record in advance.
- Slice B gets an exact firing signature: critic_verdict "keep" + WARN +
  security/correctness + singleton consensus is reachable through the new floor
  and nothing else.
- Slice A gets an OPPORTUNITY DENOMINATOR (anchor_repaired + fact_invalid), so
  a zero can be reported as "not exercised" rather than as a null result — the
  distinction the handoff asked for, made countable.
- The recall CEILING is registered: in pilot-02 only turn 2's miss involved a
  suppression layer; turn 7 recorded suppressed.critic 0. Catching turn 7 would
  be panel variance, not this fix.

Identity of the two runs is machine-checkable rather than asserted:
  diff <(jq -S 'del(.id)' rig/scripts/pilot-02.json) \
       <(jq -S 'del(.id)' rig/scripts/pilot-03.json)          -> empty
  diff <(sed -n '/^cat > reviewgate.config.ts/,/^CFG$/p' \
         rig/scripts/pilot-0{2,3}-sandbox-setup.sh)           -> empty
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant