diff --git a/docs-site/src/content/docs/reference/proxy-formats.md b/docs-site/src/content/docs/reference/proxy-formats.md index eba53cf8201..82b9208f78a 100644 --- a/docs-site/src/content/docs/reference/proxy-formats.md +++ b/docs-site/src/content/docs/reference/proxy-formats.md @@ -50,6 +50,11 @@ snapshot omits it. Auto/none, incomplete results and unknown contracts retain th ## Required tools on translated Chat routes +Before sending, the proxy rejects a mandatory tool choice that the selected Chat destination cannot +represent, such as `computer_use` or a required allowed-tool list containing MCP selections. A direct +route returns HTTP 400 without an upstream request. A routing profile can continue to another +originally eligible destination under its configured fallback policy, preserving the original choice. + A translated OpenAI Chat response must contain a complete selected client tool call when `tool_choice` is required, names a tool, or uses a required allowed-tool selection. If the upstream finishes normally without one, the turn fails with `required_tool_not_called` and preserves usage. diff --git a/scripts/test-layout/layout-responses.json b/scripts/test-layout/layout-responses.json index 60f1e7075ea..df758d8a344 100644 --- a/scripts/test-layout/layout-responses.json +++ b/scripts/test-layout/layout-responses.json @@ -1,4 +1,5 @@ { + "responses-direct-chat-tool-choice.test.ts": "responses", "responses-native-output-identity.test.ts": "responses", "graceful-max-output-tokens.test.ts": "responses", "graceful-max-output-tokens-exclusions.test.ts": "responses", diff --git a/src/server/responses/combo-required-tool-contract.ts b/src/server/responses/combo-required-tool-contract.ts index a5177b364d8..60e9f1b7091 100644 --- a/src/server/responses/combo-required-tool-contract.ts +++ b/src/server/responses/combo-required-tool-contract.ts @@ -45,7 +45,14 @@ export function hasMandatoryToolChoice(rawBody: unknown): boolean { export function chatCanPreserveMandatoryToolChoice(rawBody: unknown, provider: OcxProviderConfig, modelId: string): boolean { if (!hasMandatoryToolChoice(rawBody)) return true; if (modelInList(provider.autoToolChoiceOnlyModels, modelId)) return false; - const mapped = mapToolChoice((rawBody as Record).tool_choice); + const choice = (rawBody as Record).tool_choice; + if (choice && typeof choice === "object" && !Array.isArray(choice)) { + const selector = choice as Record; + // Chat function names cannot preserve the selected MCP server's ownership. + if (selector.type === "allowed_tools" && Array.isArray(selector.tools) + && selector.tools.some(tool => tool && typeof tool === "object" && tool.type === "mcp")) return false; + } + const mapped = mapToolChoice(choice); return mapped === "required" || (!!mapped && typeof mapped === "object"); } diff --git a/src/server/responses/core-lifetime.ts b/src/server/responses/core-lifetime.ts index 06e27a40c7b..14a24d51ee2 100644 --- a/src/server/responses/core-lifetime.ts +++ b/src/server/responses/core-lifetime.ts @@ -1,6 +1,7 @@ import { carryProxyError } from "../../lib/client-error-body"; import type { TranslatorBudget } from "../../lib/translator-budget"; import { carryResponseToolOwnership } from "./response-tool-ownership"; +import { carryPolicyCandidateRefusal } from "./policy-request-scope"; import { finalizeTranslatorBudgetResponse } from "../../lib/translator-budget"; import { carryReplayRefusal, isNonReplayableResponse, markResponseNonReplayable } from "../../lib/upstream-retry"; import { isVerifiedModelNotInPlanResponse, markVerifiedModelNotInPlanResponse } from "../../lib/errors"; @@ -57,6 +58,7 @@ export function finalizeOwnedTranslatorBudget(response: Response, budget: Transl if (isVerifiedModelNotInPlanResponse(response)) markVerifiedModelNotInPlanResponse(finalizedResponse); if (runTurnAdapterSseResponses.has(response)) runTurnAdapterSseResponses.add(finalizedResponse); carryCodexAccountCooldownResponse(response, finalizedResponse); + carryPolicyCandidateRefusal(response, finalizedResponse); return carryResponseToolOwnership(response, carryFormattedQuotaSource(response, finalizedResponse)); } @@ -86,6 +88,7 @@ export function finalizeAccountLease(response: Response, release: () => void, on if (runTurnAdapterSseResponses.has(response)) runTurnAdapterSseResponses.add(wrapped); // Retained compaction delivers through this lease wrapper; keep a local cooldown refusal local (#7850). carryCodexAccountCooldownResponse(response, wrapped); + carryPolicyCandidateRefusal(response, wrapped); return carryResponseToolOwnership(response, carryReplayRefusal(response, carryFormattedQuotaSource(response, wrapped))); } diff --git a/src/server/responses/policy-request-scope.ts b/src/server/responses/policy-request-scope.ts index 3504fcd7419..d954bd7c374 100644 --- a/src/server/responses/policy-request-scope.ts +++ b/src/server/responses/policy-request-scope.ts @@ -41,6 +41,12 @@ export function isPolicyCandidateRefusal(response: Response): boolean { return localRefusals.has(response); } +/** Preserve local refusal authority when response lifetime owners wrap the body. */ +export function carryPolicyCandidateRefusal(source: Response, target: Response): Response { + if (localRefusals.has(source)) localRefusals.add(target); + return target; +} + export function capturePolicyRequestRoute(scope: PolicyRequestScope, route: RouteResult): RouteResult { if (!scope.eligibility && route.policyEligibility && route.routeDecision?.routeKind === "policy") { scope.eligibility = new Set(route.policyEligibility); diff --git a/src/server/responses/request-prepare.ts b/src/server/responses/request-prepare.ts index 9d373415c04..1027249cc78 100644 --- a/src/server/responses/request-prepare.ts +++ b/src/server/responses/request-prepare.ts @@ -8,7 +8,7 @@ import type { PolicyRequestEvidence } from "../../routing/evaluator"; import type { RequestLogContext } from "../request-log"; import { markProtocolEntry, protocolMarkFeatures } from "../../protocols/trace"; import { isAntigravityRoute } from "../antigravity-direct-eligibility"; -import { capturePolicyRequestRoute, assertPolicyNormalizedRoute, recordPolicyPreparedDestination, policyCandidateRefusalResponse } from "./policy-request-scope"; +import { capturePolicyRequestRoute, assertPolicyNormalizedRoute, recordPolicyPreparedDestination, policyCandidateRefusalResponse, PolicyCandidateUnavailableError } from "./policy-request-scope"; import { agentTaskRecoveryConfig, restoreCachedEncryptedAgentTasks, @@ -74,6 +74,7 @@ import { previousResponseProviderState, } from "../../responses/state"; import { formatErrorResponse } from "../../bridge"; +import { chatCanPreserveMandatoryToolChoice } from "./combo-required-tool-contract"; import { mapCodexAuthContextErrorToResponse } from "./codex-auth-error"; import type { OcxParsedRequest } from "../../types"; import { buildToolBridgeMaps } from "./collaboration"; @@ -1459,6 +1460,12 @@ export async function prepareResponsesRequest( }); try { assertPolicyNormalizedRoute(policyScope, admittedPolicyRoute, route); + if (route.provider.adapter === "openai-chat" + && !chatCanPreserveMandatoryToolChoice(parsed._rawBody, route.provider, route.modelId)) { + if (policyScope.eligibility) throw new PolicyCandidateUnavailableError(); + return formatErrorResponse(400, "invalid_request_error", + "This mandatory tool_choice cannot be preserved for a translated Chat destination."); + } } catch (error) { const policyRefusal = policyCandidateRefusalResponse(error); if (policyRefusal) return policyRefusal; diff --git a/structure/transports/policy-fallback.md b/structure/transports/policy-fallback.md index 196d0c90a96..9ee70b16e76 100644 --- a/structure/transports/policy-fallback.md +++ b/structure/transports/policy-fallback.md @@ -16,6 +16,11 @@ When a main-account hard-lock fallback selects a profile, `src/server/responses/ checks its first ranked destination against the original model under the [fallback policy](../fallback-policy.md#sites), before authentication or upstream dispatch. +After final normalization, preparation refuses a Chat destination that cannot preserve a mandatory +tool choice using `PolicyCandidateUnavailableError`. The local response identity allows traversal +to a compatible originally eligible candidate without sending to the incompatible destination. +Ordinary direct routes keep the client-facing 400; upstream errors do not acquire this identity. + Normal virtual-model wire mapping remains attached to its admitted selector and does not add a new eligible model. Policy traversal records settled physical destinations and skips a destination already attempted through another candidate or redirect; same-target credential and transient retries retain @@ -27,6 +32,8 @@ The initial decision remains the selection evidence while physical attempts reco Cancellation, non-replayable responses and committed output keep the [shared replay boundaries](responses-failover.md). A local policy refusal is identified by its in-process response identity; matching upstream prose or error codes cannot authorize another attempt. +The translator-budget and account-lease wrappers in `src/server/responses/core-lifetime.ts` carry +that identity from their source response without deriving it from serialized error contents. Formatted quota errors retain [their upstream-code provenance](responses-combo-boundaries.md#upstream-account-quota-exhaustion-failover) through lifecycle wrappers; a reflected quota message on a 400/422 response does not authorize a candidate hop merely because formatting serialized it as `insufficient_quota` or its text mentions a free-tier single-request cap. @@ -35,3 +42,7 @@ Coverage: `tests/routing/routing-policy-dispatch-scope.test.ts` exercises real r normalization and configured subagent selection with synthetic dispatch. It also covers recovery, alias collisions, redirects, bounded traces and virtual models. `tests/routing/routing-policy-fallback.test.ts` pins request snapshots, terminal responses, cancellation, credential handoff and attempt accounting. +`tests/responses/responses-direct-chat-tool-choice.test.ts` exercises the real endpoint's pre-send +Chat refusal and preserved selector on a compatible Responses fallback. +`tests/adapters/translator-budget-lifetime.test.ts` distinguishes a local refusal from identical +upstream wire bytes through both lifetime wrappers. diff --git a/structure/transports/responses-combo-boundaries.md b/structure/transports/responses-combo-boundaries.md index d333e07a5c1..80f48158957 100644 --- a/structure/transports/responses-combo-boundaries.md +++ b/structure/transports/responses-combo-boundaries.md @@ -39,6 +39,13 @@ tool-search lowering, still applies after parsing. Chat translation maps a direc choice to its existing synthetic function name. MCP null names also parse inside required allowed-tools lists; function/custom null names remain invalid. A Chat destination is skipped before send when translation would degrade a mandatory choice to auto/none, including the resolved model's auto-only tool-choice policy; exhaustion reports the tool-choice constraint. Eligibility resolves synthetic Fast/effort selectors before model policy, matching child request preparation while preserving exact configured IDs. Registered OpenAI API virtual models resolve to their wire model before adapter and auto-only policy checks, using the same registry resolver as child normalization. Other providers and unregistered IDs remain literal. Coverage: `tests/server/server-combo-required-tool-contract.test.ts`. +Required allowed-tools lists containing MCP entries also exclude Chat: a function name cannot retain +the MCP server's ownership, even when a client function has the same name. The shared predicate in +`src/server/responses/combo-required-tool-contract.ts` also guards direct Chat destinations after final +normalization in `src/server/responses/request-prepare.ts`. An incompatible direct route returns 400 +before dispatch; an authorized policy candidate uses the [local policy refusal](policy-fallback.md) +so another originally eligible destination can serve the unchanged choice. + Before combo success bookkeeping, native delivery in `src/server/responses/passthrough-delivery.ts` uses `src/server/responses/combo-required-tool-contract.ts` to check successful native `openai-responses` JSON children with explicit required or named function/custom choices. diff --git a/tests/adapters/translator-budget-lifetime.test.ts b/tests/adapters/translator-budget-lifetime.test.ts index dc6de8e7c53..910f56cb097 100644 --- a/tests/adapters/translator-budget-lifetime.test.ts +++ b/tests/adapters/translator-budget-lifetime.test.ts @@ -7,6 +7,9 @@ import { finalizeAccountLease, finalizeOwnedTranslatorBudget, runTurnAdapterSseResponses, } from "../../src/server/responses/core-lifetime"; import { isNonReplayableResponse, markResponseNonReplayable } from "../../src/lib/upstream-retry"; +import { + isPolicyCandidateRefusal, PolicyCandidateUnavailableError, policyCandidateRefusalResponse, +} from "../../src/server/responses/policy-request-scope"; import { markNativePassthroughSseResponse, isNativePassthroughSseResponse, markEagerRelaySseResponse, isEagerRelaySseResponse, @@ -176,6 +179,20 @@ for (const outerLease of [false, true]) { } } +test.each([true, false])("policy refusal authority survives lifetime wrappers only for a local response (%s)", async local => { + const refusal = policyCandidateRefusalResponse(new PolicyCandidateUnavailableError())!; + // An upstream can reproduce every wire byte, but cannot mint the local response identity. + const source = local ? refusal : new Response(await refusal.text(), { status: refusal.status, headers: refusal.headers }); + let released = 0; + const budget = createTranslatorBudget(); + const wrapped = finalizeAccountLease(finalizeOwnedTranslatorBudget(source, budget), () => { released++; }); + expect(isPolicyCandidateRefusal(wrapped)).toBe(local); + expect(wrapped.status).toBe(409); + expect((await wrapped.json()).error.code).toBe("policy_candidate_unavailable"); + expect(released).toBe(1); + expect(budget.snapshot().currentBytes).toBe(0); +}); + for (const surface of ["responses", "chat-completions", "claude-messages"] as const) { test(`${surface}: HTTP owner links request abort to its unread response budget`, async () => { const handle = surface === "responses" diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 37927c69741..b695f16a86a 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -305,7 +305,8 @@ "openai-chat-tool-call-id-remint.test.ts": "adapters/openai", "coding-agent-json-lines-scaling.test.ts": "providers", "usage-snapshot-digest-reuse.test.ts": "usage", "worker-embed.test.ts": "lib", "worker-embed-coverage.test.ts": "lib", "jev-stats.test.ts": "usage", - "responses-devin-401-replay.test.ts": "responses", "responses-oauth-refresh-capacity.test.ts": "responses", "responses-grok-devin-preflight.test.ts": "responses", + "responses-devin-401-replay.test.ts": "responses", + "responses-direct-chat-tool-choice.test.ts": "responses", "responses-oauth-refresh-capacity.test.ts": "responses", "responses-grok-devin-preflight.test.ts": "responses", "abort-idle-deadline.test.ts": "lib", "tool-envelope-echo-whole-line.test.ts": "adapters", "abort-race.test.ts": "adapters", "account-import.test.ts": "server", "account-import-project-fallback.test.ts": "server", diff --git a/tests/responses/responses-direct-chat-tool-choice.test.ts b/tests/responses/responses-direct-chat-tool-choice.test.ts new file mode 100644 index 00000000000..8da303ddb03 --- /dev/null +++ b/tests/responses/responses-direct-chat-tool-choice.test.ts @@ -0,0 +1,107 @@ +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { clearResponseStateForTests, flushResponseState } from "../../src/responses/state"; +import { closeRequestHistoryIndex } from "../../src/routing/history/indexer"; +import { handleResponses } from "../../src/server/responses"; +import type { RequestLogContext } from "../../src/server/request-log"; +import type { OcxConfig } from "../../src/types"; +import { acquireOwnedSpendHome } from "../helpers/owned-spend-home"; +import { createTempHome } from "../helpers/temp-home"; + +const clientTool = { type: "function", name: "shell", parameters: { type: "object", properties: {} } }; +const mcpTool = { type: "mcp", server_label: "remote", server_url: "https://mcp.fixture.invalid/mcp" }; +const mcpChoice = { type: "allowed_tools", mode: "required", tools: [{ type: "mcp", server_label: "remote", name: "shell" }] }; +const originalFetch = globalThis.fetch; +let home: ReturnType; +let releaseHome: () => void; + +beforeEach(() => { + home = createTempHome("ocx-direct-chat-choice-"); + releaseHome = acquireOwnedSpendHome(); + clearResponseStateForTests(); + globalThis.fetch = (() => { throw new Error("unexpected network access in tool-choice fixture"); }) as typeof fetch; +}); + +afterEach(async () => { + globalThis.fetch = originalFetch; + await flushResponseState(); + clearResponseStateForTests(); + closeRequestHistoryIndex(); + releaseHome(); + home.remove(); +}); + +function fixture() { + const sent: Array<{ provider: string; body: Record }> = []; + const send = (provider: string) => (async (_input, init) => { + sent.push({ provider, body: JSON.parse(String(init?.body)) }); + return Response.json(provider === "chat" ? { + id: "chat_fixture", object: "chat.completion", model: "model", + choices: [{ index: 0, finish_reason: "tool_calls", message: { role: "assistant", content: null, + tool_calls: [{ id: "call_fixture", type: "function", function: { name: "shell", arguments: "{}" } }], + } }], + usage: { prompt_tokens: 2, completion_tokens: 1, total_tokens: 3 }, + } : { + id: "resp_fixture", object: "response", status: "completed", model: "model", + output: [{ id: "computer_fixture", type: "computer_call", status: "completed" }], + usage: { input_tokens: 2, output_tokens: 1, total_tokens: 3 }, + }); + }) as typeof fetch; + const config: OcxConfig = { + port: 0, defaultProvider: "chat", multiAgentGuidanceEnabled: false, + providers: { + chat: { adapter: "openai-chat", baseUrl: "https://api.openai.com/v1", authMode: "key", + apiKey: "fixture-key", models: ["model"], liveModels: false, fetch: send("chat") }, + responses: { adapter: "openai-responses", baseUrl: "https://responses.fixture.invalid/v1", authMode: "key", + apiKey: "fixture-key", models: ["model"], liveModels: false, fetch: send("responses") }, + }, + routingProfiles: { tools: { candidates: [{ provider: "chat", model: "model" }, { provider: "responses", model: "model" }] } }, + }; + return { config, sent }; +} + +function request(toolChoice: unknown, tools: unknown[], model = "chat/model"): Request { + return new Request("http://localhost/v1/responses", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model, input: "Use the selected tool.", stream: false, store: false, tools, tool_choice: toolChoice }), + }); +} + +test("direct translated Chat rejects an unrepresentable computer_use choice before send", async () => { + const { config, sent } = fixture(); + const response = await handleResponses(request({ type: "computer_use" }, [{ type: "computer_use" }, clientTool]), config, {}); + expect(response.status).toBe(400); + expect(await response.text()).toContain("mandatory tool_choice cannot be preserved"); + expect(sent).toEqual([]); +}); + +test("native Chat cannot replace a required named MCP tool with a same-named client function", async () => { + const { config, sent } = fixture(); + const response = await handleResponses(request(mcpChoice, [mcpTool, clientTool]), config, {}); + expect(response.status, await response.text()).toBe(400); + expect(sent).toEqual([]); +}); + +test.each(["required", { type: "function", name: "shell" }, { + type: "allowed_tools", mode: "required", tools: [{ type: "function", name: "shell" }], +}])("representable client tool choice %j reaches Chat unchanged", async choice => { + const { config, sent } = fixture(); + const response = await handleResponses(request(choice, [clientTool]), config, {}); + const body = await response.json(); + expect(response.status).toBe(200); + expect(body.status).toBe("completed"); + expect(sent).toHaveLength(1); + expect(sent[0]!.provider).toBe("chat"); + expect(sent[0]!.body.tool_choice).toEqual(choice === "required" ? "required" : { type: "function", function: { name: "shell" } }); +}); + +test("a policy skips incompatible Chat and preserves the mandatory selector for Responses", async () => { + const { config, sent } = fixture(); + const choice = { type: "computer_use" }; + const log: RequestLogContext = {}; + const response = await handleResponses(request(choice, [{ type: "computer_use" }, clientTool], "policy/tools"), config, log); + expect(response.status, await response.text()).toBe(200); + expect(log.routeDecision?.selected.provider).toBe("chat"); + expect(sent.map(row => row.provider)).toEqual(["responses"]); + expect(sent[0]!.body.tool_choice).toEqual(choice); +});