review-gate.sh: review what the branch ships and pass only on a PASS verdict

Scripts/review-gate.sh

@@ -10,7 +10,8 @@ Run the pre-PR review gate for the current branch.
 Exit codes:
   0  Local review gate ran and passed.
   2  Remote review gate required; PR must wait for GitHub claude-review.
-  1  Local review gate ran and failed, or repository state is invalid.
+  1  Local review gate ran and failed (a CONFIRMED finding, a review that did
+     not complete, or one without a verdict), or repository state is invalid.
 
 Options:
   --require-local  Treat unavailable local review tooling as a failure.
@@ -53,8 +54,21 @@ if ! git rev-parse --verify origin/master >/dev/null 2>&1; then
     exit 1
 fi
 
-changed_summary=$(git diff --stat origin/master...HEAD)
-if [[ -z "$changed_summary" ]] && [[ -z "$(git diff --stat)" ]]; then
+# Review everything the branch would ship: commits since the merge base, plus
+# uncommitted tracked changes, plus untracked files. A diff of commits only
+# reviews nothing for a branch whose work is still in the working tree.
+base=$(git merge-base origin/master HEAD)
+# BSD mktemp replaces only trailing X's, so the template must end in them;
+# "….XXXXXX.patch" made one fixed filename and a second run collided.
+diff_file=$(mktemp "${TMPDIR:-/tmp}/keypath-review-diff.XXXXXX")
+output_file=""
+trap 'rm -f "$diff_file" ${output_file:+"$output_file"}' EXIT
+git diff --find-renames "$base" > "$diff_file"
+while IFS= read -r -d '' untracked; do
+    git diff --no-index -- /dev/null "$untracked" >> "$diff_file" || true
+done < <(git ls-files -z --others --exclude-standard)
+
+if [[ ! -s "$diff_file" ]]; then
     echo "review-gate: no branch or working-tree diff to review"
     exit 0
 fi
@@ -66,21 +80,36 @@ if command -v thermo-nuclear-swift-review >/dev/null 2>&1; then
 fi
 
 if command -v claude >/dev/null 2>&1; then
-    echo "review-gate: running claude review prompt against origin/master...HEAD"
-    diff_file=$(mktemp "${TMPDIR:-/tmp}/keypath-review-diff.XXXXXX.patch")
-    trap 'rm -f "$diff_file"' EXIT
-    git diff --find-renames origin/master...HEAD > "$diff_file"
-    claude <<EOF
-/thermo-nuclear-swift-review
+    echo "review-gate: running claude review of $(wc -l < "$diff_file" | tr -d ' ') diff lines against $base"
+    output_file=$(mktemp "${TMPDIR:-/tmp}/keypath-review-output.XXXXXX")
+    prompt='/thermo-nuclear-swift-review
 
-Review the KeyPath branch diff in this file:
-$diff_file
+Review the KeyPath branch diff given on standard input.
 
 Focus on bugs, behavioral regressions, concurrency/lifecycle hazards, installer
 postcondition violations, missing tests, and false-success paths. Classify each
 finding as CONFIRMED, PLAUSIBLE, or SPECULATIVE.
-EOF
-    exit $?
+
+End your reply with exactly one final line:
+REVIEW-VERDICT: FAIL   if any finding is CONFIRMED
+REVIEW-VERDICT: PASS   otherwise'
+    set +e
+    claude -p "$prompt" < "$diff_file" | tee "$output_file"
+    status=${PIPESTATUS[0]}
+    set -e
+    if [[ "$status" -ne 0 ]]; then
+        echo "review-gate: claude exited $status; the review did not complete" >&2
+        exit 1
+    fi
+    # The exit code says the CLI ran, not what the review found: gate on the verdict.
+    # `|| true`: under pipefail a grep that finds no verdict would otherwise end
+    # the script with status 1 and no message — a silent failure.
+    verdict=$(grep -E '^REVIEW-VERDICT: (PASS|FAIL)[[:space:]]*$' "$output_file" | tail -1 | awk '{print $2}' || true)
+    case "$verdict" in
+        PASS) echo "review-gate: review passed"; exit 0 ;;
+        FAIL) echo "review-gate: review found CONFIRMED issues" >&2; exit 1 ;;
+        *) echo "review-gate: the review gave no REVIEW-VERDICT line; cannot tell whether it passed" >&2; exit 1 ;;
+    esac
 fi
 
 message="review-gate: remote review gate selected; GitHub claude-review must pass before merge"

