I1: a Codex review eval needs a completed command, not just review-shaped text

Run 37158847998 recorded codex-review-findings as passed again: the agent
wrote 'I couldn't run the review because the execution sandbox failed
(bwrap: Failed to make / slave)', the failure never reached stderr or a
command event, and no phrase rule matched. validateCodexReview now also
requires at least one command_execution event that completed with exit 0.
Fixtures gain that event; the CI output is a new negative control.
This commit is contained in:
garrytan committed 2026-10-03 23:02:10 +00:00
1 parent 7ae7776e50
commit dd9e4e75c9
3 files changed
+25 -2

No files matched your search

+8 -1
View File
@@ -19,10 +19,12 @@ import { CODEX_DRAIN_GRACE_MS, CodexHarnessError, type CodexResult } from './hel
import { EvalCollector, findPreviousRun, listEvalJsonFiles, isFinalizedEvalResultFile, type EvalTestEntry } from './helpers/eval-store';
import { isPaidTestFile } from './helpers/paid-test-set';
const gitDiffRan = JSON.stringify({ type: 'item.completed', item: { id: 'item_1', type: 'command_execution',
command: "/bin/bash -lc 'git diff'", aggregated_output: '', exit_code: 0, status: 'completed' } });
const result = (overrides: Partial<CodexResult> = {}): CodexResult => ({
output: 'The gstack review found no issues in the current branch diff.',
reasoning: [], toolCalls: ['git diff'], tokens: 31, exitCode: 0,
durationMs: 25, sessionId: 'fixture', rawLines: [], stderr: '', ...overrides,
durationMs: 25, sessionId: 'fixture', rawLines: [gitDiffRan], stderr: '', ...overrides,
});
const prose = 'This explanation describes what the choice means for the project. '.repeat(8);
const kindQuestion = `${prose}\nRECOMMENDATION: Choose A\nThese options differ in kind.`;
@@ -54,6 +56,11 @@ describe('Codex assertion and record parity', () => {
['skipped skill', { stderr: 'Skipped loading gstack-review' }, validateCodexDiscovery, 'validation_failed'],
['missing skill reference', { output: 'Nothing available.' }, validateCodexDiscovery, 'validation_failed'],
['tool error saying invalid is not a skill problem', { stderr: 'ERROR codex_core::tools::router: error=apply_patch verification failed: invalid hunk at line 4' }, validateCodexDiscovery, 'success'],
['review whose tool errors never became command events', {
// Run 37158847998: "bwrap: Failed to make / slave" reached only the agent text; stderr and events were clean.
output: "I couldn't run the review because the execution sandbox failed before any command could run:\n\n```text\nbwrap: Failed to make / slave: Permission denied\n```\n\nNo files were changed, and I have no review findings to report.",
rawLines: [], stderr: 'Reading additional input from stdin...\n',
}, validateCodexReview, 'validation_failed'],
['review whose sandbox never started', {
// Periodic run 37151477069 recorded this as a passing review; its only command failed in bwrap.
output: "I couldn't run the review because the sandbox failed before executing the command to read the gstack-review skill:\n\n> `bwrap: No permissions to create a new namespace`\n\nNo repository files or branch diff were inspected, so I have no findings to report.",
+3
View File
@@ -309,6 +309,9 @@ mock.module('child_process', () => ({
current.home = options.env.HOME;
queueMicrotask(() => {
const { child, scenario } = current;
child.stdout.write(JSON.stringify({ type: 'item.completed', item: {
type: 'command_execution', command: 'git diff', aggregated_output: '', exit_code: 0, status: 'completed',
} }) + '\\n');
child.stdout.write(JSON.stringify({ type: 'item.completed', item: {
type: 'agent_message', text: 'The gstack review found no issues in the current branch diff.',
} }) + '\\n');
+14 -1
View File
@@ -43,12 +43,25 @@ export function validateCodexDiscovery(result: CodexResult): void {
requireCondition(/review|gstack|skill/.test(result.output.toLowerCase()), 'Codex discovery did not reference the skill');
}
/** Commands Codex reported as completed with exit 0 in its `exec --json` events. */
function succeededCommands(rawLines: string[]): number {
return rawLines.filter(line => {
try {
const event = JSON.parse(line);
return event?.type === 'item.completed' && event.item?.type === 'command_execution'
&& event.item.status === 'completed' && event.item.exit_code === 0;
} catch { return false; }
}).length;
}
export function validateCodexReview(result: CodexResult): void {
// A review whose commands never ran (sandbox could not start) is not a review.
// A review whose commands never ran (sandbox could not start) is not a review:
// it must show a completed command, and nothing may classify it unavailable.
const ran = classifyOutsideReview({ text: result.output, gate: 'execution', stderr: result.stderr,
exit: result.exitCode, events: result.rawLines.join('\n') });
requireCondition(ran.execution.state === 'ran',
`Codex review did not execute: ${ran.reason}${ran.detail ? ` (${ran.detail})` : ''}`);
requireCondition(succeededCommands(result.rawLines) > 0, 'Codex review did not execute: no command completed (exec --json events)');
requireCondition(result.output.length > 50, 'Codex review output must contain more than 50 characters');
requireCondition(/finding|issue|review|change|diff|clean|no issues|p1|p2/.test(result.output.toLowerCase()),
'Codex output did not contain review findings or a clean-review result');