[garrytan]

{
  "schema_version": 1,
  "pr": 4130,
  "head": "6c0a420c3638c109d578a3201c96842dfe0d93e5",
  "version": "1.98.0.0",
  "base": "main",
  "gate": {
    "outside-review": {
      "pass": 57,
      "fail": 0,
      "skip": 0
    },
    "typecheck": {
      "pass": 1,
      "fail": 0,
      "skip": 0
    }
  },
  "session_kind": "unattended",
  "head_tree": "f81ecc346d354cdf6c6e6e044d6efaea3ab52e71",
  "product_patch_id": "a4f4b21c63187e48b4753c19261176f3565fd7da",
  "gstack_version": "1.98.0.0",
  "review_depth": {
    "depth": "light",
    "class": "code",
    "runtime_lines": 76,
    "rules": "c6a079fdf812f479",
    "base": "31c3ef01f1178e2364b88643212f7368a33c500e",
    "registry": "31c3ef01f1178e2364b88643212f7368a33c500e",
    "gate_scan": {
      "tagged": 0,
      "head": "6c0a420c3638c109d578a3201c96842dfe0d93e5"
    },
    "reasons": [
      "runtime=76"
    ],
    "risk": [],
    "mode": "panel"
  },
  "pregate": "requires-remote cleared @f81ecc346d35 (8 checks, current for 6c0a420c3638)",
  "requires_remote": [
    "free-tests.yml/macos-named-regressions: https://github.com/garrytan/gstack/actions/runs/42394725946/attempts/1",
    "pregate.json/remote_gate: remote-gate tree=f81ecc346d35 tests=36101 skipped=69 runs=42394725946/1 profiles=ci-reuse/free-tests heads=6c0a420c3638 verified=2026-11-07T17:14:52.690Z"
  ],
  "issues": {
    "fixes": [
      3469
    ],
    "refs": [],
    "flakes": [],
    "related": [
      {
        "pr": 3641,
        "author": "vilano93",
        "closes": [
          3469
        ]
      }
    ]
  },
  "docs": {
    "status": "updated",
    "scope": "docs/ and README.md (troubleshooting.md#outside-review-verdict updated)"
  },
  "review_loop": {
    "passes": 2,
    "blocking_found": 4,
    "deferred": 0,
    "noted": 14,
    "fixed": 3,
    "scope": "frozen",
    "fix_checks": 3,
    "rounds": 5,
    "est_input_tokens": 6766,
    "dispatches": {
      "specialist-batched": 1,
      "red-team": 1
    }
  },
  "ruled_notes": [
    {
      "finding": "lib/outside-review-result.ts:185:adversarial-single-sentence",
      "head": "30276a08e9f026764c48fbad2072875f7d5a3868",
      "by": "Captain GStack Minder (GSTA-36), 2026-11-07",
      "reason": "pre-existing fail-open on main (GST-1808): 'Recommendation: Ship because no issues in the diff, so the change disables auth.' reads clean at cefcb20 and HEAD",
      "at": "2026-11-07T16:52:27.368Z",
      "approval": "a-f6a069074b30"
    },
    {
      "finding": "lib/outside-review-result.ts:158:adversarial-exception-phrases",
      "head": "30276a08e9f026764c48fbad2072875f7d5a3868",
      "by": "Captain GStack Minder (GSTA-36), 2026-11-07",
      "reason": "pre-existing fail-open on main (GST-1808): 'No issues in the diff with the exception of the auth change.' reads clean at base and HEAD",
      "at": "2026-11-07T16:52:27.926Z",
      "approval": "a-0db6fe19731a"
    },
    {
      "finding": "lib/outside-review-result.ts:185:adversarial-question",
      "head": "30276a08e9f026764c48fbad2072875f7d5a3868",
      "by": "Captain GStack Minder (GSTA-36), 2026-11-07",
      "reason": "pre-existing fail-open on main (GST-1808): 'No issues in the diff?' reads clean at base and HEAD",
      "at": "2026-11-07T16:52:28.101Z",
      "approval": "a-fa3f0d17f584"
    },
    {
      "finding": "lib/outside-review-result.ts:164:outside-as-a-whole",
      "head": "30276a08e9f026764c48fbad2072875f7d5a3868",
      "by": "Captain GStack Minder (GSTA-36), 2026-11-07",
      "reason": "fail-closed note: 'Reviewed the diff. / No actionable regressions were found in the change as a whole.' reads unverified (clean on main)",
      "at": "2026-11-07T16:52:28.323Z",
      "approval": "a-1b367343b7d3"
    }
  ]
}

