From dd9e4e75c93ad7f2938dce93a29424f5e72a6375 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 23:02:10 +0000 Subject: [PATCH] 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. --- test/codex-eval-recording.test.ts | 9 ++++++++- test/codex-session-lifecycle.test.ts | 3 +++ test/helpers/codex-eval.ts | 15 ++++++++++++++- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/test/codex-eval-recording.test.ts b/test/codex-eval-recording.test.ts index 50ace19bb..99a4f2c04 100644 --- a/test/codex-eval-recording.test.ts +++ b/test/codex-eval-recording.test.ts @@ -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 => ({ 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.", diff --git a/test/codex-session-lifecycle.test.ts b/test/codex-session-lifecycle.test.ts index 46ac8c496..4388f9a9f 100644 --- a/test/codex-session-lifecycle.test.ts +++ b/test/codex-session-lifecycle.test.ts @@ -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'); diff --git a/test/helpers/codex-eval.ts b/test/helpers/codex-eval.ts index f3761a207..6faa04e80 100644 --- a/test/helpers/codex-eval.ts +++ b/test/helpers/codex-eval.ts @@ -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');