From 5154ff82ac63a9c372ccf1607ebd94cc8f6c4c21 Mon Sep 17 00:00:00 2001 From: Noah Lindner Date: Fri, 17 Jul 2026 22:35:27 -0400 Subject: [PATCH] sweep: accept a spoken noop token when codex skips the file write codex occasionally ends with STUPIFY_NO_NEW_ISSUES as its final message without writing the review file (seen on bevyl #7528/#7537/#7627); the run then read as FAILED and the head was throttled for an hour. Parse the final codex message (last 'codex' line to its 'tokens used' footer, never the diff-bearing transcript) and treat an exact token match as the noop it meant to write. --- src/review-sweep.test.ts | 25 ++++++++++++++++++++++++- src/review-sweep.ts | 24 +++++++++++++++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/review-sweep.test.ts b/src/review-sweep.test.ts index 1ff8ecd..ff046d3 100644 --- a/src/review-sweep.test.ts +++ b/src/review-sweep.test.ts @@ -4,7 +4,7 @@ // would thrash and this test would go red. We render against the repo's own real .review/ (no mocks). import { expect, test } from 'bun:test' import { join } from 'node:path' -import { type Config, commitStatusDescription, commitStatusForSweepResult, dismissedFindings, diffRightLines, FIXED_TOKEN, isFixedReview, isNoopReview, isRateLimited, NOOP_TOKEN, parseFindings, type Pr, pidAlive, priorReviewThread, reviewPrompt, stablePrefix, STILL_NOTE, stripSignoff } from './review-sweep' +import { type Config, commitStatusDescription, commitStatusForSweepResult, dismissedFindings, diffRightLines, finalCodexMessage, FIXED_TOKEN, isFixedReview, isNoopReview, isRateLimited, NOOP_TOKEN, parseFindings, type Pr, pidAlive, priorReviewThread, reviewPrompt, stablePrefix, STILL_NOTE, stripSignoff } from './review-sweep' const REVIEW_DIR = join(import.meta.dir, '..', '.review') // the real spec/rubric/corpus shipped in this repo const THIS_PR = '===== THIS PR' // the boundary between the cached prefix and the per-PR tail @@ -270,6 +270,29 @@ test('isRateLimited flags plan exhaustion, not ordinary failures', () => { expect(isRateLimited('the diff had no reviewable changes')).toBe(false) }) +// codex sometimes says a convergence token as its final message without writing the review file — the transcript's +// final message (last `codex` line to its `tokens used` footer) is the only part allowed to stand in for the file. +test('finalCodexMessage extracts the last codex message, not the transcript', () => { + const transcript = [ + 'exec', + "/bin/bash -lc \"printf 'STUPIFY_NO_NEW_ISSUES'\" in /home/exedev/.stupify/repo", + ' succeeded in 0ms:', + 'codex', + 'I compared the change against the corpus. Writing the token now.', + 'tokens used', + '12,345', + 'codex', + NOOP_TOKEN, + 'tokens used', + '50,953', + ].join('\n') + expect(finalCodexMessage(transcript)).toBe(NOOP_TOKEN) + // an inlined diff LINE containing the token is not a final message — it must sit between codex and tokens used + expect(finalCodexMessage(`+ const x = '${NOOP_TOKEN}'\ncodex\nreviewing…\ntokens used\n1`)).toBe('reviewing…') + expect(finalCodexMessage('no codex markers at all')).toBe('') + expect(finalCodexMessage(`codex\n${NOOP_TOKEN}`)).toBe('') // no tokens-used footer ⇒ truncated run, trust nothing +}) + test('commitStatusDescription fits GitHub commit status limits', () => { expect(commitStatusDescription('short and sweet')).toBe('short and sweet') const long = 'x'.repeat(200) diff --git a/src/review-sweep.ts b/src/review-sweep.ts index ffd250a..a3f4ab6 100755 --- a/src/review-sweep.ts +++ b/src/review-sweep.ts @@ -337,6 +337,21 @@ const stripWrap = (review: string): string => review.replace(/[`*\s]/g, '') // s export const isNoopReview = (review: string): boolean => stripWrap(review) === NOOP_TOKEN export const isFixedReview = (review: string): boolean => stripWrap(review) === FIXED_TOKEN +// codex sometimes SAYS a convergence token as its final message instead of writing it to the review file (observed +// on #7528/#7537/#7627 — the run then read as FAILED and the head got throttled for an hour). Recover the final +// message from the transcript: it's the text between the last bare `codex` line and its `tokens used` footer. +// Only that message — never the whole transcript, which inlines the untrusted diff — may stand in for the file. +export const finalCodexMessage = (out: string): string => { + const lines = out.split('\n') + const end = lines.findLastIndex((l) => /^tokens used\b/i.test(l.trim())) + const start = lines.slice(0, end === -1 ? lines.length : end).findLastIndex((l) => l.trim() === 'codex') + if (start === -1 || end === -1) return '' + return lines + .slice(start + 1, end) + .join('\n') + .trim() +} + // A hidden tag stamped in every inline finding comment, so a later sweep can find stupify's OWN review threads // (to resolve them) without knowing the bot login — `gh api user` 403s for GitHub-App integrations, so we identify // our content by marker, not author (same trick as the head marker). @@ -868,7 +883,14 @@ export function runReview(cfg: Config, pr: Pr, priorThread: string, diff: string const cx = exec('codex', codexArgs, { cwd: cfg.repoDir, timeoutMs: 1_200_000, input: reviewPrompt(cfg, pr, priorThread, diff, dismissed) }) appendFileSync(LOG, `${cx.combined}\n`) - const review = cx.ok && existsSync(outPath) ? readFileSync(outPath, 'utf8').trim() : '' + let review = cx.ok && existsSync(outPath) ? readFileSync(outPath, 'utf8').trim() : '' + // Flake recovery: codex declared convergence in its final message but skipped the file write. Accept a spoken + // token — exact match only, so a paraphrase still fails visibly — as the token it meant to write, instead of + // failing the run and throttling the head for an hour. + if (review.length === 0 && cx.ok) { + const spoken = finalCodexMessage(cx.combined) + if (isNoopReview(spoken) || isFixedReview(spoken)) review = spoken + } if (review.length === 0) { const reason = failureReason(cx.combined) return isRateLimited(cx.combined) ? { kind: 'limit', reason, raw: cx.combined } : { kind: 'fail', reason }