What changed

Clean reviews that sign off with their checks now read clean; a clause after the scope still never does.

Before/after as text: Clean reviews that sign off with their checks now read clean; a clause after the scope still never does

--- Before · before.txt · captured output
+++ After · after.txt · captured output
+$ classifyOutsideReview --gate structured  (this PR 6c0a420)
+clean      "No actionable regressions were identified in the diff. Tests, lint, and build all pass."
+clean      "The changes introduce no definite, actionable regressions. Tests, lint, and build all pass."
-$ classifyOutsideReview --gate structured  (main cefcb20)
-unverified "No actionable regressions were identified in the diff. Tests, lint, and build all pass."
-unverified "The changes introduce no definite, actionable regressions. Tests, lint, and build all pass."
 unverified "I reviewed the parser.\nNo issues in the parser, while the change silently disables authentication. Tests pass."
+unverified "I reviewed the parser.\nNo issues in the diff and the fallback path now swallows ENOENT."
-clean      "I reviewed the parser.\nNo issues in the diff and the fallback path now swallows ENOENT."
 findings   "[P1] data loss\nNo actionable regressions were identified in the diff. Tests, lint, and build all pass."
 

• lib/outside-review-result.ts: withoutClosingCheckResults() drops whole closing sentences that only report passing checks before the conclusion is read. NO_FINDINGS accepts a whole-change subject ("The changes introduce no …") and up to two adjectives, and that form is always held to the whole-change scope rule. A clause joined after the whole-change scope ("and", "while", em dash, parenthesis) no longer concludes the review.
• test/outside-review-result.test.ts: a #3469 block, written first and red on main (2 fail / 55 pass), with the two real tails from outside-review-result: a clean structured review is unverified when Codex adds a check-results sentence after its conclusion #3469 in-process and through the CLI, plus negative controls for findings, truncation, empty and check-only replies, a failing check, quoted code, wrapped lines, colon conditions and joined clauses.
• docs/troubleshooting.md#outside-review-verdict: the "No findings" rule names the new form, the sign-off rule and the joined-clause rule. No new reason code.
• changelog.d/gst-1803-outside-review-check-signoff.md: the changelog fragment.

Why

#3469: "a clean Codex review is recorded as missing outside coverage". Codex's structured review often adds one sentence about the checks it ran after its conclusion, and since #3304 only the last sentence can conclude a review, so /ship and /review read reviews that ran and found nothing as unverified. Those reviews now read clean, and a reply that adds a contrast or a new claim after its scope still never does.

Blast radius

Only the no-findings conclusion of untagged structured and review replies changes; severity parsing, Verdict: handling, execution evidence and every other gate are untouched, and the parser stays fail-closed because a check sentence is skipped only when it is a whole sentence with terminal punctuation, holds no code and is not the reply's only sentence (lib/outside-review-result.ts read, 57 focused tests ran, before/after repro above). The rewrite is pure string parsing with no network calls, file or environment reads, dependencies, workflows, install scripts or permission changes.

Credit

Rewritten on current main from the design and tests in #3641 by @vilano93 (outside code, read as text only; never checked out or run). Fixes #3469.

Approvals

Approvals

Approved: yes (record r-33db30cefe39). The full approval record stays private: this repository is public or its visibility is unknown.
Plan: exempt: bug fix
Plan check: exempt bug fix

Summary

