Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 41 additions & 33 deletions .github/scripts/codex-verdict.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -61,25 +61,15 @@ const findings = [...text.matchAll(/\bregression\s*:\s*([^;]*)/g)]
// nothing. Counting it would block a clean PR.
.filter((d) => !NON_FINDING.test(d));

// THE TRAILER IS THE VERDICT. codex-review.yml's prompt requires the response
// to end with exactly `VERDICT: CLEAN` or `VERDICT: REGRESSION`, and that line
// is what decides here. Everything else in the response is prose for a human.
// The prompt asks the response to end with `VERDICT: CLEAN` or
// `VERDICT: REGRESSION`. When it complies, that line is used. It often does
// not — see the measured note on the state decision below.
//
// This exists because BOTH prose-reading rules fail, and each was tried:
//
// - Requiring a recognizable clean phrase false-fails clean reviews. Codex
// phrases "nothing found" freely: "no regressions found", "No actionable
// regressions were identified", and one live verdict that just described
// the diff approvingly with no negative-finding phrase at all. A gate
// that reddens clean PRs gets switched off.
// - Treating any prose without a `regression:` token as clean lets an
// off-format finding through — "The new fallback branch has no test."
// reads clean and, worse, restores the auto-merge bypass. (Codex review
// round 1 caught exactly this on the previous revision.)
//
// One required output line is far more reliable to produce than a three-shape
// contract, and non-compliance is now a single crisp signal instead of a
// judgement call about English.
// Requiring a recognizable CLEAN phrase instead was tried and is worse: Codex
// phrases "nothing found" freely ("no regressions found", "No actionable
// regressions were identified", and one verdict that just described the diff
// approvingly with no negative-finding phrase at all), so the rule reddened
// clean PRs.
// It must be the FINAL non-empty line, because that is what the prompt asks
// for. Accepting the marker anywhere would let trailing prose slip past it:
// "VERDICT: CLEAN" followed by "Also, the new branch has no test." would read
Expand Down Expand Up @@ -120,25 +110,43 @@ const trailer = trailerMatch ? trailerMatch[1].toLowerCase() : null;
// up empty.
const NO_VERDICT_SENTINEL = /codex produced no parseable verdict/;

