diff --git a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts index 4471be814bf39..6c3d7c9d7cb07 100644 --- a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts @@ -3375,6 +3375,7 @@ describe("CodexAdapterV2 post-settle continuation", () => { assert.lengthOf(retryItems, 2); assert.equal(retryItems[0]?.status, "running"); assert.equal(retryItems[0]?.failure.code, "responseStreamDisconnected"); + assert.equal(retryItems[0]?.failure.class, "transport_error"); assert.deepEqual(retryItems[0]?.retry, { attempt: 2, maxAttempts: 5, @@ -3413,6 +3414,100 @@ describe("CodexAdapterV2 post-settle continuation", () => { ), ); + // regression: #28936 a Codex retry on HTTP 429 is a usage limit, like exhaustion on 429 + it.effect.each([ + { httpStatusCode: 429, expectedClass: "usage_limit" }, + { httpStatusCode: 502, expectedClass: "transport_error" }, + ] as const)( + "classifies a Codex retry on HTTP $httpStatusCode and the turn that fails on it", + ({ httpStatusCode, expectedClass }) => + Effect.scoped( + Effect.gen(function* () { + const scenario = `codex-retry-status-${httpStatusCode}`; + const nativeThreadId = `native-${scenario}-thread`; + const nativeTurnId = `native-${scenario}-turn`; + const additionalDetails = `exceeded retry limit, last status: ${httpStatusCode}`; + const transcript = makeCodexReplayTranscript({ + scenario, + entries: [ + ...codexReplayPreamble({ nativeThreadId, nativeTurnId, prompt: "Continue." }), + { + type: "emit_inbound", + label: "error/retry", + frame: { + method: "error", + params: { + threadId: nativeThreadId, + turnId: nativeTurnId, + willRetry: true, + error: { + message: "Reconnecting... 1/5", + additionalDetails, + codexErrorInfo: { responseStreamDisconnected: { httpStatusCode } }, + }, + }, + }, + }, + // A turn that ends without evidence of its own keeps the pending retry's failure. + { + type: "emit_inbound", + label: "turn/completed", + frame: { + method: "turn/completed", + params: { + threadId: nativeThreadId, + turn: makeCodexReplayTurn({ id: nativeTurnId, status: "failed" }), + }, + }, + }, + ], + }); + const harness = yield* makeCodexReplayHarness(transcript); + yield* harness.runtime.startTurn( + makeCodexTestTurnInput({ + threadId: harness.threadId, + providerThread: harness.providerThread, + now: yield* DateTime.now, + attemptId: RunAttemptId.make(`attempt-${scenario}`), + text: "Continue.", + }), + ); + yield* harness.firstTerminal; + + const runningRetry = harness.events.find( + (event) => + event.type === "turn_item.updated" && + event.turnItem.type === "error" && + event.turnItem.retry !== undefined && + event.turnItem.status === "running", + ); + assert.equal( + runningRetry?.type === "turn_item.updated" && runningRetry.turnItem.type === "error" + ? runningRetry.turnItem.failure.class + : undefined, + expectedClass, + ); + const terminal = harness.terminalEvents()[0]; + assert.equal(terminal?.status, "failed"); + if (terminal?.status !== "failed") return; + assert.equal(terminal.failure.class, expectedClass); + assert.equal(terminal.failure.code, "responseStreamDisconnected"); + assert.equal(terminal.failure.message, additionalDetails); + assert.equal(terminal.retry?.attempt, 1); + assert.equal(terminal.threadDisposition, "reusable"); + }).pipe( + Effect.provide( + Layer.mergeAll( + IdAllocator.layer, + McpProviderSessions.layer, + NodeServices.layer, + NodeCrypto.layer, + ), + ), + ), + ), + ); + it.effect.each(["completed", "interrupted"] as const)( "retains Codex reasoning parts when the turn is %s", (terminalStatus) => @@ -5474,6 +5569,34 @@ describe("CodexAdapterV2 post-settle continuation", () => { notification: false, expectedClass: "provider_error", }, + { + name: "disconnected-429", + code: "responseStreamDisconnected", + errorInfo: { responseStreamDisconnected: { httpStatusCode: 429 } }, + notification: false, + expectedClass: "usage_limit", + }, + { + name: "stream-connection-429", + code: "responseStreamConnectionFailed", + errorInfo: { responseStreamConnectionFailed: { httpStatusCode: 429 } }, + notification: false, + expectedClass: "usage_limit", + }, + { + name: "http-connection-429", + code: "httpConnectionFailed", + errorInfo: { httpConnectionFailed: { httpStatusCode: 429 } }, + notification: false, + expectedClass: "usage_limit", + }, + { + name: "http-connection-503", + code: "httpConnectionFailed", + errorInfo: { httpConnectionFailed: { httpStatusCode: 503 } }, + notification: false, + expectedClass: "provider_error", + }, { name: "chatgpt-workspace-limit", code: "other", @@ -5681,38 +5804,52 @@ describe("CodexAdapterV2 post-settle continuation", () => { ), ); - // regression: #17501 Codex 429 retry exhaustion classifies as a usage limit - it.effect.each([ - { - notificationStatus: 500, - completionStatus: 429, - willRetry: false, - expectedClass: "usage_limit", - }, - { - notificationStatus: 429, - completionStatus: 500, - willRetry: true, - expectedClass: "provider_error", - }, - { - notificationStatus: 429, - completionStatus: null, - willRetry: false, - expectedClass: "usage_limit", - }, - { - notificationStatus: 429, - completionStatus: null, - willRetry: true, - expectedClass: "usage_limit", - }, - ] as const)( - "honors completion HTTP status from $notificationStatus to $completionStatus (retry: $willRetry)", - ({ notificationStatus, completionStatus, expectedClass, willRetry }) => + // regression: #17501 Codex 429 retry exhaustion classifies as a usage limit, and #28936 every + // other error info that carries an HTTP status + it.effect.each( + ( + [ + "responseTooManyFailedAttempts", + "responseStreamDisconnected", + "responseStreamConnectionFailed", + "httpConnectionFailed", + ] as const + ).flatMap((variant) => + ( + [ + { + notificationStatus: 500, + completionStatus: 429, + willRetry: false, + expectedClass: "usage_limit", + }, + { + notificationStatus: 429, + completionStatus: 500, + willRetry: true, + expectedClass: "provider_error", + }, + { + notificationStatus: 429, + completionStatus: null, + willRetry: false, + expectedClass: "usage_limit", + }, + { + notificationStatus: 429, + completionStatus: null, + willRetry: true, + expectedClass: "usage_limit", + }, + ] as const + ).map((row) => ({ variant, ...row })), + ), + )( + "honors completion HTTP status on $variant from $notificationStatus to $completionStatus (retry: $willRetry)", + ({ variant, notificationStatus, completionStatus, expectedClass, willRetry }) => Effect.scoped( Effect.gen(function* () { - const name = `${notificationStatus}-${completionStatus}-${willRetry}`; + const name = `${variant}-${notificationStatus}-${completionStatus}-${willRetry}`; const nativeThreadId = `native-exhaustion-status-${name}`; const nativeTurnId = `turn-exhaustion-status-${name}`; const message = "Provider stopped this request."; @@ -5732,9 +5869,7 @@ describe("CodexAdapterV2 post-settle continuation", () => { willRetry, error: { message, - codexErrorInfo: { - responseTooManyFailedAttempts: { httpStatusCode: notificationStatus }, - }, + codexErrorInfo: { [variant]: { httpStatusCode: notificationStatus } }, additionalDetails, }, }, @@ -5751,9 +5886,7 @@ describe("CodexAdapterV2 post-settle continuation", () => { ...makeCodexReplayTurn({ id: nativeTurnId, status: "failed" }), error: { message, - codexErrorInfo: { - responseTooManyFailedAttempts: { httpStatusCode: completionStatus }, - }, + codexErrorInfo: { [variant]: { httpStatusCode: completionStatus } }, }, }, }, @@ -5776,7 +5909,7 @@ describe("CodexAdapterV2 post-settle continuation", () => { assert.equal(terminal?.status, "failed"); if (terminal?.status !== "failed") return; assert.equal(terminal.failure.class, expectedClass); - assert.equal(terminal.failure.code, "responseTooManyFailedAttempts"); + assert.equal(terminal.failure.code, variant); assert.equal(terminal.threadDisposition, "reusable"); const notificationClass = notificationStatus === 429 ? "usage_limit" : "provider_error"; assert.equal( diff --git a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts index 688de092c03d0..1c31153cc95b4 100644 --- a/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts +++ b/apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts @@ -1121,6 +1121,22 @@ function codexExhaustionHttpStatus(errorInfo: CodexErrorInfo | null | undefined) : null; } +/** + * The upstream HTTP status on any error info that carries one. Codex reports + * each retry it schedules as `responseStreamDisconnected` with the failed + * attempt's status, so a retry on 429 is the same limit as exhaustion on 429. + */ +function codexErrorHttpStatus(errorInfo: CodexErrorInfo | null | undefined): number | null { + if (typeof errorInfo !== "object" || errorInfo === null) return null; + if ("responseStreamDisconnected" in errorInfo) + return errorInfo.responseStreamDisconnected.httpStatusCode ?? null; + if ("responseStreamConnectionFailed" in errorInfo) + return errorInfo.responseStreamConnectionFailed.httpStatusCode ?? null; + if ("httpConnectionFailed" in errorInfo) + return errorInfo.httpConnectionFailed.httpStatusCode ?? null; + return codexExhaustionHttpStatus(errorInfo); +} + function codexUsageLimitError( code: string | null, errorInfo: CodexErrorInfo | null | undefined, @@ -1128,7 +1144,7 @@ function codexUsageLimitError( return ( code === "usageLimitExceeded" || code === "rateLimitExceeded" || - codexExhaustionHttpStatus(errorInfo) === 429 + codexErrorHttpStatus(errorInfo) === 429 ); } @@ -5773,15 +5789,15 @@ export const makeCodexAdapterV2 = Effect.fn("makeCodexAdapterV2")(function* ( ? "usage_limit" : "provider_error"; // The notification carries the richer message, so it wins when the - // completion repeats it. An explicit exhaustion status on the - // completion still decides the class; a missing one is not evidence. - const exhaustionStatus = codexExhaustionHttpStatus(input.failureErrorInfo); + // completion repeats it. An explicit HTTP status on the completion + // still decides the class; a missing one is not evidence. + const completionStatus = codexErrorHttpStatus(input.failureErrorInfo); const failure = previousFailure !== undefined && (input.failureMessage === undefined || input.failureMessage === previousFailure.nativeMessage) && (failureCode === undefined || failureCode === previousFailure.nativeCode) && - (exhaustionStatus === null || previousFailure.failure.class === failureClass) + (completionStatus === null || previousFailure.failure.class === failureClass) ? previousFailure.failure : withCodexManagedFailure( makeProviderFailure({