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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions src/responses/code-mode-helper-compat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ function unwrapPatchInput(value: string): string {
*/
export function compileCodeModeHelperInput(argumentsText: unknown, toolName: string): string {
if (typeof argumentsText !== "string") return "";
if (toolName === "apply_patch") {
const helperName = toolName.startsWith("default.")
? toolName.slice("default.".length)
: toolName;
if (helperName === "apply_patch") {
const patch = normalizeApplyPatchDelimiters(unwrapPatchInput(argumentsText));
return `const result = await tools.apply_patch(${JSON.stringify(patch)});\ntext(result);`;
}
Expand All @@ -43,17 +46,33 @@ export function compileCodeModeHelperInput(argumentsText: unknown, toolName: str
}
const args: unknown = isPlainObject(parsed) ? { ...parsed } : parsed;
if (
toolName === "shell_command"
helperName === "shell_command"
&& isPlainObject(args)
&& typeof args.command === "string"
&& args.cmd === undefined
) {
args.cmd = args.command;
delete args.command;
}
if (toolName === "write_stdin") {
if (helperName === "write_stdin") {
return `const result = await tools.write_stdin(${JSON.stringify(args)});\ntext(result);`;
}
if (helperName === "view_image") {
// Codex code-mode `exec` exposes `tools.view_image({path, detail?})`; the host answers
// with a custom_tool_call_output carrying `input_image`, which `image()` surfaces back
// to the model. Aliases map onto Codex's `path`/`detail`; anything else is passed as
// data so nested validation can reject it.
const viewArgs: unknown = isPlainObject(args) ? { ...args } : args;
if (isPlainObject(viewArgs)) {
for (const alias of ["file_path", "file", "image_path"]) {
if (typeof viewArgs.path !== "string" && typeof viewArgs[alias] === "string") {
viewArgs.path = viewArgs[alias];
}
delete viewArgs[alias];
}
}
return `const result = await tools.view_image(${JSON.stringify(viewArgs)});\nif (result && result.image_url) { image(result.image_url); } else { text(result); }`;
}
return `const result = await tools.exec_command(${JSON.stringify(args)});\ntext(result);`;
}

Expand Down
2 changes: 1 addition & 1 deletion src/server/responses-undeclared-tool-guard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ function addWireToolName(
}
// `exec` is the one name that also switches on nested-helper normalization, so a bare alias
// for a namespaced MCP tool would silently authorize `exec_command`/`shell_command`/
// `apply_patch` the request never declared. Every other inner name keeps the bare alias.
// `apply_patch`/`view_image` the request never declared. Every other inner name keeps the bare alias.
if (name !== CODE_MODE_EXEC_TOOL_NAME) names.add(name);
}

Expand Down
2 changes: 1 addition & 1 deletion src/server/responses/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5071,7 +5071,7 @@ async function handleResponsesInner(
// `buildToolBridgeMaps` also aliases a namespaced tool under its bare name when the
// caller's `tool_choice` selected it unambiguously, which the bridge needs to route the
// call back. For `exec` alone that alias would also switch on nested-helper
// normalization and re-authorize `exec_command`/`shell_command`/`apply_patch`, so it is
// normalization and re-authorize `exec_command`/`shell_command`/`apply_patch`/`view_image`, so it is
// admitted here only when the caller's own catalog declared a bare `exec`. Selecting an
// MCP `exec` is not a declaration of the code-mode shell tool.
if (
Expand Down
22 changes: 17 additions & 5 deletions src/types/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,17 @@ export function dottedToolName(namespace: string | undefined, name: string): str
*
* 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). 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).
* echo that helper name as the tool-call name, emitting `exec_command`, `write_stdin`,
* `apply_patch`, or `view_image` 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,
"write_stdin",
"apply_patch",
"view_image",
] as const;

/**
Expand All @@ -71,7 +72,7 @@ export const CODE_MODE_EXEC_TOOL_NAME = "exec";
*
* Rewrites invented `default.<name>` prefixes back to a declared bare tool when that bare tool
* is declared and neither `default.<name>` nor `default__<name>` was explicitly declared (#4176).
* Also normalizes legacy helper names (`exec_command`, `shell_command`, `apply_patch`) to
* Also normalizes legacy helper names (`exec_command`, `shell_command`, `apply_patch`, `view_image`) to
* `exec` when code-mode `exec` is declared in the request catalog.
*
* @param name - The tool name emitted on the wire by the provider.
Expand All @@ -98,6 +99,17 @@ export function normalizeDeclaredToolName(
&& !declared.has("default__" + bare)
) {
candidate = bare;
} else if (
// Code mode never declares bare helper names; a provider that invents `default.`
// for one still means the nested helper. Strip the prefix so the helper list
// below can rewrite it to `exec` (#4412).
bare.length > 0
&& declared.has(CODE_MODE_EXEC_TOOL_NAME)
&& (CODE_MODE_HELPER_TOOL_NAMES as readonly string[]).includes(bare)
&& !declared.has("default." + bare)
&& !declared.has("default__" + bare)
) {
candidate = bare;
}
}
if (!declared.has(CODE_MODE_EXEC_TOOL_NAME)) return candidate;
Expand Down
4 changes: 1 addition & 3 deletions structure/transports/responses.md
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,7 @@ Cursor's error classification and Kiro's whitespace and failed-wrapper grouping
halves live in `src/adapters/exec-tool-result-normalize.ts`
so the pre-call and post-hoc wording cannot drift. This guidance and annotation change rewrites
neither the model's JavaScript nor its patch payload; the existing name-alias delimiter
normalization in `src/responses/code-mode-helper-compat.ts` is unchanged, and the host still rejects a
malformed call exactly as before. Anthropic, Google, OpenAI-chat and command-code result paths
have no exec-result seam today and are not annotated.
normalization in `src/responses/code-mode-helper-compat.ts` remains separate from result annotation. That boundary maps a routed provider's bare or synthetic-`default.` `view_image` call onto the declared code-mode `exec`, invokes nested `tools.view_image`, and emits the returned `image_url` through `image()`. Explicit `path` wins; otherwise `file_path`, `file`, and `image_path` normalize in that order. Malformed input still reaches nested validation as data. Anthropic, Google, OpenAI-chat and command-code result paths have no exec-result seam today and are not annotated.

> Decision record: [ADR-0040](../decisions/ADR-0040-responses-http-sse.md)

Expand Down
35 changes: 35 additions & 0 deletions tests/adapters/bridge-legacy-shell-normalization.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,41 @@ describe("bridge normalizes code-mode helper names against the declared catalog"
expect(sse).toContain("image.png");
});

test("view_image is compiled through code-mode exec and surfaces the image", async () => {
const sse = await drain(bridgeToResponsesSSE(
toolTurn("view_image", '{"file_path":"/tmp/image.png","detail":"high"}'),
"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.view_image({\\"detail\\":\\"high\\",\\"path\\":\\"/tmp/image.png\\"})');
expect(sse).toContain("image(result.image_url)");
expect(sse).not.toContain("tools.exec_command");
});

test("default.view_image is compiled through code-mode exec", async () => {
const sse = await drain(bridgeToResponsesSSE(
toolTurn("default.view_image", '{"path":"/tmp/image.png"}'),
"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.view_image");
expect(sse).not.toContain("tools.exec_command");
});

test("a catalog that declares exec_command itself is never rewritten", async () => {
const sse = await drain(bridgeToResponsesSSE(
toolTurn("exec_command"), "deepseek-x", undefined, undefined, undefined, undefined, 50_000,
Expand Down
134 changes: 134 additions & 0 deletions tests/responses/legacy-shell-compat.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,138 @@ describe("code-mode helper compatibility", () => {
expect(received).toEqual(input === "[]" ? [] : input);
}
});

test("view_image compiles to tools.view_image and forwards image_url to image()", async () => {
const source = compileCodeModeHelperInput(
JSON.stringify({ path: "/tmp/shot.png", detail: "high" }),
"default.view_image",
);
let received: unknown;
let surfaced: unknown;
const run = new AsyncFunction("tools", "text", "image", source);

await run(
{
view_image: async (args: unknown) => {
received = args;
return { image_url: "data:image/png;base64,AAAA" };
},
},
() => { throw new Error("image result leaked into text output"); },
(value: unknown) => { surfaced = value; },
);

expect(received).toEqual({ path: "/tmp/shot.png", detail: "high" });
expect(surfaced).toBe("data:image/png;base64,AAAA");
});

test("view_image maps file_path/file/image_path aliases onto path", async () => {
for (const alias of ["file_path", "file", "image_path"]) {
const source = compileCodeModeHelperInput(
JSON.stringify({ [alias]: "/tmp/alias.png" }),
"view_image",
);
let received: unknown;
const run = new AsyncFunction("tools", "text", "image", source);
await run(
{
view_image: async (args: unknown) => {
received = args;
return {};
},
},
() => {},
() => {},
);
expect(received).toEqual({ path: "/tmp/alias.png" });
}
});

test("view_image keeps explicit path precedence and removes provider aliases", async () => {
const source = compileCodeModeHelperInput(
JSON.stringify({ path: "/tmp/right.png", file_path: "/tmp/wrong.png", detail: "original" }),
"view_image",
);
let received: unknown;
const run = new AsyncFunction("tools", "text", "image", source);
await run(
{
view_image: async (args: unknown) => {
received = args;
return {};
},
},
() => {},
() => {},
);
expect(received).toEqual({ path: "/tmp/right.png", detail: "original" });
});

test("view_image aliases use deterministic precedence when providers send more than one", async () => {
const source = compileCodeModeHelperInput(
JSON.stringify({
file_path: "/tmp/file-path.png",
file: "/tmp/file.png",
image_path: "/tmp/image-path.png",
}),
"view_image",
);
let received: unknown;
const run = new AsyncFunction("tools", "text", "image", source);
await run(
{
view_image: async (args: unknown) => {
received = args;
return {};
},
},
() => {},
() => {},
);

expect(received).toEqual({ path: "/tmp/file-path.png" });
});

test("view_image without image_url still returns the host result", async () => {
const source = compileCodeModeHelperInput(
JSON.stringify({ path: "/tmp/missing.png" }),
"view_image",
);
let surfaced = false;
let output: unknown;
const run = new AsyncFunction("tools", "text", "image", source);
await run(
{
view_image: async () => ({ error: "not found" }),
},
(value: unknown) => { output = value; },
() => { surfaced = true; },
);
expect(surfaced).toBe(false);
expect(output).toEqual({ error: "not found" });
});

test("invalid view_image input remains data instead of becoming JavaScript", async () => {
const input = "{not-json`); throw new Error('escaped') //";
let received: unknown;
const run = new AsyncFunction(
"tools",
"text",
"image",
compileCodeModeHelperInput(input, "view_image"),
);

await run(
{
view_image: async (args: unknown) => {
received = args;
return { error: "invalid input" };
},
},
() => {},
() => {},
);

expect(received).toBe(input);
});
});
Loading
Loading