From 70129acd6be9610c0f03f5c9cf4405535f324685 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 16 Sep 2026 11:31:45 +0900 Subject: [PATCH 1/4] fix(providers): give Kimi the Responses tool-result adjacency repair (#4726) [skip ci] Kimi's Code Plan Responses endpoint requires a tool result to follow its call immediately. When the desktop LSP hook injects a developer message between a code-mode exec call and its output, Kimi rejects the whole request with HTTP 400 naming the unanswered tool_call_id. Because the row replays full history every turn, the session then fails permanently rather than once. opencodex already implements exactly this repair in normalizeResponsesToolResultAdjacency, but passthrough gates it on requiresAdjacentResponsesToolResults, which only DeepSeek's entry seeded. Kimi inherited no normalization, so a user who configures Kimi onto the Responses wire hits the 400 on every affected turn. Seed the flag on both kimi and kimi-code. The existing fill-only derivation in providerConfigSeed, enrichProviderFromRegistry and routedProviderConfig carries it into new and already-persisted rows without overriding an explicit user value, and the flag is inert while these presets use the Chat wire. The repair reorders; it does not delete. The intervening developer message is preserved and moves after the batch, so the fix cannot be mistaken for silencing the 400 by dropping hook context. Coverage pins that, plus call_id pairing with two outstanding calls and interleaved noise, and an already-adjacent input being left untouched. No upstream specification documents the requirement; the evidence is the reported 400 and DeepSeek's identical failure shape under #1292. Upstream Codex deliberately leaves an intervening developer message where it is, so this stays a per-provider capability rather than a wire-wide default. --- scripts/test-layout/layout.json | 1 + src/providers/registry/entries-core.ts | 4 + src/providers/registry/entries-extended.ts | 2 + structure/providers/chat-compat.md | 8 ++ tests/fixtures/test-layout-expected.json | 1 + .../kimi-responses-adjacency.test.ts | 131 ++++++++++++++++++ 6 files changed, 147 insertions(+) create mode 100644 tests/providers/kimi-responses-adjacency.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index bc654f60c61..7e9cc529597 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -789,6 +789,7 @@ "key-login-preserves-model-costs.test.ts": "oauth", "keyring-smoke.test.ts": "ci-workflows", "kimi-oauth-identity.test.ts": "providers", + "kimi-responses-adjacency.test.ts": "providers", "kiro-account-quota.test.ts": "providers/kiro", "kiro-adapter.test.ts": "providers/kiro", "kiro-auth-context-continuation.test.ts": "providers/kiro", diff --git a/src/providers/registry/entries-core.ts b/src/providers/registry/entries-core.ts index 32e5cc2d957..9df05ff5f60 100644 --- a/src/providers/registry/entries-core.ts +++ b/src/providers/registry/entries-core.ts @@ -420,6 +420,10 @@ export const PROVIDER_REGISTRY_CORE: readonly ProviderRegistryEntry[] = [ // or the one the Claude /v1/messages inbound derives); the adapter itself never invents one. // Evidence: https://platform.kimi.com/docs/api/chat promptCacheKey: true, + // Kimi's Responses endpoint rejects hook-provided context between a tool call and + // its matching result (#4726), the same strict shape DeepSeek exposed in #1292. + // The flag is inert while this preset uses the Chat wire. + requiresAdjacentResponsesToolResults: true, featured: true, oauthId: "kimi", jawcodeBundle: "moonshot", diff --git a/src/providers/registry/entries-extended.ts b/src/providers/registry/entries-extended.ts index 5cea61a84ff..17916596155 100644 --- a/src/providers/registry/entries-extended.ts +++ b/src/providers/registry/entries-extended.ts @@ -890,6 +890,8 @@ export const PROVIDER_REGISTRY_EXTENDED: readonly ProviderRegistryEntry[] = [ modelSuffixBracketStrip: true, // API-key form of the same Kimi Code Plan transport; keep cache affinity identical to OAuth. promptCacheKey: true, + // Keep Responses tool-result adjacency aligned with the OAuth preset (#4726). + requiresAdjacentResponsesToolResults: true, models: KIMI_CODING_MODELS, modelContextWindows: KIMI_CODING_MODEL_CONTEXT_WINDOWS, modelInputModalities: KIMI_CODING_MODEL_INPUT_MODALITIES, diff --git a/structure/providers/chat-compat.md b/structure/providers/chat-compat.md index 5ee17ed8070..b2eaadee795 100644 --- a/structure/providers/chat-compat.md +++ b/structure/providers/chat-compat.md @@ -131,6 +131,14 @@ dropped. This preserves #1292's single-call adjacency repair without splitting a batch away from its preceding plaintext reasoning (#1477). Tolerant providers never enter this pass, and duplicate, missing, or backwards call/result pairs are left for the upstream to reject rather than guessed. +That pass is gated by `requiresAdjacentResponsesToolResults`, not by provider name. Kimi's Code Plan +Responses endpoint enforces the same strict shape and rejects a hook-split pair with HTTP 400 (#4726), +so `kimi` and `kimi-code` carry the flag as well. The flag is inert while those presets use the Chat +wire and takes effect when a row is configured onto `openai-responses`, which is the configuration the +report exercised. No upstream specification documents the requirement; the evidence is the observed +400 and DeepSeek's identical failure shape, which is why this stays a per-provider capability rather +than a wire-wide default — upstream Codex leaves an intervening developer message where it is. + > Decision record: [ADR-0052](../decisions/ADR-0052-reasoning-and-tool-result-compatibility.md) ## OpenRouter provider routing diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index b67edffd477..91a58a6461c 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -619,6 +619,7 @@ "key-login-preserves-model-costs.test.ts": "oauth", "keyring-smoke.test.ts": "ci-workflows", "kimi-oauth-identity.test.ts": "providers", + "kimi-responses-adjacency.test.ts": "providers", "kiro-account-quota.test.ts": "providers/kiro", "kiro-adapter.test.ts": "providers/kiro", "kiro-auth-context-continuation.test.ts": "providers/kiro", diff --git a/tests/providers/kimi-responses-adjacency.test.ts b/tests/providers/kimi-responses-adjacency.test.ts new file mode 100644 index 00000000000..e498fbae4cd --- /dev/null +++ b/tests/providers/kimi-responses-adjacency.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, test } from "bun:test"; +import { createResponsesPassthroughAdapter as createResponsesPassthroughAdapterProduction } from "../../src/adapters/openai-responses"; +import { normalizeResponsesToolResultAdjacency } from "../../src/adapters/openai-responses/tool-output-recovery"; +import { enrichProviderFromRegistry, providerConfigSeed } from "../../src/providers/derive"; +import { getProviderRegistryEntry } from "../../src/providers/registry"; +import { routedProviderConfig } from "../../src/router"; +import type { OcxProviderConfig } from "../../src/types"; +import { withTestTranslatorBudget } from "../helpers/translator-budget"; + +const MODEL = "kimi-k2.7-code"; + +const createResponsesPassthroughAdapter = ( + ...args: Parameters +) => withTestTranslatorBudget(createResponsesPassthroughAdapterProduction(...args)); + +function buildBody(provider: OcxProviderConfig, input: unknown[]): { input: unknown[] } { + const built = createResponsesPassthroughAdapter(provider).buildRequest({ + modelId: MODEL, + context: { messages: [] }, + stream: true, + options: {}, + _rawBody: { model: MODEL, input }, + } as Parameters["buildRequest"]>[0], { + headers: new Headers(), + }); + return JSON.parse(String(built.body)) as { input: unknown[] }; +} + +describe("Kimi Responses tool-result adjacency", () => { + test("both Kimi registry entries seed the adjacency capability", () => { + for (const providerId of ["kimi", "kimi-code"]) { + const entry = getProviderRegistryEntry(providerId)!; + expect(entry.requiresAdjacentResponsesToolResults).toBe(true); + expect(providerConfigSeed(entry).requiresAdjacentResponsesToolResults).toBe(true); + } + }); + + test("a stale persisted Kimi row is backfilled and activates adjacency repair on replay", () => { + const stale: OcxProviderConfig = { + adapter: "openai-responses", + baseUrl: "https://api.kimi.com/coding/v1", + authMode: "oauth", + statelessResponses: true, + }; + const routedStale = routedProviderConfig("kimi", { ...stale }); + const call = { type: "custom_tool_call", call_id: "exec_replay", name: "exec", input: "text('hi')" }; + const injected = { + type: "message", + role: "developer", + content: [{ type: "input_text", text: "[hook] replay diagnostics" }], + }; + const output = { type: "custom_tool_call_output", call_id: "exec_replay", output: "hi" }; + const nextTurn = { + type: "message", + role: "user", + content: [{ type: "input_text", text: "continue" }], + }; + + expect(stale.requiresAdjacentResponsesToolResults).toBeUndefined(); + expect(routedStale.requiresAdjacentResponsesToolResults).toBe(true); + enrichProviderFromRegistry("kimi", stale); + expect(stale.requiresAdjacentResponsesToolResults).toBe(true); + expect(buildBody(stale, [call, injected, output, nextTurn]).input).toEqual([ + call, + output, + injected, + nextTurn, + ]); + }); + + test("moves a result next to its call while preserving an intervening developer message", () => { + const call = { type: "custom_tool_call", call_id: "exec_single", name: "exec", input: "text('ok')" }; + const injected = { + type: "message", + role: "developer", + content: [{ type: "input_text", text: "[hook] LSP diagnostics: none" }], + }; + const output = { type: "custom_tool_call_output", call_id: "exec_single", output: "ok" }; + const body = { input: [call, injected, output] }; + + expect(normalizeResponsesToolResultAdjacency(body)).toEqual({ input: [call, output, injected] }); + }); + + test("keeps call_id pairing and all interleaved history with two outstanding replayed calls", () => { + const priorUser = { + type: "message", + role: "user", + content: [{ type: "input_text", text: "inspect both files" }], + }; + const callA = { type: "function_call", call_id: "call_a", name: "read_file", arguments: "{\"path\":\"a\"}" }; + const firstInjected = { + type: "message", + role: "developer", + content: [{ type: "input_text", text: "[hook] first diagnostic" }], + }; + const callB = { type: "custom_tool_call", call_id: "call_b", name: "exec", input: "text('b')" }; + const secondInjected = { + type: "message", + role: "developer", + content: [{ type: "input_text", text: "[hook] second diagnostic" }], + }; + const outputA = { type: "function_call_output", call_id: "call_a", output: "A" }; + const outputB = { type: "custom_tool_call_output", call_id: "call_b", output: "B" }; + const nextTurn = { + type: "message", + role: "user", + content: [{ type: "input_text", text: "continue" }], + }; + + const normalized = normalizeResponsesToolResultAdjacency({ + input: [priorUser, callA, firstInjected, callB, secondInjected, outputA, outputB, nextTurn], + }); + + expect(normalized).toEqual({ + input: [priorUser, callA, callB, outputA, outputB, firstInjected, secondInjected, nextTurn], + }); + }); + + test("leaves an already-adjacent call and result input untouched", () => { + const call = { type: "custom_tool_call", call_id: "exec_adjacent", name: "exec", input: "text('ok')" }; + const output = { type: "custom_tool_call_output", call_id: "exec_adjacent", output: "ok" }; + const tail = { + type: "message", + role: "developer", + content: [{ type: "input_text", text: "retained context" }], + }; + const body = { input: [call, output, tail] }; + + expect(normalizeResponsesToolResultAdjacency(body)).toBe(body); + }); +}); From 217fe0179fbdb45f68ea1e0e9421d809505281fa Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 16 Sep 2026 11:32:53 +0900 Subject: [PATCH 2/4] fix(catalog): normalize the custom-provider model-discovery join (#4724) [skip ci] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A custom provider whose baseUrl ends in a slash produced a doubled discovery path: https://gateway.example.com/v1//models. Gateways that route the doubled path as a distinct route reject it — the report observed HTTP 403 — so discovery failed and the catalog silently fell back to the configured models. The send paths were normalized already, by openaiChatCompletionsUrl and openaiResponsesUrl, but discovery was not. buildModelsRequest appended the endpoint verbatim, and resolveProviderModelDiscoveryUrl returns that default unchanged for a provider with no registry spec, which is exactly the custom case. Registry providers escaped it because new URL(spec.path, base) collapses the doubled slash. providerModelsUrl mirrors openaiChatCompletionsUrl rather than inventing a second policy: trim outer whitespace and trailing slashes, drop an already pasted /models, then append exactly one. Both production callers of the default discovery URL use it — catalog discovery and API-key validation. An existing path prefix is preserved, so /api/openai/v1 is not collapsed to the origin. Registry spec.path, absolute endpoint overrides and relative endpoint overrides all resolve exactly as before; a baseUrl already written without a trailing slash is byte-identical to its previous output. --- src/oauth/index.ts | 4 +- src/oauth/key-providers.ts | 4 +- src/providers/model-discovery.ts | 16 +++++ structure/runtime.md | 6 +- .../provider-model-discovery-contract.test.ts | 67 +++++++++++++++++++ .../management-client-config-route.test.ts | 8 ++- .../model-discovery-management-api.test.ts | 13 +++- 7 files changed, 111 insertions(+), 7 deletions(-) diff --git a/src/oauth/index.ts b/src/oauth/index.ts index e4904846517..471e16534ca 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -47,7 +47,7 @@ import { ANTIGRAVITY_REQUEST_UA } from "../adapters/google-antigravity-wire"; import { deriveOAuthDefaultModel, deriveOAuthProviderConfig } from "../providers/derive"; import { apiKeyPoolEntryId, sanitizeApiKeyValue } from "../providers/api-keys"; import { effectiveGoogleMode, getProviderRegistryEntry, mergeRegistryStaticHeaders, providerMatchesRegistryTransport } from "../providers/registry"; -import { resolveProviderModelDiscoveryUrl } from "../providers/model-discovery"; +import { providerModelsUrl, resolveProviderModelDiscoveryUrl } from "../providers/model-discovery"; import { resolveProviderTransport } from "../providers/xai-transport"; import { detectClaudeCodeToken, detectGrokCliToken, hasComparableGrokIdentity, isSameGrokIdentity, shouldAdoptGrokGeneration } from "./local-token-detect"; import { logOAuthEvent } from "./log"; @@ -1228,7 +1228,7 @@ export function buildModelsRequest( return { url: discoveryUrl(`${base}/v1/models?limit=1000`), headers }; } if (apiKey) headers["Authorization"] = `Bearer ${apiKey}`; - return { url: discoveryUrl(`${effectiveProvider.baseUrl}/models`), headers }; + return { url: discoveryUrl(providerModelsUrl(effectiveProvider.baseUrl)), headers }; } /** diff --git a/src/oauth/key-providers.ts b/src/oauth/key-providers.ts index f147b6a44ff..29386cb8b6e 100644 --- a/src/oauth/key-providers.ts +++ b/src/oauth/key-providers.ts @@ -1,6 +1,6 @@ import type { OcxProviderConfig } from "../types"; import { deriveKeyLoginMap, enrichProviderFromRegistry, type DerivedKeyLoginProvider } from "../providers/derive"; -import { resolveProviderModelDiscoveryUrl } from "../providers/model-discovery"; +import { providerModelsUrl, resolveProviderModelDiscoveryUrl } from "../providers/model-discovery"; /** * API-key "login" providers: not OAuth — the flow opens the provider's dashboard so the user can @@ -117,7 +117,7 @@ export async function validateApiKey( providerName, configuredProvider, provider.baseUrl, - `${provider.baseUrl}/models`, + providerModelsUrl(provider.baseUrl), ); const res = await fetch(modelsUrl, { headers: { Authorization: `Bearer ${key}` }, diff --git a/src/providers/model-discovery.ts b/src/providers/model-discovery.ts index a0718ac0684..78b68e55ef7 100644 --- a/src/providers/model-discovery.ts +++ b/src/providers/model-discovery.ts @@ -23,6 +23,8 @@ import { const MODEL_DISCOVERY_MAX_FILTER_VALUES = 256; const MODEL_DISCOVERY_MAX_FILTER_STRING_LENGTH = 1_024; +const TRAILING_SLASHES = /\/+$/; +const TRAILING_MODELS = /\/models$/; export interface ResolvedProviderModelDiscovery { spec?: ProviderModelDiscoverySpec; @@ -50,6 +52,20 @@ export type ModelEnvelopeRowsResult = | { ok: true; rows: unknown[] } | { ok: false; reason: "invalid_shape" | "too_many_models" }; +/** + * Build the default OpenAI-compatible model-discovery URL from a configured baseUrl. + * + * `baseUrl` is required on both `OcxProviderConfig` and the persisted-config schema, so a row + * without one is not a state configuration loading can produce. It is deliberately not tolerated + * here: the old template-literal join silently produced `"undefined/models"`, which is not a usable + * fallback either — it only ever survived because a static row returns before the URL is parsed. + */ +export function providerModelsUrl(baseUrl: string): string { + const trimmed = baseUrl.trim().replace(TRAILING_SLASHES, ""); + const withoutEndpoint = trimmed.replace(TRAILING_MODELS, ""); + return `${withoutEndpoint}/models`; +} + function positiveIntegerAtMost(value: number | undefined, hardLimit: number): number { if (typeof value !== "number" || !Number.isFinite(value) || value <= 0) return hardLimit; return Math.min(Math.floor(value), hardLimit); diff --git a/structure/runtime.md b/structure/runtime.md index 248c3011c64..89aaa82df6a 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -201,7 +201,11 @@ The image/video loop bounds each hidden iteration before replay or fulfillment; [media iteration retention](transports/inventory.md#media-iteration-retention). Live model discovery is bounded and registry-driven through `src/providers/model-discovery.ts`. -Custom providers keep the conventional `${baseUrl}/models` request; canonical presets may select a +Custom providers keep the conventional `${baseUrl}/models` request, normalized by +`providerModelsUrl` the same way `openaiChatCompletionsUrl` normalizes the send path: outer +whitespace and trailing slashes are trimmed and an already-pasted `/models` is not doubled, so a +`baseUrl` written with or without a trailing slash yields the identical discovery URL and an +existing path prefix is preserved. Canonical presets may select a trusted URL/path/query and declarative eligibility filter without persisting that policy into user config. A response is rejected before caching when it exceeds 4 MiB, contains more than 2,000 raw rows, has a malformed OpenAI list envelope, or includes an invalid model id. Tests use fixtures and diff --git a/tests/providers/provider-model-discovery-contract.test.ts b/tests/providers/provider-model-discovery-contract.test.ts index 89cfef37b75..9a4a6fae256 100644 --- a/tests/providers/provider-model-discovery-contract.test.ts +++ b/tests/providers/provider-model-discovery-contract.test.ts @@ -12,6 +12,7 @@ import { deriveKeyLoginMap, providerConfigSeed } from "../../src/providers/deriv import { extractProviderModelItems, isRegistryModelDiscoveryUrl, + providerModelsUrl, providerModelDiscoverySpecError, readBoundedDiscoveryJson, resolveProviderModelDiscovery, @@ -68,6 +69,58 @@ function togetherConfig(overrides: Partial = {}): OcxConfig { } describe("registry-owned provider model discovery", () => { + test("normalizes custom model discovery URLs without collapsing path prefixes (#4724)", () => { + const buildCustom = (baseUrl: string) => buildModelsRequest({ + adapter: "openai-responses", + baseUrl, + authMode: "key", + }, "secret", "custom-gateway").url; + + expect(buildCustom("https://gw.example.com/v1")).toBe("https://gw.example.com/v1/models"); + expect(buildCustom("https://gw.example.com/v1/")).toBe("https://gw.example.com/v1/models"); + expect(buildCustom("https://gw.example.com/v1////")).toBe("https://gw.example.com/v1/models"); + expect(buildCustom("https://gw.example.com/api/openai/v1/")) + .toBe("https://gw.example.com/api/openai/v1/models"); + expect(buildCustom("https://gw.example.com/tenant/acme/api/openai/v1///")) + .toBe("https://gw.example.com/tenant/acme/api/openai/v1/models"); + expect(buildCustom("https://gw.example.com/v1/models")) + .toBe("https://gw.example.com/v1/models"); + expect(buildCustom("https://gw.example.com/v1/models/")) + .toBe("https://gw.example.com/v1/models"); + }); + + test("keeps registry path discovery independent of default URL normalization (#4724)", () => { + const url = resolveProviderModelDiscoveryUrl( + "cloudflare-workers-ai", + { + adapter: "openai-chat", + baseUrl: "https://api.cloudflare.com/client/v4/accounts/acct/ai/v1/", + }, + "https://api.cloudflare.com/client/v4/accounts/acct/ai/v1/", + providerModelsUrl("https://api.cloudflare.com/client/v4/accounts/acct/ai/v1/"), + ); + expect(url).toBe( + "https://api.cloudflare.com/client/v4/accounts/acct/ai/models/search?format=openrouter&per_page=1000", + ); + }); + + test("keeps absolute and relative discovery endpoint overrides unchanged (#4724)", async () => { + await withTogetherDiscovery({ + url: "https://catalog.example.test/custom/models?source=registry", + }, () => { + expect(buildModelsRequest(togetherConfig().providers.together!, "secret", "together").url) + .toBe("https://catalog.example.test/custom/models?source=registry"); + }); + + await withTogetherDiscovery({ path: "catalog/models" }, () => { + expect(buildModelsRequest( + togetherConfig({ baseUrl: "https://api.together.xyz/v1/" }).providers.together!, + "secret", + "together", + ).url).toBe("https://api.together.xyz/v1/catalog/models"); + }); + }); + test("keeps every registry discovery contract inside static safety bounds", () => { for (const entry of PROVIDER_REGISTRY) { if (!entry.modelDiscovery) continue; @@ -170,6 +223,20 @@ describe("registry-owned provider model discovery", () => { }); }); + test("normalizes a custom API-key validation discovery URL (#4724)", async () => { + globalThis.fetch = (async (input, init) => { + expect(String(input)).toBe("https://custom.example/api/openai/v1/models"); + expect(new Headers(init?.headers).get("authorization")).toBe("Bearer secret"); + expect(init?.redirect).toBe("error"); + return Response.json({ data: [] }); + }) as typeof fetch; + + expect(await validateApiKey("custom-gateway", { + ...KEY_LOGIN_PROVIDERS.together!, + baseUrl: "https://custom.example/api/openai/v1///", + }, "secret")).toBe(true); + }); + test("pins fixed OAuth discovery before resolving relative and default endpoints", async () => { const staleConfig: OcxProviderConfig = { adapter: "openai-chat", diff --git a/tests/server/management-client-config-route.test.ts b/tests/server/management-client-config-route.test.ts index 45514c9754f..ba83820c72a 100644 --- a/tests/server/management-client-config-route.test.ts +++ b/tests/server/management-client-config-route.test.ts @@ -401,7 +401,13 @@ describe("GET /api/client-config", () => { const config = baseConfig({ providers: { ...baseConfig().providers, - openai: { authMode: "forward", liveModels: false, models: [] }, + // Spelled out the same way as the other `openai` fixture in this file: `adapter` and + // `baseUrl` are required by the persisted-config schema, so a row without them is not a + // state configuration loading can produce, and discovery builds a request from it. + openai: { + adapter: "openai-responses", authMode: "forward", liveModels: false, + baseUrl: "https://chatgpt.com/backend-api/codex", models: [], + }, }, }); const response = await clientConfigApi(config, "?client=opencode"); diff --git a/tests/server/model-discovery-management-api.test.ts b/tests/server/model-discovery-management-api.test.ts index ad132847bb7..dc8dea96fe0 100644 --- a/tests/server/model-discovery-management-api.test.ts +++ b/tests/server/model-discovery-management-api.test.ts @@ -4,7 +4,18 @@ import type { OcxConfig } from "../../src/types"; import { ManagementRequest as Request } from "../helpers/management-auth"; function config(): OcxConfig { - return { port: 10100, defaultProvider: "vendor", providers: { vendor: { liveModels: false, models: ["known"] } }, disabledModels: ["vendor/new"] }; + // `adapter` and `baseUrl` are both required by the persisted-config schema, so a row without + // them cannot reach this handler in production. The fixture used to omit them and still crossed + // into the real catalog gather path, which builds a discovery request before the static-provider + // branch returns; that is how it reached a URL join at all. + return { + port: 10100, + defaultProvider: "vendor", + providers: { + vendor: { adapter: "openai-chat", baseUrl: "https://vendor.example/v1", liveModels: false, models: ["known"] }, + }, + disabledModels: ["vendor/new"], + }; } async function call(live: OcxConfig, path: string, method = "GET", body?: unknown) { From 915af60e52d12de0bddd62037671d96983e83345 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 16 Sep 2026 11:35:26 +0900 Subject: [PATCH 3/4] fix(codebuddy): refuse leaked vendor tool-call scaffolding (#4596) The CodeBuddy route launches the vendor CLI with --tools "" and --strict-mcp-config, so the routed model has no native tool channel and writes its call as prose. The shared coding-agent projection forwards text_delta unrepaired, so that markup reached the client as an ordinary assistant answer. Qoder's guard does not match it. The leaked tags are wrapped in FULLWIDTH VERTICAL LINE (U+FF5C), which none of the shipped UNREPAIRABLE_MARKERS cover, so this needed a signature of its own rather than a port. Refusal requires the observed two-line grammar: a calls control line at column zero, outside a Markdown fence, immediately followed by an invoke line naming a functions.* tool. A lone tag, a quoted or inline-code literal, a fenced example, a blockquote, indented source, or prose discussing the markup all carry extra syntax before the tag and are forwarded untouched. Matching the marker alone would refuse a legitimate answer that merely explains this protocol, which is why the detector is narrower than the marker spelling. A detected leak preserves the answer text already proven safe, emits one non-retryable vendor_scaffold_detected error, and suppresses the vendor's later success terminal so the client never sees a completed turn. Markers split across streamed deltas are caught by holding only a bounded suffix that could still complete a control sequence or a fence; unrelated pending text is released at the next mismatch or terminal. The reasoning channel is guarded independently. Leaked prose is never promoted into a real tool call. The text channel carries no authenticated call envelope and no validated arguments, so converting it would manufacture execution authority out of model output. Kept CodeBuddy-owned rather than lifted into the shared coding-agent path, the same containment #4234 chose for Qoder: the contract observed here is this vendor's, and #4190's lane packet asked for a report rather than symmetry. Co-authored-by: Ingwannu --- .../src/content/docs/guides/providers.md | 2 +- src/adapters/codebuddy/adapter.ts | 3 +- src/adapters/codebuddy/scaffold-guard.ts | 248 ++++++++++++++++++ structure/providers/chat-compat.md | 6 +- tests/providers/codebuddy-adapter.test.ts | 231 ++++++++++++++++ 5 files changed, 487 insertions(+), 3 deletions(-) create mode 100644 src/adapters/codebuddy/scaffold-guard.ts diff --git a/docs-site/src/content/docs/guides/providers.md b/docs-site/src/content/docs/guides/providers.md index f45f653c395..236ea50fa94 100644 --- a/docs-site/src/content/docs/guides/providers.md +++ b/docs-site/src/content/docs/guides/providers.md @@ -759,7 +759,7 @@ OpenCodex provides official adapter support for Tencent Cloud's CodeBuddy Code C - Global: [CodeBuddy Global API Keys](https://www.codebuddy.ai/profile/keys) - CN: [CodeBuddy CN API Keys](https://copilot.tencent.com/profile/keys) - **Region Isolation:** `codebuddy` and `codebuddy-cn` use separate canonical endpoints (`https://www.codebuddy.ai` and `https://www.codebuddy.cn`) and isolated child environments (`CODEBUDDY_INTERNET_ENVIRONMENT=public` vs `internal`). Credentials are strictly region-scoped and never exchanged across environments. Overriding the canonical base URL fails closed. -- **Tool Ownership:** In v1, the CLI is spawned with `--tools ""` and `--strict-mcp-config`, ensuring Codex maintains exclusive tool ownership. The provider operates in text and reasoning mode; client tool execution is not delegated to the vendor CLI. +- **Tool Ownership:** In v1, the CLI is spawned with `--tools ""` and `--strict-mcp-config`, ensuring Codex maintains exclusive tool ownership. The provider operates in text and reasoning mode; client tool execution is not delegated to the vendor CLI. If the CLI writes an unquoted DSML `calls` control line followed by a `functions.*` invoke control line into text or reasoning, OpenCodex refuses the turn instead of forwarding the scaffold or interpreting it as an executable call. DSML discussed or quoted in prose, inline code, fenced code, or source examples remains ordinary answer text. - **Entitlements and Billing:** The provider uses the same vendor-documented CodeBuddy account/CLI authentication surface. Availability and billing of free, promotional, trial, or subscription credits remain determined by the user's CodeBuddy account entitlement. ### Official Qoder CLI (Global & CN) diff --git a/src/adapters/codebuddy/adapter.ts b/src/adapters/codebuddy/adapter.ts index 234e06907e3..a769ac37dae 100644 --- a/src/adapters/codebuddy/adapter.ts +++ b/src/adapters/codebuddy/adapter.ts @@ -4,6 +4,7 @@ import { mapReasoningEffort } from "../../reasoning-effort"; import { buildSystemPrompt } from "../coding-agent/protocol"; import { baseScopedEnv, runCodingAgentTurn, type CodingAgentDeps, type SpawnFn } from "../coding-agent/turn"; import { CODEBUDDY_PROFILES, type CodeBuddyProfile } from "./profiles"; +import { guardCodeBuddyScaffolding } from "./scaffold-guard"; export type { SpawnFn } from "../coding-agent/turn"; export type CodeBuddyAdapterDeps = CodingAgentDeps; @@ -75,7 +76,7 @@ export function createCodeBuddyAdapter(provider: OcxProviderConfig, deps: CodeBu provider, parsed, incoming, - emit, + emit: guardCodeBuddyScaffolding(emit), buildArgs: (resolved, req, prov) => buildArgs(resolved as CodeBuddyProfile, req, prov), buildEnv: (resolved, apiKey) => buildChildEnv(resolved as CodeBuddyProfile, apiKey), deps, diff --git a/src/adapters/codebuddy/scaffold-guard.ts b/src/adapters/codebuddy/scaffold-guard.ts new file mode 100644 index 00000000000..6f8c80aed57 --- /dev/null +++ b/src/adapters/codebuddy/scaffold-guard.ts @@ -0,0 +1,248 @@ +import type { AdapterEvent } from "../../types"; + +/** Error code for a CodeBuddy turn whose output contains vendor agent scaffolding. */ +export const CODEBUDDY_SCAFFOLD_ERROR_CODE = "vendor_scaffold_detected"; + +// The observed control protocol uses FULLWIDTH VERTICAL LINE (U+FF5C). Detection stays +// deliberately narrower than the marker spelling: a calls control line must be followed by an +// invoke line for a functions.* tool. That distinguishes an agent scaffold from prose quoting or +// discussing one tag. +const DSML_CALLS_LINE = "<｜｜dsml｜｜ calls>"; +const DSML_INVOKE_PREFIX = "<｜｜dsml｜｜ invoke name=\"functions."; + +export interface CodeBuddyScaffoldFilterResult { + /** Bytes released from a suffix withheld by an earlier event on this channel. */ + releasedPending: string; + /** Safe bytes belonging to the event currently being processed. */ + text: string; + /** The earlier pending event still owns the extended candidate. */ + pendingContinues: boolean; + fail: boolean; +} + +interface ScanResult { + safe: string; + held: string; + fail: boolean; + fence: "`" | "~" | null; + lineStart: boolean; +} + +function prefixAtEnd(text: string, at: number, expected: string): boolean { + const rest = text.slice(at).toLowerCase(); + return rest.length < expected.length && expected.startsWith(rest); +} + +/** + * Scan complete bytes and retain only a bounded suffix that can still become a control sequence. + * + * Control tags are recognized only at column zero and outside fenced Markdown. Inline code, + * quoted strings, blockquotes, indented source, and prose all add syntax before the tag and are + * therefore forwarded unchanged. A calls line alone is harmless; refusal requires the observed + * two-line calls-plus-functions-invoke grammar. + */ +function scan( + text: string, + initialFence: "`" | "~" | null, + initialLineStart: boolean, +): ScanResult { + let fence = initialFence; + let lineStart = initialLineStart; + let index = 0; + + while (index < text.length) { + if (lineStart) { + const fenceMarkers = fence ? [fence.repeat(3)] : ["```", "~~~"]; + const completeFence = fenceMarkers.find(marker => text.startsWith(marker, index)); + if (completeFence) { + fence = fence ? null : (completeFence[0] as "`" | "~"); + index += completeFence.length; + lineStart = false; + continue; + } + if (fenceMarkers.some(marker => prefixAtEnd(text, index, marker))) { + return { safe: text.slice(0, index), held: text.slice(index), fail: false, fence, lineStart }; + } + + if (!fence) { + const lowered = text.slice(index).toLowerCase(); + if (lowered.startsWith(DSML_CALLS_LINE)) { + const afterCalls = index + DSML_CALLS_LINE.length; + let invokeAt = -1; + if (text[afterCalls] === "\n") invokeAt = afterCalls + 1; + else if (text[afterCalls] === "\r" && text[afterCalls + 1] === "\n") invokeAt = afterCalls + 2; + else if (afterCalls === text.length || (text[afterCalls] === "\r" && afterCalls + 1 === text.length)) { + return { safe: text.slice(0, index), held: text.slice(index), fail: false, fence, lineStart }; + } + + if (invokeAt >= 0) { + const invokeRest = text.slice(invokeAt).toLowerCase(); + if (invokeRest.startsWith(DSML_INVOKE_PREFIX)) { + return { safe: text.slice(0, index), held: "", fail: true, fence, lineStart }; + } + if (invokeRest.length === 0 || DSML_INVOKE_PREFIX.startsWith(invokeRest)) { + return { safe: text.slice(0, index), held: text.slice(index), fail: false, fence, lineStart }; + } + } + } else if (prefixAtEnd(text, index, DSML_CALLS_LINE)) { + return { safe: text.slice(0, index), held: text.slice(index), fail: false, fence, lineStart }; + } + } + } + + const char = text[index]!; + index += 1; + lineStart = char === "\n"; + } + + return { safe: text, held: "", fail: false, fence, lineStart }; +} + +/** Streaming DSML control-sequence filter for one text or reasoning channel. */ +export class CodeBuddyScaffoldFilter { + private pending = ""; + private failed = false; + private fence: "`" | "~" | null = null; + private lineStart = true; + + /** True while an earlier event owns an unresolved marker or fence prefix. */ + hasPending(): boolean { + return this.pending.length > 0; + } + + push(chunk: string): CodeBuddyScaffoldFilterResult { + if (this.failed) { + return { releasedPending: "", text: "", pendingContinues: false, fail: false }; + } + if (!chunk) { + return { + releasedPending: "", + text: "", + pendingContinues: this.hasPending(), + fail: false, + }; + } + + const priorPending = this.pending; + const result = scan(priorPending + chunk, this.fence, this.lineStart); + this.pending = result.held; + this.fence = result.fence; + this.lineStart = result.lineStart; + this.failed = result.fail; + + const releasedLength = Math.min(priorPending.length, result.safe.length); + return { + releasedPending: result.safe.slice(0, releasedLength), + text: result.safe.slice(releasedLength), + pendingContinues: priorPending.length > 0 && result.safe.length === 0 && result.held.length > 0, + fail: result.fail, + }; + } + + /** Release a suffix that never completed the two-line control grammar. */ + flush(): CodeBuddyScaffoldFilterResult { + if (this.failed) { + return { releasedPending: "", text: "", pendingContinues: false, fail: false }; + } + const text = this.pending; + this.pending = ""; + return { releasedPending: text, text: "", pendingContinues: false, fail: false }; + } +} + +function codeBuddyScaffoldErrorMessage(): string { + return "CodeBuddy CLI emitted vendor tool-call markup in an assistant output channel. This route" + + " runs the CLI with its own tools and MCP servers disabled and Codex owns tool control, so" + + " the turn was refused rather than forwarding or executing vendor agent scaffolding."; +} + +/** Guard both streamed channels while preserving event order around withheld marker prefixes. */ +export function guardCodeBuddyScaffolding(emit: (event: AdapterEvent) => void): (event: AdapterEvent) => void { + const textFilter = new CodeBuddyScaffoldFilter(); + const thinkingFilter = new CodeBuddyScaffoldFilter(); + type PendingChannel = "text" | "thinking"; + type EventSlot = { resolved: boolean; event?: AdapterEvent }; + const eventQueue: EventSlot[] = []; + const pendingSlots = new Map(); + let closed = false; + + const channelEvent = (channel: PendingChannel, text: string): AdapterEvent => channel === "text" + ? { type: "text_delta", text } + : { type: "thinking_delta", thinking: text }; + + const drainResolved = (): void => { + while (eventQueue[0]?.resolved) { + const slot = eventQueue.shift()!; + if (slot.event) emit(slot.event); + } + }; + + const enqueueResolved = (event: AdapterEvent): void => { + eventQueue.push({ resolved: true, event }); + drainResolved(); + }; + + const resolvePendingSlot = (channel: PendingChannel, text: string): void => { + const slot = pendingSlots.get(channel); + if (!slot) return; + slot.resolved = true; + if (text) slot.event = channelEvent(channel, text); + pendingSlots.delete(channel); + drainResolved(); + }; + + const enqueuePendingSlot = (channel: PendingChannel): void => { + const slot: EventSlot = { resolved: false }; + eventQueue.push(slot); + pendingSlots.set(channel, slot); + }; + + const flushAllPending = (): void => { + for (const channel of ["text", "thinking"] as const) { + if (!pendingSlots.has(channel)) continue; + const filter = channel === "text" ? textFilter : thinkingFilter; + resolvePendingSlot(channel, filter.flush().releasedPending); + } + drainResolved(); + }; + + const refuse = (): void => { + if (closed) return; + flushAllPending(); + closed = true; + emit({ + type: "error", + message: codeBuddyScaffoldErrorMessage(), + status: 502, + errorType: "upstream_error", + code: CODEBUDDY_SCAFFOLD_ERROR_CODE, + retryable: false, + }); + }; + + return (event: AdapterEvent): void => { + if (closed) return; + if (event.type === "text_delta" || event.type === "thinking_delta") { + const channel: PendingChannel = event.type === "text_delta" ? "text" : "thinking"; + const filter = channel === "text" ? textFilter : thinkingFilter; + const hadPending = filter.hasPending(); + const cleaned = filter.push(event.type === "text_delta" ? event.text : event.thinking); + if (hadPending && !cleaned.pendingContinues) resolvePendingSlot(channel, cleaned.releasedPending); + if (cleaned.text) { + enqueueResolved(event.type === "text_delta" + ? { ...event, text: cleaned.text } + : { ...event, thinking: cleaned.text }); + } + if (filter.hasPending() && !cleaned.pendingContinues) enqueuePendingSlot(channel); + if (cleaned.fail) refuse(); + return; + } + if (event.type === "done" || event.type === "error" || event.type === "incomplete") { + flushAllPending(); + closed = true; + emit(event); + return; + } + enqueueResolved(event); + }; +} diff --git a/structure/providers/chat-compat.md b/structure/providers/chat-compat.md index 92b19a97a45..270641484da 100644 --- a/structure/providers/chat-compat.md +++ b/structure/providers/chat-compat.md @@ -354,6 +354,10 @@ The shared coding-agent projection (CodeBuddy, Qoder) carries tool-result images real image blocks rather than flattening them to the text `[image]`, and orders image blocks chronologically — history before current — so attachment order matches the prose the model reads beside them. Vendor tool execution stays disabled on both -adapters, and Qoder's explicit refusal of original images is unchanged. +adapters. CodeBuddy refuses an unquoted, line-oriented full-width-bar DSML `calls` +container followed by a `functions.*` invoke control line in either output channel; it +preserves preceding answer text, never promotes vendor prose into execution authority, +and leaves discussed or quoted literals and code examples untouched. Qoder's explicit +refusal of original images is unchanged. Canonical Responses identity sanitation and narrowly scoped pre-output combo recovery follow [request-local target compatibility](../runtime.md#request-local-target-compatibility); other adapter contracts remain unchanged. diff --git a/tests/providers/codebuddy-adapter.test.ts b/tests/providers/codebuddy-adapter.test.ts index 0da898ea829..76caabfae99 100644 --- a/tests/providers/codebuddy-adapter.test.ts +++ b/tests/providers/codebuddy-adapter.test.ts @@ -3,6 +3,7 @@ import { EventEmitter } from "node:events"; import { Readable, Writable } from "node:stream"; import type { ChildProcess } from "node:child_process"; import { buildArgs, buildChildEnv, createCodeBuddyAdapter, type SpawnFn } from "../../src/adapters/codebuddy/adapter"; +import { guardCodeBuddyScaffolding } from "../../src/adapters/codebuddy/scaffold-guard"; import { CODEBUDDY_CN_PROFILE, CODEBUDDY_GLOBAL_PROFILE, clearCodeBuddyBinaryCache } from "../../src/adapters/codebuddy/profiles"; import type { AdapterEvent, OcxParsedRequest, OcxProviderConfig } from "../../src/types"; import { createTestTranslatorBudget } from "../helpers/translator-budget"; @@ -221,6 +222,236 @@ describe("codebuddy runTurn streams a headless turn", () => { expect(child.written.join("")).toContain('"text":"hello"'); }); + test("refuses a full-message DSML calls-and-invoke scaffold", async () => { + const leaked = "I'll inspect it.\n<｜｜DSML｜｜ calls>\n" + + "<｜｜DSML｜｜ invoke name=\"functions.exec\">\nsecret-command"; + const stdout = [ + enc.encode(`${JSON.stringify({ + type: "assistant", + message: { role: "assistant", content: [{ type: "text", text: leaked }] }, + })}\n`), + enc.encode('{"type":"result","subtype":"success","is_error":false}\n'), + ]; + const adapter = createCodeBuddyAdapter(provider(), { + spawn: () => fakeChild(stdout) as unknown as ChildProcess, + which: () => "/usr/bin/codebuddy", + killGraceMs: 20, + }); + + const events = await run(adapter, parsed()); + expect(events.filter(event => event.type === "text_delta")) + .toEqual([{ type: "text_delta", text: "I'll inspect it.\n" }]); + expect(events.some(event => event.type === "done")).toBe(false); + expect(events.at(-1)).toMatchObject({ + type: "error", + code: "vendor_scaffold_detected", + retryable: false, + status: 502, + }); + expect(JSON.stringify(events)).not.toContain("secret-command"); + }); + + test("detects a DSML control sequence split across streamed text deltas", async () => { + const frame = (text: string) => `${JSON.stringify({ + type: "stream_event", + event: { type: "content_block_delta", delta: { type: "text_delta", text } }, + })}\n`; + const stdout = [ + enc.encode(frame("Safe prefix.\n<｜｜DS")), + enc.encode(frame("ML｜｜ calls>\n<｜｜DSML｜｜ invoke name=\"funct")), + enc.encode(frame("ions.exec\">private-body")), + enc.encode('{"type":"result","subtype":"success","is_error":false}\n'), + ]; + const adapter = createCodeBuddyAdapter(provider(), { + spawn: () => fakeChild(stdout) as unknown as ChildProcess, + which: () => "/usr/bin/codebuddy", + killGraceMs: 20, + }); + + const events = await run(adapter, parsed()); + expect(events.filter(event => event.type === "text_delta")) + .toEqual([{ type: "text_delta", text: "Safe prefix.\n" }]); + expect(events.at(-1)).toMatchObject({ type: "error", code: "vendor_scaffold_detected" }); + expect(events.some(event => event.type === "done")).toBe(false); + expect(JSON.stringify(events)).not.toContain("private-body"); + }); + + test("refuses DSML calls-and-invoke scaffolding from reasoning independently", async () => { + const stdout = [ + enc.encode(`${JSON.stringify({ + type: "stream_event", + event: { + type: "content_block_delta", + delta: { + type: "thinking_delta", + thinking: "Safe thought.\n<｜｜DSML｜｜ calls>\n" + + "<｜｜DSML｜｜ invoke name=\"functions.exec\">private-body", + }, + }, + })}\n`), + enc.encode('{"type":"result","subtype":"success","is_error":false}\n'), + ]; + const adapter = createCodeBuddyAdapter(provider(), { + spawn: () => fakeChild(stdout) as unknown as ChildProcess, + which: () => "/usr/bin/codebuddy", + killGraceMs: 20, + }); + + const events = await run(adapter, parsed()); + expect(events.filter(event => event.type === "thinking_delta")) + .toEqual([{ type: "thinking_delta", thinking: "Safe thought.\n" }]); + expect(events.at(-1)).toMatchObject({ + type: "error", + code: "vendor_scaffold_detected", + retryable: false, + }); + expect(events.some(event => event.type === "done")).toBe(false); + expect(JSON.stringify(events)).not.toContain("private-body"); + }); + + test("delivers a lone discussed DSML calls tag unchanged", () => { + const events: AdapterEvent[] = []; + const guarded = guardCodeBuddyScaffolding(event => events.push(event)); + const answer = "The string <｜｜DSML｜｜ calls> names the calls container."; + + guarded({ type: "text_delta", text: answer }); + guarded({ type: "done", stopReason: "stop" }); + + expect(events).toEqual([ + { type: "text_delta", text: answer }, + { type: "done", stopReason: "stop" }, + ]); + }); + + test("delivers quoted and inline-code DSML literals unchanged", () => { + const events: AdapterEvent[] = []; + const guarded = guardCodeBuddyScaffolding(event => events.push(event)); + const answer = "\"<｜｜DSML｜｜ calls>\"\n" + + "\"<｜｜DSML｜｜ invoke name=\\\"functions.exec\\\">\"\n" + + "Use `<｜｜DSML｜｜ calls>` when discussing the literal.\n" + + "> <｜｜DSML｜｜ calls>\n> <｜｜DSML｜｜ invoke name=\"functions.exec\">"; + + guarded({ type: "text_delta", text: answer }); + guarded({ type: "done", stopReason: "stop" }); + + expect(events).toEqual([ + { type: "text_delta", text: answer }, + { type: "done", stopReason: "stop" }, + ]); + }); + + test("delivers a fenced DSML source example unchanged across deltas", () => { + const events: AdapterEvent[] = []; + const guarded = guardCodeBuddyScaffolding(event => events.push(event)); + const first = "```text\n<｜｜DSML｜｜ calls>\n"; + const second = "<｜｜DSML｜｜ invoke name=\"functions.exec\">\n```"; + + guarded({ type: "text_delta", text: first }); + guarded({ type: "text_delta", text: second }); + guarded({ type: "done", stopReason: "stop" }); + + expect(events).toEqual([ + { type: "text_delta", text: first }, + { type: "text_delta", text: second }, + { type: "done", stopReason: "stop" }, + ]); + }); + + test("delivers source strings containing both DSML literals unchanged", () => { + const events: AdapterEvent[] = []; + const guarded = guardCodeBuddyScaffolding(event => events.push(event)); + const answer = "const calls = '<｜｜DSML｜｜ calls>';\n" + + "const invoke = '<｜｜DSML｜｜ invoke name=\"functions.exec\">';"; + + guarded({ type: "text_delta", text: answer }); + guarded({ type: "done", stopReason: "stop" }); + + expect(events).toEqual([ + { type: "text_delta", text: answer }, + { type: "done", stopReason: "stop" }, + ]); + }); + + test("delivers an unquoted invoke line when no calls container precedes it", () => { + const events: AdapterEvent[] = []; + const guarded = guardCodeBuddyScaffolding(event => events.push(event)); + const answer = "<｜｜DSML｜｜ invoke name=\"functions.exec\">"; + + guarded({ type: "text_delta", text: answer }); + guarded({ type: "done", stopReason: "stop" }); + + expect(events).toEqual([ + { type: "text_delta", text: answer }, + { type: "done", stopReason: "stop" }, + ]); + }); + + test("releases a lone control-line candidate at the terminal", () => { + const events: AdapterEvent[] = []; + const guarded = guardCodeBuddyScaffolding(event => events.push(event)); + + guarded({ type: "text_delta", text: "<｜｜DSML｜｜ calls>" }); + expect(events).toEqual([]); + guarded({ type: "done", stopReason: "stop" }); + + expect(events).toEqual([ + { type: "text_delta", text: "<｜｜DSML｜｜ calls>" }, + { type: "done", stopReason: "stop" }, + ]); + }); + + test("queues later events behind an unresolved marker prefix", () => { + const events: AdapterEvent[] = []; + const guarded = guardCodeBuddyScaffolding(event => events.push(event)); + + guarded({ type: "thinking_delta", thinking: "<" }); + guarded({ type: "text_delta", text: "Hello" }); + guarded({ type: "tool_call_start", id: "call_1", name: "exec" }); + expect(events).toEqual([]); + guarded({ type: "done", stopReason: "stop" }); + + expect(events).toEqual([ + { type: "thinking_delta", thinking: "<" }, + { type: "text_delta", text: "Hello" }, + { type: "tool_call_start", id: "call_1", name: "exec" }, + { type: "done", stopReason: "stop" }, + ]); + }); + + test("keeps an existing pending slot when its channel receives an empty delta", () => { + const events: AdapterEvent[] = []; + const guarded = guardCodeBuddyScaffolding(event => events.push(event)); + + guarded({ type: "thinking_delta", thinking: "<" }); + guarded({ type: "text_delta", text: "Hello" }); + guarded({ type: "thinking_delta", thinking: "" }); + guarded({ type: "done", stopReason: "stop" }); + + expect(events).toEqual([ + { type: "thinking_delta", thinking: "<" }, + { type: "text_delta", text: "Hello" }, + { type: "done", stopReason: "stop" }, + ]); + }); + + test("moves a replaced pending marker prefix to its new arrival position", () => { + const events: AdapterEvent[] = []; + const guarded = guardCodeBuddyScaffolding(event => events.push(event)); + + guarded({ type: "thinking_delta", thinking: "<" }); + guarded({ type: "text_delta", text: "<" }); + guarded({ type: "thinking_delta", thinking: "not marker\n<" }); + guarded({ type: "done", stopReason: "stop" }); + + expect(events).toEqual([ + { type: "thinking_delta", thinking: "<" }, + { type: "text_delta", text: "<" }, + { type: "thinking_delta", thinking: "not marker\n" }, + { type: "thinking_delta", thinking: "<" }, + { type: "done", stopReason: "stop" }, + ]); + }); + test("region isolation: the global adapter never spawns with the CN environment", async () => { let seenEnv: NodeJS.ProcessEnv | undefined; const spawn: SpawnFn = (_cmd, _args, opts) => { seenEnv = opts.env as NodeJS.ProcessEnv; return fakeChild([enc.encode('{"type":"result","subtype":"success"}\n')]) as unknown as ChildProcess; }; From 99c977d7bed1ffb4fc707d151a8da7040100bb57 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 16 Sep 2026 11:34:09 +0900 Subject: [PATCH 4/4] fix(openai-chat): bound flattened tool wire names for strict gateways (#4679) [skip ci] Command Code's gateway rejects a request outright with 400 name must be at most 64 characters, got 66. Codex Desktop built-in app tools flatten to __ past that bound, a user cannot exclude them, and Responses-Lite catalogs bundle every declared tool, so the surface cannot be shrunk from configuration. The bound belongs to the adapter, not to the shared name helper. Three adapters already solve this for themselves: Kiro normalizes to its own charset with a deterministic 8-hex suffix, Google compiles and restores names in its wire compiler, and Meta Muse aliases names on api.meta.ai. The translated openai-chat path is the only one with no answer, and it is the path Command Code uses. A request-scoped registry now owns one collision domain per translated Chat Completions request, following Kiro's shape. A namespaced name whose flattened spelling exceeds 64 characters becomes a charset-safe alias derived purely from the native identity, so it is stable across processes, catalog order and catalog membership. Declarations, replayed assistant tool calls and tool_choice all pass through the same registry, and both the streaming and buffered parsers restore the echoed alias before tool_call_start, so the existing bridge map still hands the client its native {namespace, name}. The registry is seeded from the union of the current catalog and the structured tool calls still present in replay history, because a historical call can keep its namespace without being redeclared; seeding from the catalog alone would let exactly the reported over-limit name reach the gateway again on a later turn. Nothing else changes. Names at or under 64 characters and bare names are byte-identical on the wire, and Kiro, Google and Muse still receive the raw flattened name and run their own normalization. 64 is the Chat Completions function-name limit and a strict-gateway compatibility concern, not OpenAI Responses parity: upstream Codex raised its own MCP ceiling to 128 bytes in openai/codex#39594 because native Responses accepts 128. Applying it on this wire is correct for that wire alone. Carried from #4715. That PR placed the bound in the shared namespacedToolName helper and was provisionally accepted there. Hosted CI then showed twice that the shared point intercepts adapters which already had an answer: it broke Google's wire-compiler restore, and after that was narrowed it broke Kiro's normalizer. The problem statement and issue analysis are the original author's; only the placement changed. Co-authored-by: Hulian Buligon <205309211+HulianBuligon@users.noreply.github.com> --- scripts/test-layout/layout.json | 1 + src/adapters/openai-chat.ts | 16 +- .../openai-chat/tool-name-registry.ts | 166 ++++++++++ src/adapters/openai-chat/tool-schema.ts | 32 +- structure/providers/chat-compat.md | 18 +- .../openai-chat-bounded-tool-names.test.ts | 301 ++++++++++++++++++ tests/fixtures/test-layout-expected.json | 1 + 7 files changed, 516 insertions(+), 19 deletions(-) create mode 100644 src/adapters/openai-chat/tool-name-registry.ts create mode 100644 tests/adapters/openai/openai-chat-bounded-tool-names.test.ts diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 7e9cc529597..c8cf88898ec 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -990,6 +990,7 @@ "omp-path-contract.test.ts": "clients", "omp-yaml-source-inline-comments.test.ts": "clients", "openai-api-virtual-models.test.ts": "adapters/openai", + "openai-chat-bounded-tool-names.test.ts": "adapters/openai", "openai-chat-dangling-toolcalls.test.ts": "adapters/openai", "openai-chat-eof.test.ts": "adapters/openai", "openai-chat-hardening.test.ts": "adapters/openai", diff --git a/src/adapters/openai-chat.ts b/src/adapters/openai-chat.ts index 8503210e465..d74640a3e23 100644 --- a/src/adapters/openai-chat.ts +++ b/src/adapters/openai-chat.ts @@ -6,7 +6,6 @@ import { mapReasoningEffort, modelRecordValue } from "../reasoning-effort"; import { debugProviderDiagnostic } from "../lib/debug"; import { sseFieldValue } from "../lib/sse-decoder"; import { isDebugEnabled } from "../lib/debug-settings"; -import { frameAgentRouterMessages } from "./agentrouter"; import { openRouterProviderPayload, resolveOpenRouterRouting } from "../providers/openrouter-routing"; import { resolveVercelGatewayRouting, vercelGatewayProviderPayload } from "../providers/vercel-gateway-routing"; import { fastPolicyForModel } from "../providers/service-tier"; @@ -39,6 +38,7 @@ import { upstreamErrorEvent, } from "./openai-chat/errors"; import { messagesToChatFormat } from "./openai-chat/messages"; +import { withOpenAIChatToolNames } from "./openai-chat/tool-name-registry"; import { isNativeOpenAIChatTarget, openAIChatTransport, stripBracketedModelSuffix } from "./openai-chat/wire"; import { toolChoiceToChatFormat, toolsToChatFormatForProvider } from "./openai-chat/tool-schema"; @@ -88,7 +88,7 @@ function canSerializeOpenAIChatServiceTier( export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAdapter { let lastRequestedModelId: string | undefined; - return { + return withOpenAIChatToolNames(toolNames => ({ name: "openai-chat", formatErrorBody: formatOpenAIChatErrorBody, @@ -96,10 +96,10 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd buildRequest(parsed: OcxParsedRequest, incoming?: IncomingMeta) { lastRequestedModelId = parsed.modelId; const { url, headers, hasCredential } = openAIChatTransport(provider); - const messages = frameAgentRouterMessages(provider.baseUrl, messagesToChatFormat(parsed, provider)); + const messages = toolNames.messages(parsed, provider.baseUrl, messagesToChatFormat(parsed, provider)); const finish = (): AdapterRequest => { - const tools = toolsToChatFormatForProvider(parsed, provider); - const toolChoice = toolChoiceToChatFormat(parsed.options.toolChoice, parsed.context.tools, provider); + const tools = toolsToChatFormatForProvider(parsed, provider, toolNames.registry()); + const toolChoice = toolChoiceToChatFormat(parsed.options.toolChoice, parsed.context.tools, provider, toolNames.registry()); const body: Record = { model: provider.modelSuffixBracketStrip ? stripBracketedModelSuffix(parsed.modelId) : parsed.modelId, @@ -365,7 +365,7 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd return "terminate"; } if (!call.id) call.id = `call_${++toolCallSeq}`; - yield { type: "tool_call_start", id: call.id, name: call.name }; + yield { type: "tool_call_start", id: call.id, name: toolNames.restore(call.name) }; if (call.args.length > 0) yield { type: "tool_call_delta", arguments: call.args }; yield { type: "tool_call_end" }; } @@ -801,7 +801,7 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd logInvalidToolCalls("response", rawToolCalls); return [invalidToolCallsEvent(rawToolCalls, "response", usage)]; } - events.push({ type: "tool_call_start", id, name }); + events.push({ type: "tool_call_start", id, name: toolNames.restore(name) }); events.push({ type: "tool_call_delta", arguments: args }); events.push({ type: "tool_call_end" }); } @@ -818,5 +818,5 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd budget.releaseRetained(responseBytes, { kind: "retained_collectors" }); } }, - }; + })); } diff --git a/src/adapters/openai-chat/tool-name-registry.ts b/src/adapters/openai-chat/tool-name-registry.ts new file mode 100644 index 00000000000..c9c8e1fa5ff --- /dev/null +++ b/src/adapters/openai-chat/tool-name-registry.ts @@ -0,0 +1,166 @@ +import { createHash } from "node:crypto"; +import { namespacedToolName, type OcxParsedRequest, type OcxTool } from "../../types"; +import { frameAgentRouterMessages } from "../agentrouter"; + +const MAX_CHAT_TOOL_NAME_LENGTH = 64; +const ALIAS_HINT_CHARS = 16; +const RESERVED_ALIAS_PATTERN = /^ocx_[a-zA-Z0-9_-]{16}_[a-zA-Z0-9_-]{43}$/; +type ToolIdentity = Readonly>; + +export interface OpenAIChatToolNameRegistry { + alias(tool: ToolIdentity): string; + aliasWireName(wireName: string): string; + restore(wireName: string): string; +} + +interface OpenAIChatToolNameScope { + messages(parsed: OcxParsedRequest, baseUrl: string, messages: readonly unknown[]): unknown; + registry(): OpenAIChatToolNameRegistry; + restore(wireName: string): string; +} + +function identityKey(tool: ToolIdentity): string { + return JSON.stringify([tool.namespace ?? null, tool.name]); +} + +function boundedAlias(tool: ToolIdentity, wireName: string): string { + const hint = wireName + .replace(/[^a-zA-Z0-9_-]/g, "_") + .slice(-ALIAS_HINT_CHARS) + .padStart(ALIAS_HINT_CHARS, "_"); + const key = identityKey(tool); + const digest = createHash("sha256") + .update(key) + .digest("base64url"); + return `ocx_${hint}_${digest}`; +} + +/** Catalog declarations plus structured calls retained in replay history. */ +export function openAIChatToolNameIdentities(parsed: OcxParsedRequest): ToolIdentity[] { + const identities: ToolIdentity[] = [...(parsed.context.tools ?? [])]; + for (const message of parsed.context.messages) { + if (message.role !== "assistant" || !Array.isArray(message.content)) continue; + for (const part of message.content) { + if (part.type !== "toolCall") continue; + identities.push({ + name: part.name, + ...(part.namespace === undefined ? {} : { namespace: part.namespace }), + }); + } + } + return identities; +} + +/** + * One collision domain for a translated Chat Completions request. + * + * Namespaced names whose flattened spelling exceeds Chat Completions' 64-character function-name + * bound are rewritten. Ordinary names and bare names pass through byte-for-byte unless they occupy + * the reserved alias spelling; those are re-aliased so no declaration can shadow another identity's + * deterministic alias. Distinct identities sharing one flattened spelling each keep an identity + * alias, while replay rewriting leaves that ambiguous spelling untouched. Echoed aliases restore to + * the original flattened name consumed by the Responses bridge's existing namespace map. + */ +export function createOpenAIChatToolNameRegistry( + tools: readonly ToolIdentity[] | undefined, +): OpenAIChatToolNameRegistry { + const identities = new Map(); + for (const tool of tools ?? []) identities.set(identityKey(tool), tool); + const sortedIdentities = [...identities.entries()] + .sort(([left], [right]) => left < right ? -1 : left > right ? 1 : 0); + + const aliasesByIdentity = new Map(); + const aliasesByWireName = new Map(); + const originalsByAlias = new Map(); + const wireOwners = new Map(); + for (const [key, tool] of sortedIdentities) { + const wireName = namespacedToolName(tool.namespace, tool.name); + const owner = wireOwners.get(wireName); + if (owner === undefined) wireOwners.set(wireName, key); + else if (owner !== key) wireOwners.set(wireName, null); + } + + const aliasOwners = new Map(); + const wireClaims = new Map(); + for (const [key, tool] of sortedIdentities) { + const wireName = namespacedToolName(tool.namespace, tool.name); + const candidate = (tool.namespace !== undefined && wireName.length > MAX_CHAT_TOOL_NAME_LENGTH) + || RESERVED_ALIAS_PATTERN.test(wireName) + || wireOwners.get(wireName) === null + ? boundedAlias(tool, wireName) + : wireName; + // A full SHA-256 collision is not safely attributable. Keep the later identity's native + // spelling instead of failing the request or stealing the first identity's restore entry. + const alias = aliasOwners.has(candidate) ? wireName : candidate; + aliasesByIdentity.set(key, alias); + if (!aliasOwners.has(alias)) aliasOwners.set(alias, key); + if (alias !== wireName) originalsByAlias.set(alias, wireName); + + const claim = wireClaims.get(wireName); + if (claim === undefined) wireClaims.set(wireName, { key, alias }); + else if (claim !== null && claim.key !== key) wireClaims.set(wireName, null); + } + for (const [wireName, claim] of wireClaims) { + if (claim !== null) aliasesByWireName.set(wireName, claim.alias); + } + + return { + alias(tool: ToolIdentity): string { + const key = identityKey(tool); + const known = aliasesByIdentity.get(key); + if (known !== undefined) return known; + return namespacedToolName(tool.namespace, tool.name); + }, + aliasWireName(wireName: string): string { + return aliasesByWireName.get(wireName) ?? wireName; + }, + restore(wireName: string): string { + return originalsByAlias.get(wireName) ?? wireName; + }, + }; +} + +export function restoreOpenAIChatToolName( + registry: OpenAIChatToolNameRegistry, + wireName: string, +): string { + return registry.restore(wireName); +} + +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +/** Rewrite replayed assistant tool calls after the ordinary message converter has flattened them. */ +export function aliasOpenAIChatMessageToolNames( + messages: readonly unknown[], + registry: OpenAIChatToolNameRegistry, +): unknown[] { + return messages.map(message => { + if (!isRecord(message) || !Array.isArray(message.tool_calls)) return message; + let changed = false; + const toolCalls = message.tool_calls.map(toolCall => { + if (!isRecord(toolCall) || !isRecord(toolCall.function) + || typeof toolCall.function.name !== "string") return toolCall; + const name = registry.aliasWireName(toolCall.function.name); + if (name === toolCall.function.name) return toolCall; + changed = true; + return { ...toolCall, function: { ...toolCall.function, name } }; + }); + return changed ? { ...message, tool_calls: toolCalls } : message; + }); +} + +export function withOpenAIChatToolNames( + build: (scope: OpenAIChatToolNameScope) => T, +): T { + let registry = createOpenAIChatToolNameRegistry(undefined); + return build({ + messages(parsed, baseUrl, messages): unknown { + registry = createOpenAIChatToolNameRegistry(openAIChatToolNameIdentities(parsed)); + return frameAgentRouterMessages(baseUrl, aliasOpenAIChatMessageToolNames(messages, registry)); + }, + registry: () => registry, + restore: wireName => restoreOpenAIChatToolName(registry, wireName), + }); +} diff --git a/src/adapters/openai-chat/tool-schema.ts b/src/adapters/openai-chat/tool-schema.ts index c056a6a043a..23f77121bd5 100644 --- a/src/adapters/openai-chat/tool-schema.ts +++ b/src/adapters/openai-chat/tool-schema.ts @@ -1,7 +1,8 @@ import { isNativeOpenAIChatTarget } from "./wire"; +import { createOpenAIChatToolNameRegistry, type OpenAIChatToolNameRegistry } from "./tool-name-registry"; import { isXaiSchemaTarget, lookupLocalJsonPointer, normalizeXaiToolParameters } from "../xai-tool-schema"; import { stripResponsesOnlyEncryptedMarker, stripUnicodePropertyPatterns } from "../responses-tool-schema"; -import { isAllowedToolChoice, namespacedToolName, resolveToolChoiceWireName, toolChoiceToolPredicate } from "../../types"; +import { isAllowedToolChoice, resolveToolChoiceWireName, toolChoiceToolPredicate } from "../../types"; import type { OcxParsedRequest, OcxProviderConfig } from "../../types"; const ZEN_SCHEMA_MAP_KEYS = new Set(["properties", "$defs", "definitions"]); @@ -409,7 +410,11 @@ function normalizeMoonshotToolParameters(parameters: unknown): Record 0 ? formatted : undefined; } -export function toolsToChatFormatForProvider(parsed: OcxParsedRequest, provider: OcxProviderConfig): unknown[] | undefined { - const base = toolsToChatFormat(parsed, provider); +export function toolsToChatFormatForProvider( + parsed: OcxParsedRequest, + provider: OcxProviderConfig, + registry: OpenAIChatToolNameRegistry = createOpenAIChatToolNameRegistry(parsed.context.tools), +): unknown[] | undefined { + const base = toolsToChatFormat(parsed, provider, registry); const azureChat = isAzureOpenAiChatTarget(provider); const zenChat = shouldSanitizeZenToolParameters(provider); if (!base || (!zenChat && !azureChat)) return base; @@ -463,15 +472,24 @@ export function toolChoiceToChatFormat( tc: OcxParsedRequest["options"]["toolChoice"], tools: OcxParsedRequest["context"]["tools"], provider: OcxProviderConfig, + registry: OpenAIChatToolNameRegistry = createOpenAIChatToolNameRegistry(tools), ): unknown { if (!tc) return undefined; if (isAllowedToolChoice(tc)) { if (tc.mode === "required" && tc.allowedTools.length === 1 && isNativeOpenAIChatTarget(provider)) { - return { type: "function", function: { name: resolveToolChoiceWireName(tools, tc.allowedTools[0]) } }; + return { + type: "function", + function: { name: registry.aliasWireName(resolveToolChoiceWireName(tools, tc.allowedTools[0])) }, + }; } return tc.mode === "required" ? "required" : "auto"; } if (tc === "auto" || tc === "none" || tc === "required") return tc; - if ("name" in tc) return { type: "function", function: { name: resolveToolChoiceWireName(tools, tc.name) } }; + if ("name" in tc) { + return { + type: "function", + function: { name: registry.aliasWireName(resolveToolChoiceWireName(tools, tc.name)) }, + }; + } return undefined; } diff --git a/structure/providers/chat-compat.md b/structure/providers/chat-compat.md index b2eaadee795..92b19a97a45 100644 --- a/structure/providers/chat-compat.md +++ b/structure/providers/chat-compat.md @@ -62,10 +62,20 @@ request-local alias. Raw API-key continuations deliberately preserve ids because continuation may reference a call stored upstream under its original id; proxy-expanded API-key replays are explicit and receive the same repair. -Separately, Meta Muse Responses (`src/responses/muse-tool-name-alias.ts`) aliases function *tool -names* that exceed 64 characters or contain characters outside `[a-zA-Z0-9_-]` on `api.meta.ai` -only. That map is not the call-id repair: it covers tools, `additional_tools`, history calls, and -`tool_choice`, then restores original names inbound. +Tool-name normalization stays adapter-scoped. The translated Chat Completions path uses a +request-scoped registry in `src/adapters/openai-chat/`: only flattened namespaced names over 64 +characters receive a deterministic, charset-safe alias. Catalog declarations, replayed calls and +`tool_choice` share that registry, and streamed or buffered echoes restore to the original flattened +name before the Responses bridge restores `{namespace, name}`. Names at or below the bound and bare +names pass through unchanged, except declarations matching the reserved alias shape; those are +re-aliased so they cannot shadow an identity-derived alias. + +The 64-character bound is a Chat Completions and strict-gateway compatibility concern: Command Code +rejects a 66-character function name (#4679). Upstream Codex raised its own MCP ceiling to 128 bytes +in `openai/codex#39594` because native Responses accepts 128, so that Responses limit does not govern +this translated wire. Kiro (`src/adapters/kiro-tools.ts`), Google (its wire compiler), and Meta Muse +Responses (`src/responses/muse-tool-name-alias.ts`, gated to `api.meta.ai`) each retain their own +equivalent normalization and restoration. These compatibility guards are covered by focused tests and should stay close to the adapters that need them. diff --git a/tests/adapters/openai/openai-chat-bounded-tool-names.test.ts b/tests/adapters/openai/openai-chat-bounded-tool-names.test.ts new file mode 100644 index 00000000000..f7cdc756a39 --- /dev/null +++ b/tests/adapters/openai/openai-chat-bounded-tool-names.test.ts @@ -0,0 +1,301 @@ +import { describe, expect, test } from "bun:test"; +import { createOpenAIChatAdapter } from "../../../src/adapters/openai-chat"; +import { createOpenAIChatToolNameRegistry } from "../../../src/adapters/openai-chat/tool-name-registry"; +import { compileGoogleWireBody } from "../../../src/adapters/google-wire-compiler"; +import { kiroToolName } from "../../../src/adapters/kiro-wire"; +import { buildResponseJSON } from "../../../src/bridge"; +import { parseRequest } from "../../../src/responses/parser"; +import { buildToolBridgeMaps } from "../../../src/server/responses"; +import type { AdapterEvent, OcxParsedRequest, OcxProviderConfig, OcxTool } from "../../../src/types"; +import { namespacedToolName } from "../../../src/types/tools"; +import { createTestTranslatorBudget } from "../../helpers/translator-budget"; + +const LONG_NAMESPACE = "mcp__codex_apps__codex_document_control"; +const REPORTED_NAME = "execute_document_command"; +const OTHER_LONG_NAME = "get_document_tool_schemas"; + +function provider(): OcxProviderConfig { + return { + adapter: "openai-chat", + baseUrl: "https://example.test/v1", + apiKey: "sk-test", + authMode: "key", + }; +} + +function tool(namespace: string | undefined, name: string): OcxTool { + return { namespace, name, description: "Test tool", parameters: { type: "object" } }; +} + +function parsedWith( + tools: OcxTool[], + options: OcxParsedRequest["options"] = {}, + messages: OcxParsedRequest["context"]["messages"] = [{ role: "user", content: "Use the tool", timestamp: 0 }], +): OcxParsedRequest { + return { modelId: "test-model", stream: false, options, context: { tools, messages } }; +} + +describe("bounded OpenAI Chat tool wire names (#4679)", () => { + test("bounds and restores the exact reported identity across request, replay, tool_choice, and response", async () => { + const declared = tool(LONG_NAMESPACE, REPORTED_NAME); + const originalWireName = namespacedToolName(LONG_NAMESPACE, REPORTED_NAME); + const replayMessages: OcxParsedRequest["context"]["messages"] = [ + { + role: "assistant", + content: [{ + type: "toolCall", + id: "call_replay", + namespace: LONG_NAMESPACE, + name: REPORTED_NAME, + arguments: {}, + }], + timestamp: 0, + }, + { role: "toolResult", toolCallId: "call_replay", toolName: REPORTED_NAME, content: "ok", timestamp: 1 }, + { role: "user", content: "Run it again", timestamp: 2 }, + ]; + const parsed = parsedWith([declared], { toolChoice: { name: REPORTED_NAME } }, replayMessages); + const adapter = createOpenAIChatAdapter(provider()); + const request = adapter.buildRequest(parsed, { + headers: new Headers(), + translatorBudget: createTestTranslatorBudget(), + }); + if (request instanceof Promise) throw new Error("OpenAI Chat request unexpectedly became async"); + const body = JSON.parse(request.body) as { + tools: Array<{ function: { name: string } }>; + messages: Array<{ tool_calls?: Array<{ function: { name: string } }> }>; + tool_choice: { function: { name: string } }; + }; + const alias = body.tools[0].function.name; + + expect(new TextEncoder().encode(originalWireName).byteLength).toBeGreaterThan(64); + expect(alias).not.toBe(originalWireName); + expect(alias).toMatch(/^[a-zA-Z0-9_-]{1,64}$/); + expect(new TextEncoder().encode(alias).byteLength).toBeLessThanOrEqual(64); + expect(body.messages.find(message => message.tool_calls)?.tool_calls?.[0].function.name).toBe(alias); + expect(body.tool_choice.function.name).toBe(alias); + + const events = await adapter.parseResponse!(new Response(JSON.stringify({ + choices: [{ + message: { tool_calls: [{ id: "call_echo", function: { name: alias, arguments: "{}" } }] }, + finish_reason: "tool_calls", + }], + })), createTestTranslatorBudget()); + expect(events.find(event => event.type === "tool_call_start")).toMatchObject({ + type: "tool_call_start", + id: "call_echo", + name: originalWireName, + }); + + const streamed: AdapterEvent[] = []; + const streamBody = `data: ${JSON.stringify({ + choices: [{ + delta: { tool_calls: [{ index: 0, id: "call_stream", function: { name: alias, arguments: "{}" } }] }, + finish_reason: "tool_calls", + }], + })}\n\ndata: [DONE]\n\n`; + for await (const event of adapter.parseStream( + new Response(streamBody), + createTestTranslatorBudget(), + )) streamed.push(event); + expect(streamed.find(event => event.type === "tool_call_start")).toMatchObject({ + type: "tool_call_start", + id: "call_stream", + name: originalWireName, + }); + + const responseRequest = parseRequest({ + model: "test-model", + input: "Use the tool", + tools: [{ + type: "namespace", + name: LONG_NAMESPACE, + tools: [{ type: "function", name: REPORTED_NAME, parameters: { type: "object" } }], + }], + }); + const maps = buildToolBridgeMaps(responseRequest); + const bridged = buildResponseJSON(events, "test-model", maps); + const call = (bridged.output as Record[])[0]; + if (!call) throw new Error("Expected a bridged function call"); + expect(call).toMatchObject({ + type: "function_call", + namespace: LONG_NAMESPACE, + name: REPORTED_NAME, + }); + + const replayed = parseRequest({ + model: "test-model", + tools: [{ + type: "namespace", + name: LONG_NAMESPACE, + tools: [{ type: "function", name: REPORTED_NAME, parameters: { type: "object" } }], + }], + input: [call], + }); + const replayedCall = replayed.context.messages + .flatMap(message => Array.isArray(message.content) ? message.content : []) + .find(part => part.type === "toolCall"); + expect(replayedCall).toMatchObject({ namespace: LONG_NAMESPACE, name: REPORTED_NAME }); + }); + + test("leaves names at or under 64 characters and ordinary bare names byte-identical", () => { + const exactly64 = tool("n".repeat(30), "x".repeat(32)); + const ordinary = tool("mcp__short", "read"); + const longBare = tool(undefined, "b".repeat(200)); + const registry = createOpenAIChatToolNameRegistry([exactly64, ordinary, longBare]); + + expect(new TextEncoder().encode(namespacedToolName(exactly64.namespace, exactly64.name)).byteLength).toBe(64); + expect(registry.alias(exactly64)).toBe(namespacedToolName(exactly64.namespace, exactly64.name)); + expect(registry.alias(ordinary)).toBe(namespacedToolName(ordinary.namespace, ordinary.name)); + expect(registry.alias(longBare)).toBe(longBare.name); + expect(namespacedToolName(undefined, longBare.name)).toBe(longBare.name); + + const adapter = createOpenAIChatAdapter(provider()); + const request = adapter.buildRequest(parsedWith([exactly64, ordinary, longBare]), { + headers: new Headers(), + translatorBudget: createTestTranslatorBudget(), + }); + if (request instanceof Promise) throw new Error("OpenAI Chat request unexpectedly became async"); + const body = JSON.parse(request.body) as { tools: Array<{ function: { name: string } }> }; + expect(body.tools.map(entry => entry.function.name)).toEqual([ + namespacedToolName(exactly64.namespace, exactly64.name), + namespacedToolName(ordinary.namespace, ordinary.name), + longBare.name, + ]); + }); + + test("bounds and restores a replay-only historical call absent from the current catalog", async () => { + const originalWireName = namespacedToolName(LONG_NAMESPACE, REPORTED_NAME); + const replayed = parseRequest({ + model: "test-model", + tools: [], + input: [ + { + type: "function_call", + call_id: "call_historical", + namespace: LONG_NAMESPACE, + name: REPORTED_NAME, + arguments: "{}", + }, + { type: "function_call_output", call_id: "call_historical", output: "ok" }, + { role: "user", content: "Continue" }, + ], + }); + expect(replayed.context.tools ?? []).toHaveLength(0); + + const adapter = createOpenAIChatAdapter(provider()); + const request = adapter.buildRequest(replayed, { + headers: new Headers(), + translatorBudget: createTestTranslatorBudget(), + }); + if (request instanceof Promise) throw new Error("OpenAI Chat request unexpectedly became async"); + const body = JSON.parse(request.body) as { + tools?: unknown; + messages: Array<{ tool_calls?: Array<{ function: { name: string } }> }>; + }; + const alias = body.messages.find(message => message.tool_calls)?.tool_calls?.[0].function.name; + if (!alias) throw new Error("Expected the historical tool call on replay"); + + expect(body.tools).toBeUndefined(); + expect(alias).not.toBe(originalWireName); + expect(alias).toMatch(/^[a-zA-Z0-9_-]{1,64}$/); + + const events = await adapter.parseResponse!(new Response(JSON.stringify({ + choices: [{ + message: { tool_calls: [{ id: "call_echo", function: { name: alias, arguments: "{}" } }] }, + finish_reason: "tool_calls", + }], + })), createTestTranslatorBudget()); + expect(events.find(event => event.type === "tool_call_start")).toMatchObject({ + type: "tool_call_start", + name: originalWireName, + }); + }); + + test("derives deterministic distinct aliases independent of catalog order", () => { + const catalog = [ + tool(LONG_NAMESPACE, REPORTED_NAME), + tool(LONG_NAMESPACE, OTHER_LONG_NAME), + tool(`${LONG_NAMESPACE}_other`, REPORTED_NAME), + ]; + const forward = createOpenAIChatToolNameRegistry(catalog); + const reverse = createOpenAIChatToolNameRegistry([...catalog].reverse()); + const forwardAliases = catalog.map(entry => forward.alias(entry)); + + expect(catalog.map(entry => reverse.alias(entry))).toEqual(forwardAliases); + expect(new Set(forwardAliases).size).toBe(catalog.length); + for (const alias of forwardAliases) expect(alias).toMatch(/^[a-zA-Z0-9_-]{1,64}$/); + + const longTool = catalog[0]!; + const identityAlias = createOpenAIChatToolNameRegistry([longTool]).alias(longTool); + const aliasShapedBareTool = tool(undefined, identityAlias); + const collisionCatalog = [longTool, aliasShapedBareTool]; + const collisionRegistry = createOpenAIChatToolNameRegistry(collisionCatalog); + const reversedCollisionRegistry = createOpenAIChatToolNameRegistry([...collisionCatalog].reverse()); + const reservedNameAlias = collisionRegistry.alias(aliasShapedBareTool); + expect(collisionRegistry.alias(longTool)).toBe(identityAlias); + expect(reversedCollisionRegistry.alias(longTool)).toBe(identityAlias); + expect(reservedNameAlias).not.toBe(identityAlias); + expect(reservedNameAlias).toMatch(/^ocx_[a-zA-Z0-9_-]{16}_[a-zA-Z0-9_-]{43}$/); + expect(collisionRegistry.restore(reservedNameAlias)).toBe(identityAlias); + }); + + test("aliases colliding identities but leaves their ambiguous replay spelling unchanged", () => { + const first = tool("a__b", "c"); + const second = tool("a", "b__c"); + const flattened = namespacedToolName(first.namespace, first.name); + expect(namespacedToolName(second.namespace, second.name)).toBe(flattened); + + const registry = createOpenAIChatToolNameRegistry([first, second]); + const reversed = createOpenAIChatToolNameRegistry([second, first]); + const firstAlias = registry.alias(first); + const secondAlias = registry.alias(second); + expect(firstAlias).not.toBe(flattened); + expect(secondAlias).not.toBe(flattened); + expect(firstAlias).not.toBe(secondAlias); + expect(reversed.alias(first)).toBe(firstAlias); + expect(reversed.alias(second)).toBe(secondAlias); + expect(registry.aliasWireName(flattened)).toBe(flattened); + + const adapter = createOpenAIChatAdapter(provider()); + const request = adapter.buildRequest(parsedWith([first, second], {}, [ + { + role: "assistant", + content: [{ type: "toolCall", id: "call_ambiguous", namespace: first.namespace, name: first.name, arguments: {} }], + timestamp: 0, + }, + { role: "user", content: "Continue", timestamp: 1 }, + ]), { + headers: new Headers(), + translatorBudget: createTestTranslatorBudget(), + }); + if (request instanceof Promise) throw new Error("OpenAI Chat request unexpectedly became async"); + const body = JSON.parse(request.body) as { + tools: Array<{ function: { name: string } }>; + messages: Array<{ tool_calls?: Array<{ function: { name: string } }> }>; + }; + expect(body.tools.map(entry => entry.function.name)).toEqual([firstAlias, secondAlias]); + expect(body.messages.find(message => message.tool_calls)?.tool_calls?.[0].function.name).toBe(flattened); + }); + + test("keeps shared naming untouched so Kiro and Google retain adapter-owned normalization", () => { + const original = namespacedToolName(LONG_NAMESPACE, REPORTED_NAME); + expect(original).toBe(`${LONG_NAMESPACE}__${REPORTED_NAME}`); + expect(new TextEncoder().encode(original).byteLength).toBeGreaterThan(64); + + const kiro = kiroToolName(original); + expect(kiro).not.toBe(original); + expect(kiro).toMatch(/_[0-9a-f]{8}$/); + expect(kiro.length).toBeLessThanOrEqual(64); + + const google = compileGoogleWireBody({ + tools: [{ functionDeclarations: [{ name: original, parameters: { type: "object" } }] }], + }); + const googleName = (google.body.tools as Array<{ + functionDeclarations: Array<{ name: string }>; + }>)[0].functionDeclarations[0].name; + expect(googleName).not.toBe(original); + expect(googleName).toMatch(/^[A-Za-z_][A-Za-z0-9_-]{0,63}$/); + expect(google.restoreToolName(googleName)).toBe(original); + }); +}); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 91a58a6461c..ecdb5983b75 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -818,6 +818,7 @@ "omp-path-contract.test.ts": "clients", "omp-yaml-source-inline-comments.test.ts": "clients", "openai-api-virtual-models.test.ts": "adapters/openai", + "openai-chat-bounded-tool-names.test.ts": "adapters/openai", "openai-chat-dangling-toolcalls.test.ts": "adapters/openai", "openai-chat-eof.test.ts": "adapters/openai", "openai-chat-hardening.test.ts": "adapters/openai",