diff --git a/src/agent/apply-patch-tool.test.ts b/src/agent/apply-patch-tool.test.ts new file mode 100644 index 000000000..97a2f6a24 --- /dev/null +++ b/src/agent/apply-patch-tool.test.ts @@ -0,0 +1,67 @@ +import { mkdtemp, readFile, writeFile, stat } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { expect, test } from "bun:test"; + +import { createApplyPatchTool } from "./apply-patch-tool.js"; + +async function run(cwd: string, input: string): Promise { + const tool = createApplyPatchTool(cwd); + if (tool.kind !== "string") throw new Error("expected a string tool"); + return tool.handler({ input }, new AbortController().signal); +} + +test("adds, updates, and deletes files in one envelope", async () => { + const cwd = await mkdtemp(join(tmpdir(), "apply-patch-")); + await writeFile(join(cwd, "a.txt"), "one\ntwo\nthree\n"); + await writeFile(join(cwd, "gone.txt"), "x\n"); + const out = await run( + cwd, + [ + "*** Begin Patch", + "*** Add File: sub/new.txt", + "+hello", + "*** Update File: a.txt", + "@@", + " one", + "-two", + "+2", + " three", + "*** Delete File: gone.txt", + "*** End Patch", + ].join("\n"), + ); + expect(out).toContain("Success"); + expect(await readFile(join(cwd, "sub/new.txt"), "utf8")).toBe("hello\n"); + expect(await readFile(join(cwd, "a.txt"), "utf8")).toBe("one\n2\nthree\n"); + await expect(stat(join(cwd, "gone.txt"))).rejects.toThrow(); +}); + +test("a failing hunk leaves every file untouched", async () => { + const cwd = await mkdtemp(join(tmpdir(), "apply-patch-")); + await writeFile(join(cwd, "a.txt"), "one\n"); + const out = await run( + cwd, + [ + "*** Begin Patch", + "*** Add File: b.txt", + "+b", + "*** Update File: missing.txt", + "@@", + "-x", + "+y", + "*** End Patch", + ].join("\n"), + ); + expect(out).toContain("Error"); + await expect(stat(join(cwd, "b.txt"))).rejects.toThrow(); +}); + +test("rejects paths that escape the workspace", async () => { + const cwd = await mkdtemp(join(tmpdir(), "apply-patch-")); + const out = await run( + cwd, + "*** Begin Patch\n*** Add File: ../escape.txt\n+x\n*** End Patch", + ); + expect(out).toContain("escapes the workspace"); +}); diff --git a/src/agent/apply-patch-tool.ts b/src/agent/apply-patch-tool.ts new file mode 100644 index 000000000..4909e4c04 --- /dev/null +++ b/src/agent/apply-patch-tool.ts @@ -0,0 +1,109 @@ +import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; +import { dirname, isAbsolute, relative, resolve } from "node:path"; +import { stringTool } from "@intx/agent"; +import type { AgentTool } from "@intx/agent"; +import type { ToolDefinition } from "@intx/types/runtime"; +import { type } from "arktype"; + +import { + applyUpdateHunks, + CodexApplyPatchError, + parseCodexApplyPatch, + type PatchOp, +} from "./codex-apply-patch.js"; + +export const applyPatchDefinition: ToolDefinition = { + name: "apply_patch", + description: + "Edit files with a patch envelope: *** Begin Patch, then *** Add File: / *** Update File: (optional *** Move to:) / *** Delete File: sections, then *** End Patch. Update hunks start with @@ and use ' ', '-', '+' line prefixes. Paths are workspace-relative.", + inputSchema: { + type: "object", + properties: { + input: { type: "string", description: "The full patch envelope" }, + }, + required: ["input"], + }, +}; + +const ApplyPatchArgs = type({ input: "string" }); + +type Planned = + | { kind: "write"; path: string; content: string; label: string } + | { kind: "remove"; path: string; label: string }; + +function contained(cwd: string, path: string): string { + const abs = resolve(cwd, path); + const rel = relative(cwd, abs); + if (rel === "" || rel.startsWith("..") || isAbsolute(rel)) { + throw new CodexApplyPatchError(`path escapes the workspace: ${path}`); + } + return abs; +} + +async function plan(cwd: string, op: PatchOp): Promise { + if (op.type === "add") { + const path = contained(cwd, op.path); + return [ + { kind: "write", path, content: op.content, label: `A ${op.path}` }, + ]; + } + if (op.type === "delete") { + return [ + { kind: "remove", path: contained(cwd, op.path), label: `D ${op.path}` }, + ]; + } + const source = contained(cwd, op.path); + let original: string; + try { + original = await readFile(source, "utf8"); + } catch { + throw new CodexApplyPatchError(`cannot read ${op.path} to update it`); + } + const content = applyUpdateHunks(original, op.hunks); + if (op.moveTo === undefined) { + return [{ kind: "write", path: source, content, label: `M ${op.path}` }]; + } + return [ + { + kind: "write", + path: contained(cwd, op.moveTo), + content, + label: `M ${op.moveTo}`, + }, + { kind: "remove", path: source, label: "" }, + ]; +} + +/** + * Plans every op before touching disk so a bad hunk in the last file cannot + * leave the earlier files half-patched. + */ +export function createApplyPatchTool(cwd: string): AgentTool { + return stringTool({ + definition: applyPatchDefinition, + handler: async (rawArgs: Record): Promise => { + const parsedArgs = ApplyPatchArgs(rawArgs); + if (parsedArgs instanceof type.errors) { + return "Error: apply_patch requires input (string)."; + } + try { + const patch = parseCodexApplyPatch(parsedArgs.input); + const steps: Planned[] = []; + for (const op of patch.ops) steps.push(...(await plan(cwd, op))); + for (const step of steps) { + if (step.kind === "write") { + await mkdir(dirname(step.path), { recursive: true }); + await writeFile(step.path, step.content); + } else { + await rm(step.path, { force: true }); + } + } + const labels = steps.map((s) => s.label).filter((l) => l !== ""); + return `Success. Updated the following files:\n${labels.join("\n")}`; + } catch (err) { + if (err instanceof CodexApplyPatchError) return `Error: ${err.message}`; + throw err; + } + }, + }); +} diff --git a/src/agent/codex-tool-mount.test.ts b/src/agent/codex-tool-mount.test.ts index 8715fb359..6e9e965d6 100644 --- a/src/agent/codex-tool-mount.test.ts +++ b/src/agent/codex-tool-mount.test.ts @@ -37,7 +37,6 @@ describe("Codex tool proxy mount", () => { isCodex: false, }); const names = toolset.dynamicRunner.currentDefinitions().map((d) => d.name); - expect(names).not.toContain("apply_patch"); expect(names).not.toContain("shell"); expect(names).not.toContain("update_plan"); await toolset.dispose(); @@ -61,7 +60,6 @@ describe("Codex tool proxy mount", () => { isCodex: true, }); const names = toolset.dynamicRunner.currentDefinitions().map((d) => d.name); - expect(names).not.toContain("apply_patch"); expect(names).toContain("write_file"); expect(names).toContain("edit_file"); expect(names).toContain("delete_file"); diff --git a/src/agent/tool-search.ts b/src/agent/tool-search.ts index 322f6cb11..6593667f8 100644 --- a/src/agent/tool-search.ts +++ b/src/agent/tool-search.ts @@ -168,7 +168,10 @@ function isAlreadyAdvertised( // Mounted built-ins that stay off the advertised prefix and off tool_search. // Promote-on-execute must not flush these onto the wire either — dispatch // without advertising. glob is the advertised replacement for list_dir. -export const UNADVERTISED_MOUNTED_BUILTINS = new Set(["list_dir"]); +export const UNADVERTISED_MOUNTED_BUILTINS = new Set([ + "list_dir", + "apply_patch", +]); // Project the live tool registry onto the advertised set: the fixed built-in // prefix (its order never changes — this is what keeps the provider cache diff --git a/src/agent/tools.test.ts b/src/agent/tools.test.ts index 250ecb1ff..f61c99628 100644 --- a/src/agent/tools.test.ts +++ b/src/agent/tools.test.ts @@ -248,8 +248,8 @@ test("dynamicRunner contains posix tool names plus ask_operator", async () => { expect(names).toContain("write_file"); expect(names).toContain("edit_file"); expect(names).toContain("delete_file"); - // apply_patch is Codex-only and stripped on primary even when mounted. - expect(names).not.toContain("apply_patch"); + // apply_patch is mounted everywhere but only the gpt profile advertises it. + expect(names).toContain("apply_patch"); }); test("dynamicRunner omits ask_operator when onOperatorGate is not provided", async () => { diff --git a/src/agent/tools.ts b/src/agent/tools.ts index 56e400edf..6804c9b63 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -106,6 +106,7 @@ import { createWebSearchTool, disposeWebSearchClients, } from "../tools/web-search.js"; +import { createApplyPatchTool } from "./apply-patch-tool.js"; import { createUseSkillTool } from "./use-skill.js"; import { createSkillSearchTool } from "./skill-search.js"; import { @@ -669,6 +670,7 @@ export async function createAgentToolset( rootsProvider: createWorktreeRootsProvider(cwd), }), createUseSkillTool(cwd, skillDirs, args.telemetry), + createApplyPatchTool(cwd), createSkillSearchTool({ skills }), builtinExaEnabled ? createExaMCPWebFetchTool({ connect: waitForBuiltinExaConnection }) diff --git a/src/subagent/run.ts b/src/subagent/run.ts index 49f32b705..0debd52a3 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -67,6 +67,7 @@ import { } from "./intervention-log.js"; import { normalizeToolDefinitionsForProvider } from "../agent/tool-schema-normalize.js"; import { canonicalToolName } from "../agent/canonical-tool-name.js"; +import { createApplyPatchTool } from "../agent/apply-patch-tool.js"; import { checkMountedRequiresTools, formatCapabilityUnavailable, @@ -809,6 +810,16 @@ async function runSubAgentInner( if (params.capabilities !== undefined) { tools = applyCapabilityFilter(tools, params.capabilities); } + if ( + toolProfileForModel(params.provider) === "gpt" && + tools.some((t) => + ["write_file", "edit_file", "delete_file"].includes( + canonicalToolName(t.definition.name), + ), + ) + ) { + tools = [...tools, createApplyPatchTool(params.cwd)]; + } backgroundCollectMounted = tools.some( (tool) => tool.definition.name === "shell_collect", );