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
100 changes: 100 additions & 0 deletions devlog/_plan/261012_6740_ws_error_status/010_plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
# 6740 — keep the Codex WebSocket error frame's own status and terminal kind

Issue: lidge-jun/opencodex#6740. Base: origin/dev 7af092ca4f.
Prior work on the same issue: #6889 (v2.83.0, recognized bare-error classes keep 429/503),
#6860 (v2.82.0, sliding failure window + opt-in WS reuse). The reporter runs 2.80.0, which has neither.

## What the reported row means (verified by source reading)

Reported row: status 502, errorCode upstream_server_error, failureStage protocol-prelude,
failureCause upstream-fault, terminalSource synthetic, codexWsStage sent=true, upstreamFrames 5-6,
relayedEvents 3-4, closeCode null, attempts 1, upstreamError "An error occurred while processing your request...".

1. `src/server/responses/codex-ws-exchange.ts` onMessage relays every non-control `response.*` or
`error` frame as SSE and ends the exchange on `response.completed|failed|incomplete|error`
(line ~548). A post-commit `error` frame is relayed verbatim; `wrappedRejectionResponse` converts
only a pre-commit 4xx.
2. The canonical passthrough eager relay (`passthrough-delivery.ts` onSynthetic, reason
`upstream_error`, ~1142) synthesizes the failed terminal for a bare `error` and records
`terminalSource=synthetic` WITHOUT touching transportPhase, at `logCtx.terminalHttpStatus ?? 502`.
3. `request-log-terminal-status.ts` captureTerminalHttpStatus sets a provisional status for a bare
`error` only when `recognizedUpstreamError` matches (rate-limit/overload codes). The wrapper's
`status`/`status_code` is ignored, so the row stays 502.
4. `request-failure-attribution.ts`: synthetic + no mid_stream/terminal_sse phase + 502 gives
upstream-fault; terminalStatus failed + no output gives protocol-prelude.
A socket death instead sets transportPhase mid_stream/streamAborted (transport-ambiguous) and a
close-code message; the reported closeCode=null and upstream-fault exclude that.

So, on the ordinary canonical streaming path, the reported signature is produced by an upstream
bare `error` frame that this proxy relays and then re-labels 502 (source-based conclusion; the
counters do not name the preceding events, and the wrapper status/backend cause are unobserved). The native client (openai/codex
codex-rs/codex-api/src/endpoint/responses_websocket.rs map_wrapped_websocket_error_event) maps the
same frame's `status` to an HTTP transport error of that status. combo-stream-preflight
`bareErrorStatus` already reads the explicit status; the canonical path does not.

## Change (platform independent)

A. `src/server/request-log-terminal-status.ts`: in the `type === "error"` branch, after the
recognized-class check, read the Codex WebSocket wrapper status (top-level `status` or its alias
`status_code`; both present -> ignore, matching wrappedRejectionResponse) and note it provisionally
when it is an integer in 500..599. 4xx is excluded on purpose: 429/402/401/403 move quota and
credential routing and need their own review. Every 5xx is the same `transient` class in
`classifyCodexUpstreamOutcome`, so account health, the #6860 failure window and rotation are
unchanged; only the recorded status/errorCode/failureCause become truthful (503 ->
server_is_overloaded / upstream-declined; 500 -> 500 / upstream-fault). A genuine later terminal
still supersedes the provisional value. Client-facing tail is unchanged (native parity: native
maps a 503 without an overload code to an unexpected-status retry, ours keeps upstream_server_error).
B. `CodexWsStageRecord` (`codex-ws-wire.ts`) gains two content-free fields filled in
`codex-ws-exchange.ts`: `terminalEvent` ("response.completed" | "response.failed" |
"response.incomplete" | "error" | null — the normalized type of the frame that ended the exchange,
observed before any synthetic tail) and `terminalErrorStatus` (wrapper status of a terminal
`error` frame, integer 100..599, else null). `src/usage/log.ts` normalizes them (older rows read
as null; invalid values are dropped to null, never failing the row).
C. Docs: `structure/dashboard-and-usage.md` (bare-error status rule), `structure/transports/
responses-wire-shapes.md` if it lists stage fields, docs-site management-api reference if it lists
codexWsStage fields.

Out of scope: retry/replay changes, account auto-quarantine, client tail codes, 4xx wrapper status,
Windows-only transport changes.

## Verification

