ci(codex-review): let callers fail the job on a bad Codex verdict

.github/scripts/codex-verdict.mjs

@@ -0,0 +1,121 @@
+// Codex verdict gate. Classifies the verdict text produced by
+// codex-review.yml and, when the caller opts in, FAILS the job on a
+// regression instead of merely commenting it.
+//
+// Why this exists: codex-review.yml has always posted Codex's verdict as a
+// PR comment and exited 0 regardless of what the verdict said (its only
+// `exit 1` is the 3000-file cost gate). A green check plus a comment nobody
+// re-reads is not a gate. On topcoder1/domain-rank#74 and #79 — both on
+// 2026-07-27 — Codex reported a missing-test regression on each PR and both
+// merged with the finding unaddressed.
+//
+// Opt-in by design: callers pass `fail_on_regression: true`. The default is
+// false, so the reusable's existing consumers keep today's advisory
+// behavior. Many Codex findings are advisories that get correctly declined,
+// so blocking-by-default would be wrong.
+//
+// Secondary effect worth knowing when you opt in: claude-author-automerge.yml
+// treats a SUCCESS conclusion on the configured `codex_check_name` as a
+// bypass of the risk-tier manual-merge gate (Option B). It requires an
+// explicit success, so a job that fails here also withdraws that bypass —
+// a regression can no longer read as "second-model reviewed".
+//
+// Inputs (from env):
+//   VERDICT_FILE        path to the extracted verdict (default /tmp/codex.verdict)
+//   FAIL_ON_REGRESSION  "true" to enforce; anything else reports only
+//   GITHUB_OUTPUT       (optional) — verdict_state=clean|regression|unparseable
+//   GITHUB_STEP_SUMMARY (optional) — append markdown summary
+//
+// Exit code: 1 only when FAIL_ON_REGRESSION=true AND the verdict is not
+// clean. Report-only mode always exits 0.
+
+import { readFileSync, existsSync, appendFileSync } from 'node:fs';
+
+const verdictFile = process.env.VERDICT_FILE || '/tmp/codex.verdict';
+const enforce = (process.env.FAIL_ON_REGRESSION || '').trim().toLowerCase() === 'true';
+
+// A missing file means the review step did not produce a verdict. Treat it
+// like an unparseable one (fail closed under enforcement) rather than
+// silently passing — "no verdict" is exactly the state we cannot vouch for.
+const raw = existsSync(verdictFile) ? readFileSync(verdictFile, 'utf8') : '';
+
+// Normalize for matching only; `raw` is still what gets reported. Markdown
+// emphasis is stripped because Codex sometimes bolds its verdict line, which
+// would otherwise break the `regression:` anchor.
+const text = raw
+  .replace(/[`*_]/g, ' ')
+  .replace(/\s+/g, ' ')
+  .trim()
+  .toLowerCase();
+
+// Codex is instructed to answer in one of three shapes:
+//   no regressions found
+//   regression: <file:line> - <one sentence>
+//   regression: <file:line> - <one>; regression: <file:line> - <two>
+// Multi-finding verdicts are semicolon-separated, so split findings on `;`.
+const NON_FINDING = /^(none|nothing|n\/a|na|no)\b/;
+const findings = [...text.matchAll(/\bregression\s*:\s*([^;]*)/g)]
+  .map((m) => m[1].trim())
+  .filter(Boolean)
+  // "regression: none" is Codex answering in the finding shape but reporting
+  // nothing. Counting it would block a clean PR.
+  .filter((d) => !NON_FINDING.test(d));
+
+// Codex frequently paraphrases the clean verdict rather than emitting the
+// canonical string — the pre-review of this very change answered "No
+// actionable regressions were identified", which an exact-match on
+// "no regressions found" would have failed under enforcement. Match the
+// negative-finding family instead: "no <adj>* <noun> <adv>* <verb>".
+// Widening this is safe in the direction that matters, because a verdict
+// using the `regression:` form is already decided above.
+const CLEAN =
+  /\bno\s+(?:\w+\s+){0,3}(?:regressions?|issues?|problems?|concerns?|findings?)\s+(?:\w+\s+){0,2}(?:found|identified|detected|observed|spotted|noted)\b/;
+
+// A hedge AFTER the clean phrase means Codex cleared the diff and then
+// qualified it ("no regressions found, but the new branch is untested").
+// That is not a clean verdict this gate can vouch for, and because the
+// qualifier skips the `regression:` form it would otherwise pass. Scoped to
+// the text following the clean phrase so an incidental "but" earlier in the
+// reasoning does not demote an otherwise clean answer.
+const HEDGE = /\b(?:but|however|although|though|except|caveat)\b/;
+
+// Order matters: a verdict that lists findings is a regression even if the
+// surrounding prose also contains a clean phrase somewhere.
+let state;
+const cleanMatch = text.match(CLEAN);
+if (findings.length > 0) {
+  state = 'regression';
+} else if (cleanMatch && !HEDGE.test(text.slice(cleanMatch.index + cleanMatch[0].length))) {
+  state = 'clean';
+} else {
+  state = 'unparseable';
+}
+
+const summary = {
+  clean: 'Codex reported no regressions.',
+  regression: `Codex reported ${findings.length} regression(s).`,
+  unparseable:
+    'Codex produced no verdict this gate could classify — treated as unresolved.'
+}[state];
+
+if (process.env.GITHUB_OUTPUT) {
+  appendFileSync(process.env.GITHUB_OUTPUT, `verdict_state=${state}\n`);
+}
+if (process.env.GITHUB_STEP_SUMMARY) {
+  appendFileSync(
+    process.env.GITHUB_STEP_SUMMARY,
+    `### Codex verdict gate\n\n- State: \`${state}\`\n- Enforcing: \`${enforce}\`\n- ${summary}\n`
+  );
+}
+
+console.log(`Verdict: state=${state}; enforcing=${enforce}; ${summary}`);
+for (const f of findings) console.log(`  - regression: ${f}`);
+
+if (enforce && state !== 'clean') {
+  const hint =
+    state === 'regression'
+      ? 'Address the finding, or reply on the PR explaining why it is declined and re-run this check.'
+      : 'Re-run the Codex review; if it keeps producing no parseable verdict, check the workflow logs.';
+  console.log(`::error::Codex review did not come back clean (${state}). ${hint}`);
+  process.exit(1);
+}

