diff --git a/src/responses/code-mode-helper-compat.ts b/src/responses/code-mode-helper-compat.ts index a9140f8f30..627f201b6e 100644 --- a/src/responses/code-mode-helper-compat.ts +++ b/src/responses/code-mode-helper-compat.ts @@ -21,7 +21,7 @@ function unwrapPatchInput(value: string): string { * Convert a nested Code Mode helper call into unified-exec JavaScript. * * Parsed values are serialized as data, never interpolated as source, so command and patch text - * cannot escape the generated call. Invalid structured shell payloads are also passed as data so + * cannot escape the generated call. Invalid structured helper payloads are also passed as data so * nested-tool validation can reject them without evaluating provider text as JavaScript. */ export function compileCodeModeHelperInput(argumentsText: unknown, toolName: string): string { @@ -46,5 +46,8 @@ export function compileCodeModeHelperInput(argumentsText: unknown, toolName: str args.cmd = args.command; delete args.command; } + if (toolName === "write_stdin") { + return `const result = await tools.write_stdin(${JSON.stringify(args)});\ntext(result);`; + } return `const result = await tools.exec_command(${JSON.stringify(args)});\ntext(result);`; } diff --git a/src/types/tools.ts b/src/types/tools.ts index 8f713be620..3de31c2de6 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -35,20 +35,23 @@ export function namespacedToolName(namespace: string | undefined, name: string): * Codex unified-exec name normalization. * * Codex's code-mode shell tool is declared as `exec` (a freeform custom tool whose own - * description mentions the nested `await tools.exec_command(...)` helper). Routed models — - * DeepSeek in particular — sometimes echo that helper name as the tool-call name, emitting - * `exec_command` or `apply_patch` instead of the declared `exec`. Accept these nested helper - * names only when the request catalog actually declares `exec` and does not itself declare the - * emitted name (an MCP server may legitimately advertise one under its own namespace). + * description mentions the nested `await tools.exec_command(...)` helper). Some routed providers + * echo that helper name as the tool-call name, emitting `exec_command`, `write_stdin`, or + * `apply_patch` instead of the declared `exec`. Accept these nested helper names only when the + * request catalog actually declares `exec` and does not itself declare the emitted name (an MCP + * server may legitimately advertise one under its own namespace). */ const LEGACY_SHELL_BRIDGE_TOOL_NAMES = ["exec_command", "shell_command"] as const; -const CODE_MODE_HELPER_TOOL_NAMES = [...LEGACY_SHELL_BRIDGE_TOOL_NAMES, "apply_patch"] as const; +const CODE_MODE_HELPER_TOOL_NAMES = [ + ...LEGACY_SHELL_BRIDGE_TOOL_NAMES, + "write_stdin", + "apply_patch", +] as const; /** * The one declared name that turns nested-helper normalization on. Declaring it is not just a - * name: it also decides whether an emitted `exec_command`/`shell_command`/`apply_patch` is - * accepted as that shell tool, so callers that build declared-name sets must add it only for a - * genuine bare declaration. + * name: it also decides whether an emitted helper name is accepted as that shell tool, so callers + * that build declared-name sets must add it only for a genuine bare declaration. */ export const CODE_MODE_EXEC_TOOL_NAME = "exec"; diff --git a/tests/bridge-legacy-shell-normalization.test.ts b/tests/bridge-legacy-shell-normalization.test.ts index 79b4e4aa98..36600084d9 100644 --- a/tests/bridge-legacy-shell-normalization.test.ts +++ b/tests/bridge-legacy-shell-normalization.test.ts @@ -14,9 +14,9 @@ async function drain(stream: ReadableStream): Promise { return out; } -async function* toolTurn(name: string): AsyncGenerator { +async function* toolTurn(name: string, argumentsText = '{"cmd":"ls"}'): AsyncGenerator { yield { type: "tool_call_start", id: "call-1", name } as AdapterEvent; - yield { type: "tool_call_delta", id: "call-1", arguments: '{"cmd":"ls"}' } as AdapterEvent; + yield { type: "tool_call_delta", id: "call-1", arguments: argumentsText } as AdapterEvent; yield { type: "tool_call_end", id: "call-1" } as AdapterEvent; yield { type: "done" } as AdapterEvent; } @@ -25,7 +25,7 @@ async function* toolTurn(name: string): AsyncGenerator { // nested `tools.exec_command(...)` helper. Routed models echo the helper name back, and the // undeclared-tool guard turned that into a 502 mid-turn. These pin the SSE path the guard // actually runs on, which the review flagged as untested. -describe("bridge normalizes legacy shell names against the declared catalog (#2493)", () => { +describe("bridge normalizes code-mode helper names against the declared catalog", () => { test("exec_command is delivered as the declared exec instead of failing the turn", async () => { const sse = await drain(bridgeToResponsesSSE( toolTurn("exec_command"), "deepseek-x", undefined, new Set(["exec"]), undefined, undefined, 50_000, @@ -47,6 +47,22 @@ describe("bridge normalizes legacy shell names against the declared catalog (#24 expect(sse).toContain('await tools.exec_command({\\"cmd\\":\\"ls\\"})'); }); + test("write_stdin is wrapped through the declared exec tool", async () => { + const sse = await drain(bridgeToResponsesSSE( + toolTurn("write_stdin", '{"session_id":17,"yield_time_ms":1000}'), + "fixture-model", + undefined, + new Set(["exec"]), + undefined, + undefined, + 50_000, + { declaredToolNames: new Set(["exec"]) }, + )); + expect(sse).not.toContain("undeclared client tool"); + expect(sse).toContain('"name":"exec"'); + expect(sse).toContain('await tools.write_stdin({\\"session_id\\":17,\\"yield_time_ms\\":1000})'); + }); + test("a genuinely undeclared tool still fails the turn", async () => { const sse = await drain(bridgeToResponsesSSE( toolTurn("other_tool"), "deepseek-x", undefined, undefined, undefined, undefined, 50_000, diff --git a/tests/legacy-shell-compat.test.ts b/tests/legacy-shell-compat.test.ts index 9fcc217b0e..60b3fb4449 100644 --- a/tests/legacy-shell-compat.test.ts +++ b/tests/legacy-shell-compat.test.ts @@ -43,6 +43,28 @@ describe("code-mode helper compatibility", () => { expect(received).toEqual({ workdir: "/tmp", cmd: "pwd" }); }); + test("write_stdin arguments remain data and target the nested helper", async () => { + const args = { + session_id: 17, + chars: "`); throw new Error('escaped') //", + yield_time_ms: 1_000, + }; + const source = compileCodeModeHelperInput(JSON.stringify(args), "write_stdin"); + let received: unknown; + let output: unknown; + const run = new AsyncFunction("tools", "text", source); + + await run({ + write_stdin: async (value: unknown) => { + received = value; + return { output: "more" }; + }, + }, (value: unknown) => { output = value; }); + + expect(received).toEqual(args); + expect(output).toEqual({ output: "more" }); + }); + test("apply_patch text remains one string argument", async () => { const patch = "*** Begin Patch\n*** Add File: note.txt\n+`); throw new Error('escaped')\n*** End Patch"; const source = compileCodeModeHelperInput(patch, "apply_patch"); diff --git a/tests/responses-custom-tool-repair.test.ts b/tests/responses-custom-tool-repair.test.ts index ba00963d3f..d739d95942 100644 --- a/tests/responses-custom-tool-repair.test.ts +++ b/tests/responses-custom-tool-repair.test.ts @@ -150,6 +150,74 @@ describe("routed Responses custom-tool compatibility", () => { rewrite.dispose?.(); }); + test("restores streamed write_stdin arguments through unified exec", () => { + const rewrite = createRoutedCustomToolRestoreBlockRewrite( + new Set(["exec"]), + undefined, + new Set(), + new Set(["exec"]), + ); + const added = rewrite(frame("response.output_item.added", { + output_index: 0, + item: { + type: "function_call", + id: "fc_stdin_alias", + call_id: "call_stdin_alias", + name: "write_stdin", + arguments: "", + status: "in_progress", + }, + })); + expect(dataPayload(added[0]!).item).toMatchObject({ type: "custom_tool_call", name: "exec" }); + expect(rewrite(frame("response.function_call_arguments.delta", { + output_index: 0, + item_id: "fc_stdin_alias", + delta: '{"session_id":17,"yield_time_ms":1000}', + }))).toEqual([]); + const done = rewrite(frame("response.function_call_arguments.done", { + output_index: 0, + item_id: "fc_stdin_alias", + arguments: '{"session_id":17,"yield_time_ms":1000}', + })); + expect(dataPayload(done[0]!)).toMatchObject({ + type: "response.custom_tool_call_input.done", + input: compileCodeModeHelperInput( + '{"session_id":17,"yield_time_ms":1000}', + "write_stdin", + ), + }); + rewrite.dispose?.(); + }); + + test("restores a non-streaming write_stdin call through unified exec", () => { + const upstream = JSON.stringify({ + id: "resp_stdin", + output: [{ + type: "function_call", + id: "fc_stdin", + call_id: "call_stdin", + name: "write_stdin", + arguments: '{"session_id":17,"yield_time_ms":1000}', + status: "completed", + }], + }); + + const restored = JSON.parse(restoreRoutedCustomCallsInJson( + upstream, + new Set(["exec"]), + new Set(), + new Set(["exec"]), + )) as { output: Array> }; + expect(restored.output[0]).toMatchObject({ + type: "custom_tool_call", + name: "exec", + input: compileCodeModeHelperInput( + '{"session_id":17,"yield_time_ms":1000}', + "write_stdin", + ), + }); + }); + test("rewrites exec definitions and paired history without touching apply_patch", () => { const raw = { model: "deepseek-v4-flash", diff --git a/tests/responses-undeclared-tool-guard.test.ts b/tests/responses-undeclared-tool-guard.test.ts index 99a73cf8c1..2daf2a7ee6 100644 --- a/tests/responses-undeclared-tool-guard.test.ts +++ b/tests/responses-undeclared-tool-guard.test.ts @@ -593,6 +593,31 @@ describe("a refused turn does not become continuation state", () => { expect(JSON.stringify(expanded)).not.toContain('"name":"apply_patch"'); }); + test("a bridged write_stdin turn is remembered as the declared exec call", async () => { + const responseId = "resp_stdin_bridged"; + const accepted = await turn(responseId, { + type: "function_call", + id: "fc_stdin", + call_id: "call_stdin", + name: "write_stdin", + arguments: JSON.stringify({ session_id: 17, yield_time_ms: 1_000 }), + status: "completed", + }); + + expect(accepted.status).toBe(200); + const expanded = expandPreviousResponseInput({ + model: "fixture-model", + previous_response_id: responseId, + input: [{ role: "user", content: [{ type: "input_text", text: "continue" }] }], + tools: declaredTools, + }) as { input?: Array> }; + const rememberedCall = expanded.input?.find(item => item.call_id === "call_stdin"); + + expect(rememberedCall).toMatchObject({ type: "custom_tool_call", name: "exec" }); + expect(rememberedCall?.input).toContain("tools.write_stdin"); + expect(JSON.stringify(expanded)).not.toContain('"name":"write_stdin"'); + }); + test("a streamed bridged apply_patch turn is remembered as the declared exec call", async () => { const responseId = "resp_stream_apply_patch_bridged"; const call = { @@ -1415,7 +1440,7 @@ describe("empty and absent tool catalogs", () => { namespace: "mcp__functions", }); - for (const name of ["apply_patch", "exec_command", "shell_command"]) { + for (const name of ["apply_patch", "exec_command", "shell_command", "write_stdin"]) { const refused = await post( false, tools, @@ -1503,6 +1528,16 @@ describe("undeclaredToolCallNameInResponse", () => { expect(undeclaredToolCallNameInResponse(response, new Set())).toBe("exec_command"); }); + test("accepts write_stdin only through a bare unified exec declaration", () => { + const response = { + output: [{ type: "function_call", name: "write_stdin" }], + }; + + expect(undeclaredToolCallNameInResponse(response, new Set(["exec"]))).toBeUndefined(); + expect(undeclaredToolCallNameInResponse(response, new Set(["write_stdin"]))).toBeUndefined(); + expect(undeclaredToolCallNameInResponse(response, new Set())).toBe("write_stdin"); + }); + test("never legacy-normalizes a namespaced shell bridge call", () => { // A namespaced call (e.g. an MCP server advertising its own exec_command) must be // matched by its full wire name only — never normalized to bare `exec`. @@ -1526,7 +1561,7 @@ describe("undeclaredToolCallNameInResponse", () => { tools: [{ type: "namespace", name: "mcp", tools: [{ type: "function", name: "exec" }] }], }); - for (const name of ["exec_command", "shell_command", "apply_patch", "exec"]) { + for (const name of ["exec_command", "shell_command", "apply_patch", "write_stdin", "exec"]) { expect(undeclaredToolCallNameInResponse( { output: [{ type: "function_call", name, call_id: "call_1" }] }, declared,