Skip to content

Commit 494c0b4

Browse files
jonathangujonathangu
committed
fix(codex): keep long turns alive through progress
1 parent 3f5054d commit 494c0b4

4 files changed

Lines changed: 174 additions & 23 deletions

File tree

‎extensions/codex/src/app-server/run-attempt.test.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -839,6 +839,75 @@ describe("runCodexAppServerAttempt", () => {
839839
expect(queueAgentHarnessMessage("session-1", "after silent turn")).toBe(false);
840840
});
841841

842+
it("keeps an accepted Codex turn alive beyond the per-request timeout", async () => {
843+
const harness = createStartedThreadHarness();
844+
const params = createParams(
845+
path.join(tempDir, "session.jsonl"),
846+
path.join(tempDir, "workspace"),
847+
);
848+
params.timeoutMs = 5;
849+
850+
const run = runCodexAppServerAttempt(params, { turnTerminalIdleTimeoutMs: 100 });
851+
await harness.waitForMethod("turn/start");
852+
await new Promise<void>((resolve) => setTimeout(resolve, 25));
853+
await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" });
854+
855+
await expect(run).resolves.toMatchObject({
856+
aborted: false,
857+
timedOut: false,
858+
promptError: null,
859+
});
860+
});
861+
862+
it("extends the Codex turn idle watch on meaningful turn progress", async () => {
863+
const harness = createStartedThreadHarness();
864+
const params = createParams(
865+
path.join(tempDir, "session.jsonl"),
866+
path.join(tempDir, "workspace"),
867+
);
868+
params.timeoutMs = 60_000;
869+
870+
const run = runCodexAppServerAttempt(params, { turnTerminalIdleTimeoutMs: 30 });
871+
await harness.waitForMethod("turn/start");
872+
await new Promise<void>((resolve) => setTimeout(resolve, 20));
873+
await harness.notify({
874+
method: "item/updated",
875+
params: {
876+
threadId: "thread-1",
877+
turnId: "turn-1",
878+
item: { id: "item-1", type: "reasoning", text: "thinking" },
879+
},
880+
});
881+
await new Promise<void>((resolve) => setTimeout(resolve, 20));
882+
await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" });
883+
884+
await expect(run).resolves.toMatchObject({
885+
aborted: false,
886+
timedOut: false,
887+
promptError: null,
888+
});
889+
});
890+
891+
it("does not extend the Codex turn idle watch for account-only updates", async () => {
892+
const harness = createStartedThreadHarness();
893+
const params = createParams(
894+
path.join(tempDir, "session.jsonl"),
895+
path.join(tempDir, "workspace"),
896+
);
897+
params.timeoutMs = 60_000;
898+
899+
const run = runCodexAppServerAttempt(params, { turnTerminalIdleTimeoutMs: 20 });
900+
await harness.waitForMethod("turn/start");
901+
await new Promise<void>((resolve) => setTimeout(resolve, 10));
902+
await harness.notify(rateLimitsUpdated(Date.now() + 60_000));
903+
904+
await expect(run).resolves.toMatchObject({
905+
aborted: true,
906+
timedOut: true,
907+
promptError: "codex app-server turn idle timed out waiting for turn/completed",
908+
});
909+
});
910+
842911
it("applies before_prompt_build to Codex developer instructions and turn input", async () => {
843912
const beforePromptBuild = vi.fn(async () => ({
844913
systemPrompt: "custom codex system",

‎extensions/codex/src/app-server/run-attempt.ts‎

Lines changed: 83 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ import { filterToolsForVisionInputs } from "./vision-tools.js";
114114

115115
const CODEX_DYNAMIC_TOOL_TIMEOUT_MS = 30_000;
116116
const CODEX_APP_SERVER_STARTUP_CONNECTION_CLOSE_MAX_ATTEMPTS = 3;
117-
const CODEX_TURN_COMPLETION_IDLE_TIMEOUT_MS = 60_000;
117+
const CODEX_TURN_COMPLETION_IDLE_TIMEOUT_MS = 5 * 60_000;
118118
const CODEX_TURN_TERMINAL_IDLE_TIMEOUT_MS = 30 * 60_000;
119119
const CODEX_STEER_ALL_DEBOUNCE_MS = 500;
120120
const LOG_FIELD_MAX_LENGTH = 160;
@@ -710,7 +710,7 @@ export async function runCodexAppServerAttempt(
710710
let turnTerminalIdleWatchArmed = false;
711711
let turnCompletionLastActivityAt = Date.now();
712712
let turnCompletionLastActivityReason = "startup";
713-
let activeAppServerTurnRequests = 0;
713+
let activeMeaningfulAppServerTurnRequests = 0;
714714

715715
const clearTurnCompletionIdleTimer = () => {
716716
if (turnCompletionIdleTimer) {
@@ -731,7 +731,7 @@ export async function runCodexAppServerAttempt(
731731
completed ||
732732
runAbortController.signal.aborted ||
733733
!turnCompletionIdleWatchArmed ||
734-
activeAppServerTurnRequests > 0
734+
activeMeaningfulAppServerTurnRequests > 0
735735
) {
736736
return;
737737
}
@@ -767,7 +767,7 @@ export async function runCodexAppServerAttempt(
767767
completed ||
768768
runAbortController.signal.aborted ||
769769
!turnTerminalIdleWatchArmed ||
770-
activeAppServerTurnRequests > 0
770+
activeMeaningfulAppServerTurnRequests > 0
771771
) {
772772
return;
773773
}
@@ -804,7 +804,7 @@ export async function runCodexAppServerAttempt(
804804
completed ||
805805
runAbortController.signal.aborted ||
806806
!turnCompletionIdleWatchArmed ||
807-
activeAppServerTurnRequests > 0
807+
activeMeaningfulAppServerTurnRequests > 0
808808
) {
809809
return;
810810
}
@@ -820,7 +820,7 @@ export async function runCodexAppServerAttempt(
820820
completed ||
821821
runAbortController.signal.aborted ||
822822
!turnTerminalIdleWatchArmed ||
823-
activeAppServerTurnRequests > 0
823+
activeMeaningfulAppServerTurnRequests > 0
824824
) {
825825
return;
826826
}
@@ -871,7 +871,9 @@ export async function runCodexAppServerAttempt(
871871
};
872872

873873
const handleNotification = async (notification: CodexServerNotification) => {
874-
touchTurnCompletionActivity(`notification:${notification.method}`);
874+
if (isMeaningfulCodexTurnNotification(notification, thread.threadId, turnId)) {
875+
touchTurnCompletionActivity(`notification:${notification.method}`);
876+
}
875877
userInputBridge?.handleNotification(notification);
876878
if (!projector || !turnId) {
877879
pendingNotifications.push(notification);
@@ -912,9 +914,12 @@ export async function runCodexAppServerAttempt(
912914

913915
const notificationCleanup = client.addNotificationHandler(enqueueNotification);
914916
const requestCleanup = client.addRequestHandler(async (request) => {
915-
activeAppServerTurnRequests += 1;
916-
clearTurnCompletionIdleTimer();
917-
touchTurnCompletionActivity(`request:${request.method}`);
917+
const meaningfulRequest = isMeaningfulCodexTurnRequest(request.method);
918+
if (meaningfulRequest) {
919+
activeMeaningfulAppServerTurnRequests += 1;
920+
clearTurnCompletionIdleTimer();
921+
touchTurnCompletionActivity(`request:${request.method}`);
922+
}
918923
let armCompletionWatchOnResponse = false;
919924
try {
920925
if (request.method === "account/chatgptAuthTokens/refresh") {
@@ -1018,10 +1023,15 @@ export async function runCodexAppServerAttempt(
10181023
});
10191024
return response as JsonValue;
10201025
} finally {
1021-
activeAppServerTurnRequests = Math.max(0, activeAppServerTurnRequests - 1);
1022-
touchTurnCompletionActivity(`request:${request.method}:response`, {
1023-
arm: armCompletionWatchOnResponse,
1024-
});
1026+
if (meaningfulRequest) {
1027+
activeMeaningfulAppServerTurnRequests = Math.max(
1028+
0,
1029+
activeMeaningfulAppServerTurnRequests - 1,
1030+
);
1031+
touchTurnCompletionActivity(`request:${request.method}:response`, {
1032+
arm: armCompletionWatchOnResponse,
1033+
});
1034+
}
10251035
}
10261036
});
10271037

@@ -1176,14 +1186,15 @@ export async function runCodexAppServerAttempt(
11761186
turnTerminalIdleWatchArmed = true;
11771187
touchTurnCompletionActivity("turn:start");
11781188

1179-
const timeout = setTimeout(
1180-
() => {
1181-
timedOut = true;
1182-
projector?.markTimedOut();
1183-
runAbortController.abort("timeout");
1184-
},
1185-
Math.max(100, params.timeoutMs),
1186-
);
1189+
const hardTimeoutMs = resolveCodexTurnHardTimeoutMs({
1190+
requestTimeoutMs: params.timeoutMs,
1191+
terminalIdleTimeoutMs: turnTerminalIdleTimeoutMs,
1192+
});
1193+
const timeout = setTimeout(() => {
1194+
timedOut = true;
1195+
projector?.markTimedOut();
1196+
runAbortController.abort("timeout");
1197+
}, hardTimeoutMs);
11871198

11881199
const abortListener = () => {
11891200
interruptCodexTurnBestEffort(client, {
@@ -1677,6 +1688,56 @@ function resolveCodexTurnTerminalIdleTimeoutMs(value: number | undefined): numbe
16771688
return Math.max(1, Math.floor(value));
16781689
}
16791690

1691+
function resolveCodexTurnHardTimeoutMs(params: {
1692+
requestTimeoutMs: number;
1693+
terminalIdleTimeoutMs: number;
1694+
}): number {
1695+
const candidates = [params.requestTimeoutMs, params.terminalIdleTimeoutMs].filter(
1696+
(value) => Number.isFinite(value) && value > 0,
1697+
);
1698+
return Math.max(100, ...candidates);
1699+
}
1700+
1701+
function isMeaningfulCodexTurnNotification(
1702+
notification: CodexServerNotification,
1703+
threadId: string,
1704+
turnId: string | undefined,
1705+
): boolean {
1706+
if (notification.method.startsWith("account/")) {
1707+
return false;
1708+
}
1709+
const params = isJsonObject(notification.params) ? notification.params : undefined;
1710+
if (!params || readString(params, "threadId") !== threadId) {
1711+
return false;
1712+
}
1713+
if (notification.method === "thread/tokenUsage/updated") {
1714+
return true;
1715+
}
1716+
if (notification.method === "hook/started" || notification.method === "hook/completed") {
1717+
const notificationTurnId = params.turnId;
1718+
return !turnId || notificationTurnId === turnId || notificationTurnId === null;
1719+
}
1720+
const notificationTurnId = readNotificationTurnId(params);
1721+
if (turnId && notificationTurnId && notificationTurnId !== turnId) {
1722+
return false;
1723+
}
1724+
return (
1725+
notification.method.startsWith("turn/") ||
1726+
notification.method.startsWith("item/") ||
1727+
notification.method.startsWith("rawResponseItem/") ||
1728+
notification.method === "error"
1729+
);
1730+
}
1731+
1732+
function isMeaningfulCodexTurnRequest(method: string): boolean {
1733+
return (
1734+
method === "item/tool/call" ||
1735+
method === "item/tool/requestUserInput" ||
1736+
method === "mcpServer/elicitation/request" ||
1737+
isCodexAppServerApprovalRequest(method)
1738+
);
1739+
}
1740+
16801741
function readDynamicToolCallParams(
16811742
value: JsonValue | undefined,
16821743
): CodexDynamicToolCallParams | undefined {

‎src/agents/pi-embedded-runner/run.overflow-compaction.loop.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,23 @@ describe("overflow compaction in run loop", () => {
635635
expect(result.payloads?.[0]?.text).toContain("timed out");
636636
});
637637

638+
it("does not add a generic timeout payload after a messaging tool delivery", async () => {
639+
mockedRunEmbeddedAttempt.mockResolvedValue(
640+
makeAttemptResult({
641+
aborted: true,
642+
timedOut: true,
643+
timedOutDuringCompaction: false,
644+
assistantTexts: [],
645+
didSendViaMessagingTool: true,
646+
messagingToolSentTexts: ["Already delivered through Telegram."],
647+
}),
648+
);
649+
650+
const result = await runEmbeddedPiAgent(baseParams);
651+
652+
expect(result.payloads).toBeUndefined();
653+
});
654+
638655
it("returns a timeout payload instead of a partial assistant fragment after stream timeout", async () => {
639656
mockedRunEmbeddedAttempt.mockResolvedValue(
640657
makeAttemptResult({

‎src/agents/pi-embedded-runner/run.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2273,9 +2273,13 @@ export async function runEmbeddedPiAgent(
22732273
});
22742274

22752275
// Timeout aborts can leave the run without payloads or with only a
2276-
// partial assistant fragment. Emit an explicit timeout error instead.
2276+
// partial assistant fragment. Emit an explicit timeout error unless
2277+
// the turn already committed a messaging-tool delivery; in that case
2278+
// a second generic timeout reply is noise and can make a successful
2279+
// visible update look like a failed user request.
22772280
if (
22782281
timedOutDuringPrompt &&
2282+
!attempt.didSendViaMessagingTool &&
22792283
(!payloadsWithToolMedia?.length || hasPartialAssistantTextAfterPromptTimeout)
22802284
) {
22812285
const timeoutText = idleTimedOut

0 commit comments

Comments
 (0)