From 43f1c19fbec47609df3dcb2288cdfc694d04d733 Mon Sep 17 00:00:00 2001 From: JUN Date: Tue, 22 Sep 2026 18:23:10 +0900 Subject: [PATCH 1/8] fix(responses): bound native steering routes and replay memory Native steering and injection accepted requests on routes that were never meant to carry them, and their replay buffers grew without an upper bound, so a long-lived WebSocket exchange could retain an unbounded amount of app-owned memory. Restrict steering to the routes that declare it, cap the replay and body sizes, and release the stores when an exchange settles. Preexisting negative route tests are retained, and the injection docs now cross-link the steering contract. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../content/docs/guides/codex-integration.md | 35 ++-- .../docs/reference/configuration/server.md | 2 +- src/lib/app-owned-memory-stores.ts | 7 + src/lib/app-owned-memory.ts | 15 +- src/server/index/websocket-handler.ts | 24 ++- src/server/responses/codex-ws-exchange.ts | 32 +++- .../responses/native-injection-replay.ts | 14 +- src/server/responses/native-injection.ts | 42 ++++- .../responses/native-response-control.ts | 6 +- .../responses/native-steering-replay.ts | 60 +++++++ src/server/responses/native-steering.ts | 10 +- src/server/responses/ws-upstream.ts | 2 +- structure/transports/streaming-health.md | 20 +-- .../app-owned-memory.test.ts | 60 ++++++- tests/responses/ws-native-injection.test.ts | 53 ++++++- tests/responses/ws-native-steering.test.ts | 150 +++++++++++++++++- .../responses/ws-steering-completion.test.ts | 19 +-- tests/server/memory-watchdog.test.ts | 2 +- 18 files changed, 492 insertions(+), 61 deletions(-) diff --git a/docs-site/src/content/docs/guides/codex-integration.md b/docs-site/src/content/docs/guides/codex-integration.md index 6513400b4a6..8d339309b3e 100644 --- a/docs-site/src/content/docs/guides/codex-integration.md +++ b/docs-site/src/content/docs/guides/codex-integration.md @@ -927,8 +927,7 @@ Do not rewrite an active paginated rollout or thread row to migrate those conver ## Experimental native mid-turn steering -For a compatible model on the canonical ChatGPT forward route or an explicitly configured -[OpenAI API WebSocket route](#steering-continuation-settings-and-public-api), and a client +For a compatible model on the canonical ChatGPT forward route, and a client that sends `response.steer`, enable both options in `~/.opencodex/config.json` and restart OpenCodex before starting a fresh turn: @@ -953,7 +952,7 @@ Do not rerun tools or resend accepted steering text. Model, account, tool declar may change in an explicit saved-result continuation as described below. Other changes require an explicitly stopped or finished turn and normal new dispatch. Multiple independent conversations use independent connections. -HTTP fallback, noncanonical gateways, translated models, sidecars, Combo attempts and plaintext V2 +HTTP fallback, noncanonical gateways, public API-key routes, translated models, sidecars, Combo attempts and plaintext V2 restoration do not support this option. It does not add steering capability to a model or a client that lacks it. Unsupported routes return a protocol error rather than silently ignoring input. Disconnected or timed-out delivery may be unknown: never automatically @@ -984,6 +983,16 @@ During that time the turn holds one physical socket and one pinned credential th because the channel deliberately never re-enters account selection. Treat an enabled steering connection as a long-lived session resource rather than an ordinary bounded request. +Steering frames also share the proxy's configured body and memory limits. A control frame above +[`maxInboundBodyBytes`](/reference/inbound-body-admission/) is refused before +it is parsed on an established control connection (an initial frame is still parsed before +its type-based limit applies), and the reconstructed body sent upstream is refused when it exceeds +[`maxUpstreamBodyBytes`](/reference/configuration/providers/). Each +connection's replay journal is capped at 32 MiB and counted as pinned state against +[`appOwnedMemoryBudgetMb`](/reference/configuration/server/); admitting a +journal demotes evictable caches first rather than failing, and the aggregate across live journals +is capped at 128 MiB regardless of the configured budget. + A timeout means **delivery is unknown**, not that the server rejected the input. Do not resend an accepted instruction or rerun a tool automatically. Inspect the actual task state before deciding how to resume. No account switch or paid API @@ -1064,6 +1073,8 @@ A missing acknowledgement or a disconnect means delivery can be **unknown**. Do not automatically resend a result, restart a tool or change accounts to retry it. The pending queue is limited to 32 frames and 8 MiB, with 1,024 advertised function calls, a 32 MiB replay journal and at most 128 responses per owned connection. +Injection journals share the pinned memory budget and 128 MiB aggregate ceiling +with steering journals; see [steering memory limits](#steering-confirmation-deadlines-and-retained-context). Each sent injection has a 90-second acknowledgement deadline that unrelated output cannot extend; a saved-result wait is limited to 30 minutes. Existing frame limits and stall timeouts still apply. @@ -1112,12 +1123,11 @@ can follow a completed multi-agent turn as a new explicit request using ordinary routing. Client support and backend entitlement still require live verification. -## Steering continuation settings and public API +## Steering continuation settings An explicit saved-result `response.create` may override `reasoning` (effort and summary), `text` (verbosity and supported structured-output format), and -`stream_options`. On an explicitly configured public API route it may also -change `max_output_tokens`. Subscription routes refuse that token-limit override +`stream_options`. Subscription routes refuse a `max_output_tokens` override instead of silently ignoring it. Normal provider pins, subagent caps, effort mapping and summary/verbosity capability exclusions still apply. @@ -1129,14 +1139,11 @@ corrected request can be submitted without rerunning its tool. The server still decides which settings the chosen model accepts. Changes to model, account, provider, tools, instructions or service tier require a separate ordinary turn. -For public API steering, configure an `openai-responses` provider with exactly -`https://api.openai.com/v1`, its API key and `upstreamWebsocket: true`, then use its -normal prefixed model selector with `websockets: true` and -`codexNativeSteering: true`. This does not buy API credit or redirect a ChatGPT -subscription to separately billed usage. A supporting single-agent model/execution -mode is still required. Conversation-bound responses and API automatic compaction -are not steerable; their ordinary responses are preserved and a steering attempt -receives an explanatory error. The multi-agent injection path stays separate. +Native steering is restricted to the canonical ChatGPT subscription route. Public +API-key and gateway routes are not steerable; their ordinary responses are preserved +and a steering attempt receives an explanatory error. This prevents successor +generations on a retained socket from bypassing normal per-request admission. The +separately gated public API multi-agent injection path remains available. ### Executable direct-versus-proxy wire probe diff --git a/docs-site/src/content/docs/reference/configuration/server.md b/docs-site/src/content/docs/reference/configuration/server.md index cf317ae8e0c..86ad7336403 100644 --- a/docs-site/src/content/docs/reference/configuration/server.md +++ b/docs-site/src/content/docs/reference/configuration/server.md @@ -714,6 +714,6 @@ wildcard `hostname`, where the public listener already holds `127.0.0.1:`. `codexNativeSteering` and `codexNativeInjection` enable separate, default-off native WebSocket control paths. See the canonical guide for -[supported steering routes and settings](../../guides/codex-integration.md#steering-continuation-settings-and-public-api), +[supported steering routes and settings](../../guides/codex-integration.md#steering-continuation-settings), [typed result and approval continuations](../../guides/codex-integration.md#rich-tool-results-and-explicit-approvals-after-response-completion), and [confirmation deadlines and retained context](../../guides/codex-integration.md#steering-confirmation-deadlines-and-retained-context). diff --git a/src/lib/app-owned-memory-stores.ts b/src/lib/app-owned-memory-stores.ts index a4c3dbad437..70f7215f901 100644 --- a/src/lib/app-owned-memory-stores.ts +++ b/src/lib/app-owned-memory-stores.ts @@ -54,6 +54,7 @@ import { translatorObservedBufferSnapshot } from "./translator-budget"; import { imageFulfillmentTailSnapshot } from "../images/fulfill"; import { oauthMutationTailSnapshot } from "../oauth/store"; import { grokApplyFlightSnapshot } from "../server/management/agent-settings-routes"; +import { nativeControlReplayRetainedStoreSnapshot } from "../server/responses/native-steering-replay"; function ringSnapshot(metrics: { entries: number; bytes: number; oldestAt: number | null }): RetainedStoreSnapshot { return { @@ -187,6 +188,12 @@ export const APP_OWNED_RETAINED_STORE_REGISTRATIONS = [ snapshot: responseContinuationRetainedStoreSnapshot, evictOldest: evictOldestResponseContinuationForBudget, }, + { + id: "native_control_replay", + category: "continuation", + snapshot: nativeControlReplayRetainedStoreSnapshot, + evictOldest: () => 0, + }, ] as const satisfies readonly RetainedStoreRegistration[]; export function registerDefaultAppOwnedMemoryStores(): void { diff --git a/src/lib/app-owned-memory.ts b/src/lib/app-owned-memory.ts index 2956a05ce40..a1a3a4b9668 100644 --- a/src/lib/app-owned-memory.ts +++ b/src/lib/app-owned-memory.ts @@ -209,14 +209,14 @@ function warnPinnedSaturation(): void { console.warn("[app-owned-memory] retained state remains over budget with no evictable candidate"); } -export function enforceAppOwnedMemoryBudget(): AppOwnedBytesSnapshot { +export function enforceAppOwnedMemoryBudget(reservedPinnedBytes = 0): AppOwnedBytesSnapshot { if (isEnforcing) return appOwnedBytesSnapshot(); isEnforcing = true; enforcementCounters.runs += 1; try { const ineligible = new Set(); const current = retainedSnapshots(); - while (current.total > budgetBytes) { + while (current.total + reservedPinnedBytes > budgetBytes) { const candidate = nextCandidate(current.stores, ineligible); if (!candidate) { enforcementCounters.noEvictableCandidate += 1; @@ -250,6 +250,17 @@ export function enforceAppOwnedMemoryBudget(): AppOwnedBytesSnapshot { } } +/** + * Admission for a pinned allocation the owner cannot demote later. The proposal is + * counted against the shared target BEFORE the normal eviction pass runs, so + * reclaimable logs, caches, blobs and continuations are demoted first and only a + * projected total still above budget — pinned state that cannot fit — is refused. + */ +export function admitAppOwnedPinnedBytes(proposedPinnedBytes: number): boolean { + const snapshot = enforceAppOwnedMemoryBudget(Math.max(0, proposedPinnedBytes)); + return snapshot.retainedBytes + Math.max(0, proposedPinnedBytes) <= snapshot.budgetBytes; +} + export function resetAppOwnedMemoryForTests(): void { retainedStores.clear(); observedBuffers.clear(); diff --git a/src/server/index/websocket-handler.ts b/src/server/index/websocket-handler.ts index 3022e62c4a3..30717dba2fe 100644 --- a/src/server/index/websocket-handler.ts +++ b/src/server/index/websocket-handler.ts @@ -86,6 +86,7 @@ import { } from "../live"; import type { ServeOptionsContext } from "./serve-options"; import type { RequestMetricsRecorder } from "../request-metrics"; +import { resolveInboundBodyLimitBytes } from "../request-decompress"; /** * The WebSocket half of the Bun.serve options, split out of serve-options.ts to keep that file @@ -190,12 +191,31 @@ export function createWebsocketHandler( ws.close(1009, "message too large"); return; } + // An established control connection only ever carries control frames, so the + // inbound body limit applies to raw bytes before the parse materializes them. + if (ws.data.nativeControl && rawBytes > resolveInboundBodyLimitBytes(config.maxInboundBodyBytes)) { + sendJsonFrame(ws, buildWsErrorFrame(413, { + type: "invalid_request_error", + code: "inbound_body_too_large", + message: "Native response control frame exceeds the configured inbound body limit.", + })); + return; + } let frame: Record; try { frame = JSON.parse(typeof raw === "string" ? raw : raw.toString()) as Record; } catch { return; // text-only contract; ignore unparseable frames } + if ((frame.type === "response.inject" || frame.type === "response.steer") + && rawBytes > resolveInboundBodyLimitBytes(config.maxInboundBodyBytes)) { + sendJsonFrame(ws, buildWsErrorFrame(413, { + type: "invalid_request_error", + code: "inbound_body_too_large", + message: "Native response control frame exceeds the configured inbound body limit.", + })); + return; + } if (frame.type === "response.inject" || frame.type === "response.steer" || (frame.type === "response.create" && ws.data.nativeControl)) { try { if (frame.type === "response.inject") { @@ -227,8 +247,8 @@ export function createWebsocketHandler( const idleMs = typeof config.stallTimeoutSec === "number" && Number.isFinite(config.stallTimeoutSec) ? Math.max(1, config.stallTimeoutSec) * 1000 : 300_000; const mode = nativeResponseControlMode(frame, config); - nativeControl = mode === "injection" ? new NativeInjectionChannel(frame, idleMs) - : mode === "steering" ? new NativeSteeringChannel(frame, idleMs) : undefined; + nativeControl = mode === "injection" ? new NativeInjectionChannel(frame, idleMs, config.maxUpstreamBodyBytes) + : mode === "steering" ? new NativeSteeringChannel(frame, idleMs, config.maxUpstreamBodyBytes) : undefined; } catch { sendJsonFrame(ws, buildWsErrorFrame(400, { type: "invalid_request_error", message: "Invalid native steering request settings" })); return; diff --git a/src/server/responses/codex-ws-exchange.ts b/src/server/responses/codex-ws-exchange.ts index d95be865fc8..62129632cea 100644 --- a/src/server/responses/codex-ws-exchange.ts +++ b/src/server/responses/codex-ws-exchange.ts @@ -1,3 +1,4 @@ +import { NativeSteeringError } from "./native-steering"; import { mergeSteeringContinuation } from "./native-steering-settings"; import { markNativeControlResponse } from "./native-response-control"; import type { NativeResponseControl } from "./native-response-control"; @@ -345,11 +346,11 @@ export function codexWsExchange(options: ExchangeOptions): Promise { // the first create frame. let base: Record | undefined; detachSteering = nativeControl.attach(frame => { - const sendControl = () => { - if (terminal || signal?.aborted || session.closed || ws.readyState !== WebSocket.OPEN) { - throw new Error("Native steering connection is no longer available"); - } - beforeDispatch?.(new Headers(headers)); + // Build, serialize and bound-check the reconstructed frame synchronously in the + // channel callback: a typed refusal must reach the channel's synchronous rollback + // (continuation slot released, journal unwritten) rather than the asynchronous + // failStream path, so a corrected continuation can still retry on this channel. + const prepare = () => { let outgoing = frame; if (frame.type === "response.create") { // Generation overrides have passed route policy; identity/tools remain pinned. @@ -361,11 +362,20 @@ export function codexWsExchange(options: ExchangeOptions): Promise { : { ...continuationBase, input: frame.input, previous_response_id: frame.previous_response_id }; } const text = JSON.stringify(outgoing); + nativeControl.assertOutboundFrame?.(text); if (codexWsCreateFrameExceedsLimit(text)) { throw new Error("Native steering frame exceeds the transport byte limit"); } - if (frame.type === "response.create") continuationBase = outgoing; - try { ws.send(text); } catch { + return { outgoing, text }; + }; + const prepared = prepare(); + const sendControl = () => { + if (terminal || signal?.aborted || session.closed || ws.readyState !== WebSocket.OPEN) { + throw new Error("Native steering connection is no longer available"); + } + beforeDispatch?.(new Headers(headers)); + if (frame.type === "response.create") continuationBase = prepared.outgoing; + try { ws.send(prepared.text); } catch { // A send failure has unknown delivery. Never replay or fall back. failStream("Native steering send failed; delivery is unknown"); throw new Error("Native steering send failed; delivery is unknown"); @@ -374,7 +384,13 @@ export function codexWsExchange(options: ExchangeOptions): Promise { if (frame.type === "response.create" && beforeContinuation) { // Explicit tool-result continuations are physical request starts; // they keep provider pacing and revalidate auth AFTER the wait. - void beforeContinuation().then(sendControl).catch(() => failStream("Native steering continuation could not be dispatched; do not automatically replay queued input")); + // A typed pre-send refusal (e.g. the configured upstream body limit) is a + // known non-delivery and keeps its own message; every other failure keeps + // the unknown-delivery wording. + void beforeContinuation().then(sendControl).catch(error => failStream( + error instanceof NativeSteeringError ? error + : new Error("Native steering continuation could not be dispatched; do not automatically replay queued input"), + )); } else sendControl(); }, error => failStream(error)); } diff --git a/src/server/responses/native-injection-replay.ts b/src/server/responses/native-injection-replay.ts index bfce0a35116..6ec84dcc0c8 100644 --- a/src/server/responses/native-injection-replay.ts +++ b/src/server/responses/native-injection-replay.ts @@ -1,4 +1,4 @@ -import { MAX_NATIVE_STEERING_REPLAY_BYTES, type NativeSteeringReplayObserver } from "./native-steering-replay"; +import { MAX_NATIVE_STEERING_REPLAY_BYTES, admitNativeControlReplayJournal, registerNativeControlReplayJournal, type NativeSteeringReplayObserver } from "./native-steering-replay"; import { injectionRecord as record, type InjectionFrame as Frame, type FunctionResult } from "./native-injection-protocol"; import { nativeResultFingerprint } from "./native-tool-results"; @@ -16,6 +16,9 @@ export class NativeInjectionReplay implements NativeSteeringReplayObserver { private acceptedBatchBytes: number[] = []; private explicit: unknown[] = []; private previous: unknown[] = []; + private unregisterAccounting?: () => void; + + get retainedBytes(): number { return this.bytes; } /** Capture a private initial prefix; existing persistence eligibility is checked by the caller. */ constructor(input: unknown, private readonly remember: (input: unknown[], response: Frame) => void) { @@ -23,12 +26,19 @@ export class NativeInjectionReplay implements NativeSteeringReplayObserver { ? [{ type: "message", role: "user", content: [{ type: "input_text", text: input }] }] : Array.isArray(input) ? [...input] : []; this.reserve(this.prefix); + this.unregisterAccounting = registerNativeControlReplayJournal(this); } /** Charge serialized bytes, refusing rather than truncating an over-budget transcript. */ private reserve(value: unknown): number { const bytes = Buffer.byteLength(JSON.stringify(value)); if (this.bytes + bytes > MAX_NATIVE_STEERING_REPLAY_BYTES) throw new Error("Native injection replay exceeded its history budget."); this.bytes += bytes; + try { + admitNativeControlReplayJournal(this); + } catch (error) { + this.bytes -= bytes; + throw error; + } return bytes; } /** Journal before physical send, with rollback usable only for a known unsent frame. */ @@ -99,6 +109,8 @@ export class NativeInjectionReplay implements NativeSteeringReplayObserver { } /** Drop all retained bodies at cancellation, connection teardown or unknown delivery. */ dispose(): void { + this.unregisterAccounting?.(); + this.unregisterAccounting = undefined; this.prefix = []; this.output.clear(); this.accepted.clear(); this.pending = undefined; this.pendingBytes = 0; this.acceptedBatchBytes = []; this.explicit = []; this.previous = []; this.bytes = 0; } diff --git a/src/server/responses/native-injection.ts b/src/server/responses/native-injection.ts index c2f4a1c8117..76e8186df27 100644 --- a/src/server/responses/native-injection.ts +++ b/src/server/responses/native-injection.ts @@ -1,4 +1,6 @@ import { CodexWsCorrelation } from "./codex-ws-correlation"; +import { NativeSteeringError } from "./native-steering"; +import { checkOutboundBodySize } from "./outbound-body-guard"; import type { NativeResponseControl } from "./native-response-control"; import type { NativeSteeringReplayObserver } from "./native-steering-replay"; import { @@ -48,11 +50,18 @@ export class NativeInjectionChannel implements NativeResponseControl { /** Pin the original settings and lane; construction never opens a connection. */ constructor(initial: Frame, private readonly idleMs = 300_000, + private readonly maxUpstreamBodyBytes?: number, private readonly deadlines = { ackMs: NATIVE_INJECTION_ACK_MS, toolMs: NATIVE_INJECTION_TOOL_MS }) { if (!isInjectionRequest(initial)) injectionError("injection_not_supported", "Native injection requires explicit multi_agent.enabled."); this.lane = initial.stream_id ?? undefined; for (const [key, value] of Object.entries(initial)) if (!ENVELOPE.has(key)) this.settings.set(key, injectionFingerprint(value)); } + /** Refuse the exact rebuilt control body before it reaches the retained socket. */ + assertOutboundFrame(text: string): void { + if (!checkOutboundBodySize(text, this.maxUpstreamBodyBytes).admitted) { + injectionError("outbound_body_too_large", "Native injection frame exceeds the configured upstream body limit."); + } + } /** Report that the real dispatch boundary has selected this owner. */ get attached(): boolean { return this.everAttached; } /** A terminal is not final until submitted results have acknowledgements. */ @@ -77,10 +86,10 @@ export class NativeInjectionChannel implements NativeResponseControl { return injectionError("native_control_mode_mismatch", "This multi-agent turn owns an injection-only channel; start a separate turn for steering."); } /** Abort unknown-delivery state without HTTP fallback, resends or invented acceptance. */ - private fail(): void { + private fail(error?: Error): void { this.finished = true; clearTimeout(this.ackTimer); clearTimeout(this.idleTimer); - this.onFailure?.(new Error("Native injection transport failed or timed out; delivery is unknown. Do not automatically resend or rerun tools.")); + this.onFailure?.(error ?? new Error("Native injection transport failed or timed out; delivery is unknown. Do not automatically resend or rerun tools.")); } /** Require the same live owner; an unbound or detached channel cannot authorize a send. */ private live(): void { @@ -135,10 +144,23 @@ export class NativeInjectionChannel implements NativeResponseControl { this.inFlight = submission; this.ackTimer = setTimeout(() => this.fail(), this.deadlines.ackMs); this.ackTimer.unref?.(); + let rollback: (() => void) | undefined; try { - this.replay?.submitted(submission.frame); + rollback = this.replay?.submitted(submission.frame); this.send!(submission.frame); - } catch { + } catch (error) { + if (error instanceof NativeSteeringError) { + // A typed refusal is a known non-delivery: unjournal and free the reservation + // so a corrected result can be queued again on the same channel. + rollback?.(); + clearTimeout(this.ackTimer); this.ackTimer = undefined; + this.inFlight = undefined; this.queue.shift(); this.queueBytes -= submission.bytes; + for (const item of submission.results) { + const call = this.calls.get(nativeResultKey(item)); + if (call?.state === "queued") call.state = "available"; + } + throw error; + } this.fail(); injectionError("injection_delivery_unknown", "Injection dispatch failed; do not automatically resend or rerun tools."); } @@ -163,7 +185,7 @@ export class NativeInjectionChannel implements NativeResponseControl { clearTimeout(this.ackTimer); this.ackTimer = undefined; this.inFlight = undefined; this.queue.shift(); this.queueBytes -= pending.bytes; // Do not let a synchronous fake peer publish the next ack before this event is relayed. - if (this.queue.length) queueMicrotask(() => { try { this.pump(); } catch { this.fail(); } }); + if (this.queue.length) queueMicrotask(() => { try { this.pump(); } catch (error) { this.fail(error instanceof NativeSteeringError ? error : undefined); } }); } /** Commit terminal replay only when no submitted injection can change its accepted inputs. */ private recordTerminal(): void { @@ -197,8 +219,14 @@ export class NativeInjectionChannel implements NativeResponseControl { } if (Buffer.byteLength(JSON.stringify(frame)) > MAX_NATIVE_INJECTION_BYTES) injectionError("invalid_injection", "Native injection continuation exceeds its byte limit."); this.continuationSent = true; - try { this.recordTerminal(); const copy = JSON.parse(JSON.stringify(frame)) as Frame; this.replay?.submitted(copy); this.send(copy); } - catch { this.fail(); injectionError("injection_delivery_unknown", "Continuation delivery is unknown; do not automatically resend results."); } + let undo: (() => void) | undefined; + try { this.recordTerminal(); const copy = JSON.parse(JSON.stringify(frame)) as Frame; undo = this.replay?.submitted(copy); this.send(copy); } + catch (error) { + // A typed refusal is a known non-delivery: unjournal, release the continuation + // slot so a corrected frame can be sent, and keep the code. + if (error instanceof NativeSteeringError) { undo?.(); this.continuationSent = false; throw error; } + this.fail(); injectionError("injection_delivery_unknown", "Continuation delivery is unknown; do not automatically resend results."); + } if (!this.finished) this.armIdle(this.deadlines.ackMs); return true; } diff --git a/src/server/responses/native-response-control.ts b/src/server/responses/native-response-control.ts index afd1558d7f7..c22a661f4a5 100644 --- a/src/server/responses/native-response-control.ts +++ b/src/server/responses/native-response-control.ts @@ -17,6 +17,8 @@ export interface NativeResponseControl { steer(frame: Record): void; inject?(frame: Record): void; continue(frame: Record): boolean; + /** Apply operator-configured limits to the exact reconstructed upstream frame. */ + assertOutboundFrame?(text: string): void; } export const OPENAI_API_RESPONSES_URL = "https://api.openai.com/v1/responses"; @@ -27,10 +29,10 @@ export function markNativeControlResponse(response: Response): Response { native /** Recognize a marked native response by identity, not by caller-controlled content. */ export function isNativeControlResponse(response: Response): boolean { return nativeControlResponses.has(response); } -/** Preserve canonical ChatGPT eligibility; public API controls require an explicit provider WebSocket opt-in. */ +/** Preserve canonical ChatGPT eligibility; only injection may use the separately billed public API. */ export function nativeResponseControlEligible(provider: OcxProviderConfig, control?: NativeResponseControl): boolean { if (isCanonicalOpenAiForwardProvider(provider)) return true; - return (control?.kind === "injection" || control?.kind === "steering") && provider.adapter === "openai-responses" + return control?.kind === "injection" && provider.adapter === "openai-responses" && provider.upstreamWebsocket === true && provider.authMode !== "forward" && provider.baseUrl?.replace(/\/+$/, "") === "https://api.openai.com/v1"; } diff --git a/src/server/responses/native-steering-replay.ts b/src/server/responses/native-steering-replay.ts index 1b6405f3a01..4532b33e101 100644 --- a/src/server/responses/native-steering-replay.ts +++ b/src/server/responses/native-steering-replay.ts @@ -1,4 +1,9 @@ import { nativeResponseOutput } from "./native-response-output"; +import { + admitAppOwnedPinnedBytes, + appOwnedBytesSnapshot, + type RetainedStoreSnapshot, +} from "../../lib/app-owned-memory"; /** * Connection-local replay journal. Only input committed by response.created enters @@ -6,6 +11,46 @@ import { nativeResponseOutput } from "./native-response-output"; * continuation cache. Bodies are bounded and discarded at connection teardown. */ export const MAX_NATIVE_STEERING_REPLAY_BYTES = 32 * 1024 * 1024; +/** + * Aggregate ceiling across every live steering and injection journal. The operator + * budget is an eviction target that can be raised to 4 GiB; pinned control journals + * keep their own finite admission cap so the documented pin-capable aggregate stays + * below the process-owned 512 MiB worst case. + */ +export const MAX_NATIVE_CONTROL_REPLAY_TOTAL_BYTES = 128 * 1024 * 1024; +let aggregateCapBytes = MAX_NATIVE_CONTROL_REPLAY_TOTAL_BYTES; +const activeReplays = new Set<{ readonly retainedBytes: number }>(); + +/** Account active journals as pinned state: protocol safety forbids evicting pending input. */ +export function nativeControlReplayRetainedStoreSnapshot(): RetainedStoreSnapshot { + let bytes = 0; + for (const replay of activeReplays) bytes += replay.retainedBytes; + return { count: activeReplays.size, bytes, evictableBytes: 0, pinnedBytes: bytes, oldestAt: null }; +} + +/** Track one live control journal (steering or injection) and return its detach hook. */ +export function registerNativeControlReplayJournal(journal: { readonly retainedBytes: number }): () => void { + activeReplays.add(journal); + return () => { activeReplays.delete(journal); }; +} + +/** + * Shared pinned-memory admission for one journal's current retained bytes. Bytes the + * retained-store registry already sees are measured in place; a journal still in its + * constructor is priced as a new proposal. Reclaimable owners are demoted before + * refusal, so cache occupancy alone never fails a journal. + */ +export function admitNativeControlReplayJournal(journal: { readonly retainedBytes: number }): void { + const replayBytes = nativeControlReplayRetainedStoreSnapshot().bytes + + (activeReplays.has(journal) ? 0 : journal.retainedBytes); + if (replayBytes > aggregateCapBytes) { + throw new Error("Native control replay exceeded the pinned journal ceiling."); + } + const registeredBytes = appOwnedBytesSnapshot().stores.native_control_replay?.bytes ?? 0; + if (!admitAppOwnedPinnedBytes(replayBytes - registeredBytes)) { + throw new Error("Native control replay exceeded the application-owned memory budget."); + } +} type Frame = Record; /** Accept JSON object envelopes without treating arrays as records. */ function record(value: unknown): value is Frame { @@ -32,16 +77,22 @@ export class NativeSteeringReplay implements NativeSteeringReplayObserver { private submissions: Array<{ parent: string; input: unknown[]; id?: string; bytes: number }> = []; private explicitInput: unknown[] = []; private explicitBytes = 0; + private disposed = false; + private unregisterAccounting?: () => void; + + get retainedBytes(): number { return this.bytes; } /** Capture the initial prefix and reject over-budget history before dispatch. */ constructor(input: unknown, private readonly remember: (input: unknown[], response: Frame) => void) { this.prefix = [...inputItems(input)]; this.bytes = Buffer.byteLength(JSON.stringify(this.prefix)); this.check(); + this.unregisterAccounting = registerNativeControlReplayJournal(this); } /** Reject overflow rather than silently truncating retained conversation input. */ private check(): void { if (this.bytes > MAX_NATIVE_STEERING_REPLAY_BYTES) throw new Error("Native steering replay exceeded its bounded history budget; input was not silently truncated."); + admitNativeControlReplayJournal(this); } /** Reserve replay bytes before send and return a rollback for synchronous failure. */ submitted(frame: Frame): () => void { @@ -116,6 +167,10 @@ export class NativeSteeringReplay implements NativeSteeringReplayObserver { } /** Release retained input, output and queued submissions when the owner detaches. */ dispose(): void { + if (this.disposed) return; + this.disposed = true; + this.unregisterAccounting?.(); + this.unregisterAccounting = undefined; this.prefix = []; this.previousOutput = []; this.submissions = []; @@ -124,3 +179,8 @@ export class NativeSteeringReplay implements NativeSteeringReplayObserver { this.bytes = 0; } } + +/** Test-only: shrink the aggregate pinned-journal ceiling (null restores the documented cap). */ +export function setNativeControlReplayTotalCapForTests(capBytes: number | null): void { + aggregateCapBytes = capBytes ?? MAX_NATIVE_CONTROL_REPLAY_TOTAL_BYTES; +} diff --git a/src/server/responses/native-steering.ts b/src/server/responses/native-steering.ts index b0ef4d68d4c..0652c88aafb 100644 --- a/src/server/responses/native-steering.ts +++ b/src/server/responses/native-steering.ts @@ -3,6 +3,7 @@ import type { NativeSteeringReplayObserver } from "./native-steering-replay"; import { createHash } from "node:crypto"; import { CODEX_WS_ID_MAX_BYTES, CodexWsCorrelation } from "./codex-ws-correlation"; import { codexWsCreateFrameExceedsLimit } from "./codex-ws-wire"; +import { checkOutboundBodySize } from "./outbound-body-guard"; export const MAX_NATIVE_STEERS = 32; export const MAX_NATIVE_STEERING_RESPONSES = 128; @@ -123,7 +124,7 @@ export class NativeSteeringChannel { private advertisedBytes = 0; /** Pin the initial lane and setting digests without opening a transport. */ - constructor(initial: Frame, private readonly idleMs = 300_000) { + constructor(initial: Frame, private readonly idleMs = 300_000, private readonly maxUpstreamBodyBytes?: number) { if (record(initial.multi_agent) && initial.multi_agent.enabled === true) { throw new NativeSteeringError("native_control_mode_mismatch", "Multi-agent responses cannot use the single-agent steering channel."); } @@ -132,6 +133,13 @@ export class NativeSteeringChannel { if (!["type", "input", "previous_response_id", "stream", "stream_id"].includes(key)) this.settings.set(key, fingerprint(value)); } } + + /** Refuse the exact rebuilt control body before it reaches the retained socket. */ + assertOutboundFrame(text: string): void { + if (!checkOutboundBodySize(text, this.maxUpstreamBodyBytes).admitted) { + throw new NativeSteeringError("outbound_body_too_large", "Native steering frame exceeds the configured upstream body limit."); + } + } /** Report whether native dispatch ever bound a physical connection to this owner. */ get attached(): boolean { return this.everAttached; } /** Report whether ordered terminal handling has finished the native chain. */ diff --git a/src/server/responses/ws-upstream.ts b/src/server/responses/ws-upstream.ts index 4348bd48cf9..4bba3ad9a15 100644 --- a/src/server/responses/ws-upstream.ts +++ b/src/server/responses/ws-upstream.ts @@ -138,7 +138,7 @@ export function codexWsUpstreamFetch( // Never infer backend support from a model name or enable controls on a gateway. const control = nativeControl?.kind === "injection" ? ((prepared.canonical || url === OPENAI_API_RESPONSES_URL) && isInjectionRequest(JSON.parse(frameText)) ? nativeControl : undefined) - : (prepared.canonical || url === OPENAI_API_RESPONSES_URL) ? nativeControl : undefined; + : prepared.canonical ? nativeControl : undefined; if (control?.kind === "injection" && url === OPENAI_API_RESPONSES_URL) { const beta = headers["openai-beta"]; if (!beta?.split(",").some(value => value.trim() === "responses_multi_agent=v1")) { diff --git a/structure/transports/streaming-health.md b/structure/transports/streaming-health.md index 9246b1e9695..21234102dd6 100644 --- a/structure/transports/streaming-health.md +++ b/structure/transports/streaming-health.md @@ -297,9 +297,9 @@ Provider-scoped approval reviewer settings are projected by the [catalog owner]( ## Experimental native mid-turn steering `codexNativeSteering: true` is an independent, default-off opt-in for the client-facing -Responses WebSocket endpoint. It requires `websockets: true`, a canonical ChatGPT forward -route or explicitly opted-in canonical OpenAI API route, an eligible Bun runtime, and a -supporting model/execution mode. HTTP fallback and translated/sidecar/Combo paths do not gain steering. Plaintext V2 +Responses WebSocket endpoint. It requires `websockets: true`, the canonical ChatGPT forward +route, an eligible Bun runtime, and a supporting model/execution mode. HTTP fallback and +translated/sidecar/Combo paths do not gain steering. Plaintext V2 restoration is excluded because it is not a transparent native event stream. `src/server/responses/native-steering.ts` owns one downstream turn and one private physical @@ -338,7 +338,9 @@ control events are preserved. A single bounded reader owns delivery; client canc account invalidation and shutdown abort its upstream. Numeric usage is summed once per response; steering control frames (which can contain returned user input) are not log samples. -Bounds: 32 outstanding submissions, 128 response IDs per chain, 32 MiB replay journal, +Bounds: 32 outstanding submissions, 128 response IDs per chain, 32 MiB replay journal +with a 128 MiB aggregate pinned-journal ceiling independent of the configured budget, +the configured inbound and upstream body ceilings, and the shared application-owned memory budget, 256 KiB / 1,024 required-input stubs, existing WS frame/queue byte limits, a 90-second control wait, and a 30-minute saved-tool-result wait. Ordinary active-response silence uses the configured stall deadline. Unsupported routes return explicit errors rather than discarding @@ -475,7 +477,7 @@ Injection retains its existing helper export names and comparison semantics. `native-steering-settings.ts` validates a bounded allowlist for explicit saved-result continuations: `reasoning`, `text` (including structured-output format), -`stream_options` and public-API `max_output_tokens`. Unknown/malformed overrides +`stream_options` and a validated `max_output_tokens` field. Unknown/malformed overrides fail before result reservation. Null resets the supplied setting; omission keeps the current authorized wire value. Models, tools, instructions, account, lane, service tier, execution mode and other settings remain pinned. The schema uses @@ -489,10 +491,10 @@ current wire base, retaining new values across later explicit continuations. Normal pacing and captured account/dispatch guards still run before physical send. No tool results are transformed by generation normalization or rerun on rejection. -Public API steering requires `openai-responses`, key-mode authentication, -`upstreamWebsocket: true` and exactly `https://api.openai.com/v1`. It uses its own -configured API key; subscription traffic is never migrated there. Injection-only -beta metadata is not attached to steering. Initial mode selection explains disabled, +Steering is restricted to the canonical ChatGPT forward route. In particular, an +API-key Responses WebSocket cannot retain a steering channel because its successor +generations do not pass through ordinary per-request send and spend admission. Public +API WebSockets remain available for multi-agent injection. Initial mode selection explains disabled, multi-agent, conversation-bound and automatic-compaction exclusions without breaking ordinary creates or inventing model entitlement. HTTP fallback remains non-steerable. diff --git a/tests/codex-integration/app-owned-memory.test.ts b/tests/codex-integration/app-owned-memory.test.ts index 375dabd36cf..0f50a2aaa41 100644 --- a/tests/codex-integration/app-owned-memory.test.ts +++ b/tests/codex-integration/app-owned-memory.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { + admitAppOwnedPinnedBytes, appOwnedBytesSnapshot, APP_OWNED_WORST_CASE_PINNED_BYTES, configureAppOwnedMemoryBudget, @@ -15,6 +16,7 @@ import { MAX_STORED_RESPONSE_BYTES } from "../../src/responses/state"; import { IMAGE_NORMALIZE_CACHE_MAX_BYTES } from "../../src/adapters/anthropic-image-normalize"; import { VISION_DESCRIPTION_CACHE_MAX_BYTES } from "../../src/vision"; import { ANTIGRAVITY_REPLAY_MAX_TOTAL_BYTES } from "../../src/adapters/google-antigravity-replay"; +import { MAX_NATIVE_CONTROL_REPLAY_TOTAL_BYTES } from "../../src/server/responses/native-steering-replay"; import { clearRequestLogsForTests, evictOldestRequestLogForBudget, @@ -85,7 +87,8 @@ describe("app-owned retained memory", () => { + MAX_STORED_RESPONSE_BYTES + IMAGE_NORMALIZE_CACHE_MAX_BYTES + VISION_DESCRIPTION_CACHE_MAX_BYTES - + ANTIGRAVITY_REPLAY_MAX_TOTAL_BYTES; + + ANTIGRAVITY_REPLAY_MAX_TOTAL_BYTES + + MAX_NATIVE_CONTROL_REPLAY_TOTAL_BYTES; expect(boundedStoreBytes).toBeLessThan(APP_OWNED_WORST_CASE_PINNED_BYTES); }); @@ -350,6 +353,61 @@ describe("app-owned retained memory", () => { expect(snapshot.enforcement.bytesReleased).toBe(18); }); + test("pinned admission demotes reclaimable owners instead of refusing", () => { + const order: string[] = []; + registerRows("logs", "logs", [{ bytes: 4, at: 1 }], order); + configureAppOwnedMemoryBudget(6); + + expect(admitAppOwnedPinnedBytes(4)).toBe(true); + expect(order).toEqual(["logs"]); + expect(appOwnedBytesSnapshot().retainedBytes).toBe(0); + }); + + test("pinned admission refuses only when the projected total still exceeds budget", () => { + const warning = spyOn(console, "warn").mockImplementation(() => {}); + const order: string[] = []; + registerRows("pinned", "blobs", [{ bytes: 6, at: 1, pinned: true }], order); + configureAppOwnedMemoryBudget(4); + + expect(admitAppOwnedPinnedBytes(2)).toBe(false); + expect(order).toEqual([]); + expect(appOwnedBytesSnapshot().retainedBytes).toBe(6); + warning.mockRestore(); + }); + + test("pinned admission inside an enforcing callback never evicts and answers honestly", () => { + const order: string[] = []; + let nested = -1; + registerRows("only", "logs", [{ bytes: 4, at: 1 }], order); + let probeBytes = 8; + registerRetainedStore({ + id: "probe", + category: "caches", + snapshot: () => ({ + count: probeBytes > 0 ? 1 : 0, + bytes: probeBytes, + evictableBytes: probeBytes, + pinnedBytes: 0, + oldestAt: probeBytes > 0 ? 0 : null, + }), + evictOldest: () => { + nested = admitAppOwnedPinnedBytes(1) ? 1 : 0; + order.push("probe"); + const released = probeBytes; + probeBytes = 0; + return released; + }, + }); + configureAppOwnedMemoryBudget(4); + + const snapshot = enforceAppOwnedMemoryBudget(); + + // The reentrant call skipped the eviction loop and measured the real retained total. + expect(nested).toBe(0); + expect(order).toEqual(["only", "probe"]); + expect(snapshot.retainedBytes).toBe(0); + }); + test("translator and serialized-tail observations never invoke budget eviction", () => { const order: string[] = []; registerRows("logs", "logs", [{ bytes: 1, at: 1 }], order); diff --git a/tests/responses/ws-native-injection.test.ts b/tests/responses/ws-native-injection.test.ts index c992cfaf813..f294e0162a0 100644 --- a/tests/responses/ws-native-injection.test.ts +++ b/tests/responses/ws-native-injection.test.ts @@ -209,7 +209,7 @@ test("accepted function results survive ordinary subsequent delta turns; no user function unitChannel(deadlines = { ackMs: 90_000, toolMs: 1_800_000 }) { const sent: Array> = []; const failures: Error[] = []; - const channel = new NativeInjectionChannel({ multi_agent: { enabled: true }, model: "fixture" }, 1000, deadlines); + const channel = new NativeInjectionChannel({ multi_agent: { enabled: true }, model: "fixture" }, 1000, undefined, deadlines); const detach = channel.attach(frame => sent.push(frame), error => failures.push(error)); channel.observe({ type: "response.created", response: { id: "root" } }); const advertise = (call: string, index = 0) => { @@ -231,6 +231,31 @@ test("injection queue counts include the in-flight frame and refuse the next fra } finally { detach(); } }); +test("an oversized paced continuation rolls back instead of failing the stream", async () => { + const settings = injectionConfig(); + settings.maxUpstreamBodyBytes = 4096; + const { socket, send, sent, ws, id } = await beginInjection({}, settings); + const call = advertiseInjection(socket); + completeInjection(socket, { output: [call] }); + await waitForInjection(() => sent.some(event => event.type === "response.completed")); + // The paced path defers dispatch to a microtask; the reconstructed frame must be + // validated before that wait so the refusal reaches the channel's synchronous + // rollback and a corrected continuation can still use this channel. + send(continuationFrame({ type: "response.create", previous_response_id: id, input: [savedResult("call-1", "x".repeat(8192))] })); + expect(sent.at(-1)?.error.code).toBe("outbound_body_too_large"); + expect(socket.frames).toHaveLength(1); + expect(socket.readyState).toBe(1); + expect(ws.data.nativeControl).toBeDefined(); + send(continuationFrame({ type: "response.create", previous_response_id: id, input: [savedResult("call-1", "recovered output")] })); + await waitForInjection(() => socket.frames.length === 2); + expect(socket.frames[1].input).toEqual([savedResult("call-1", "recovered output")]); + socket.emit({ type: "response.created", response: { id: "successor", previous_response_id: id } }); + completeInjection(socket, {}, "successor"); + await waitForInjection(() => !ws.data.nativeControl); + expect(InjectionSocket.all).toHaveLength(1); + expect(fallbackCalls).toBe(0); +}); + test("serialized-byte cap rejects oversized output before a physical send", () => { const { channel, advertise, sent, detach } = unitChannel(); try { @@ -396,3 +421,29 @@ test("HTTP fallback never acquires injection ownership or replays a control fram expect(fallbackCalls).toBe(requests); expect(InjectionSocket.all).toHaveLength(0); expect(ws.data.nativeControl).toBeUndefined(); }); + +test("injection channel refuses an oversized control body at the configured upstream limit", () => { + const channel = new NativeInjectionChannel({ multi_agent: { enabled: true } }, 300_000, 256); + expect(() => channel.assertOutboundFrame(JSON.stringify({ type: "response.create", input: "x".repeat(1024) }))) + .toThrow("configured upstream body limit"); + expect(() => channel.assertOutboundFrame(JSON.stringify({ type: "response.create", input: "x" }))).not.toThrow(); +}); + +test("a configured-size refusal keeps the channel alive and frees the call for a corrected result", () => { + const sent: Array> = []; + const failures: Error[] = []; + const channel = new NativeInjectionChannel({ multi_agent: { enabled: true }, model: "fixture" }, 1000, 256); + const detach = channel.attach(frame => { channel.assertOutboundFrame(JSON.stringify(frame)); sent.push(frame); }, + error => failures.push(error)); + try { + channel.observe({ type: "response.created", response: { id: "root" } }); + const item = { id: "item-c", type: "function_call", call_id: "c", name: "fixture", arguments: "{}" }; + channel.observe({ type: "response.output_item.added", output_index: 0, item }); + channel.observe({ type: "response.output_item.done", output_index: 0, item }); + expect(() => channel.inject({ type: "response.inject", response_id: "root", input: [savedResult("c", "x".repeat(1024))] })) + .toThrow("configured upstream body limit"); + expect(sent).toHaveLength(0); expect(failures).toHaveLength(0); expect(channel.ended).toBe(false); + channel.inject({ type: "response.inject", response_id: "root", input: [savedResult("c", "small")] }); + expect(sent).toHaveLength(1); + } finally { detach(); } +}); diff --git a/tests/responses/ws-native-steering.test.ts b/tests/responses/ws-native-steering.test.ts index dfa89bed85d..3d00d6627a6 100644 --- a/tests/responses/ws-native-steering.test.ts +++ b/tests/responses/ws-native-steering.test.ts @@ -4,13 +4,15 @@ import type { OcxConfig } from "../../src/types"; import { createWebsocketHandler } from "../../src/server/index/websocket-handler"; import type { ServeOptionsContext } from "../../src/server/index/serve-options"; import { NativeSteeringChannel, MAX_NATIVE_STEERS, validateSteeringFrame } from "../../src/server/responses/native-steering"; -import { NativeSteeringReplay, MAX_NATIVE_STEERING_REPLAY_BYTES } from "../../src/server/responses/native-steering-replay"; +import { NativeSteeringReplay, MAX_NATIVE_STEERING_REPLAY_BYTES, nativeControlReplayRetainedStoreSnapshot, setNativeControlReplayTotalCapForTests } from "../../src/server/responses/native-steering-replay"; +import { NativeInjectionReplay } from "../../src/server/responses/native-injection-replay"; import { type WsData } from "../../src/server/ws-bridge"; import { getRequestLogEntries, clearRequestLogsForTests } from "../../src/server/request-log"; import { runOptionalShutdownHooks } from "../../src/lib/optional-shutdown-hooks"; import { MAX_ACTIVE_TURNS, tryAdmitTurn } from "../../src/server/lifecycle"; import { configSchema } from "../../src/config/schema/config-schema"; import { acquireOwnedSpendHome } from "../helpers/owned-spend-home"; +import { appOwnedBytesSnapshot, configureAppOwnedMemoryBudget, registerRetainedStore, resetAppOwnedMemoryForTests } from "../../src/lib/app-owned-memory"; // The websocket handler dispatches through the real request path, so it reaches the shared spend // journal and needs the writer lease startServer would have taken. Without it the turn is refused @@ -283,6 +285,124 @@ test("replay budget refuses overflow instead of silently losing context", () => expect(() => new NativeSteeringReplay("x".repeat(MAX_NATIVE_STEERING_REPLAY_BYTES), () => {})).toThrow("budget"); }); +test("native controls obey configured inbound and reconstructed outbound body limits", () => { + const settings = config(); + settings.maxInboundBodyBytes = 1024 * 1024; + const handler = createWebsocketHandler({ config: settings, deps: {} } as ServeOptionsContext); + const sent: Frame[] = []; + const ws = { readyState: 1, data: { nativeControl: {} }, send: (text: string) => sent.push(JSON.parse(text)) } as unknown as ServerWebSocket; + handler.message(ws, JSON.stringify({ type: "response.steer", previous_response_id: "r", input: "x".repeat(1024 * 1024) })); + expect(sent.at(-1)?.error.code).toBe("inbound_body_too_large"); + + const channel = new NativeSteeringChannel({}, 300_000, 256); + expect(() => channel.assertOutboundFrame(JSON.stringify({ type: "response.create", input: "x".repeat(1024) }))) + .toThrow("configured upstream body limit"); +}); + +test("replay journals share the application-owned memory budget", () => { + resetAppOwnedMemoryForTests(); + registerRetainedStore({ + id: "native_control_replay", + category: "continuation", + snapshot: nativeControlReplayRetainedStoreSnapshot, + evictOldest: () => 0, + }); + let first: NativeSteeringReplay | undefined; + try { + first = new NativeSteeringReplay("x".repeat(200), () => {}); + configureAppOwnedMemoryBudget(appOwnedBytesSnapshot().retainedBytes); + expect(() => new NativeSteeringReplay("y".repeat(200), () => {})).toThrow("application-owned memory budget"); + } finally { + first?.dispose(); + resetAppOwnedMemoryForTests(); + } +}); + +test("injection journals share the same pinned control replay accounting", () => { + resetAppOwnedMemoryForTests(); + registerRetainedStore({ + id: "native_control_replay", + category: "continuation", + snapshot: nativeControlReplayRetainedStoreSnapshot, + evictOldest: () => 0, + }); + let steering: NativeSteeringReplay | undefined; + let injection: NativeInjectionReplay | undefined; + try { + steering = new NativeSteeringReplay("x".repeat(200), () => {}); + injection = new NativeInjectionReplay("y".repeat(200), () => {}); + const shared = nativeControlReplayRetainedStoreSnapshot(); + expect(shared.count).toBe(2); + expect(shared.bytes).toBe(steering.retainedBytes + injection.retainedBytes); + expect(shared.pinnedBytes).toBe(shared.bytes); + injection.dispose(); + expect(nativeControlReplayRetainedStoreSnapshot().bytes).toBe(steering.retainedBytes); + steering.dispose(); + steering = undefined; + configureAppOwnedMemoryBudget(1); + expect(() => new NativeInjectionReplay("z".repeat(200), () => {})).toThrow("application-owned memory budget"); + } finally { + steering?.dispose(); + injection?.dispose(); + resetAppOwnedMemoryForTests(); + } +}); + +test("reclaimable app-owned stores demote to admit a steering journal", () => { + resetAppOwnedMemoryForTests(); + registerRetainedStore({ + id: "native_control_replay", + category: "continuation", + snapshot: nativeControlReplayRetainedStoreSnapshot, + evictOldest: () => 0, + }); + const cacheRows = [{ bytes: 300, at: 1 }]; + registerRetainedStore({ + id: "cache", + category: "caches", + snapshot: () => ({ + count: cacheRows.length, + bytes: cacheRows.reduce((sum, row) => sum + row.bytes, 0), + evictableBytes: cacheRows.reduce((sum, row) => sum + row.bytes, 0), + pinnedBytes: 0, + oldestAt: cacheRows[0]?.at ?? null, + }), + evictOldest: () => cacheRows.splice(0, 1)[0]?.bytes ?? 0, + }); + let replay: NativeSteeringReplay | undefined; + try { + configureAppOwnedMemoryBudget(400); + replay = new NativeSteeringReplay("x".repeat(200), () => {}); + expect(cacheRows).toEqual([]); + expect(nativeControlReplayRetainedStoreSnapshot().bytes).toBeGreaterThan(0); + } finally { + replay?.dispose(); + resetAppOwnedMemoryForTests(); + } +}); + +test("a raised memory budget still caps the aggregate pinned steering journals", () => { + resetAppOwnedMemoryForTests(); + registerRetainedStore({ + id: "native_control_replay", + category: "continuation", + snapshot: nativeControlReplayRetainedStoreSnapshot, + evictOldest: () => 0, + }); + configureAppOwnedMemoryBudget(4096 * 1024 * 1024); + const replays: NativeSteeringReplay[] = []; + try { + replays.push(new NativeSteeringReplay("x".repeat(200), () => {})); + setNativeControlReplayTotalCapForTests(replays[0]!.retainedBytes * 2 + 1); + replays.push(new NativeSteeringReplay("y".repeat(200), () => {})); + expect(() => new NativeSteeringReplay("z".repeat(200), () => {})).toThrow("pinned journal ceiling"); + } finally { + for (const replay of replays) replay.dispose(); + setNativeControlReplayTotalCapForTests(null); + resetAppOwnedMemoryForTests(); + } +}); + test("HTTP upgrade fallback keeps ordinary streaming and rejects steering explicitly", async () => { globalThis.WebSocket = class { constructor() { throw new Error("fixture unavailable upgrade"); } } as unknown as typeof WebSocket; let finish!: () => void; @@ -420,6 +540,34 @@ test("early continuation validates advertised call and approval identities and r detach(); }); +test("an oversized paced continuation rolls back instead of failing the stream", async () => { + const settings = config(); + settings.maxUpstreamBodyBytes = 4096; + const client = downstream({}, settings); + await waitFor(() => client.sent.some(frame => frame.type === "response.created")); + const { ws, send, sent } = client; + const socket = Socket.all.at(-1)!; + const id = socket.root; + send({ type: "response.steer", previous_response_id: id, input: "accepted constraint" }); + accept(socket, id); + complete(socket, id, { output: [{ type: "function_call", call_id: "c", name: "lookup", arguments: "{}" }] }); + // The paced path defers dispatch to a microtask; the reconstructed frame must be + // validated before that wait so the refusal reaches the channel's synchronous + // rollback and a corrected continuation can still use this channel. + send({ type: "response.create", previous_response_id: id, model: "gpt-5.5", input: [{ type: "function_call_output", call_id: "c", output: "x".repeat(8192) }] }); + expect(sent.at(-1)?.error.code).toBe("outbound_body_too_large"); + expect(socket.frames).toHaveLength(2); + expect(socket.readyState).toBe(1); + expect(ws.data.nativeControl).toBeDefined(); + send({ type: "response.create", previous_response_id: id, model: "gpt-5.5", input: [{ type: "function_call_output", call_id: "c", output: "saved" }] }); + await waitFor(() => socket.frames.length === 3); + expect(socket.frames[2].input).toEqual([{ type: "function_call_output", call_id: "c", output: "saved" }]); + socket.emit({ type: "response.created", response: { id: "retry-successor", previous_response_id: id } }); + complete(socket, "retry-successor"); + await waitFor(() => !ws.data.nativeControl); + expect(fallbackCalls).toBe(0); +}); + test("warmup leaves no steering owner and the next ordinary turn gets a fresh channel", async () => { const { ws, sent, send } = downstream({ generate: false }); diff --git a/tests/responses/ws-steering-completion.test.ts b/tests/responses/ws-steering-completion.test.ts index d0dd450c4e6..cdcf53be45d 100644 --- a/tests/responses/ws-steering-completion.test.ts +++ b/tests/responses/ws-steering-completion.test.ts @@ -21,25 +21,22 @@ function pending(socket: InjectionSocket, id: string, number = 1) { reason: "waiting_for_required_input", required_input: [{ type: "function_call_output", call_id: `call-${number}` }] }); } -test("public API steering uses only its explicit API-key route and preserves its beta tokens", async () => { +test("public API steering is ineligible because retained successors bypass per-request admission", async () => { const c = await begin(true); c.send({ type: "response.steer", previous_response_id: c.id, input: "new constraint" }); - expect(c.socket.frames.at(-1)?.type).toBe("response.steer"); + expect(c.sent.at(-1)?.error.code).toBe("steering_not_supported"); expect(c.socket.url).toBe("wss://api.openai.com/v1/responses"); expect(c.socket.options.headers.authorization).toBe("Bearer fixture-public-key"); expect(c.socket.options.headers["chatgpt-account-id"]).toBeUndefined(); expect(c.socket.options.headers["openai-beta"]).toContain("fixture_beta=v1"); expect(c.socket.options.headers["openai-beta"]).not.toContain("responses_multi_agent"); - accept(c.socket, c.id); - c.socket.emit({ type: "response.incomplete", response: { id: c.id, output: [], incomplete_details: { reason: "steered" } } }); - c.socket.emit({ type: "response.created", response: { id: "successor", previous_response_id: c.id } }); - c.socket.emit({ type: "response.completed", response: { id: "successor", status: "completed", output: [] } }); - await waitForInjection(() => !c.ws.data.nativeControl); + c.socket.emit({ type: "response.completed", response: { id: c.id, status: "completed", output: [] } }); expect(InjectionSocket.all).toHaveLength(1); - expect(c.socket.frames).toHaveLength(2); + expect(c.socket.frames).toHaveLength(1); }); -for (const api of [false, true]) test(`explicit settings survive two same-socket continuations (${api ? "API" : "subscription"})`, async () => { +test("explicit settings survive two same-socket subscription continuations", async () => { + const api = false; const c = await begin(api); c.send({ type: "response.steer", previous_response_id: c.id, input: "update" }); accept(c.socket, c.id); pending(c.socket, c.id); @@ -131,6 +128,10 @@ test("queued continuation owns a private copy of both result and settings", () = } finally { detach(); } }); +test("public API eligibility excludes steering even when its WebSocket is explicitly enabled", () => { + expect(nativeResponseControlEligible(config(true).providers.api, new NativeSteeringChannel({}))).toBe(false); +}); + for (const override of [{ upstreamWebsocket: false }, { baseUrl: "https://gateway.example/v1" }, { authMode: "forward" }, { adapter: "openai-chat" }] as Partial[]) { test(`public API eligibility does not widen other routes: ${Object.keys(override)[0]}`, () => { const provider = { ...config(true).providers.api, ...override }; diff --git a/tests/server/memory-watchdog.test.ts b/tests/server/memory-watchdog.test.ts index dc1e0f45c97..573e4ea0a02 100644 --- a/tests/server/memory-watchdog.test.ts +++ b/tests/server/memory-watchdog.test.ts @@ -334,7 +334,7 @@ describe("GET /api/system/memory", () => { }; expect(Object.keys(body.appOwnedBytes.stores).sort()).toEqual([ "antigravity_replay", "claude_debug", "crash_ring", "cursor_blobs", "image_normalize", - "injection_debug", "model_cache", "provider_debug", "request_log", "responses_continuation", + "injection_debug", "model_cache", "native_control_replay", "provider_debug", "request_log", "responses_continuation", "usage_snapshot", "usage_summary", "vision_descriptions", ]); expect(Object.values(body.appOwnedBytes.stores).flatMap(snapshot => Object.values(snapshot)) From 10bf60cea3695a1b63653e7b2508d44311b6ee28 Mon Sep 17 00:00:00 2001 From: JUN Date: Tue, 22 Sep 2026 18:26:37 +0900 Subject: [PATCH 2/8] fix(responses): enforce native injection tool authorization Carry #5470 onto the bounded native-control path. Configure the resolved request-local catalog before attachment, reject undeclared added/done/terminal items before relay, and preserve structured rejection codes. Retain declared rich-result continuations and the existing size and rollback controls. Local checks: NOT RUN by instruction. Hosted CI remains required. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- src/server/responses/native-injection.ts | 30 ++++++++++ .../responses/native-response-control.ts | 2 + src/server/responses/passthrough-dispatch.ts | 7 +++ src/server/ws-bridge.ts | 18 +++++- structure/transports/streaming-health.md | 4 +- tests/responses/ws-native-injection.test.ts | 59 +++++++++++++++++++ .../ws-native-result-continuations.test.ts | 9 ++- 7 files changed, 125 insertions(+), 4 deletions(-) diff --git a/src/server/responses/native-injection.ts b/src/server/responses/native-injection.ts index 76e8186df27..3d3e53599ba 100644 --- a/src/server/responses/native-injection.ts +++ b/src/server/responses/native-injection.ts @@ -3,6 +3,12 @@ import { NativeSteeringError } from "./native-steering"; import { checkOutboundBodySize } from "./outbound-body-guard"; import type { NativeResponseControl } from "./native-response-control"; import type { NativeSteeringReplayObserver } from "./native-steering-replay"; +import { + UNDECLARED_TOOL_CALL_ERROR_CODE, + undeclaredToolCallMessage, + undeclaredToolCallNameInResponse, +} from "../responses-undeclared-tool-guard"; +import type { ProviderExecutedCallType } from "../responses-undeclared-tool-guard"; import { injectionError, injectionFingerprint, injectionId, injectionRecord as record, injectionResults, isInjectionRequest, MAX_NATIVE_INJECTIONS, MAX_NATIVE_INJECTION_BYTES, MAX_NATIVE_INJECTION_CALLS, @@ -46,6 +52,10 @@ export class NativeInjectionChannel implements NativeResponseControl { private ackTimer?: ReturnType; private idleTimer?: ReturnType; private readonly settings = new Map(); + private declaredToolNames?: ReadonlySet; + private declaredBareToolNames: ReadonlySet = new Set(); + private declaredNamelessCallTypes: ReadonlySet = new Set(); + private providerExecutedCallTypes: ReadonlySet = new Set(); private readonly lane: unknown; /** Pin the original settings and lane; construction never opens a connection. */ @@ -67,6 +77,14 @@ export class NativeInjectionChannel implements NativeResponseControl { /** A terminal is not final until submitted results have acknowledgements. */ get ended(): boolean { return this.finished; } + /** Mirror the ordinary response guard for native events that bypass its SSE rewrite. */ + configureToolAuthorization(active: boolean, names: ReadonlySet, bareNames: ReadonlySet, namelessCallTypes: ReadonlySet, providerExecuted: ReadonlySet): void { + this.declaredToolNames = active ? new Set(names) : undefined; + this.declaredBareToolNames = active ? new Set(bareNames) : new Set(); + this.declaredNamelessCallTypes = active ? new Set(namelessCallTypes) : new Set(); + this.providerExecutedCallTypes = active ? new Set(providerExecuted) : new Set(); + } + /** Attach once, after routing/auth/admission, retaining no global response-ID lookup. */ attach(send: (frame: Frame) => void, fail: (error: Error) => void): () => void { if (this.everAttached) throw new Error("Native injection transport is already owned."); @@ -95,8 +113,19 @@ export class NativeInjectionChannel implements NativeResponseControl { private live(): void { if (!this.send || this.finished) injectionError("injection_not_supported", "No live native injection transport is available on this route."); } + private authorize(item: unknown): void { + if (!this.declaredToolNames) return; + const undeclared = undeclaredToolCallNameInResponse( + { output: [item] }, this.declaredToolNames, this.declaredNamelessCallTypes, + this.providerExecutedCallTypes, this.declaredBareToolNames, + ); + if (undeclared !== undefined) { + injectionError(UNDECLARED_TOOL_CALL_ERROR_CODE, undeclaredToolCallMessage(undeclared)); + } + } /** Advertise client-owned function/custom calls and approvals, never hosted execution. */ private advertise(item: unknown): void { + this.authorize(item); const requirement = nativeToolRequirement(item); if (!requirement) return; const old = this.calls.get(requirement.key); @@ -253,6 +282,7 @@ export class NativeInjectionChannel implements NativeResponseControl { this.correlation?.finish(); this.correlation = new CodexWsCorrelation(true, () => false); } else if (!this.currentId || this.terminal) throw new Error("Unexpected native injection event outside an active response."); this.correlation?.accept({ ...event, stream_id: undefined }); + if (type === "response.output_item.added") this.authorize(event.item); if (type === "response.output_item.done") this.advertise(event.item); if (["response.completed", "response.failed", "response.incomplete"].includes(String(type))) { if (!this.currentId || response?.id !== this.currentId) throw new Error("Native injection terminal identity mismatch."); diff --git a/src/server/responses/native-response-control.ts b/src/server/responses/native-response-control.ts index c22a661f4a5..42b926df4d3 100644 --- a/src/server/responses/native-response-control.ts +++ b/src/server/responses/native-response-control.ts @@ -1,6 +1,7 @@ import type { OcxProviderConfig } from "../../types"; import { isCanonicalOpenAiForwardProvider } from "../../providers/openai-tiers"; import type { NativeSteeringReplayObserver } from "./native-steering-replay"; +import type { ProviderExecutedCallType } from "../responses-undeclared-tool-guard"; import { isInjectionRequest } from "./native-injection-protocol"; @@ -10,6 +11,7 @@ export interface NativeResponseControl { relayActive: boolean; normalizeContinuation?: (frame: Record) => Record; replayFactory?: () => NativeSteeringReplayObserver; + configureToolAuthorization?: (active: boolean, names: ReadonlySet, bareNames: ReadonlySet, namelessCallTypes: ReadonlySet, providerExecuted: ReadonlySet) => void; readonly attached: boolean; readonly ended: boolean; attach(send: (frame: Record) => void, fail: (error: Error) => void): () => void; diff --git a/src/server/responses/passthrough-dispatch.ts b/src/server/responses/passthrough-dispatch.ts index 7bd9b64253c..cebc5e9fb9c 100644 --- a/src/server/responses/passthrough-dispatch.ts +++ b/src/server/responses/passthrough-dispatch.ts @@ -480,6 +480,13 @@ export async function preparePassthroughExchange( || clientDeclaredNamelessCallTypes.size > 0 || clientExplicitWireToolCatalog ) && route.provider.authMode !== "forward"; + options.nativeControl?.configureToolAuthorization?.( + undeclaredToolGuardActive, + declaredWireToolNames, + declaredBareWireToolNames, + declaredNamelessClientCallTypes, + providerExecutedCallTypes, + ); }; refreshUndeclaredToolGuard(request); // A refused turn must not seed `previous_response_id` replay. The inspection branch reads the diff --git a/src/server/ws-bridge.ts b/src/server/ws-bridge.ts index e6782bc2355..4a5c28c567d 100644 --- a/src/server/ws-bridge.ts +++ b/src/server/ws-bridge.ts @@ -228,6 +228,22 @@ function sendProtocolError(ws: ServerWebSocket, status: number, message: sendJsonFrame(ws, buildWsErrorFrame(status, protocolError(message))); } +/** + * Report an upstream-pump failure to the client. Errors that carry a structured + * code (for example the undeclared-tool guard's undeclared_tool_call) keep it so + * clients see the same rejection identity as the SSE path; everything else stays + * a generic protocol error. + */ +function sendUpstreamError(ws: ServerWebSocket, status: number, err: unknown): void { + const code = err != null && typeof (err as { code?: unknown }).code === "string" + ? (err as { code: string }).code + : undefined; + const message = err instanceof Error ? err.message : String(err); + sendJsonFrame(ws, buildWsErrorFrame(status, code + ? { type: "upstream_error", code, message } + : protocolError(message))); +} + export async function pumpResponsesSseToWebSocket( ws: ServerWebSocket, sseStream: ReadableStream, @@ -319,7 +335,7 @@ export async function pumpResponsesSseToWebSocket( && !(err instanceof WsSendDroppedError)) { reportTerminal("incomplete"); try { - sendProtocolError(ws, 502, err instanceof Error ? err.message : String(err)); + sendUpstreamError(ws, 502, err); } catch (sendErr) { // If delivery is already dropped, there is no useful error frame left // to send. Swallow only that expected transport signal; other failures diff --git a/structure/transports/streaming-health.md b/structure/transports/streaming-health.md index 21234102dd6..578e779e2f4 100644 --- a/structure/transports/streaming-health.md +++ b/structure/transports/streaming-health.md @@ -375,7 +375,9 @@ only string-valued developer `function_call_output` items for completed calls advertised by that response and lane. IDs are never global lookup keys. One physical injection awaits acknowledgement at a time because success carries a response ID, not an injection ID; further submissions remain in a bounded FIFO. Repeated call -results, mismatched/repeated acknowledgements and unsupported shapes fail closed. +results, mismatched/repeated acknowledgements and unsupported shapes fail closed. On +non-forward routes, native events also enforce the current request's explicit tool +catalog before advertising or relaying a client-executed call. A response terminal is relayed immediately, but pending acknowledgements and unreturned advertised calls retain the socket. Late tool results still reach that diff --git a/tests/responses/ws-native-injection.test.ts b/tests/responses/ws-native-injection.test.ts index f294e0162a0..368b72cbdaf 100644 --- a/tests/responses/ws-native-injection.test.ts +++ b/tests/responses/ws-native-injection.test.ts @@ -44,6 +44,65 @@ test.each([false, true])("real handler sends saved results over the same connect expect(getRequestLogEntries().at(-1)?.usage).toMatchObject({ inputTokens: 10, outputTokens: 5 }); }); +test("public API native injection rejects a function omitted from the request catalog", async () => { + const { socket, sent, ws } = await beginInjection({}, injectionConfig(true)); + socket.emit({ + type: "response.output_item.added", + output_index: 0, + item: { id: "item-omitted", type: "function_call", call_id: "call-omitted", name: "dangerous_local_tool", arguments: "{}" }, + }); + await waitForInjection(() => !ws.data.nativeControl); + expect(sent.some(event => event.type === "response.output_item.added")).toBe(false); + expect(sent.some(event => event.type === "error")).toBe(true); + expect(socket.readyState).toBe(3); +}); + +test("public API native injection rejects an undeclared call that only appears in the terminal snapshot", async () => { + const { socket, sent, ws } = await beginInjection({}, injectionConfig(true)); + completeInjection(socket, { + output: [ + { id: "item-late", type: "function_call", call_id: "call-late", name: "dangerous_local_tool", arguments: "{}" }, + ], + }); + await waitForInjection(() => !ws.data.nativeControl); + expect(sent.some(event => event.type === "response.completed")).toBe(false); + expect(sent.some(event => event.type === "error")).toBe(true); + expect(JSON.stringify(sent)).toContain("undeclared_tool_call"); + expect(socket.readyState).toBe(3); +}); + +test("public API native injection rejects an undeclared call arriving only in output_item.done", async () => { + const { socket, sent, ws } = await beginInjection({}, injectionConfig(true)); + // Establish the item as declared so the added event passes the guard, then let + // the done frame swap in an undeclared name for the same call. + socket.emit({ + type: "response.output_item.added", + output_index: 0, + item: { id: "item-done", type: "function_call", call_id: "call-done", name: "get_value", arguments: "{}" }, + }); + socket.emit({ + type: "response.output_item.done", + output_index: 0, + item: { id: "item-done", type: "function_call", call_id: "call-done", name: "dangerous_local_tool", arguments: "{}" }, + }); + await waitForInjection(() => !ws.data.nativeControl); + expect(sent.some(event => event.type === "response.output_item.done")).toBe(false); + expect(sent.some(event => event.type === "error")).toBe(true); + expect(JSON.stringify(sent)).toContain("undeclared_tool_call"); + expect(socket.readyState).toBe(3); +}); + +test("public API native injection forwards a declared function call on the guarded path", async () => { + const { socket, sent } = await beginInjection({}, injectionConfig(true)); + socket.emit({ + type: "response.output_item.added", + output_index: 0, + item: { id: "item-ok", type: "function_call", call_id: "call-ok", name: "get_value", arguments: "{}" }, + }); + await waitForInjection(() => sent.some(event => event.type === "response.output_item.added")); + expect(sent.some(event => event.type === "error")).toBe(false); +}); + test("terminal before acknowledgement is relayed without dropping the late successful acknowledgement", async () => { const { socket, send, sent, ws, id } = await beginInjection(); const call = advertiseInjection(socket); diff --git a/tests/responses/ws-native-result-continuations.test.ts b/tests/responses/ws-native-result-continuations.test.ts index 9073ce06301..955c5bcb0d4 100644 --- a/tests/responses/ws-native-result-continuations.test.ts +++ b/tests/responses/ws-native-result-continuations.test.ts @@ -81,14 +81,19 @@ test("semantic comparison ignores object-key order but retains content-array ord }); test.each([false, true])("rich/custom/approval continuation uses one original socket; API=%s", async api => { - const { socket, send, sent, ws, id } = await beginInjection({}, injectionConfig(api)); + // The catalog authorizes by wire name; a function spec keeps the adapter wire shape verbatim. + const tools = [ + { type: "function", name: "get_value", parameters: { type: "object", properties: {} } }, + { type: "function", name: "custom", parameters: { type: "object", properties: {} } }, + ]; + const { socket, send, sent, ws, id } = await beginInjection({ tools }, injectionConfig(api)); const func = advertiseInjection(socket); const custom = customCall(); const approval = approvalCall(); emitItem(socket, custom, 1); emitItem(socket, approval, 2); completeInjection(socket, { output: [func, custom, approval] }); await waitForInjection(() => sent.some(frame => frame.type === "response.completed")); expect(ws.data.nativeControl).toBeDefined(); - const frame = continuationFrame({ type: "response.create", previous_response_id: id, + const frame = continuationFrame({ type: "response.create", previous_response_id: id, tools, input: [savedResult("call-1", "text"), customResult(), approvalResult(false)] }, api); send(frame); await waitForInjection(() => socket.frames.length === 2); From d61ec2e6037caa8195c00fef543e70de32516b9d Mon Sep 17 00:00:00 2001 From: JUN Date: Tue, 22 Sep 2026 18:27:02 +0900 Subject: [PATCH 3/8] fix(subagents): recognize Responses Lite plaintext V2 catalogs Carry #5492. Recognize the first developer additional_tools catalog only when top-level tools is absent. Preserve opt-in, canonical destination, explicit catalog precedence, conflict checks and response restoration. Include unit and server-boundary regressions and clarify the Chinese catalog wording. Local checks: NOT RUN by instruction. Hosted CI remains required. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- .../docs/reference/configuration/agents.md | 5 +++- .../zh-cn/reference/configuration/agents.md | 6 +++-- src/responses/plaintext-v2-agent-messages.ts | 11 +++++++- structure/subagents.md | 5 +++- .../plaintext-v2-agent-messages.test.ts | 22 ++++++++++++++++ ...plaintext-v2-agent-messages-server.test.ts | 25 +++++++++++++++++++ 6 files changed, 69 insertions(+), 5 deletions(-) diff --git a/docs-site/src/content/docs/reference/configuration/agents.md b/docs-site/src/content/docs/reference/configuration/agents.md index 0fd021ad65c..ed5c6110825 100644 --- a/docs-site/src/content/docs/reference/configuration/agents.md +++ b/docs-site/src/content/docs/reference/configuration/agents.md @@ -173,7 +173,10 @@ gateways, routes whose final destination is another provider, and non-Responses rewritten. For an eligible v2 request, opencodex recognizes the catalog by a top-level `collaboration` -namespace with a direct `spawn_agent` child. It removes +namespace with a direct `spawn_agent` child. The catalog can be in top-level `tools`, or in the +first input item's developer `additional_tools` when `tools` is absent (Responses Lite). +An explicit top-level catalog takes precedence; user-role and later historical catalogs do not +activate the option. It removes `parameters.properties.message.encrypted: true`, when present, only from `spawn_agent`, `send_message`, and `followup_task`. ChatGPT reserves both the `collaboration` namespace and those three tool names, so the request uses fixed private aliases for all four identities. Before making diff --git a/docs-site/src/content/docs/zh-cn/reference/configuration/agents.md b/docs-site/src/content/docs/zh-cn/reference/configuration/agents.md index dacc763c03e..f3440c0e148 100644 --- a/docs-site/src/content/docs/zh-cn/reference/configuration/agents.md +++ b/docs-site/src/content/docs/zh-cn/reference/configuration/agents.md @@ -88,8 +88,10 @@ opencodex 会跳过已禁用、不可路由、不健康、处于冷却中，或 `authMode: "forward"` 和准确的基础地址 `https://chatgpt.com/backend-api/codex`。OpenAI API key provider、自定义 OpenAI 兼容网关、最终发往其他 provider 的请求，以及非 Responses 调用都不会被改写。 -对于符合条件的 v2 请求，opencodex 只识别顶层 `collaboration` namespace，而且它必须直接包含 -`spawn_agent`。原生 ChatGPT 收到请求前，opencodex 会删除 `spawn_agent`、`send_message` 和 +对于符合条件的 v2 请求，opencodex 只识别工具目录顶层的 `collaboration` namespace，而且它必须直接包含 +`spawn_agent`。目录可以位于顶层 `tools`；如果该字段不存在，也可以位于首个输入项的 developer +`additional_tools` 中（Responses Lite）。显式顶层目录优先，user 角色和后续历史目录不会启用转换。 +原生 ChatGPT 收到请求前，opencodex 会删除 `spawn_agent`、`send_message` 和 `followup_task` 中已有的 `parameters.properties.message.encrypted: true`。ChatGPT 会按保留的 `collaboration` namespace 和三个工具名处理消息，因此请求会给这四个名称使用固定的临时别名。 修改前，opencodex 会检查顶层和 `additional_tools` 工具目录、嵌套 namespace、 diff --git a/src/responses/plaintext-v2-agent-messages.ts b/src/responses/plaintext-v2-agent-messages.ts index d1b7fd12dc2..deeb6a0d90e 100644 --- a/src/responses/plaintext-v2-agent-messages.ts +++ b/src/responses/plaintext-v2-agent-messages.ts @@ -94,7 +94,16 @@ function collaborationCatalogInfo(catalogs: readonly unknown[][]): { export function hasPlaintextV2CollaborationCatalog(body: unknown): boolean { if (!isPlainObject(body)) return false; - return Array.isArray(body.tools) && collaborationCatalogInfo([body.tools]).hasV2Catalog; + if (Array.isArray(body.tools)) return collaborationCatalogInfo([body.tools]).hasV2Catalog; + // Responses Lite carries its default catalog as the first developer input item. + // An explicit top-level catalog wins; later historical catalogs are not defaults. + if (body.tools !== undefined || !Array.isArray(body.input)) return false; + const initial = body.input[0]; + return isPlainObject(initial) + && initial.type === "additional_tools" + && initial.role === "developer" + && Array.isArray(initial.tools) + && collaborationCatalogInfo([initial.tools]).hasV2Catalog; } function hasOptimizedNamespaceConflict(catalogs: readonly unknown[][]): boolean { diff --git a/structure/subagents.md b/structure/subagents.md index cf52d556baf..c21b3c0960d 100644 --- a/structure/subagents.md +++ b/structure/subagents.md @@ -19,7 +19,10 @@ CLI installation inspection reason codes, including Windows deferral, follow the `src/responses/plaintext-v2-agent-messages.ts` owns the experimental, configuration-only `plaintextV2AgentMessages` request compiler and response restoration. The default is unset; only explicit true on Responses ingress to the final canonical ChatGPT forward route activates it. -A default top-level collaboration catalog is required. The compiler preserves caller objects, +A default collaboration catalog is required: top-level `tools`, or, when that field is absent, +the first input item's developer `additional_tools` catalog used by Responses Lite. Explicit +top-level catalogs take precedence; user-role and later historical catalogs do not opt in. +The compiler preserves caller objects, aliases the namespace and three message functions, and removes only their true encryption marker. Declaration/reference collisions refuse the whole rewrite without changing the request. diff --git a/tests/responses/plaintext-v2-agent-messages.test.ts b/tests/responses/plaintext-v2-agent-messages.test.ts index 525d23defab..cfcc4638ae5 100644 --- a/tests/responses/plaintext-v2-agent-messages.test.ts +++ b/tests/responses/plaintext-v2-agent-messages.test.ts @@ -729,6 +729,28 @@ describe("plaintext v2 agent message response restoration", () => { }); describe("plaintext v2 agent message route policy", () => { + test("accepts the initial developer additional_tools catalog used by Codex Responses Lite", () => { + const tools = [{ type: "namespace", name: "collaboration", tools: [collaborationTool("spawn_agent"), collaborationTool("followup_task")] }]; + const initial = { type: "additional_tools", role: "developer", tools }; + const body = { input: [initial, { type: "message", role: "user", content: "delegate" }] }; + const before = structuredClone(body); + expect(shouldPreparePlaintextV2AgentMessages({ enabled: true, inboundWire: "responses", canonicalChatGpt: true, requestBody: body })).toBe(true); + const prepared = preparePlaintextV2AgentMessages(body); + expect(prepared.namespaceAliased).toBe(true); + const result = prepared.body as { tools?: unknown; input: Array }; + expect(result.tools).toBeUndefined(); + expect(result.input[0]!.tools[0]!.name).toBe(PLAINTEXT_V2_COLLABORATION_NAMESPACE); + expect(result.input[0]!.tools[0]!.tools[0]!.name).toBe("start_delegated_task"); + expect(result.input[0]!.tools[0]!.tools[1]!.name).toBe("continue_delegated_task"); + expect(body).toEqual(before); + for (const rejected of [ + { ...body, tools: [] }, + { ...body, tools: null }, + { input: [{ ...initial, role: "user" }] }, + { input: [{ type: "message", role: "developer", content: "history" }, initial] }, + ]) expect(preparePlaintextV2AgentMessages(rejected).namespaceAliased).toBe(false); + }); + test("requires an explicit opt-in, Responses inbound, canonical ChatGPT, and a v2 catalog", () => { const requestBody = { tools: [{ type: "namespace", name: "collaboration", tools: [collaborationTool("spawn_agent")] }] }; const baseline = { diff --git a/tests/server/plaintext-v2-agent-messages-server.test.ts b/tests/server/plaintext-v2-agent-messages-server.test.ts index 46829c129fd..62522121f85 100644 --- a/tests/server/plaintext-v2-agent-messages-server.test.ts +++ b/tests/server/plaintext-v2-agent-messages-server.test.ts @@ -221,6 +221,31 @@ describe("plaintext v2 agent messages at the Responses server boundary", () => { expect(clientBody).toContain('"encrypted_function_args":[]'); }); + test("rewrites a Responses Lite default catalog and restores its delegated call", async () => { + takeInheritedSpendHome(); + let sent: Record | undefined; + globalThis.fetch = (async (_input: unknown, init?: RequestInit) => { + sent = JSON.parse(String(init?.body)); + return new Response(JSON.stringify(completedResponsePayload()), { + status: 200, headers: { "content-type": "application/json" }, + }); + }) as typeof fetch; + const { tools, ...body } = await collaborationRequest().json() as Record; + body.input.unshift({ type: "additional_tools", role: "developer", tools }); + const request = new Request("http://localhost/v1/responses", { + method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify(body), + }); + const response = await handleResponses(request, config(true), { model: "", provider: "" }); + expect(response.status).toBe(200); + expect(sent?.tools).toBeUndefined(); + expect(sent?.input[0].tools[0].name).toBe(PLAINTEXT_V2_COLLABORATION_NAMESPACE); + expect(sent?.input[0].tools[0].tools[0].parameters.properties.message.encrypted).toBeUndefined(); + const result = await response.json() as { output: Array> }; + expect(result.output[0]!.namespace).toBe("collaboration"); + expect(result.output[0]!.name).toBe("spawn_agent"); + expect(result.output[0]!.encrypted_function_args).toEqual([]); + }); + test("restores the namespace in bounded JSON responses", async () => { takeInheritedSpendHome(); globalThis.fetch = (async () => new Response(JSON.stringify(completedResponsePayload()), { From e555e7305b0358bbd97589e4b2cd2529a2d6f10d Mon Sep 17 00:00:00 2001 From: kosta Date: Sat, 19 Sep 2026 20:06:21 -0400 Subject: [PATCH 4/8] fix(responses): normalize wrapped MCP tool names (cherry picked from commit b4c839b8f1f8ee0ea77897b945a3a745be3306de) (cherry picked from commit f68b32646069d3e385748fc6c36848765c68e02e) --- src/types/tools.ts | 10 +++- .../decisions/ADR-0097-responses-http-sse.md | 12 +++++ structure/transports/responses.md | 8 ++++ ...s-default-namespace-emit-normalize.test.ts | 46 +++++++++++++++++++ 4 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 structure/decisions/ADR-0097-responses-http-sse.md diff --git a/src/types/tools.ts b/src/types/tools.ts index 70d777d7491..c0c7b54a63c 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -129,6 +129,8 @@ export const NAMESPACED_BARE_ALIAS_EXCLUDED_NAMES: ReadonlySet = new Set * * Rewrites invented `default.` prefixes back to a declared bare tool when that bare tool * is declared and neither `default.` nor `default__` was explicitly declared (#4176). + * The same wrapper may surround an already-flattened namespace identity; accept that exact + * declared suffix without treating its child name as a bare declaration. * Also normalizes legacy helper names (`exec_command`, `shell_command`, `apply_patch`, `view_image`) to * `exec` when code-mode `exec` is declared in the request catalog. * @@ -151,7 +153,13 @@ export function normalizeDeclaredToolName( const bareDeclared = declaredBare ?? declared; if ( bare.length > 0 - && bareDeclared.has(bare) + && ( + bareDeclared.has(bare) + // Muse can wrap the complete `namespace__tool` identity in `default.`. Requiring the + // exact flattened identity to be declared preserves the #4176 provenance boundary: + // `default.tool` still cannot borrow a namespaced tool's manufactured bare alias. + || (bare.includes("__") && declared.has(bare)) + ) && !declared.has("default." + bare) && !declared.has("default__" + bare) ) { diff --git a/structure/decisions/ADR-0097-responses-http-sse.md b/structure/decisions/ADR-0097-responses-http-sse.md new file mode 100644 index 00000000000..ac9688b76ab --- /dev/null +++ b/structure/decisions/ADR-0097-responses-http-sse.md @@ -0,0 +1,12 @@ +# ADR-0097 — decision recorded under "Responses HTTP/SSE" + +- Contract owner: [transports/responses.md](../transports/responses.md#responses-httpsse) + +## Decision record + +- 목적과 의도: Restore a Muse callback that wraps a request-declared flattened namespace identity in an invented `default.` prefix without weakening the undeclared-tool boundary. +- 기존 구현 및 제약 조건: Default-namespace normalization accepted genuine bare declarations and bounded code-mode helpers, but intentionally rejected a namespaced tool's child name; the missing case carried the complete canonical `namespace__tool` identity after the prefix. +- 검토한 주요 대안: Strip every `default.` prefix; authorize any unique bare alias; special-case Codex App or Muse model names; require the complete suffix to be a declared flattened identity. +- 선택한 방식: Strip the wrapper only when the suffix contains `__`, is present verbatim in the current declared-name set, and no explicit default-namespace identity owns the emitted spelling. +- 다른 대안 대신 이 방식을 선택한 이유: Exact current-turn membership repairs the provider formatting error while preserving rejection for namespace-dropping guesses, unknown names, pruned tools, and explicitly declared default identities. +- 장점, 단점 및 영향: Streaming and buffered Responses paths emit the canonical client identity and continue the turn; providers inventing a different wrapper syntax still fail closed until measured and reviewed. diff --git a/structure/transports/responses.md b/structure/transports/responses.md index a82e8f8cb08..257be9f4b00 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -618,6 +618,14 @@ declared bare tool and to rewrite code-mode helper names into the declared `exec input unchanged when the set is absent, so the set reaches the bridge on every wire and enforcement is expressed by a separate flag rather than by withholding it. +Muse may also wrap an already-flattened namespace identity, for example +`default.mcp__server__tool`. That form resolves only when the complete suffix is an exact declared +name containing the flattened `__` delimiter and neither explicit `default.` nor `default__` +identity exists. It does not let `default.tool` borrow a namespaced tool's manufactured bare alias, +and an unknown suffix still reaches the undeclared-tool failure. + +> Decision record: [ADR-0097](../decisions/ADR-0097-responses-http-sse.md) + The passthrough guard resolves an emitted name through that same `normalizeDeclaredToolName`, so whatever it admits it must also EMIT under the resolved name. The two halves disagreed once: `normalizeDefaultNamespaceInItem` implemented only the bare-tool case (#4176), so a diff --git a/tests/responses/responses-default-namespace-emit-normalize.test.ts b/tests/responses/responses-default-namespace-emit-normalize.test.ts index 5b7952468b9..cdf036badaa 100644 --- a/tests/responses/responses-default-namespace-emit-normalize.test.ts +++ b/tests/responses/responses-default-namespace-emit-normalize.test.ts @@ -40,6 +40,15 @@ const CLASSIC_BODY = { ], } as const; +/** Codex App MCP tool shape from the Muse callback failure: namespace plus child function. */ +const CODEX_APP_BODY = { + tools: [{ + type: "namespace", + name: "mcp__codex_app", + tools: [{ type: "function", name: "send_message_to_thread", parameters: { type: "object" } }], + }], +} as const; + function declarationsOf(body: unknown): { declared: ReadonlySet; declaredBare: ReadonlySet; @@ -110,6 +119,28 @@ describe("default-namespaced helper names under a code-mode catalog", () => { }); }); +describe("default wrapper around a declared flattened namespace identity", () => { + const canonical = "mcp__codex_app__send_message_to_thread"; + const wrapped = `default.${canonical}`; + + test("the exact Muse callback name normalizes to the declared canonical identity", () => { + const item = { type: "function_call", call_id: "c1", name: wrapped, arguments: "{}" }; + expect(normalizedNames(CODEX_APP_BODY, item)).toEqual([canonical]); + expect(guardVerdict(CODEX_APP_BODY, item)).toBeUndefined(); + }); + + test("a namespace-dropping guess and an unknown suffix stay rejected", () => { + for (const name of [ + "default.send_message_to_thread", + "default.mcp__codex_app__delete_everything", + ]) { + const item = { type: "function_call", call_id: "c1", name, arguments: "{}" }; + expect(normalizedNames(CODEX_APP_BODY, item)).toEqual([name]); + expect(guardVerdict(CODEX_APP_BODY, item)).toBe(name); + } + }); +}); + describe("names the emit boundary must not touch", () => { test("a canonical declared name passes through byte-identical", () => { const item = { type: "function_call", call_id: "c1", name: "view_image", arguments: "{}" }; @@ -199,6 +230,21 @@ describe("the streaming boundary the report actually crossed", () => { expect(emitted[0]).not.toContain("default.view_image"); }); + test("the streamed Muse callback keeps its declared namespace identity", () => { + const { declared, declaredBare } = declarationsOf(CODEX_APP_BODY); + const rewrite = createUndeclaredToolCallGuardBlockRewrite(declared, undefined, undefined, declaredBare); + const emitted = blocks(rewrite, [{ + type: "function_call", + id: "fc_1", + call_id: "c1", + name: "default.mcp__codex_app__send_message_to_thread", + arguments: "{}", + }]); + expect(emitted).toHaveLength(1); + expect(emitted[0]).toContain('"name":"mcp__codex_app__send_message_to_thread"'); + expect(emitted[0]).not.toContain("default.mcp__codex_app"); + }); + test("an unresolvable dotted name ends the turn instead of reaching the client", () => { const { declared, declaredBare } = declarationsOf(CODE_MODE_BODY); const rewrite = createUndeclaredToolCallGuardBlockRewrite(declared, undefined, undefined, declaredBare); From 7cbbf44f6caaf522ca852eaff2e9c731f993819f Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Mon, 21 Sep 2026 09:36:01 +0900 Subject: [PATCH 5/8] fix(responses): unwrap default apply patch aliases (cherry picked from commit 453df76c585853f090a514a12778477befca467e) (cherry picked from commit d63ff542415895fa25f5599aad9dc632c80de92d) --- src/responses/code-mode-helper-compat.ts | 6 +++++- .../responses-code-mode-patch-compile.test.ts | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/responses/code-mode-helper-compat.ts b/src/responses/code-mode-helper-compat.ts index d390bb76cb4..0af986fe212 100644 --- a/src/responses/code-mode-helper-compat.ts +++ b/src/responses/code-mode-helper-compat.ts @@ -43,8 +43,12 @@ export function compileCodeModeHelperInput( // is an apply_patch wrapper and is not an `exec` fallback field, and the recognizer already // declines it under `exec`; reading it here would compile a body that recognition rejected, // which is exactly the drift a second, looser unwrap introduces. + const bodyToolName = wireToolName ?? helperName; + const normalizedBodyToolName = bodyToolName.startsWith("default.") + ? bodyToolName.slice("default.".length) + : bodyToolName; const patch = normalizeApplyPatchDelimiters( - unwrapFreeformToolInput(argumentsText, wireToolName ?? helperName), + unwrapFreeformToolInput(argumentsText, normalizedBodyToolName), ); return `const result = await tools.apply_patch(${JSON.stringify(patch)});\ntext(result);`; } diff --git a/tests/responses/responses-code-mode-patch-compile.test.ts b/tests/responses/responses-code-mode-patch-compile.test.ts index aa12e7c63fd..2f3b174628b 100644 --- a/tests/responses/responses-code-mode-patch-compile.test.ts +++ b/tests/responses/responses-code-mode-patch-compile.test.ts @@ -52,6 +52,16 @@ describe("code-mode apply_patch compiles the body recognition accepted", () => { expect(compileAsBridge(JSON.stringify({ patch: PATCH }))).toBeUndefined(); }); + test("a default.apply_patch alias keeps the native apply_patch vocabulary", () => { + for (const key of ["patch", "content"]) { + expect(compileCodeModeHelperInput( + JSON.stringify({ [key]: PATCH }), + "default.apply_patch", + "default.apply_patch", + )).toBe(EXPECTED); + } + }); + test("a normal code-mode JavaScript body is left alone", () => { for (const body of [ 'const result = await tools.exec_command({ cmd: "ls" });\ntext(result);', From 19a2005e418f039cbaca14ab3a38623c14b3a833 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 21 Sep 2026 23:48:44 +0000 Subject: [PATCH 6/8] docs(structure): record default.-prefixed alias recovery in the freeform contract The responses owners still claimed only bare exec/apply_patch calls accept alternate-field or outer-fence recovery; compileCodeModeHelperInput now also unwraps provider-invented default. aliases. Update runtime.md, transports/responses.md, providers/{chat-compat,kiro,xai-grok}.md. Co-Authored-By: Epinephrine (cherry picked from commit 165ddd0f283b5c06725426ba475c0a86e66ce8be) (cherry picked from commit d0776e703681729e2ea447775e77397faab16f29) --- structure/providers/chat-compat.md | 2 +- structure/providers/kiro.md | 2 +- structure/providers/xai-grok.md | 2 +- structure/runtime.md | 3 ++- structure/transports/responses.md | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/structure/providers/chat-compat.md b/structure/providers/chat-compat.md index 1c7465bde6e..38d11b18232 100644 --- a/structure/providers/chat-compat.md +++ b/structure/providers/chat-compat.md @@ -52,7 +52,7 @@ compatibility layer. Its endpoint profile and privacy boundary are specified in Chat models sometimes return a freeform call body under a common alternate field or wrap the whole body in a Markdown fence. Restoration in `src/responses/apply-patch-envelope.ts` is deliberately -narrow: only bare `exec` and `apply_patch` accept one recognized alternate field or one complete +narrow: only bare or `default.`-prefixed `exec` and `apply_patch` accept one recognized alternate field or one complete outer fence, while ambiguous wrappers and provider-owned freeform grammars remain byte-exact. Kiro groups only consecutive original-message tool results whose raw call ID exactly matches diff --git a/structure/providers/kiro.md b/structure/providers/kiro.md index b1d0b503d55..1fb17b688a2 100644 --- a/structure/providers/kiro.md +++ b/structure/providers/kiro.md @@ -26,7 +26,7 @@ reserves the private completion tool. Meta Muse 64-character MCP aliases live in Kiro shares the Responses freeform restoration boundary in `src/responses/apply-patch-envelope.ts`: contractual `input` wrappers are unwrapped, while alternate -field and outer-fence recovery is limited to unambiguous bare `exec` and `apply_patch` bodies. +field and outer-fence recovery is limited to unambiguous bare or `default.`-prefixed `exec` and `apply_patch` bodies. Kiro refuses structured output and tolerates every other Responses `text` member. `text.format` of type `json_schema` or `json_object` is a contract the CodeWhisperer wire cannot honour, so the diff --git a/structure/providers/xai-grok.md b/structure/providers/xai-grok.md index 4dfa27723f0..fcef52b4dc8 100644 --- a/structure/providers/xai-grok.md +++ b/structure/providers/xai-grok.md @@ -28,7 +28,7 @@ Shared parsing and streaming follow the [request-copy](../transports/byte-accoun Grok's Responses path shares `src/responses/apply-patch-envelope.ts` for freeform restoration. The declared `input` field remains authoritative; alternate-field and outer-fence recovery is -limited to unambiguous bare `exec` and `apply_patch` calls and does not rewrite foreign grammars. +limited to unambiguous bare or `default.`-prefixed `exec` and `apply_patch` calls and does not rewrite foreign grammars. Grounded in the open-sourced official client (xai-org/grok-build); unit + evidence: `devlog/_fin/260716_grok_build_hardening/`. diff --git a/structure/runtime.md b/structure/runtime.md index 26f5b2a43f8..942533258f4 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -336,7 +336,8 @@ On `error` / incomplete / stall / EOF — and when assembled non-freeform tool a an open tool call is cancelled as `status: "incomplete"` without `function_call_arguments.done`, so the client never sees a completed call ahead of `response.failed` / `response.incomplete`. At the freeform boundary, `src/responses/apply-patch-envelope.ts` unwraps the contractual `input` -field for every tool. Only bare `exec` and `apply_patch` calls may recover one recognized alternate +field for every tool. Only bare or `default.`-prefixed `exec` and `apply_patch` calls may recover +one recognized alternate body field or remove one complete outer Markdown fence; ambiguous alternate fields and every other freeform grammar pass through unchanged. diff --git a/structure/transports/responses.md b/structure/transports/responses.md index 257be9f4b00..09203e2fbf2 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -121,7 +121,7 @@ discarded to manufacture a bare name. Function-call wrappers around freeform bodies are restored by `src/responses/apply-patch-envelope.ts`. The declared `input` field is authoritative. For bare -`exec` and `apply_patch`, one tool-specific alternate field or one complete outer Markdown fence +`exec` and `apply_patch` (including their `default.`-prefixed provider aliases), one tool-specific alternate field or one complete outer Markdown fence is recoverable because the wrapper is otherwise unusable; two alternate fields are ambiguous and therefore remain untouched. Foreign freeform grammars never receive that compatibility rewrite. From 96625281951967527c430ba23432d703ae10a697 Mon Sep 17 00:00:00 2001 From: Flowershangfromthebranches <152056395+Flowershangfromthebranches@users.noreply.github.com> Date: Sun, 20 Sep 2026 22:23:33 +0800 Subject: [PATCH 7/8] fix(responses): compile structured shell payloads sent to code-mode exec (cherry picked from commit 9430bbdad79ed3a6dfc2ce40a8114b888466aa92) (cherry picked from commit 59bc70bc30f954eea5a63f8a6fcf4de4dcbfd865) --- .../content/docs/guides/codex-integration.md | 8 + .../docs/zh-cn/guides/codex-integration.md | 6 + scripts/test-layout/layout.json | 1 + src/bridge/sse.ts | 2 + src/responses/code-mode-helper-compat.ts | 12 +- src/responses/code-mode-shell-input.ts | 54 +++++++ src/server/responses-custom-tool-repair.ts | 2 + structure/providers/chat-compat.md | 3 + structure/transports/responses.md | 10 ++ tests/fixtures/test-layout-expected.json | 1 + .../responses-code-mode-shell-compile.test.ts | 150 ++++++++++++++++++ 11 files changed, 246 insertions(+), 3 deletions(-) create mode 100644 src/responses/code-mode-shell-input.ts create mode 100644 tests/responses/responses-code-mode-shell-compile.test.ts diff --git a/docs-site/src/content/docs/guides/codex-integration.md b/docs-site/src/content/docs/guides/codex-integration.md index 8d339309b3e..388eac2fdbd 100644 --- a/docs-site/src/content/docs/guides/codex-integration.md +++ b/docs-site/src/content/docs/guides/codex-integration.md @@ -586,6 +586,14 @@ Codex. Native custom calls and converted function calls use the same completion patch previews are held while their executable form is unresolved. JavaScript that merely contains patch text and unrelated native custom payloads stay unchanged. +A routed model can also mistakenly send a shell-argument object such as +`{"cmd":"git status --short"}` to code-mode `exec`. For a verified code-mode catalog, +opencodex converts an unambiguous shell object into `tools.exec_command(...)` JavaScript +and forwards its output through `text(...)`. Shell options are preserved, and Codex still +executes and authorizes the command. Valid JavaScript fallback fields, ambiguous objects, +and unrelated tool namespaces are not converted. This compatibility repair does not bypass +provider rate limits or change the configured retry policy. + Routed code-mode turns are also told the host's rules for the nested helpers before the first call: `tools.apply_patch` takes one string that opens and closes with the bare patch marker lines, the isolate has no `import`, and long-running commands are polled through `write_stdin`. When a diff --git a/docs-site/src/content/docs/zh-cn/guides/codex-integration.md b/docs-site/src/content/docs/zh-cn/guides/codex-integration.md index 94a00079d97..69060abdabe 100644 --- a/docs-site/src/content/docs/zh-cn/guides/codex-integration.md +++ b/docs-site/src/content/docs/zh-cn/guides/codex-integration.md @@ -202,6 +202,12 @@ Codex 显示的模型来自一个磁盘上的 catalog（默认是 `$CODEX_HOME/o 结果仍包含宿主的某条失败消息，opencodex 会追加一行提示，指出对应规则。此变更不会重写模型的 代码或补丁文本。 +如果路由模型误把 `{"cmd":"git status --short"}` 这样的 shell 参数对象传给 code-mode `exec`， +opencodex 会在确认工具目录为 code mode 且内容无歧义时，将它转换为调用 +`tools.exec_command(...)` 并通过 `text(...)` 返回结果的 JavaScript。shell 选项会保留， +命令执行与权限检查仍由 Codex 处理。合法的 JavaScript 后备字段、歧义对象和其他工具命名空间 +不会被转换；这项兼容修复不会绕过提供方限流，也不改变配置的重试策略。 + 所选 provider 必须支持 function/tool calling。不支持 tool call 的 text-only provider 无法使用 `exec`、 Browser 或 Computer Use。原生 OpenAI 条目会保持其上游 tool mode 不变。 diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 7117b4679ea..65dae05a414 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1619,6 +1619,7 @@ "devin-stated-reset-retry.test.ts": "providers", "ci-structure-gate.test.ts": "ci-workflows", "responses-code-mode-patch-compile.test.ts": "responses", + "responses-code-mode-shell-compile.test.ts": "responses", "gui-codex-usage-score-parity.test.ts": "gui", "gui-tray-vibrancy-surface.test.ts": "gui", "web-search-sidecar-429.test.ts": "web-search", diff --git a/src/bridge/sse.ts b/src/bridge/sse.ts index a1f5a0fb8a4..99e7c5ba129 100644 --- a/src/bridge/sse.ts +++ b/src/bridge/sse.ts @@ -24,6 +24,7 @@ import { import { progressiveFreeformInput } from "../responses/progressive-freeform-input"; import { encodeCompactionSummary } from "../responses/compaction"; import { compileCodeModeHelperInput, resolveCodeModeHelperName } from "../responses/code-mode-helper-compat"; +import { mayBecomeCodeModeShellInput } from "../responses/code-mode-shell-input"; import { isTruncatedStopReason, truncationReasonFor } from "../responses/truncated-stop-reason"; import { encodeReasoningEnvelope, type ReasoningEnvelope } from "../responses/reasoning-envelope"; import { rememberReasoningForCall } from "../responses/reasoning-replay-cache"; @@ -1089,6 +1090,7 @@ export function bridgeToResponsesSSE( // replaced by the normalized ones. const mayNormalize = ownsFreeformGrammar && currentToolCall.name === "apply_patch"; if (!((mayCompile || mayNormalize) && mayBecomePatchEnvelope(full)) + && !(mayCompile && mayBecomeCodeModeShellInput(currentToolCall.args, full)) && full.startsWith(emitted) && full.length > emitted.length) { emit("response.custom_tool_call_input.delta", { item_id: currentToolCall.itemId, output_index: currentToolCall.outputIndex, diff --git a/src/responses/code-mode-helper-compat.ts b/src/responses/code-mode-helper-compat.ts index 0af986fe212..e103590c4a0 100644 --- a/src/responses/code-mode-helper-compat.ts +++ b/src/responses/code-mode-helper-compat.ts @@ -4,6 +4,7 @@ import { unwrapFreeformToolInput, } from "./apply-patch-envelope"; import { declaresCodeModeExec } from "../types/tools"; +import { parseCodeModeShellInput } from "./code-mode-shell-input"; function isPlainObject(value: unknown): value is Record { return !!value && typeof value === "object" && !Array.isArray(value); @@ -31,6 +32,10 @@ export function compileCodeModeHelperInput( const helperName = toolName.startsWith("default.") ? toolName.slice("default.".length) : toolName; + if (helperName === "exec_command" && wireToolName === "exec") { + const args = parseCodeModeShellInput(argumentsText); + if (args) return `const result = await tools.exec_command(${JSON.stringify(args)});\ntext(result);`; + } if (helperName === "apply_patch") { // `resolveCodeModeHelperName` decides this IS an apply-patch call by reading // `unwrapFreeformToolInput(argumentsText, wireToolName)`, which strips an outer Markdown @@ -100,8 +105,8 @@ export function compileCodeModeHelperInput( * wrong. * * This adds that second case: the name is already `exec` so nothing was rewritten, but - * the body is a complete patch envelope and therefore cannot be the JavaScript that - * `exec` runs. Same inference the name-based path makes, drawn from the payload. + * the body is a complete patch envelope or an unambiguous structured shell call. + * Same inference the name-based path makes, drawn from the payload. * * Returns undefined for everything else, including JavaScript that merely mentions a * patch envelope — that body is a real program and is forwarded byte-identical. @@ -120,5 +125,6 @@ export function resolveCodeModeHelperName( // `tools.apply_patch(...)` JavaScript would be the mis-route this repair exists to avoid. if (!declaresCodeModeExec(declaredNames)) return undefined; if (typeof argumentsText !== "string" || argumentsText === "") return undefined; - return isCompletePatchEnvelope(unwrapFreeformToolInput(argumentsText, "exec")) ? "apply_patch" : undefined; + if (isCompletePatchEnvelope(unwrapFreeformToolInput(argumentsText, "exec"))) return "apply_patch"; + return parseCodeModeShellInput(argumentsText) ? "exec_command" : undefined; } diff --git a/src/responses/code-mode-shell-input.ts b/src/responses/code-mode-shell-input.ts new file mode 100644 index 00000000000..79e83225ca4 --- /dev/null +++ b/src/responses/code-mode-shell-input.ts @@ -0,0 +1,54 @@ +import { unwrapFreeformToolInput } from "./apply-patch-envelope"; +import { scanFreeformWrapper } from "./freeform-wrapper-scan"; + +const SHELL_ARGUMENT_KEYS = new Set([ + "cmd", "command", "workdir", "shell", "login", "tty", "yield_time_ms", + "max_output_tokens", "sandbox_permissions", "justification", "prefix_rule", +]); +let javascriptParser: Bun.Transpiler | undefined; + +/** Recognize shell arguments, never guess a shell from an ordinary freeform program. */ +export function parseCodeModeShellInput(argumentsText: string): Record | undefined { + let parsed: unknown; + try { + // Only the canonical input wrapper is removed: the cmd/command object is the payload. + parsed = JSON.parse(unwrapFreeformToolInput(argumentsText)); + } catch { + return undefined; + } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return undefined; + const args = parsed as Record; + if (Object.keys(args).some(key => !SHELL_ARGUMENT_KEYS.has(key))) return undefined; + const keys = ["cmd", "command"].filter(key => Object.hasOwn(args, key)); + if (keys.length !== 1) return undefined; + const command = args[keys[0]!]; + if (typeof command !== "string" || command.trim() === "") return undefined; + // cmd/command also exist as historical JavaScript fallback fields. Preserve every valid + // program, including ambiguous identifiers such as `ls`. Parsing never executes the source. + try { + javascriptParser ??= new Bun.Transpiler({ loader: "js" }); + javascriptParser.scan(`async function __codeModeInput() {\n${command}\n}`); + return undefined; + } catch { + const { command: _alias, ...rest } = args; + return { ...rest, cmd: command }; + } +} + +/** Hold possible shell objects until completion can choose their executable representation. */ +export function mayBecomeCodeModeShellInput(argumentsText: string, input: string): boolean { + const head = input.trimStart(); + if (head === "" || head.startsWith("{")) return true; + // Canonical JavaScript streams progressively; avoid reparsing its growing wrapper on every + // delta. The shared prefix scanner is bounded independently of the command's size. + if (input === argumentsText || scanFreeformWrapper(argumentsText).kind === "input") return false; + try { + const args = JSON.parse(argumentsText); + // A fallback cmd value becomes visible only when the outer object closes. Do not emit + // that command before completion replaces it with tools.exec_command JavaScript. + return !!args && typeof args === "object" && !Object.hasOwn(args, "input") + && (Object.hasOwn(args, "cmd") || Object.hasOwn(args, "command")); + } catch { + return false; + } +} diff --git a/src/server/responses-custom-tool-repair.ts b/src/server/responses-custom-tool-repair.ts index 3b4130ae7f9..b0ba7f637ab 100644 --- a/src/server/responses-custom-tool-repair.ts +++ b/src/server/responses-custom-tool-repair.ts @@ -1,6 +1,7 @@ import type { TranslatorBudget } from "../lib/translator-budget"; import { mayBecomePatchEnvelope, normalizeApplyPatchDelimiters } from "../responses/apply-patch-envelope"; import { compileCodeModeHelperInput, resolveCodeModeHelperName } from "../responses/code-mode-helper-compat"; +import { mayBecomeCodeModeShellInput } from "../responses/code-mode-shell-input"; import { progressiveFreeformInput } from "../responses/progressive-freeform-input"; import { declaresCodeModeExec } from "../types/tools"; import { @@ -320,6 +321,7 @@ export function createRoutedCustomToolRestoreBlockRewrite( && itemName?.name === "exec"; const mayNormalize = ownsFreeformGrammar && itemName?.name === "apply_patch"; if ((mayCompile || mayNormalize) && mayBecomePatchEnvelope(fullInput)) return []; + if (mayCompile && mayBecomeCodeModeShellInput(open.argumentsText, fullInput)) return []; if (!fullInput.startsWith(open.emittedInput) || fullInput.length === open.emittedInput.length) return []; const inputDelta = fullInput.slice(open.emittedInput.length); open.emittedInput = fullInput; diff --git a/structure/providers/chat-compat.md b/structure/providers/chat-compat.md index 38d11b18232..2817b3dfbf4 100644 --- a/structure/providers/chat-compat.md +++ b/structure/providers/chat-compat.md @@ -54,6 +54,9 @@ Chat models sometimes return a freeform call body under a common alternate field body in a Markdown fence. Restoration in `src/responses/apply-patch-envelope.ts` is deliberately narrow: only bare or `default.`-prefixed `exec` and `apply_patch` accept one recognized alternate field or one complete outer fence, while ambiguous wrappers and provider-owned freeform grammars remain byte-exact. +Structured shell arguments mistakenly sent to code-mode `exec` follow the shared +[Responses restoration contract](../transports/responses.md#responses-httpsse), including preview +holding and preservation of valid JavaScript fallback fields. Kiro groups only consecutive original-message tool results whose raw call ID exactly matches the originating call. Its wire-ID map retains the original ID privately so replacement or diff --git a/structure/transports/responses.md b/structure/transports/responses.md index 09203e2fbf2..22cf579af50 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -125,6 +125,16 @@ Function-call wrappers around freeform bodies are restored by is recoverable because the wrapper is otherwise unusable; two alternate fields are ambiguous and therefore remain untouched. Foreign freeform grammars never receive that compatibility rewrite. +For a verified code-mode catalog, `src/responses/code-mode-shell-input.ts` recognizes a +structured `cmd` or `command` object submitted under `exec` and the canonical `input` wrapper. +Only known shell options and one command field are accepted, and any command that parses as +JavaScript remains unchanged, including ambiguous single identifiers. The existing helper +compiler serializes the recognized arguments into `tools.exec_command(...)` and emits its result +through `text(...)`; the proxy executes nothing. JSON, native Responses and adapter-event SSE +use the same completion rule. Possible shell-object previews stay held until completion so raw +JSON or shell text cannot precede the compiled JavaScript. Ordinary JavaScript stays progressive. +`tests/responses/responses-code-mode-shell-compile.test.ts` covers those paths and boundaries. + #### Schema-bound flat shell repair Completed Responses function calls have one separate schema-bound flat-shell repair. When the diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 1fd592857c0..b420de1acad 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1452,6 +1452,7 @@ "devin-stated-reset-retry.test.ts": "providers", "ci-structure-gate.test.ts": "ci-workflows", "responses-code-mode-patch-compile.test.ts": "responses", + "responses-code-mode-shell-compile.test.ts": "responses", "gui-codex-usage-score-parity.test.ts": "gui", "gui-tray-vibrancy-surface.test.ts": "gui", "web-search-sidecar-429.test.ts": "web-search", diff --git a/tests/responses/responses-code-mode-shell-compile.test.ts b/tests/responses/responses-code-mode-shell-compile.test.ts new file mode 100644 index 00000000000..5c5ced36510 --- /dev/null +++ b/tests/responses/responses-code-mode-shell-compile.test.ts @@ -0,0 +1,150 @@ +import { describe, expect, test } from "bun:test"; +import { compileCodeModeHelperInput, resolveCodeModeHelperName } from "../../src/responses/code-mode-helper-compat"; +import { restoreRoutedCustomCallsInJson } from "../../src/responses/custom-tool-compat"; +import { bridgeToResponsesSSE, buildResponseJSON } from "../../src/bridge"; +import type { AdapterEvent } from "../../src/types"; +import { createRoutedCustomToolRestoreBlockRewrite } from "../../src/server/responses-custom-tool-repair"; +import { dataPayload, frame } from "../helpers/custom-tool-repair-fixtures"; + +const CODE_MODE = new Set(["exec"]); +const COMMAND = 'cd "/tmp/example repo" && git status --short'; + +describe("structured shell arguments submitted to code-mode exec", () => { + test("compiles the observed cmd object without losing shell options", async () => { + const args = { cmd: COMMAND, workdir: "/tmp", yield_time_ms: 1000, max_output_tokens: 2000 }; + const body = JSON.stringify(args); + expect(resolveCodeModeHelperName(undefined, "exec", body, undefined, CODE_MODE)).toBe("exec_command"); + const restored = JSON.parse(restoreRoutedCustomCallsInJson(JSON.stringify({ + output: [{ type: "function_call", id: "fc_shell", call_id: "call_shell", name: "exec", arguments: body }], + }), CODE_MODE, new Set(), CODE_MODE)); + const item = restored.output[0]; + expect(item).toMatchObject({ type: "custom_tool_call", name: "exec", call_id: "call_shell" }); + const calls: unknown[] = []; + const outputs: unknown[] = []; + const run = new Function("tools", "text", `return (async () => { ${item.input} })();`); + await run({ exec_command: async (value: unknown) => { calls.push(value); return { output: "ok" }; } }, + (value: unknown) => outputs.push(value)); + expect(calls).toEqual([args]); + expect(outputs).toEqual([{ output: "ok" }]); + }); + + test("the canonical input wrapper and command alias use the same shell payload", () => { + for (const args of [{ cmd: COMMAND }, { command: COMMAND }]) { + for (const body of [JSON.stringify(args), JSON.stringify({ input: JSON.stringify(args) })]) { + const helper = resolveCodeModeHelperName(undefined, "exec", body, undefined, CODE_MODE); + expect(helper).toBe("exec_command"); + expect(compileCodeModeHelperInput(body, helper!, "exec")) + .toBe(`const result = await tools.exec_command(${JSON.stringify({ cmd: COMMAND })});\ntext(result);`); + } + } + }); + + test("leaves JavaScript, ambiguous objects and unrelated catalogs alone", () => { + for (const body of [ + 'text("hello")', + JSON.stringify({ cmd: 'await tools.exec_command({ cmd: "pwd" });' }), + JSON.stringify({ command: 'text("hello")' }), + JSON.stringify({ cmd: "ls" }), // Also a valid JavaScript identifier: do not guess. + JSON.stringify({ input: "text(1)", cmd: COMMAND }), + JSON.stringify({ cmd: COMMAND, code: "text(1)" }), + JSON.stringify({ cmd: COMMAND, command: "echo other" }), + JSON.stringify({ cmd: COMMAND, unknownOption: true }), + JSON.stringify({ cmd: 42 }), + JSON.stringify([{ cmd: COMMAND }]), + '{"cmd":', + ]) expect(resolveCodeModeHelperName(undefined, "exec", body, undefined, CODE_MODE)).toBeUndefined(); + for (const declared of [undefined, new Set(["exec", "shell_command"]), new Set(["mcp__exec"])]) { + expect(resolveCodeModeHelperName(undefined, "exec", JSON.stringify({ cmd: COMMAND }), undefined, declared)).toBeUndefined(); + } + expect(resolveCodeModeHelperName(undefined, "exec", JSON.stringify({ cmd: COMMAND }), "mcp", CODE_MODE)).toBeUndefined(); + }); + + test("shell metacharacters remain data passed to the nested tool", async () => { + const args = { cmd: 'printf "%s" "`id` $(whoami)"\n# ${text("not source")}', tty: false }; + const body = JSON.stringify(args); + const helper = resolveCodeModeHelperName(undefined, "exec", body, undefined, CODE_MODE); + expect(helper).toBe("exec_command"); + const calls: unknown[] = []; + const run = new Function("tools", "text", `return (async () => { ${compileCodeModeHelperInput(body, helper!, "exec")} })();`); + await run({ exec_command: async (value: unknown) => { calls.push(value); return "ok"; } }, () => {}); + expect(calls).toEqual([args]); + }); + + test("Chat adapter JSON and fragmented SSE deliver the same executable call", async () => { + const args = { cmd: COMMAND, workdir: "/tmp" }; + const expected = `const result = await tools.exec_command(${JSON.stringify(args)});\ntext(result);`; + for (const body of [JSON.stringify(args), JSON.stringify({ input: JSON.stringify(args) })]) { + async function* events(): AsyncGenerator { + yield { type: "tool_call_start", id: "call-shell", name: "exec" }; + for (const arguments_ of body) yield { type: "tool_call_delta", id: "call-shell", arguments: arguments_ }; + yield { type: "tool_call_end", id: "call-shell" }; + yield { type: "done" }; + } + const options = { declaredToolNames: CODE_MODE }; + const collected: AdapterEvent[] = []; + for await (const event of events()) collected.push(event); + const json = buildResponseJSON(collected, "fixture", { ...options, freeformToolNames: CODE_MODE }); + expect(json.output).toMatchObject([{ type: "custom_tool_call", name: "exec", input: expected }]); + const stream = bridgeToResponsesSSE(events(), "fixture", undefined, CODE_MODE, undefined, undefined, 50_000, options); + const text = await new Response(stream).text(); + const payloads = text.split(/\r?\n\r?\n/).filter(block => block.includes("data: {")).map(dataPayload); + const preview = payloads.filter(p => p.type === "response.custom_tool_call_input.delta").map(p => p.delta).join(""); + expect(expected.startsWith(preview)).toBe(true); + expect(payloads.find(p => p.type === "response.custom_tool_call_input.done")?.input).toBe(expected); + expect(payloads.find(p => p.type === "response.output_item.done")?.item).toMatchObject({ input: expected }); + expect(payloads.find(p => p.type === "response.completed")?.response).toMatchObject({ output: [{ input: expected }] }); + } + }); + + test("native and lowered Responses streams agree at every split boundary", () => { + const args = { cmd: COMMAND }; + const expected = `const result = await tools.exec_command(${JSON.stringify(args)});\ntext(result);`; + for (const native of [false, true]) { + for (const body of [JSON.stringify(args), JSON.stringify({ input: JSON.stringify(args) })]) { + for (let split = 0; split <= body.length; split++) { + const rewrite = createRoutedCustomToolRestoreBlockRewrite(CODE_MODE, undefined, new Set(), CODE_MODE); + const type = native ? "custom_tool_call" : "function_call"; + const field = native ? "input" : "arguments"; + const event = native ? "response.custom_tool_call_input" : "response.function_call_arguments"; + const item = { type, id: "fc_shell", call_id: "call_shell", name: "exec", [field]: body }; + try { + rewrite(frame("response.output_item.added", { output_index: 0, item: { ...item, [field]: "" } })); + let preview = ""; + for (const delta of [body.slice(0, split), body.slice(split)]) { + preview += rewrite(frame(`${event}.delta`, { output_index: 0, item_id: "fc_shell", delta })) + .map(block => dataPayload(block).delta ?? "").join(""); + } + expect(preview).toBe(""); + const done = rewrite(frame(`${event}.done`, { output_index: 0, item_id: "fc_shell", [field]: body })); + expect(dataPayload(done[0]!).input).toBe(expected); + const itemDone = rewrite(frame("response.output_item.done", { output_index: 0, item })); + expect(dataPayload(itemDone[0]!).item).toMatchObject({ input: expected, call_id: "call_shell" }); + const terminal = rewrite(frame("response.completed", { response: { output: [item] } })); + expect(dataPayload(terminal[0]!).response).toMatchObject({ output: [{ input: expected }] }); + } finally { + rewrite.dispose?.(); + } + } + } + } + }); + + test("canonical JavaScript retains progressive output under a code-mode catalog", () => { + const rewrite = createRoutedCustomToolRestoreBlockRewrite(CODE_MODE, undefined, new Set(), CODE_MODE); + try { + rewrite(frame("response.output_item.added", { + output_index: 0, + item: { type: "function_call", id: "fc_js", call_id: "call_js", name: "exec", arguments: "" }, + })); + let preview = ""; + for (const delta of ['{"input":"text(', '1)', '"}']) { + preview += rewrite(frame("response.function_call_arguments.delta", { item_id: "fc_js", delta })) + .map(block => dataPayload(block).delta ?? "").join(""); + expect(preview.length).toBeGreaterThan(0); + } + expect(preview).toBe("text(1)"); + } finally { + rewrite.dispose?.(); + } + }); +}); From b57d7c5da03bc7493dafad77ef4c3c050e2633ae Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Tue, 22 Sep 2026 20:51:42 +0900 Subject: [PATCH 8/8] test(responses): verify combined tool normalization boundaries Verify that default.apply_patch names and patch/content wrappers emit the same executable input through JSON and fragmented SSE. Move the carried decision record to unused ADR-0099 because current dev already owns ADR-0097; preserve both the structured code-mode shell and existing schema-bound flat-shell contracts. Co-authored-by: kosta Co-authored-by: Flowershangfromthebranches <152056395+Flowershangfromthebranches@users.noreply.github.com> (cherry picked from commit 3da6366a60ac5964e8a8de43486125d72729f8c8) --- ...-sse.md => ADR-0099-responses-http-sse.md} | 2 +- structure/transports/responses.md | 2 +- .../responses-code-mode-patch-compile.test.ts | 26 +++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) rename structure/decisions/{ADR-0097-responses-http-sse.md => ADR-0099-responses-http-sse.md} (96%) diff --git a/structure/decisions/ADR-0097-responses-http-sse.md b/structure/decisions/ADR-0099-responses-http-sse.md similarity index 96% rename from structure/decisions/ADR-0097-responses-http-sse.md rename to structure/decisions/ADR-0099-responses-http-sse.md index ac9688b76ab..3bb32aacce5 100644 --- a/structure/decisions/ADR-0097-responses-http-sse.md +++ b/structure/decisions/ADR-0099-responses-http-sse.md @@ -1,4 +1,4 @@ -# ADR-0097 — decision recorded under "Responses HTTP/SSE" +# ADR-0099 — decision recorded under "Responses HTTP/SSE" - Contract owner: [transports/responses.md](../transports/responses.md#responses-httpsse) diff --git a/structure/transports/responses.md b/structure/transports/responses.md index 22cf579af50..de0d52ef299 100644 --- a/structure/transports/responses.md +++ b/structure/transports/responses.md @@ -634,7 +634,7 @@ name containing the flattened `__` delimiter and neither explicit `default.` nor identity exists. It does not let `default.tool` borrow a namespaced tool's manufactured bare alias, and an unknown suffix still reaches the undeclared-tool failure. -> Decision record: [ADR-0097](../decisions/ADR-0097-responses-http-sse.md) +> Decision record: [ADR-0099](../decisions/ADR-0099-responses-http-sse.md) The passthrough guard resolves an emitted name through that same `normalizeDeclaredToolName`, so whatever it admits it must also EMIT under the resolved name. The two halves disagreed once: diff --git a/tests/responses/responses-code-mode-patch-compile.test.ts b/tests/responses/responses-code-mode-patch-compile.test.ts index 2f3b174628b..702d9390245 100644 --- a/tests/responses/responses-code-mode-patch-compile.test.ts +++ b/tests/responses/responses-code-mode-patch-compile.test.ts @@ -1,6 +1,9 @@ import { describe, expect, test } from "bun:test"; import { compileCodeModeHelperInput, resolveCodeModeHelperName } from "../../src/responses/code-mode-helper-compat"; import { restoreRoutedCustomCallsInJson } from "../../src/responses/custom-tool-compat"; +import { bridgeToResponsesSSE, buildResponseJSON } from "../../src/bridge"; +import type { AdapterEvent } from "../../src/types"; +import { dataPayload } from "../helpers/custom-tool-repair-fixtures"; /** * Recognition and compilation must read ONE canonical body (#5046). @@ -62,6 +65,29 @@ describe("code-mode apply_patch compiles the body recognition accepted", () => { } }); + test("default.apply_patch normalization and body repair agree in JSON and fragmented SSE", async () => { + for (const key of ["patch", "content"]) { + const body = JSON.stringify({ [key]: PATCH }); + async function* events(): AsyncGenerator { + yield { type: "tool_call_start", id: "call-patch", name: "default.apply_patch" }; + for (const part of body) yield { type: "tool_call_delta", id: "call-patch", arguments: part }; + yield { type: "tool_call_end", id: "call-patch" }; + yield { type: "done" }; + } + const options = { declaredToolNames: CODE_MODE }; + const collected: AdapterEvent[] = []; + for await (const event of events()) collected.push(event); + const json = buildResponseJSON(collected, "fixture", { ...options, freeformToolNames: CODE_MODE }); + expect(json.output).toMatchObject([{ type: "custom_tool_call", name: "exec", input: EXPECTED }]); + const stream = bridgeToResponsesSSE(events(), "fixture", undefined, CODE_MODE, undefined, undefined, 50_000, options); + const text = await new Response(stream).text(); + const payloads = text.split(/\r?\n\r?\n/).filter(block => block.includes("data: {")).map(dataPayload); + expect(payloads.find(p => p.type === "response.custom_tool_call_input.done")?.input).toBe(EXPECTED); + const preview = payloads.filter(p => p.type === "response.custom_tool_call_input.delta").map(p => p.delta).join(""); + expect(EXPECTED.startsWith(preview)).toBe(true); + } + }); + test("a normal code-mode JavaScript body is left alone", () => { for (const body of [ 'const result = await tools.exec_command({ cmd: "ls" });\ntext(result);',