Tests/KeyPathTests/BuildScripts/ReviewGateScriptTests.swift

@@ -0,0 +1,128 @@
+import Foundation
+@preconcurrency import XCTest
+
+/// Scripts/review-gate.sh must review what the branch would ship and pass only
+/// on an explicit PASS verdict. Runs the real script in a scratch repository
+/// with a stub `claude` on PATH.
+final class ReviewGateScriptTests: XCTestCase {
+    private var repo: URL!
+    private var stubs: URL!
+
+    private static var repositoryRoot: URL {
+        var url = URL(fileURLWithPath: #filePath)
+        while url.path != "/", !FileManager.default.fileExists(atPath: url.appendingPathComponent("Package.swift").path) {
+            url.deleteLastPathComponent()
+        }
+        return url
+    }
+
+    @discardableResult
+    private func run(_ executable: String, _ arguments: [String], env extra: [String: String] = [:]) throws -> (code: Int32, out: String, err: String) {
+        let process = Process()
+        process.executableURL = URL(fileURLWithPath: executable)
+        process.arguments = arguments
+        process.currentDirectoryURL = repo
+        var env = ["PATH": "\(stubs.path):/usr/bin:/bin:/opt/homebrew/bin", "HOME": NSTemporaryDirectory(),
+                   "TMPDIR": NSTemporaryDirectory(), "GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@t",
+                   "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@t", "GIT_CONFIG_NOSYSTEM": "1"]
+        env.merge(extra) { $1 }
+        process.environment = env
+        let out = Pipe(), err = Pipe()
+        process.standardOutput = out
+        process.standardError = err
+        try process.run()
+        let outData = out.fileHandleForReading.readDataToEndOfFile()
+        let errData = err.fileHandleForReading.readDataToEndOfFile()
+        process.waitUntilExit()
+        return (process.terminationStatus, String(decoding: outData, as: UTF8.self), String(decoding: errData, as: UTF8.self))
+    }
+
+    private func git(_ args: String...) throws {
+        try run("/usr/bin/env", ["git"] + args)
+    }
+
+    private func gate(_ env: [String: String] = [:]) throws -> (code: Int32, out: String, err: String) {
+        try run("/bin/bash", ["Scripts/review-gate.sh"], env: env)
+    }
+
+    private var reviewed: String {
+        (try? String(contentsOf: stubs.appendingPathComponent("stdin.txt"), encoding: .utf8)) ?? ""
+    }
+
+    override func setUpWithError() throws {
+        let root = FileManager.default.temporaryDirectory.appendingPathComponent("review-gate-\(UUID().uuidString)")
+        repo = root.appendingPathComponent("repo")
+        stubs = root.appendingPathComponent("stubs")
+        try FileManager.default.createDirectory(at: repo.appendingPathComponent("Scripts"), withIntermediateDirectories: true)
+        try FileManager.default.createDirectory(at: stubs, withIntermediateDirectories: true)
+        try FileManager.default.copyItem(at: Self.repositoryRoot.appendingPathComponent("Scripts/review-gate.sh"),
+                                         to: repo.appendingPathComponent("Scripts/review-gate.sh"))
+        // The stub records what it was asked to review and answers as told.
+        let stub = stubs.appendingPathComponent("claude")
+        try """
+        #!/bin/bash
+        cat > "\(stubs.path)/stdin.txt"
+        echo "Findings: none worth noting."
+        [[ -n "${STUB_VERDICT:-}" ]] && echo "REVIEW-VERDICT: $STUB_VERDICT"
+        exit "${STUB_EXIT:-0}"
+        """.write(to: stub, atomically: true, encoding: .utf8)
+        try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: stub.path)
+        try git("init", "-q", "-b", "master")
+        try "base\n".write(to: repo.appendingPathComponent("README"), atomically: true, encoding: .utf8)
+        try git("add", "-A")
+        try git("commit", "-q", "-m", "base")
+        try git("update-ref", "refs/remotes/origin/master", "HEAD")
+        try git("checkout", "-q", "-b", "feature")
+    }
+
+    override func tearDownWithError() throws {
+        try? FileManager.default.removeItem(at: repo.deletingLastPathComponent())
+    }
+
+    private func commitChange() throws {
+        try "committed change\n".write(to: repo.appendingPathComponent("committed.txt"), atomically: true, encoding: .utf8)
+        try git("add", "-A")
+        try git("commit", "-q", "-m", "change")
+    }
+
+    func testPassesOnlyOnAnExplicitPassVerdictAndRunsTwice() throws {
+        try commitChange()
+        for _ in 0 ..< 2 { // the old fixed mktemp name collided on the second run
+            let result = try gate(["STUB_VERDICT": "PASS"])
+            XCTAssertEqual(result.code, 0, result.err)
+        }
+        XCTAssertTrue(reviewed.contains("committed change"), reviewed)
+    }
+
+    func testAConfirmedFindingFailsTheGate() throws {
+        try commitChange()
+        XCTAssertEqual(try gate(["STUB_VERDICT": "FAIL"]).code, 1)
+    }
+
+    func testAReviewWithoutAVerdictFailsTheGate() throws {
+        try commitChange()
+        let result = try gate()
+        XCTAssertEqual(result.code, 1)
+        XCTAssertTrue(result.err.contains("no REVIEW-VERDICT line"), result.err)
+    }
+
+    func testAReviewThatDidNotCompleteFailsTheGate() throws {
+        try commitChange()
+        XCTAssertEqual(try gate(["STUB_VERDICT": "PASS", "STUB_EXIT": "3"]).code, 1)
+    }
+
+    func testUncommittedAndUntrackedWorkIsReviewed() throws {
+        try "edited\n".write(to: repo.appendingPathComponent("README"), atomically: true, encoding: .utf8)
+        try "brand new\n".write(to: repo.appendingPathComponent("new.txt"), atomically: true, encoding: .utf8)
+        XCTAssertEqual(try gate(["STUB_VERDICT": "PASS"]).code, 0)
+        XCTAssertTrue(reviewed.contains("edited"), reviewed)
+        XCTAssertTrue(reviewed.contains("brand new"), reviewed)
+    }
+
+    func testNothingToReviewNeverAsksTheReviewer() throws {
+        let result = try gate(["STUB_VERDICT": "FAIL"])
+        XCTAssertEqual(result.code, 0)
+        XCTAssertTrue(result.out.contains("no branch or working-tree diff"), result.out)
+        XCTAssertEqual(reviewed, "")
+    }
+}

docs/process/agent-pr-workflow.md

@@ -30,7 +30,9 @@ End-to-end process for shepherding code from initial request through to a clean
 
 6b. **Run `./Scripts/review-gate.sh`** — before creating the PR, run the
 review gate against the branch diff. If local review tooling is available, the
-script runs it and returns 0 only when it passes. If local review tooling is
+script runs it over the branch's commits, uncommitted changes and untracked
+files, and returns 0 only when the review's `REVIEW-VERDICT` line says PASS
+(a CONFIRMED finding, a failed run or a missing verdict is exit 1). If local review tooling is
 unavailable, exit 2 is the expected remote-review path: record `remote review
 gate selected` in the PR and do not merge until the GitHub `claude-review`
 check passes. Do not report missing local review tools as a PR issue; that is