.github/workflows/codex-review.yml

@@ -15,6 +15,25 @@ name: Codex Review (reusable)
 
 on:
   workflow_call:
+    inputs:
+      fail_on_regression:
+        description: >-
+          Fail this job when Codex's verdict is not clean, instead of only
+          posting it as a comment. Default false, which is the historical
+          behavior every existing caller relies on. Opt in per repo — many
+          Codex findings are advisories that are correctly declined, so
+          blocking by default would be wrong. When true, an unparseable
+          verdict also fails (fail closed): "no verdict" is precisely the
+          state this gate cannot vouch for.
+
+          Opting in also withdraws the auto-merge bypass on a bad review:
+          claude-author-automerge.yml's Option B treats a SUCCESS conclusion
+          on `codex_check_name` as a substitute for the risk-tier manual-merge
+          gate, and requires an explicit success — so a failed job no longer
+          lets a regression read as "second-model reviewed".
+        required: false
+        type: boolean
+        default: false
     secrets:
       OPENAI_API_KEY:
         required: true
@@ -43,13 +62,18 @@ jobs:
       # in the caller's risk-paths.yml lists bug-attractor files that bypass
       # the gate (they ALWAYS get reviewed regardless of size). See
       # .github/scripts/codex-gate.mjs for the decision rules.
-      - name: Fetch cost-gate script
+      - name: Fetch gate scripts
         env:
           GH_TOKEN: ${{ github.token }}
         run: |
+          set -euo pipefail
           mkdir -p .github/scripts
-          gh api repos/topcoder1/ci-workflows/contents/.github/scripts/codex-gate.mjs \
-            --jq '.content' | base64 -d > .github/scripts/codex-gate.mjs
+          # Both scripts live in this reusable's repo, not the caller's
+          # checkout, so they have to be pulled at run time.
+          for s in codex-gate.mjs codex-verdict.mjs; do
+            gh api "repos/topcoder1/ci-workflows/contents/.github/scripts/$s" \
+              --jq '.content' | base64 -d > ".github/scripts/$s"
+          done
 
       - name: Install gate deps
         run: npm install --no-save yaml@2 minimatch@10
@@ -211,3 +235,18 @@ jobs:
             echo "<sub>Model: \`$(cat /tmp/codex.model 2>/dev/null || echo unknown)\` · CLI: \`$(cat /tmp/codex.cliversion 2>/dev/null || echo unknown)\` · reasoning: \`low\`</sub>"
           } > /tmp/comment.md
           gh pr comment "$PR" --body-file /tmp/comment.md
+
+      # Runs on every caller, enforcing only where `fail_on_regression: true`.
+      # Report-only mode is deliberate: it exercises the classifier against
+      # real verdicts fleet-wide and records the state in the step summary,
+      # so the false-positive rate is observable BEFORE any repo lets it
+      # block a merge.
+      #
+      # Ordered after the comment step so the finding is always visible on
+      # the PR even when this step then fails the job.
+      - name: Evaluate Codex verdict
+        if: steps.gate.outputs.should_run == 'true'
+        env:
+          VERDICT_FILE: /tmp/codex.verdict
+          FAIL_ON_REGRESSION: ${{ inputs.fail_on_regression }}
+        run: node .github/scripts/codex-verdict.mjs