- New focused tests: bare `error` with `status`/`status_code` 503/500 -> provisional status,
superseded by a genuine terminal, ignored for 4xx/invalid/both-fields; exchange stage record carries
terminalEvent/terminalErrorStatus for error/failed/completed; usage log round-trip and legacy rows.
- bun test on touched suites, bun run test:changed, bun run typecheck, bun run structure:check,
bun run privacy:scan, file-size ratchet.

## Audit round 1 (GPT-6.1-Sol high): NEAR-PASS, folded

1. Native control can end on acknowledgement frames outside the enum: terminalEvent/terminalErrorStatus
stay null for nativeControl exchanges.
2. Plan wording made conditional (above).
3. Change A uses only noteStatus() after recognition fails; never a default 502; policy precedence,
existing-status guard, genuine-terminal retirement and late-bare-error guard unchanged. Only the
native wrapper contract is read: top-level integer `status` or `status_code` (not both); nested
status, http_status and numeric strings are ignored. 4xx stays excluded.
4. Buffered JSON callers (buffered-sse-json.ts / passthrough-delivery presentation) keep their own
status derivation; this patch is streaming-path only and documents that.
5. Normalization keeps legacy stages; malformed new values become null without dropping the stage.
Pre-commit 4xx rejections still carry no stage record (unchanged, documented).
6. Docs: structure/dashboard-and-usage.md, structure/transports/responses-wire-shapes.md (status rule
and stage field list), docs-site reference/management-api.md. New tests go into existing uncapped
suites (upstream-failure-diagnostics, ws-failure-stage, usage-log-ws-stage); no layout changes.

## Not verifiable here (inference)

Whether the backend's frames carry `status` at all, and which value, needs a Windows/real-account
row from a build with this change. Upstream root cause (account/backend admission) is not provable
from source.

## Implementation and review (B/C)

- Change A: `codexWsWrapperStatus` (codex-ws-wire.ts) reads the native wrapper contract;
`wrappedServerErrorStatus` (request-log-terminal-status.ts) adopts only 5xx after recognition fails.
- Change B: `terminalEvent` / `terminalErrorStatus` captured in the accepted non-control branch of
codex-ws-exchange.ts, before relay-size/queue checks (review round 1 finding 1), null on native control.
- Review round 1 (GPT-6.1-Sol high): NEAR-PASS — capture position, missing terminal cases, docs-site
streaming qualification; all fixed with regressions that fail on the prior code.
- Focused: 7 suites 413 pass / 1 skip / 0 fail. test:changed selected 1672 files; 37 fails in unrelated
service/Lab/audio/toggle files that pass 430/430 alone on both base and change (load-dependent).
5 changes: 4 additions & 1 deletion docs-site/src/content/docs/reference/management-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,10 @@ Failed request rows can include bounded `upstreamErrorType`, `upstreamErrorCode`
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; explicit unknown codes remain generic 502 failures. Bare-error status is provisional:
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:
a genuine completed, failed, or incomplete terminal replaces it before account-health recording.
Refusal precedence is unchanged.
This diagnostic mapping adds no automatic replay or retry.
Expand Down
26 changes: 24 additions & 2 deletions src/server/request-log-terminal-status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
import { redactSecretString } from "../lib/redact";
import { outboundCredentials, type OutboundCredentialRegistry } from "../lib/outbound-credential-registry";
import { configuredCredentials, type ConfiguredCredentials } from "./configured-credentials";
import { codexWsWrapperStatus } from "./responses/codex-ws-wire";