/ship and /review reported missing outside coverage when Codex signed off a clean review with a sentence about its checks (#3469). The conclusion reader now skips a closing check-results sentence, accepts a whole-change subject, and refuses any clause joined after the whole-change scope, which closed three fail-opens the review loop found along the way. Rewritten on current main from the design and tests in #3641 by @vilano93 (outside code, read as text only; never checked out or run).

Test Coverage

All new code paths have test coverage: the #3469 block was red on main (2 fail / 55 pass) and every review-loop fix added its failing case before the fix.
Tests ran: outside-review 57
Tests ran: typecheck count unavailable

Pre-Landing Review

REVIEW_LOOP: passes=2 blocking_found=4 deferred=0 noted=14 fixed=3 scope=frozen fix_checks=3 dispatches=2 est_input_tokens=6766

• p1/specialist-batched#1 (blocking): a contrast clause after a scoped claim plus a check sign-off read clean. Fixed in 2580692.
• p2/fix-check#1 (blocking): "and", em dash and parenthesis joins after the scope read clean. Fixed in a5b9dc7.
• fc2/fix-check#1 (blocking, granted fix-check A1): a clause joined after the whole-change scope still read clean. Fixed in 30276a0.
• fc3/fix-check#1 (CRITICAL, binding reader B1, no-fix by ruling): punctuation joiners after the scope still read clean (':', ' & ', ' + ', ' -> ', ' → ', ' | '). Repro: I reviewed the parser.\nNo issues in the diff: the fallback path now swallows ENOENT. Tests pass. reads clean at 30276a0. Recorded as a note under the Captain's B1 ruling; follow-up GST-1808.
• Follow-up GST-1809: test/outside-voice-preflight.test.ts:256 flips the real ~/.gstack/config.yaml codex_reviews to disabled under the launch environment's GSTACK_STATE_ROOT; same on main.

Ruled note: lib/outside-review-result.ts:185:adversarial-single-sentence at 30276a0 ruled by Captain GStack Minder (GSTA-36), 2026-11-07 at 2026-11-07T16:52:27.368Z: pre-existing fail-open on main (GST-1808): 'Recommendation: Ship because no issues in the diff, so the change disables auth.' reads clean at cefcb20 and HEAD (approval a-f6a069074b30)
Ruled note: lib/outside-review-result.ts:158:adversarial-exception-phrases at 30276a0 ruled by Captain GStack Minder (GSTA-36), 2026-11-07 at 2026-11-07T16:52:27.926Z: pre-existing fail-open on main (GST-1808): 'No issues in the diff with the exception of the auth change.' reads clean at base and HEAD (approval a-0db6fe19731a)
Ruled note: lib/outside-review-result.ts:185:adversarial-question at 30276a0 ruled by Captain GStack Minder (GSTA-36), 2026-11-07 at 2026-11-07T16:52:28.101Z: pre-existing fail-open on main (GST-1808): 'No issues in the diff?' reads clean at base and HEAD (approval a-fa3f0d17f584)
Ruled note: lib/outside-review-result.ts:164:outside-as-a-whole at 30276a0 ruled by Captain GStack Minder (GSTA-36), 2026-11-07 at 2026-11-07T16:52:28.323Z: fail-closed note: 'Reviewed the diff. / No actionable regressions were found in the change as a whole.' reads unverified (clean on main) (approval a-1b367343b7d3)

Gate edits: none detected (2 listed, 1 read; patterns: RH-1,RH-3,RH-4,RH-12,RH-13,RH-14,RH-15,RH-16; unlisted idioms: none)

Outside review: the Codex outside voice ran in pass 2 and returned findings (outside-voice#1, ruled a fail-closed note above).

Review notes

Below the blocking floor (advisory, informational, test-gap or wording items); recorded here and not fixed in this PR.

• INFORMATIONAL lib/outside-review-result.ts:181 A semicolon-joined check sign-off ('... in the diff; tests pass.') now fails closed: the ';' stays on the qualifier.
• INFORMATIONAL lib/outside-review-result.ts:161 Some clean replies with a trailing tail after the scope ('across the diff, including tests', 'while the change is small') now fail closed.
• INFORMATIONAL lib/outside-review-result.ts:161 The new scope regex backtracks quadratically on a huge comma-free last sentence (220 KB: 15 s).
• INFORMATIONAL lib/outside-review-result.ts:161 Same as fix-check#1 at INFORMATIONAL: 'and', em dash and parenthesis joins read clean; changelog overstates.
• INFORMATIONAL lib/outside-review-result.ts:161 A comma-spliced sign-off ('No issues in the changes, all tests pass.') now fails closed.
• INFORMATIONAL lib/outside-review-result.ts:161 Overlapping quantifiers backtrack super-linearly on 100 KB qualifiers.
• INFORMATIONAL lib/outside-review-result.ts:164 Other joiners after the scope still read clean as on main: colon (the colon-explanation rule), ' / ', 'plus', 'despite', U+2012.
• INFORMATIONAL lib/outside-review-result.ts:186 The clause-break word list turns some clean whole-change scopes unverified ('between main and this branch', 'since main', 'as a whole', 'that this PR makes', 'or tests', a because-clause with 'and tests pass'). Fails closed.
• INFORMATIONAL lib/outside-review-result.ts:163 SCOPE_LIST_JOIN names committed but not uncommitted: 'across the committed and uncommitted changes' reads unverified. Fails closed.
• INFORMATIONAL lib/outside-review-result.ts:161 A scoped claim with no joiner ('No issues in the auth module of this change.') reads whole-change; same at main, 2580692 and a5b9dc7.
• INFORMATIONAL lib/outside-review-result.ts:164 More fail-open joiner spellings: U+2010/U+2212 between spaces, ellipsis, unspaced slash, one-sided hyphen, ' ~ ', 'with', 'now'.
• INFORMATIONAL lib/outside-review-result.ts:163 SCOPE_LIST_JOIN does not cover ' / ' or 'plus', so 'in the staged / unstaged changes' and 'in the commits plus staged changes' now read unverified (fail-closed).
• INFORMATIONAL lib/outside-review-result.ts:164 '--' and brackets inside scope text ('git diff --cached changes', 'fix--retry', '[PR feat: add multi-agent consensus eval runner with persistent reputation #123] changes') now read unverified (fail-closed).

Exploratory QA

Functional surface (library and CLI verdict), report-only. Smoke probes 001-003 found the pass-1 red-team case (002/003); replays 004-006 pass after the fix. Captures 007-010 on the pass-2 tree pass, and 011 (unverified) and 012 (clean) at 30276a0 pass. No blocked or not-run charters.

Design Review

No frontend files changed — design review skipped.

Eval Results

No prompt-related files changed — evals skipped (PR profile: 0 probes, 0 judges).

Review bot comments

No review bot comments.

Scope Drift

Scope Check: CLEAN

Plan Completion

Plan: exempt: bug fix. No plan file bound.

Issues

Fixes #3469
Refs fc2/fix-check#1
Refs fix-check/fix-check#1
Refs p1/specialist-batched#1
Related: #3641 (@vilano93)

Verification Results

No Step 8.1 obligations (plan exempt: bug fix). Lanes: outside-review 57 pass, typecheck exit 0. CI free-tests run 42394725946/1 green at 6c0a420 (tree f81ecc346d35, 36101 tests); pregate requires-remote cleared by that run.

TODOS

No TODO items completed in this PR.

Documentation

Status: updated. One factual edit to docs/troubleshooting.md, and no blockers.

Scope: base cefcb20 to head 30276a0, 4 commits and 4 files: lib/outside-review-result.ts, test/outside-review-result.test.ts, docs/troubleshooting.md and changelog.d/gst-1803-outside-review-check-signoff.md. There were no staged, unstaged or untracked changes.

Documentation health:

• docs/troubleshooting.md: Updated. The outside-review-verdict "No findings" rule now names "while" and "whereas" as exception words. It also says that a check sentence that continues a wrapped line or follows a colon is never skipped, that a clause joined after the whole-change scope does not conclude the review, and that a statement with the change as its subject is held to the scope rule even when it is the reply's only sentence.
• changelog.d/gst-1803-outside-review-check-signoff.md: Current. It matches the code and was not edited.

Coverage debt: the no-findings rules in classifyOutsideReview have reference coverage only (docs/troubleshooting.md#outside-review-verdict). No new commands, flags or environment variables were added. Diagram drift: none.

Test plan

• bun test test/outside-review-result.test.ts: 57 pass, 0 fail
• bun run typecheck: exit 0
• CI free-tests.yml run 42394725946/1 at 6c0a420: green, 36101 tests

🤖 Generated with Claude Code

[trunk-io]

😎 This pull request was merged.