Skip to main content
← Back to list
01Issue
BugShippedSwamp CLIPublic
Assigneesskunk-ape

Relationships

#2265 verification harness: code review reviews the wrong diff, and a missing VERDICT is recorded as a pass

Opened by skunk-ape · 9/17/2026· Shipped 9/18/2026

Two defects in the pre-PR verification harness compose into a single failure: the code review reviews the wrong code, and the harness records that review as an approval. Either alone is survivable; together they produce an attestation asserting a clean review of a change nothing examined.

1. The review prompt asks for a tool the step does not grant

verification/review-prompts/code-review.md:5-6 opens with:

First, run git diff main --name-only to identify the changed files. Only review those files -- do not review unchanged code.

The step invokes the reviewer as claude -p - --model claude-opus-4-6 --allowedTools "Read,Glob,Grep" (verification/workflow-verify-reviews.yaml:132-133). Bash is not granted, so the reviewer cannot run that command. Its first instruction is impossible, and it contradicts the footer the step appends at runtime ("The diff to review is at: "). The same instruction appears in the other review prompts.

Secondary problem in the same line: it says main, not origin/main. The reviews run in a detached worktree at the verified commit, where a local main ref is whatever that branch last pointed at -- stale in general. The step's own diff computation correctly uses git merge-base origin/main HEAD.

Observed effect: two consecutive runs on a branch whose diff was exactly one markdown file produced reviews of origin/main's newest commits instead.

  • Run 2c796e78-88e6-4b39-b702-9444aeae09c1 reported a pushDataChanges removal as a correctness regression. That symbol was introduced by #2496, which the branch's base predated, so it could not appear in the diff at all.
  • Run a7aed29b-6c7d-4a18-b2b3-faf4d808e7c4 opened "This change adds YAML workflow-run orphan reaping after rolling restarts/crashes across three code paths" -- that is #2498, already merged -- and folded the branch's actual skill-file change in as "Design docs and the issue-lifecycle skill are updated to match." Log: .swamp/data/command/shell/5338483a-9ee6-47ac-97bb-0e81cd115c69/log/1/raw

Ruled out as causes before filing:

  • The changed-file list is correct. All three src/-guarded reviews skipped, which only happens when the list contains no src/ paths.
  • The diff itself is correct. git merge-base origin/main HEAD + git diff in a fresh worktree at the same commit reproduces exactly one file by hand.
  • The reviewer can read the diff file. A canary test confirmed claude -p with --allowedTools "Read,Glob,Grep" reads an mktemp path outside its cwd.

So the diff handoff works; the instructions send the reviewer elsewhere.

2. A missing VERDICT marker is inferred as a pass

verification/workflow-verify-reviews.yaml:147-158: when the reviewer emits no VERDICT: line, the step infers fail only if the text matches [*]{0,2}(CRITICAL|HIGH|Blocking)[*]{0,2}[[:space:]]*: and otherwise infers pass.

Run 2c796e78 hit exactly that path. Its output opens "Two blocking findings reported. ... the pushDataChanges removal is a correctness regression that would cause data mutations to silently fail to sync to the remote datastore" -- prose that never matches the regex, because the word "blocking" appears in a sentence rather than as a Blocking: label. The step logged :⚠️:No VERDICT marker found; inferred pass (no blocking findings detected) and recorded a pass. The workflow reported success.

A survey of the local data store found exactly two logs carrying that warning: this one, and one from 2026-09-16 that genuinely had no findings. So the dangerous variant is live, not theoretical.

A reviewer that does not answer in the required format is precisely when a human should look. Suggest failing the step outright on a missing marker rather than inferring either way.

Why this matters

gate.allPassed is what the CI merge gate validates (.github/workflows/ci.yml:585), and nothing in CI re-runs these reviews -- ci.yml only checksums the prompts and workflows. The local run is the only place agent review happens, so a wrong review that records itself as a pass is the whole signal.

Same shape as the patchIssue swallow found separately in the issue-lifecycle extension: a missing or failed signal read as success. Worth auditing the trust chain for other instances.

Note on scope

Both files are inside the configIntegrity checksum set, so fixing them is a trust-root change that needs its own PR through the claude-review-integrity gate. Deliberately excluded from #2251 / #2500 for that reason.

02Bog Flow
✓OPEN✓TRIAGED✓IN PROGRESS✓SHIPPED+ 2 MORETRIAGE+ 5 MOREREVIEW+ 7 MOREPR_MERGED+ 2 MORESESSION_SUMMARIZED

Shipped

9/18/2026, 5:41:52 PM

Click a lifecycle step above to view its details.

03Sludge Pulse
skunk-ape assigned skunk-ape9/18/2026, 4:28:45 PM

Sign in to post a ripple.