diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 4a3c53da..283bfc47 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -74,6 +74,8 @@ const TASK_WORKER_RECORD_WAIT_TIMEOUT_MS = 1000; // structured error instead of being killed with an empty result. // Ported from @russjhammond's openai/codex-plugin-cc#376. const FOREGROUND_TURN_TIMEOUT_MS = 110000; +// Background runs have no external Bash ceiling — give them the full default budget. +const DEFAULT_TURN_TIMEOUT_MS = 600000; const REASONING_EFFORTS = ["none", "minimal", "low", "medium", "high", "xhigh", "max", "ultra"]; const VALID_REASONING_EFFORTS = new Set(REASONING_EFFORTS); const MODEL_ALIASES = new Map([ @@ -419,6 +421,7 @@ async function executeReviewRun(request) { target: reviewTarget, model: request.model, effort: request.effort, + turnTimeoutMs: request.turnTimeoutMs, onProgress: request.onProgress }); const payload = { @@ -544,6 +547,7 @@ async function executeTaskRun(request) { sandbox: request.write ? "workspace-write" : "read-only", onProgress: request.onProgress, persistThread: true, + turnTimeoutMs: request.turnTimeoutMs, threadName: resumeThreadId ? null : buildPersistentTaskThreadName(request.prompt || DEFAULT_CONTINUE_PROMPT) }); @@ -655,7 +659,7 @@ function buildTaskJob(workspaceRoot, taskMetadata, write) { }); } -function buildTaskRequest({ cwd, model, effort, prompt, write, resumeLast, jobId }) { +function buildTaskRequest({ cwd, model, effort, prompt, write, resumeLast, jobId, turnTimeoutMs }) { return { cwd, model, @@ -663,7 +667,8 @@ function buildTaskRequest({ cwd, model, effort, prompt, write, resumeLast, jobId prompt, write, resumeLast, - jobId + jobId, + turnTimeoutMs }; } @@ -694,19 +699,21 @@ async function executeTransfer(cwd, options = {}) { }; } -// Set the per-turn budget for a FOREGROUND command (codex.mjs reads -// CODEX_TURN_TIMEOUT_MS at call time). Precedence: explicit --turn-timeout-ms -// flag > a pre-set CODEX_TURN_TIMEOUT_MS env > the foreground default just -// under the host Bash ceiling. Only call on foreground path: a detached -// background worker inherits the parent env, so capping here would shrink -// the background budget too. Ported from openai#376. -function applyForegroundTurnBudget(options) { +// Resolve the per-turn timeout from CLI options. Precedence: +// --turn-timeout-ms flag > CODEX_TURN_TIMEOUT_MS env > foreground/background default. +// Foreground default is just under the Bash-tool ceiling (110s) so a stalled turn +// returns a structured error instead of being SIGKILLed. Background gets the full +// 600s default (no external ceiling to collide with). +function resolveTurnTimeoutMsFromOptions(options) { const explicit = Number(options["turn-timeout-ms"]); if (Number.isFinite(explicit) && explicit > 0) { - process.env.CODEX_TURN_TIMEOUT_MS = String(explicit); - } else if (!process.env.CODEX_TURN_TIMEOUT_MS) { - process.env.CODEX_TURN_TIMEOUT_MS = String(FOREGROUND_TURN_TIMEOUT_MS); + return explicit; } + const fromEnv = Number(process.env.CODEX_TURN_TIMEOUT_MS); + if (Number.isFinite(fromEnv) && fromEnv > 0) { + return fromEnv; + } + return options.background ? DEFAULT_TURN_TIMEOUT_MS : FOREGROUND_TURN_TIMEOUT_MS; } function readTaskPrompt(cwd, options, positionals) { @@ -808,9 +815,6 @@ async function handleReviewCommand(argv, config) { jobClass: "review", summary: metadata.summary }); - if (!options.background) { - applyForegroundTurnBudget(options); - } await runForegroundCommand( job, (progress) => @@ -822,6 +826,7 @@ async function handleReviewCommand(argv, config) { effort, focusText, reviewName: config.reviewName, + turnTimeoutMs: resolveTurnTimeoutMsFromOptions(options), onProgress: progress }), { json: options.json } @@ -874,7 +879,8 @@ async function handleTask(argv) { prompt, write, resumeLast, - jobId: job.id + jobId: job.id, + turnTimeoutMs: resolveTurnTimeoutMsFromOptions(options) }); const { payload } = enqueueBackgroundTask(cwd, job, request); outputCommandResult(payload, renderQueuedTaskLaunch(payload), options.json); @@ -882,7 +888,6 @@ async function handleTask(argv) { } const job = buildTaskJob(workspaceRoot, taskMetadata, write); - applyForegroundTurnBudget(options); await runForegroundCommand( job, (progress) => @@ -894,6 +899,7 @@ async function handleTask(argv) { write, resumeLast, jobId: job.id, + turnTimeoutMs: resolveTurnTimeoutMsFromOptions(options), onProgress: progress }), { json: options.json } diff --git a/plugins/codex/scripts/lib/codex.mjs b/plugins/codex/scripts/lib/codex.mjs index 30c35b74..30665845 100644 --- a/plugins/codex/scripts/lib/codex.mjs +++ b/plugins/codex/scripts/lib/codex.mjs @@ -603,33 +603,19 @@ async function captureTurn(client, threadId, startRequest, options = {}) { }); try { - const response = await startRequest(); - options.onResponse?.(response, state); - state.turnId = response.turn?.id ?? null; - if (state.turnId) { - state.threadTurnIds.set(state.threadId, state.turnId); - } - for (const message of state.bufferedNotifications) { - if (belongsToTurn(state, message)) { - applyTurnNotification(state, message); - } else { - if (previousHandler) { - previousHandler(message); - } - } - } - state.bufferedNotifications.length = 0; - - if (response.turn?.status && response.turn.status !== "inProgress") { - completeTurn(state, response.turn); - } + // Arm the deadline BEFORE startRequest so a stalled turn/start (app-server + // alive but not responding) is also bounded. Finding #27-2. + const turnTimeoutMs = resolveTurnTimeoutMs(options); + let deadlineTimer = null; + const deadline = new Promise((_resolve, reject) => { + deadlineTimer = setTimeout(() => { + reject(new Error(`codex turn exceeded the ${turnTimeoutMs}ms turn budget.`)); + }, turnTimeoutMs); + deadlineTimer.unref?.(); + }); - // Bound the await so it can never outlast a dead process or a runaway turn. - // Ported from @russjhammond's openai/codex-plugin-cc#376. - // 1. state.completion — resolves on turn/completed (or inferred). Wire the - // previously-dead rejectCompletion to the client exit so an app-server - // death AFTER startRequest resolved rejects the await immediately. - // 2. deadline — hard per-turn budget (resolveTurnTimeoutMs). + // Wire exitPromise to rejectCompletion so an app-server death at any point + // (during startRequest OR during completion) rejects immediately. client.exitPromise.then(() => { if (state.completed) { return; @@ -638,21 +624,55 @@ async function captureTurn(client, threadId, startRequest, options = {}) { client.exitError ?? new Error("codex app-server exited before the turn completed.") ); }); - const turnTimeoutMs = resolveTurnTimeoutMs(options); - let deadlineTimer = null; - const deadline = new Promise((_resolve, reject) => { - deadlineTimer = setTimeout(() => { - reject(new Error(`codex turn exceeded the ${turnTimeoutMs}ms turn budget.`)); - }, turnTimeoutMs); - deadlineTimer.unref?.(); - }); + + let result; try { - return await Promise.race([state.completion, deadline]); + // Race the entire lifecycle (startRequest + completion) against the deadline. + result = await Promise.race([ + (async () => { + const response = await startRequest(); + options.onResponse?.(response, state); + state.turnId = response.turn?.id ?? null; + if (state.turnId) { + state.threadTurnIds.set(state.threadId, state.turnId); + } + for (const message of state.bufferedNotifications) { + if (belongsToTurn(state, message)) { + applyTurnNotification(state, message); + } else { + if (previousHandler) { + previousHandler(message); + } + } + } + state.bufferedNotifications.length = 0; + + if (response.turn?.status && response.turn.status !== "inProgress") { + completeTurn(state, response.turn); + } + + return await state.completion; + })(), + deadline + ]); + } catch (error) { + // Finding #27-1: on deadline, interrupt the in-flight turn so the broker + // stops executing a write-capable task after we've reported it failed. + // Best-effort: if interrupt fails (broker gone, network down), the error + // from the deadline still propagates — we just don't block on it. + if (state.threadId && state.turnId && options.cwd) { + await interruptAppServerTurn(options.cwd, { + threadId: state.threadId, + turnId: state.turnId + }).catch(() => {}); + } + throw error; } finally { if (deadlineTimer) { clearTimeout(deadlineTimer); } } + return result; } finally { clearCompletionTimer(state); client.setNotificationHandler(previousHandler ?? null); @@ -1095,6 +1115,8 @@ export async function runAppServerReview(cwd, options = {}) { target: options.target }), { + cwd, + turnTimeoutMs: options.turnTimeoutMs, onProgress: options.onProgress, onResponse(response, state) { if (response.reviewThreadId) { @@ -1230,6 +1252,8 @@ export async function runAppServerTurn(cwd, options = {}) { outputSchema: options.outputSchema ?? null }), { + cwd, + turnTimeoutMs: options.turnTimeoutMs, onProgress: options.onProgress, onResponse() { if (!options.effort) { diff --git a/tests/fake-codex-fixture.mjs b/tests/fake-codex-fixture.mjs index 7f2f2940..445ffc39 100644 --- a/tests/fake-codex-fixture.mjs +++ b/tests/fake-codex-fixture.mjs @@ -538,6 +538,11 @@ rl.on("line", (line) => { } case "turn/start": { + if (BEHAVIOR === "stalled-turn-start") { + // Never respond — simulates app-server alive but network stalled. + // companion's deadline must timeout and reject. + break; + } if (BEHAVIOR === "turn-start-fails") { throw new Error("turn/start failed after thread resolution"); } diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 9f0682df..052abe40 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -3029,3 +3029,25 @@ test("setup and status honor --cwd when reading shared session runtime", () => { input: JSON.stringify({ hook_event_name: "SessionEnd", cwd: targetWorkspace }) }); }); + +test("task with stalled turn/start times out via --turn-timeout-ms instead of hanging forever", () => { + const repo = makeTempDir(); + const binDir = makeTempDir(); + installFakeCodex(binDir, "stalled-turn-start"); + initGitRepo(repo); + fs.writeFileSync(path.join(repo, "README.md"), "hello\n"); + run("git", ["add", "README.md"], { cwd: repo }); + run("git", ["commit", "-m", "init"], { cwd: repo }); + + // 3s timeout — must reject within that, not hang forever. + const result = run("node", [SCRIPT, "task", "--turn-timeout-ms", "3000", "test prompt"], { + cwd: repo, + env: buildEnv(binDir) + }); + + // Must NOT hang — exit with a timeout error. + assert.notEqual(result.status, 0, "must exit non-zero on timeout, not hang"); + assert.match(result.stderr, /turn budget/i, "error must mention the turn budget"); + const storedJob = readPersistedJob(repo); + assert.equal(storedJob.status, "failed", "job must be marked failed"); +});