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
40 changes: 23 additions & 17 deletions plugins/codex/scripts/codex-companion.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down Expand Up @@ -419,6 +421,7 @@ async function executeReviewRun(request) {
target: reviewTarget,
model: request.model,
effort: request.effort,
turnTimeoutMs: request.turnTimeoutMs,
onProgress: request.onProgress
});
const payload = {
Expand Down Expand Up @@ -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)
});

Expand Down Expand Up @@ -655,15 +659,16 @@ 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,
effort,
prompt,
write,
resumeLast,
jobId
jobId,
turnTimeoutMs
};
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -808,9 +815,6 @@ async function handleReviewCommand(argv, config) {
jobClass: "review",
summary: metadata.summary
});
if (!options.background) {
applyForegroundTurnBudget(options);
}
await runForegroundCommand(
job,
(progress) =>
Expand All @@ -822,6 +826,7 @@ async function handleReviewCommand(argv, config) {
effort,
focusText,
reviewName: config.reviewName,
turnTimeoutMs: resolveTurnTimeoutMsFromOptions(options),
onProgress: progress
}),
{ json: options.json }
Expand Down Expand Up @@ -874,15 +879,15 @@ 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);
return;
}

const job = buildTaskJob(workspaceRoot, taskMetadata, write);
applyForegroundTurnBudget(options);
await runForegroundCommand(
job,
(progress) =>
Expand All @@ -894,6 +899,7 @@ async function handleTask(argv) {
write,
resumeLast,
jobId: job.id,
turnTimeoutMs: resolveTurnTimeoutMsFromOptions(options),
onProgress: progress
}),
{ json: options.json }
Expand Down
94 changes: 59 additions & 35 deletions plugins/codex/scripts/lib/codex.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions tests/fake-codex-fixture.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
22 changes: 22 additions & 0 deletions tests/runtime.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});