selftest/README.md

@@ -30,6 +30,14 @@ arbitrary helper scripts; that's a different kind of repo.
   it would demote an unmatched path from the strict `standard` fallback into
   an auto-merge-eligible class (a PR adding `Tests/release.py` under
   `safe_test: ['tests/**']`).
+- `test_codex_verdict_gate.sh` — `codex-verdict.mjs` classification and its
+  opt-in enforcement. Codex's verdict used to be a comment only: the job
+  exited 0 whatever it said, so domain-rank#74 and #79 (both 2026-07-27) each
+  merged with a reported missing-test regression unaddressed. Replays both
+  verdicts verbatim. Pins the two properties that keep the gate safe for the
+  other 26 consumers — enforcement happens only on the literal
+  `FAIL_ON_REGRESSION=true`, and under enforcement an unparseable or missing
+  verdict fails CLOSED.
 - `test_pr_files_listing.sh` — no reusable may fetch changed files via
   `gh pr diff` (HTTP 406 past 20k diff lines); pins the paginated
   files-API idiom instead.

selftest/test_codex_verdict_gate.sh

@@ -0,0 +1,146 @@
+#!/usr/bin/env bash
+# Guards codex-verdict.mjs — the gate that decides whether a Codex review
+# verdict fails the job or merely gets commented.
+#
+# Background: codex-review.yml posted Codex's verdict as a PR comment and
+# exited 0 no matter what it said. On topcoder1/domain-rank#74 and #79 (both
+# 2026-07-27) Codex reported a missing-test regression on each PR; the check
+# went green, nobody re-read the comment, and both merged with the finding
+# unaddressed. The verdicts those two PRs actually produced are replayed
+# verbatim below, so this test pins the real-world case and not just a
+# synthetic one.
+#
+# Two properties matter and both are asserted:
+#   1. Enforcement is OPT-IN. Without FAIL_ON_REGRESSION=true the script
+#      must always exit 0 — 26 other repos consume this reusable and a
+#      default-on gate would start blocking their merges.
+#   2. Under enforcement it fails CLOSED: an unparseable or missing verdict
+#      is a failure, because "no verdict" is exactly the state the gate
+#      cannot vouch for.
+#
+# Run from the repo root:
+#   bash selftest/test_codex_verdict_gate.sh
+set -euo pipefail
+
+script="$PWD/.github/scripts/codex-verdict.mjs"
+tmp=$(mktemp -d)
+trap 'rm -rf "$tmp"' EXIT
+
+failed=0
+
+# Run the gate over a verdict. Usage: run <enforce> <verdict-text>
+# Echoes "<rc> <state>". Captures rc without tripping `set -e`.
+run() {
+  local enforce="$1" verdict="$2" rc state out
+  printf '%s' "$verdict" > "$tmp/verdict"
+  : > "$tmp/ghout"
+  set +e
+  out=$(VERDICT_FILE="$tmp/verdict" FAIL_ON_REGRESSION="$enforce" \
+    GITHUB_OUTPUT="$tmp/ghout" node "$script" 2>&1)
+  rc=$?
+  set -e
+  state=$(sed -n 's/^verdict_state=//p' "$tmp/ghout" | head -1)
+  printf '%s %s' "$rc" "${state:-<none>}"
+  # Surface the script's own output when a case fails.
+  printf '%s' "$out" > "$tmp/last_out"
+}
+
+check() {
+  local label="$1" want="$2" got="$3"
+  if [ "$got" = "$want" ]; then
+    echo "✓ $label"
+  else
+    echo "✗ $label — expected '$want', got '$got'"
+    sed 's/^/    /' "$tmp/last_out" 2>/dev/null || true
+    failed=1
+  fi
+}
+
+# --- The two verdicts that motivated this gate ------------------------------
+# Replayed verbatim (including the duplicated line the extractor produces).
+# shellcheck disable=SC2016  # the backticks are literal verdict text, not substitutions
+d74='regression: deploy/redeploy-code.sh:303 - No test forces either fallback `mv` failure and asserts that the new `failed=1` mutation produces an incomplete-restore status.
+regression: deploy/redeploy-code.sh:303 - No test forces either fallback `mv` failure and asserts that the new `failed=1` mutation produces an incomplete-restore status.'
+check "domain-rank#74 verdict fails under enforcement" "1 regression" "$(run true "$d74")"
+check "domain-rank#74 verdict is report-only by default" "0 regression" "$(run false "$d74")"
+
+d79='regression: deploy/redeploy-code.sh:171 - no test exercises the new failure path when the resolved compose config cannot be read from the box'
+check "domain-rank#79 verdict fails under enforcement" "1 regression" "$(run true "$d79")"
+
+# --- Clean verdicts must never fail ----------------------------------------
+# A false positive here blocks a good PR, so cover the spelling variants
+# Codex actually emits rather than only the canonical string.
+check "canonical clean verdict passes" "0 clean" "$(run true 'no regressions found')"
+check "clean verdict with capitals and period passes" "0 clean" "$(run true 'No regressions found.')"
+check "singular clean verdict passes" "0 clean" "$(run true 'no regression found')"
+check "clean verdict with 'were' passes" "0 clean" "$(run true 'No regressions were found')"
+check "bolded clean verdict passes" "0 clean" "$(run true '**no regressions found**')"
+# Codex paraphrases far more often than it emits the canonical string. This
+# is the verbatim verdict from the pre-review OF THIS CHANGE — an exact-match
+# gate called it unparseable and would have failed a clean PR.
+check "paraphrased clean verdict passes" "0 clean" \
+  "$(run true 'The opt-in verdict gate is correctly wired, fails closed under enforcement, and preserves advisory behavior by default. No actionable regressions were identified.')"
+check "'no issues detected' passes" "0 clean" "$(run true 'No issues detected.')"
+check "'no problems were observed' passes" "0 clean" "$(run true 'no problems were observed')"
+
+# A hedge after the clean phrase is not clean: Codex cleared the diff and
+# then qualified it, and the qualifier never uses the `regression:` form, so
+# nothing else would catch it.
+check "hedged clean verdict fails closed" "1 unparseable" \
+  "$(run true 'No regressions found, but the new fallback branch has no test.')"
+check "'however' hedge fails closed" "1 unparseable" \
+  "$(run true 'No issues found. However, the state mutation on line 40 is unasserted.')"
+# ...while a hedge BEFORE the clean phrase is just reasoning prose.
+check "hedge before the clean phrase stays clean" "0 clean" \
+  "$(run true 'I considered the error path, but it is covered. No regressions found.')"
+# Codex answering in the finding shape but reporting nothing must not block.
+check "'regression: none' is clean, not a finding" "0 clean" \
+  "$(run true 'no regressions found
+regression: none')"
+
+# --- Multi-finding format ---------------------------------------------------
+check "semicolon-separated findings fail" "1 regression" \
+  "$(run true 'regression: a.py:1 - one; regression: b.py:2 - two')"
+
+# --- Fail closed ------------------------------------------------------------
+# The workflow writes this literal sentinel when awk extracts nothing.
+check "workflow's no-verdict sentinel fails closed" "1 unparseable" \
+  "$(run true '(Codex produced no parseable verdict — see workflow logs)')"
+check "empty verdict fails closed" "1 unparseable" "$(run true '')"
+check "chatty off-format verdict fails closed" "1 unparseable" \
+  "$(run true 'I reviewed the diff and it looks fine to me overall.')"
+# ...but never in report-only mode.
+check "unparseable verdict is report-only by default" "0 unparseable" \
+  "$(run false '(Codex produced no parseable verdict — see workflow logs)')"
+
+# A missing verdict file is not the same as an empty one — it means the
+# review step never wrote one. Same fail-closed treatment.
+set +e
+VERDICT_FILE="$tmp/does-not-exist" FAIL_ON_REGRESSION=true node "$script" >/dev/null 2>&1
+rc=$?
+set -e
+check "missing verdict file fails closed" "1" "$rc"
+
+# --- Mixed verdict: findings win -------------------------------------------
+# If Codex hedges with both phrasings, the finding must decide. Otherwise a
+# stray "no regressions found" in the reasoning tail would clear a real one.
+check "clean phrase alongside a finding still fails" "1 regression" \
+  "$(run true 'no regressions found in the tests, but regression: src/x.py:9 - state mutation unasserted')"
+
+# --- Enforcement is strictly opt-in ----------------------------------------
+# Anything other than the exact string "true" must leave the gate advisory;
+# a caller that never sets the input gets an empty env var.
+for val in "" "false" "FALSE" "0" "yes" "1"; do
+  got="$(run "$val" "$d79")"
+  if [ "${got%% *}" = "0" ]; then
+    echo "✓ FAIL_ON_REGRESSION='$val' does not enforce"
+  else
+    echo "✗ FAIL_ON_REGRESSION='$val' enforced — only the literal 'true' may"
+    failed=1
+  fi
+done
+# ...and the boolean GitHub renders for `fail_on_regression: true` does.
+check "FAIL_ON_REGRESSION='true' enforces" "1 regression" "$(run "true" "$d79")"
+check "FAIL_ON_REGRESSION=' TRUE ' enforces" "1 regression" "$(run " TRUE " "$d79")"
+
+exit "$failed"