/** First bounded upstream diagnostics shared by request contexts and final rows. */
export interface UpstreamErrorDiagnostics {
Expand All @@ -26,6 +27,9 @@ export interface UpstreamErrorPayload {
type?: unknown;
code?: unknown;
message?: unknown;
/** Codex WebSocket wrapper status (`status_code` is its alias); read only on bare `error` events. */
status?: unknown;
status_code?: unknown;
error?: { type?: unknown; code?: unknown; message?: unknown };
last_error?: { type?: unknown; code?: unknown; message?: unknown };
response?: {
Expand Down Expand Up @@ -136,6 +140,17 @@ export function upstreamDiagnosticLogFields(logCtx: UpstreamErrorDiagnostics): U
};
}

/**
* The server status a Codex WebSocket `error` frame states on its wrapper ({@link codexWsWrapperStatus}),
* or undefined. Only 5xx is returned. Every 5xx is the same transient class for account health, so
* this changes what a row says, not where the next turn goes; a wrapper 4xx would move quota and
* credential routing and is left to its own review (#6740).
*/
export function wrappedServerErrorStatus(json: UpstreamErrorPayload): number | undefined {
const status = codexWsWrapperStatus(json as Record<string, unknown>);
return status !== undefined && status >= 500 ? status : undefined;
}

function captureTerminalHttpStatus(
logCtx: TerminalStatusContext,
json: UpstreamErrorPayload,
Expand Down Expand Up @@ -205,8 +220,15 @@ function captureTerminalHttpStatus(
code: candidates.find(candidate => typeof candidate?.code === "string")?.code,
type: candidates.slice(0, -1).find(candidate => typeof candidate?.type === "string")?.type,
});
// Unrecognized bare errors leave the status to explicit event fields (combo preflight).
if (structured) noteStatus(structured.httpStatus);
if (structured) {
noteStatus(structured.httpStatus);
return;
}
// An unrecognized class keeps only the status the upstream itself stated on the WebSocket
// wrapper. Nothing is invented here: with no wrapper status the row stays the synthetic 502, and
// combo preflight still reads its wider set of explicit event fields after this one.
const wrapped = wrappedServerErrorStatus(json);
if (wrapped !== undefined) noteStatus(wrapped);
return;
}
if (type !== "response.failed" || !responseError || typeof responseError !== "object") return;
Expand Down
14 changes: 13 additions & 1 deletion src/server/responses/codex-ws-exchange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import type { CodexWsSession } from "./codex-ws-session";
import { UPGRADE_DEADLINE_MS, CODEX_WS_LIVENESS_PING_INTERVAL_MS, CODEX_WS_RESPONSE_PRELUDE_TIMEOUT_MS, MAX_CODEX_WS_FRAME_BYTES,
MAX_CODEX_WS_QUEUE_BYTES, markCodexWsResponse, markCodexWsPreludeProjection, normalizeResponsesWsRelayEvent, closedBeforeTerminalMessage,
codexWsCreateFrameExceedsLimit, codexWsFailureDetail, codexWsPreResponseFailure, markCodexWsStage, codexWsOcxVersion,
markCodexWsSocketDeath, type CodexWsFailureStage, type CodexWsStageRecord } from "./codex-ws-wire";
markCodexWsSocketDeath, codexWsWrapperStatus, isCodexWsTerminalEvent, type CodexWsFailureStage, type CodexWsStageRecord,
type CodexWsTerminalEvent } from "./codex-ws-wire";

interface ExchangeOptions {
nativeControl?: NativeResponseControl;
Expand Down Expand Up @@ -120,6 +121,9 @@ export function codexWsExchange(options: ExchangeOptions): Promise<Response> {
// Numeric close code for the durable stage record; the reason string stays
// out of it on purpose (#4191 content-free contract).
let closeCode: number | null = null;
// #6740: which terminal frame the upstream itself sent, before any synthetic tail.
let terminalEvent: CodexWsTerminalEvent | null = null;
let terminalErrorStatus: number | null = null;
let controller: ReadableStreamDefaultController<Uint8Array> | null = null;
const encoder = new TextEncoder();
const metadata = url === CODEX_RESPONSES_HTTP_URL ? new CodexWsMetadata(onQuota) : null;
Expand Down Expand Up @@ -200,6 +204,8 @@ export function codexWsExchange(options: ExchangeOptions): Promise<Response> {
reused: session.reused,
ocxVersion: codexWsOcxVersion(),
bunVersion: bunVersion ?? "unknown",
terminalEvent,
terminalErrorStatus,
});

const commitResponse = () => {
Expand Down Expand Up @@ -521,6 +527,12 @@ export function codexWsExchange(options: ExchangeOptions): Promise<Response> {
}
}
commitResponse();
// Record the terminal the upstream sent as soon as it is accepted, before the relay-size and
// queue checks below: a body failure that follows is still an exchange that received it.
if (!nativeControl && isCodexWsTerminalEvent(type)) {
terminalEvent = type;
terminalErrorStatus = type === "error" ? (codexWsWrapperStatus(normalized.payload) ?? null) : null;
}
}
const prefix = encoder.encode(`event: ${type}\ndata: `);
const suffix = encoder.encode("\n\n");
Expand Down
42 changes: 42 additions & 0 deletions src/server/responses/codex-ws-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,50 @@ export type CodexWsStageRecord = Omit<CodexWsFailureStage, "requestBytes"> & {
ocxVersion: string;
/** Bun runtime version the exchange gated on. */
bunVersion: string;
/**
* Normalized type of the Responses terminal frame that ended a plain exchange, as the upstream
* sent it and before any tail this proxy synthesizes (#6740). It is what separates an upstream
* `error` frame from a genuine `response.failed` when both reach a row as the same synthetic 502.
* Null when no terminal frame arrived, and on native-control exchanges, which can end on an
* acknowledgement instead. Records written before it existed read as null.
*/
terminalEvent: CodexWsTerminalEvent | null;
/** Wrapper status of a terminal `error` frame ({@link codexWsWrapperStatus}); null otherwise. */
terminalErrorStatus: number | null;
};

