internal/review at de0d9860ee, reviewed by codex, verified by gemini-3.6-flash.
OutcomeOf records a REPRODUCED finding as held by execution without requiring an exit code, so an unexecuted or incomplete ledger entry can receive a successful outcome and reviewer credit.
#!/bin/sh
set -eu
p=internal/review/zz_repro_unexecuted_outcome_test.go
trap 'rm -f "$p"' EXIT HUP INT TERM
cat >"$p" <<'EOF'
// SPDX-License-Identifier: Elastic-2.0
package review
import (
"fmt"
"testing"
)
func TestReproUnexecutedFindingGetsExecutionOutcome(t *testing.T) {
f := Finding{Declared: TierReproduced, Tier: TierReproduced}
o := OutcomeOf(f, nil)
if !o.Known || !o.Held || o.By != "execution" {
t.Fatalf("defect not demonstrated: outcome=%+v", o)
}
fmt.Printf("wrong outcome with nil ExitCode: Known=%v Held=%v By=%q ExitCode=%v\n", o.Known, o.Held, o.By, f.ExitCode)
}
EOF
go test -v ./internal/review -run '^TestReproUnexecutedFindingGetsExecutionOutcome$'=== RUN TestReproUnexecutedFindingGetsExecutionOutcome wrong outcome with nil ExitCode: Known=true Held=true By="execution" ExitCode=<nil> --- PASS: TestReproUnexecutedFindingGetsExecutionOutcome (0.00s) PASS ok github.com/pdbethke/corralai/internal/review 0.003s
Tried to refute R1 by checking whether OutcomeOf requires f.ExitCode != nil or *f.ExitCode == 0 for findings declared REPRODUCED. In outcome.go:47, OutcomeOf only checks f.Declared == TierReproduced && f.Unrun == '' and returns Held: true if f.Tier == TierReproduced. In contrast, line 41 explicitly checks x.ExitCode != nil && *x.ExitCode == 0 for refutations. Consequently, an unexecuted or incomplete finding with nil ExitCode is incorrectly credited as held by execution.
confirmed by pdbethke — real, reproduced: OutcomeOf credits a REPRODUCED finding as held without requiring an exit code on the record
ref a906b2676dca290667704441f4e9f832c2e370d91f8847fda12dffbaed3ce5b7#R1
Parse accepts an object containing only an opinion key as the review payload, so an incidental JSON literal before the real payload silently replaces the opinion and drops all actual findings and sound evidence.
#!/bin/sh
set -eu
p=internal/review/zz_repro_decoy_json_test.go
trap 'rm -f "$p"' EXIT HUP INT TERM
cat >"$p" <<'EOF'
// SPDX-License-Identifier: Elastic-2.0
package review
import (
"fmt"
"testing"
)
func TestReproOpinionLiteralDisplacesReview(t *testing.T) {
text := `I considered {"opinion":"temporary note"} before returning the result.
{"opinion":"actual review","findings":[{"claim":"gate is bypassed","tier":"CODE-READ","file":"x.go","line":7,"severity":"high","script":""}],"sound":["parser","walker","grader"]}`
opinion, findings, sound, err := Parse(text)
if err != nil {
t.Fatalf("defect not demonstrated: %v", err)
}
if opinion != "temporary note" || len(findings) != 0 || len(sound) != 0 {
t.Fatalf("defect not demonstrated: opinion=%q findings=%d sound=%d", opinion, len(findings), len(sound))
}
fmt.Printf("wrong payload selected: opinion=%q findings=%d sound=%d\n", opinion, len(findings), len(sound))
}
EOF
go test -v ./internal/review -run '^TestReproOpinionLiteralDisplacesReview$'=== RUN TestReproOpinionLiteralDisplacesReview wrong payload selected: opinion="temporary note" findings=0 sound=0 --- PASS: TestReproOpinionLiteralDisplacesReview (0.00s) PASS ok github.com/pdbethke/corralai/internal/review 0.003s
Tried to refute R2 by checking extractJSON and hasShape in internal/review/review.go. Parse calls extractJSON with keys 'opinion', 'findings', and 'sound'. hasShape returns true if any single key is present in the object. When text contains a JSON literal like {"opinion":"..."} before the full review object, extractJSON selects the first object because it contains the 'opinion' key, causing Parse to discard all findings and sound evidence in the subsequent real payload.
confirmed by pdbethke — real, reproduced: an object with only an opinion key passes the review-shape check; a stray literal replaces the payload
ref a906b2676dca290667704441f4e9f832c2e370d91f8847fda12dffbaed3ce5b7#R2
Recorded so that "found nothing here" can be told apart from "never looked here".
The review record can claim execution that never occurred (finding 1), violating the package's central evidence invariant and corrupting model grading. Its response parser can also mistake an incidental opinion-shaped JSON literal for the actual review and silently discard findings (finding 2).
Both findings R1 and R2 stand. R1 correctly identifies that OutcomeOf checks f.Declared == TierReproduced and f.Unrun == "" without verifying f.ExitCode != nil, treating unexecuted findings as held by execution. R2 accurately demonstrates that extractJSON accepts an object with only an opinion key, allowing a preceding JSON literal in prose to displace the actual review payload and drop findings.