Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions scripts/test-layout/layout.json
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,7 @@
"anthropic-empty-content.test.ts": "adapters/anthropic",
"anthropic-eof-tolerance.test.ts": "adapters/anthropic",
"anthropic-error-body.test.ts": "adapters/anthropic",
"anthropic-error-stop-reason.test.ts": "adapters/anthropic",
"anthropic-thinking-usage.test.ts": "adapters/anthropic",
"anthropic-error-stop-reason.test.ts": "adapters/anthropic", "anthropic-thinking-usage.test.ts": "adapters/anthropic",
"anthropic-fast-opt-in.test.ts": "adapters/anthropic",
"anthropic-fast-speed.test.ts": "adapters/anthropic",
"anthropic-hardening.test.ts": "adapters/anthropic",
Expand Down Expand Up @@ -284,7 +283,7 @@
"catalog-verbosity-default.test.ts": "codex-integration",
"catalog-vision-sidecar-modalities.test.ts": "codex-integration",
"catalog-zero-credit-picker.test.ts": "codex-integration",
"chat-completions-deferred-tools.test.ts": "responses",
"chat-completions-deferred-tools.test.ts": "responses", "passthrough-deferred-tools.test.ts": "responses",
"chat-completions-endpoint.test.ts": "responses",
"chat-completions-pool-mode.test.ts": "responses",
"chat-conversation-affinity.test.ts": "responses",
Expand Down
6 changes: 5 additions & 1 deletion src/server/responses-undeclared-tool-guard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,7 @@ function failedBlocks(name: string, newline: string): readonly string[] {
* @param providerExecutedCallTypes - Call types executed by the provider.
* @param declaredBare - Explicitly declared bare tool names without namespace provenance.
* @param declaredCustom - Current bare custom declarations eligible for code-mode recovery.
* @param enforceDeclaredToolNames - Refuse undeclared calls; normalization remains active when false.
* @returns An SSE block rewrite function.
*/
export function createUndeclaredToolCallGuardBlockRewrite(
Expand All @@ -711,6 +712,7 @@ export function createUndeclaredToolCallGuardBlockRewrite(
providerExecutedCallTypes: ProviderExecutedCallTypes = EMPTY_PROVIDER_EXECUTED_CALL_TYPES,
declaredBare?: ReadonlySet<string>,
declaredCustom?: ReadonlySet<string>,
enforceDeclaredToolNames = true,
): SseBlockRewrite {
let tripped = false;
return (block: string) => {
Expand All @@ -723,7 +725,9 @@ export function createUndeclaredToolCallGuardBlockRewrite(
} catch {
return [block];
}
const name = undeclaredToolCallName(parsed, declared, declaredNamelessClientCallTypes, providerExecutedCallTypes, declaredBare, declaredCustom);
const name = enforceDeclaredToolNames
? undeclaredToolCallName(parsed, declared, declaredNamelessClientCallTypes, providerExecutedCallTypes, declaredBare, declaredCustom)
: undefined;
if (name !== undefined) {
tripped = true;
return failedBlocks(name, block.includes("\r\n") ? "\r\n" : "\n");
Expand Down
9 changes: 6 additions & 3 deletions src/server/responses/passthrough-delivery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ export async function deliverPassthroughResponse(
| "subagentFallbackAccountId"
| "clientRequestedStream"
| "translatorBudget"
| "inboundWire"
>,
transportState: Pick<ResponsesTransport, "requestBindings">,
sidecarState: Pick<ResponsesSidecarAuth, "openAiSidecar">,
Expand Down Expand Up @@ -428,7 +429,8 @@ export async function deliverPassthroughResponse(
normalizeFunctionCompletionJson,
} = nativeExchange;
const { commitReasoningReplayServingRoute, recordTerminalOutcomes } = responseEffects;
const { parsed, route, subagentQuotaFailureModel, clientRequestedStream, translatorBudget } = requestState;
const { parsed, route, subagentQuotaFailureModel, clientRequestedStream, translatorBudget, inboundWire } = requestState;
const enforceDeclaredToolNames = inboundWire !== "chat" && inboundWire !== "anthropic";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update the chat framing tests for relay semantics

When a Responses upstream emits an undeclared tool for a Chat inbound request, this line now disables the refusal, but the five cases in tests/responses/chat-sse-framing-guard.test.ts:93-121 still require an undeclared client tool error and forbid tool_calls. Those tests exercise this exact passthrough path with both streaming modes and CR-fragmented SSE, so the changed-source test suite will fail consistently; update them to assert successful relay while retaining their framing coverage.

AGENTS.md reference: src/AGENTS.md:L24-L26

Useful? React with 👍 / 👎.

const { openAiSidecar } = sidecarState;
const { requestBindings } = transportState;

Expand Down Expand Up @@ -836,6 +838,7 @@ export async function deliverPassthroughResponse(
providerExecutedCallTypes,
declaredBareWireToolNames,
recoverableBareCustomWireToolNames,
enforceDeclaredToolNames,
)
: undefined,
grokUpstreamEchoEnabled
Expand Down Expand Up @@ -1293,7 +1296,7 @@ export async function deliverPassthroughResponse(
if (grokUpstreamEchoEnabled) {
clientJson = stripGrokUpstreamEnvelopeEchoFromResponsesJson(clientJson);
}
// #1700: same fail-closed policy as the SSE relay above. Both the plain JSON answer and
// #1700: same inbound-wire refusal policy as the SSE relay above. Both the plain JSON answer and
// the reframed-SSE branch below are built from this body, so one check covers them. This
// runs BEFORE the continuation cache write below: a refused turn must not become state a
// later `previous_response_id` replay can expand from.
Expand All @@ -1312,7 +1315,7 @@ export async function deliverPassthroughResponse(
return undefined;
}
})();
if (undeclared !== undefined) {
if (enforceDeclaredToolNames && undeclared !== undefined) {
return formatErrorResponse(502, "upstream_error", undeclaredToolCallMessage(undeclared));
}
clientJson = normalizeDefaultNamespaceInJson(
Expand Down
10 changes: 5 additions & 5 deletions structure/transports/responses-wire-shapes.md
Original file line number Diff line number Diff line change
Expand Up @@ -315,12 +315,12 @@ an empty tool list is making a statement rather than omitting one, which is how
guard reads it through `clientExplicitWireToolCatalog` in
`src/server/responses/passthrough-dispatch.ts`.

The passthrough guard is not wire-scoped. `undeclaredToolGuardActive` gates namespace normalization
and continuation-state suppression as well as the refusal, and it stands down only for
`authMode: "forward"` and for a request that declares no catalog at all.
The passthrough SSE/JSON refusal is wire-scoped too: only `responses` rejects undeclared calls.
`undeclaredToolGuardActive` still gates namespace normalization and continuation suppression on every wire,
standing down only for `authMode: "forward"` and requests with no catalog; relayed undeclared calls are not remembered.

`src/server/responses/run-turn-execution.ts` and `src/server/responses/adapter-delivery.ts` set the
flag from `inboundWire` on the streaming, buffered, and JSON paths alike, so the three cannot drift.
`src/server/responses/run-turn-execution.ts`, `src/server/responses/adapter-delivery.ts`, and
`src/server/responses/passthrough-delivery.ts` scope membership refusal from `inboundWire` for SSE and JSON alike.

## Selection outlives the declaration check

Expand Down
2 changes: 1 addition & 1 deletion tests/fixtures/test-layout-expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@
"catalog-slug-uniqueness-boundary.test.ts": "codex-integration",
"catalog-verbosity-default.test.ts": "codex-integration",
"catalog-vision-sidecar-modalities.test.ts": "codex-integration",
"catalog-zero-credit-picker.test.ts": "codex-integration", "chat-completions-deferred-tools.test.ts": "responses",
"catalog-zero-credit-picker.test.ts": "codex-integration", "chat-completions-deferred-tools.test.ts": "responses", "passthrough-deferred-tools.test.ts": "responses",
"chat-completions-endpoint.test.ts": "responses", "chat-completions-pool-mode.test.ts": "responses",
"chat-conversation-affinity.test.ts": "responses", "chat-inbound-developer-position.test.ts": "responses",
"protocol-contract.test.ts": "responses", "protocol-features.test.ts": "responses",
Expand Down
57 changes: 47 additions & 10 deletions tests/responses/chat-sse-framing-guard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { mkdtempSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { handleChatCompletions } from "../../src/server/chat-completions";
import { handleResponses } from "../../src/server/responses";
import type { OcxConfig, OcxProviderConfig } from "../../src/types";
import { acquireOwnedSpendHome } from "../helpers/owned-spend-home";
import { removeTreeWithRetry } from "../helpers/remove-tree";
Expand Down Expand Up @@ -32,7 +33,7 @@ function terminal(newline: string): string {
return `event: response.completed${newline}data: ${JSON.stringify({ type: "response.completed", response: { id: "resp_fixture", status: "completed", output: [], usage: { input_tokens: 1, output_tokens: 1 } } })}${newline}${newline}`;
}

async function request(chunks: string[], stream: boolean, live = false) {
async function request(chunks: string[], stream: boolean, live = false, wire: "chat" | "responses" = "chat") {
let requests = 0;
let cancelled = false;
const provider: OcxProviderConfig & { fetch: typeof fetch } = {
Expand All @@ -59,10 +60,15 @@ async function request(chunks: string[], stream: boolean, live = false) {
}) as typeof fetch,
};
const config: OcxConfig = { port: 0, defaultProvider: "fixture", providers: { fixture: provider } };
const response = await handleChatCompletions(new Request("http://localhost/v1/chat/completions", {
const handler = wire === "responses" ? handleResponses : handleChatCompletions;
const input = wire === "responses"
? { input: "fixture", tools: [{ type: "function", name: "declared_tool", parameters: { type: "object" } }] }
: { messages: [{ role: "user", content: "fixture" }],
tools: [{ type: "function", function: { name: "declared_tool", parameters: { type: "object" } } }] };
const endpoint = wire === "responses" ? "/v1/responses" : "/v1/chat/completions";
const response = await handler(new Request(`http://localhost${endpoint}`, {
method: "POST", headers: { "content-type": "application/json" },
body: JSON.stringify({ model: "fixture/model", stream, messages: [{ role: "user", content: "fixture" }],
tools: [{ type: "function", function: { name: "declared_tool", parameters: { type: "object" } } }] }),
body: JSON.stringify({ model: "fixture/model", stream, ...input }),
}), config, { model: "", provider: "" });
expect(requests).toBe(1);
return { response, cancelled: () => cancelled };
Expand Down Expand Up @@ -91,13 +97,16 @@ async function readBounded(response: Response): Promise<string> {

describe("Responses-to-Chat shared SSE security framing", () => {
test.each([[true, false], [false, false], [true, true], [false, true]])(
"rejects CR-hidden undeclared calls (stream=%s, fragmented=%s)", async (stream, fragmented) => {
"chat relays CR-hidden undeclared calls (stream=%s, fragmented=%s)", async (stream, fragmented) => {
const wire = event("unlisted_tool", "\r", "\r\r\n\n") + terminal("\n");
const { response } = await request(fragmented ? [...wire] : [wire], stream);
const text = await readBounded(response);
expect(text).toContain("routed provider emitted undeclared client tool");
expect(text).toContain('"error"');
expect(text).not.toContain('"tool_calls"');
expect(text).toContain('"tool_calls"');
expect(text).toContain('"name":"unlisted_tool"');
expect(text).toContain('"id":"call_fixture"');
expect(text).toContain('"finish_reason":"tool_calls"');
expect(text).not.toContain("undeclared client tool");
expect(text).not.toContain('"error"');
});
test.each(["\n", "\r\n", "\r"])("keeps declared calls with %j framing", async newline => {
const { response } = await request([...event("declared_tool", newline), ...terminal(newline)], true);
Expand All @@ -113,12 +122,40 @@ describe("Responses-to-Chat shared SSE security framing", () => {
expect(text).toContain("[DONE]");
expect(result.cancelled()).toBe(true);
});
test("rejects consecutive CR-only events before upstream EOF", async () => {
test("chat relays consecutive CR-only undeclared events before upstream EOF", async () => {
const result = await request([event("unlisted_tool", "\r") + terminal("\r")], true, true);
const text = await readBounded(result.response);
expect(text).toContain('"tool_calls"');
expect(text).toContain('"name":"unlisted_tool"');
expect(text).toContain('"id":"call_fixture"');
expect(text).toContain('"finish_reason":"tool_calls"');
expect(text).toContain("[DONE]");
expect(text).not.toContain("undeclared client tool");
expect(text).not.toContain('"error"');
expect(result.cancelled()).toBe(true);
});
// #6461 framing protection remains on the refusing inbound wire after #6648.
test.each([[true, false], [false, false], [true, true], [false, true]])(
"responses rejects CR-hidden undeclared calls (stream=%s, fragmented=%s)", async (stream, fragmented) => {
const wire = event("unlisted_tool", "\r", "\r\r\n\n") + terminal("\n");
const { response } = await request(fragmented ? [...wire] : [wire], stream, false, "responses");
const text = await readBounded(response);
expect(text).toContain("routed provider emitted undeclared client tool");
expect(text).toContain('"error"');
expect(text).toContain("response.failed");
expect(text).not.toContain("response.output_item.done");
expect(text).not.toContain('"type":"function_call"');
expect(text).not.toContain("response.completed");
});
test("responses rejects consecutive CR-only undeclared events before upstream EOF", async () => {
const result = await request([event("unlisted_tool", "\r") + terminal("\r")], true, true, "responses");
const text = await readBounded(result.response);
expect(text).toContain("routed provider emitted undeclared client tool");
expect(text).toContain('"error"');
expect(text).not.toContain('"tool_calls"');
expect(text).toContain("response.failed");
expect(text).not.toContain("response.output_item.done");
expect(text).not.toContain('"type":"function_call"');
expect(text).not.toContain("response.completed");
expect(result.cancelled()).toBe(true);
});
});
Loading
Loading