/** The frames a plain exchange ends on, closed so a stage record can never carry free text. */
export const CODEX_WS_TERMINAL_EVENTS = [
"response.completed",
"response.failed",
"response.incomplete",
"error",
] as const;
export type CodexWsTerminalEvent = typeof CODEX_WS_TERMINAL_EVENTS[number];

export function isCodexWsTerminalEvent(value: unknown): value is CodexWsTerminalEvent {
return typeof value === "string" && (CODEX_WS_TERMINAL_EVENTS as readonly string[]).includes(value);
}

/**
* The HTTP status a Codex WebSocket `error` frame states on its wrapper, or undefined.
*
* The backend wraps a failure as `{type:"error", status|status_code, error}` and the native client
* turns that status into an HTTP error of the same status
* (codex-rs/codex-api/src/endpoint/responses_websocket.rs map_wrapped_websocket_error_event). Only
* that contract is read: a top-level integer in 100..599 under exactly one of the two aliases, the
* same rule the pre-commit rejection projection applies.
*/
export function codexWsWrapperStatus(payload: Record<string, unknown>): number | undefined {
if (payload.type !== "error") return undefined;
const hasStatus = Object.hasOwn(payload, "status");
if (hasStatus === Object.hasOwn(payload, "status_code")) return undefined;
const status = hasStatus ? payload.status : payload.status_code;
return typeof status === "number" && Number.isInteger(status) && status >= 100 && status <= 599
? status
: undefined;
}

const codexWsStageByResponse = new WeakMap<Response, CodexWsStageRecord>();

export function markCodexWsStage(response: Response, record: CodexWsStageRecord): void {
Expand Down
20 changes: 19 additions & 1 deletion src/usage/log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { normalizeRouteDecisionTrace, type RouteDecisionTraceV1 } from "../routi
import { parseProtocolTraceV1, type ProtocolTraceV1 } from "../protocols/dto";
import { ACCOUNT_LOG_LABEL_RE, CODEX_ACCOUNT_LOG_LABEL_RE } from "../codex/account-label";
import { claudeCompatibilityReason, normalizeClaudeFeatureCodes, type ClaudeFeatureCode } from "../claude/compatibility";
import type { CodexWsStageRecord } from "../server/responses/codex-ws-wire";
import type { CodexWsStageRecord, CodexWsTerminalEvent } from "../server/responses/codex-ws-wire";
import {
ATTEMPT_RECOVERY_KIND_ROSTER,
ATTEMPT_RECOVERY_WITHHELD_ROSTER,
Expand Down Expand Up @@ -805,6 +805,15 @@ function normalizeUsageAttempt(raw: unknown): PersistedUsageAttempt | null {
* injected free-form field — drops the whole record rather than passing
* attacker text into the DTO.
*/
// Total over the terminal union by `satisfies`; a type-only import keeps the WS transport out of
// this module's runtime graph.
const CODEX_WS_TERMINAL_EVENT_NAMES = {
"response.completed": true,
"response.failed": true,
"response.incomplete": true,
error: true,
} as const satisfies Record<CodexWsTerminalEvent, true>;

function normalizeCodexWsStageRecord(value: unknown): CodexWsStageRecord | undefined {
if (value === null || typeof value !== "object" || Array.isArray(value)) return undefined;
const stage = value as Record<string, unknown>;
Expand Down Expand Up @@ -839,6 +848,15 @@ function normalizeCodexWsStageRecord(value: unknown): CodexWsStageRecord | undef
reused: stage.reused,
ocxVersion: stage.ocxVersion,
bunVersion: stage.bunVersion,
// #6740 fields are newer than the record: absent or malformed values read as unobserved
// rather than costing the row the rest of its stage evidence.
terminalEvent: typeof stage.terminalEvent === "string" && Object.hasOwn(CODEX_WS_TERMINAL_EVENT_NAMES, stage.terminalEvent)
? stage.terminalEvent as CodexWsTerminalEvent
: null,
terminalErrorStatus: typeof stage.terminalErrorStatus === "number" && Number.isInteger(stage.terminalErrorStatus)
&& stage.terminalErrorStatus >= 100 && stage.terminalErrorStatus <= 599
? stage.terminalErrorStatus
: null,
};
}

Expand Down
Loading
Loading