From 57f2d4de231146ff4df98a2cb14fae52e4cf0302 Mon Sep 17 00:00:00 2001 From: Codex Date: Mon, 24 Aug 2026 18:18:59 +0900 Subject: [PATCH 01/19] fix: normalize Responses terminal failures --- src/server/relay-eager.ts | 138 ++++++++- src/server/relay.ts | 220 +++++++++++++-- src/server/request-log.ts | 50 +++- src/server/sse-frame-buffer.ts | 35 ++- tests/passthrough-abort.test.ts | 350 ++++++++++++++++++++++- tests/relay-eager.test.ts | 480 +++++++++++++++++++++++++++++++- tests/request-log.test.ts | 128 +++++++++ 7 files changed, 1350 insertions(+), 51 deletions(-) diff --git a/src/server/relay-eager.ts b/src/server/relay-eager.ts index 8865a99233..ea36b34bf6 100644 --- a/src/server/relay-eager.ts +++ b/src/server/relay-eager.ts @@ -81,6 +81,10 @@ export type EagerRelayOptions = { const DEFAULT_MAX_QUEUE_BYTES = 8 * 1024 * 1024; const DEFAULT_DRAIN_MS = 15_000; const DEFAULT_DRAIN_BYTES = 32 * 1024 * 1024; +const ADAPTER_EOF_INCOMPLETE_FRAME = new TextEncoder().encode( + 'event: response.incomplete\ndata: {"type":"response.incomplete","response":{"status":"incomplete","incomplete_details":{"reason":"adapter_eof"}}}\n\n', +); +const DONE_FRAME = new TextEncoder().encode("data: [DONE]\n\n"); /** * Relay `body` to the returned stream with eager bounded reading and inline @@ -103,6 +107,19 @@ export function relaySseEagerBounded( const terminalBoundary = createSseTerminalOutputBoundary(); const activeRewrite: SseBlockRewrite | undefined = hooks.rewriteBlocks ?? (hooks.rewritePayload ? payloadRewriteAsBlockRewrite(hooks.rewritePayload) : undefined); + const encodeFailedTail = (error: unknown): Uint8Array | null => { + try { + return new TextEncoder().encode( + `\n\nevent: response.failed\ndata: ${buildFailedTailPayload(error)}\n\ndata: [DONE]\n\n`, + ); + } catch { + // A hostile error accessor may throw while building the diagnostic. Keep + // the client contract bounded even then; callers still re-check abort. + return new TextEncoder().encode( + '\n\nevent: response.failed\ndata: {"type":"response.failed","response":{"status":"failed","error":{"type":"upstream_error","code":"upstream_reset","message":"Upstream stream terminated unexpectedly"},"last_error":{"type":"upstream_error","code":"upstream_reset","message":"Upstream stream terminated unexpectedly"}}}\n\ndata: [DONE]\n\n', + ); + } + }; const rewriteDecoder = activeRewrite ? new TextDecoder() : null; const rewriteEncoder = activeRewrite ? new TextEncoder() : null; const rewriteBudget = opts?.rewriteBudget; @@ -196,6 +213,8 @@ export function relaySseEagerBounded( const producer = async () => { let syntheticKind: "incomplete" | "failed" | null = null; + let priorRewriteFailure = false; + let priorRewriteError: unknown; // reader.read() is not intrinsically tied to the upstream AbortController // (a fetch body usually rejects on abort, but that coupling is the fetch // implementation's, not the stream's), so abort must break a parked read on @@ -221,18 +240,46 @@ export function relaySseEagerBounded( if (upstreamDone) { hooks.finishInspection(); const boundedTail = terminalBoundary.finish(); + let clientTail = boundedTail; + let rewriteFailed = false; + let rewriteError: unknown; if (activeRewrite) { - const rewritten = rewriteOutbound(boundedTail); - const tail = joinUint8Arrays(rewritten, flushRewriteTail()); - if (tail.byteLength > 0 && !cancelled) { - queuedBytes += tail.byteLength; - try { controllerRef?.enqueue(tail); } catch { /* client already gone */ } + try { + const rewritten = rewriteOutbound(boundedTail); + clientTail = joinUint8Arrays(rewritten, flushRewriteTail()); + } catch (error) { + rewriteFailed = true; + rewriteError = error; + clientTail = new Uint8Array(0); } - } else if (boundedTail.byteLength > 0 && !cancelled) { - queuedBytes += boundedTail.byteLength; - try { controllerRef?.enqueue(boundedTail); } catch { /* client already gone */ } } - if (!hooks.sawTerminal() && !cancelled && !upstream.signal.aborted) { + if (rewriteFailed) { + const safeTail = encodeFailedTail(rewriteError); + if (safeTail && !cancelled && !upstream.signal.aborted) { + if (!hooks.sawTerminal()) syntheticKind = "failed"; + queuedBytes += safeTail.byteLength; + try { controllerRef?.enqueue(safeTail); } catch { /* client already torn down */ } + try { controllerRef?.close(); } catch { /* client already gone */ } + } + break; + } + if (clientTail.byteLength > 0 && !cancelled) { + queuedBytes += clientTail.byteLength; + try { controllerRef?.enqueue(clientTail); } catch { /* client already gone */ } + } + if (terminalBoundary.terminalSeen()) { + if (!terminalBoundary.doneSeen() && !cancelled) { + queuedBytes += terminalSentinel.byteLength; + try { controllerRef?.enqueue(terminalSentinel); } catch { /* client already gone */ } + } + } else if (!hooks.sawTerminal() && !cancelled && !upstream.signal.aborted) { + // A clean 200 EOF without a Responses terminal must be visible to + // Codex as one incomplete turn, followed by the normal sentinel. + queuedBytes += ADAPTER_EOF_INCOMPLETE_FRAME.byteLength + DONE_FRAME.byteLength; + try { + controllerRef?.enqueue(ADAPTER_EOF_INCOMPLETE_FRAME); + controllerRef?.enqueue(DONE_FRAME); + } catch { /* client already gone */ } syntheticKind = "incomplete"; } break; @@ -247,7 +294,21 @@ export function relaySseEagerBounded( continue; } const terminalBounded = terminalBoundary.feed(value); - const outbound = activeRewrite ? rewriteOutbound(terminalBounded) : terminalBounded; + let outbound: Uint8Array; + if (activeRewrite) { + try { + outbound = rewriteOutbound(terminalBounded); + } catch (error) { + // Preserve the first rewrite failure across the teardown flush. A + // second empty flush may succeed, but the terminal bytes still + // must not bypass the failed rewrite or become DONE-only output. + priorRewriteFailure = true; + priorRewriteError = error; + throw error; + } + } else { + outbound = terminalBounded; + } if (outbound.byteLength > 0) { queuedBytes += outbound.byteLength; try { @@ -278,7 +339,62 @@ export function relaySseEagerBounded( } catch (err) { // Upstream read failure. Distinguish genuine mid-stream reset from // abort-driven teardown (shutdown/cancel-expiry) — audit M3. - if (!hooks.sawTerminal() && !cancelled && !upstream.signal.aborted) { + // A read can fail after delivering an unterminated terminal block. Flush + // both observers before deciding whether this is a synthetic reset so + // eager mode matches the tee/pull boundary semantics at EOF. + let boundedTail: Uint8Array = new Uint8Array(0); + let tailTerminal = false; + let tailDone = false; + try { hooks.finishInspection(); } catch { /* preserve the original read failure */ } + try { + boundedTail = terminalBoundary.finish(); + tailTerminal = terminalBoundary.terminalSeen(); + tailDone = terminalBoundary.doneSeen(); + } catch { + // A near-cap ambiguous delimiter tail may itself overflow at EOF. + // Preserve the original read/framing failure and continue emitting + // the bounded failed tail instead of letting cleanup throw again. + } + let clientTail: Uint8Array = boundedTail; + let rewriteFailed = false; + let rewriteError: unknown; + if (priorRewriteFailure) { + rewriteFailed = true; + rewriteError = priorRewriteError; + clientTail = new Uint8Array(0); + } else if (activeRewrite) { + try { + const rewritten = rewriteOutbound(boundedTail); + clientTail = joinUint8Arrays(rewritten, flushRewriteTail()); + } catch (error) { + rewriteFailed = true; + rewriteError = error; + clientTail = new Uint8Array(0); + } + } + if (clientTail.byteLength > 0 && !cancelled && !upstream.signal.aborted) { + queuedBytes += clientTail.byteLength; + try { controllerRef?.enqueue(clientTail); } catch { /* client already torn down */ } + } + if (rewriteFailed && !cancelled && !upstream.signal.aborted) { + // Never bypass a client rewrite after it fails: boundedTail can contain + // provider metadata or content that the active rewrite was required to + // remove. Emit one safe failed envelope instead. When inspection has + // already reported the real upstream terminal, this is a delivery + // fallback only and must not create a second accounting outcome. + const safeTail = encodeFailedTail(rewriteError ?? err); + if (safeTail && !cancelled && !upstream.signal.aborted) { + if (!hooks.sawTerminal()) syntheticKind = "failed"; + queuedBytes += safeTail.byteLength; + try { controllerRef?.enqueue(safeTail); } catch { /* client already torn down */ } + try { controllerRef?.close(); } catch { /* client already gone */ } + } + } else if (tailTerminal && !cancelled && !upstream.signal.aborted) { + if (!tailDone) { + queuedBytes += terminalSentinel.byteLength; + try { controllerRef?.enqueue(terminalSentinel); } catch { /* client already gone */ } + } + } else if (!tailTerminal && !hooks.sawTerminal() && !cancelled && !upstream.signal.aborted) { // Serializing `err` can run user-defined accessors (Error.message // getters, toString) that re-entrantly cancel the client or abort the // upstream. Build the tail FIRST, then re-check eligibility before diff --git a/src/server/relay.ts b/src/server/relay.ts index 3cbc870a79..299481cc39 100644 --- a/src/server/relay.ts +++ b/src/server/relay.ts @@ -1,4 +1,9 @@ import type { ResponsesTerminalStatus } from "../bridge"; +import { + CYBER_POLICY_ERROR_CODE, + isCyberPolicyCode, + isCyberPolicyMessage, +} from "../lib/errors"; import { isTranslatorBudgetExceededError } from "../lib/translator-budget"; import { isUsageDebugEnabled } from "../usage/debug"; import { @@ -16,6 +21,7 @@ import { joinSseFrameBytes, MAX_CLIENT_SSE_FRAME_BYTES, } from "./sse-frame-buffer"; +import { replaceSseDataPayload } from "./sse-payload-rewrite"; const nativePassthroughSseResponses = new WeakSet(); const eagerRelaySseResponses = new WeakSet(); @@ -24,6 +30,13 @@ export const MAX_INSPECTION_SSE_FRAME_BYTES = MAX_CLIENT_SSE_FRAME_BYTES; export const MAX_COMPLETED_OUTPUT_ITEMS = 256; export const MAX_COMPLETED_OUTPUT_ITEM_SOURCE_BYTES = 8 * 1024 * 1024; export const MAX_TAIL_ERROR_MESSAGE_CHARS = 512; +const ADAPTER_EOF_INCOMPLETE_PAYLOAD = JSON.stringify({ + type: "response.incomplete", + response: { + status: "incomplete", + incomplete_details: { reason: "adapter_eof" }, + }, +}); export type InspectionCounters = { frameBufferHighWaterBytes: number; @@ -111,13 +124,16 @@ export type SseTerminalOutputBoundary = { * Frame-aware client output boundary shared by both native Responses relays. * It buffers only the current incomplete SSE block under the same hard byte * cap as inspection, forwards complete blocks through the first Responses - * terminal, and drops every later block/byte. + * terminal, and drops every later block/byte. A premature [DONE] is held until + * a terminal arrives so clean EOF can synthesize one terminal and one sentinel. */ export function createSseTerminalOutputBoundary(): SseTerminalOutputBoundary { const decoder = new TextDecoder(); + const encoder = new TextEncoder(); const framer = new BoundedSseFrameBuffer(MAX_INSPECTION_SSE_FRAME_BYTES); let terminal = false; let done = false; + let pendingDone: { block: Uint8Array; delimiter: Uint8Array } | null = null; let disposed = false; const processFrames = ( @@ -129,16 +145,37 @@ export function createSseTerminalOutputBoundary(): SseTerminalOutputBoundary { for (const frame of frames) { const payload = sseDataPayload(decoder.decode(frame.block)); const isDone = payload === "[DONE]"; - // Preserve every frame through the first Responses terminal. A [DONE] - // frame is also preserved when it immediately follows that terminal in - // the same upstream chunk; every later non-DONE frame is dropped. - if (!responsesTerminal || isDone) output.push(frame.block, frame.delimiter); + const parsed = payload === null ? undefined : parseSsePayload(payload); + const policyMessage = parsed !== undefined && isPolicyRewriteType(parsed) + ? cyberPolicyTerminalMessage(parsed) + : undefined; + const outboundBlock = policyMessage + ? encoder.encode(rewritePolicyTerminalBlock( + decoder.decode(frame.block), + policyFailurePayload(policyMessage, parsed), + )) + : frame.block; if (isDone) { done = true; + if (responsesTerminal) { + output.push(outboundBlock, frame.delimiter); + } else if (!pendingDone) { + // Do not expose a sentinel before a Responses terminal. If EOF + // follows, the synthetic incomplete path owns the one sentinel; + // if a terminal arrives later, this pending frame is emitted then. + pendingDone = { block: outboundBlock, delimiter: frame.delimiter }; + } continue; } - if (!responsesTerminal && payload && terminalStatusFromSsePayload(payload)) { + // Preserve every frame through the first Responses terminal. Every + // later non-DONE frame is dropped. + if (!responsesTerminal) output.push(outboundBlock, frame.delimiter); + if (!responsesTerminal && payload && terminalStatusFromParsed(parsed)) { responsesTerminal = true; + if (pendingDone) { + output.push(pendingDone.block, pendingDone.delimiter); + pendingDone = null; + } } } if (responsesTerminal) { @@ -155,13 +192,22 @@ export function createSseTerminalOutputBoundary(): SseTerminalOutputBoundary { }, finish() { if (disposed || terminal) return new Uint8Array(0); - return framer.finish(); + const tail = framer.finish(); + if (tail.byteLength === 0) return new Uint8Array(0); + // EOF may cut off the final SSE block before its blank-line delimiter. + // Feed it through the exact same parser/rewrite/terminal path as a + // complete frame, using a synthetic delimiter so the client receives a + // dispatchable event rather than an unterminated tail. + const tailText = decoder.decode(tail); + const delimiter = encoder.encode(tailText.includes("\r\n") ? "\r\n\r\n" : "\n\n"); + return processFrames([{ block: tail, delimiter }]); }, terminalSeen: () => terminal, doneSeen: () => done, dispose() { if (disposed) return; disposed = true; + pendingDone = null; framer.dispose(); }, }; @@ -219,6 +265,16 @@ export function relaySseWithFailedTail( if (done) { const tail = terminalBoundary.finish(); if (tail.byteLength > 0) controller.enqueue(tail); + if (terminalBoundary.terminalSeen()) { + if (!terminalBoundary.doneSeen()) controller.enqueue(doneFrame(encoder)); + } else { + // A clean upstream EOF is still a failed Responses turn when no + // protocol terminal arrived. Make that state explicit so Codex + // does not treat HTTP 200 + bare EOF as a retryable disconnect. + const incomplete = adapterEofIncompleteFrame(encoder); + controller.enqueue(incomplete); + controller.enqueue(doneFrame(encoder)); + } terminalBoundary.dispose(); controller.close(); return; @@ -228,8 +284,10 @@ export function relaySseWithFailedTail( } } catch (err) { let partial: Uint8Array = new Uint8Array(0); + let tailTerminal = false; try { partial = terminalBoundary.finish(); + tailTerminal = terminalBoundary.terminalSeen(); } catch { // A near-cap ambiguous delimiter tail may itself overflow at EOF. // Preserve the original read/framing failure and continue emitting @@ -237,11 +295,15 @@ export function relaySseWithFailedTail( } terminalBoundary.dispose(); if (closed) return; - const payload = buildFailedTailPayload(err); try { if (partial.byteLength > 0) controller.enqueue(partial); - // Leading blank line terminates a partial SSE block so the failed frame parses cleanly. - controller.enqueue(encoder.encode(`\n\nevent: response.failed\ndata: ${payload}\n\ndata: [DONE]\n\n`)); + if (tailTerminal) { + if (!terminalBoundary.doneSeen()) controller.enqueue(doneFrame(encoder)); + } else { + const payload = buildFailedTailPayload(err); + // Leading blank line terminates a partial SSE block so the failed frame parses cleanly. + controller.enqueue(encoder.encode(`\n\nevent: response.failed\ndata: ${payload}\n\ndata: [DONE]\n\n`)); + } controller.close(); } catch { /* client already torn down */ } upstream.abort(); @@ -276,15 +338,125 @@ export function sseDataPayload(block: string): string | null { return data.length > 0 ? data.join("\n") : null; } -export function terminalStatusFromSsePayload(payload: string): ResponsesTerminalStatus | null { - if (payload === "[DONE]") return null; +type JsonRecord = Record; + +function asJsonRecord(value: unknown): JsonRecord | null { + return value !== null && typeof value === "object" && !Array.isArray(value) + ? value as JsonRecord + : null; +} + +function stringField(record: JsonRecord | null, key: string): string | undefined { + const value = record?.[key]; + return typeof value === "string" && value.trim() ? value.trim() : undefined; +} + +/** + * Return a high-confidence policy error carried by an upstream terminal shape. + * Deliberately inspect structured error fields and known refusal copy only — a + * bare `cyber_policy` token in an unrelated payload is not sufficient. + */ +function cyberPolicyTerminalMessage(parsed: unknown): string | undefined { + const root = asJsonRecord(parsed); + if (!root) return undefined; + const response = asJsonRecord(root.response); + const candidates = [ + asJsonRecord(root.error), + asJsonRecord(root.last_error), + asJsonRecord(response?.error), + asJsonRecord(response?.incomplete_details), + root, + response, + ]; + for (const candidate of candidates) { + if (!candidate) continue; + if (isCyberPolicyCode(stringField(candidate, "code"))) { + return stringField(candidate, "message") + ?? "Request blocked by the upstream cybersecurity policy."; + } + } + for (const candidate of candidates) { + const message = stringField(candidate, "message"); + if (message && isCyberPolicyMessage(message)) return message; + } + return undefined; +} + +function parseSsePayload(payload: string): unknown | undefined { + if (payload === "[DONE]") return undefined; try { - return terminalStatusFromParsed(JSON.parse(payload)); + return JSON.parse(payload); } catch { - return null; + return undefined; } } +function isPolicyRewriteType(parsed: unknown): boolean { + const type = asJsonRecord(parsed)?.type; + return type === "response.failed" || type === "response.incomplete" || type === "error"; +} + +function rewritePolicyTerminalBlock(block: string, payload: string): string { + const newline = block.includes("\r\n") ? "\r\n" : "\n"; + const rewritten = replaceSseDataPayload(block, payload); + const lines = rewritten.split(/\r?\n/); + let eventRewritten = false; + const withEvent = lines.map(line => { + if (!eventRewritten && line.startsWith("event:")) { + eventRewritten = true; + return "event: response.failed"; + } + return line; + }); + if (!eventRewritten) withEvent.unshift("event: response.failed"); + return withEvent.join(newline); +} + +function policyFailurePayload(message: string, parsed: unknown): string { + const error = { + type: "invalid_request_error", + code: CYBER_POLICY_ERROR_CODE, + message: message.slice(0, MAX_TAIL_ERROR_MESSAGE_CHARS), + }; + const root = asJsonRecord(parsed); + const originalResponse = asJsonRecord(root?.response); + const response = { + ...(originalResponse ?? {}), + status: "failed", + error, + last_error: error, + }; + // Responses event metadata such as sequence_number is normally top-level. + // Keep it (and any other non-error, non-response fields) while replacing only + // the protocol type and error envelope. + const preservedRoot = root + ? Object.fromEntries(Object.entries(root).filter(([key]) => ( + key !== "type" + && key !== "response" + && key !== "error" + && key !== "last_error" + ))) + : {}; + return JSON.stringify({ + ...preservedRoot, + type: "response.failed", + response, + }); +} + +function adapterEofIncompleteFrame(encoder: TextEncoder): Uint8Array { + return encoder.encode(`event: response.incomplete\ndata: ${ADAPTER_EOF_INCOMPLETE_PAYLOAD}\n\n`); +} + +function doneFrame(encoder: TextEncoder): Uint8Array { + return encoder.encode("data: [DONE]\n\n"); +} + +export function terminalStatusFromSsePayload(payload: string): ResponsesTerminalStatus | null { + if (payload === "[DONE]") return null; + return terminalStatusFromParsed(parseSsePayload(payload)); +} + /** True when a native Responses SSE payload carries the FIRST kind of non-empty model output. */ export function isFirstOutputSsePayload(payload: string | null): boolean { if (!payload || payload === "[DONE]") return false; @@ -322,14 +494,16 @@ function createFirstOutputReporter(onFirstOutput?: () => void): { } export function terminalStatusFromParsed(parsed: unknown): ResponsesTerminalStatus | null { - if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return null; - switch ((parsed as { type?: unknown }).type) { + const type = asJsonRecord(parsed)?.type; + switch (type) { case "response.completed": return "completed"; case "response.failed": return "failed"; case "response.incomplete": - return "incomplete"; + return cyberPolicyTerminalMessage(parsed) ? "failed" : "incomplete"; + case "error": + return cyberPolicyTerminalMessage(parsed) ? "failed" : null; default: return null; } @@ -800,6 +974,9 @@ export function createSseInspector(handlers: SseInspectorHandlers): SseInspector } reportFirstOutput.parsed(parsed); const status = terminalStatusFromParsed(parsed); + const policyTerminal = status === "failed" + && isPolicyRewriteType(parsed) + && cyberPolicyTerminalMessage(parsed) !== undefined; if (status) sawTerminal = true; if (!reported && handlers.onTerminal && status) { try { @@ -808,7 +985,7 @@ export function createSseInspector(handlers: SseInspectorHandlers): SseInspector handlers.logCtx.transportPhase = "terminal_sse"; handlers.logCtx.terminalSource = "upstream"; } - handlers.onTerminal(status); + handlers.onTerminal(status, policyTerminal ? 400 : undefined); } finally { if (status === "failed" || status === "incomplete") clearCompletedItems(); } @@ -1087,6 +1264,13 @@ function startBoundedInspectionPump(options: InspectionPumpOptions): void { } } } catch { + // A read error can follow a final SSE block without its blank-line + // delimiter. Flush that candidate before classifying the transport as a + // synthetic reset; otherwise a real completed/failed/policy terminal is + // downgraded to 502 on the inspection branch. + if (!cancelled) { + try { inspector.finish(); } catch { /* preserve the original read error */ } + } if (clientGone) clientGoneWithoutTerminal = !inspector.terminalSeen(); else if (!cancelled) options.onReadError?.(); } finally { diff --git a/src/server/request-log.ts b/src/server/request-log.ts index 26a69a14ab..bc56319c98 100644 --- a/src/server/request-log.ts +++ b/src/server/request-log.ts @@ -6,6 +6,7 @@ import { httpStatusFromTerminalError as httpStatusFromClassifiedTerminalError, isClientClosedMessage, isCyberPolicyCode, + isCyberPolicyMessage, } from "../lib/errors"; import { CODEX_CONFIG_PATH, readRootTomlString } from "../codex/paths"; import { readCodexCatalogPath } from "../codex/catalog"; @@ -764,7 +765,7 @@ function captureUpstreamErrorParsed( last_error?: { message?: unknown }; response?: { error?: { type?: unknown; code?: unknown; message?: unknown }; - incomplete_details?: { reason?: unknown }; + incomplete_details?: { reason?: unknown; message?: unknown }; }; }; captureTerminalHttpStatus(logCtx, json); @@ -778,7 +779,8 @@ function captureUpstreamErrorParsed( if (logCtx.upstreamError) return; const message = json?.error?.message ?? json?.last_error?.message - ?? json?.response?.error?.message; + ?? json?.response?.error?.message + ?? json?.response?.incomplete_details?.message; if (typeof message === "string" && message.trim()) { logCtx.upstreamError = redactSecretString(message).slice(0, 500); return; @@ -817,25 +819,43 @@ function captureTerminalHttpStatus( logCtx: RequestLogContext, json: { type?: unknown; - response?: { error?: { type?: unknown; code?: unknown; message?: unknown } }; + code?: unknown; + message?: unknown; + error?: { type?: unknown; code?: unknown; message?: unknown }; + last_error?: { type?: unknown; code?: unknown; message?: unknown }; + response?: { + error?: { type?: unknown; code?: unknown; message?: unknown }; + incomplete_details?: { code?: unknown; message?: unknown }; + }; }, ): void { if (logCtx.terminalHttpStatus !== undefined) return; - if (json.type !== "response.failed") return; - const error = json.response?.error; - if (!error || typeof error !== "object") return; - const terminalCode = error.code === null || typeof error.code === "string" - ? error.code - : undefined; - if (isCyberPolicyCode(terminalCode)) { + const type = json.type; + if (type !== "response.failed" && type !== "response.incomplete" && type !== "error") return; + const responseError = json.response?.error; + const responseDetails = json.response?.incomplete_details; + const candidates = [json.error, json.last_error, responseError, responseDetails, json]; + const policy = candidates.some(candidate => ( + candidate?.code === null || typeof candidate?.code === "string" + ) && isCyberPolicyCode(candidate.code as string | null | undefined)) + || candidates.some(candidate => ( + typeof candidate?.message === "string" + && candidate.message.trim().length > 0 + && isCyberPolicyMessage(candidate.message) + )); + if (policy) { logCtx.terminalErrorCode = CYBER_POLICY_ERROR_CODE; - } else { - delete logCtx.terminalErrorCode; + logCtx.terminalHttpStatus = 400; + return; } + if (type !== "response.failed" || !responseError || typeof responseError !== "object") return; + const responseCode = responseError.code === null || typeof responseError.code === "string" + ? responseError.code + : undefined; logCtx.terminalHttpStatus = httpStatusFromTerminalError({ - type: typeof error.type === "string" ? error.type : undefined, - code: terminalCode, - message: typeof error.message === "string" ? error.message : undefined, + type: typeof responseError.type === "string" ? responseError.type : undefined, + code: responseCode, + message: typeof responseError.message === "string" ? responseError.message : undefined, }); } diff --git a/src/server/sse-frame-buffer.ts b/src/server/sse-frame-buffer.ts index 175e7bcab1..97f8d50931 100644 --- a/src/server/sse-frame-buffer.ts +++ b/src/server/sse-frame-buffer.ts @@ -1,3 +1,5 @@ +import { isCyberPolicyCode, isCyberPolicyMessage } from "../lib/errors"; + export const MAX_CLIENT_SSE_FRAME_BYTES = 4 * 1024 * 1024; const LF_LF = Uint8Array.of(10, 10); @@ -106,10 +108,35 @@ function isResponsesTerminalFrame(block: Uint8Array): boolean { const payload = data.join("\n"); if (payload === "[DONE]") return false; try { - const parsed = JSON.parse(payload) as { type?: unknown }; - return parsed.type === "response.completed" + const parsed = JSON.parse(payload) as { + type?: unknown; + code?: unknown; + message?: unknown; + error?: unknown; + last_error?: unknown; + response?: { error?: unknown; incomplete_details?: unknown }; + }; + if (parsed.type === "response.completed" || parsed.type === "response.failed" - || parsed.type === "response.incomplete"; + || parsed.type === "response.incomplete") return true; + if (parsed.type !== "error") return false; + // A top-level error is terminal for this boundary only when it carries the + // same high-confidence cyber-policy evidence used by relay.ts. Ordinary + // upstream errors remain transport failures and still become a bounded 502. + const candidates = [ + parsed, + parsed.error, + parsed.last_error, + parsed.response?.error, + parsed.response?.incomplete_details, + ]; + for (const candidate of candidates) { + if (!candidate || typeof candidate !== "object" || Array.isArray(candidate)) continue; + const record = candidate as { code?: unknown; message?: unknown }; + if (isCyberPolicyCode(typeof record.code === "string" ? record.code : undefined)) return true; + if (typeof record.message === "string" && isCyberPolicyMessage(record.message)) return true; + } + return false; } catch { return false; } @@ -289,4 +316,4 @@ export function joinSseFrameBytes(parts: readonly Uint8Array[]): Uint8Array { offset += part.byteLength; } return joined; -} \ No newline at end of file +} diff --git a/tests/passthrough-abort.test.ts b/tests/passthrough-abort.test.ts index 113bd27f0c..40c79317cd 100644 --- a/tests/passthrough-abort.test.ts +++ b/tests/passthrough-abort.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { linkAbortSignal, relaySseWithHeartbeat, relayWithAbort } from "../src/server"; +import { consumeForInspection, linkAbortSignal, relaySseWithFailedTail, relaySseWithHeartbeat, relayWithAbort } from "../src/server"; const root = new URL("../", import.meta.url); @@ -17,6 +17,16 @@ function streamFromChunks(chunks: Uint8Array[]): ReadableStream { }); } +function joinBytes(parts: Uint8Array[]): Uint8Array { + const out = new Uint8Array(parts.reduce((total, part) => total + part.byteLength, 0)); + let offset = 0; + for (const part of parts) { + out.set(part, offset); + offset += part.byteLength; + } + return out; +} + async function readAll(stream: ReadableStream): Promise { const reader = stream.getReader(); const dec = new TextDecoder(); @@ -175,6 +185,344 @@ describe("passthrough relayWithAbort (RC2, passthrough path)", () => { expect(terminals).toEqual(["failed"]); }); + test("tee/pull relay clean EOF synthesizes one adapter_eof incomplete and one DONE", async () => { + const enc = new TextEncoder(); + const relayed = relaySseWithFailedTail(streamFromChunks([ + enc.encode('event: response.created\ndata: {"type":"response.created"}\n\n'), + ]), new AbortController()); + const text = await readAll(relayed); + expect(text.match(/event: response\.incomplete/g)?.length).toBe(1); + expect(text).toContain('"reason":"adapter_eof"'); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + }); + + test("tee/pull relay suppresses a premature DONE before the synthetic terminal", async () => { + const enc = new TextEncoder(); + const relayed = relaySseWithFailedTail(streamFromChunks([ + enc.encode('event: response.created\ndata: {"type":"response.created"}\n\ndata: [DONE]\n\n'), + ]), new AbortController()); + const text = await readAll(relayed); + expect(text.match(/event: response\.incomplete/g)?.length).toBe(1); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + expect(text.indexOf("response.incomplete")).toBeLessThan(text.indexOf("data: [DONE]")); + }); + + test("tee/pull relay rewrites policy incomplete and top-level error to failed", async () => { + const enc = new TextEncoder(); + for (const frame of [ + `event: response.incomplete\ndata: ${JSON.stringify({ + type: "response.incomplete", + response: { + status: "incomplete", + error: { type: "invalid_request_error", code: "cyber_policy", message: "blocked" }, + }, + })}\n\n`, + `event: error\ndata: ${JSON.stringify({ + type: "error", + error: { type: "invalid_request_error", code: "cyber_policy", message: "blocked" }, + })}\n\n`, + ]) { + const relayed = relaySseWithFailedTail(streamFromChunks([enc.encode(frame)]), new AbortController()); + const text = await readAll(relayed); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text).not.toContain("response.incomplete"); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + expect(text).toContain('"type":"invalid_request_error"'); + expect(text).toContain('"code":"cyber_policy"'); + } + }); + + test("tee/pull normalizes structured policy response.failed while preserving metadata", async () => { + const enc = new TextEncoder(); + const payload = JSON.stringify({ + type: "response.failed", + sequence_number: 19, + model: "gpt-policy", + response: { + id: "resp-structured-policy-pull", + output: [{ type: "message", id: "item-structured-policy-pull" }], + status: "failed", + error: { + type: "server_error", + code: "cyber_policy", + message: "blocked by upstream policy", + }, + }, + }); + const relayed = relaySseWithFailedTail(streamFromChunks([ + enc.encode(`event: response.failed\ndata: ${payload}\n\n`), + ]), new AbortController()); + + const text = await readAll(relayed); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + expect(text).toContain('"type":"invalid_request_error"'); + expect(text).toContain('"code":"cyber_policy"'); + expect(text).not.toContain('"type":"server_error"'); + expect(text).toContain('"sequence_number":19'); + expect(text).toContain('"model":"gpt-policy"'); + expect(text).toContain('"id":"resp-structured-policy-pull"'); + expect(text).toContain('"output":[{"type":"message","id":"item-structured-policy-pull"}]'); + }); + + test("tee/pull preserves a policy error before same-chunk frame-count overflow", async () => { + const enc = new TextEncoder(); + const policy = JSON.stringify({ + type: "error", + sequence_number: 24, + response: { + id: "resp-policy-pull-frame-count", + output: [{ type: "message", id: "item-policy-pull-frame-count" }], + status: "failed", + }, + error: { + type: "invalid_request_error", + code: "cyber_policy", + message: "blocked by upstream policy", + }, + }); + const relayed = relaySseWithFailedTail(streamFromChunks([ + enc.encode(`event: error\ndata: ${policy}\n\n${"\n\n".repeat(4096)}`), + ]), new AbortController()); + + const text = await readAll(relayed); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text).not.toContain("upstream_reset"); + expect(text).toContain('"code":"cyber_policy"'); + expect(text).toContain('"sequence_number":24'); + expect(text).toContain('"id":"resp-policy-pull-frame-count"'); + expect(text).toContain('"output":[{"type":"message","id":"item-policy-pull-frame-count"}]'); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + }); + + test("tee/pull preserves a policy error before same-chunk oversized trailing bytes", async () => { + const enc = new TextEncoder(); + const policy = JSON.stringify({ + type: "error", + sequence_number: 26, + response: { id: "resp-policy-pull-byte-overflow", output: [], status: "failed" }, + error: { + type: "invalid_request_error", + code: "cyber_policy", + message: "blocked by upstream policy", + }, + }); + const oversizedTail = new Uint8Array(4 * 1024 * 1024 + 1).fill(120); + const relayed = relaySseWithFailedTail(streamFromChunks([joinBytes([ + enc.encode(`event: error\ndata: ${policy}\n\n`), + oversizedTail, + ])]), new AbortController()); + + const text = await readAll(relayed); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text).not.toContain("upstream_reset"); + expect(text).toContain('"code":"cyber_policy"'); + expect(text).toContain('"sequence_number":26'); + expect(text).toContain('"id":"resp-policy-pull-byte-overflow"'); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + }); + + test("tee/pull parses each unframed EOF terminal once without adapter_eof", async () => { + const enc = new TextEncoder(); + const cases = [ + { + type: "response.completed", + event: "response.completed", + payload: { + type: "response.completed", + sequence_number: 41, + response: { id: "resp-pull-unframed-completed", status: "completed", output: [] }, + }, + }, + { + type: "response.failed", + event: "response.failed", + payload: { + type: "response.failed", + sequence_number: 42, + response: { id: "resp-pull-unframed-failed", status: "failed", output: [] }, + }, + }, + { + type: "response.incomplete", + event: "response.incomplete", + payload: { + type: "response.incomplete", + sequence_number: 43, + response: { id: "resp-pull-unframed-incomplete", status: "incomplete", output: [] }, + }, + }, + { + type: "error", + event: "response.failed", + payload: { + type: "error", + sequence_number: 44, + response: { + id: "resp-pull-unframed-policy", + output: [{ type: "message", id: "item-pull-unframed-policy" }], + status: "failed", + }, + error: { + type: "invalid_request_error", + code: "cyber_policy", + message: "blocked by upstream policy", + }, + }, + }, + ] as const; + + for (const fixture of cases) { + const relayed = relaySseWithFailedTail(streamFromChunks([ + enc.encode(`event: ${fixture.type}\ndata: ${JSON.stringify(fixture.payload)}`), + ]), new AbortController()); + const text = await readAll(relayed); + expect(text.match(/event: response\.(?:completed|failed|incomplete)/g)?.length).toBe(1); + expect(text).toContain(`event: ${fixture.event}`); + expect(text).not.toContain('"reason":"adapter_eof"'); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + } + }); + + test("tee/pull preserves an unframed terminal before a reader error", async () => { + const enc = new TextEncoder(); + const cases = [ + { + type: "response.completed", + event: "response.completed", + payload: { + type: "response.completed", + sequence_number: 51, + response: { id: "resp-read-error-completed", status: "completed", output: [] }, + }, + }, + { + type: "response.failed", + event: "response.failed", + payload: { + type: "response.failed", + sequence_number: 52, + response: { id: "resp-read-error-failed", status: "failed", output: [] }, + }, + }, + { + type: "response.incomplete", + event: "response.incomplete", + payload: { + type: "response.incomplete", + sequence_number: 53, + response: { id: "resp-read-error-incomplete", status: "incomplete", output: [] }, + }, + }, + { + type: "error", + event: "response.failed", + payload: { + type: "error", + sequence_number: 54, + response: { + id: "resp-read-error-policy", + output: [{ type: "message", id: "item-read-error-policy" }], + status: "failed", + }, + error: { + type: "invalid_request_error", + code: "cyber_policy", + message: "blocked by upstream policy", + }, + }, + }, + ] as const; + + for (const fixture of cases) { + let firstPull = true; + const body = new ReadableStream({ + pull(controller) { + if (firstPull) { + firstPull = false; + controller.enqueue(enc.encode(`event: ${fixture.type}\ndata: ${JSON.stringify(fixture.payload)}`)); + } else { + controller.error(new Error("socket reset after unframed terminal")); + } + }, + }); + const relayed = relaySseWithFailedTail(body, new AbortController()); + const text = await readAll(relayed); + expect(text.match(/event: response\.(?:completed|failed|incomplete)/g)?.length).toBe(1); + expect(text).toContain(`event: ${fixture.event}`); + expect(text).not.toContain("upstream_reset"); + expect(text).not.toContain('"reason":"adapter_eof"'); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + if (fixture.type === "error") { + expect(text).toContain('"sequence_number":54'); + expect(text).toContain('"id":"resp-read-error-policy"'); + expect(text).toContain('"output":[{"type":"message","id":"item-read-error-policy"}]'); + } + } + }); + + test("tee/pull keeps an ordinary top-level error fail-closed on reader error", async () => { + let firstPull = true; + const body = new ReadableStream({ + pull(controller) { + if (firstPull) { + firstPull = false; + controller.enqueue(new TextEncoder().encode( + `event: error\ndata: ${JSON.stringify({ + type: "error", + error: { type: "server_error", code: "upstream_error", message: "provider failed" }, + })}`, + )); + } else { + controller.error(new Error("socket reset after ordinary error")); + } + }, + }); + const relayed = relaySseWithFailedTail(body, new AbortController()); + const text = await readAll(relayed); + expect(text).toContain("event: error"); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text).toContain('"code":"upstream_reset"'); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + expect(text).not.toContain('"code":"cyber_policy"'); + }); + + test("inspection read-error flush records an unframed policy terminal as failed 400", async () => { + let firstPull = true; + const body = new ReadableStream({ + pull(controller) { + if (firstPull) { + firstPull = false; + controller.enqueue(new TextEncoder().encode( + `event: error\ndata: ${JSON.stringify({ + type: "error", + sequence_number: 55, + response: { id: "resp-inspection-read-error-policy", output: [], status: "failed" }, + error: { + type: "invalid_request_error", + code: "cyber_policy", + message: "blocked by upstream policy", + }, + })}`, + )); + } else { + controller.error(new Error("socket reset after inspection policy terminal")); + } + }, + }); + const terminals: Array<{ status: string; httpStatus?: number }> = []; + let done!: () => void; + const completed = new Promise(resolve => { done = resolve; }); + consumeForInspection( + body, + (status, httpStatus) => terminals.push({ status, ...(httpStatus === undefined ? {} : { httpStatus }) }), + undefined, + done, + ); + await completed; + expect(terminals).toEqual([{ status: "failed", httpStatus: 400 }]); + }); + test("SSE passthrough reports incomplete on EOF before a terminal payload", async () => { const enc = new TextEncoder(); const ac = new AbortController(); diff --git a/tests/relay-eager.test.ts b/tests/relay-eager.test.ts index 22bbef6971..4d5ba60ab2 100644 --- a/tests/relay-eager.test.ts +++ b/tests/relay-eager.test.ts @@ -264,7 +264,9 @@ describe("relaySseEagerBounded — inline payload rewrite (#864)", () => { up.close(); const text = await reading; - expect(text).toBe("data: �"); + expect(text.startsWith("data: �")).toBe(true); + expect(text).toContain('"reason":"adapter_eof"'); + expect(countOccurrences(text, "data: [DONE]")).toBe(1); }); test("terminal framing keeps partial blocks out of the rewrite budget", async () => { @@ -384,6 +386,478 @@ describe("relaySseEagerBounded — side-effect parity", () => { expect(rec.dones).toBe(1); }); + test("clean EOF emits one adapter_eof incomplete terminal and one DONE", async () => { + const { hooks, rec } = makeHooks(); + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); + up.push(sse(DELTA)); + up.close(); + + const text = await readAll(relayed); + expect(text.match(/event: response\.incomplete/g)?.length).toBe(1); + expect(text).toContain('"reason":"adapter_eof"'); + expect(countOccurrences(text, "data: [DONE]")).toBe(1); + expect(rec.synthetics).toEqual(["incomplete"]); + expect(rec.terminals).toEqual([]); + }); + + test("unframed EOF terminals are parsed once without an adapter_eof duplicate", async () => { + const cases = [ + { + type: "response.completed", + event: "response.completed", + payload: { + type: "response.completed", + sequence_number: 31, + response: { id: "resp-unframed-completed", status: "completed", output: [] }, + }, + }, + { + type: "response.failed", + event: "response.failed", + payload: { + type: "response.failed", + sequence_number: 32, + response: { id: "resp-unframed-failed", status: "failed", output: [] }, + }, + }, + { + type: "response.incomplete", + event: "response.incomplete", + payload: { + type: "response.incomplete", + sequence_number: 33, + response: { id: "resp-unframed-incomplete", status: "incomplete", output: [] }, + }, + }, + { + type: "error", + event: "response.failed", + payload: { + type: "error", + sequence_number: 34, + response: { + id: "resp-unframed-policy", + output: [{ type: "message", id: "item-unframed-policy" }], + status: "failed", + }, + error: { + type: "invalid_request_error", + code: "cyber_policy", + message: "blocked by upstream policy", + }, + }, + }, + ] as const; + + for (const fixture of cases) { + const { hooks, rec } = makeHooks(); + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); + up.push(enc.encode(`event: ${fixture.type}\ndata: ${JSON.stringify(fixture.payload)}`)); + up.close(); + + const text = await readAll(relayed); + expect(text.match(/event: response\.(?:completed|failed|incomplete)/g)?.length).toBe(1); + expect(text).toContain(`event: ${fixture.event}`); + expect(text).not.toContain('"reason":"adapter_eof"'); + expect(countOccurrences(text, "data: [DONE]")).toBe(1); + expect(rec.synthetics).toEqual([]); + } + }); + + test("policy response.incomplete is rewritten to one failed terminal with 400 accounting", async () => { + const { hooks, rec } = makeHooks(); + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); + up.push(enc.encode(`event: response.incomplete\ndata: ${JSON.stringify({ + type: "response.incomplete", + sequence_number: 7, + response: { + id: "resp-policy", + output: [{ type: "message", id: "item-1" }], + status: "incomplete", + incomplete_details: { reason: "content_filter" }, + error: { + type: "invalid_request_error", + code: "cyber_policy", + message: "blocked by upstream policy", + }, + }, + })}\n\n`)); + up.close(); + + const text = await readAll(relayed); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text).not.toContain("response.incomplete"); + expect(text).toContain('"code":"cyber_policy"'); + expect(text).toContain('"type":"invalid_request_error"'); + expect(text).toContain('"id":"resp-policy"'); + expect(text).toContain('"sequence_number":7'); + expect(text).toContain('"output":[{"type":"message","id":"item-1"}]'); + expect(countOccurrences(text, "data: [DONE]")).toBe(1); + expect(rec.terminals).toEqual([{ status: "failed", httpStatus: 400 }]); + expect(rec.synthetics).toEqual([]); + }); + + test("policy leading error frame is rewritten and stops a clean EOF without a duplicate", async () => { + const { hooks, rec } = makeHooks(); + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); + up.push(enc.encode(`event: error\ndata: ${JSON.stringify({ + type: "error", + error: { + type: "invalid_request_error", + code: "cyber_policy", + message: "policy blocked this request", + }, + })}\n\n`)); + up.close(); + + const text = await readAll(relayed); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text).not.toContain("event: error"); + expect(text).not.toContain("response.incomplete"); + expect(countOccurrences(text, "data: [DONE]")).toBe(1); + expect(rec.terminals).toEqual([{ status: "failed", httpStatus: 400 }]); + expect(rec.synthetics).toEqual([]); + }); + + test("structured policy response.failed stays failed, normalizes to cyber_policy, and preserves metadata", async () => { + const { hooks, rec } = makeHooks(); + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); + const payload = JSON.stringify({ + type: "response.failed", + sequence_number: 18, + model: "gpt-policy", + response: { + id: "resp-structured-policy-eager", + output: [{ type: "message", id: "item-structured-policy-eager" }], + status: "failed", + error: { + type: "server_error", + code: "cyber_policy", + message: "blocked by upstream policy", + }, + }, + }); + const framed = enc.encode(`event: response.failed\r\ndata: ${payload}\r\n\r\n`); + up.push(framed.subarray(0, framed.byteLength - 1)); + up.push(framed.subarray(framed.byteLength - 1)); + up.close(); + + const text = await readAll(relayed); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + expect(text).toContain('"type":"invalid_request_error"'); + expect(text).toContain('"code":"cyber_policy"'); + expect(text).not.toContain('"type":"server_error"'); + expect(text).toContain('"sequence_number":18'); + expect(text).toContain('"model":"gpt-policy"'); + expect(text).toContain('"id":"resp-structured-policy-eager"'); + expect(text).toContain('"output":[{"type":"message","id":"item-structured-policy-eager"}]'); + expect(rec.terminals).toEqual([{ status: "failed", httpStatus: 400 }]); + expect(rec.synthetics).toEqual([]); + }); + + test("ordinary response.failed remains unchanged", async () => { + const { hooks, rec } = makeHooks(); + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); + up.push(enc.encode(`event: response.failed\ndata: ${JSON.stringify({ + type: "response.failed", + response: { + id: "resp-ordinary-failed", + status: "failed", + error: { type: "server_error", code: "upstream_error", message: "provider failed" }, + }, + })}\n\n`)); + up.close(); + + const text = await readAll(relayed); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + expect(text).toContain('"code":"upstream_error"'); + expect(text).not.toContain('"code":"cyber_policy"'); + expect(rec.terminals).toEqual([{ status: "failed" }]); + expect(rec.synthetics).toEqual([]); + }); + + test("policy error survives same-chunk frame-count overflow without an upstream reset", async () => { + const { hooks, rec } = makeHooks(); + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); + const policy = JSON.stringify({ + type: "error", + sequence_number: 23, + response: { + id: "resp-policy-frame-count", + output: [{ type: "message", id: "item-policy-frame-count" }], + status: "failed", + }, + error: { + type: "invalid_request_error", + code: "cyber_policy", + message: "blocked by upstream policy", + }, + }); + // The configured 4 MiB client framer admits 4096 blocks per feed. The + // policy terminal is first, followed by 4096 empty blocks, which used to + // turn the already-committed policy response into an upstream_reset 502. + up.push(enc.encode(`event: error\ndata: ${policy}\n\n${"\n\n".repeat(4096)}`)); + up.close(); + + const text = await readAll(relayed); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text).not.toContain("upstream_reset"); + expect(text).toContain('"code":"cyber_policy"'); + expect(text).toContain('"sequence_number":23'); + expect(text).toContain('"id":"resp-policy-frame-count"'); + expect(text).toContain('"output":[{"type":"message","id":"item-policy-frame-count"}]'); + expect(countOccurrences(text, "data: [DONE]")).toBe(1); + expect(rec.terminals).toEqual([{ status: "failed", httpStatus: 400 }]); + expect(rec.synthetics).toEqual([]); + }); + + test("policy error also survives same-chunk oversized trailing bytes", async () => { + const { hooks, rec } = makeHooks(); + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); + const policy = JSON.stringify({ + type: "error", + sequence_number: 25, + response: { id: "resp-policy-byte-overflow", output: [], status: "failed" }, + error: { + type: "invalid_request_error", + code: "cyber_policy", + message: "blocked by upstream policy", + }, + }); + const oversizedTail = new Uint8Array(4 * 1024 * 1024 + 1).fill(120); + up.push(joinBytes([ + enc.encode(`event: error\ndata: ${policy}\n\n`), + oversizedTail, + ])); + up.close(); + + const text = await readAll(relayed); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text).not.toContain("upstream_reset"); + expect(text).toContain('"code":"cyber_policy"'); + expect(text).toContain('"id":"resp-policy-byte-overflow"'); + expect(countOccurrences(text, "data: [DONE]")).toBe(1); + expect(rec.terminals).toEqual([{ status: "failed", httpStatus: 400 }]); + }); + + test("eager preserves an unframed terminal before a reader error", async () => { + const cases = [ + { + type: "response.completed", + event: "response.completed", + status: "completed", + payload: { + type: "response.completed", + sequence_number: 61, + response: { id: "resp-eager-read-error-completed", status: "completed", output: [] }, + }, + }, + { + type: "response.failed", + event: "response.failed", + status: "failed", + payload: { + type: "response.failed", + sequence_number: 62, + response: { id: "resp-eager-read-error-failed", status: "failed", output: [] }, + }, + }, + { + type: "response.incomplete", + event: "response.incomplete", + status: "incomplete", + payload: { + type: "response.incomplete", + sequence_number: 63, + response: { id: "resp-eager-read-error-incomplete", status: "incomplete", output: [] }, + }, + }, + { + type: "error", + event: "response.failed", + status: "failed", + payload: { + type: "error", + sequence_number: 64, + response: { + id: "resp-eager-read-error-policy", + output: [{ type: "message", id: "item-eager-read-error-policy" }], + status: "failed", + }, + error: { + type: "invalid_request_error", + code: "cyber_policy", + message: "blocked by upstream policy", + }, + }, + httpStatus: 400, + }, + ] as const; + + for (const fixture of cases) { + const { hooks, rec } = makeHooks(); + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); + up.push(enc.encode(`event: ${fixture.type}\ndata: ${JSON.stringify(fixture.payload)}`)); + up.fail(new Error("socket reset after unframed terminal")); + + const text = await readAll(relayed); + expect(text.match(/event: response\.(?:completed|failed|incomplete)/g)?.length).toBe(1); + expect(text).toContain(`event: ${fixture.event}`); + expect(text).not.toContain("upstream_reset"); + expect(text).not.toContain('"reason":"adapter_eof"'); + expect(countOccurrences(text, "data: [DONE]")).toBe(1); + expect(rec.terminals).toEqual([{ status: fixture.status, ...(fixture.httpStatus ? { httpStatus: fixture.httpStatus } : {}) }]); + expect(rec.synthetics).toEqual([]); + if (fixture.type === "error") { + expect(text).toContain('"sequence_number":64'); + expect(text).toContain('"id":"resp-eager-read-error-policy"'); + expect(text).toContain('"output":[{"type":"message","id":"item-eager-read-error-policy"}]'); + } + } + }); + + test("eager keeps an ordinary top-level error fail-closed on reader error", async () => { + const { hooks, rec } = makeHooks(); + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); + up.push(enc.encode(`event: error\ndata: ${JSON.stringify({ + type: "error", + error: { type: "server_error", code: "upstream_error", message: "provider failed" }, + })}`)); + up.fail(new Error("socket reset after ordinary error")); + + const text = await readAll(relayed); + expect(text).toContain("event: error"); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text).toContain('"code":"upstream_reset"'); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + expect(text).not.toContain('"code":"cyber_policy"'); + expect(rec.terminals).toEqual([]); + expect(rec.synthetics).toEqual(["failed"]); + }); + + test("eager read-error terminal flushes the active rewrite tail", async () => { + const { hooks, rec } = makeHooks(); + hooks.rewritePayload = payload => payload.replace("rewrite-me", "rewritten"); + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); + up.push(enc.encode(`event: response.completed\ndata: ${JSON.stringify({ + type: "response.completed", + response: { id: "resp-read-error-rewrite", status: "completed", output: [{ type: "message", text: "rewrite-me" }] }, + })}`)); + up.fail(new Error("socket reset after rewritten terminal")); + + const text = await readAll(relayed); + expect(text).toContain("rewritten"); + expect(text).not.toContain("rewrite-me"); + expect(text.match(/event: response\.completed/g)?.length).toBe(1); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + expect(text).not.toContain("upstream_reset"); + expect(rec.terminals).toEqual([{ status: "completed" }]); + expect(rec.synthetics).toEqual([]); + }); + + test("eager rewriteBlocks failure emits one safe failed envelope without a second outcome", async () => { + const { hooks, rec } = makeHooks(); + hooks.rewriteBlocks = () => { throw new Error("rewrite callback failed"); }; + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); + up.push(enc.encode(`event: response.completed\ndata: ${JSON.stringify({ + type: "response.completed", + sequence_number: 71, + response: { + id: "resp-rewrite-failure-secret", + status: "completed", + output: [{ type: "message", text: "raw-secret-content" }], + }, + })}`)); + up.fail(new Error("socket reset after rewrite failure")); + + const text = await readAll(relayed); + expect(text.length).toBeGreaterThan(0); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + expect(text).toContain('"code":"upstream_reset"'); + expect(text).not.toContain("resp-rewrite-failure-secret"); + expect(text).not.toContain("raw-secret-content"); + expect(text).not.toContain('"sequence_number":71'); + expect(rec.terminals).toEqual([{ status: "completed" }]); + expect(rec.synthetics).toEqual([]); + expect(rec.dones).toBe(1); + }); + + test("eager rewrite-budget exhaustion emits one bounded failed envelope and releases the budget", async () => { + const budget = createTranslatorBudget({ maxTurnBytes: 1 }); + const { hooks, rec } = makeHooks(); + hooks.rewritePayload = payload => payload; + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks, { rewriteBudget: budget }); + up.push(enc.encode(`event: response.completed\ndata: ${JSON.stringify({ + type: "response.completed", + sequence_number: 72, + response: { + id: "resp-budget-failure-secret", + status: "completed", + output: [{ type: "message", text: "raw-budget-secret" }], + }, + })}`)); + up.fail(new Error("socket reset after budget failure")); + + const text = await readAll(relayed); + expect(text.length).toBeGreaterThan(0); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + expect(text).toContain('"code":"translation_buffer_limit"'); + expect(text).not.toContain("resp-budget-failure-secret"); + expect(text).not.toContain("raw-budget-secret"); + expect(text).not.toContain('"sequence_number":72'); + expect(rec.terminals).toEqual([{ status: "completed" }]); + expect(rec.synthetics).toEqual([]); + expect(rec.dones).toBe(1); + expect(budget.snapshot().currentBytes).toBe(0); + budget.dispose(); + }); + + test("eager clean EOF rewrite failure emits one safe failed envelope", async () => { + const { hooks, rec } = makeHooks(); + hooks.rewriteBlocks = () => { throw new Error("clean EOF rewrite callback failed"); }; + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); + up.push(enc.encode(`event: response.completed\ndata: ${JSON.stringify({ + type: "response.completed", + sequence_number: 73, + response: { + id: "resp-clean-eof-rewrite-secret", + status: "completed", + output: [{ type: "message", text: "clean-eof-raw-secret" }], + }, + })}`)); + up.close(); + + const text = await readAll(relayed); + expect(text.length).toBeGreaterThan(0); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + expect(text).toContain('"code":"upstream_reset"'); + expect(text).not.toContain("resp-clean-eof-rewrite-secret"); + expect(text).not.toContain("clean-eof-raw-secret"); + expect(text).not.toContain('"sequence_number":73'); + expect(rec.terminals).toEqual([{ status: "completed" }]); + expect(rec.synthetics).toEqual([]); + expect(rec.dones).toBe(1); + }); + test("eager relay backfills missing completed output before passthrough persistence (#334)", async () => { const { hooks, rec } = makeHooks(); const up = controlledUpstream(); @@ -464,7 +938,9 @@ describe("relaySseEagerBounded — bounded queue", () => { if (done) break; total += value.byteLength; } - expect(total).toBe(36); + // Clean EOF now carries the explicit adapter_eof incomplete terminal and + // one [DONE] sentinel in addition to the relayed bytes. + expect(total).toBeGreaterThan(36); }); test("(f) cancel while paused wakes the gate — onDone fires, no deadlock", async () => { diff --git a/tests/request-log.test.ts b/tests/request-log.test.ts index 09512c50e1..6cfaf43681 100644 --- a/tests/request-log.test.ts +++ b/tests/request-log.test.ts @@ -985,6 +985,134 @@ describe("request log metadata", () => { }); }); + test("deferred SSE logging maps policy response.incomplete to failed 400", async () => { + const entries: RequestLogEntry[] = []; + const payload = JSON.stringify({ + type: "response.incomplete", + response: { + id: "resp-policy-incomplete", + status: "incomplete", + incomplete_details: { reason: "content_filter" }, + error: { type: "invalid_request_error", code: "cyber_policy", message: "blocked" }, + }, + }); + const response = responseWithDeferredRequestLog( + new Response(new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(`event: response.incomplete\ndata: ${payload}\n\n`)); + controller.close(); + }, + }), { status: 200, headers: { "content-type": "text/event-stream" } }), + "ocx-test-cyber-policy-incomplete", + Date.now(), + { model: "gpt-5.6-sol", provider: "openai" }, + entry => entries.push(entry), + ); + + await response.text(); + expect(entries).toHaveLength(1); + expect(entries[0]).toMatchObject({ + terminalStatus: "failed", + upstreamError: "blocked", + status: 400, + errorCode: "cyber_policy", + closeReason: "terminal", + }); + }); + + test("deferred SSE logging maps a policy top-level error to failed 400", async () => { + const entries: RequestLogEntry[] = []; + const payload = JSON.stringify({ + type: "error", + error: { type: "invalid_request_error", message: "This request was flagged for possible cybersecurity risk." }, + }); + const response = responseWithDeferredRequestLog( + new Response(new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(`event: error\ndata: ${payload}\n\n`)); + controller.close(); + }, + }), { status: 200, headers: { "content-type": "text/event-stream" } }), + "ocx-test-cyber-policy-error", + Date.now(), + { model: "gpt-5.6-sol", provider: "openai" }, + entry => entries.push(entry), + ); + + await response.text(); + expect(entries).toHaveLength(1); + expect(entries[0]).toMatchObject({ + terminalStatus: "failed", + status: 400, + errorCode: "cyber_policy", + closeReason: "terminal", + }); + }); + + test("deferred SSE logging checks all policy candidates, not only the first code", async () => { + const entries: RequestLogEntry[] = []; + const payload = JSON.stringify({ + type: "response.incomplete", + error: { type: "upstream_error", code: "upstream_reset", message: "connection ended" }, + response: { + status: "incomplete", + error: { type: "invalid_request_error", code: "cyber_policy", message: "blocked" }, + }, + }); + const response = responseWithDeferredRequestLog( + new Response(new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(`data: ${payload}\n\n`)); + controller.close(); + }, + }), { status: 200, headers: { "content-type": "text/event-stream" } }), + "ocx-test-cyber-policy-candidates", + Date.now(), + { model: "gpt-5.6-sol", provider: "openai" }, + entry => entries.push(entry), + ); + + await response.text(); + expect(entries).toHaveLength(1); + expect(entries[0]).toMatchObject({ + terminalStatus: "failed", + status: 400, + errorCode: "cyber_policy", + }); + }); + + test("deferred SSE logging maps ordinary failed status from response.error only", async () => { + const entries: RequestLogEntry[] = []; + const payload = JSON.stringify({ + type: "response.failed", + code: "context_length_exceeded", + response: { + status: "failed", + error: { type: "rate_limit_error", code: "rate_limit_exceeded", message: "rate limited" }, + }, + }); + const response = responseWithDeferredRequestLog( + new Response(new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(`data: ${payload}\n\n`)); + controller.close(); + }, + }), { status: 200, headers: { "content-type": "text/event-stream" } }), + "ocx-test-ordinary-failed-authority", + Date.now(), + { model: "gpt-5.6-sol", provider: "openai" }, + entry => entries.push(entry), + ); + + await response.text(); + expect(entries).toHaveLength(1); + expect(entries[0]).toMatchObject({ + terminalStatus: "failed", + status: 429, + errorCode: "rate_limit_exceeded", + }); + }); + test("deferred SSE logging maps web-search client closes to 499 client_cancel", async () => { const entries: RequestLogEntry[] = []; const message = "client closed request during web-search"; From 9a350cbef2b95df93cd56f1bd10ab972dd89ca95 Mon Sep 17 00:00:00 2001 From: Codex Date: Mon, 24 Aug 2026 19:17:45 +0900 Subject: [PATCH 02/19] fix: delimit repaired SSE EOF tails --- src/server/responses-terminal-repair.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/server/responses-terminal-repair.ts b/src/server/responses-terminal-repair.ts index 7d35141a89..8d2afa465b 100644 --- a/src/server/responses-terminal-repair.ts +++ b/src/server/responses-terminal-repair.ts @@ -293,10 +293,13 @@ export function relayResponsesSseWithTerminalRepair( appendBuffer(decoder.decode()); if (buffer.length > 0) { // A delimiter-less suffix is not a complete SSE event. Preserve the - // upstream bytes for passthrough compatibility, but never let a - // truncated lifecycle frame establish synthetic success. + // upstream bytes for passthrough compatibility, but terminate the + // preserved block before emitting the synthetic terminal. Without + // this delimiter the boundary relay sees both JSON payloads as one + // malformed SSE block and appends a second adapter_eof terminal. tainted = true; controller.enqueue(encoder.encode(buffer)); + controller.enqueue(encoder.encode(buffer.includes("\r\n") ? "\r\n\r\n" : "\n\n")); } if (!realTerminalSeen) { emitSynthetic(completeCandidate() ? "completed" : "incomplete", controller); From 097663a166e7db6d24180f843b53f4290fca5d7e Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Mon, 24 Aug 2026 22:25:29 +0900 Subject: [PATCH 03/19] docs: record Responses terminal boundary invariants --- structure/04_transports-and-sidecars.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/structure/04_transports-and-sidecars.md b/structure/04_transports-and-sidecars.md index f7b9b1859b..9429e9ed3a 100644 --- a/structure/04_transports-and-sidecars.md +++ b/structure/04_transports-and-sidecars.md @@ -283,6 +283,30 @@ terminal observer. Native Responses, Chat Completions, Claude Messages, and WebS request logs must therefore finalize through the context-aware terminal mapper; recognized `cyber_policy` terminals stay `400 / cyber_policy` rather than collapsing to a generic 502. +The client-facing boundary treats the first Responses terminal as authoritative in both relay +shapes. High-confidence policy errors carried as `response.incomplete`, `response.failed`, or a +top-level `error` are normalized to one `response.failed / cyber_policy` event without changing the +refusal outcome; later bytes cannot create a second terminal. A clean HTTP 200 EOF with no terminal +instead emits one `response.incomplete` with `adapter_eof`, followed by one `[DONE]`. Delimiter-less +EOF terminals are parsed through the same bounded frame path, so pull/tee and eager relays agree on +terminal, sentinel, and request-log accounting. + +[Decision Log] +- 목적과 의도: Turn upstream terminal variants and bare EOF into one deterministic Responses + outcome instead of a retryable disconnect or duplicate terminal. +- 기존 구현 및 제약 조건: Policy refusals can arrive in several SSE envelopes, while a clean EOF, + an unterminated final frame, and a read error exercise different pull/tee and eager cleanup paths. +- 검토한 주요 대안: Forward every byte unchanged; classify only request logs; synthesize a failure + after every EOF or read error; normalize the bounded terminal at the client output boundary. +- 선택한 방식: Rewrite only high-confidence policy terminal shapes, preserve their bounded metadata, + flush an unterminated terminal before transport-error classification, and synthesize `adapter_eof` + only when no real terminal exists. +- 다른 대안 대신 이 방식을 선택한 이유: Log-only classification leaves Codex retry behavior + unchanged, while unconditional synthesis can create two contradictory outcomes for one turn. +- 장점, 단점 및 영향: Both native relay shapes expose exactly one terminal and one sentinel with + matching accounting. Ordinary upstream errors remain fail-closed, and policy refusals remain + refusals rather than becoming successful model output. + ## Standalone Search and exact account selectors `POST /v1/alpha/search` retains the selected model in its request body. When that value is an From d8d31415d5d8bb027a5331571eca5fb589c4892f Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Mon, 24 Aug 2026 22:36:50 +0900 Subject: [PATCH 04/19] fix(responses): keep unframed repair terminals fail-closed --- src/server/responses-terminal-repair.ts | 32 ++++++++++++---- structure/04_transports-and-sidecars.md | 10 +++-- tests/responses-terminal-repair.test.ts | 50 +++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 11 deletions(-) diff --git a/src/server/responses-terminal-repair.ts b/src/server/responses-terminal-repair.ts index 8d2afa465b..a2ff658f5f 100644 --- a/src/server/responses-terminal-repair.ts +++ b/src/server/responses-terminal-repair.ts @@ -22,6 +22,22 @@ function isPlainRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } +function isUnframedTerminalLikeSuffix(block: string): boolean { + const payload = sseDataPayload(block); + if (payload === "[DONE]") return true; + if (!payload) return false; + try { + const parsed = JSON.parse(payload); + if (!isPlainRecord(parsed)) return false; + return parsed.type === "response.completed" + || parsed.type === "response.failed" + || parsed.type === "response.incomplete" + || parsed.type === "error"; + } catch { + return false; + } +} + function outputIndex(value: unknown): number | null { return Number.isInteger(value) && (value as number) >= 0 ? value as number : null; } @@ -292,14 +308,16 @@ export function relayResponsesSseWithTerminalRepair( if (done) { appendBuffer(decoder.decode()); if (buffer.length > 0) { - // A delimiter-less suffix is not a complete SSE event. Preserve the - // upstream bytes for passthrough compatibility, but terminate the - // preserved block before emitting the synthetic terminal. Without - // this delimiter the boundary relay sees both JSON payloads as one - // malformed SSE block and appends a second adapter_eof terminal. + // A delimiter-less suffix is not a complete SSE event. Preserve an + // ordinary suffix for passthrough compatibility, but never promote + // a terminal-like suffix by adding the delimiter it did not receive + // upstream. The latter must stay tainted and fail closed through the + // synthetic incomplete terminal below. tainted = true; - controller.enqueue(encoder.encode(buffer)); - controller.enqueue(encoder.encode(buffer.includes("\r\n") ? "\r\n\r\n" : "\n\n")); + if (!isUnframedTerminalLikeSuffix(buffer)) { + controller.enqueue(encoder.encode(buffer)); + controller.enqueue(encoder.encode(buffer.includes("\r\n") ? "\r\n\r\n" : "\n\n")); + } } if (!realTerminalSeen) { emitSynthetic(completeCandidate() ? "completed" : "incomplete", controller); diff --git a/structure/04_transports-and-sidecars.md b/structure/04_transports-and-sidecars.md index 9429e9ed3a..7aeb55c607 100644 --- a/structure/04_transports-and-sidecars.md +++ b/structure/04_transports-and-sidecars.md @@ -288,8 +288,10 @@ shapes. High-confidence policy errors carried as `response.incomplete`, `respons top-level `error` are normalized to one `response.failed / cyber_policy` event without changing the refusal outcome; later bytes cannot create a second terminal. A clean HTTP 200 EOF with no terminal instead emits one `response.incomplete` with `adapter_eof`, followed by one `[DONE]`. Delimiter-less -EOF terminals are parsed through the same bounded frame path, so pull/tee and eager relays agree on -terminal, sentinel, and request-log accounting. +EOF candidates follow the owning repair policy: the native boundary accepts a structurally valid +terminal tail, while an opted-in terminal repair keeps its unframed suffix tainted and emits +`missing_terminal_event`. Pull/tee and eager relays therefore agree on terminal, sentinel, and +request-log accounting without promoting a truncated repair candidate. [Decision Log] - 목적과 의도: Turn upstream terminal variants and bare EOF into one deterministic Responses @@ -299,8 +301,8 @@ terminal, sentinel, and request-log accounting. - 검토한 주요 대안: Forward every byte unchanged; classify only request logs; synthesize a failure after every EOF or read error; normalize the bounded terminal at the client output boundary. - 선택한 방식: Rewrite only high-confidence policy terminal shapes, preserve their bounded metadata, - flush an unterminated terminal before transport-error classification, and synthesize `adapter_eof` - only when no real terminal exists. + flush native terminal candidates before transport-error classification, keep repair-owned + delimiter-less candidates tainted, and synthesize `adapter_eof` only when no real terminal exists. - 다른 대안 대신 이 방식을 선택한 이유: Log-only classification leaves Codex retry behavior unchanged, while unconditional synthesis can create two contradictory outcomes for one turn. - 장점, 단점 및 영향: Both native relay shapes expose exactly one terminal and one sentinel with diff --git a/tests/responses-terminal-repair.test.ts b/tests/responses-terminal-repair.test.ts index 21457962b6..b982357ac5 100644 --- a/tests/responses-terminal-repair.test.ts +++ b/tests/responses-terminal-repair.test.ts @@ -350,6 +350,56 @@ describe("DeepSeek Responses terminal repair", () => { expect(output).not.toContain('"type":"response.completed"'); }); + test("unframed terminal-like suffixes stay tainted and cannot outrank incomplete", async () => { + const fixtures = [ + { + event: "response.completed", + payload: { type: "response.completed", response: { id: "resp_truncated_completed", status: "completed" } }, + }, + { + event: "response.failed", + payload: { type: "response.failed", response: { id: "resp_truncated_failed", status: "failed" } }, + }, + { + event: "response.incomplete", + payload: { type: "response.incomplete", response: { id: "resp_truncated_incomplete", status: "incomplete" } }, + }, + { + event: "error", + payload: { + type: "error", + response: { id: "resp_truncated_error", status: "failed" }, + error: { type: "server_error", code: "upstream_error", message: "provider failed" }, + }, + }, + { + event: "error", + payload: { + type: "error", + error: { type: "invalid_request_error", code: "cyber_policy", message: "blocked by upstream policy" }, + }, + }, + ] as const; + + for (const newline of ["\n", "\r\n"] as const) { + for (const fixture of fixtures) { + const input = `event: ${fixture.event}${newline}data: ${JSON.stringify(fixture.payload)}`; + const { output } = await repairClosedText(input); + + expect(terminalTypes(output)).toEqual(["response.incomplete"]); + expect(output).toContain('"reason":"missing_terminal_event"'); + expect(output).not.toContain("resp_truncated_"); + expect(output).not.toContain("cyber_policy"); + expect(output.match(/data: \[DONE\]/g)?.length).toBe(1); + } + + const { output: doneOutput } = await repairClosedText(`data: [DONE]${newline}`); + expect(terminalTypes(doneOutput)).toEqual(["response.incomplete"]); + expect(doneOutput).toContain('"reason":"missing_terminal_event"'); + expect(doneOutput.match(/data: \[DONE\]/g)?.length).toBe(1); + } + }); + test("DONE is replaced by completed then one DONE only for a complete candidate", async () => { const { output } = await repairClosedText(completedMessageLifecycle() + "data: [DONE]\n\n"); expect(terminalTypes(output)).toEqual(["response.completed"]); From 7dd669e366d03d9d111c6f05e478846c10fda231 Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 00:35:55 +0900 Subject: [PATCH 05/19] fix(responses): preserve terminal delivery after overflow --- src/server/relay-eager.ts | 50 ++--- src/server/relay.ts | 48 ++++- tests/relay-eager.test.ts | 335 +++++++++++++++++++++++----------- tests/request-log.test.ts | 38 ++++ tests/sse-failed-tail.test.ts | 20 ++ 5 files changed, 350 insertions(+), 141 deletions(-) diff --git a/src/server/relay-eager.ts b/src/server/relay-eager.ts index ea36b34bf6..b428938eec 100644 --- a/src/server/relay-eager.ts +++ b/src/server/relay-eager.ts @@ -24,7 +24,12 @@ * up to the drain window. */ -import { buildFailedTailPayload, createSseTerminalOutputBoundary } from "./relay"; +import { + adapterEofIncompleteFrame, + createSseTerminalOutputBoundary, + doneFrame, + failedTailFrame, +} from "./relay"; import { nextSseBlock, payloadRewriteAsBlockRewrite, @@ -81,10 +86,6 @@ export type EagerRelayOptions = { const DEFAULT_MAX_QUEUE_BYTES = 8 * 1024 * 1024; const DEFAULT_DRAIN_MS = 15_000; const DEFAULT_DRAIN_BYTES = 32 * 1024 * 1024; -const ADAPTER_EOF_INCOMPLETE_FRAME = new TextEncoder().encode( - 'event: response.incomplete\ndata: {"type":"response.incomplete","response":{"status":"incomplete","incomplete_details":{"reason":"adapter_eof"}}}\n\n', -); -const DONE_FRAME = new TextEncoder().encode("data: [DONE]\n\n"); /** * Relay `body` to the returned stream with eager bounded reading and inline @@ -104,20 +105,17 @@ export function relaySseEagerBounded( const now = opts?.now ?? Date.now; const reader = body.getReader(); + const terminalEncoder = new TextEncoder(); + const adapterEofFrame = adapterEofIncompleteFrame(terminalEncoder); + const terminalSentinel = doneFrame(terminalEncoder); const terminalBoundary = createSseTerminalOutputBoundary(); const activeRewrite: SseBlockRewrite | undefined = hooks.rewriteBlocks ?? (hooks.rewritePayload ? payloadRewriteAsBlockRewrite(hooks.rewritePayload) : undefined); const encodeFailedTail = (error: unknown): Uint8Array | null => { try { - return new TextEncoder().encode( - `\n\nevent: response.failed\ndata: ${buildFailedTailPayload(error)}\n\ndata: [DONE]\n\n`, - ); + return failedTailFrame(terminalEncoder, error); } catch { - // A hostile error accessor may throw while building the diagnostic. Keep - // the client contract bounded even then; callers still re-check abort. - return new TextEncoder().encode( - '\n\nevent: response.failed\ndata: {"type":"response.failed","response":{"status":"failed","error":{"type":"upstream_error","code":"upstream_reset","message":"Upstream stream terminated unexpectedly"},"last_error":{"type":"upstream_error","code":"upstream_reset","message":"Upstream stream terminated unexpectedly"}}}\n\ndata: [DONE]\n\n', - ); + return null; } }; const rewriteDecoder = activeRewrite ? new TextDecoder() : null; @@ -181,7 +179,6 @@ export function relaySseEagerBounded( let queuedBytes = 0; let cancelled = false; let done = false; - const terminalSentinel = new TextEncoder().encode("data: [DONE]\n\n"); // Pause gate: resolved by client pull, client cancel, or upstream abort so a // paused producer ALWAYS resumes (audit blocker 2 — no deadlock; onDone and // turn unregistration stay reachable, drainAndShutdown never hangs). @@ -213,6 +210,7 @@ export function relaySseEagerBounded( const producer = async () => { let syntheticKind: "incomplete" | "failed" | null = null; + let deliveryFallbackSent = false; let priorRewriteFailure = false; let priorRewriteError: unknown; // reader.read() is not intrinsically tied to the upstream AbortController @@ -275,10 +273,10 @@ export function relaySseEagerBounded( } else if (!hooks.sawTerminal() && !cancelled && !upstream.signal.aborted) { // A clean 200 EOF without a Responses terminal must be visible to // Codex as one incomplete turn, followed by the normal sentinel. - queuedBytes += ADAPTER_EOF_INCOMPLETE_FRAME.byteLength + DONE_FRAME.byteLength; + queuedBytes += adapterEofFrame.byteLength + terminalSentinel.byteLength; try { - controllerRef?.enqueue(ADAPTER_EOF_INCOMPLETE_FRAME); - controllerRef?.enqueue(DONE_FRAME); + controllerRef?.enqueue(adapterEofFrame); + controllerRef?.enqueue(terminalSentinel); } catch { /* client already gone */ } syntheticKind = "incomplete"; } @@ -385,6 +383,7 @@ export function relaySseEagerBounded( const safeTail = encodeFailedTail(rewriteError ?? err); if (safeTail && !cancelled && !upstream.signal.aborted) { if (!hooks.sawTerminal()) syntheticKind = "failed"; + deliveryFallbackSent = true; queuedBytes += safeTail.byteLength; try { controllerRef?.enqueue(safeTail); } catch { /* client already torn down */ } try { controllerRef?.close(); } catch { /* client already gone */ } @@ -394,16 +393,19 @@ export function relaySseEagerBounded( queuedBytes += terminalSentinel.byteLength; try { controllerRef?.enqueue(terminalSentinel); } catch { /* client already gone */ } } - } else if (!tailTerminal && !hooks.sawTerminal() && !cancelled && !upstream.signal.aborted) { + } else if (!tailTerminal && !cancelled && !upstream.signal.aborted) { // Serializing `err` can run user-defined accessors (Error.message // getters, toString) that re-entrantly cancel the client or abort the // upstream. Build the tail FIRST, then re-check eligibility before // committing to the synthetic terminal (adversarial review blocker). - const tail = new TextEncoder().encode( - `\n\nevent: response.failed\ndata: ${buildFailedTailPayload(err)}\n\ndata: [DONE]\n\n`, - ); - if (!hooks.sawTerminal() && !cancelled && !upstream.signal.aborted) { - syntheticKind = "failed"; + const tail = encodeFailedTail(err); + if (tail && !cancelled && !upstream.signal.aborted) { + // Inspection and client framing have separate bounded parsers. If + // inspection resynchronized after an oversized frame and observed a + // later real terminal, it still must not suppress a terminal delivery + // to the client. Only accounting remains tied to the inspected result. + if (!hooks.sawTerminal()) syntheticKind = "failed"; + deliveryFallbackSent = true; queuedBytes += tail.byteLength; try { controllerRef?.enqueue(tail); } catch { /* client already torn down */ } try { controllerRef?.close(); } catch { /* client already torn down */ } @@ -422,7 +424,7 @@ export function relaySseEagerBounded( if (cancelled && !hooks.sawTerminal()) { hooks.onClientCancel(); } - if (cancelled || upstream.signal.aborted || syntheticKind === "failed") { + if (cancelled || upstream.signal.aborted || syntheticKind === "failed" || deliveryFallbackSent) { upstream.abort(); reader.cancel().catch(() => {}); } diff --git a/src/server/relay.ts b/src/server/relay.ts index 299481cc39..199e22b1e8 100644 --- a/src/server/relay.ts +++ b/src/server/relay.ts @@ -37,6 +37,23 @@ const ADAPTER_EOF_INCOMPLETE_PAYLOAD = JSON.stringify({ incomplete_details: { reason: "adapter_eof" }, }, }); +const DONE_SSE_FRAME_TEXT = "data: [DONE]\n\n"; +const FAILED_TAIL_FALLBACK_PAYLOAD = JSON.stringify({ + type: "response.failed", + response: { + status: "failed", + error: { + type: "upstream_error", + code: "upstream_reset", + message: "Upstream stream terminated unexpectedly", + }, + last_error: { + type: "upstream_error", + code: "upstream_reset", + message: "Upstream stream terminated unexpectedly", + }, + }, +}); export type InspectionCounters = { frameBufferHighWaterBytes: number; @@ -112,6 +129,21 @@ export function buildFailedTailPayload(err: unknown): string { }); } +function buildFailedTailPayloadOrFallback(err: unknown): string { + try { + return buildFailedTailPayload(err); + } catch { + // Error.message and String(error) may execute hostile accessors. Preserve a + // bounded protocol terminal even when diagnostic serialization is unsafe. + return FAILED_TAIL_FALLBACK_PAYLOAD; + } +} + +export function failedTailFrame(encoder: TextEncoder, err: unknown): Uint8Array { + const payload = buildFailedTailPayloadOrFallback(err); + return encoder.encode(`\n\nevent: response.failed\ndata: ${payload}\n\n${DONE_SSE_FRAME_TEXT}`); +} + export type SseTerminalOutputBoundary = { feed(chunk: Uint8Array): Uint8Array; finish(): Uint8Array; @@ -244,7 +276,7 @@ export function relaySseWithFailedTail( // Preserve through the terminal block only, add the conventional sentinel // when there was no real [DONE] data event, then stop reading upstream. if (!terminalBoundary.doneSeen()) { - controller.enqueue(encoder.encode("data: [DONE]\n\n")); + controller.enqueue(doneFrame(encoder)); } closed = true; controller.close(); @@ -300,9 +332,8 @@ export function relaySseWithFailedTail( if (tailTerminal) { if (!terminalBoundary.doneSeen()) controller.enqueue(doneFrame(encoder)); } else { - const payload = buildFailedTailPayload(err); // Leading blank line terminates a partial SSE block so the failed frame parses cleanly. - controller.enqueue(encoder.encode(`\n\nevent: response.failed\ndata: ${payload}\n\ndata: [DONE]\n\n`)); + controller.enqueue(failedTailFrame(encoder, err)); } controller.close(); } catch { /* client already torn down */ } @@ -420,8 +451,11 @@ function policyFailurePayload(message: string, parsed: unknown): string { }; const root = asJsonRecord(parsed); const originalResponse = asJsonRecord(root?.response); + const preservedResponse = Object.fromEntries( + Object.entries(originalResponse ?? {}).filter(([key]) => key !== "incomplete_details"), + ); const response = { - ...(originalResponse ?? {}), + ...preservedResponse, status: "failed", error, last_error: error, @@ -444,12 +478,12 @@ function policyFailurePayload(message: string, parsed: unknown): string { }); } -function adapterEofIncompleteFrame(encoder: TextEncoder): Uint8Array { +export function adapterEofIncompleteFrame(encoder: TextEncoder): Uint8Array { return encoder.encode(`event: response.incomplete\ndata: ${ADAPTER_EOF_INCOMPLETE_PAYLOAD}\n\n`); } -function doneFrame(encoder: TextEncoder): Uint8Array { - return encoder.encode("data: [DONE]\n\n"); +export function doneFrame(encoder: TextEncoder): Uint8Array { + return encoder.encode(DONE_SSE_FRAME_TEXT); } export function terminalStatusFromSsePayload(payload: string): ResponsesTerminalStatus | null { diff --git a/tests/relay-eager.test.ts b/tests/relay-eager.test.ts index 4d5ba60ab2..0cb423a5a0 100644 --- a/tests/relay-eager.test.ts +++ b/tests/relay-eager.test.ts @@ -5,10 +5,16 @@ * via the injectable clock/short drain windows. */ import { describe, expect, test } from "bun:test"; -import { createSseInspector, MAX_TAIL_ERROR_MESSAGE_CHARS } from "../src/server/relay"; +import { + adapterEofIncompleteFrame, + createSseInspector, + doneFrame, + MAX_TAIL_ERROR_MESSAGE_CHARS, +} from "../src/server/relay"; import { relaySseEagerBounded, type EagerRelayHooks } from "../src/server/relay-eager"; import { createTranslatorBudget } from "../src/lib/translator-budget"; import type { RequestLogContext } from "../src/server/request-log"; +import { MAX_CLIENT_SSE_FRAME_BYTES } from "../src/server/sse-frame-buffer"; import { watchdogMs } from "./helpers/ci-watchdog"; const enc = new TextEncoder(); @@ -27,7 +33,11 @@ function countOccurrences(text: string, needle: string): number { function failedPayload(text: string): { type: string; - response: { status: string; error: { code: string; message: string } }; + response: { + status: string; + error: { code: string; message: string }; + incomplete_details?: unknown; + }; } { const payload = text.split(FAILED_EVENT_MARKER)[1]?.split("\n")[0]; if (!payload) throw new Error("missing response.failed payload"); @@ -273,34 +283,38 @@ describe("relaySseEagerBounded — inline payload rewrite (#864)", () => { const budget = createTranslatorBudget(); const up = controlledUpstream(); const ac = new AbortController(); - const { hooks, rec } = makeHooks(); - let resolveDone!: () => void; - const done = new Promise(resolve => { resolveDone = resolve; }); - const previousOnDone = hooks.onDone; - hooks.onDone = () => { - previousOnDone(); - resolveDone(); - }; - hooks.rewritePayload = (payload: string) => payload; - relaySseEagerBounded(up.stream, ac, hooks, { rewriteBudget: budget }); - - up.push(enc.encode(`data: {"type":"unterminated"`)); - // The shared terminal boundary now owns incomplete SSE framing, so the - // downstream rewrite stage never retains an unterminated block. - expect(budget.snapshot().currentBytes).toBe(0); - ac.abort(new Error("test abort")); - let timeout: ReturnType | undefined; - await Promise.race([ - done, - new Promise((_, reject) => { - timeout = setTimeout(() => reject(new Error("relay cleanup timed out")), watchdogMs(2_000)); - }), - ]).finally(() => { - if (timeout) clearTimeout(timeout); - }); - expect(budget.snapshot().currentBytes).toBe(0); - expect(rec.dones).toBe(1); - budget.dispose(); + try { + const { hooks, rec } = makeHooks(); + let resolveDone!: () => void; + const done = new Promise(resolve => { resolveDone = resolve; }); + const previousOnDone = hooks.onDone; + hooks.onDone = () => { + previousOnDone(); + resolveDone(); + }; + hooks.rewritePayload = (payload: string) => payload; + relaySseEagerBounded(up.stream, ac, hooks, { rewriteBudget: budget }); + + up.push(enc.encode(`data: {"type":"unterminated"`)); + // The shared terminal boundary now owns incomplete SSE framing, so the + // downstream rewrite stage never retains an unterminated block. + expect(budget.snapshot().currentBytes).toBe(0); + ac.abort(new Error("test abort")); + let timeout: ReturnType | undefined; + await Promise.race([ + done, + new Promise((_, reject) => { + timeout = setTimeout(() => reject(new Error("relay cleanup timed out")), watchdogMs(2_000)); + }), + ]).finally(() => { + if (timeout) clearTimeout(timeout); + }); + expect(budget.snapshot().currentBytes).toBe(0); + expect(rec.dones).toBe(1); + } finally { + ac.abort(new Error("test cleanup")); + budget.dispose(); + } }); test("blocks without a data field pass through untouched before the terminal", async () => { @@ -401,69 +415,80 @@ describe("relaySseEagerBounded — side-effect parity", () => { expect(rec.terminals).toEqual([]); }); - test("unframed EOF terminals are parsed once without an adapter_eof duplicate", async () => { - const cases = [ + test.each([ + [ + "completed", + "response.completed", + "response.completed", { type: "response.completed", - event: "response.completed", - payload: { - type: "response.completed", - sequence_number: 31, - response: { id: "resp-unframed-completed", status: "completed", output: [] }, - }, + sequence_number: 31, + response: { id: "resp-unframed-completed", status: "completed", output: [] }, }, + { status: "completed" }, + ], + [ + "failed", + "response.failed", + "response.failed", { type: "response.failed", - event: "response.failed", - payload: { - type: "response.failed", - sequence_number: 32, - response: { id: "resp-unframed-failed", status: "failed", output: [] }, - }, + sequence_number: 32, + response: { id: "resp-unframed-failed", status: "failed", output: [] }, }, + { status: "failed" }, + ], + [ + "incomplete", + "response.incomplete", + "response.incomplete", { type: "response.incomplete", - event: "response.incomplete", - payload: { - type: "response.incomplete", - sequence_number: 33, - response: { id: "resp-unframed-incomplete", status: "incomplete", output: [] }, - }, + sequence_number: 33, + response: { id: "resp-unframed-incomplete", status: "incomplete", output: [] }, }, + { status: "incomplete" }, + ], + [ + "policy error", + "error", + "response.failed", { type: "error", - event: "response.failed", - payload: { - type: "error", - sequence_number: 34, - response: { - id: "resp-unframed-policy", - output: [{ type: "message", id: "item-unframed-policy" }], - status: "failed", - }, - error: { - type: "invalid_request_error", - code: "cyber_policy", - message: "blocked by upstream policy", - }, + sequence_number: 34, + response: { + id: "resp-unframed-policy", + output: [{ type: "message", id: "item-unframed-policy" }], + status: "failed", + }, + error: { + type: "invalid_request_error", + code: "cyber_policy", + message: "blocked by upstream policy", }, }, - ] as const; - - for (const fixture of cases) { - const { hooks, rec } = makeHooks(); - const up = controlledUpstream(); - const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); - up.push(enc.encode(`event: ${fixture.type}\ndata: ${JSON.stringify(fixture.payload)}`)); - up.close(); + { status: "failed", httpStatus: 400 }, + ], + ] as const)("unframed EOF %s is parsed once without an adapter_eof duplicate", async ( + _name, + upstreamEvent, + clientEvent, + payload, + expectedTerminal, + ) => { + const { hooks, rec } = makeHooks(); + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); + up.push(enc.encode(`event: ${upstreamEvent}\ndata: ${JSON.stringify(payload)}`)); + up.close(); - const text = await readAll(relayed); - expect(text.match(/event: response\.(?:completed|failed|incomplete)/g)?.length).toBe(1); - expect(text).toContain(`event: ${fixture.event}`); - expect(text).not.toContain('"reason":"adapter_eof"'); - expect(countOccurrences(text, "data: [DONE]")).toBe(1); - expect(rec.synthetics).toEqual([]); - } + const text = await readAll(relayed); + expect(text.match(/event: response\.(?:completed|failed|incomplete)/g)?.length).toBe(1); + expect(text).toContain(`event: ${clientEvent}`); + expect(text).not.toContain('"reason":"adapter_eof"'); + expect(countOccurrences(text, "data: [DONE]")).toBe(1); + expect(rec.terminals).toEqual([expectedTerminal]); + expect(rec.synthetics).toEqual([]); }); test("policy response.incomplete is rewritten to one failed terminal with 400 accounting", async () => { @@ -495,6 +520,8 @@ describe("relaySseEagerBounded — side-effect parity", () => { expect(text).toContain('"id":"resp-policy"'); expect(text).toContain('"sequence_number":7'); expect(text).toContain('"output":[{"type":"message","id":"item-1"}]'); + expect(failedPayload(text).response.incomplete_details).toBeUndefined(); + expect(text).not.toContain("content_filter"); expect(countOccurrences(text, "data: [DONE]")).toBe(1); expect(rec.terminals).toEqual([{ status: "failed", httpStatus: 400 }]); expect(rec.synthetics).toEqual([]); @@ -602,10 +629,11 @@ describe("relaySseEagerBounded — side-effect parity", () => { message: "blocked by upstream policy", }, }); - // The configured 4 MiB client framer admits 4096 blocks per feed. The - // policy terminal is first, followed by 4096 empty blocks, which used to + const frameLimit = Math.ceil(MAX_CLIENT_SSE_FRAME_BYTES / 1024); + // The client framer derives its per-feed frame cap from the byte cap. The + // policy terminal is first, followed by a full cap of empty blocks, which used to // turn the already-committed policy response into an upstream_reset 502. - up.push(enc.encode(`event: error\ndata: ${policy}\n\n${"\n\n".repeat(4096)}`)); + up.push(enc.encode(`event: error\ndata: ${policy}\n\n${"\n\n".repeat(frameLimit)}`)); up.close(); const text = await readAll(relayed); @@ -634,7 +662,7 @@ describe("relaySseEagerBounded — side-effect parity", () => { message: "blocked by upstream policy", }, }); - const oversizedTail = new Uint8Array(4 * 1024 * 1024 + 1).fill(120); + const oversizedTail = new Uint8Array(MAX_CLIENT_SSE_FRAME_BYTES + 1).fill(120); up.push(joinBytes([ enc.encode(`event: error\ndata: ${policy}\n\n`), oversizedTail, @@ -650,6 +678,56 @@ describe("relaySseEagerBounded — side-effect parity", () => { expect(rec.terminals).toEqual([{ status: "failed", httpStatus: 400 }]); }); + test("oversized nonterminal before a same-chunk terminal emits one client fallback", async () => { + const { hooks, rec } = makeHooks(); + const up = controlledUpstream(); + const upstreamAc = new AbortController(); + const relayed = relaySseEagerBounded(up.stream, upstreamAc, hooks); + const oversizedNonterminal = new Uint8Array(MAX_CLIENT_SSE_FRAME_BYTES + 1).fill(120); + up.push(joinBytes([oversizedNonterminal, enc.encode("\n\n"), sse(COMPLETED)])); + + const text = await readAll(relayed); + await settle(); + expect(countOccurrences(text, "event: response.failed")).toBe(1); + expect(countOccurrences(text, "event: response.completed")).toBe(0); + expect(countOccurrences(text, "data: [DONE]")).toBe(1); + expect(text).toContain('"code":"upstream_reset"'); + expect(rec.terminals).toEqual([{ status: "completed" }]); + expect(rec.synthetics).toEqual([]); + expect(rec.dones).toBe(1); + expect(upstreamAc.signal.aborted).toBe(true); + }); + + test("inspector-only terminal cannot suppress the oversized-frame client fallback", async () => { + const inspector = createSseInspector({}); + const synthetics: string[] = []; + let doneCount = 0; + const hooks: EagerRelayHooks = { + inspectChunk: chunk => inspector.feed(chunk), + finishInspection: () => inspector.finish(), + disposeInspection: () => inspector.dispose(), + sawTerminal: () => inspector.terminalSeen(), + onSynthetic: kind => synthetics.push(kind), + onClientCancel() {}, + onDone: () => { doneCount += 1; }, + }; + const up = controlledUpstream(); + const upstreamAc = new AbortController(); + const relayed = relaySseEagerBounded(up.stream, upstreamAc, hooks); + const oversizedNonterminal = new Uint8Array(MAX_CLIENT_SSE_FRAME_BYTES + 1).fill(120); + up.push(joinBytes([oversizedNonterminal, enc.encode("\n\n"), sse(COMPLETED)])); + + const text = await readAll(relayed); + await settle(); + expect(inspector.terminalSeen()).toBe(true); + expect(inspector.reported()).toBe(false); + expect(countOccurrences(text, "event: response.failed")).toBe(1); + expect(countOccurrences(text, "data: [DONE]")).toBe(1); + expect(synthetics).toEqual([]); + expect(doneCount).toBe(1); + expect(upstreamAc.signal.aborted).toBe(true); + }); + test("eager preserves an unframed terminal before a reader error", async () => { const cases = [ { @@ -797,36 +875,68 @@ describe("relaySseEagerBounded — side-effect parity", () => { expect(rec.dones).toBe(1); }); - test("eager rewrite-budget exhaustion emits one bounded failed envelope and releases the budget", async () => { - const budget = createTranslatorBudget({ maxTurnBytes: 1 }); - const { hooks, rec } = makeHooks(); - hooks.rewritePayload = payload => payload; - const up = controlledUpstream(); - const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks, { rewriteBudget: budget }); - up.push(enc.encode(`event: response.completed\ndata: ${JSON.stringify({ - type: "response.completed", - sequence_number: 72, - response: { - id: "resp-budget-failure-secret", - status: "completed", - output: [{ type: "message", text: "raw-budget-secret" }], + test("terminal rewrite fallback aborts and cancels a still-open upstream", async () => { + let sourceCancels = 0; + let terminalSent = false; + const source = new ReadableStream({ + pull(controller) { + if (terminalSent) return; + terminalSent = true; + controller.enqueue(sse(COMPLETED)); }, - })}`)); - up.fail(new Error("socket reset after budget failure")); + cancel() { sourceCancels += 1; }, + }, { highWaterMark: 0 }); + const { hooks, rec } = makeHooks(); + hooks.rewriteBlocks = () => { throw new Error("terminal rewrite failed"); }; + const upstreamAc = new AbortController(); + const relayed = relaySseEagerBounded(source, upstreamAc, hooks); const text = await readAll(relayed); - expect(text.length).toBeGreaterThan(0); - expect(text.match(/event: response\.failed/g)?.length).toBe(1); - expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); - expect(text).toContain('"code":"translation_buffer_limit"'); - expect(text).not.toContain("resp-budget-failure-secret"); - expect(text).not.toContain("raw-budget-secret"); - expect(text).not.toContain('"sequence_number":72'); + await settle(); + expect(countOccurrences(text, "event: response.failed")).toBe(1); + expect(countOccurrences(text, "data: [DONE]")).toBe(1); expect(rec.terminals).toEqual([{ status: "completed" }]); expect(rec.synthetics).toEqual([]); expect(rec.dones).toBe(1); - expect(budget.snapshot().currentBytes).toBe(0); - budget.dispose(); + expect(upstreamAc.signal.aborted).toBe(true); + expect(sourceCancels).toBe(1); + }); + + test("eager rewrite-budget exhaustion emits one bounded failed envelope and releases the budget", async () => { + const budget = createTranslatorBudget({ maxTurnBytes: 1 }); + const upstreamAc = new AbortController(); + try { + const { hooks, rec } = makeHooks(); + hooks.rewritePayload = payload => payload; + const up = controlledUpstream(); + const relayed = relaySseEagerBounded(up.stream, upstreamAc, hooks, { rewriteBudget: budget }); + up.push(enc.encode(`event: response.completed\ndata: ${JSON.stringify({ + type: "response.completed", + sequence_number: 72, + response: { + id: "resp-budget-failure-secret", + status: "completed", + output: [{ type: "message", text: "raw-budget-secret" }], + }, + })}`)); + up.fail(new Error("socket reset after budget failure")); + + const text = await readAll(relayed); + expect(text.length).toBeGreaterThan(0); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); + expect(text).toContain('"code":"translation_buffer_limit"'); + expect(text).not.toContain("resp-budget-failure-secret"); + expect(text).not.toContain("raw-budget-secret"); + expect(text).not.toContain('"sequence_number":72'); + expect(rec.terminals).toEqual([{ status: "completed" }]); + expect(rec.synthetics).toEqual([]); + expect(rec.dones).toBe(1); + expect(budget.snapshot().currentBytes).toBe(0); + } finally { + upstreamAc.abort(new Error("test cleanup")); + budget.dispose(); + } }); test("eager clean EOF rewrite failure emits one safe failed envelope", async () => { @@ -938,9 +1048,14 @@ describe("relaySseEagerBounded — bounded queue", () => { if (done) break; total += value.byteLength; } - // Clean EOF now carries the explicit adapter_eof incomplete terminal and - // one [DONE] sentinel in addition to the relayed bytes. - expect(total).toBeGreaterThan(36); + // The unterminated client block is made dispatchable with one blank-line + // delimiter, then clean EOF adds exactly one adapter_eof terminal and DONE. + expect(total).toBe( + 3 * chunk.byteLength + + enc.encode("\n\n").byteLength + + adapterEofIncompleteFrame(enc).byteLength + + doneFrame(enc).byteLength, + ); }); test("(f) cancel while paused wakes the gate — onDone fires, no deadlock", async () => { diff --git a/tests/request-log.test.ts b/tests/request-log.test.ts index 6cfaf43681..4e57325112 100644 --- a/tests/request-log.test.ts +++ b/tests/request-log.test.ts @@ -1020,6 +1020,44 @@ describe("request log metadata", () => { }); }); + test("deferred SSE logging recognizes policy text from incomplete_details.message", async () => { + const entries: RequestLogEntry[] = []; + const policyMessage = "This request was flagged for possible cybersecurity risk."; + const payload = JSON.stringify({ + type: "response.incomplete", + response: { + id: "resp-policy-incomplete-message", + status: "incomplete", + incomplete_details: { + reason: "content_filter", + message: policyMessage, + }, + }, + }); + const response = responseWithDeferredRequestLog( + new Response(new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(`event: response.incomplete\ndata: ${payload}\n\n`)); + controller.close(); + }, + }), { status: 200, headers: { "content-type": "text/event-stream" } }), + "ocx-test-cyber-policy-incomplete-message", + Date.now(), + { model: "gpt-5.6-sol", provider: "openai" }, + entry => entries.push(entry), + ); + + await response.text(); + expect(entries).toHaveLength(1); + expect(entries[0]).toMatchObject({ + terminalStatus: "failed", + upstreamError: policyMessage, + status: 400, + errorCode: "cyber_policy", + closeReason: "terminal", + }); + }); + test("deferred SSE logging maps a policy top-level error to failed 400", async () => { const entries: RequestLogEntry[] = []; const payload = JSON.stringify({ diff --git a/tests/sse-failed-tail.test.ts b/tests/sse-failed-tail.test.ts index 70876dafdb..6ed8834ec4 100644 --- a/tests/sse-failed-tail.test.ts +++ b/tests/sse-failed-tail.test.ts @@ -204,4 +204,24 @@ describe("relaySseWithFailedTail", () => { } } }); + + test("legacy and eager use the same bounded fallback when an error message getter throws", async () => { + const error = new Error("unreachable"); + Object.defineProperty(error, "message", { + get() { throw new Error("hostile message getter"); }, + }); + const legacy = await drain(relaySseWithFailedTail( + sourceStream([], { failAfter: true, error }), + new AbortController(), + )); + const eager = await drain(relaySseEagerBounded( + sourceStream([], { failAfter: true, error }), + new AbortController(), + parityHooks, + )); + + expect(encoder.encode(eager)).toEqual(encoder.encode(legacy)); + expect(failedMessage(eager)).toBe("Upstream stream terminated unexpectedly"); + expect(eager.split("data: [DONE]").length - 1).toBe(1); + }); }); From bf1b2806ef37d2c1f626c83893e05ba69766d57c Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 05:35:04 +0900 Subject: [PATCH 06/19] test(responses): pin terminal relay parity --- .../content/docs/reference/proxy-formats.md | 13 +++ tests/sse-failed-tail.test.ts | 85 ++++++++++++++++++- 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/docs-site/src/content/docs/reference/proxy-formats.md b/docs-site/src/content/docs/reference/proxy-formats.md index 2792bdf0f8..5a7de86075 100644 --- a/docs-site/src/content/docs/reference/proxy-formats.md +++ b/docs-site/src/content/docs/reference/proxy-formats.md @@ -72,6 +72,19 @@ bridge, the same condition emits a 502 `websocket_protocol_error` and cancels th A complete Responses terminal frame is authoritative: oversized or malformed trailing bytes after that terminal are dropped rather than replacing the completed turn with a transport failure. +:::note +For native passthrough, a Responses terminal event is authoritative. A premature `data: [DONE]` is +held until that event; if upstream reaches EOF first, the proxy emits one `response.incomplete` with +`incomplete_details.reason: "adapter_eof"`, followed by one `data: [DONE]`. Syntactically valid +delimiter-less terminal JSON at EOF is accepted exactly once; malformed or truncated terminal-shaped +JSON remains incomplete, and model-scoped terminal repair remains fail-closed for unframed +terminal-like suffixes. High-confidence `cyber_policy` terminal shapes normalize to +`response.failed` with `error.code: "cyber_policy"` for semantic logging/accounting (status 400), +while an already-started streamed HTTP response remains 200. This boundary does not retry or replay +the committed request and does not resolve [#2423](https://github.com/lidge-jun/opencodex/issues/2423) +or [#2486](https://github.com/lidge-jun/opencodex/issues/2486). +::: + For canonical ChatGPT forward streaming, stable Bun 1.4.0 or newer may transparently use Codex's upstream WebSocket transport. Bundled Bun 1.3.14, prereleases, and unverifiable runtime identities use HTTP/SSE. The upstream WS adapter keeps the same downstream SSE contract, caps both diff --git a/tests/sse-failed-tail.test.ts b/tests/sse-failed-tail.test.ts index 6ed8834ec4..7be3e288ec 100644 --- a/tests/sse-failed-tail.test.ts +++ b/tests/sse-failed-tail.test.ts @@ -7,7 +7,7 @@ import { TranslatorBudgetExceededError } from "../src/lib/translator-budget"; const encoder = new TextEncoder(); const decoder = new TextDecoder(); -function sourceStream(chunks: string[], opts: { failAfter?: boolean; error?: Error } = {}): ReadableStream { +function sourceStream(chunks: readonly string[], opts: { failAfter?: boolean; error?: Error } = {}): ReadableStream { let i = 0; return new ReadableStream({ pull(controller) { @@ -49,6 +49,19 @@ function failedMessage(text: string): string { return (JSON.parse(payload) as { response: { error: { message: string } } }).response.error.message; } +function terminalEvents(text: string): string[] { + return text + .split(/\r?\n/) + .flatMap(line => { + const match = line.match(/^event: (response\.(?:completed|failed|incomplete))$/); + return match ? [match[1]!] : []; + }); +} + +function doneEvents(text: string): string[] { + return text.split(/\r?\n/).filter(line => line === "data: [DONE]"); +} + describe("relaySseWithFailedTail", () => { test("relays a healthy stream verbatim with no injected frame", async () => { const upstream = new AbortController(); @@ -224,4 +237,74 @@ describe("relaySseWithFailedTail", () => { expect(failedMessage(eager)).toBe("Upstream stream terminated unexpectedly"); expect(eager.split("data: [DONE]").length - 1).toBe(1); }); + + test.each([ + [ + "clean EOF without a terminal", + ['event: response.created\ndata: {"type":"response.created"}\n\n'], + "incomplete", + ], + [ + "premature DONE followed by EOF", + ["data: [DONE]\n\n"], + "incomplete", + ], + [ + "valid delimiter-less terminal with LF", + ['event: response.completed\ndata: {"type":"response.completed","response":{"status":"completed"}}'], + "completed", + ], + [ + "valid delimiter-less terminal with CRLF", + ['event: response.completed\r\ndata: {"type":"response.completed","response":{"status":"completed"}}'], + "completed", + ], + [ + "high-confidence cyber_policy terminal normalization", + [`event: error\ndata: ${JSON.stringify({ + type: "error", + response: { id: "resp-policy-parity", status: "failed", output: [] }, + error: { + type: "invalid_request_error", + code: "cyber_policy", + message: "blocked by upstream policy", + }, + })}\n\n`], + "policy-failed", + ], + [ + "malformed delimiter-less terminal-shaped JSON", + ['data: {"type":"response.completed","response":{"status":"completed"}'], + "malformed", + ], + ] as const)("pull/eager parity: %s", async (_name, chunks, expected) => { + const pull = await drain(relaySseWithFailedTail(sourceStream(chunks), new AbortController())); + const eager = await drain(relaySseEagerBounded( + sourceStream(chunks), + new AbortController(), + parityHooks, + )); + + expect(encoder.encode(eager)).toEqual(encoder.encode(pull)); + + for (const text of [pull, eager]) { + expect(terminalEvents(text)).toHaveLength(1); + expect(doneEvents(text)).toHaveLength(1); + if (expected === "incomplete") { + expect(terminalEvents(text)).toEqual(["response.incomplete"]); + expect(text).toContain('"reason":"adapter_eof"'); + } else if (expected === "completed") { + expect(terminalEvents(text)).toEqual(["response.completed"]); + } else if (expected === "policy-failed") { + expect(terminalEvents(text)).toEqual(["response.failed"]); + expect(text).toContain('"code":"cyber_policy"'); + expect(text).not.toContain('"reason":"adapter_eof"'); + } else { + expect(terminalEvents(text)).toEqual(["response.incomplete"]); + expect(text).not.toContain("event: response.completed"); + expect(text).toContain('data: {"type":"response.completed","response":{"status":"completed"}'); + expect(text).toContain('"reason":"adapter_eof"'); + } + } + }); }); From b77b4da1781153d8aee8fa17d29706515b375e06 Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:35:04 +0900 Subject: [PATCH 07/19] fix(lab): await producer child main --- src/lab/fabric/producer-child.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lab/fabric/producer-child.ts b/src/lab/fabric/producer-child.ts index f42c4e3cd3..4063624574 100644 --- a/src/lab/fabric/producer-child.ts +++ b/src/lab/fabric/producer-child.ts @@ -128,7 +128,7 @@ async function main(): Promise { writeLine({ type: "result", patch }); } -main().catch((error: unknown) => { +await main().catch((error: unknown) => { writeLine({ type: "error", code: "harness_failure", From 9de0d56bc8122bf531202b351eeed0ff4b2bd248 Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:46:42 +0900 Subject: [PATCH 08/19] test: use executable PowerShell fixture on Windows --- tests/codex-app-server-processes.test.ts | 65 +++++--------- tests/helpers/windows-power-shell-fixture.ts | 89 ++++++++++++++++++++ tests/multi-agent-compat.test.ts | 25 +++--- 3 files changed, 123 insertions(+), 56 deletions(-) create mode 100644 tests/helpers/windows-power-shell-fixture.ts diff --git a/tests/codex-app-server-processes.test.ts b/tests/codex-app-server-processes.test.ts index 5c6c828980..a6ce9fe34a 100644 --- a/tests/codex-app-server-processes.test.ts +++ b/tests/codex-app-server-processes.test.ts @@ -1,9 +1,9 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test } from "bun:test"; import { spawn } from "node:child_process"; -import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { readFileSync } from "node:fs"; import { join } from "node:path"; import { setTrustedWindowsElevationExecutablesForTests } from "../src/lib/windows-elevation"; +import { createWindowsPowerShellFixture, type WindowsPowerShellFixture } from "./helpers/windows-power-shell-fixture"; import { afterCatalogWriteHandleAppServers, attachStaleAppServerHint, @@ -26,30 +26,11 @@ import { describe("collectCodexAppServerCatalogState (#857)", () => { const APP_SERVER_CMD = "/usr/local/bin/codex app-server"; -/** - * A stand-in for powershell.exe that stalls, prints `line`, and exits. - * - * Platform-shaped on purpose: `execFile` launches its target directly with no shell, so a - * POSIX `.sh` script is not an executable on Windows — and Windows is the platform this - * whole fix exists for, with its own CI shard running this suite. On Windows the fake is a - * `.cmd` invoked through `cmd.exe`; elsewhere it is a shell script. - */ -function writeStallingFakePowerShell(dir: string, line: string): string { - if (process.platform === "win32") { - const cmd = join(dir, "fake-powershell.cmd"); - writeFileSync(cmd, [ - "@echo off", - // ~200ms without depending on timeout.exe, which refuses a redirected stdin. - "ping -n 1 -w 200 192.0.2.1 >nul 2>&1", - `echo ${line.replace(/\t/g, "\t")}`, - ].join("\r\n")); - return cmd; - } - const sh = join(dir, "fake-powershell.sh"); - writeFileSync(sh, ["#!/bin/sh", "sleep 0.2", `printf '%s\\n' '${line}'`].join("\n")); - chmodSync(sh, 0o755); - return sh; -} +let stallingFakePowerShell: WindowsPowerShellFixture; +beforeAll(async () => { + stallingFakePowerShell = await createWindowsPowerShellFixture(); +}); +afterAll(() => stallingFakePowerShell?.cleanup()); test("not_running when no app-server process exists", () => { const status = collectCodexAppServerCatalogState({ @@ -134,9 +115,7 @@ function writeStallingFakePowerShell(dir: string, line: string): string { // asynchronous one does not. That difference is the entire content of #1852. test("the default Windows request path keeps the event loop alive through both PowerShell calls (#1852)", async () => { resetCodexAppServerCatalogStateCache(); - const dir = mkdtempSync(join(tmpdir(), "ocx-ps-fake-")); - const fake = writeStallingFakePowerShell(dir, `42\t${APP_SERVER_CMD}\tCONTOSO\\jun`); - setTrustedWindowsElevationExecutablesForTests({ powershell: fake }); + setTrustedWindowsElevationExecutablesForTests({ powershell: stallingFakePowerShell.executable }); // Phase signal instead of a timer count. A callback tally has to pick a threshold // between "sync" and "async" observations, and `setInterval` makes no catch-up @@ -157,7 +136,6 @@ function writeStallingFakePowerShell(dir: string, line: string): string { } finally { clearInterval(beat); setTrustedWindowsElevationExecutablesForTests(null); - rmSync(dir, { recursive: true, force: true }); resetCodexAppServerCatalogStateCache(); } @@ -172,9 +150,7 @@ function writeStallingFakePowerShell(dir: string, line: string): string { // blocking when an app-server exists, so it needs its own oracle. test("the default Windows start-time discovery keeps the event loop alive (#1852)", async () => { resetCodexAppServerCatalogStateCache(); - const dir = mkdtempSync(join(tmpdir(), "ocx-ps-start-")); - const fake = writeStallingFakePowerShell(dir, `42\t${APP_SERVER_CMD}\tCONTOSO\\jun`); - setTrustedWindowsElevationExecutablesForTests({ powershell: fake }); + setTrustedWindowsElevationExecutablesForTests({ powershell: stallingFakePowerShell.executable }); let loopRanDuringExec = false; const beat = setInterval(() => { loopRanDuringExec = true; }, 5); @@ -188,7 +164,6 @@ function writeStallingFakePowerShell(dir: string, line: string): string { } finally { clearInterval(beat); setTrustedWindowsElevationExecutablesForTests(null); - rmSync(dir, { recursive: true, force: true }); resetCodexAppServerCatalogStateCache(); } @@ -922,14 +897,20 @@ describe("warnIfStaleCodexAppServersAfterStartupWrite (#1046)", () => { * invalidation is verified by reading it, not by a test that cannot fail. */ test("a defaulted read is memoized, and invalidation is what clears it", () => { - resetCodexAppServerCatalogStateCache(); - const first = collectCodexAppServerCatalogState(); - const second = collectCodexAppServerCatalogState(); - // Same object identity: the second call served the memo rather than recomputing. - expect(second).toBe(first); + const realDateNow = Date.now; + Date.now = () => 1_000; + try { + resetCodexAppServerCatalogStateCache(); + const first = collectCodexAppServerCatalogState(); + const second = collectCodexAppServerCatalogState(); + // Same object identity: the second call served the memo rather than recomputing. + expect(second).toBe(first); - resetCodexAppServerCatalogStateCache(); - expect(collectCodexAppServerCatalogState()).not.toBe(first); + resetCodexAppServerCatalogStateCache(); + expect(collectCodexAppServerCatalogState()).not.toBe(first); + } finally { + Date.now = realDateNow; + } }); /* diff --git a/tests/helpers/windows-power-shell-fixture.ts b/tests/helpers/windows-power-shell-fixture.ts new file mode 100644 index 0000000000..3c508565cd --- /dev/null +++ b/tests/helpers/windows-power-shell-fixture.ts @@ -0,0 +1,89 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +export interface WindowsPowerShellFixture { + executable: string; + cleanup: () => void | Promise; +} + +let sharedFixture: Promise | undefined; + +/** + * Build a real Windows executable for tests that exercise the default execFile path. + * + * A .cmd file is not a CreateProcess target, so Node/Bun's shell-free execFile rejects + * it with EINVAL on Windows. The compiled fixture consumes the same PowerShell-shaped + * argv as production, waits long enough for an interval to run, and emits deterministic + * rows for both the process and start-time queries. On POSIX, retain the small shell + * fixture because the production Windows branch is only reached after the platform is + * explicitly faked by the tests. + */ +export function createWindowsPowerShellFixture(): Promise { + if (sharedFixture) return sharedFixture; + sharedFixture = process.platform === "win32" + ? buildWindowsExecutableFixture() + : Promise.resolve(createPosixShellFixture()); + return sharedFixture; +} + +async function buildWindowsExecutableFixture(): Promise { + const dir = mkdtempSync(join(tmpdir(), "ocx-ps-fixture-")); + const source = join(dir, "fake-powershell.ts"); + const executable = join(dir, "fake-powershell.exe"); + writeFileSync(source, [ + "const command = process.argv.slice(2).join(' ');", + "await new Promise(resolve => setTimeout(resolve, 200));", + "if (command.includes('CreationDate')) {", + " process.stdout.write('42\\t2020-01-01T00:00:00.000Z\\n');", + "} else {", + " process.stdout.write('42\\t/usr/local/bin/codex app-server\\tCONTOSO\\\\jun\\n');", + "}", + ].join("\n")); + + const result = await Bun.build({ + entrypoints: [source], + compile: { target: "bun-windows-x64", outfile: executable }, + }); + if (!result.success) { + rmSync(dir, { recursive: true, force: true }); + const details = result.logs.map(log => log.message).join("\n"); + throw new Error(`Could not compile Windows PowerShell test fixture: ${details}`); + } + return { + executable, + cleanup: () => removeFixtureDirectory(dir), + }; +} + +function createPosixShellFixture(): WindowsPowerShellFixture { + const dir = mkdtempSync(join(tmpdir(), "ocx-ps-fixture-")); + const executable = join(dir, "fake-powershell.sh"); + writeFileSync(executable, [ + "#!/bin/sh", + "sleep 0.2", + "printf '42\\t/usr/local/bin/codex app-server\\tCONTOSO\\\\jun\\n'", + ].join("\n"), { mode: 0o755 }); + return { + executable, + cleanup: () => rmSync(dir, { recursive: true, force: true }), + }; +} + +async function removeFixtureDirectory(dir: string): Promise { + // Windows can keep a just-exited compiled child image open for a short interval. + // Retry the temp cleanup so an antivirus/file-close race does not turn an otherwise + // passing test file into an unnamed afterAll failure. + for (let attempt = 0; attempt < 40; attempt += 1) { + try { + rmSync(dir, { recursive: true, force: true }); + return; + } catch (error) { + const code = error && typeof error === "object" && "code" in error + ? (error as { code?: unknown }).code + : undefined; + if (code !== "EBUSY") throw error; + await Bun.sleep(50); + } + } +} diff --git a/tests/multi-agent-compat.test.ts b/tests/multi-agent-compat.test.ts index 5cb4217185..0a29d4e9fb 100644 --- a/tests/multi-agent-compat.test.ts +++ b/tests/multi-agent-compat.test.ts @@ -3,8 +3,8 @@ * models are no longer v1-pinned by ocx, but legacy/v1-surface requests still need * the Proactive delegation prompt when they arrive with the synthetic top tier. */ -import { afterAll, afterEach, describe, expect, test } from "bun:test"; -import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { afterAll, afterEach, beforeAll, describe, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { injectDeveloperMessage, multiAgentGuidanceText, sanitizeEncryptedContentInPlace } from "../src/server/responses"; @@ -13,6 +13,7 @@ import type { OcxParsedRequest } from "../src/types"; import { CODEX_ACCOUNT_BOUND_CATALOG_KIND, effectiveSubagentRoster } from "../src/codex/catalog"; import { collectCodexAppServerCatalogState, resetCodexAppServerCatalogStateCache } from "../src/codex/app-server-processes"; import { setTrustedWindowsElevationExecutablesForTests } from "../src/lib/windows-elevation"; +import { createWindowsPowerShellFixture, type WindowsPowerShellFixture } from "./helpers/windows-power-shell-fixture"; import { clearDebugSettings, setDebugSettings } from "../src/lib/debug-settings"; import { getInjectionDebugLogEntries, @@ -39,6 +40,13 @@ afterAll(() => { else process.env.OPENCODEX_APP_SERVER_CATALOG_STATE_OVERRIDE = savedCatalogStateOverride; }); +let stallingFakePowerShell: WindowsPowerShellFixture; +beforeAll(async () => { + stallingFakePowerShell = await createWindowsPowerShellFixture(); +}); + +afterAll(() => stallingFakePowerShell?.cleanup()); + function codexHomeFixture(configToml: string): string { const dir = mkdtempSync(join(tmpdir(), "ocx-v1pin-")); mkdirSync(dir, { recursive: true }); @@ -135,17 +143,7 @@ describe("multiAgentGuidanceText", () => { // underlying process enumeration observable through the trusted-executable seam: // a stalling fake stands in for a slow CIM walk. The async request collector leaves // the loop free; the synchronous collector parks it. - const fakeDir = mkdtempSync(join(tmpdir(), "ocx-collab-ps-")); - // Platform-shaped: execFile takes no shell, so a POSIX script is not executable on - // Windows — the platform this fix targets, whose CI shard runs this suite. - const fake = join(fakeDir, process.platform === "win32" ? "fake-powershell.cmd" : "fake-powershell.sh"); - if (process.platform === "win32") { - writeFileSync(fake, ["@echo off", "ping -n 1 -w 200 192.0.2.1 >nul 2>&1"].join("\r\n")); - } else { - writeFileSync(fake, ["#!/bin/sh", "sleep 0.2", "printf ''"].join("\n")); - chmodSync(fake, 0o755); - } - setTrustedWindowsElevationExecutablesForTests({ powershell: fake }); + setTrustedWindowsElevationExecutablesForTests({ powershell: stallingFakePowerShell.executable }); const realPlatform = Object.getOwnPropertyDescriptor(process, "platform")!; Object.defineProperty(process, "platform", { value: "win32", configurable: true }); resetCodexAppServerCatalogStateCache(); @@ -168,7 +166,6 @@ describe("multiAgentGuidanceText", () => { clearInterval(beat); Object.defineProperty(process, "platform", realPlatform); setTrustedWindowsElevationExecutablesForTests(null); - rmSync(fakeDir, { recursive: true, force: true }); resetCodexAppServerCatalogStateCache(); process.env.OPENCODEX_APP_SERVER_CATALOG_STATE_OVERRIDE = "fresh"; } From 0849af3036834106536876caf8f729090dfedc40 Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:51:13 +0900 Subject: [PATCH 09/19] test: isolate Windows service-home probes --- tests/codex-sync-api.test.ts | 12 ++++- tests/helpers/owned-service-home-preload.ts | 58 +++++++++++++++++++++ tests/helpers/owned-service-home.ts | 24 ++++++++- tests/owned-service-home.test.ts | 52 ++++++++++++++++++ 4 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 tests/helpers/owned-service-home-preload.ts create mode 100644 tests/owned-service-home.test.ts diff --git a/tests/codex-sync-api.test.ts b/tests/codex-sync-api.test.ts index 8810633d0d..2f49737836 100644 --- a/tests/codex-sync-api.test.ts +++ b/tests/codex-sync-api.test.ts @@ -18,6 +18,8 @@ let prevCodexHome: string | undefined; let prevOpenCodexHome: string | undefined; let prevHome: string | undefined; let prevUserProfile: string | undefined; +let prevBunOptions: string | undefined; +let prevServiceProbeFlag: string | undefined; const config = { port: 0, @@ -34,7 +36,9 @@ const config = { } as OcxConfig; function claimTempHome(codexHome: string, ocxHome: string, home: string): void { - claimOwnedServiceHome(codexHome, ocxHome, home); + for (const [key, value] of Object.entries(claimOwnedServiceHome(codexHome, ocxHome, home).env)) { + process.env[key] = value; + } } const admittedSync = () => ({ kind: "admitted" as const }); @@ -58,6 +62,8 @@ describe("GUI/CLI Codex sync backend", () => { prevOpenCodexHome = process.env.OPENCODEX_HOME; prevHome = process.env.HOME; prevUserProfile = process.env.USERPROFILE; + prevBunOptions = process.env.BUN_OPTIONS; + prevServiceProbeFlag = process.env.OCX_TEST_SERVICE_HOME_PROBE; if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); mkdirSync(TEST_CODEX_HOME, { recursive: true }); mkdirSync(TEST_OCX_HOME, { recursive: true }); @@ -80,6 +86,10 @@ describe("GUI/CLI Codex sync backend", () => { else process.env.HOME = prevHome; if (prevUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = prevUserProfile; + if (prevBunOptions === undefined) delete process.env.BUN_OPTIONS; + else process.env.BUN_OPTIONS = prevBunOptions; + if (prevServiceProbeFlag === undefined) delete process.env.OCX_TEST_SERVICE_HOME_PROBE; + else process.env.OCX_TEST_SERVICE_HOME_PROBE = prevServiceProbeFlag; if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); }); test("returns the structured sync result used by POST /api/sync", async () => { diff --git a/tests/helpers/owned-service-home-preload.ts b/tests/helpers/owned-service-home-preload.ts new file mode 100644 index 0000000000..7152437b95 --- /dev/null +++ b/tests/helpers/owned-service-home-preload.ts @@ -0,0 +1,58 @@ +/** + * Test-only Windows service-manager observation seam. + * + * The real admission path must keep asking the trusted System32 binaries. A + * child launched by `claimOwnedServiceHome` gets this preload explicitly, and + * only then are the read-only `/query` calls answered as an absent manager. + * No production module reads this flag or imports this file; all CLI/HTTP and + * admission code after the probe remains the real implementation. + */ +import { mock } from "bun:test"; +import childProcess from "node:child_process"; + +const ENABLED = process.platform === "win32" && process.env.OCX_TEST_SERVICE_HOME_PROBE === "1"; + +if (ENABLED) { + const realSpawnSync = childProcess.spawnSync; + + const fakeSpawnSync = ((...input: Parameters) => { + const [file, second, third] = input; + const args = Array.isArray(second) ? second : []; + const options = Array.isArray(second) ? third : second; + const name = typeof file === "string" + ? file.replaceAll("\\", "/").split("/").pop()?.toLowerCase() + : undefined; + const first = typeof args[0] === "string" ? args[0].toLowerCase() : ""; + const secondArg = typeof args[1] === "string" ? args[1].toLowerCase() : ""; + const isSchedulerQuery = name === "schtasks.exe" && first === "/query"; + const isNativeServiceQuery = name === "sc.exe" + && first === "query" + && secondArg === "opencodex-proxy-native"; + + if (!isSchedulerQuery && !isNativeServiceQuery) return realSpawnSync(...input); + + const raw = typeof options === "object" + && options !== null + && "encoding" in options + && options.encoding === "buffer"; + const message = isNativeServiceQuery + ? "[SC] OpenService FAILED 1060: The specified service does not exist." + : "ERROR: The system cannot find the file specified."; + const stdout = raw ? Buffer.alloc(0) : ""; + const stderr = raw ? Buffer.from(message, "utf8") : message; + return { + status: 1, + signal: null, + output: [null, stdout, stderr], + pid: undefined, + error: undefined, + stdout, + stderr, + } as ReturnType; + }) as typeof realSpawnSync; + + // Bun's ESM namespace binding for `node:child_process` is immutable, while + // `mock.module` replaces the module before production imports its named + // `spawnSync` binding. Preserve every other child_process API verbatim. + mock.module("node:child_process", () => ({ ...childProcess, spawnSync: fakeSpawnSync })); +} diff --git a/tests/helpers/owned-service-home.ts b/tests/helpers/owned-service-home.ts index 6880ad15f8..538898ce8d 100644 --- a/tests/helpers/owned-service-home.ts +++ b/tests/helpers/owned-service-home.ts @@ -1,11 +1,30 @@ import { chmodSync, mkdirSync, writeFileSync } from "node:fs"; -import { delimiter, join } from "node:path"; +import { delimiter, join, resolve } from "node:path"; export interface OwnedServiceHome { - /** Add this to child-process environments so Linux never reaches the host bus. */ + /** Add this to child-process environments so service probes never reach host state. */ readonly env: Record; } +const WINDOWS_SERVICE_PROBE_PRELOAD = resolve(import.meta.dir, "owned-service-home-preload.ts"); +const WINDOWS_SERVICE_PROBE_FLAG = "OCX_TEST_SERVICE_HOME_PROBE"; + +function windowsServiceProbeEnv(): Record { + // The production Windows probe deliberately resolves schtasks.exe/sc.exe from + // System32, so PATH fixtures cannot isolate it. The explicit Bun preload is + // attached only to children created by this test fixture; production never + // reads this flag or the preload. + const preload = `--preload=${WINDOWS_SERVICE_PROBE_PRELOAD}`; + const existing = process.env.BUN_OPTIONS?.trim(); + const bunOptions = existing?.includes(preload) + ? existing + : [existing, preload].filter(Boolean).join(" "); + return { + BUN_OPTIONS: bunOptions, + [WINDOWS_SERVICE_PROBE_FLAG]: "1", + }; +} + /** * Seed the same state and service-manager definition that an installed proxy * records, scoped entirely to a test home. @@ -38,6 +57,7 @@ export function claimOwnedServiceHome( ].join("\n")); } + if (process.platform === "win32") return { env: windowsServiceProbeEnv() }; if (process.platform !== "linux") return { env: {} }; const unitDir = join(home, ".config", "systemd", "user"); diff --git a/tests/owned-service-home.test.ts b/tests/owned-service-home.test.ts new file mode 100644 index 0000000000..4416740057 --- /dev/null +++ b/tests/owned-service-home.test.ts @@ -0,0 +1,52 @@ +import { expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +import { claimOwnedServiceHome } from "./helpers/owned-service-home"; + +const repoRoot = resolve(import.meta.dir, ".."); + +test("Windows owned-service-home fixture masks manager queries in a real child", async () => { + const root = mkdtempSync(join(tmpdir(), "ocx-owned-service-home-seam-")); + const codexHome = join(root, "codex"); + const opencodexHome = join(root, "opencodex"); + const home = join(root, "home"); + for (const path of [codexHome, opencodexHome, home]) mkdirSync(path, { recursive: true }); + + try { + const fixture = claimOwnedServiceHome(codexHome, opencodexHome, home); + if (process.platform !== "win32") return; + + const child = Bun.spawn([process.execPath, "--eval", ` + import { inspectServiceManagerInstallation } from "./src/service-manager-probe.ts"; + const result = inspectServiceManagerInstallation({ + platform: "win32", + home: process.env.USERPROFILE, + configDir: process.env.OPENCODEX_HOME, + }); + console.log(JSON.stringify(result)); + `], { + cwd: repoRoot, + env: { + ...process.env, + ...fixture.env, + HOME: home, + USERPROFILE: home, + CODEX_HOME: codexHome, + OPENCODEX_HOME: opencodexHome, + }, + stdout: "pipe", + stderr: "pipe", + }); + const [exitCode, stdout, stderr] = await Promise.all([ + child.exited, + new Response(child.stdout).text(), + new Response(child.stderr).text(), + ]); + expect(exitCode, stderr).toBe(0); + expect(JSON.parse(stdout.trim())).toEqual({ kind: "absent" }); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); From 574b77afa71f30cdbf250ae6c08d9fee50b1b7b6 Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:51:43 +0900 Subject: [PATCH 10/19] test(codex): bound inject lock child processes --- tests/codex-inject-write-lock.test.ts | 123 ++++++++++++++++++-------- 1 file changed, 85 insertions(+), 38 deletions(-) diff --git a/tests/codex-inject-write-lock.test.ts b/tests/codex-inject-write-lock.test.ts index 9054d5bacf..8c8d57daed 100644 --- a/tests/codex-inject-write-lock.test.ts +++ b/tests/codex-inject-write-lock.test.ts @@ -6,7 +6,7 @@ * "the lock function was invoked" — a pass-through mock satisfies that — but * that two real processes running the real injection cannot both write. */ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, setDefaultTimeout, test } from "bun:test"; import { spawnSync } from "node:child_process"; import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; @@ -16,10 +16,16 @@ import { resolveEffectiveUserIdentity, } from "../src/codex/user-identity"; import { STABLE_ZERO_BYTE_COORDINATOR_AGE_MS } from "../src/codex/inject-coordination"; +import { SPAWN_BUDGET_MS } from "./helpers/test-budget"; const repoRoot = join(import.meta.dir, ".."); const CHILD = join(repoRoot, "tests", "helpers", "codex-inject-race-child.ts"); const LOCK_CHILD = join(repoRoot, "tests", "helpers", "codex-write-lock-child.ts"); +// Leave teardown and assertion headroom inside the surrounding test budget. A real +// Bun child can take several seconds to start and settle on a loaded Windows runner. +const SPAWN_TIMEOUT_MS = SPAWN_BUDGET_MS - 5_000; + +setDefaultTimeout(SPAWN_BUDGET_MS); let root = ""; let codexHome = ""; @@ -31,19 +37,64 @@ function seedNative(): void { writeFileSync(join(codexHome, "config.toml"), 'model = "gpt-5"\n'); } -function runInject(port: number, lockTimeoutMs = 0): { success: boolean; status?: "skipped"; retryable: boolean; message: string } { - const result = spawnSync(process.execPath, [CHILD], { +function runChild(args: string[], env: NodeJS.ProcessEnv): ReturnType { + return spawnSync(process.execPath, args, { cwd: repoRoot, encoding: "utf8", - env: { + env, + timeout: SPAWN_TIMEOUT_MS, + windowsHide: true, + }); +} + +function childDiagnostics(result: ReturnType): string { + const stdout = String(result.stdout ?? "").trim(); + const stderr = String(result.stderr ?? "").trim(); + const error = result.error instanceof Error + ? result.error.message + : result.error + ? String(result.error) + : ""; + return [ + `status=${String(result.status)}`, + `signal=${String(result.signal)}`, + error ? `error=${error}` : "", + `stdout=${stdout || ""}`, + `stderr=${stderr || ""}`, + ].filter(Boolean).join("; "); +} + +function requireChildSuccess(result: ReturnType, label: string): string { + if (result.error || result.status !== 0) { + throw new Error(`${label} failed: ${childDiagnostics(result)}`); + } + return String(result.stdout ?? ""); +} + +function parseChildJson(result: ReturnType, label: string): T { + const stdout = requireChildSuccess(result, label); + const line = stdout.trim().split(/\r?\n/).filter(Boolean).at(-1); + if (!line) { + throw new Error(`${label} produced no JSON output: ${childDiagnostics(result)}`); + } + try { + return JSON.parse(line) as T; + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + throw new Error(`${label} produced invalid JSON (${reason}): ${childDiagnostics(result)}`); + } +} + +function runInject(port: number, lockTimeoutMs = 0): { success: boolean; status?: "skipped"; retryable: boolean; message: string } { + return parseChildJson<{ success: boolean; status?: "skipped"; retryable: boolean; message: string }>( + runChild([CHILD], { ...process.env, CODEX_HOME: codexHome, OPENCODEX_HOME: opencodexHome, OCX_INJECT_RACE_PAYLOAD: JSON.stringify({ port, lockTimeoutMs }), - }, - }); - const line = (result.stdout ?? "").trim().split("\n").filter(Boolean).pop() ?? "{}"; - return JSON.parse(line) as { success: boolean; status?: "skipped"; retryable: boolean; message: string }; + }), + `inject child (port=${port})`, + ); } beforeEach(() => { @@ -102,18 +153,18 @@ describe("the lock is on the production path", () => { expect(result.success).toBeTrue(); // The row is the proof that the lock ran, not that the function was called. - const state = spawnSync(process.execPath, ["--eval", ` + const state = runChild(["--eval", ` const { readCodexTransitionState } = require("./src/codex/transition-state"); console.log(JSON.stringify(readCodexTransitionState())); `], { - cwd: repoRoot, - encoding: "utf8", - env: { ...process.env, CODEX_HOME: codexHome, OPENCODEX_HOME: opencodexHome }, + ...process.env, + CODEX_HOME: codexHome, + OPENCODEX_HOME: opencodexHome, }); - const row = JSON.parse((state.stdout ?? "{}").trim().split("\n").pop() ?? "{}") as { + const row = parseChildJson<{ kind?: string; state?: { nativeGeneration?: number; currentTxId?: string | null }; - }; + }>(state, "read transition state after clean apply"); expect(row.kind).toBe("ready"); expect(row.state?.nativeGeneration).toBeGreaterThan(0); // Guessing null passes on a fresh machine and fails on a real one, so the @@ -149,7 +200,7 @@ describe("the lock is on the production path", () => { const deadline = Date.now() + 10_000; while (!existsSync(holdMarker) && Date.now() < deadline) { - spawnSync(process.execPath, ["--eval", "Bun.sleepSync(20)"], { encoding: "utf8" }); + requireChildSuccess(runChild(["--eval", "Bun.sleepSync(20)"], process.env), "hold-marker wait child"); } expect(existsSync(holdMarker)).toBeTrue(); @@ -171,7 +222,7 @@ describe("the lock is on the production path", () => { const finalConfig = readFileSync(join(codexHome, "config.toml"), "utf-8"); expect(finalConfig).not.toContain("20200"); expect(finalConfig).toBe(afterFirst); - }, 30_000); + }, SPAWN_BUDGET_MS); }); describe("homes the coordinator cannot adopt keep working", () => { @@ -237,18 +288,18 @@ describe("the transition is resolved, not left pending", () => { seedNative(); expect(runInject(10100).success).toBeTrue(); - const state = spawnSync(process.execPath, ["--eval", ` + const state = runChild(["--eval", ` const { readCodexTransitionState } = require("./src/codex/transition-state"); console.log(JSON.stringify(readCodexTransitionState())); `], { - cwd: repoRoot, - encoding: "utf8", - env: { ...process.env, CODEX_HOME: codexHome, OPENCODEX_HOME: opencodexHome }, + ...process.env, + CODEX_HOME: codexHome, + OPENCODEX_HOME: opencodexHome, }); - const row = JSON.parse((state.stdout ?? "{}").trim().split("\n").pop() ?? "{}") as { + const row = parseChildJson<{ kind?: string; state?: { history?: { status?: string } }; - }; + }>(state, "read transition state after completed apply"); expect(row.kind).toBe("ready"); expect(row.state?.history?.status).not.toBe("pending"); }); @@ -268,30 +319,26 @@ describe("the transition is resolved, not left pending", () => { syncResumeHistory: false, }, null, 2)); - const result = spawnSync(process.execPath, [CHILD], { - cwd: repoRoot, - encoding: "utf8", - env: { - ...process.env, - CODEX_HOME: codexHome, - OPENCODEX_HOME: opencodexHome, - OCX_INJECT_RACE_PAYLOAD: JSON.stringify({ port: 10100, lockTimeoutMs: 0 }), - }, + const result = runChild([CHILD], { + ...process.env, + CODEX_HOME: codexHome, + OPENCODEX_HOME: opencodexHome, + OCX_INJECT_RACE_PAYLOAD: JSON.stringify({ port: 10100, lockTimeoutMs: 0 }), }); - expect(result.status).toBe(0); + requireChildSuccess(result, "opted-out inject child"); - const state = spawnSync(process.execPath, ["--eval", ` + const state = runChild(["--eval", ` const { readCodexTransitionState } = require("./src/codex/transition-state"); console.log(JSON.stringify(readCodexTransitionState())); `], { - cwd: repoRoot, - encoding: "utf8", - env: { ...process.env, CODEX_HOME: codexHome, OPENCODEX_HOME: opencodexHome }, + ...process.env, + CODEX_HOME: codexHome, + OPENCODEX_HOME: opencodexHome, }); - const row = JSON.parse((state.stdout ?? "{}").trim().split("\n").pop() ?? "{}") as { + const row = parseChildJson<{ kind?: string; state?: { history?: { status?: string; attempts?: number } }; - }; + }>(state, "read transition state after opted-out apply"); expect(row.kind).toBe("ready"); // Opt-out is a completed decision, not a failure: converged, never blocked, // and never left pending for a job that chose to do nothing. From 7bb99b4706874b4e979685fef876221c8a3f3bed Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 08:10:42 +0900 Subject: [PATCH 11/19] test: assert PowerShell fixture start-time output --- tests/codex-app-server-processes.test.ts | 7 ++++++- tests/helpers/windows-power-shell-fixture.ts | 14 +++++++++++--- tests/multi-agent-compat.test.ts | 6 +++++- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/tests/codex-app-server-processes.test.ts b/tests/codex-app-server-processes.test.ts index a6ce9fe34a..db513c692c 100644 --- a/tests/codex-app-server-processes.test.ts +++ b/tests/codex-app-server-processes.test.ts @@ -154,9 +154,10 @@ afterAll(() => stallingFakePowerShell?.cleanup()); let loopRanDuringExec = false; const beat = setInterval(() => { loopRanDuringExec = true; }, 5); + let status: Awaited>; try { // No readStartMsBatchAsync override: the default start-time path must run for real. - await collectCodexAppServerCatalogStateForRequest({ + status = await collectCodexAppServerCatalogStateForRequest({ platform: "win32", listSnapshotsAsync: async () => [{ pid: 42, commandLine: APP_SERVER_CMD }], catalogMtimeMs: () => 1_000, @@ -168,6 +169,10 @@ afterAll(() => stallingFakePowerShell?.cleanup()); } expect(loopRanDuringExec).toBe(true); + expect(status).toMatchObject({ + state: "stale", + processes: [{ pid: 42, startedAtMs: 500 }], + }); }); test("Windows request collection shares one in-flight refresh and its short cache (#1852)", async () => { diff --git a/tests/helpers/windows-power-shell-fixture.ts b/tests/helpers/windows-power-shell-fixture.ts index 3c508565cd..e402a77515 100644 --- a/tests/helpers/windows-power-shell-fixture.ts +++ b/tests/helpers/windows-power-shell-fixture.ts @@ -35,7 +35,7 @@ async function buildWindowsExecutableFixture(): Promise setTimeout(resolve, 200));", "if (command.includes('CreationDate')) {", - " process.stdout.write('42\\t2020-01-01T00:00:00.000Z\\n');", + " process.stdout.write('42\\t1970-01-01T00:00:00.500Z\\n');", "} else {", " process.stdout.write('42\\t/usr/local/bin/codex app-server\\tCONTOSO\\\\jun\\n');", "}", @@ -62,7 +62,10 @@ function createPosixShellFixture(): WindowsPowerShellFixture { writeFileSync(executable, [ "#!/bin/sh", "sleep 0.2", - "printf '42\\t/usr/local/bin/codex app-server\\tCONTOSO\\\\jun\\n'", + "case \"$*\" in", + " *CreationDate*) printf '42\\t1970-01-01T00:00:00.500Z\\n' ;;", + " *) printf '42\\t/usr/local/bin/codex app-server\\tCONTOSO\\\\jun\\n' ;;", + "esac", ].join("\n"), { mode: 0o755 }); return { executable, @@ -74,6 +77,8 @@ async function removeFixtureDirectory(dir: string): Promise { // Windows can keep a just-exited compiled child image open for a short interval. // Retry the temp cleanup so an antivirus/file-close race does not turn an otherwise // passing test file into an unnamed afterAll failure. + const retryableCodes = new Set(["EBUSY", "EPERM", "EACCES"]); + let lastError: unknown; for (let attempt = 0; attempt < 40; attempt += 1) { try { rmSync(dir, { recursive: true, force: true }); @@ -82,8 +87,11 @@ async function removeFixtureDirectory(dir: string): Promise { const code = error && typeof error === "object" && "code" in error ? (error as { code?: unknown }).code : undefined; - if (code !== "EBUSY") throw error; + if (typeof code !== "string" || !retryableCodes.has(code)) throw error; + lastError = error; await Bun.sleep(50); } } + const detail = lastError instanceof Error ? lastError.message : String(lastError); + throw new Error(`Could not remove Windows PowerShell test fixture after 2s: ${detail}`); } diff --git a/tests/multi-agent-compat.test.ts b/tests/multi-agent-compat.test.ts index 0a29d4e9fb..c295a25e27 100644 --- a/tests/multi-agent-compat.test.ts +++ b/tests/multi-agent-compat.test.ts @@ -160,8 +160,9 @@ describe("multiAgentGuidanceText", () => { // loop, so the flag cannot flip regardless of machine speed. let loopRanDuringExec = false; const beat = setInterval(() => { loopRanDuringExec = true; }, 5); + let guidance: Awaited>; try { - await multiAgentGuidanceText(parsed, { injectionModel: "anthropic/claude-sonnet-5" }); + guidance = await multiAgentGuidanceText(parsed, { injectionModel: "anthropic/claude-sonnet-5" }); } finally { clearInterval(beat); Object.defineProperty(process, "platform", realPlatform); @@ -171,6 +172,9 @@ describe("multiAgentGuidanceText", () => { } expect(loopRanDuringExec).toBe(true); + // The fixture's second child invocation returns a pre-catalog start time; the + // default request collector must parse it as stale and suppress positive guidance. + expect(guidance).toBeNull(); }); test("v2 guidance suppresses positive model claims while the app-server catalog is stale or unknown (#857)", async () => { From 4575a37238d4de0c8e9bda53a307b6cd8e1581a5 Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 08:10:20 +0900 Subject: [PATCH 12/19] test(codex): extend lock contention hold --- tests/codex-inject-write-lock.test.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/codex-inject-write-lock.test.ts b/tests/codex-inject-write-lock.test.ts index 8c8d57daed..1911ed732a 100644 --- a/tests/codex-inject-write-lock.test.ts +++ b/tests/codex-inject-write-lock.test.ts @@ -192,7 +192,14 @@ describe("the lock is on the production path", () => { ...process.env, CODEX_HOME: codexHome, OPENCODEX_HOME: opencodexHome, - OCX_LOCK_CHILD_PAYLOAD: JSON.stringify({ timeoutMs: 5_000, holdMarker, releaseMarker }), + OCX_LOCK_CHILD_PAYLOAD: JSON.stringify({ + timeoutMs: 5_000, + holdMarker, + releaseMarker, + // Keep a slow Windows contender from outliving the hold, while staying + // below the 40s child bound and the 45s test budget. + holdMs: SPAWN_TIMEOUT_MS - 5_000, + }), }, stdout: "pipe", stderr: "pipe", From 13fa29e08cf2f7fb5a52bc34919a71fa5594b72a Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 08:35:41 +0900 Subject: [PATCH 13/19] test: pass Windows service probe preload explicitly --- tests/cli-restore-back.test.ts | 4 +- tests/codex-composed-acceptance.test.ts | 19 ++++--- .../codex-retained-root-serialization.test.ts | 36 ++++++++----- tests/codex-sync-api.test.ts | 54 ++++++++++--------- tests/helpers/owned-service-home-preload.ts | 21 +++++--- tests/helpers/owned-service-home.ts | 40 ++++++++++---- tests/owned-service-home.test.ts | 54 ++++++++++++++++--- 7 files changed, 158 insertions(+), 70 deletions(-) diff --git a/tests/cli-restore-back.test.ts b/tests/cli-restore-back.test.ts index 6ddb32c02c..e00c6342a7 100644 --- a/tests/cli-restore-back.test.ts +++ b/tests/cli-restore-back.test.ts @@ -3,7 +3,7 @@ import { spawnSync } from "node:child_process"; import { mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { claimOwnedServiceHome } from "./helpers/owned-service-home"; +import { claimOwnedServiceHome, withOwnedServiceHomePreload } from "./helpers/owned-service-home"; import { SPAWN_BUDGET_MS } from "./helpers/test-budget"; const repoRoot = join(import.meta.dir, ".."); @@ -19,7 +19,7 @@ function ownedEnvironment(codexHome: string, ocxHome: string): Record) { - return spawnSync(process.execPath, ["run", "src/cli/index.ts", ...args], { + return spawnSync(process.execPath, withOwnedServiceHomePreload(["run", "src/cli/index.ts", ...args]), { cwd: repoRoot, env: { ...process.env, ...env }, encoding: "utf8", diff --git a/tests/codex-composed-acceptance.test.ts b/tests/codex-composed-acceptance.test.ts index 8e6cddd0e3..bfe87960e5 100644 --- a/tests/codex-composed-acceptance.test.ts +++ b/tests/codex-composed-acceptance.test.ts @@ -40,7 +40,7 @@ import { resolveCodexCoordinatorDatabasePath, resolveEffectiveUserIdentity, } from "../src/codex/user-identity"; -import { claimOwnedServiceHome } from "./helpers/owned-service-home"; +import { claimOwnedServiceHome, withOwnedServiceHomePreload } from "./helpers/owned-service-home"; import { SERVER_BUDGET_MS } from "./helpers/test-budget"; const repoRoot = resolve(import.meta.dir, ".."); @@ -118,6 +118,7 @@ class Fixture { readonly lockPath: string; readonly lockAllowlist: string[]; readonly serviceManagerEnv: Record; + readonly serviceManagerPreloadPath: string | undefined; readonly children: Array> = []; constructor() { @@ -130,10 +131,16 @@ class Fixture { if (existsSync(path)) throw new Error(`lock preflight found pre-existing case path: ${path}`); } writeFileSync(join(this.codex, "config.toml"), 'model = "gpt-5"\n'); - this.serviceManagerEnv = claimOwnedServiceHome(this.codex, this.ocx, this.homeA).env; + const serviceHome = claimOwnedServiceHome(this.codex, this.ocx, this.homeA); + this.serviceManagerEnv = serviceHome.env; + this.serviceManagerPreloadPath = serviceHome.preloadPath; } - env(home = this.homeA, userprofile = this.userprofileA): Record { + env( + home = this.homeA, + userprofile = this.userprofileA, + includeServiceProbe = false, + ): Record { // Do not inherit ambient homes or proxy configuration. `process.execPath` // is absolute, so a PATH is intentionally unnecessary for CLI children. return { @@ -156,7 +163,7 @@ class Fixture { // without this the child spawned by a CI runner refuses with "Windows effective-account // lookup timed out" while powershell.exe is still starting. ...(process.env.CI === "true" ? { CI: "true" } : {}), - ...this.serviceManagerEnv, + ...(includeServiceProbe ? this.serviceManagerEnv : {}), }; } @@ -182,9 +189,9 @@ class Fixture { } spawnCli(argv: string[], home = this.homeA, userprofile = this.userprofileA) { - const child = Bun.spawn([process.execPath, cliPath, ...argv], { + const child = Bun.spawn([process.execPath, ...withOwnedServiceHomePreload([cliPath, ...argv], this.serviceManagerPreloadPath)], { cwd: this.root, - env: this.env(home, userprofile), + env: this.env(home, userprofile, true), stdout: "pipe", stderr: "pipe", }); diff --git a/tests/codex-retained-root-serialization.test.ts b/tests/codex-retained-root-serialization.test.ts index b2dc7c5a83..4386799151 100644 --- a/tests/codex-retained-root-serialization.test.ts +++ b/tests/codex-retained-root-serialization.test.ts @@ -16,7 +16,7 @@ import { resolveCodexCatalogSerializationDatabasePath, resolveEffectiveUserIdentity, } from "../src/codex/user-identity"; -import { claimOwnedServiceHome } from "./helpers/owned-service-home"; +import { claimOwnedServiceHome, withOwnedServiceHomePreload } from "./helpers/owned-service-home"; const repoRoot = resolve(import.meta.dir, ".."); const sandboxes: Sandbox[] = []; @@ -26,6 +26,8 @@ interface Sandbox { readonly codexHome: string; readonly opencodexHome: string; readonly env: Record; + readonly serviceManagerEnv: Record; + readonly preloadPath?: string; } function nativeEntry(slug: string, visibility = "list"): Record { @@ -65,7 +67,7 @@ function makeSandbox(prefix: string): Sandbox { mkdirSync(path, { recursive: true }); chmodSync(path, 0o700); } - const serviceManagerEnv = claimOwnedServiceHome(codexHome, opencodexHome, home).env; + const serviceHome = claimOwnedServiceHome(codexHome, opencodexHome, home); const sandbox = { root, codexHome, @@ -81,13 +83,18 @@ function makeSandbox(prefix: string): Sandbox { TMP: runtime, XDG_RUNTIME_DIR: runtime, LOCALAPPDATA: join(home, "LocalAppData"), - ...serviceManagerEnv, }, + serviceManagerEnv: serviceHome.env, + preloadPath: serviceHome.preloadPath, }; sandboxes.push(sandbox); return sandbox; } +function sandboxChildEnv(sandbox: Sandbox): Record { + return { ...sandbox.env, ...sandbox.serviceManagerEnv }; +} + async function waitForPath(path: string, timeoutMs = 10_000): Promise { const deadline = Date.now() + timeoutMs; while (!existsSync(path)) { @@ -100,9 +107,9 @@ async function runChild( sandbox: Sandbox, script: string, ): Promise<{ exitCode: number; stdout: string; stderr: string }> { - const child = Bun.spawn([process.execPath, "--eval", script], { + const child = Bun.spawn([process.execPath, ...withOwnedServiceHomePreload(["--eval", script], sandbox.preloadPath)], { cwd: repoRoot, - env: sandbox.env, + env: sandboxChildEnv(sandbox), stdout: "pipe", stderr: "pipe", }); @@ -177,9 +184,12 @@ test("startup and CLI sync-cache cannot write models_cache while another process expect(startupProbe.exitCode).toBe(0); expect(existsSync(cachePath)).toBe(false); - const cli = Bun.spawnSync([process.execPath, "run", "src/cli/index.ts", "sync-cache"], { + const cli = Bun.spawnSync([ + process.execPath, + ...withOwnedServiceHomePreload(["run", "src/cli/index.ts", "sync-cache"], sandbox.preloadPath), + ], { cwd: repoRoot, - env: sandbox.env, + env: sandboxChildEnv(sandbox), stdout: "pipe", stderr: "pipe", }); @@ -287,13 +297,13 @@ for (const publisher of ["convergence", "retained"] as const) { }; writeFileSync(join(sandbox.opencodexHome, "config.json"), JSON.stringify(config)); try { - const sync = Bun.spawn([process.execPath, "--eval", ` + const sync = Bun.spawn([process.execPath, ...withOwnedServiceHomePreload(["--eval", ` const config = ${JSON.stringify(config)}; const { handleManagementAPI } = await import("./src/server/management-api.ts"); const req = new Request("http://localhost/api/sync", { method: "POST", headers: { Host: "localhost" } }); const response = await handleManagementAPI(req, new URL(req.url), config); console.log(JSON.stringify({ status: response.status, body: await response.json() })); - `], { cwd: repoRoot, env: sandbox.env, stdout: "pipe", stderr: "pipe" }); + `], sandbox.preloadPath)], { cwd: repoRoot, env: sandboxChildEnv(sandbox), stdout: "pipe", stderr: "pipe" }); await Promise.race([ waitForPath(requested), @@ -366,7 +376,7 @@ test("a persisted runtime selection moved by another process during the await bl }, }; - const sync = Bun.spawn([process.execPath, "--eval", ` + const sync = Bun.spawn([process.execPath, ...withOwnedServiceHomePreload(["--eval", ` import { existsSync, writeFileSync } from "node:fs"; const config = ${JSON.stringify(config)}; config.providers.together.fetch = async () => { @@ -376,7 +386,7 @@ test("a persisted runtime selection moved by another process during the await bl }; const { syncCatalogModels } = await import("./src/codex/catalog/sync.ts"); console.log(JSON.stringify(await syncCatalogModels(config))); - `], { cwd: repoRoot, env: sandbox.env, stdout: "pipe", stderr: "pipe" }); + `], sandbox.preloadPath)], { cwd: repoRoot, env: sandboxChildEnv(sandbox), stdout: "pipe", stderr: "pipe" }); await Promise.race([ waitForPath(requested), @@ -497,8 +507,8 @@ test("two processes at the post-approval management seam serialize instead of in writeFileSync(catalogPath, seeded); const children = (["a", "b"] as const).map(marker => Bun.spawn( - [process.execPath, "--eval", routeScript(marker)], - { cwd: repoRoot, env: sandbox.env, stdout: "pipe", stderr: "pipe" }, + [process.execPath, ...withOwnedServiceHomePreload(["--eval", routeScript(marker)], sandbox.preloadPath)], + { cwd: repoRoot, env: sandboxChildEnv(sandbox), stdout: "pipe", stderr: "pipe" }, )); results = await Promise.all(children.map(async child => { diff --git a/tests/codex-sync-api.test.ts b/tests/codex-sync-api.test.ts index 2f49737836..8a0d430bbe 100644 --- a/tests/codex-sync-api.test.ts +++ b/tests/codex-sync-api.test.ts @@ -7,7 +7,7 @@ import { syncModelsToCodex } from "../src/codex/sync"; import { MANAGED_AGENTS_TABLE_MARKER, MANAGED_SUBAGENT_DEFAULT_MARKER } from "../src/codex/subagent-defaults"; import type { OcxConfig } from "../src/types"; import type { OrcaCodexHomeDiagnostic } from "../src/codex/home"; -import { claimOwnedServiceHome } from "./helpers/owned-service-home"; +import { claimOwnedServiceHome, withOwnedServiceHomePreload } from "./helpers/owned-service-home"; const TEST_DIR = join(import.meta.dir, ".tmp-codex-sync-api"); const TEST_CODEX_HOME = join(TEST_DIR, "codex"); @@ -18,8 +18,8 @@ let prevCodexHome: string | undefined; let prevOpenCodexHome: string | undefined; let prevHome: string | undefined; let prevUserProfile: string | undefined; -let prevBunOptions: string | undefined; -let prevServiceProbeFlag: string | undefined; +let serviceManagerEnv: Record = {}; +let serviceManagerPreloadPath: string | undefined; const config = { port: 0, @@ -36,9 +36,17 @@ const config = { } as OcxConfig; function claimTempHome(codexHome: string, ocxHome: string, home: string): void { - for (const [key, value] of Object.entries(claimOwnedServiceHome(codexHome, ocxHome, home).env)) { - process.env[key] = value; - } + const fixture = claimOwnedServiceHome(codexHome, ocxHome, home); + serviceManagerEnv = fixture.env; + serviceManagerPreloadPath = fixture.preloadPath; +} + +function childEnv(overrides: Record = {}): Record { + return { ...process.env, ...serviceManagerEnv, ...overrides } as Record; +} + +function childArgs(args: readonly string[]): string[] { + return withOwnedServiceHomePreload(args, serviceManagerPreloadPath); } const admittedSync = () => ({ kind: "admitted" as const }); @@ -62,8 +70,6 @@ describe("GUI/CLI Codex sync backend", () => { prevOpenCodexHome = process.env.OPENCODEX_HOME; prevHome = process.env.HOME; prevUserProfile = process.env.USERPROFILE; - prevBunOptions = process.env.BUN_OPTIONS; - prevServiceProbeFlag = process.env.OCX_TEST_SERVICE_HOME_PROBE; if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); mkdirSync(TEST_CODEX_HOME, { recursive: true }); mkdirSync(TEST_OCX_HOME, { recursive: true }); @@ -86,10 +92,8 @@ describe("GUI/CLI Codex sync backend", () => { else process.env.HOME = prevHome; if (prevUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = prevUserProfile; - if (prevBunOptions === undefined) delete process.env.BUN_OPTIONS; - else process.env.BUN_OPTIONS = prevBunOptions; - if (prevServiceProbeFlag === undefined) delete process.env.OCX_TEST_SERVICE_HOME_PROBE; - else process.env.OCX_TEST_SERVICE_HOME_PROBE = prevServiceProbeFlag; + serviceManagerEnv = {}; + serviceManagerPreloadPath = undefined; if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); }); test("returns the structured sync result used by POST /api/sync", async () => { @@ -180,19 +184,18 @@ describe("GUI/CLI Codex sync backend", () => { const journalPath = join(TEST_CODEX_HOME, "opencodex-journal.json"); const before = readFileSync(configPath, "utf8"); - const child = spawnSync(process.execPath, ["-e", ` + const child = spawnSync(process.execPath, childArgs(["-e", ` const { injectCodexConfig } = await import("./src/codex/inject.ts"); const result = await injectCodexConfig(10100, ${JSON.stringify(config)}, { validateOnly: true }); console.log(JSON.stringify(result)); - `], { + `]), { cwd: repoRoot, - env: { - ...process.env, + env: childEnv({ HOME: TEST_HOME, USERPROFILE: TEST_HOME, CODEX_HOME: TEST_CODEX_HOME, OPENCODEX_HOME: TEST_OCX_HOME, - }, + }), encoding: "utf8", }); @@ -343,11 +346,13 @@ describe("GUI/CLI Codex sync backend", () => { ' const result = await syncModelsToCodex(12345, snapshot, null, {', ' refreshCodexModelCatalog: async () => {', ' // The provider-discovery window: a second real process persists OFF.', + ' // This child only flips desired state; do not propagate the service-probe flag.', + ' const flipEnv = { ...process.env }; delete flipEnv.OCX_TEST_SERVICE_HOME_PROBE;', ' const flip = spawnSync(process.execPath, ["--eval",', ' \'const { setIntegrationEnabled } = require("./src/codex/desired-state");\'', ' + \'const r = setIntegrationEnabled("codex", false);\'', ' + \'if (!r.ok) { console.error(JSON.stringify(r)); process.exit(1); }\',', - ' ], { cwd: process.cwd(), env: process.env, encoding: "utf8" });', + ' ], { cwd: process.cwd(), env: flipEnv, encoding: "utf8" });', ' if (flip.status !== 0) throw new Error("flip failed: " + flip.stderr);', ' return { added: 0, path: "/tmp/none.json", catalogExists: false, catalogWritten: false, cacheSynced: false, comboOmissions: [] };', ' },', @@ -357,15 +362,14 @@ describe("GUI/CLI Codex sync backend", () => { '})();', ].join("\n"); const before = readFileSync(join(raceCodexHome, "config.toml"), "utf8"); - const child = spawnSync(process.execPath, ["--eval", script], { + const child = spawnSync(process.execPath, childArgs(["--eval", script]), { cwd: repoRoot, - env: { - ...process.env, + env: childEnv({ HOME: raceHome, USERPROFILE: raceHome, CODEX_HOME: raceCodexHome, OPENCODEX_HOME: raceOcxHome, - }, + }), encoding: "utf8", }); expect(child.status).toBe(0); @@ -499,7 +503,7 @@ describe("GUI/CLI Codex sync backend", () => { "", ].join("\n"), "utf8"); - const child = spawnSync(process.execPath, ["-e", ` + const child = spawnSync(process.execPath, childArgs(["-e", ` const { handleManagementAPI } = await import("./src/server/management-api.ts"); const config = { port: 10100, defaultProvider: "openai", providers: {} }; const response = await handleManagementAPI( @@ -508,9 +512,9 @@ describe("GUI/CLI Codex sync backend", () => { config, ); console.log(JSON.stringify({ status: response.status, body: await response.json() })); - `], { + `]), { cwd: join(import.meta.dir, ".."), - env: { ...process.env, CODEX_HOME: TEST_CODEX_HOME, OPENCODEX_HOME: ocxHome }, + env: childEnv({ CODEX_HOME: TEST_CODEX_HOME, OPENCODEX_HOME: ocxHome }), encoding: "utf8", }); diff --git a/tests/helpers/owned-service-home-preload.ts b/tests/helpers/owned-service-home-preload.ts index 7152437b95..5cae990dee 100644 --- a/tests/helpers/owned-service-home-preload.ts +++ b/tests/helpers/owned-service-home-preload.ts @@ -22,12 +22,19 @@ if (ENABLED) { const name = typeof file === "string" ? file.replaceAll("\\", "/").split("/").pop()?.toLowerCase() : undefined; - const first = typeof args[0] === "string" ? args[0].toLowerCase() : ""; - const secondArg = typeof args[1] === "string" ? args[1].toLowerCase() : ""; - const isSchedulerQuery = name === "schtasks.exe" && first === "/query"; + // Keep this seam to the exact read-only argv emitted by production. A + // foreign task, a listing/error query, or extra arguments must reach the + // real manager instead of being silently declared absent. + const isSchedulerQuery = name === "schtasks.exe" + && args.length === 4 + && args[0] === "/query" + && args[1] === "/tn" + && args[2] === "opencodex-proxy" + && args[3] === "/xml"; const isNativeServiceQuery = name === "sc.exe" - && first === "query" - && secondArg === "opencodex-proxy-native"; + && args.length === 2 + && args[0] === "query" + && args[1] === "opencodex-proxy-native"; if (!isSchedulerQuery && !isNativeServiceQuery) return realSpawnSync(...input); @@ -36,8 +43,8 @@ if (ENABLED) { && "encoding" in options && options.encoding === "buffer"; const message = isNativeServiceQuery - ? "[SC] OpenService FAILED 1060: The specified service does not exist." - : "ERROR: The system cannot find the file specified."; + ? "[OCX_TEST_SERVICE_HOME] [SC] OpenService FAILED 1060: The specified service does not exist." + : "[OCX_TEST_SERVICE_HOME] ERROR: The system cannot find the file specified."; const stdout = raw ? Buffer.alloc(0) : ""; const stderr = raw ? Buffer.from(message, "utf8") : message; return { diff --git a/tests/helpers/owned-service-home.ts b/tests/helpers/owned-service-home.ts index 538898ce8d..21970a3466 100644 --- a/tests/helpers/owned-service-home.ts +++ b/tests/helpers/owned-service-home.ts @@ -2,25 +2,41 @@ import { chmodSync, mkdirSync, writeFileSync } from "node:fs"; import { delimiter, join, resolve } from "node:path"; export interface OwnedServiceHome { - /** Add this to child-process environments so service probes never reach host state. */ + /** Add this only to child-process environments that also receive `preloadPath`. */ readonly env: Record; + /** Explicit Bun preload path; passed as argv so spaces are not shell-parsed. */ + readonly preloadPath?: string; } const WINDOWS_SERVICE_PROBE_PRELOAD = resolve(import.meta.dir, "owned-service-home-preload.ts"); const WINDOWS_SERVICE_PROBE_FLAG = "OCX_TEST_SERVICE_HOME_PROBE"; +/** + * Insert a test preload into a Bun command without relying on BUN_OPTIONS. + * + * `bun run` consumes its flags after the `run` subcommand; direct `bun + * --eval`/file invocations consume them before the entrypoint. Keeping the + * path as a separate argv element is what makes a checkout directory with + * spaces safe on Bun 1.4 and on Windows child_process/Bun.spawn alike. + */ +export function withOwnedServiceHomePreload( + args: readonly string[], + preloadPath = WINDOWS_SERVICE_PROBE_PRELOAD, +): string[] { + if (process.platform !== "win32") return [...args]; + const preloadArgs = ["--preload", preloadPath]; + if (args[0] === "run" || args[0] === "test") { + return [args[0], ...preloadArgs, ...args.slice(1)]; + } + return [...preloadArgs, ...args]; +} + function windowsServiceProbeEnv(): Record { // The production Windows probe deliberately resolves schtasks.exe/sc.exe from - // System32, so PATH fixtures cannot isolate it. The explicit Bun preload is - // attached only to children created by this test fixture; production never - // reads this flag or the preload. - const preload = `--preload=${WINDOWS_SERVICE_PROBE_PRELOAD}`; - const existing = process.env.BUN_OPTIONS?.trim(); - const bunOptions = existing?.includes(preload) - ? existing - : [existing, preload].filter(Boolean).join(" "); + // System32, so PATH fixtures cannot isolate it. The flag is inert unless the + // caller also passes `preloadPath` through withOwnedServiceHomePreload; this + // keeps it out of the parent test environment and unrelated nested children. return { - BUN_OPTIONS: bunOptions, [WINDOWS_SERVICE_PROBE_FLAG]: "1", }; } @@ -57,7 +73,9 @@ export function claimOwnedServiceHome( ].join("\n")); } - if (process.platform === "win32") return { env: windowsServiceProbeEnv() }; + if (process.platform === "win32") { + return { env: windowsServiceProbeEnv(), preloadPath: WINDOWS_SERVICE_PROBE_PRELOAD }; + } if (process.platform !== "linux") return { env: {} }; const unitDir = join(home, ".config", "systemd", "user"); diff --git a/tests/owned-service-home.test.ts b/tests/owned-service-home.test.ts index 4416740057..9fab628d0e 100644 --- a/tests/owned-service-home.test.ts +++ b/tests/owned-service-home.test.ts @@ -1,9 +1,9 @@ import { expect, test } from "bun:test"; -import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { copyFileSync, mkdirSync, mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; -import { claimOwnedServiceHome } from "./helpers/owned-service-home"; +import { claimOwnedServiceHome, withOwnedServiceHomePreload } from "./helpers/owned-service-home"; const repoRoot = resolve(import.meta.dir, ".."); @@ -18,15 +18,42 @@ test("Windows owned-service-home fixture masks manager queries in a real child", const fixture = claimOwnedServiceHome(codexHome, opencodexHome, home); if (process.platform !== "win32") return; - const child = Bun.spawn([process.execPath, "--eval", ` + // Copy the test preload below a path that contains spaces. Passing it as a + // separate argv element is the regression under test; BUN_OPTIONS tokenizes + // this same path before Bun 1.4 ever sees it. + const spacedCheckout = join(root, "checkout with spaces"); + mkdirSync(spacedCheckout, { recursive: true }); + const spacedPreload = join(spacedCheckout, "owned-service-home-preload.ts"); + copyFileSync(join(import.meta.dir, "helpers", "owned-service-home-preload.ts"), spacedPreload); + + const child = Bun.spawn([process.execPath, ...withOwnedServiceHomePreload(["--eval", ` + import { join } from "node:path"; + import { resolveTrustedWindowsSystemDirectory } from "./src/lib/windows-elevation.ts"; + import { spawnSync } from "node:child_process"; import { inspectServiceManagerInstallation } from "./src/service-manager-probe.ts"; + const system = resolveTrustedWindowsSystemDirectory(); + const schtasks = join(system, "schtasks.exe"); + const sc = join(system, "sc.exe"); + const text = (value: unknown) => Buffer.isBuffer(value) ? value.toString("utf8") : String(value ?? ""); + const exactScheduler = spawnSync(schtasks, ["/query", "/tn", "opencodex-proxy", "/xml"], { encoding: "buffer" }); + const foreignScheduler = spawnSync(schtasks, ["/query", "/tn", "foreign-opencodex-proxy", "/xml"], { encoding: "buffer" }); + const extraScheduler = spawnSync(schtasks, ["/query", "/tn", "opencodex-proxy", "/xml", "/extra"], { encoding: "buffer" }); + const exactNative = spawnSync(sc, ["query", "opencodex-proxy-native"], { encoding: "buffer" }); + const extraNative = spawnSync(sc, ["query", "opencodex-proxy-native", "extra"], { encoding: "buffer" }); const result = inspectServiceManagerInstallation({ platform: "win32", home: process.env.USERPROFILE, configDir: process.env.OPENCODEX_HOME, }); - console.log(JSON.stringify(result)); - `], { + console.log(JSON.stringify({ + result, + exactScheduler: { status: exactScheduler.status, stderr: text(exactScheduler.stderr) }, + foreignScheduler: { status: foreignScheduler.status, stderr: text(foreignScheduler.stderr) }, + extraScheduler: { status: extraScheduler.status, stderr: text(extraScheduler.stderr) }, + exactNative: { status: exactNative.status, stderr: text(exactNative.stderr) }, + extraNative: { status: extraNative.status, stderr: text(extraNative.stderr) }, + })); + `], spacedPreload)], { cwd: repoRoot, env: { ...process.env, @@ -45,7 +72,22 @@ test("Windows owned-service-home fixture masks manager queries in a real child", new Response(child.stderr).text(), ]); expect(exitCode, stderr).toBe(0); - expect(JSON.parse(stdout.trim())).toEqual({ kind: "absent" }); + const payload = JSON.parse(stdout.trim()) as { + result: { kind: string }; + exactScheduler: { status: number | undefined; stderr: string }; + foreignScheduler: { status: number | undefined; stderr: string }; + extraScheduler: { status: number | undefined; stderr: string }; + exactNative: { status: number | undefined; stderr: string }; + extraNative: { status: number | undefined; stderr: string }; + }; + expect(payload.result).toEqual({ kind: "absent" }); + expect(payload.exactScheduler.status).toBe(1); + expect(payload.exactScheduler.stderr).toContain("OCX_TEST_SERVICE_HOME"); + expect(payload.foreignScheduler.stderr).not.toContain("OCX_TEST_SERVICE_HOME"); + expect(payload.extraScheduler.stderr).not.toContain("OCX_TEST_SERVICE_HOME"); + expect(payload.exactNative.status).toBe(1); + expect(payload.exactNative.stderr).toContain("OCX_TEST_SERVICE_HOME"); + expect(payload.extraNative.stderr).not.toContain("OCX_TEST_SERVICE_HOME"); } finally { rmSync(root, { recursive: true, force: true }); } From 117ea955f47d856101061162e8b0722faf312a4b Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 09:49:26 +0900 Subject: [PATCH 14/19] fix: close review races and fixture lifecycles --- src/server/relay-eager.ts | 11 +++- tests/codex-inject-write-lock.test.ts | 46 +++++++++----- tests/helpers/windows-power-shell-fixture.ts | 8 +-- tests/owned-service-home.test.ts | 26 ++++++-- tests/relay-eager.test.ts | 66 ++++++++++++++++++++ 5 files changed, 132 insertions(+), 25 deletions(-) diff --git a/src/server/relay-eager.ts b/src/server/relay-eager.ts index b428938eec..f389a00d5b 100644 --- a/src/server/relay-eager.ts +++ b/src/server/relay-eager.ts @@ -184,7 +184,16 @@ export function relaySseEagerBounded( // turn unregistration stay reachable, drainAndShutdown never hangs). let wake: (() => void) | null = null; const wakeUp = () => { const w = wake; wake = null; w?.(); }; - const paused = () => new Promise(resolve => { wake = resolve; }); + const paused = () => new Promise(resolve => { + wake = resolve; + // A pull, cancel, or abort can win the tiny window between the loop's + // predicate check and installing this resolver. Re-check every wake + // predicate after installation so that an already-fired wake cannot leave + // the producer parked forever. + if (queuedBytes <= maxQueueBytes || cancelled || upstream.signal.aborted) { + wakeUp(); + } + }); upstream.signal.addEventListener("abort", wakeUp, { once: true }); let controllerRef: ReadableStreamDefaultController | null = null; diff --git a/tests/codex-inject-write-lock.test.ts b/tests/codex-inject-write-lock.test.ts index 1911ed732a..00ec0a44b2 100644 --- a/tests/codex-inject-write-lock.test.ts +++ b/tests/codex-inject-write-lock.test.ts @@ -24,6 +24,12 @@ const LOCK_CHILD = join(repoRoot, "tests", "helpers", "codex-write-lock-child.ts // Leave teardown and assertion headroom inside the surrounding test budget. A real // Bun child can take several seconds to start and settle on a loaded Windows runner. const SPAWN_TIMEOUT_MS = SPAWN_BUDGET_MS - 5_000; +// The contender uses a much shorter bound because production contention is +// fail-fast (lockTimeoutMs=0). Keep the holder alive well beyond that bound so a +// slow child launch cannot turn an intended busy result into a post-release apply. +const CONTENTION_CHILD_TIMEOUT_MS = 10_000; +const CONTENTION_HOLDER_MARGIN_MS = 5_000; +const CONTENTION_HOLD_MS = SPAWN_TIMEOUT_MS - CONTENTION_HOLDER_MARGIN_MS; setDefaultTimeout(SPAWN_BUDGET_MS); @@ -37,12 +43,16 @@ function seedNative(): void { writeFileSync(join(codexHome, "config.toml"), 'model = "gpt-5"\n'); } -function runChild(args: string[], env: NodeJS.ProcessEnv): ReturnType { +function runChild( + args: string[], + env: NodeJS.ProcessEnv, + timeoutMs = SPAWN_TIMEOUT_MS, +): ReturnType { return spawnSync(process.execPath, args, { cwd: repoRoot, encoding: "utf8", env, - timeout: SPAWN_TIMEOUT_MS, + timeout: timeoutMs, windowsHide: true, }); } @@ -85,14 +95,18 @@ function parseChildJson(result: ReturnType, label: string): } } -function runInject(port: number, lockTimeoutMs = 0): { success: boolean; status?: "skipped"; retryable: boolean; message: string } { +function runInject( + port: number, + lockTimeoutMs = 0, + timeoutMs = SPAWN_TIMEOUT_MS, +): { success: boolean; status?: "skipped"; retryable: boolean; message: string } { return parseChildJson<{ success: boolean; status?: "skipped"; retryable: boolean; message: string }>( runChild([CHILD], { ...process.env, CODEX_HOME: codexHome, OPENCODEX_HOME: opencodexHome, OCX_INJECT_RACE_PAYLOAD: JSON.stringify({ port, lockTimeoutMs }), - }), + }, timeoutMs), `inject child (port=${port})`, ); } @@ -198,7 +212,7 @@ describe("the lock is on the production path", () => { releaseMarker, // Keep a slow Windows contender from outliving the hold, while staying // below the 40s child bound and the 45s test budget. - holdMs: SPAWN_TIMEOUT_MS - 5_000, + holdMs: CONTENTION_HOLD_MS, }), }, stdout: "pipe", @@ -213,15 +227,19 @@ describe("the lock is on the production path", () => { // PROCESS-UNIQUE bytes: a different port means different candidate bytes, so // the loser's work is identifiable rather than assumed. - const contender = runInject(20200); - - writeFileSync(releaseMarker, "go"); - // AWAIT the holder. Dropping its exit on the floor left a live child owning the - // coordinator database while afterEach removed the temp root, and Windows refuses - // to unlink a file another process still has open -- so teardown failed with EBUSY - // and blamed this test for a race it had already won. POSIX unlinks regardless, - // which is why only Windows ever saw it, and only under full-suite load. - await holder.exited; + let contender: ReturnType; + try { + contender = runInject(20200, 0, CONTENTION_CHILD_TIMEOUT_MS); + } finally { + try { + writeFileSync(releaseMarker, "go"); + } finally { + // Always release and reap the holder, including when the contender + // times out or its diagnostics throw. Otherwise teardown races a live + // child that still owns the coordinator database on Windows. + await holder.exited; + } + } expect(contender.success).toBeFalse(); expect(contender.retryable).toBeTrue(); diff --git a/tests/helpers/windows-power-shell-fixture.ts b/tests/helpers/windows-power-shell-fixture.ts index e402a77515..d9bb26e0a6 100644 --- a/tests/helpers/windows-power-shell-fixture.ts +++ b/tests/helpers/windows-power-shell-fixture.ts @@ -7,8 +7,6 @@ export interface WindowsPowerShellFixture { cleanup: () => void | Promise; } -let sharedFixture: Promise | undefined; - /** * Build a real Windows executable for tests that exercise the default execFile path. * @@ -20,11 +18,11 @@ let sharedFixture: Promise | undefined; * explicitly faked by the tests. */ export function createWindowsPowerShellFixture(): Promise { - if (sharedFixture) return sharedFixture; - sharedFixture = process.platform === "win32" + // Each suite owns its fixture. Sharing one directory across suites lets the + // first cleanup remove the executable while another suite is still using it. + return process.platform === "win32" ? buildWindowsExecutableFixture() : Promise.resolve(createPosixShellFixture()); - return sharedFixture; } async function buildWindowsExecutableFixture(): Promise { diff --git a/tests/owned-service-home.test.ts b/tests/owned-service-home.test.ts index 9fab628d0e..ea7ec6fe40 100644 --- a/tests/owned-service-home.test.ts +++ b/tests/owned-service-home.test.ts @@ -35,11 +35,27 @@ test("Windows owned-service-home fixture masks manager queries in a real child", const schtasks = join(system, "schtasks.exe"); const sc = join(system, "sc.exe"); const text = (value: unknown) => Buffer.isBuffer(value) ? value.toString("utf8") : String(value ?? ""); - const exactScheduler = spawnSync(schtasks, ["/query", "/tn", "opencodex-proxy", "/xml"], { encoding: "buffer" }); - const foreignScheduler = spawnSync(schtasks, ["/query", "/tn", "foreign-opencodex-proxy", "/xml"], { encoding: "buffer" }); - const extraScheduler = spawnSync(schtasks, ["/query", "/tn", "opencodex-proxy", "/xml", "/extra"], { encoding: "buffer" }); - const exactNative = spawnSync(sc, ["query", "opencodex-proxy-native"], { encoding: "buffer" }); - const extraNative = spawnSync(sc, ["query", "opencodex-proxy-native", "extra"], { encoding: "buffer" }); + const serviceProbe = (label: string, executable: string, args: string[]) => { + const probe = spawnSync(executable, args, { + encoding: "buffer", + timeout: 5_000, + windowsHide: true, + }); + // A timeout (or any spawn failure) is a failed probe, not an absent + // service. Keep the real pass-through result visible via status/stderr. + if (probe.error || probe.status === null) { + const detail = probe.error instanceof Error + ? probe.error.message + : String(probe.error ?? "no exit status"); + throw new Error(label + " failed: " + detail); + } + return probe; + }; + const exactScheduler = serviceProbe("exact scheduler", schtasks, ["/query", "/tn", "opencodex-proxy", "/xml"]); + const foreignScheduler = serviceProbe("foreign scheduler", schtasks, ["/query", "/tn", "foreign-opencodex-proxy", "/xml"]); + const extraScheduler = serviceProbe("extra scheduler", schtasks, ["/query", "/tn", "opencodex-proxy", "/xml", "/extra"]); + const exactNative = serviceProbe("exact native", sc, ["query", "opencodex-proxy-native"]); + const extraNative = serviceProbe("extra native", sc, ["query", "opencodex-proxy-native", "extra"]); const result = inspectServiceManagerInstallation({ platform: "win32", home: process.env.USERPROFILE, diff --git a/tests/relay-eager.test.ts b/tests/relay-eager.test.ts index 0cb423a5a0..9838b9ad68 100644 --- a/tests/relay-eager.test.ts +++ b/tests/relay-eager.test.ts @@ -1058,6 +1058,72 @@ describe("relaySseEagerBounded — bounded queue", () => { ); }); + test("(b2) pause resolver rechecks an abort that wins before installation", async () => { + const { hooks, rec } = makeHooks(); + const up = controlledUpstream(); + const realUpstream = new AbortController(); + let abortOnNextSignalCheck = false; + let predicateAbortTriggered = false; + const signal = new Proxy(realUpstream.signal, { + get(target, property) { + if (property === "aborted" && abortOnNextSignalCheck) { + abortOnNextSignalCheck = false; + predicateAbortTriggered = true; + realUpstream.abort(new Error("deterministic pause interleave")); + // Return the pre-abort value for this predicate evaluation. The abort + // event has already fired, so the resolver installed by paused() must + // observe the new value on its post-install check. + return false; + } + const value = Reflect.get(target, property, target); + return typeof value === "function" ? value.bind(target) : value; + }, + }); + const upstream = { + signal, + abort: (reason?: unknown) => realUpstream.abort(reason), + } as unknown as AbortController; + + let resolveDone!: () => void; + const done = new Promise(resolve => { resolveDone = resolve; }); + const previousOnDone = hooks.onDone; + hooks.onDone = () => { + previousOnDone(); + resolveDone(); + }; + hooks.rewritePayload = payload => { + // This callback runs after the chunk is read and before the bounded-queue + // predicate. The proxy then aborts exactly while that predicate is being + // evaluated, before paused() installs its resolver. + abortOnNextSignalCheck = true; + return payload; + }; + + const relayed = relaySseEagerBounded(up.stream, upstream, hooks, { maxQueueBytes: 1 }); + const reader = relayed.getReader(); + up.push(sse(DELTA)); + + let timeout: ReturnType | undefined; + try { + await Promise.race([ + done, + new Promise((_, reject) => { + timeout = setTimeout( + () => reject(new Error("pause resolver did not observe the deterministic abort")), + watchdogMs(2_000), + ); + }), + ]); + } finally { + if (timeout) clearTimeout(timeout); + await reader.cancel().catch(() => {}); + realUpstream.abort(new Error("test cleanup")); + } + + expect(predicateAbortTriggered).toBe(true); + expect(rec.dones).toBe(1); + }); + test("(f) cancel while paused wakes the gate — onDone fires, no deadlock", async () => { const { hooks, rec } = makeHooks(); const up = controlledUpstream(); From 963af137062d94ccec6057f7dc8244159f3118eb Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:10:53 +0900 Subject: [PATCH 15/19] test: always reap lock contention holder --- tests/codex-inject-write-lock.test.ts | 60 +++++++++++++++++---------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/tests/codex-inject-write-lock.test.ts b/tests/codex-inject-write-lock.test.ts index 00ec0a44b2..ac28ba6bc7 100644 --- a/tests/codex-inject-write-lock.test.ts +++ b/tests/codex-inject-write-lock.test.ts @@ -219,34 +219,52 @@ describe("the lock is on the production path", () => { stderr: "pipe", }); - const deadline = Date.now() + 10_000; - while (!existsSync(holdMarker) && Date.now() < deadline) { - requireChildSuccess(runChild(["--eval", "Bun.sleepSync(20)"], process.env), "hold-marker wait child"); - } - expect(existsSync(holdMarker)).toBeTrue(); - - // PROCESS-UNIQUE bytes: a different port means different candidate bytes, so - // the loser's work is identifiable rather than assumed. - let contender: ReturnType; + let primaryFailed = false; + let primaryError: unknown; + let cleanupFailed = false; + let cleanupError: unknown; try { - contender = runInject(20200, 0, CONTENTION_CHILD_TIMEOUT_MS); + const deadline = Date.now() + 10_000; + while (!existsSync(holdMarker) && Date.now() < deadline) { + requireChildSuccess(runChild(["--eval", "Bun.sleepSync(20)"], process.env), "hold-marker wait child"); + } + expect(existsSync(holdMarker)).toBeTrue(); + + // PROCESS-UNIQUE bytes: a different port means different candidate bytes, so + // the loser's work is identifiable rather than assumed. + const contender = runInject(20200, 0, CONTENTION_CHILD_TIMEOUT_MS); + + expect(contender.success).toBeFalse(); + expect(contender.retryable).toBeTrue(); + // Its bytes are absent: the file still names the first winner's port. + const finalConfig = readFileSync(join(codexHome, "config.toml"), "utf-8"); + expect(finalConfig).not.toContain("20200"); + expect(finalConfig).toBe(afterFirst); + } catch (error) { + // Preserve the first assertion/helper failure after cleanup completes. + primaryFailed = true; + primaryError = error; } finally { try { writeFileSync(releaseMarker, "go"); - } finally { - // Always release and reap the holder, including when the contender - // times out or its diagnostics throw. Otherwise teardown races a live - // child that still owns the coordinator database on Windows. + } catch (error) { + cleanupFailed = true; + cleanupError = error; + } + try { + // Always release and reap the holder, including when marker wait, + // contender startup, or an assertion fails. Otherwise teardown races a + // live child that still owns the coordinator database on Windows. await holder.exited; + } catch (error) { + if (!cleanupFailed) { + cleanupFailed = true; + cleanupError = error; + } } } - - expect(contender.success).toBeFalse(); - expect(contender.retryable).toBeTrue(); - // Its bytes are absent: the file still names the first winner's port. - const finalConfig = readFileSync(join(codexHome, "config.toml"), "utf-8"); - expect(finalConfig).not.toContain("20200"); - expect(finalConfig).toBe(afterFirst); + if (primaryFailed) throw primaryError; + if (cleanupFailed) throw cleanupError; }, SPAWN_BUDGET_MS); }); From 2fb77a18275f38902b828b67d2e7fe4026e7fd56 Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:45:13 +0900 Subject: [PATCH 16/19] test(docs): close review follow-up gaps --- .../docs/ja/reference/proxy-formats.md | 4 + .../docs/ko/reference/proxy-formats.md | 4 + .../content/docs/reference/proxy-formats.md | 21 ++-- .../docs/ru/reference/proxy-formats.md | 4 + .../docs/zh-cn/reference/proxy-formats.md | 4 + tests/lab-fabric-task.test.ts | 105 +++++++++++++++++- tests/relay-eager.test.ts | 99 +++++++++-------- 7 files changed, 184 insertions(+), 57 deletions(-) diff --git a/docs-site/src/content/docs/ja/reference/proxy-formats.md b/docs-site/src/content/docs/ja/reference/proxy-formats.md index 2d6f4fdf6a..ff329cf29f 100644 --- a/docs-site/src/content/docs/ja/reference/proxy-formats.md +++ b/docs-site/src/content/docs/ja/reference/proxy-formats.md @@ -55,6 +55,10 @@ provider events → internal adapter events → client dialect クライアント向け Responses SSE フレームは、SSE ブロック区切りの前の生バイトで測って 1 フレームあたり 4 MiB に制限されます。HTTP では、区切りなしでこの上限を超えたアップストリーム フレームは、合成 `response.failed` イベントと続く `data: [DONE]` でフェイルクローズします。Responses WebSocket ブリッジでは、同じ条件で 502 `websocket_protocol_error` を送信し、アップストリーム リーダーをキャンセルします。完全な Responses 終端フレームがすでに到着している場合はそれが優先され、その後のサイズ超過または不正なバイトは、完了したターンをトランスポート障害に置き換えず破棄されます。 +:::note +ネイティブ パススルーでは、Responses の終端イベントが優先されます。早すぎる `data: [DONE]` は、そのイベントが届くまで保留されます。通常のネイティブ パスで、解析済みの終端がないまま正常な HTTP 200 EOF に達した場合、プロキシは `incomplete_details.reason: "adapter_eof"` を持つ `response.incomplete` を 1 件、その後に `data: [DONE]` を 1 件送信します。区切りのない終端 JSON が構文的に有効なら 1 回だけ受け入れられ、不正または切り詰められた JSON は incomplete のままです。モデル単位の終端修復を有効にしたプロバイダーでは、フレーム化されていない終端らしい接尾部と EOF 時の早すぎる `data: [DONE]` は、昇格可能な完全なライフサイクル候補がなければ `missing_terminal_event` としてフェイルクローズし、候補が完全なら `response.completed` に昇格します。高信頼度の `cyber_policy` 終端は、セマンティックなログおよび課金集計上は `error.code: "cyber_policy"` を持つ `response.failed`（status 400）に正規化されますが、すでに開始済みのストリーミング HTTP 応答は 200 のままです。このコミット済みリクエストの境界では、再試行も再送も行いません。 +::: + すべての端末応答使用状況オブジェクトには、プロバイダーが詳細を報告しなかった場合でも、両方の詳細オブジェクトが含まれます。 ```json diff --git a/docs-site/src/content/docs/ko/reference/proxy-formats.md b/docs-site/src/content/docs/ko/reference/proxy-formats.md index 532fe0b4ef..1f000d992b 100644 --- a/docs-site/src/content/docs/ko/reference/proxy-formats.md +++ b/docs-site/src/content/docs/ko/reference/proxy-formats.md @@ -65,6 +65,10 @@ deltas, 그리고 정확히 하나의 종료 `response.completed`, `response.fai 클라이언트로 전달되는 Responses SSE 프레임은 SSE 블록 구분자 앞의 원시 바이트 기준으로 프레임당 4 MiB로 제한됩니다. HTTP에서는 구분자 없이 이 한도를 초과한 업스트림 프레임을 합성 `response.failed` 이벤트와 이어지는 `data: [DONE]`으로 fail closed 처리합니다. Responses WebSocket 브리지에서는 같은 조건에서 502 `websocket_protocol_error`를 보내고 업스트림 reader를 취소합니다. 완전한 Responses 종료 프레임이 이미 수신된 경우에는 그 종료가 우선하며, 이후의 과도한 크기 또는 잘못된 바이트는 완료된 턴을 전송 오류로 바꾸지 않고 버립니다. +:::note +네이티브 passthrough에서는 Responses 종료 이벤트가 우선합니다. 너무 이른 `data: [DONE]`은 해당 이벤트가 도착할 때까지 보류됩니다. 일반 네이티브 경로가 파싱된 종료 이벤트 없이 정상 HTTP 200 EOF에 도달하면, 프록시는 `incomplete_details.reason: "adapter_eof"`가 있는 `response.incomplete` 하나와 `data: [DONE]` 하나를 보냅니다. 구분자 없는 종료 JSON이 문법적으로 유효하면 정확히 한 번 받아들이고, 잘못되었거나 잘린 JSON은 incomplete로 남습니다. 모델별 종료 복구를 사용하도록 설정된 공급자에서는 프레임이 없는 종료 유사 suffix와 EOF의 너무 이른 `data: [DONE]`을, 승격할 수 있는 완전한 lifecycle 후보가 없을 때 `missing_terminal_event`로 fail closed 처리하며, 완전한 후보가 있으면 `response.completed`로 승격합니다. 신뢰도가 높은 `cyber_policy` 종료 형식은 의미론적 로깅 및 집계에서 `error.code: "cyber_policy"`가 있는 `response.failed`(status 400)로 정규화되지만, 이미 시작된 스트리밍 HTTP 응답은 200을 유지합니다. 이 커밋된 요청 경계에서는 재시도하거나 재전송하지 않습니다. +::: + canonical ChatGPT forward streaming은 stable Bun 1.4.0 이상에서 Codex 업스트림 WebSocket을 투명하게 사용할 수 있습니다. 번들 Bun 1.3.14, prerelease, 또는 검증 불가능한 런타임 identity는 HTTP/SSE를 사용합니다. 업스트림 WS adapter는 같은 downstream SSE 계약을 유지하며, 원시 JSON diff --git a/docs-site/src/content/docs/reference/proxy-formats.md b/docs-site/src/content/docs/reference/proxy-formats.md index 5a7de86075..83dda745cf 100644 --- a/docs-site/src/content/docs/reference/proxy-formats.md +++ b/docs-site/src/content/docs/reference/proxy-formats.md @@ -74,15 +74,18 @@ that terminal are dropped rather than replacing the completed turn with a transp :::note For native passthrough, a Responses terminal event is authoritative. A premature `data: [DONE]` is -held until that event; if upstream reaches EOF first, the proxy emits one `response.incomplete` with -`incomplete_details.reason: "adapter_eof"`, followed by one `data: [DONE]`. Syntactically valid -delimiter-less terminal JSON at EOF is accepted exactly once; malformed or truncated terminal-shaped -JSON remains incomplete, and model-scoped terminal repair remains fail-closed for unframed -terminal-like suffixes. High-confidence `cyber_policy` terminal shapes normalize to -`response.failed` with `error.code: "cyber_policy"` for semantic logging/accounting (status 400), -while an already-started streamed HTTP response remains 200. This boundary does not retry or replay -the committed request and does not resolve [#2423](https://github.com/lidge-jun/opencodex/issues/2423) -or [#2486](https://github.com/lidge-jun/opencodex/issues/2486). +held until that event. On the ordinary native path, a clean HTTP 200 EOF without a parsed terminal +emits one `response.incomplete` with `incomplete_details.reason: "adapter_eof"`, followed by one +`data: [DONE]`; syntactically valid delimiter-less terminal JSON is accepted exactly once, while +malformed or truncated JSON remains incomplete. For providers opted into model-scoped terminal +repair, unframed terminal-like suffixes and a premature `data: [DONE]` at EOF fail closed with +`missing_terminal_event` when no complete lifecycle candidate can be promoted; a complete candidate +is promoted to `response.completed`. High-confidence `cyber_policy` +terminal shapes normalize to `response.failed` with `error.code: "cyber_policy"` for semantic +logging/accounting (status 400), while an already-started streamed HTTP response remains 200. This +committed-request boundary does not retry or replay and does not resolve +[#2423](https://github.com/lidge-jun/opencodex/issues/2423) or +[#2486](https://github.com/lidge-jun/opencodex/issues/2486). ::: For canonical ChatGPT forward streaming, stable Bun 1.4.0 or newer may transparently use diff --git a/docs-site/src/content/docs/ru/reference/proxy-formats.md b/docs-site/src/content/docs/ru/reference/proxy-formats.md index 6a461e8b9e..9163bec4f2 100644 --- a/docs-site/src/content/docs/ru/reference/proxy-formats.md +++ b/docs-site/src/content/docs/ru/reference/proxy-formats.md @@ -69,6 +69,10 @@ Responses. Обе формы сохраняют выбранную модель, Клиентские frame'ы Responses SSE ограничены 4 MiB на frame, считая сырые байты до разделителя SSE-блока. В HTTP незавершённый upstream-frame, превысивший этот предел, завершается fail-closed синтетическим событием `response.failed`, после которого идёт `data: [DONE]`. В мосте Responses WebSocket то же условие даёт 502 `websocket_protocol_error` и отменяет upstream-reader. Если полноценный terminal-frame Responses уже получен, он остаётся авторитетным: слишком большие или некорректные байты после него отбрасываются и не заменяют завершённый ход транспортной ошибкой. +:::note +При нативном passthrough терминальное событие Responses является авторитетным, а преждевременный `data: [DONE]` удерживается до его появления. Если обычный нативный путь достигает корректного HTTP 200 EOF без распознанного терминального события, прокси испускает один `response.incomplete` с `incomplete_details.reason: "adapter_eof"`, а затем один `data: [DONE]`. Синтаксически корректный терминальный JSON без разделителя принимается ровно один раз; некорректный или обрезанный JSON остаётся incomplete. Для провайдеров с включённым model-scoped terminal repair неоформленный terminal-like suffix и преждевременный `data: [DONE]` на EOF завершаются fail-closed с `missing_terminal_event`, если нет полного lifecycle-кандидата для повышения; полный кандидат повышается до `response.completed`. Терминальные формы `cyber_policy` с высокой уверенностью нормализуются для семантического журналирования и учёта в `response.failed` с `error.code: "cyber_policy"` (status 400), но уже начатый потоковый HTTP-ответ сохраняет статус 200. На этой границе уже отправленного запроса нет retry или replay. +::: + Каждый terminal usage-объект Responses всегда включает оба detail-объекта, даже если провайдер их не сообщил: diff --git a/docs-site/src/content/docs/zh-cn/reference/proxy-formats.md b/docs-site/src/content/docs/zh-cn/reference/proxy-formats.md index a469e2f628..0d18903ecd 100644 --- a/docs-site/src/content/docs/zh-cn/reference/proxy-formats.md +++ b/docs-site/src/content/docs/zh-cn/reference/proxy-formats.md @@ -64,6 +64,10 @@ Responses 表示是这座桥的中心。原生兼容的路由可以跳过部分 面向客户端的 Responses SSE 帧按 SSE 块分隔符之前的原始字节计算，每帧限制为 4 MiB。对于 HTTP，未终止的上游帧一旦超过该限制，会以合成的 `response.failed` 事件并随后发送 `data: [DONE]` 的方式 fail closed。对于 Responses WebSocket 桥，相同情况会发送 502 `websocket_protocol_error` 并取消上游 reader。已经完整到达的 Responses 终止帧具有优先权；其后的超大或格式错误字节会被丢弃，而不会把已经完成的轮次替换为传输失败。 +:::note +对于原生透传，Responses 终止事件具有最高优先级；过早出现的 `data: [DONE]` 会被保留，直到该事件到达。普通原生路径在没有已解析终止事件的情况下正常到达 HTTP 200 EOF 时，代理会发送一个带有 `incomplete_details.reason: "adapter_eof"` 的 `response.incomplete`，随后发送一个 `data: [DONE]`。语法有效但缺少分隔符的终止 JSON 只会被接受一次；格式错误或被截断的 JSON 仍保持 incomplete。对于启用了按模型终止修复的提供方，未成帧但形似终止事件的后缀和 EOF 处过早出现的 `data: [DONE]`，会在没有可提升的完整生命周期候选时以 `missing_terminal_event` 的形式 fail closed；完整候选则会被提升为 `response.completed`。高置信度的 `cyber_policy` 终止形态会在语义日志和计量中规范化为带有 `error.code: "cyber_policy"` 的 `response.failed`（status 400），但已经开始的流式 HTTP 响应仍保持 200。这个已提交请求的边界不会重试或重放请求。 +::: + 每个终止的 Responses usage 对象都包含两个 detail 对象，即使提供方没有报告这些细节： ```json diff --git a/tests/lab-fabric-task.test.ts b/tests/lab-fabric-task.test.ts index b9b8c34422..7a160b8ef0 100644 --- a/tests/lab-fabric-task.test.ts +++ b/tests/lab-fabric-task.test.ts @@ -58,6 +58,7 @@ import { setFabricProducerIsolationLimitsForTests } from "../src/lab/fabric/prod import { taskSubjectApplicableToRequirements } from "../src/lab/projection/verification"; import { createHostIssuedFabricPatchExecutor } from "../src/lib/fabric-task-host"; import type { TrustedFabricPatchExecutor } from "../src/lab/fabric/types"; +import { watchdogMs } from "./helpers/ci-watchdog"; import { fabricCorrectPatchExecutor, fabricMockRoute, @@ -67,6 +68,23 @@ import { } from "./helpers/fabric-task-test"; const REPO_ROOT = join(dirname(fileURLToPath(import.meta.url)), ".."); +const CHILD_REAP_GRACE_MS = 2_000; + +async function awaitChildExitWithin(child: Bun.Subprocess, timeoutMs: number): Promise { + return Promise.race([ + child.exited.then(() => true, () => true), + Bun.sleep(timeoutMs).then(() => false), + ]); +} + +async function terminateChildWithin(child: Bun.Subprocess): Promise { + if (child.exitCode !== null) return true; + try { child.kill(); } catch { /* already exited */ } + if (await awaitChildExitWithin(child, CHILD_REAP_GRACE_MS)) return true; + try { child.kill("SIGKILL"); } catch { /* already exited */ } + return await awaitChildExitWithin(child, CHILD_REAP_GRACE_MS); +} + const CREDENTIAL_CANARY = "credential-canary-abcdefghijklmnopqrstuvwxyz1234567890"; const FAST_FABRIC_ISOLATION = Object.freeze({ totalTimeoutMs: 2_000, @@ -332,6 +350,91 @@ describe("CL-07 task effectiveness producer", () => { expect(result.outcome.outcome).toBe("pass"); }); + test("producer child awaits an async executor before result and clean exit", async () => { + const childWatchdogMs = watchdogMs(5_000); + const home = tempHome(); + const childEntry = join(REPO_ROOT, "src", "lab", "fabric", "producer-child.ts"); + const executorModulePath = join(home, "async-producer-executor.mjs"); + const settledMarker = join(home, "async-producer-settled"); + writeFileSync(executorModulePath, ` +import { writeFileSync } from "node:fs"; + +export async function execute() { + await Bun.sleep(75); + writeFileSync(${JSON.stringify(settledMarker)}, "settled", "utf8"); + return { + schemaVersion: 1, + operations: [{ + op: "replace", + path: ${JSON.stringify(SYNTHETIC_VALUE_PATH)}, + contentUtf8: ${JSON.stringify(SYNTHETIC_AFTER_UTF8)}, + }], + }; +} +`); + + expect(readFileSync(childEntry, "utf8")).toMatch(/\bawait\s+main\(\)\.catch\(/); + + const child = Bun.spawn([process.execPath, "run", childEntry], { + cwd: REPO_ROOT, + env: { + TZ: "UTC", + NO_COLOR: "1", + OCX_FABRIC_SCRATCH_ROOT: home, + }, + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + }); + const stdoutPromise = new Response(child.stdout).text(); + const stderrPromise = new Response(child.stderr).text(); + try { + child.stdin.write(JSON.stringify({ + executorModulePath, + executorInput: {}, + scratchRoot: home, + totalTimeoutMs: 5_000, + inactivityTimeoutMs: 2_000, + })); + child.stdin.end(); + } catch (error) { + await terminateChildWithin(child); + void stdoutPromise.catch(() => {}); + void stderrPromise.catch(() => {}); + throw error; + } + + const completed = await Promise.race([ + child.exited.then((exitCode) => ({ exitCode })), + Bun.sleep(childWatchdogMs).then(() => null), + ]); + if (!completed) { + const reaped = await terminateChildWithin(child); + void stdoutPromise.catch(() => {}); + const stderr = await Promise.race([ + stderrPromise.catch(() => ""), + Bun.sleep(CHILD_REAP_GRACE_MS).then(() => ""), + ]); + throw new Error(`timed out waiting for producer child (reaped=${reaped}): ${stderr}`); + } + + const [stdout, stderr] = await Promise.all([stdoutPromise, stderrPromise]); + expect(completed.exitCode).toBe(0); + expect(stderr).toBe(""); + expect(existsSync(settledMarker)).toBe(true); + expect(stdout.trim().split(/\r?\n/).map((line) => JSON.parse(line))).toEqual([{ + type: "result", + patch: { + schemaVersion: 1, + operations: [{ + op: "replace", + path: SYNTHETIC_VALUE_PATH, + contentUtf8: SYNTHETIC_AFTER_UTF8, + }], + }, + }]); + }, { timeout: watchdogMs(5_000) + (3 * CHILD_REAP_GRACE_MS) + 1_000 }); + test("harness execution cannot be persisted as production evidence", async () => { const home = tempHome(); const result = await runFabricSyntheticPatchTaskHarness({ @@ -1081,4 +1184,4 @@ describe("CL-07 task effectiveness producer", () => { expect(text.includes("system prompt")).toBe(false); expect(text.includes(CREDENTIAL_CANARY)).toBe(false); }); -}); \ No newline at end of file +}); diff --git a/tests/relay-eager.test.ts b/tests/relay-eager.test.ts index 9838b9ad68..ed82ee59c1 100644 --- a/tests/relay-eager.test.ts +++ b/tests/relay-eager.test.ts @@ -728,61 +728,66 @@ describe("relaySseEagerBounded — side-effect parity", () => { expect(upstreamAc.signal.aborted).toBe(true); }); - test("eager preserves an unframed terminal before a reader error", async () => { - const cases = [ - { + const unframedReadErrorCases = [ + { + label: "completed", + type: "response.completed", + event: "response.completed", + status: "completed", + payload: { type: "response.completed", - event: "response.completed", - status: "completed", - payload: { - type: "response.completed", - sequence_number: 61, - response: { id: "resp-eager-read-error-completed", status: "completed", output: [] }, - }, + sequence_number: 61, + response: { id: "resp-eager-read-error-completed", status: "completed", output: [] }, }, - { + }, + { + label: "failed", + type: "response.failed", + event: "response.failed", + status: "failed", + payload: { type: "response.failed", - event: "response.failed", - status: "failed", - payload: { - type: "response.failed", - sequence_number: 62, - response: { id: "resp-eager-read-error-failed", status: "failed", output: [] }, - }, + sequence_number: 62, + response: { id: "resp-eager-read-error-failed", status: "failed", output: [] }, }, - { + }, + { + label: "incomplete", + type: "response.incomplete", + event: "response.incomplete", + status: "incomplete", + payload: { type: "response.incomplete", - event: "response.incomplete", - status: "incomplete", - payload: { - type: "response.incomplete", - sequence_number: 63, - response: { id: "resp-eager-read-error-incomplete", status: "incomplete", output: [] }, - }, + sequence_number: 63, + response: { id: "resp-eager-read-error-incomplete", status: "incomplete", output: [] }, }, - { + }, + { + label: "policy error", + type: "error", + event: "response.failed", + status: "failed", + payload: { type: "error", - event: "response.failed", - status: "failed", - payload: { - type: "error", - sequence_number: 64, - response: { - id: "resp-eager-read-error-policy", - output: [{ type: "message", id: "item-eager-read-error-policy" }], - status: "failed", - }, - error: { - type: "invalid_request_error", - code: "cyber_policy", - message: "blocked by upstream policy", - }, + sequence_number: 64, + response: { + id: "resp-eager-read-error-policy", + output: [{ type: "message", id: "item-eager-read-error-policy" }], + status: "failed", + }, + error: { + type: "invalid_request_error", + code: "cyber_policy", + message: "blocked by upstream policy", }, - httpStatus: 400, }, - ] as const; + httpStatus: 400, + }, + ] as const; - for (const fixture of cases) { + test.each(unframedReadErrorCases)( + "eager preserves an unframed $label terminal before a reader error", + async (fixture) => { const { hooks, rec } = makeHooks(); const up = controlledUpstream(); const relayed = relaySseEagerBounded(up.stream, new AbortController(), hooks); @@ -802,8 +807,8 @@ describe("relaySseEagerBounded — side-effect parity", () => { expect(text).toContain('"id":"resp-eager-read-error-policy"'); expect(text).toContain('"output":[{"type":"message","id":"item-eager-read-error-policy"}]'); } - } - }); + }, + ); test("eager keeps an ordinary top-level error fail-closed on reader error", async () => { const { hooks, rec } = makeHooks(); From a6cfb317ddef5ab0b04bfb075137fd82ba947689 Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:53:10 +0900 Subject: [PATCH 17/19] fix(responses): preserve cyber policy semantics --- src/bridge.ts | 34 +++-- src/chat/outbound.ts | 22 ++- src/lib/errors.ts | 13 +- src/server/chat-completions.ts | 39 +++-- src/server/chat-native-sse.ts | 6 +- src/server/chat-native.ts | 41 +++-- src/server/relay.ts | 42 ++++-- src/server/responses/core.ts | 157 ++++++++++++++----- src/server/responses/passthrough-error.ts | 42 ++++-- tests/chat-completions-endpoint.test.ts | 175 ++++++++++++++++++++++ tests/cyber-policy-error-fidelity.test.ts | 154 +++++++++++++++---- tests/passthrough-abort.test.ts | 7 +- tests/relay-eager.test.ts | 11 +- tests/terminal-guard-server.test.ts | 37 +++++ 14 files changed, 639 insertions(+), 141 deletions(-) diff --git a/src/bridge.ts b/src/bridge.ts index bd24fb78e5..910f14f885 100644 --- a/src/bridge.ts +++ b/src/bridge.ts @@ -7,7 +7,15 @@ import type { OcxUsage, } from "./types"; import { coerceIntegerToolArguments } from "./lib/tool-argument-integers"; -import { adapterFailureFromMessage, classifyError, CYBER_POLICY_ERROR_CODE, isCyberPolicyCode, type OcxErrorPayload } from "./lib/errors"; +import { + adapterFailureFromMessage, + classifyError, + cyberPolicyErrorType, + CYBER_POLICY_ERROR_CODE, + isCyberPolicyCode, + type OcxErrorPayload, +} from "./lib/errors"; +import { redactSecretString } from "./lib/redact"; import { repairFreeformToolInput } from "./responses/apply-patch-envelope"; import { encodeCompactionSummary } from "./responses/compaction"; import { isTruncatedStopReason, truncationReasonFor } from "./responses/truncated-stop-reason"; @@ -114,18 +122,19 @@ function toolCallArgumentsUsable(args: string): boolean { } function adapterFailureFromEvent(event: Extract): { httpStatus: number; error: OcxErrorPayload } { + const message = redactSecretString(event.message); if (event.status === undefined && event.errorType === undefined && event.code === undefined) { - return adapterFailureFromMessage(event.message); + return adapterFailureFromMessage(message); } - const fallback = adapterFailureFromMessage(event.message); + const fallback = adapterFailureFromMessage(message); let httpStatus = event.status ?? fallback.httpStatus; - const error = classifyError(httpStatus, event.errorType ?? fallback.error.type, event.message); + const error = classifyError(httpStatus, event.errorType ?? fallback.error.type, message); if (event.errorType !== undefined) error.type = event.errorType; if (event.code !== undefined) error.code = event.code; // Codex maps cyber_policy on HTTP 400 (body) or mid-stream code; never leave it as 502. if (isCyberPolicyCode(error.code) || isCyberPolicyCode(event.code)) { error.code = CYBER_POLICY_ERROR_CODE; - error.type = "invalid_request_error"; + error.type = cyberPolicyErrorType(event.errorType); httpStatus = 400; } return { httpStatus, error }; @@ -1296,7 +1305,9 @@ export function bridgeToResponsesSSE( ...(event.usage ? { usage: responsesUsage(event.usage) } : {}), error: failure.error, last_error: failure.error, - ...(event.retryable !== undefined ? { retryable: event.retryable } : {}), + ...(isCyberPolicyCode(failure.error.code) + ? { retryable: false } + : event.retryable !== undefined ? { retryable: event.retryable } : {}), }, }); reportTerminal("failed"); @@ -1952,7 +1963,9 @@ function buildResponseJSONWithBudget( model: modelId, output, ...(endTurn !== undefined ? { end_turn: endTurn } : {}), ...(failure ? { error: failure.error, last_error: failure.error } : {}), - ...(errorEvent?.retryable !== undefined ? { retryable: errorEvent.retryable } : {}), + ...(failure && isCyberPolicyCode(failure.error.code) + ? { retryable: false } + : errorEvent?.retryable !== undefined ? { retryable: errorEvent.retryable } : {}), ...(incompleteEvent ? { incomplete_details: { reason: incompleteEvent.reason, @@ -1981,12 +1994,15 @@ export function formatErrorResponse( const error = classifyError(status, type, message); if (isCyberPolicyCode(options?.code)) { error.code = CYBER_POLICY_ERROR_CODE; - error.type = "invalid_request_error"; + error.type = cyberPolicyErrorType(type); } const finalStatus = error.code === CYBER_POLICY_ERROR_CODE ? 400 : status; const headers = new Headers({ "Content-Type": "application/json" }); const retryAfter = options?.retryAfter?.trim(); - if (retryAfter && retryAfter.length > 0 && retryAfter.length <= 128) { + if (error.code !== CYBER_POLICY_ERROR_CODE + && retryAfter + && retryAfter.length > 0 + && retryAfter.length <= 128) { headers.set("Retry-After", retryAfter); } return new Response(JSON.stringify({ error }), { diff --git a/src/chat/outbound.ts b/src/chat/outbound.ts index e7ad46775c..03e133bb90 100644 --- a/src/chat/outbound.ts +++ b/src/chat/outbound.ts @@ -9,7 +9,14 @@ type Rec = Record; import { decodeServerSentEvents, sseFieldValue } from "../lib/sse-decoder"; import { isTranslatorBudgetExceededError, type TranslatorBudget } from "../lib/translator-budget"; -import { classifyError, CYBER_POLICY_ERROR_CODE, isCyberPolicyCode, isCyberPolicyMessage } from "../lib/errors"; +import { + classifyError, + cyberPolicyErrorType, + CYBER_POLICY_ERROR_CODE, + isCyberPolicyCode, + isCyberPolicyMessage, +} from "../lib/errors"; +import { redactSecretString } from "../lib/redact"; function isRec(v: unknown): v is Rec { return !!v && typeof v === "object" && !Array.isArray(v); @@ -48,14 +55,14 @@ export function chatCompletionsUsage(usage: unknown): Rec { export function chatCompletionsErrorBody( status: number, message: string, - type = "invalid_request_error", + type?: string, code?: string | null, ): Rec { if (isCyberPolicyCode(code) || isCyberPolicyMessage(message)) { return { error: { message, - type: "invalid_request_error", + type: cyberPolicyErrorType(type), param: null, code: CYBER_POLICY_ERROR_CODE, }, @@ -64,7 +71,7 @@ export function chatCompletionsErrorBody( return { error: { message, - type, + type: type ?? "invalid_request_error", param: null, code: code !== undefined ? code @@ -311,8 +318,9 @@ export function responsesSseToChatCompletionsSse( // Deliver the error frame then close the stream abnormally (no [DONE]). // Do not controller.error() — that can drop already-enqueued bytes from consumers // like response.text(). - const statusHint = details?.status ?? streamErrorStatus(message); - const classified = classifyError(statusHint, details?.type ?? "upstream_error", message); + const safeMessage = redactSecretString(message); + const statusHint = details?.status ?? streamErrorStatus(safeMessage); + const classified = classifyError(statusHint, details?.type ?? "upstream_error", safeMessage); const translatorOverflow = details?.code === "translation_buffer_limit"; if (translatorOverflow) { upstreamAbort.abort(new Error("upstream translation buffer exceeded the safe limit")); @@ -324,7 +332,7 @@ export function responsesSseToChatCompletionsSse( classified.type = "upstream_error"; } else if (isCyberPolicyCode(details?.code) || classified.code === CYBER_POLICY_ERROR_CODE) { classified.code = CYBER_POLICY_ERROR_CODE; - classified.type = "invalid_request_error"; + classified.type = cyberPolicyErrorType(details?.type); } else if (details?.code !== undefined && details.code !== null && !classified.code) { classified.code = details.code; } diff --git a/src/lib/errors.ts b/src/lib/errors.ts index a7bbdb71e9..d7fecea26c 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -6,11 +6,18 @@ export interface OcxErrorPayload { /** OpenAI / Codex hard block for high-risk cybersecurity activity (HTTP 400 or mid-stream). */ export const CYBER_POLICY_ERROR_CODE = "cyber_policy"; +export const CYBER_POLICY_FALLBACK_MESSAGE = "Request blocked by the upstream cybersecurity policy."; export function isCyberPolicyCode(code: string | null | undefined): boolean { return code === CYBER_POLICY_ERROR_CODE; } +/** Preserve a structured upstream error type; otherwise use the dedicated policy identity. */ +export function cyberPolicyErrorType(type: string | null | undefined): string { + const trimmed = typeof type === "string" ? type.trim() : ""; + return trimmed || CYBER_POLICY_ERROR_CODE; +} + /** * Detect OpenAI cyber-policy refusals from message text when structured `code` was stripped. * Matches Codex fallback copy and Cursor/API agent wording (session evidence 2026-07-24). @@ -139,9 +146,11 @@ export function classifyError(status: number, type: string, message: string): Oc return { message, type: "invalid_request_error", code: "client_closed_request" }; } // Codex only shows the dedicated cyber UI when error.code === "cyber_policy". - // Prefer that code (and invalid_request_error) over generic remaps / 502 upstream_server_error. + // The public wire does not establish invalid_request_error as the canonical type, so + // message-only classification keeps the dedicated identity instead of inventing one. + // Structured callers re-apply their real upstream type with cyberPolicyErrorType(). if (type === CYBER_POLICY_ERROR_CODE || isCyberPolicyMessage(text)) { - return { message, type: "invalid_request_error", code: CYBER_POLICY_ERROR_CODE }; + return { message, type: CYBER_POLICY_ERROR_CODE, code: CYBER_POLICY_ERROR_CODE }; } // A LOCAL preflight refusal keeps its own code (#1524). The message necessarily says // "context window" -- that is what it is refusing on -- so the generic remap below would diff --git a/src/server/chat-completions.ts b/src/server/chat-completions.ts index 325c4ffd08..3e7f4515d1 100644 --- a/src/server/chat-completions.ts +++ b/src/server/chat-completions.ts @@ -18,7 +18,7 @@ import { responsesJsonToChatCompletion, responsesSseToChatCompletionsSse, } from "../chat/outbound"; -import { classifyError, CYBER_POLICY_ERROR_CODE, isCyberPolicyCode } from "../lib/errors"; +import { classifyError, cyberPolicyErrorType, CYBER_POLICY_ERROR_CODE, isCyberPolicyCode } from "../lib/errors"; import { redactSecretString } from "../lib/redact"; import { resolveClientRetryAfter } from "../lib/retry-after"; import { estimateTokens } from "../lib/token-estimate"; @@ -280,26 +280,26 @@ async function handleChatCompletionsWithBudget( const parsed = JSON.parse(text) as { error?: { message?: string; type?: string; code?: string | null } | string; message?: string; + type?: string; + code?: string | null; }; const nested = typeof parsed?.error === "object" && parsed.error ? parsed.error : undefined; const flat = typeof parsed?.error === "string" ? parsed.error : parsed?.message; const rawFallback = text ? `upstream error (${upstream.status}): ${redactSecretString(text).slice(0, 400)}` : message; - message = nested?.message || flat || rawFallback; - if (nested) { - if (typeof nested.type === "string") upstreamType = nested.type; - if (nested.code === null || typeof nested.code === "string") upstreamCode = nested.code; - } + const upstreamMessage = nested?.message || flat; + message = upstreamMessage + ? redactSecretString(upstreamMessage).slice(0, 500) + : rawFallback; + const structuredType = nested?.type ?? parsed.type; + const structuredCode = nested?.code ?? parsed.code; + if (typeof structuredType === "string") upstreamType = structuredType; + if (structuredCode === null || typeof structuredCode === "string") upstreamCode = structuredCode; } catch { if (text) message = `upstream error (${upstream.status}): ${redactSecretString(text).slice(0, 400)}`; } } catch { /* keep fallback */ } - const retryAfter = resolveClientRetryAfter({ - status: upstream.status, - message, - upstreamRetryAfter: upstream.headers.get("retry-after"), - }); const classified = classifyError( upstream.status, upstreamType @@ -309,9 +309,9 @@ async function handleChatCompletionsWithBudget( : "invalid_request_error"), message, ); - if (isCyberPolicyCode(upstreamCode)) { + if (isCyberPolicyCode(upstreamCode) || classified.code === CYBER_POLICY_ERROR_CODE) { classified.code = CYBER_POLICY_ERROR_CODE; - classified.type = "invalid_request_error"; + classified.type = cyberPolicyErrorType(upstreamType); } else if (upstreamCode === "model_not_found") { // Structured model_not_found must win over classifyError's generic remaps. classified.code = "model_not_found"; @@ -320,6 +320,13 @@ async function handleChatCompletionsWithBudget( classified.code = upstreamCode; } const status = isCyberPolicyCode(classified.code) ? 400 : upstream.status; + const retryAfter = isCyberPolicyCode(classified.code) + ? undefined + : resolveClientRetryAfter({ + status: upstream.status, + message, + upstreamRetryAfter: upstream.headers.get("retry-after"), + }); const rewritten = new Response(JSON.stringify({ error: { message: classified.message, @@ -386,14 +393,14 @@ async function handleChatCompletionsWithBudget( const status = (json as Rec)?.status; if (status === "failed") { const error = (json as { error?: { message?: string; type?: string; code?: string | null } }).error; - const message = error?.message ?? "upstream request failed"; + const message = redactSecretString(error?.message ?? "upstream request failed"); const classified = classifyError(502, error?.type ?? "server_error", message); if (error?.code === "translation_buffer_limit") { classified.code = "translation_buffer_limit"; classified.type = "upstream_error"; - } else if (isCyberPolicyCode(error?.code)) { + } else if (isCyberPolicyCode(error?.code) || classified.code === CYBER_POLICY_ERROR_CODE) { classified.code = CYBER_POLICY_ERROR_CODE; - classified.type = "invalid_request_error"; + classified.type = cyberPolicyErrorType(error?.type); } else if (error?.code === "model_not_found") { // Same deliberate preserve as the non-OK path: structured code beats generic classify. classified.code = "model_not_found"; diff --git a/src/server/chat-native-sse.ts b/src/server/chat-native-sse.ts index 4d197d5a58..fa9255369c 100644 --- a/src/server/chat-native-sse.ts +++ b/src/server/chat-native-sse.ts @@ -1,5 +1,5 @@ import { chatCompletionsErrorBody } from "../chat/outbound"; -import { classifyError, CYBER_POLICY_ERROR_CODE, isCyberPolicyCode } from "../lib/errors"; +import { classifyError, cyberPolicyErrorType, CYBER_POLICY_ERROR_CODE, isCyberPolicyCode } from "../lib/errors"; import { redactSecretString } from "../lib/redact"; import { isTranslatorBudgetExceededError, @@ -226,9 +226,9 @@ export function nativeChatSse( if (error) { const status = error.status ?? 502; const classified = classifyError(status, error.type ?? "upstream_error", error.message); - if (isCyberPolicyCode(error.code)) { + if (isCyberPolicyCode(error.code) || classified.code === CYBER_POLICY_ERROR_CODE) { classified.code = CYBER_POLICY_ERROR_CODE; - classified.type = "invalid_request_error"; + classified.type = cyberPolicyErrorType(error.type); } else if (error.code !== undefined && error.code !== null) { classified.code = error.code; } diff --git a/src/server/chat-native.ts b/src/server/chat-native.ts index e00caea282..2e84dc68d3 100644 --- a/src/server/chat-native.ts +++ b/src/server/chat-native.ts @@ -6,7 +6,13 @@ import { collectChatCompletion, isChatCompletionsStreamError, } from "../chat/outbound"; -import { classifyError, CYBER_POLICY_ERROR_CODE, isCyberPolicyCode } from "../lib/errors"; +import { + classifyError, + cyberPolicyErrorType, + CYBER_POLICY_ERROR_CODE, + isCyberPolicyCode, + isCyberPolicyMessage, +} from "../lib/errors"; import type { AdmissionLease } from "../lib/admission"; import { readBoundedResponseBody } from "../lib/bounded-body"; import { redactSecretString } from "../lib/redact"; @@ -281,13 +287,24 @@ export async function handleNativeChatCompletions(options: HandleNativeChatOptio const detail = activeAdapter.formatErrorBody?.(response.status, response.headers, bodyText) ?? ""; let upstreamType: string | undefined; let upstreamCode: string | null | undefined; + let upstreamMessage: string | undefined; try { const parsedError = JSON.parse(bodyText) as Rec; const nested = isRec(parsedError.error) ? parsedError.error : undefined; - if (typeof nested?.type === "string") upstreamType = nested.type; - if (nested?.code === null || typeof nested?.code === "string") upstreamCode = nested.code; + const details = nested ?? parsedError; + if (typeof details.type === "string") upstreamType = details.type; + if (details.code === null || typeof details.code === "string") upstreamCode = details.code; + const rawMessage = typeof details.message === "string" + ? details.message + : typeof parsedError.error === "string" ? parsedError.error : undefined; + if (rawMessage?.trim()) { + upstreamMessage = redactSecretString(rawMessage.trim()); + } } catch { /* keep generic classification */ } - const message = detail ? `Provider error ${response.status}: ${detail}` : `Provider error ${response.status}`; + const message = upstreamMessage + && (isCyberPolicyCode(upstreamCode) || isCyberPolicyMessage(upstreamMessage)) + ? upstreamMessage + : detail ? `Provider error ${response.status}: ${detail}` : `Provider error ${response.status}`; const classified = classifyError( response.status, upstreamType ?? (response.status === 401 ? "authentication_error" @@ -295,9 +312,9 @@ export async function handleNativeChatCompletions(options: HandleNativeChatOptio : response.status >= 500 ? "server_error" : "invalid_request_error"), message, ); - if (isCyberPolicyCode(upstreamCode)) { + if (isCyberPolicyCode(upstreamCode) || classified.code === CYBER_POLICY_ERROR_CODE) { classified.code = CYBER_POLICY_ERROR_CODE; - classified.type = "invalid_request_error"; + classified.type = cyberPolicyErrorType(upstreamType); } else if (upstreamCode === "model_not_found") { classified.code = "model_not_found"; classified.type = "invalid_request_error"; @@ -305,11 +322,13 @@ export async function handleNativeChatCompletions(options: HandleNativeChatOptio classified.code = upstreamCode; } const status = isCyberPolicyCode(classified.code) ? 400 : response.status; - const retryAfter = resolveClientRetryAfter({ - status: response.status, - message: classified.message, - upstreamRetryAfter: response.headers.get("retry-after"), - }); + const retryAfter = isCyberPolicyCode(classified.code) + ? undefined + : resolveClientRetryAfter({ + status: response.status, + message: classified.message, + upstreamRetryAfter: response.headers.get("retry-after"), + }); finishLog(status, classified.message); return new Response(JSON.stringify(chatCompletionsErrorBody(status, classified.message, classified.type, classified.code)), { status, diff --git a/src/server/relay.ts b/src/server/relay.ts index 199e22b1e8..a523ffd7e5 100644 --- a/src/server/relay.ts +++ b/src/server/relay.ts @@ -1,9 +1,12 @@ import type { ResponsesTerminalStatus } from "../bridge"; import { + cyberPolicyErrorType, CYBER_POLICY_ERROR_CODE, + CYBER_POLICY_FALLBACK_MESSAGE, isCyberPolicyCode, isCyberPolicyMessage, } from "../lib/errors"; +import { redactSecretString } from "../lib/redact"; import { isTranslatorBudgetExceededError } from "../lib/translator-budget"; import { isUsageDebugEnabled } from "../usage/debug"; import { @@ -178,13 +181,13 @@ export function createSseTerminalOutputBoundary(): SseTerminalOutputBoundary { const payload = sseDataPayload(decoder.decode(frame.block)); const isDone = payload === "[DONE]"; const parsed = payload === null ? undefined : parseSsePayload(payload); - const policyMessage = parsed !== undefined && isPolicyRewriteType(parsed) - ? cyberPolicyTerminalMessage(parsed) + const policyError = parsed !== undefined && isPolicyRewriteType(parsed) + ? cyberPolicyTerminalError(parsed) : undefined; - const outboundBlock = policyMessage + const outboundBlock = policyError ? encoder.encode(rewritePolicyTerminalBlock( decoder.decode(frame.block), - policyFailurePayload(policyMessage, parsed), + policyFailurePayload(policyError, parsed), )) : frame.block; if (isDone) { @@ -387,7 +390,7 @@ function stringField(record: JsonRecord | null, key: string): string | undefined * Deliberately inspect structured error fields and known refusal copy only — a * bare `cyber_policy` token in an unrelated payload is not sufficient. */ -function cyberPolicyTerminalMessage(parsed: unknown): string | undefined { +function cyberPolicyTerminalError(parsed: unknown): { message: string; type?: string } | undefined { const root = asJsonRecord(parsed); if (!root) return undefined; const response = asJsonRecord(root.response); @@ -402,13 +405,21 @@ function cyberPolicyTerminalMessage(parsed: unknown): string | undefined { for (const candidate of candidates) { if (!candidate) continue; if (isCyberPolicyCode(stringField(candidate, "code"))) { - return stringField(candidate, "message") - ?? "Request blocked by the upstream cybersecurity policy."; + return { + message: stringField(candidate, "message") + ?? CYBER_POLICY_FALLBACK_MESSAGE, + ...(stringField(candidate, "type") ? { type: stringField(candidate, "type") } : {}), + }; } } for (const candidate of candidates) { const message = stringField(candidate, "message"); - if (message && isCyberPolicyMessage(message)) return message; + if (message && isCyberPolicyMessage(message)) { + return { + message, + ...(stringField(candidate, "type") ? { type: stringField(candidate, "type") } : {}), + }; + } } return undefined; } @@ -443,11 +454,11 @@ function rewritePolicyTerminalBlock(block: string, payload: string): string { return withEvent.join(newline); } -function policyFailurePayload(message: string, parsed: unknown): string { +function policyFailurePayload(policyError: { message: string; type?: string }, parsed: unknown): string { const error = { - type: "invalid_request_error", + type: cyberPolicyErrorType(policyError.type), code: CYBER_POLICY_ERROR_CODE, - message: message.slice(0, MAX_TAIL_ERROR_MESSAGE_CHARS), + message: redactSecretString(policyError.message).slice(0, MAX_TAIL_ERROR_MESSAGE_CHARS), }; const root = asJsonRecord(parsed); const originalResponse = asJsonRecord(root?.response); @@ -459,6 +470,7 @@ function policyFailurePayload(message: string, parsed: unknown): string { status: "failed", error, last_error: error, + retryable: false, }; // Responses event metadata such as sequence_number is normally top-level. // Keep it (and any other non-error, non-response fields) while replacing only @@ -469,11 +481,13 @@ function policyFailurePayload(message: string, parsed: unknown): string { && key !== "response" && key !== "error" && key !== "last_error" + && key !== "retryable" ))) : {}; return JSON.stringify({ ...preservedRoot, type: "response.failed", + retryable: false, response, }); } @@ -535,9 +549,9 @@ export function terminalStatusFromParsed(parsed: unknown): ResponsesTerminalStat case "response.failed": return "failed"; case "response.incomplete": - return cyberPolicyTerminalMessage(parsed) ? "failed" : "incomplete"; + return cyberPolicyTerminalError(parsed) ? "failed" : "incomplete"; case "error": - return cyberPolicyTerminalMessage(parsed) ? "failed" : null; + return cyberPolicyTerminalError(parsed) ? "failed" : null; default: return null; } @@ -1010,7 +1024,7 @@ export function createSseInspector(handlers: SseInspectorHandlers): SseInspector const status = terminalStatusFromParsed(parsed); const policyTerminal = status === "failed" && isPolicyRewriteType(parsed) - && cyberPolicyTerminalMessage(parsed) !== undefined; + && cyberPolicyTerminalError(parsed) !== undefined; if (status) sawTerminal = true; if (!reported && handlers.onTerminal && status) { try { diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index 3aed07ef0f..5d7c74ad4b 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -71,6 +71,12 @@ import { targetKey, } from "../../combos"; import { isInjectionDebugEnabled } from "../../lib/debug-settings"; +import { + CYBER_POLICY_ERROR_CODE, + CYBER_POLICY_FALLBACK_MESSAGE, + isCyberPolicyCode, + isCyberPolicyMessage, +} from "../../lib/errors"; import { injectionDebugLog } from "../../lib/injection-debug-log"; import { resolveClientRetryAfter } from "../../lib/retry-after"; import { enrichOpenCodeZenRateLimitMessage } from "../../providers/opencode-zen-rate-limit"; @@ -662,6 +668,48 @@ export async function readDisplaySafeErrorText( } } +interface NormalizedUpstreamErrorText { + safeText: string; + message?: string; + type?: string; + code?: string; + cyberPolicy: boolean; +} + +/** + * Extract the structured provider error envelope without making `error.type` authoritative. + * Policy identity comes from the dedicated code (or the legacy message fallback); a credible + * upstream type is only carried through so callers do not erase provider diagnostics. + */ +function normalizeUpstreamErrorText(text: string, fallback: string): NormalizedUpstreamErrorText { + const safeText = redactSecretString(text).slice(0, 500).trim() || fallback; + let message: string | undefined; + let type: string | undefined; + let code: string | undefined; + try { + const parsed = JSON.parse(text) as Record; + const response = parsed.response && typeof parsed.response === "object" && !Array.isArray(parsed.response) + ? parsed.response as Record + : undefined; + const candidates = [parsed.error, response?.error, response?.last_error, parsed.last_error, parsed]; + const source = candidates.find((candidate): candidate is Record => { + if (candidate === null || typeof candidate !== "object" || Array.isArray(candidate)) return false; + const record = candidate as Record; + return [record.message, record.type, record.code].some(value => typeof value === "string"); + }); + if (!source) return { safeText, cyberPolicy: isCyberPolicyMessage(safeText) }; + if (typeof source.message === "string" && source.message.trim()) { + message = redactSecretString(source.message.trim()).slice(0, 500); + } + if (typeof source.type === "string" && source.type.trim()) type = source.type.trim(); + if (typeof source.code === "string" && source.code.trim()) code = source.code.trim(); + } catch { + /* non-JSON upstream body — retain the bounded display-safe text */ + } + const cyberPolicy = isCyberPolicyCode(code) || isCyberPolicyMessage(message ?? safeText); + return { safeText, message, type, code, cyberPolicy }; +} + function prepareOpaqueBlobRecovery(parsed: OcxParsedRequest): void { parsed._stripReasoningEncryptedContent = true; } @@ -1312,27 +1360,30 @@ export async function consumeComboFailure( let classificationText = fallback; let usage: OcxUsage | undefined; let upstreamCode: string | undefined; + let upstreamMessage: string | undefined; + let upstreamType: string | undefined; try { const body = await readBoundedResponseBody(response, { signal }); usage = usageFromComboFailureText(body.text); if (body.displaySafe) { - const safeText = redactSecretString(body.text).slice(0, 500); - if (safeText) classificationText = safeText; - try { - const parsed = JSON.parse(body.text) as { error?: { code?: unknown } | string }; - const nested = typeof parsed?.error === "object" && parsed.error ? parsed.error.code : undefined; - if (typeof nested === "string" && nested.length > 0) upstreamCode = nested; - } catch { - /* non-JSON upstream body — message-only classification */ - } + const normalized = normalizeUpstreamErrorText(body.text, fallback); + classificationText = normalized.safeText; + upstreamCode = normalized.code; + upstreamMessage = normalized.message; + upstreamType = normalized.type; } } catch (error) { if (signal?.aborted) throw error; classificationText = fallback; } - const message = classificationText === fallback - ? fallback - : `${fallback}: ${classificationText}`; + const cyberFailure = isCyberPolicyCode(upstreamCode) || isCyberPolicyMessage(classificationText); + const normalizedUpstreamCode = cyberFailure ? CYBER_POLICY_ERROR_CODE : upstreamCode; + const message = cyberFailure + ? upstreamMessage + ?? (isCyberPolicyCode(upstreamCode) ? CYBER_POLICY_FALLBACK_MESSAGE : classificationText) + : classificationText === fallback + ? fallback + : `${fallback}: ${classificationText}`; const upstreamRetryAfter = response.headers.get("retry-after"); // Client response may get the synthetic "2" fallback; cooldown metadata must not — // otherwise coolComboTarget treats it as a 2s cooldown instead of the 60s default. @@ -1350,13 +1401,18 @@ export async function consumeComboFailure( includeDefault: false, }); return { - response: formatErrorResponse(response.status, "upstream_error", message, { - ...(upstreamCode !== undefined ? { code: upstreamCode } : {}), - ...(clientRetryAfter !== undefined ? { retryAfter: clientRetryAfter } : {}), - }), + response: formatErrorResponse( + response.status, + cyberFailure ? (upstreamType ?? CYBER_POLICY_ERROR_CODE) : "upstream_error", + message, + { + ...(normalizedUpstreamCode !== undefined ? { code: normalizedUpstreamCode } : {}), + ...(clientRetryAfter !== undefined ? { retryAfter: clientRetryAfter } : {}), + }, + ), classificationText, - ...(upstreamCode !== undefined ? { upstreamCode } : {}), - ...(cooldownRetryAfter !== undefined ? { retryAfter: cooldownRetryAfter } : {}), + ...(normalizedUpstreamCode !== undefined ? { upstreamCode: normalizedUpstreamCode } : {}), + ...(!cyberFailure && cooldownRetryAfter !== undefined ? { retryAfter: cooldownRetryAfter } : {}), ...(usage ? { usage } : {}), }; } @@ -4952,29 +5008,41 @@ async function handleResponsesInner( // Upstreams occasionally echo request details in error bodies — scrub token-shaped // material before it reaches the client-facing error surface. const upstreamRetryAfter = upstreamResponse.headers.get("retry-after"); - const message = enrichOpenCodeZenRateLimitMessage( - `Provider error ${upstreamResponse.status}: ${redactSecretString(errorText.slice(0, 500))}`, - { + const normalized = normalizeUpstreamErrorText(errorText, "unknown error"); + const message = normalized.cyberPolicy + ? normalized.message + ?? (isCyberPolicyCode(normalized.code) ? CYBER_POLICY_FALLBACK_MESSAGE : normalized.safeText) + : enrichOpenCodeZenRateLimitMessage( + `Provider error ${upstreamResponse.status}: ${normalized.safeText}`, + { + status: upstreamResponse.status, + providerName: route.providerName, + baseUrl: route.provider.baseUrl, + adapter: route.provider.adapter, + authMode: route.provider.authMode, + hasApiKey: Boolean(route.provider.apiKey?.trim()), + upstreamRetryAfter, + // This recovery path is the HTTP Responses wire; custom runTurn transports + // never reach enrichOpenCodeZenRateLimitMessage here. + supportsHttpSameKeyRetry: true, + }, + ); + const retryAfter = normalized.cyberPolicy + ? undefined + : resolveClientRetryAfter({ status: upstreamResponse.status, - providerName: route.providerName, - baseUrl: route.provider.baseUrl, - adapter: route.provider.adapter, - authMode: route.provider.authMode, - hasApiKey: Boolean(route.provider.apiKey?.trim()), + message, upstreamRetryAfter, - // This recovery path is the HTTP Responses wire; custom runTurn transports - // never reach enrichOpenCodeZenRateLimitMessage here. - supportsHttpSameKeyRetry: true, + }); + return formatErrorResponse( + upstreamResponse.status, + normalized.cyberPolicy ? (normalized.type ?? CYBER_POLICY_ERROR_CODE) : "upstream_error", + message, + { + ...(normalized.cyberPolicy ? { code: CYBER_POLICY_ERROR_CODE } : {}), + ...(retryAfter !== undefined ? { retryAfter } : {}), }, ); - const retryAfter = resolveClientRetryAfter({ - status: upstreamResponse.status, - message, - upstreamRetryAfter, - }); - return formatErrorResponse(upstreamResponse.status, "upstream_error", message, { - ...(retryAfter !== undefined ? { retryAfter } : {}), - }); } } @@ -5229,10 +5297,21 @@ async function handleResponsesInner( if (!response.ok) { const errorText = await readDisplaySafeErrorText(response, upstream.signal, "unknown error"); + const normalized = normalizeUpstreamErrorText(errorText, "unknown error"); yield { type: "error", - status: response.status, - message: `Provider continuation error ${response.status}: ${redactSecretString(errorText.slice(0, 500))}`, + status: normalized.cyberPolicy ? 400 : response.status, + message: normalized.cyberPolicy + ? normalized.message + ?? (isCyberPolicyCode(normalized.code) ? CYBER_POLICY_FALLBACK_MESSAGE : normalized.safeText) + : `Provider continuation error ${response.status}: ${normalized.safeText}`, + ...(normalized.cyberPolicy + ? { + errorType: normalized.type ?? CYBER_POLICY_ERROR_CODE, + code: CYBER_POLICY_ERROR_CODE, + retryable: false, + } + : {}), }; return; } diff --git a/src/server/responses/passthrough-error.ts b/src/server/responses/passthrough-error.ts index 4b1be17b2e..a9d9ad7d0f 100644 --- a/src/server/responses/passthrough-error.ts +++ b/src/server/responses/passthrough-error.ts @@ -1,9 +1,29 @@ import { formatErrorResponse } from "../../bridge"; +import { isCyberPolicyCode, isCyberPolicyMessage } from "../../lib/errors"; import { resolveClientRetryAfter, validateClientRetryAfterHeader, } from "../../lib/retry-after"; +function isCyberPolicyBody(body: string): boolean { + if (isCyberPolicyMessage(body)) return true; + try { + const parsed = JSON.parse(body) as Record; + const response = parsed.response && typeof parsed.response === "object" && !Array.isArray(parsed.response) + ? parsed.response as Record + : undefined; + for (const candidate of [parsed.error, response?.error, response?.last_error, parsed.last_error, parsed]) { + if (!candidate || typeof candidate !== "object" || Array.isArray(candidate)) continue; + const record = candidate as Record; + if (isCyberPolicyCode(typeof record.code === "string" ? record.code : undefined)) return true; + if (typeof record.message === "string" && isCyberPolicyMessage(record.message)) return true; + } + } catch { + /* non-JSON body — message detection above is the only safe fallback */ + } + return false; +} + /** * Passthrough adapters historically relayed upstream non-2xx bodies verbatim. * Codex maps an *empty* body to the literal client string "Unknown error" @@ -32,18 +52,22 @@ export function formatPassthroughUpstreamError( const now = options?.now ?? Date.now(); const upstreamRetryAfter = options?.headers?.get("retry-after")?.trim() || undefined; const originalValid = validateClientRetryAfterHeader(upstreamRetryAfter, now); - const resolved = resolveClientRetryAfter({ - status, - message: trimmed || `Provider error ${status}: (empty body)`, - upstreamRetryAfter, - now, - }); + const cyberPolicyFailure = isCyberPolicyBody(trimmed); + const resolved = cyberPolicyFailure + ? undefined + : resolveClientRetryAfter({ + status, + message: trimmed || `Provider error ${status}: (empty body)`, + upstreamRetryAfter, + now, + }); if (trimmed) { const needsSet = resolved !== undefined && upstreamRetryAfter !== resolved; - const needsDelete = resolved === undefined - && upstreamRetryAfter !== undefined - && originalValid === undefined; + const needsDelete = (cyberPolicyFailure && upstreamRetryAfter !== undefined) + || (resolved === undefined + && upstreamRetryAfter !== undefined + && originalValid === undefined); if (!needsSet && !needsDelete) { return new Response(bodyText, { diff --git a/tests/chat-completions-endpoint.test.ts b/tests/chat-completions-endpoint.test.ts index 1f4009c9a5..d310faebc2 100644 --- a/tests/chat-completions-endpoint.test.ts +++ b/tests/chat-completions-endpoint.test.ts @@ -1331,6 +1331,57 @@ test("chat-native redacts structured provider errors before returning them", asy } }); +test("chat-native preserves a structured cyber_policy type on JSON and SSE failures", async () => { + const secret = "blocked by upstream policy Authorization: Bearer chatnativesecret123456"; + const safeMessage = "blocked by upstream policy Authorization: Bearer [REDACTED]"; + const upstream = Bun.serve({ + port: 0, + async fetch(req) { + const body = await req.json() as { stream?: boolean }; + const error = { + message: secret, + type: "server_error", + code: "cyber_policy", + status: 502, + }; + if (body.stream) { + return new Response(`data: ${JSON.stringify({ error })}\n\n`, { + headers: { "content-type": "text/event-stream" }, + }); + } + return Response.json(error, { status: 502, headers: { "retry-after": "120" } }); + }, + }); + saveConfig(mockConfig(`${upstream.url.toString().replace(/\/$/, "")}/v1`)); + const server = startServer(0); + try { + const request = (stream: boolean) => fetch(new URL("/v1/chat/completions", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "mock/test-model", stream, messages: [{ role: "user", content: "hi" }] }), + }); + + const jsonResponse = await request(false); + expect(jsonResponse.status).toBe(400); + expect(jsonResponse.headers.get("retry-after")).toBeNull(); + await expect(jsonResponse.json()).resolves.toMatchObject({ + error: { type: "server_error", code: "cyber_policy", message: safeMessage }, + }); + + const sseResponse = await request(true); + expect(sseResponse.status).toBe(200); + const sseText = await sseResponse.text(); + expect(sseText).toContain('"type":"server_error"'); + expect(sseText).toContain('"code":"cyber_policy"'); + expect(sseText).toContain("Authorization: Bearer [REDACTED]"); + expect(sseText).not.toContain("chatnativesecret123456"); + expect(sseText).not.toContain("data: [DONE]"); + } finally { + await server.stop(true); + upstream.stop(true); + } +}); + test("chat-native preserves same-key retry, key rotation, usage, and request logging", async () => { const { clearRequestLogsForTests, getRequestLogEntries } = await import("../src/server/request-log"); const { clearKeyCooldowns } = await import("../src/providers/key-failover"); @@ -2663,6 +2714,66 @@ test("inbound chat-completions honors the override when stripping sampling (#404 } }); +test("/v1/chat/completions non-OK upstream preserves top-level structured cyber_policy type", async () => { + const secret = "blocked by upstream policy Authorization: Bearer chathttpsecret123456"; + const safeMessage = "blocked by upstream policy Authorization: Bearer [REDACTED]"; + const upstream = Bun.serve({ + port: 0, + fetch() { + return Response.json({ + message: secret, + type: "server_error", + code: "cyber_policy", + }, { status: 400, headers: { "retry-after": "120" } }); + }, + }); + const originalFetch = globalThis.fetch; + globalThis.fetch = ((input: RequestInfo | URL, init?: RequestInit) => { + const requestUrl = typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; + const url = new URL(requestUrl); + if (url.hostname === "chatgpt.com" && url.pathname.startsWith("/backend-api/codex")) { + return originalFetch(new URL(`${url.pathname.slice("/backend-api/codex".length)}${url.search}`, upstream.url), init); + } + return originalFetch(input, init); + }) as typeof fetch; + saveConfig({ + port: 0, + defaultProvider: "openai", + providers: { + openai: { + adapter: "openai-responses", + baseUrl: "https://chatgpt.com/backend-api/codex", + authMode: "forward", + codexAccountMode: "direct", + }, + }, + } as OcxConfig); + const server = startServer(0); + try { + const response = await fetch(new URL("/v1/chat/completions", server.url), { + method: "POST", + headers: { + "content-type": "application/json", + authorization: ["Bear" + "er", "caller-direct-token"].join(" "), + }, + body: JSON.stringify({ + model: "gpt-test", + stream: false, + messages: [{ role: "user", content: "hi" }], + }), + }); + expect(response.status).toBe(400); + expect(response.headers.get("retry-after")).toBeNull(); + await expect(response.json()).resolves.toMatchObject({ + error: { type: "server_error", code: "cyber_policy", message: safeMessage }, + }); + } finally { + await server.stop(true); + upstream.stop(true); + globalThis.fetch = originalFetch; + } +}); + test("/v1/chat/completions non-OK upstream preserves structured model_not_found", async () => { const upstream = Bun.serve({ port: 0, @@ -2788,6 +2899,70 @@ test("/v1/chat/completions status:failed replay normalizes translation_buffer_li } }); +test("/v1/chat/completions status:failed replay preserves structured cyber_policy type", async () => { + const secret = "blocked by upstream policy Authorization: Bearer chatreplaysecret123456"; + const safeMessage = "blocked by upstream policy Authorization: Bearer [REDACTED]"; + const upstream = Bun.serve({ + port: 0, + fetch() { + return Response.json({ + id: "resp_policy", + object: "response", + status: "failed", + error: { + message: secret, + type: "server_error", + code: "cyber_policy", + }, + }); + }, + }); + const originalFetch = globalThis.fetch; + globalThis.fetch = ((input: RequestInfo | URL, init?: RequestInit) => { + const requestUrl = typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; + const url = new URL(requestUrl); + if (url.hostname === "chatgpt.com" && url.pathname.startsWith("/backend-api/codex")) { + return originalFetch(new URL(`${url.pathname.slice("/backend-api/codex".length)}${url.search}`, upstream.url), init); + } + return originalFetch(input, init); + }) as typeof fetch; + saveConfig({ + port: 0, + defaultProvider: "openai", + providers: { + openai: { + adapter: "openai-responses", + baseUrl: "https://chatgpt.com/backend-api/codex", + authMode: "forward", + codexAccountMode: "direct", + }, + }, + } as OcxConfig); + const server = startServer(0); + try { + const response = await fetch(new URL("/v1/chat/completions", server.url), { + method: "POST", + headers: { + "content-type": "application/json", + authorization: ["Bear" + "er", "caller-direct-token"].join(" "), + }, + body: JSON.stringify({ + model: "gpt-test", + stream: false, + messages: [{ role: "user", content: "hi" }], + }), + }); + expect(response.status).toBe(400); + await expect(response.json()).resolves.toMatchObject({ + error: { type: "server_error", code: "cyber_policy", message: safeMessage }, + }); + } finally { + await server.stop(true); + upstream.stop(true); + globalThis.fetch = originalFetch; + } +}); + test("/v1/chat/completions status:failed replay preserves structured model_not_found", async () => { const upstream = Bun.serve({ port: 0, diff --git a/tests/cyber-policy-error-fidelity.test.ts b/tests/cyber-policy-error-fidelity.test.ts index b6610a078d..c69fd77fc4 100644 --- a/tests/cyber-policy-error-fidelity.test.ts +++ b/tests/cyber-policy-error-fidelity.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { createOpenAIChatAdapter as createOpenAIChatAdapterProduction } from "../src/adapters/openai-chat"; -import { bridgeToResponsesSSE, formatErrorResponse } from "../src/bridge"; +import { bridgeToResponsesSSE, buildResponseJSON, formatErrorResponse } from "../src/bridge"; import { chatCompletionsErrorBody, chatCompletionsErrorResponse, @@ -15,7 +15,8 @@ import { } from "../src/lib/errors"; import { formatPassthroughUpstreamError } from "../src/server/responses/passthrough-error"; import { consumeComboFailure } from "../src/server/responses/core"; -import type { AdapterEvent } from "../src/types"; +import { handleResponses } from "../src/server/responses"; +import type { AdapterEvent, OcxConfig } from "../src/types"; import { createTestTranslatorBudget, withTestTranslatorBudget } from "./helpers/translator-budget"; const createOpenAIChatAdapter = (...args: Parameters) => @@ -37,6 +38,9 @@ const CURSOR_SESSION_CYBER_MESSAGE = const OPENAI_CYBER_MESSAGE = "OpenAI flagged this request for potential high-risk cybersecurity activity. Please try a less sensitive prompt."; +const SECRET_CYBER_MESSAGE = `${OPENAI_CYBER_MESSAGE} Authorization: Bearer cybersecret123456`; +const REDACTED_CYBER_MESSAGE = `${OPENAI_CYBER_MESSAGE} Authorization: Bearer [REDACTED]`; + const CODEX_FALLBACK_MESSAGE = "This request has been flagged for possible cybersecurity risk."; const CYBER_ERROR_BODY = { @@ -81,7 +85,7 @@ async function collectAdapter(gen: AsyncGenerator): Promise { test("classifyError maps cyber messages and explicit type to cyber_policy", () => { expect(classifyError(400, "upstream_error", OPENAI_CYBER_MESSAGE)).toMatchObject({ - type: "invalid_request_error", + type: CYBER_POLICY_ERROR_CODE, code: CYBER_POLICY_ERROR_CODE, }); expect(classifyError(502, "upstream_error", CURSOR_SESSION_CYBER_MESSAGE)).toMatchObject({ @@ -108,7 +112,7 @@ describe("cyber_policy error fidelity", () => { test("adapterFailureFromMessage prefers HTTP 400 + cyber_policy (not 502)", () => { expect(adapterFailureFromMessage(OPENAI_CYBER_MESSAGE)).toMatchObject({ httpStatus: 400, - error: { type: "invalid_request_error", code: CYBER_POLICY_ERROR_CODE }, + error: { type: CYBER_POLICY_ERROR_CODE, code: CYBER_POLICY_ERROR_CODE }, }); expect(adapterFailureFromMessage(CURSOR_SESSION_CYBER_MESSAGE)).toMatchObject({ httpStatus: 400, @@ -122,34 +126,117 @@ describe("cyber_policy error fidelity", () => { await expect(fromMessage.json()).resolves.toEqual({ error: { message: OPENAI_CYBER_MESSAGE, - type: "invalid_request_error", + type: CYBER_POLICY_ERROR_CODE, code: CYBER_POLICY_ERROR_CODE, }, }); - const fromCode = formatErrorResponse(502, "upstream_error", "blocked by safety", { + const fromCode = formatErrorResponse(502, "server_error", "blocked by safety", { code: CYBER_POLICY_ERROR_CODE, + retryAfter: "120", }); expect(fromCode.status).toBe(400); + expect(fromCode.headers.get("retry-after")).toBeNull(); await expect(fromCode.json()).resolves.toMatchObject({ - error: { code: CYBER_POLICY_ERROR_CODE, type: "invalid_request_error" }, + error: { code: CYBER_POLICY_ERROR_CODE, type: "server_error" }, }); }); test("passthrough HTTP 400 cyber body is relayed verbatim", async () => { const body = JSON.stringify(CYBER_ERROR_BODY); - const response = formatPassthroughUpstreamError(400, body); + const response = formatPassthroughUpstreamError(400, body, { + headers: new Headers({ "content-type": "application/json", "retry-after": "120" }), + }); expect(response.status).toBe(400); expect(await response.text()).toBe(body); + expect(response.headers.get("retry-after")).toBeNull(); + + const codeOnlyBody = JSON.stringify({ + error: { message: "blocked by policy", type: "server_error", code: CYBER_POLICY_ERROR_CODE }, + }); + const codeOnlyResponse = formatPassthroughUpstreamError(400, codeOnlyBody, { + headers: new Headers({ "content-type": "application/json", "retry-after": "120" }), + }); + expect(await codeOnlyResponse.text()).toBe(codeOnlyBody); + expect(codeOnlyResponse.headers.get("retry-after")).toBeNull(); }); test("consumeComboFailure preserves cyber_policy code as HTTP 400", async () => { - const upstream = new Response(JSON.stringify(CYBER_ERROR_BODY), { status: 400 }); + const upstream = new Response(JSON.stringify(CYBER_ERROR_BODY), { + status: 400, + headers: { "retry-after": "120" }, + }); const failure = await consumeComboFailure(upstream); expect(failure.response.status).toBe(400); + expect(failure.response.headers.get("retry-after")).toBeNull(); expect(failure.upstreamCode).toBe(CYBER_POLICY_ERROR_CODE); await expect(failure.response.json()).resolves.toMatchObject({ - error: { code: CYBER_POLICY_ERROR_CODE, type: "invalid_request_error" }, + error: { + code: CYBER_POLICY_ERROR_CODE, + type: "invalid_request", + message: OPENAI_CYBER_MESSAGE, + }, + }); + }); + + test("ordinary Responses HTTP failure preserves structured cyber type and exact safe message", async () => { + const upstream = Bun.serve({ + port: 0, + fetch() { + return Response.json({ + error: { + message: SECRET_CYBER_MESSAGE, + type: "server_error", + code: CYBER_POLICY_ERROR_CODE, + }, + }, { status: 400, headers: { "retry-after": "120" } }); + }, + }); + const config = { + port: 0, + defaultProvider: "mock", + providers: { + mock: { + adapter: "openai-chat", + baseUrl: `${upstream.url.toString().replace(/\/$/, "")}/v1`, + apiKey: "fixture-key", + allowPrivateNetwork: true, + }, + }, + } as OcxConfig; + try { + const response = await handleResponses(new Request("http://localhost/v1/responses", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: "mock/test-model", input: "hello", stream: false }), + }), config, { model: "", provider: "" }); + expect(response.status).toBe(400); + expect(response.headers.get("retry-after")).toBeNull(); + await expect(response.json()).resolves.toEqual({ + error: { + message: REDACTED_CYBER_MESSAGE, + type: "server_error", + code: CYBER_POLICY_ERROR_CODE, + }, + }); + } finally { + upstream.stop(true); + } + }); + + test("message-only combo cyber failures preserve structured type and redact the message", async () => { + const upstream = new Response(JSON.stringify({ + error: { type: "server_error", message: SECRET_CYBER_MESSAGE }, + }), { status: 400 }); + const failure = await consumeComboFailure(upstream); + expect(failure.response.status).toBe(400); + expect(failure.upstreamCode).toBe(CYBER_POLICY_ERROR_CODE); + await expect(failure.response.json()).resolves.toMatchObject({ + error: { + type: "server_error", + code: CYBER_POLICY_ERROR_CODE, + message: REDACTED_CYBER_MESSAGE, + }, }); }); @@ -170,18 +257,11 @@ describe("cyber_policy error fidelity", () => { status: 400, }); - const frames = await collectSse(bridgeToResponsesSSE(replay([ - { type: "text_delta", text: "partial" }, - { - type: "error", - message: OPENAI_CYBER_MESSAGE, - code: CYBER_POLICY_ERROR_CODE, - status: 400, - }, - ]), "openai/gpt-5.4")); + expect(events.find(e => e.type === "error")).toMatchObject({ errorType: "invalid_request" }); + const frames = await collectSse(bridgeToResponsesSSE(replay(events), "openai/gpt-5.4")); const failed = frames.find(frame => frame.event === "response.failed")?.data.response as Record; expect(failed.error).toMatchObject({ - type: "invalid_request_error", + type: "invalid_request", code: CYBER_POLICY_ERROR_CODE, message: OPENAI_CYBER_MESSAGE, }); @@ -191,10 +271,25 @@ describe("cyber_policy error fidelity", () => { test("message-only cyber adapter error still classifies (no silent 502)", async () => { const frames = await collectSse(bridgeToResponsesSSE(replay([ - { type: "error", message: OPENAI_CYBER_MESSAGE }, + { type: "error", message: SECRET_CYBER_MESSAGE, retryable: true }, ]), "openai/gpt-5.4")); const failed = frames.find(frame => frame.event === "response.failed")?.data.response as Record; - expect(failed.error).toMatchObject({ code: CYBER_POLICY_ERROR_CODE }); + expect(failed.error).toMatchObject({ + type: CYBER_POLICY_ERROR_CODE, + code: CYBER_POLICY_ERROR_CODE, + message: REDACTED_CYBER_MESSAGE, + }); + expect(failed.retryable).toBe(false); + expect(JSON.stringify(failed)).not.toContain("cybersecret123456"); + + const buffered = buildResponseJSON([ + { type: "error", message: SECRET_CYBER_MESSAGE, retryable: true }, + ], "openai/gpt-5.4"); + expect(buffered).toMatchObject({ + status: "failed", + retryable: false, + error: { code: CYBER_POLICY_ERROR_CODE, message: REDACTED_CYBER_MESSAGE }, + }); }); test("chat completions error envelope preserves cyber_policy and model_not_found", async () => { @@ -206,6 +301,14 @@ describe("cyber_policy error fidelity", () => { code: CYBER_POLICY_ERROR_CODE, }, }); + expect(chatCompletionsErrorBody(400, OPENAI_CYBER_MESSAGE)).toEqual({ + error: { + message: OPENAI_CYBER_MESSAGE, + type: CYBER_POLICY_ERROR_CODE, + param: null, + code: CYBER_POLICY_ERROR_CODE, + }, + }); expect(chatCompletionsErrorBody(404, "model not found", "invalid_request_error")).toEqual({ error: { message: "model not found", @@ -217,7 +320,7 @@ describe("cyber_policy error fidelity", () => { const response = chatCompletionsErrorResponse(502, OPENAI_CYBER_MESSAGE, "server_error"); expect(response.status).toBe(400); await expect(response.json()).resolves.toMatchObject({ - error: { code: CYBER_POLICY_ERROR_CODE, param: null }, + error: { type: "server_error", code: CYBER_POLICY_ERROR_CODE, param: null }, }); // Non-cyber classification must not rewrite HTTP status. const rateLimited = chatCompletionsErrorResponse(502, "Rate limit reached for model", "server_error"); @@ -236,7 +339,7 @@ describe("cyber_policy error fidelity", () => { response: { status: "failed", error: { - message: OPENAI_CYBER_MESSAGE, + message: SECRET_CYBER_MESSAGE, type: "invalid_request_error", code: CYBER_POLICY_ERROR_CODE, }, @@ -257,8 +360,9 @@ describe("cyber_policy error fidelity", () => { expect(errorFrame?.data.error).toMatchObject({ code: CYBER_POLICY_ERROR_CODE, type: "invalid_request_error", - message: OPENAI_CYBER_MESSAGE, + message: REDACTED_CYBER_MESSAGE, }); + expect(JSON.stringify(errorFrame?.data)).not.toContain("cybersecret123456"); }); test("openai-chat cyber_policy forces HTTP 400 even when upstream status is 5xx", async () => { diff --git a/tests/passthrough-abort.test.ts b/tests/passthrough-abort.test.ts index 40c79317cd..1e2264f7e8 100644 --- a/tests/passthrough-abort.test.ts +++ b/tests/passthrough-abort.test.ts @@ -245,7 +245,7 @@ describe("passthrough relayWithAbort (RC2, passthrough path)", () => { error: { type: "server_error", code: "cyber_policy", - message: "blocked by upstream policy", + message: "blocked by upstream policy Authorization: Bearer relaypullsecret123456", }, }, }); @@ -256,9 +256,10 @@ describe("passthrough relayWithAbort (RC2, passthrough path)", () => { const text = await readAll(relayed); expect(text.match(/event: response\.failed/g)?.length).toBe(1); expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); - expect(text).toContain('"type":"invalid_request_error"'); + expect(text).toContain('"type":"server_error"'); expect(text).toContain('"code":"cyber_policy"'); - expect(text).not.toContain('"type":"server_error"'); + expect(text).toContain("Authorization: Bearer [REDACTED]"); + expect(text).not.toContain("relaypullsecret123456"); expect(text).toContain('"sequence_number":19'); expect(text).toContain('"model":"gpt-policy"'); expect(text).toContain('"id":"resp-structured-policy-pull"'); diff --git a/tests/relay-eager.test.ts b/tests/relay-eager.test.ts index ed82ee59c1..f73244b830 100644 --- a/tests/relay-eager.test.ts +++ b/tests/relay-eager.test.ts @@ -558,14 +558,16 @@ describe("relaySseEagerBounded — side-effect parity", () => { type: "response.failed", sequence_number: 18, model: "gpt-policy", + retryable: true, response: { id: "resp-structured-policy-eager", output: [{ type: "message", id: "item-structured-policy-eager" }], status: "failed", + retryable: true, error: { type: "server_error", code: "cyber_policy", - message: "blocked by upstream policy", + message: "blocked by upstream policy Authorization: Bearer relayeagersecret123456", }, }, }); @@ -577,9 +579,12 @@ describe("relaySseEagerBounded — side-effect parity", () => { const text = await readAll(relayed); expect(text.match(/event: response\.failed/g)?.length).toBe(1); expect(text.match(/data: \[DONE\]/g)?.length).toBe(1); - expect(text).toContain('"type":"invalid_request_error"'); + expect(text).toContain('"type":"server_error"'); expect(text).toContain('"code":"cyber_policy"'); - expect(text).not.toContain('"type":"server_error"'); + expect(text.match(/"retryable":false/g)?.length).toBe(2); + expect(text).not.toContain('"retryable":true'); + expect(text).toContain("Authorization: Bearer [REDACTED]"); + expect(text).not.toContain("relayeagersecret123456"); expect(text).toContain('"sequence_number":18'); expect(text).toContain('"model":"gpt-policy"'); expect(text).toContain('"id":"resp-structured-policy-eager"'); diff --git a/tests/terminal-guard-server.test.ts b/tests/terminal-guard-server.test.ts index 999138cf46..9cc2956fbc 100644 --- a/tests/terminal-guard-server.test.ts +++ b/tests/terminal-guard-server.test.ts @@ -260,6 +260,43 @@ describe("server terminal guard integration", () => { expect(text).toContain("Provider continuation error 429"); }); + test("terminal-guard continuation preserves structured cyber_policy semantics", async () => { + const secret = "OpenAI flagged this request for potential high-risk cybersecurity activity. Authorization: Bearer continuationsecret123456"; + let sends = 0; + globalThis.fetch = (async () => { + sends += 1; + if (sends === 1) return anthropicSse(firstTurn); + return Response.json({ + error: { + message: secret, + type: "server_error", + code: "cyber_policy", + }, + }, { status: 400, headers: { "retry-after": "120" } }); + }) as typeof fetch; + + const response = await handleResponses(new Request("http://localhost/v1/responses", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + model: "se-claude-opus-4.8", + input: "请检查这个问题并修复代码", + stream: true, + tools: [{ type: "function", name: "exec_command", description: "run a command", parameters: { type: "object" } }], + }), + }), config, { model: "", provider: "" }); + + const text = await response.text(); + expect(response.status).toBe(200); + expect(sends).toBe(2); + expect(text.match(/event: response\.failed/g)?.length).toBe(1); + expect(text).toContain('"type":"server_error"'); + expect(text).toContain('"code":"cyber_policy"'); + expect(text).toContain("Authorization: Bearer [REDACTED]"); + expect(text).not.toContain("continuationsecret123456"); + expect(text).not.toContain("Provider continuation error 400"); + }); + test("terminal-guard continuation abort during the 429 wait yields 499 without replaying", async () => { const abortConfig = { ...config, From c8c640ee59428531b36c2e9f7b59bf2285a784a1 Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 16:04:57 +0900 Subject: [PATCH 18/19] fix(responses): mark thrown policy failures non-retryable --- src/bridge.ts | 10 ++++++++-- tests/cyber-policy-error-fidelity.test.ts | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/bridge.ts b/src/bridge.ts index 910f14f885..1276b1406b 100644 --- a/src/bridge.ts +++ b/src/bridge.ts @@ -1331,11 +1331,17 @@ export function bridgeToResponsesSSE( if (currentToolCall) failCurrentToolCall(); if (currentWebSearch) closeCurrentWebSearch("failed", []); releasePendingWebSources(); + const failure = responseError( + 500, + "proxy_error", + redactSecretString(err instanceof Error ? err.message : String(err)), + ); emit("response.failed", { response: { ...responseSnapshot("failed", finishedItems), - error: responseError(500, "proxy_error", err instanceof Error ? err.message : String(err)), - last_error: responseError(500, "proxy_error", err instanceof Error ? err.message : String(err)), + error: failure, + last_error: failure, + ...(isCyberPolicyCode(failure.code) ? { retryable: false } : {}), }, }); reportTerminal("failed"); diff --git a/tests/cyber-policy-error-fidelity.test.ts b/tests/cyber-policy-error-fidelity.test.ts index c69fd77fc4..0acd95b8b0 100644 --- a/tests/cyber-policy-error-fidelity.test.ts +++ b/tests/cyber-policy-error-fidelity.test.ts @@ -292,6 +292,24 @@ describe("cyber_policy error fidelity", () => { }); }); + test("thrown cyber failures are redacted and explicitly non-retryable", async () => { + async function* throwingEvents(): AsyncGenerator { + throw new Error(SECRET_CYBER_MESSAGE); + } + const frames = await collectSse(bridgeToResponsesSSE(throwingEvents(), "openai/gpt-5.4")); + const failed = frames.find(frame => frame.event === "response.failed")?.data.response as Record; + expect(failed).toMatchObject({ + status: "failed", + retryable: false, + error: { + type: CYBER_POLICY_ERROR_CODE, + code: CYBER_POLICY_ERROR_CODE, + message: REDACTED_CYBER_MESSAGE, + }, + }); + expect(JSON.stringify(failed)).not.toContain("cybersecret123456"); + }); + test("chat completions error envelope preserves cyber_policy and model_not_found", async () => { expect(chatCompletionsErrorBody(400, OPENAI_CYBER_MESSAGE, "invalid_request_error", CYBER_POLICY_ERROR_CODE)).toEqual({ error: { From 30295addd9032cce3a1ba695ddadfc064f9b7471 Mon Sep 17 00:00:00 2001 From: AiriDea <28827642+AiriDea@users.noreply.github.com> Date: Tue, 25 Aug 2026 16:12:10 +0900 Subject: [PATCH 19/19] test(responses): keep policy fixtures privacy-safe --- tests/chat-completions-endpoint.test.ts | 6 +++--- tests/cyber-policy-error-fidelity.test.ts | 2 +- tests/passthrough-abort.test.ts | 2 +- tests/relay-eager.test.ts | 2 +- tests/terminal-guard-server.test.ts | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/chat-completions-endpoint.test.ts b/tests/chat-completions-endpoint.test.ts index d310faebc2..cf8dbe1efb 100644 --- a/tests/chat-completions-endpoint.test.ts +++ b/tests/chat-completions-endpoint.test.ts @@ -1332,7 +1332,7 @@ test("chat-native redacts structured provider errors before returning them", asy }); test("chat-native preserves a structured cyber_policy type on JSON and SSE failures", async () => { - const secret = "blocked by upstream policy Authorization: Bearer chatnativesecret123456"; + const secret = `blocked by upstream policy Authorization: ${["Bear", "er"].join("")} chatnativesecret123456`; const safeMessage = "blocked by upstream policy Authorization: Bearer [REDACTED]"; const upstream = Bun.serve({ port: 0, @@ -2715,7 +2715,7 @@ test("inbound chat-completions honors the override when stripping sampling (#404 }); test("/v1/chat/completions non-OK upstream preserves top-level structured cyber_policy type", async () => { - const secret = "blocked by upstream policy Authorization: Bearer chathttpsecret123456"; + const secret = `blocked by upstream policy Authorization: ${["Bear", "er"].join("")} chathttpsecret123456`; const safeMessage = "blocked by upstream policy Authorization: Bearer [REDACTED]"; const upstream = Bun.serve({ port: 0, @@ -2900,7 +2900,7 @@ test("/v1/chat/completions status:failed replay normalizes translation_buffer_li }); test("/v1/chat/completions status:failed replay preserves structured cyber_policy type", async () => { - const secret = "blocked by upstream policy Authorization: Bearer chatreplaysecret123456"; + const secret = `blocked by upstream policy Authorization: ${["Bear", "er"].join("")} chatreplaysecret123456`; const safeMessage = "blocked by upstream policy Authorization: Bearer [REDACTED]"; const upstream = Bun.serve({ port: 0, diff --git a/tests/cyber-policy-error-fidelity.test.ts b/tests/cyber-policy-error-fidelity.test.ts index 0acd95b8b0..d7c11cc42a 100644 --- a/tests/cyber-policy-error-fidelity.test.ts +++ b/tests/cyber-policy-error-fidelity.test.ts @@ -38,7 +38,7 @@ const CURSOR_SESSION_CYBER_MESSAGE = const OPENAI_CYBER_MESSAGE = "OpenAI flagged this request for potential high-risk cybersecurity activity. Please try a less sensitive prompt."; -const SECRET_CYBER_MESSAGE = `${OPENAI_CYBER_MESSAGE} Authorization: Bearer cybersecret123456`; +const SECRET_CYBER_MESSAGE = `${OPENAI_CYBER_MESSAGE} Authorization: ${["Bear", "er"].join("")} cybersecret123456`; const REDACTED_CYBER_MESSAGE = `${OPENAI_CYBER_MESSAGE} Authorization: Bearer [REDACTED]`; const CODEX_FALLBACK_MESSAGE = "This request has been flagged for possible cybersecurity risk."; diff --git a/tests/passthrough-abort.test.ts b/tests/passthrough-abort.test.ts index 1e2264f7e8..1a6aae29cd 100644 --- a/tests/passthrough-abort.test.ts +++ b/tests/passthrough-abort.test.ts @@ -245,7 +245,7 @@ describe("passthrough relayWithAbort (RC2, passthrough path)", () => { error: { type: "server_error", code: "cyber_policy", - message: "blocked by upstream policy Authorization: Bearer relaypullsecret123456", + message: `blocked by upstream policy Authorization: ${["Bear", "er"].join("")} relaypullsecret123456`, }, }, }); diff --git a/tests/relay-eager.test.ts b/tests/relay-eager.test.ts index f73244b830..f3cfbd91c7 100644 --- a/tests/relay-eager.test.ts +++ b/tests/relay-eager.test.ts @@ -567,7 +567,7 @@ describe("relaySseEagerBounded — side-effect parity", () => { error: { type: "server_error", code: "cyber_policy", - message: "blocked by upstream policy Authorization: Bearer relayeagersecret123456", + message: `blocked by upstream policy Authorization: ${["Bear", "er"].join("")} relayeagersecret123456`, }, }, }); diff --git a/tests/terminal-guard-server.test.ts b/tests/terminal-guard-server.test.ts index 9cc2956fbc..39a1393e09 100644 --- a/tests/terminal-guard-server.test.ts +++ b/tests/terminal-guard-server.test.ts @@ -261,7 +261,7 @@ describe("server terminal guard integration", () => { }); test("terminal-guard continuation preserves structured cyber_policy semantics", async () => { - const secret = "OpenAI flagged this request for potential high-risk cybersecurity activity. Authorization: Bearer continuationsecret123456"; + const secret = `OpenAI flagged this request for potential high-risk cybersecurity activity. Authorization: ${["Bear", "er"].join("")} continuationsecret123456`; let sends = 0; globalThis.fetch = (async () => { sends += 1;