// THE TRAILER IS A BONUS SIGNAL, NOT A REQUIREMENT — and that is a measured
// decision, not a preference. An earlier revision required it and failed the
// first real PR it ran on: topcoder1/domain-rank#82, 2026-07-28T04:57Z. The
// prompt asking for the trailer was verifiably in that run's log, and
// gpt-5.6-sol at reasoning=low answered "No regressions found in the
// requested coverage, state-mutation, or contract-drift axes." with no
// trailer. The gate logged `state=no_verdict; enforcing=true` and reddened a
// clean PR.
//
// So compliance cannot be assumed, and the failure signals are the ones that
// have actually proven reliable:
//
// - `regression:` findings. Three for three on real findings.
// - an explicit `VERDICT: REGRESSION` trailer, when the model does comply.
// - no output at all, which means the review did not happen.
//
// Residual risk, accepted and documented: a finding phrased outside BOTH
// forms is missed. That is a miss, and the review comment still puts it in
// front of a human. The alternative failure — blocking correct work — is
// what gets a gate switched off, and then nothing is reviewed at all.
let state;
if (trailer) {
// Authoritative. A response that lists findings and then claims CLEAN is
// still a regression — the findings are the evidence, the trailer is not
// allowed to retract them.
state = trailer === 'regression' || findings.length > 0 ? 'regression' : 'clean';
} else if (findings.length > 0) {
// No trailer, but it reported in the finding form. Believe the findings.
if (findings.length > 0 || trailer === 'regression') {
// Findings outrank a CLEAN trailer: the trailer may not retract its own
// evidence.
state = 'regression';
} else {
// No trailer and no findings: the response did not answer the contract, so
// there is nothing to vouch for. Fail closed. This also covers an empty or
// missing verdict file and the workflow's own no-parseable-verdict
// sentinel — a review that did not happen must never read as "clean".
} else if (text === '' || NO_VERDICT_SENTINEL.test(text)) {
// A review that produced nothing must never read as "nothing found".
state = 'no_verdict';
if (text !== '' && !NO_VERDICT_SENTINEL.test(text)) {
} else {
state = 'clean';
if (!trailer) {
// Not fatal, but worth surfacing: report-only callers make the fleet-wide
// compliance rate visible here, which is what would justify requiring the
// trailer later.
console.log(
'::warning::Codex answered without the required `VERDICT:` trailer. ' +
'Treating as no verdict.'
'::warning::Codex answered without the `VERDICT:` trailer the prompt ' +
'asks for. Treated as clean because it reported no findings.'
);
}
}
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/codex-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ on:
Codex findings are advisories that are correctly declined, so
blocking by default would be wrong.

The verdict is read from the `VERDICT: CLEAN` / `VERDICT:
REGRESSION` trailer the prompt requires, not from prose. When true,
a response carrying neither that trailer nor a `regression:` finding
also fails (fail closed): a review that did not answer the contract
is precisely the state this gate cannot vouch for.
When true, the job fails on evidence of a finding — a `regression:`
line, or the `VERDICT: REGRESSION` trailer the prompt asks for — and
on a review that produced no output at all. Clean prose passes even
without the trailer: the model frequently omits it, and requiring it
reddened a clean PR the first time this ran for real.

Opting in also withdraws the auto-merge bypass on a bad review:
claude-author-automerge.yml's Option B treats a SUCCESS conclusion
Expand Down
13 changes: 7 additions & 6 deletions selftest/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,13 @@ arbitrary helper scripts; that's a different kind of repo.
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 the `VERDICT:` trailer
the prompt requires is what decides — a response answering neither that
trailer nor the `regression:` form fails CLOSED. Both prose-reading rules
were tried first and both failed: requiring a recognizable clean phrase
false-failed a live clean review, and treating any prose without
`regression:` as clean let an off-format finding through.
`FAIL_ON_REGRESSION=true`, and under enforcement it fails on evidence of a
finding (a `regression:` line or a `VERDICT: REGRESSION` trailer) plus on no
output at all. Requiring the trailer was tried and failed the first real PR
it ran on (domain-rank#82): the model ignores the instruction and a clean
review went red. Requiring a recognizable CLEAN phrase failed the same way.
The accepted cost — asserted explicitly — is that a finding phrased outside
both forms is missed.
- `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.
Expand Down
78 changes: 39 additions & 39 deletions selftest/test_codex_verdict_gate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@
# 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 the `VERDICT:` trailer decides, and a response
# answering neither the trailer nor the `regression:` form fails
# CLOSED. Both prose-reading rules were tried first and both failed:
# requiring a recognizable clean phrase false-failed a live clean
# review, and treating any prose without `regression:` as clean let an
# off-format finding through. One required output line replaces a
# judgement call about English.
# 2. Under enforcement it fails on evidence of a FINDING — a `regression:`
# line or a `VERDICT: REGRESSION` trailer — plus on no output at all.
# Requiring the trailer was tried and failed the first real PR it ran
# on (domain-rank#82, below): the model ignores the instruction, and a
# clean review went red. Requiring a recognizable CLEAN phrase failed
# the same way. The accepted cost is that a finding phrased outside
# both forms is missed; the review comment still reaches a human.
#
# Run from the repo root:
# bash selftest/test_codex_verdict_gate.sh
Expand Down Expand Up @@ -74,9 +74,11 @@ check "domain-rank#74 verdict is report-only by default" "0 regression" "$(run f
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")"

# --- The trailer decides ----------------------------------------------------
# The prompt requires the response to END with `VERDICT: CLEAN` or
# `VERDICT: REGRESSION`. Prose above it is for humans.
# --- The trailer, when the model bothers to emit it --------------------------
# The prompt asks the response to END with `VERDICT: CLEAN` or
# `VERDICT: REGRESSION`, and it is read from the final non-empty line. It is
# used when present and simply absent when not (see domain-rank#82 below);
# only `VERDICT: REGRESSION` can fail a PR on its own.
check "CLEAN trailer passes" "0 clean" \
"$(run true 'Reviewed the diff.
VERDICT: CLEAN')"
Expand Down Expand Up @@ -111,35 +113,30 @@ VERDICT: CLEAN')"
check "only the final line decides" "1 regression" \
"$(run true 'The format asks me to answer VERDICT: CLEAN when nothing is wrong.
VERDICT: REGRESSION')"
# ...and prose AFTER the trailer means the contract was not followed. Reading
# the earlier marker would be the off-format-finding hole in a new costume.
check "prose after a CLEAN trailer fails closed" "1 no_verdict" \
# KNOWN MISS, accepted deliberately. Prose after a CLEAN trailer is not read,
# so a concern written there is not caught. Catching it means requiring the
# trailer to be last, which requires the trailer — and domain-rank#82 below
# shows that reddens clean PRs. The review comment still shows this to a human.
check "prose after a CLEAN trailer is accepted (known miss)" "0 clean" \
"$(run true 'VERDICT: CLEAN
Also, the new fallback branch has no test.')"
check "trailer with a trailing period passes" "0 clean" "$(run true 'ok
VERDICT: CLEAN.')"

# NOTHING below the trailer is forgiven, and that is the decision, not an
# oversight. Three attempts to whitelist trailing CLI telemetry were each
# shown to swallow a real finding written in the same shape, so the gate now
# trusts only the last line. codex-cli 0.145.0 prints nothing after the
# response (verified against real output), and if a future CLI adds a footer
# these cases go no_verdict — RED and diagnosable — rather than silently
# promoting an earlier CLEAN marker.
check "a footer below the trailer fails closed, not open" "1 no_verdict" \
# THE CASE THAT DECIDED THE DESIGN. Verbatim from topcoder1/domain-rank#82,
# 2026-07-28T04:57Z, the first real PR the gate ran on. The prompt asking for
# the trailer was in that run's log; gpt-5.6-sol answered without it, the gate
# scored no_verdict, and a clean PR went red. Requiring the trailer is
# therefore not viable — this must pass.
check "clean prose without the trailer passes (domain-rank#82)" "0 clean" \
"$(run true 'No regressions found in the requested coverage, state-mutation, or contract-drift axes.
No regressions found in the requested coverage, state-mutation, or contract-drift axes.')"
# Trailing CLI noise is harmless now that the trailer is not required to be
# last. codex-cli 0.145.0 emits none (verified against real output).
check "a footer below the trailer is harmless" "0 clean" \
"$(run true 'Reviewed.
VERDICT: CLEAN
tokens used: 12345')"
# The three real findings that killed each successive filter. All must fail.
check "prose below the trailer fails closed" "1 no_verdict" \
"$(run true 'VERDICT: CLEAN
The token refresh path has no test.')"
check "a finding opening with 'Usage:' fails closed" "1 no_verdict" \
"$(run true 'VERDICT: CLEAN
Usage: the token refresh path has no test.')"
check "a finding shaped like a token count fails closed" "1 no_verdict" \
"$(run true 'VERDICT: CLEAN
Tokens used: 1 token can authorize every tenant.')"
# Codex answering in the finding shape but reporting nothing must not block.
check "'regression: none' is clean, not a finding" "0 clean" \
"$(run true 'regression: none
Expand All @@ -150,19 +147,22 @@ check "semicolon-separated findings fail" "1 regression" \
"$(run true 'regression: a.py:1 - one; regression: b.py:2 - two
VERDICT: REGRESSION')"

# --- Fail closed when the contract is not answered --------------------------
# --- What still fails, and what is knowingly let through ---------------------
# Both live findings predate the trailer and carry none, so they also pin the
# fallback: a `regression:` line is believed even without a trailer.
# primary signal: a `regression:` line is believed with or without a trailer.
check "findings without a trailer still fail" "1 regression" "$(run true "$d79")"

# An off-format FINDING — no trailer, no `regression:` token. Reading this as
# clean is the hole Codex round 1 caught in the previous revision: it would
# have passed AND restored the auto-merge bypass.
check "off-format finding fails closed" "1 no_verdict" \
# THE ACCEPTED RESIDUAL RISK, stated out loud: a finding phrased in neither
# the `regression:` form nor a REGRESSION trailer is missed. Every rule that
# caught it also failed clean PRs, because the two are indistinguishable
# without reading English — "The new fallback branch has no test." and "Looks
# good to me." are both trailer-less prose. This gate optimizes for not
# blocking correct work, and leaves the miss to the human reading the comment.
check "off-format finding is missed (accepted risk)" "0 clean" \
"$(run true 'The new fallback branch has no test.')"
check "terse approval without a trailer fails closed" "1 no_verdict" \
check "terse approval without a trailer passes" "0 clean" \
"$(run true 'Looks good to me.')"
check "clean phrasing without a trailer fails closed" "1 no_verdict" \
check "clean phrasing without a trailer passes" "0 clean" \
"$(run true 'no regressions found')"

# The workflow writes this literal sentinel when awk extracts nothing.
Expand Down
Loading