diff --git a/docs-site/src/content/docs/reference/management-api.md b/docs-site/src/content/docs/reference/management-api.md index 3651cadf52d..465ed84f734 100644 --- a/docs-site/src/content/docs/reference/management-api.md +++ b/docs-site/src/content/docs/reference/management-api.md @@ -290,10 +290,11 @@ upstream model evidence remains absent rather than being inferred from the reque Failed request rows can include bounded `upstreamErrorType`, `upstreamErrorCode`, and `upstreamRequestId` diagnostics. On the Codex upstream WebSocket path, a bare error followed by -EOF retains recognized rate-limit or overload codes in the synthesized failed terminal. -Request logs record these as 429 for `rate_limit_exceeded` / `rate_limit_error`, or 503 for -`server_is_overloaded` / `overloaded_error`. A recognized type without a code uses the same -status. On the streaming path, an explicit unknown code records the 5xx status the Codex +EOF retains recognized rate-limit, overload, or client-refusal codes in the synthesized failed terminal. +Request logs record these as 429 for `rate_limit_exceeded` / `rate_limit_error`, 503 for +`server_is_overloaded` / `overloaded_error`, and 400/401/403/404 for `invalid_request_error` / +`authentication_error` / `permission_error` / `not_found_error`. A recognized type without a code +uses the same status. On the streaming path, an explicit unknown code records the 5xx status the Codex WebSocket wrapper states as a top-level integer `status` or `status_code` (one of the two, not both); otherwise, and for buffered JSON callers, it remains a generic 502 failure. The client terminal keeps `upstream_server_error` for unknown codes either way. Bare-error status is provisional: diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 9925e111815..79675636cc7 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -369,6 +369,7 @@ "cl01-openai-chat-review-regressions.test.ts": "routing", "cl01-review-regressions.test.ts": "routing", "claude-529-mapping.test.ts": "claude-integration", + "claude-client-refusal-status.test.ts": "claude-integration", "claude-agent-startup-sync.test.ts": "claude-integration", "claude-agents-inject-client.test.ts": "claude-integration", "claude-agents-inject.test.ts": "claude-integration", diff --git a/src/lib/errors.ts b/src/lib/errors.ts index 9de95942dfe..14793fa92ec 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -650,10 +650,22 @@ export function adapterFailureFromMessage(message: string): { httpStatus: number return { httpStatus, error }; } +/** A declared client refusal: retrying the identical request cannot change the verdict. */ +const CLIENT_ERROR_STATUS: Record = { + invalid_request_error: 400, + authentication_error: 401, + permission_error: 403, + not_found_error: 404, +}; + /** Narrow structured classes carried through a synthesized bare-error terminal. */ export function recognizedUpstreamError(error: { type?: unknown; code?: unknown }): - { type: string; code: string; httpStatus: 429 | 503 } | undefined { + { type: string; code: string; httpStatus: 400 | 401 | 403 | 404 | 429 | 503 } | undefined { const code = typeof error.code === "string" ? error.code : undefined; + const clientClass = code ?? (typeof error.type === "string" ? error.type : undefined); + if (clientClass !== undefined && Object.hasOwn(CLIENT_ERROR_STATUS, clientClass)) { + return { type: clientClass, code: clientClass, httpStatus: CLIENT_ERROR_STATUS[clientClass] }; + } // An explicit unknown code must not gain a recognized class from its type or copy. if (code !== undefined && !["rate_limit_exceeded", "rate_limit_error", "server_is_overloaded", "overloaded_error"].includes(code)) return undefined; if (code === "rate_limit_exceeded" || code === "rate_limit_error" @@ -680,8 +692,13 @@ export function httpStatusFromTerminalError(error: { if (isCyberPolicyCode(error.code) || (error.message ? isCyberPolicyMessage(error.message) : false)) { return 400; } + const message = error.message ?? ""; const recognized = recognizedUpstreamError(error); - if (recognized) return recognized.httpStatus; + // Client-closed messages often arrive as invalid_request_error after classifyError, so a + // recognized 400 yields to the client-closed check below; every other class keeps its order. + if (recognized && !(recognized.httpStatus === 400 && message && isClientClosedMessage(message))) { + return recognized.httpStatus; + } if (error.type === "rate_limit_error" || error.code === "rate_limit_exceeded") return 429; if (error.type === "authentication_error" || error.code === "invalid_api_key") return 401; if ( @@ -693,7 +710,6 @@ export function httpStatusFromTerminalError(error: { if (error.type === "server_error" && error.code === "server_is_overloaded") return 503; // Client-closed messages often arrive as invalid_request_error after classifyError; check message // before treating every invalid_request_error as HTTP 400. - const message = error.message ?? ""; if (message && isClientClosedMessage(message)) return 499; if (error.type === "invalid_request_error") return 400; if (error.type === "proxy_error") return 500; diff --git a/src/server/claude-messages.ts b/src/server/claude-messages.ts index 7bf4e6fa456..8b82510dd49 100644 --- a/src/server/claude-messages.ts +++ b/src/server/claude-messages.ts @@ -47,7 +47,7 @@ import { REPLAY_REFUSED_STATUS, UPSTREAM_RESET_REPLAY_REFUSED_CODE, } from "../lib/upstream-retry"; -import { UNDECLARED_TOOL_CALL_ERROR_CODE } from "../lib/errors"; +import { isClientClosedMessage, recognizedUpstreamError, UNDECLARED_TOOL_CALL_ERROR_CODE } from "../lib/errors"; import { resolveClientRetryAfter } from "../lib/retry-after"; import { anthropicRateLimitHeaders } from "./anthropic-rate-limit-headers"; import { upstreamMessagesRequestIdHeaders } from "./messages-response-headers"; @@ -1560,8 +1560,17 @@ async function handleClaudeMessagesWithBudget( "translation_buffer_limit", ); } + // A declared client refusal keeps its 4xx: as a 502 Claude Code retries a verdict that cannot + // change. Server classes and rate limits keep the 502 this path has always answered. A 400 + // worded as a client close answers the 499 httpStatusFromTerminalError logs for it. + const refusalStatus = recognizedUpstreamError(translatedError ?? {})?.httpStatus; return new Response(JSON.stringify(message), { - status: isError ? (translatedError?.code === "context_length_exceeded" ? 400 : 502) : 200, + status: isError + ? translatedError?.code === "context_length_exceeded" ? 400 + : refusalStatus !== undefined && refusalStatus < 429 + ? refusalStatus === 400 && isClientClosedMessage(String(translatedError?.message ?? "")) ? 499 : refusalStatus + : 502 + : 200, headers: { "Content-Type": "application/json" }, }); } diff --git a/src/server/relay.ts b/src/server/relay.ts index 8659c25d1f1..133d11d3ae0 100644 --- a/src/server/relay.ts +++ b/src/server/relay.ts @@ -170,7 +170,7 @@ export function failedTailFrame(encoder: TextEncoder, err: unknown, maskCredenti * its fatal set (codex-rs/codex-api/src/sse/responses.rs:423-450), so stamping * `upstream_server_error` on a refusal delivered it as a retryable disconnect * and drove the reconnect loop in #5176. Without a refusal code the terminal - * carries only recognized rate-limit/overload classes; unknown codes stay generic. + * carries only recognized client-refusal/rate-limit/overload classes; unknown codes stay generic. */ export function upstreamErrorTailFrame( encoder: TextEncoder, diff --git a/src/server/request-log-terminal-status.ts b/src/server/request-log-terminal-status.ts index 38486d4bc01..80174c82084 100644 --- a/src/server/request-log-terminal-status.ts +++ b/src/server/request-log-terminal-status.ts @@ -67,7 +67,7 @@ const KNOWN_UPSTREAM_ERROR_TYPES: ReadonlySet = new Set([ "RESOURCE_EXHAUSTED", "INTERNAL", "UNAVAILABLE", "DEADLINE_EXCEEDED", ]); -/** Bare top-level error codes worth keeping: the classes recognizedUpstreamError maps, plus server_error. */ +/** Bare top-level error codes worth keeping: the rate-limit and overload classes recognizedUpstreamError maps, plus server_error. */ const KNOWN_BARE_ERROR_CODES: ReadonlySet = new Set([ "rate_limit_exceeded", "rate_limit_error", "server_is_overloaded", "overloaded_error", "server_error", ]); diff --git a/structure/transports/responses-wire-shapes.md b/structure/transports/responses-wire-shapes.md index 8709593b69a..e88250a6285 100644 --- a/structure/transports/responses-wire-shapes.md +++ b/structure/transports/responses-wire-shapes.md @@ -397,7 +397,7 @@ Native passthrough SSE has TWO shapes, selected per request in The shared `createSseInspector` factory in `relay.ts` retains the complete inspection side effects, including the #44 late-terminal semantics. -Both client readers retain a bounded, redacted message and the first structured refusal code from a bare upstream `error`. EOF synthesis also retains narrowly recognized rate-limit codes (`rate_limit_exceeded`, `rate_limit_error`) and overload codes (`server_is_overloaded`, `overloaded_error`); recognized types apply only without an explicit code. +Both client readers retain a bounded, redacted message and the first structured refusal code from a bare upstream `error`. EOF synthesis also retains narrowly recognized rate-limit codes (`rate_limit_exceeded`, `rate_limit_error`) overload codes (`server_is_overloaded`, `overloaded_error`), and the client-refusal classes `invalid_request_error`, `authentication_error`, `permission_error`, `not_found_error` (400/401/403/404, the code repeating the class); recognized types apply only without an explicit code. At EOF without a real terminal they synthesize `response.failed` rather than `adapter_eof`; a code without a message still produces a terminal. Codex retries codes outside its fatal set, so code and message follow the same candidate precedence; recognized refusal copy is used only when the event has no code. A read failure after refusal reports that refusal (#5176). diff --git a/tests/claude-integration/claude-client-refusal-status.test.ts b/tests/claude-integration/claude-client-refusal-status.test.ts new file mode 100644 index 00000000000..67da06fc2df --- /dev/null +++ b/tests/claude-integration/claude-client-refusal-status.test.ts @@ -0,0 +1,116 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { saveConfig } from "../../src/config"; +import { httpStatusFromTerminalError } from "../../src/lib/errors"; +import { startServer } from "../../src/server"; +import type { OcxConfig } from "../../src/types"; +import { installIsolatedCodexHome, type IsolatedCodexHome } from "../helpers/isolated-codex-home"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +// A 200 SSE that the upstream ends with a bare `error` frame declaring a client-class type +// (no refusal code). It used to be stamped `upstream_error` and answered as a retryable 502, +// so Claude Code retried a request the upstream had already refused. + +let testDir = ""; +let previousHome: string | undefined; +let isolatedCodexHome: IsolatedCodexHome | null = null; + +beforeEach(() => { + previousHome = process.env.OPENCODEX_HOME; + isolatedCodexHome = installIsolatedCodexHome("ocx-claude-refusal-"); + testDir = mkdtempSync(join(tmpdir(), "ocx-claude-refusal-")); + process.env.OPENCODEX_HOME = testDir; +}); + +afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + isolatedCodexHome?.restore(); + isolatedCodexHome = null; + if (testDir) removeTreeWithRetry(testDir); +}); + +async function send( + errorType: string, + stream: boolean, + message = `upstream says ${errorType}`, +): Promise<{ status: number; calls: number; text: string }> { + let calls = 0; + const frames = 'data: {"type":"response.in_progress"}\n\n' + + `data: ${JSON.stringify({ type: "error", error: { type: errorType, message } })}\n\n`; + const upstream = Bun.serve({ + port: 0, + fetch() { + calls++; + return new Response(frames, { headers: { "content-type": "text/event-stream" } }); + }, + }); + saveConfig({ + port: 0, + defaultProvider: "native", + providers: { + native: { + adapter: "openai-responses", + baseUrl: `${upstream.url.toString().replace(/\/$/, "")}/v1`, + authMode: "forward", + allowPrivateNetwork: true, + }, + }, + } as OcxConfig); + const server = startServer(0); + try { + const response = await fetch(new URL("/v1/messages", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "native/gpt-test", max_tokens: 64, stream, messages: [{ role: "user", content: "hi" }] }), + }); + return { status: response.status, calls, text: await response.text() }; + } finally { + await server.stop(true); + await upstream.stop(true); + } +} + +test.each([ + ["invalid_request_error", 400], + ["authentication_error", 401], + ["permission_error", 403], + ["not_found_error", 404], +])("non-streaming: a declared %s answers %i, not a retryable 502", async (errorType, status) => { + const result = await send(errorType, false); + expect(result.status).toBe(status); + expect(JSON.parse(result.text).error).toMatchObject({ type: errorType, message: `upstream says ${errorType}` }); + expect(result.calls).toBe(1); +}, 15_000); + +test.each(["invalid_request_error", "permission_error"])( + "streaming: a declared %s reaches the client as that type, not a retryable overload", + async (errorType) => { + const result = await send(errorType, true); + expect(result.status).toBe(200); + const data = JSON.parse(result.text.split("event: error\ndata: ")[1]!.split("\n")[0]!); + expect(data.error.type).toBe(errorType); + }, + 15_000, +); + +test("server-class overload keeps the 502 it had", async () => { + const result = await send("overloaded_error", false); + expect(result.status).toBe(502); +}, 15_000); + +test("non-streaming: an invalid_request_error worded as a client close answers 499, as it is logged", async () => { + const result = await send("invalid_request_error", false, "client closed request during web-search"); + expect(result.status).toBe(499); + expect((await send("permission_error", false, "client closed request")).status).toBe(403); +}, 15_000); + +test("a codeless invalid_request_error worded as a client close stays 499", () => { + expect(httpStatusFromTerminalError({ type: "invalid_request_error", message: "client closed request" })).toBe(499); + expect(httpStatusFromTerminalError({ type: "invalid_request_error", message: "bad shape" })).toBe(400); + // Only the recognized 400 yields to client-closed wording; other classes keep their dev order. + expect(httpStatusFromTerminalError({ type: "permission_error", message: "client closed request" })).toBe(403); + expect(httpStatusFromTerminalError({ type: "rate_limit_error", message: "client closed request" })).toBe(429); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index f5047535934..a04ae609cbd 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -304,7 +304,7 @@ "citation-markers.test.ts": "responses", "visualization-directives.test.ts": "responses", "cl01-claude-outbound-review-regressions.test.ts": "routing", "cl01-openai-chat-review-regressions.test.ts": "routing", "cl01-review-regressions.test.ts": "routing", - "claude-529-mapping.test.ts": "claude-integration", "claude-agent-startup-sync.test.ts": "claude-integration", + "claude-529-mapping.test.ts": "claude-integration", "claude-client-refusal-status.test.ts": "claude-integration", "claude-agent-startup-sync.test.ts": "claude-integration", "claude-agents-inject-client.test.ts": "claude-integration", "claude-agents-inject.test.ts": "claude-integration", "claude-alias.test.ts": "claude-integration", "claude-auth-detect.test.ts": "claude-integration", "claude-auth-mode.test.ts": "claude-integration", "claude-authmode-migration.test.ts": "claude-integration",