diff --git a/docs/PRODUCT.md b/docs/PRODUCT.md index e369e96f..4e18a675 100644 --- a/docs/PRODUCT.md +++ b/docs/PRODUCT.md @@ -92,7 +92,7 @@ is the direct, explicit resume path. - Paths outside the workspace and writes under the session state root still ask; mutating MCP and unknown tools still prompt. - **Path sandboxing** — Tool path arguments are resolved against the working directory; paths that escape it are blocked unless `--dangerously-skip-permissions` / `/yolo` is on (secret-guard and authz hard denies still apply). -- **Write verification** — After every write/edit the file is re-read and compared to confirm the change actually landed. +- **Write verification** — After every write/edit the file is re-read and compared to confirm the change actually landed; the result returned to the model (and shown to the operator) includes a bounded diff of the changed region — `write_file`, `edit_file`, `delete_file`, and each op inside `apply_patch` — so a follow-up `read_file` is never needed just to confirm an edit landed. A whole-file rewrite's diff is truncated (and says so) rather than blowing the result size cap. ## Slash Commands (TUI) diff --git a/src/agent/apply-patch-diff.test.ts b/src/agent/apply-patch-diff.test.ts new file mode 100644 index 00000000..af6b3dff --- /dev/null +++ b/src/agent/apply-patch-diff.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, test } from "bun:test"; +import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createPosixTools } from "@intx/tools-posix"; +import { createToolRunner } from "@intx/agent"; +import type { AgentTool } from "@intx/agent"; + +import { createCodexToolProxies, type CodexRunTool } from "./codex-tool-proxies.js"; +import { buildCorePosixToolPlugins } from "./posix-tool-plugins.js"; +import { createPermissionGate } from "../permission/gate.js"; + +/** + * apply_patch forwards each op through the same posixTools.run chain the rest + * of the agent uses (see tools.ts), so verify-plugin's and delete-file-plugin's + * diffs surface here too without any apply_patch-specific plumbing. + */ +async function invokeApplyPatch(tools: AgentTool[], input: string) { + const runner = createToolRunner(tools); + return runner.run( + { id: "call-1", name: "apply_patch", arguments: { input } }, + new AbortController().signal, + ); +} + +async function makeApplyPatch(cwd: string): Promise { + const gate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: true, + auto: false, + cwd, + }); + const posixTools = createPosixTools({ + cwd, + plugins: buildCorePosixToolPlugins({ cwd, permissionGate: gate }), + }); + const runTool: CodexRunTool = async (name, args) => { + // read_file's real tool output is cat -n formatted (line numbers), which + // is not the raw content applyUpdateHunks needs — in production this + // means Update File hunks generally fail to match context (filed as + // CL-6966, Urgent; not this issue's bug to fix). This stub bypasses that + // known defect by returning raw content, so the "Update File shows the + // diff" test below is NOT proof that apply_patch Update works end to end + // — it only proves the diff-surfacing added here is correct once the op + // succeeds. Add File / Delete File below do not depend on read_file and + // are real, unstubbed coverage. + if (name === "read_file") { + const path = String((args as { path?: unknown }).path ?? ""); + try { + return { content: await readFile(join(cwd, path), "utf8") }; + } catch (err) { + return { content: err instanceof Error ? err.message : String(err), isError: true }; + } + } + const result = await posixTools.run( + { id: "codex-proxy", name, arguments: args }, + new AbortController().signal, + ); + return { + content: typeof result.content === "string" ? result.content : JSON.stringify(result.content), + ...(result.isError === true ? { isError: true } : {}), + }; + }; + return createCodexToolProxies({ + isCodex: true, + runTool, + runManageTasks: async () => ({ content: "ok" }), + }); +} + +describe("apply_patch surfaces the changed region", () => { + test("Update File shows the diff", async () => { + const cwd = await mkdtemp(join(tmpdir(), "apply-patch-diff-")); + try { + await writeFile(join(cwd, "a.txt"), "line1\nworld\nline3\n"); + const tools = await makeApplyPatch(cwd); + const input = [ + "*** Begin Patch", + "*** Update File: a.txt", + "@@", + " line1", + "-world", + "+universe", + " line3", + "*** End Patch", + ].join("\n"); + + const result = await invokeApplyPatch(tools, input); + + expect(result.isError).not.toBe(true); + expect(String(result.content)).toContain("-world"); + expect(String(result.content)).toContain("+universe"); + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + + test("Delete File shows the removed content", async () => { + const cwd = await mkdtemp(join(tmpdir(), "apply-patch-diff-")); + try { + await writeFile(join(cwd, "gone.txt"), "bye\n"); + const tools = await makeApplyPatch(cwd); + const input = ["*** Begin Patch", "*** Delete File: gone.txt", "*** End Patch"].join("\n"); + + const result = await invokeApplyPatch(tools, input); + + expect(result.isError).not.toBe(true); + expect(String(result.content)).toContain("-bye"); + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); + + test("Add File shows the added content", async () => { + const cwd = await mkdtemp(join(tmpdir(), "apply-patch-diff-")); + try { + const tools = await makeApplyPatch(cwd); + const input = ["*** Begin Patch", "*** Add File: new.txt", "+hello", "*** End Patch"].join( + "\n", + ); + + const result = await invokeApplyPatch(tools, input); + + expect(result.isError).not.toBe(true); + expect(String(result.content)).toContain("+hello"); + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }); +}); diff --git a/src/plugins/change-diff.test.ts b/src/plugins/change-diff.test.ts new file mode 100644 index 00000000..1fb1c368 --- /dev/null +++ b/src/plugins/change-diff.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, test } from "bun:test"; +import { formatChangeDiff, MAX_DIFF_CHARS } from "./change-diff.js"; + +describe("formatChangeDiff", () => { + test("returns undefined when content is unchanged", () => { + expect(formatChangeDiff("a.txt", "same\n", "same\n")).toBeUndefined(); + }); + + test("small edit produces a unified diff with context", () => { + const before = "line1\nline2\nline3\nline4\nline5\n"; + const after = "line1\nline2\nCHANGED\nline4\nline5\n"; + + const diff = formatChangeDiff("a.txt", before, after); + + expect(diff).toBeDefined(); + expect(diff).toContain("--- a.txt"); + expect(diff).toContain("+++ a.txt"); + expect(diff).toContain("-line3"); + expect(diff).toContain("+CHANGED"); + expect(diff).toContain(" line2"); + expect(diff).toContain(" line4"); + }); + + test("whole-file rewrite is bounded by the char cap and says so", () => { + const before = "old content\n".repeat(2000); + const after = "new content\n".repeat(2000); + + const diff = formatChangeDiff("a.txt", before, after); + + expect(diff).toBeDefined(); + // The cap must hold exactly — the truncation note is reserved WITHIN + // maxChars, not appended after it. + expect(diff!.length).toBeLessThanOrEqual(MAX_DIFF_CHARS); + expect(diff).toContain("truncated"); + }); + + test("truncation note never pushes the result past the cap at a small boundary", () => { + // A tiny maxChars stresses the fixed-point loop in truncate(): the note's + // own length (which depends on the digit counts it reports) must still + // fit within the cap it is describing. + const before = "a\n".repeat(50); + const after = "b\n".repeat(50); + + for (const maxChars of [50, 80, 120, 200]) { + const diff = formatChangeDiff("a.txt", before, after, maxChars); + expect(diff).toBeDefined(); + expect(diff!.length).toBeLessThanOrEqual(maxChars); + } + }); + + test("very large files skip full LCS and report a bounded summary", () => { + const before = Array.from({ length: 3000 }, (_, i) => `l${i}`).join("\n"); + const after = Array.from({ length: 3000 }, (_, i) => `m${i}`).join("\n"); + + const diff = formatChangeDiff("big.txt", before, after); + + expect(diff).toBeDefined(); + expect(diff).toContain("large change"); + expect(diff).toContain("exceeds"); + expect(diff!.length).toBeLessThanOrEqual(MAX_DIFF_CHARS); + }); + + test("deletion (after is empty) shows removed lines", () => { + const diff = formatChangeDiff("gone.txt", "keep me\n", ""); + expect(diff).toBeDefined(); + expect(diff).toContain("-keep me"); + }); +}); diff --git a/src/plugins/change-diff.ts b/src/plugins/change-diff.ts new file mode 100644 index 00000000..9f4a36d4 --- /dev/null +++ b/src/plugins/change-diff.ts @@ -0,0 +1,227 @@ +/** + * Bounded unified-diff formatting for product-mutation tool results. + * + * Surfaces the changed region computed by verify-plugin / delete-file-plugin + * so a model can see its edit landed without issuing a follow-up read_file. + * Kept intentionally small: a plain LCS diff over line arrays, with an escape + * hatch for large files (skip the O(n*m) LCS, report a boundary-only summary) + * and a hard char cap so a whole-file rewrite never dominates the result. + */ + +const MAX_DIFF_CHARS = 4_000; +const MAX_LCS_LINES = 2_000; +const CONTEXT_LINES = 3; + +function splitLines(content: string): string[] { + if (content === "") return []; + const lines = content.split("\n"); + // Drop a single trailing empty segment from a final newline so line counts + // match what a reader would call "N lines", not N+1. + if (lines[lines.length - 1] === "") lines.pop(); + return lines; +} + +interface DiffOp { + kind: "same" | "add" | "del"; + text: string; +} + +/** Longest-common-subsequence line diff. Callers must bound input size. */ +function lcsDiff(oldLines: string[], newLines: string[]): DiffOp[] { + const n = oldLines.length; + const m = newLines.length; + const dp: Uint32Array[] = new Array(n + 1); + for (let i = 0; i <= n; i++) dp[i] = new Uint32Array(m + 1); + for (let i = n - 1; i >= 0; i--) { + for (let j = m - 1; j >= 0; j--) { + dp[i]![j] = + oldLines[i] === newLines[j] + ? dp[i + 1]![j + 1]! + 1 + : Math.max(dp[i + 1]![j]!, dp[i]![j + 1]!); + } + } + + const ops: DiffOp[] = []; + let i = 0; + let j = 0; + while (i < n && j < m) { + if (oldLines[i] === newLines[j]) { + ops.push({ kind: "same", text: oldLines[i]! }); + i++; + j++; + } else if (dp[i + 1]![j]! >= dp[i]![j + 1]!) { + ops.push({ kind: "del", text: oldLines[i]! }); + i++; + } else { + ops.push({ kind: "add", text: newLines[j]! }); + j++; + } + } + while (i < n) { + ops.push({ kind: "del", text: oldLines[i]! }); + i++; + } + while (j < m) { + ops.push({ kind: "add", text: newLines[j]! }); + j++; + } + return ops; +} + +interface Hunk { + oldStart: number; + oldLines: number; + newStart: number; + newLines: number; + ops: DiffOp[]; +} + +/** Group diff ops into hunks, collapsing runs of "same" longer than 2*context. */ +function toHunks(ops: DiffOp[]): Hunk[] { + const hunks: Hunk[] = []; + let oldLine = 1; + let newLine = 1; + let cur: Hunk | undefined; + let sameRun = 0; + + const flush = () => { + if (cur !== undefined) hunks.push(cur); + cur = undefined; + }; + + for (let idx = 0; idx < ops.length; idx++) { + const op = ops[idx]!; + if (op.kind === "same") { + sameRun++; + if (cur !== undefined) { + cur.ops.push(op); + cur.oldLines++; + cur.newLines++; + // Close the hunk once trailing context is satisfied and the run of + // unchanged lines continues for longer than one context window. + if (sameRun > CONTEXT_LINES) { + const trimBy = sameRun - CONTEXT_LINES; + cur.ops.splice(cur.ops.length - trimBy, trimBy); + cur.oldLines -= trimBy; + cur.newLines -= trimBy; + flush(); + } + } + oldLine++; + newLine++; + continue; + } + + sameRun = 0; + if (cur === undefined) { + const ctxStart = Math.max(0, idx - CONTEXT_LINES); + const ctxOps = ops.slice(ctxStart, idx).filter((o) => o.kind === "same"); + cur = { + oldStart: oldLine - ctxOps.length, + oldLines: ctxOps.length, + newStart: newLine - ctxOps.length, + newLines: ctxOps.length, + ops: [...ctxOps], + }; + } + cur.ops.push(op); + if (op.kind === "del") { + cur.oldLines++; + oldLine++; + } else { + cur.newLines++; + newLine++; + } + } + flush(); + return hunks; +} + +// Unified-diff convention: a zero-length side reports the line *before* the +// empty range (one less than where content would start), not that position +// itself — e.g. an insertion at the top of the file is "-0,0", not "-1,0". +function hunkRangeStart(start: number, count: number): number { + return count === 0 ? Math.max(0, start - 1) : start; +} + +function formatHunk(h: Hunk): string { + const oldStart = hunkRangeStart(h.oldStart, h.oldLines); + const newStart = hunkRangeStart(h.newStart, h.newLines); + const lines = [`@@ -${oldStart},${h.oldLines} +${newStart},${h.newLines} @@`]; + for (const op of h.ops) { + const prefix = op.kind === "same" ? " " : op.kind === "add" ? "+" : "-"; + lines.push(prefix + op.text); + } + return lines.join("\n"); +} + +function truncationNote(sliceLen: number, discarded: number): string { + return ( + `\n[diff truncated at ${sliceLen.toLocaleString()} chars — ${discarded.toLocaleString()} chars discarded. ` + + `The write/edit still applied in full; this is only a display cutoff.]` + ); +} + +/** + * Truncates so the FINAL result (slice + note) never exceeds maxChars — the + * note is reserved before slicing, not appended after. The note's own length + * depends on the digit counts of sliceLen/discarded, which depend on + * sliceLen, so shrink sliceLen until the assembled result fits (a handful of + * iterations at most — the note only grows when a digit-count boundary is + * crossed) and hard-clamp as a fallback. + */ +function truncate(diff: string, maxChars: number): string { + if (diff.length <= maxChars) return diff; + + let sliceLen = maxChars; + for (let i = 0; i < 8; i++) { + const discarded = diff.length - sliceLen; + const note = truncationNote(sliceLen, discarded); + const total = sliceLen + note.length; + if (total <= maxChars) return diff.slice(0, sliceLen) + note; + sliceLen -= total - maxChars; + if (sliceLen < 0) sliceLen = 0; + } + + // Fallback: guaranteed to fit even if the loop above didn't converge. + const discarded = diff.length - sliceLen; + const note = truncationNote(sliceLen, discarded); + return (diff.slice(0, sliceLen) + note).slice(0, maxChars); +} + +/** + * Bounded unified diff between `before` and `after` file content. Returns + * undefined when the two are identical (nothing to show). + */ +export function formatChangeDiff( + path: string, + before: string, + after: string, + maxChars: number = MAX_DIFF_CHARS, +): string | undefined { + if (before === after) return undefined; + + const oldLines = splitLines(before); + const newLines = splitLines(after); + + if (oldLines.length > MAX_LCS_LINES || newLines.length > MAX_LCS_LINES) { + // Large file: skip LCS (O(n*m) is too expensive) and report a bounded + // summary instead of a full line-by-line diff. + const header = `--- ${path}\n+++ ${path}\n`; + const summary = + `@@ large change: ${oldLines.length} lines -> ${newLines.length} lines @@\n` + + `[file exceeds ${MAX_LCS_LINES.toLocaleString()} lines; full diff omitted to stay bounded — ` + + `re-read the file directly if you need exact content]`; + return truncate(header + summary, maxChars); + } + + const ops = lcsDiff(oldLines, newLines); + const hunks = toHunks(ops); + if (hunks.length === 0) return undefined; + + const body = hunks.map(formatHunk).join("\n"); + const header = `--- ${path}\n+++ ${path}\n`; + return truncate(header + body, maxChars); +} + +export { MAX_DIFF_CHARS }; diff --git a/src/plugins/delete-file-plugin.test.ts b/src/plugins/delete-file-plugin.test.ts index 682630da..ed2e62bd 100644 --- a/src/plugins/delete-file-plugin.test.ts +++ b/src/plugins/delete-file-plugin.test.ts @@ -46,7 +46,12 @@ describe("deleteFilePlugin", () => { const result = await handler()(call("old.txt"), new AbortController().signal); - expect(result).toEqual({ callId: "delete-call", content: "Deleted file: old.txt" }); + // Match on the parts that matter (callId, deletion message, removed + // content) rather than the exact hunk header text, which is a + // formatChangeDiff implementation detail covered by change-diff.test.ts. + expect(result.callId).toBe("delete-call"); + expect(String(result.content)).toContain("Deleted file: old.txt"); + expect(String(result.content)).toContain("-old"); expect(await exists(path)).toBe(false); }); @@ -107,7 +112,9 @@ describe("deleteFilePlugin", () => { const result = await tool.handler(call(path), new AbortController().signal); - expect(result).toEqual({ callId: "delete-call", content: `Deleted file: ${path}` }); + expect(result.callId).toBe("delete-call"); + expect(String(result.content)).toContain(`Deleted file: ${path}`); + expect(String(result.content)).toContain("-gone"); expect(await exists(path)).toBe(false); await rm(outside, { recursive: true, force: true }); }); @@ -127,7 +134,9 @@ describe("deleteFilePlugin", () => { allow = true; const result = await tool.handler(call(path), new AbortController().signal); - expect(result).toEqual({ callId: "delete-call", content: `Deleted file: ${path}` }); + expect(result.callId).toBe("delete-call"); + expect(String(result.content)).toContain(`Deleted file: ${path}`); + expect(String(result.content)).toContain("-gone"); expect(await exists(path)).toBe(false); await rm(outside, { recursive: true, force: true }); }); diff --git a/src/plugins/delete-file-plugin.ts b/src/plugins/delete-file-plugin.ts index a85bc1e5..2643c1a9 100644 --- a/src/plugins/delete-file-plugin.ts +++ b/src/plugins/delete-file-plugin.ts @@ -1,8 +1,9 @@ -import { lstat, realpath, unlink } from "node:fs/promises"; +import { lstat, readFile, realpath, unlink } from "node:fs/promises"; import { dirname, isAbsolute, relative, resolve, sep } from "node:path"; import { type } from "arktype"; import type { ExtraTool, ToolPlugin } from "@intx/tools-posix"; import type { ToolCall, ToolResult } from "@intx/types/runtime"; +import { formatChangeDiff } from "./change-diff.js"; const DeleteFileArgs = type({ path: "string>0" }); @@ -54,6 +55,12 @@ function resolveAllowOutside(value: boolean | (() => boolean) | undefined): bool return value === true; } +// Above this size, skip reading the file into memory just to show a diff — +// the diff output is already char-capped (MAX_DIFF_CHARS), so buffering a +// large file for it is pure waste, and deleting a large file is a common +// enough case that the read must not become a resource regression. +const MAX_DELETE_PREVIEW_BYTES = 256 * 1024; + export function deleteFilePlugin(cwd: string, options: DeleteFilePluginOptions = {}): ToolPlugin { const tool: ExtraTool = { definition: DELETE_FILE_DEFINITION, @@ -80,8 +87,30 @@ export function deleteFilePlugin(cwd: string, options: DeleteFilePluginOptions = `${args.path} is a directory; delete_file only deletes files`, ); } + // Best-effort content capture before removal, so the result can show + // what was deleted (bounded, same as edit/write diffs). A failed read + // (binary, permissions) never blocks the delete itself. Large files + // skip the read entirely (see MAX_DELETE_PREVIEW_BYTES) and get a + // byte-count summary instead. + let before: string | undefined; + const tooLargeToPreview = info.size > MAX_DELETE_PREVIEW_BYTES; + if (!tooLargeToPreview) { + try { + before = await readFile(target, "utf8"); + } catch { + // Unreadable or binary; delete proceeds without a diff. + } + } + await unlink(target); - return { callId: call.id, content: `Deleted file: ${args.path}` }; + let content = `Deleted file: ${args.path}`; + if (tooLargeToPreview) { + content += ` (${info.size.toLocaleString()} bytes; too large to preview, content omitted)`; + } else if (before !== undefined) { + const diff = formatChangeDiff(args.path, before, ""); + if (diff !== undefined) content += `\n\n${diff}`; + } + return { callId: call.id, content }; } catch (error) { if (errorCode(error) === "ENOENT") { return { diff --git a/src/plugins/verify-plugin.test.ts b/src/plugins/verify-plugin.test.ts index 86df8842..44913de6 100644 --- a/src/plugins/verify-plugin.test.ts +++ b/src/plugins/verify-plugin.test.ts @@ -303,4 +303,88 @@ describe("verifyPlugin", () => { await rm(dir, { recursive: true, force: true }); } }); + + test("successful edit_file result includes the changed region", async () => { + const dir = await mkdtemp(join(tmpdir(), "verify-test-")); + try { + const plugin = verifyPlugin(); + const editHandler = async (call: ToolCall): Promise => { + const path = String(call.arguments.path ?? ""); + const oldStr = String(call.arguments.old_string ?? ""); + const newStr = String(call.arguments.new_string ?? ""); + const content = await readFile(path, "utf8"); + await writeFile(path, content.replace(oldStr, newStr)); + return { callId: call.id, content: "edited" }; + }; + const handler = plugin.middleware ? plugin.middleware(editHandler) : editHandler; + + const path = join(dir, "diff.txt"); + await writeFile(path, "line1\nworld\nline3\n"); + const result = await handler( + { + id: "call-diff", + name: "edit_file", + arguments: { path, old_string: "world", new_string: "universe" }, + }, + new AbortController().signal, + ); + + expect(result.isError).not.toBe(true); + expect(result.content).toContain("-world"); + expect(result.content).toContain("+universe"); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("successful write_file result includes a bounded diff for a whole-file rewrite", async () => { + const dir = await mkdtemp(join(tmpdir(), "verify-test-")); + try { + const plugin = verifyPlugin(); + const writeHandler = async (call: ToolCall): Promise => { + const path = String(call.arguments.path ?? ""); + await writeFile(path, String(call.arguments.content ?? "")); + return { callId: call.id, content: "written" }; + }; + const handler = plugin.middleware ? plugin.middleware(writeHandler) : writeHandler; + + const path = join(dir, "rewrite.txt"); + await writeFile(path, "old content\n".repeat(2000)); + const newContent = "new content\n".repeat(2000); + const result = await handler( + { id: "call-rewrite", name: "write_file", arguments: { path, content: newContent } }, + new AbortController().signal, + ); + + expect(result.isError).not.toBe(true); + expect(result.content).toContain("truncated"); + expect(String(result.content).length).toBeLessThan(6_000); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + test("write_file creating a new file shows the added content, not an error", async () => { + const dir = await mkdtemp(join(tmpdir(), "verify-test-")); + try { + const plugin = verifyPlugin(); + const writeHandler = async (call: ToolCall): Promise => { + const path = String(call.arguments.path ?? ""); + await writeFile(path, String(call.arguments.content ?? "")); + return { callId: call.id, content: "written" }; + }; + const handler = plugin.middleware ? plugin.middleware(writeHandler) : writeHandler; + + const path = join(dir, "new.txt"); + const result = await handler( + { id: "call-new", name: "write_file", arguments: { path, content: "brand new\n" } }, + new AbortController().signal, + ); + + expect(result.isError).not.toBe(true); + expect(result.content).toContain("+brand new"); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); }); diff --git a/src/plugins/verify-plugin.ts b/src/plugins/verify-plugin.ts index 4719b4b5..a146cdee 100644 --- a/src/plugins/verify-plugin.ts +++ b/src/plugins/verify-plugin.ts @@ -3,6 +3,7 @@ import type { ToolPlugin } from "@intx/tools-posix"; import type { ToolCall, ToolResult } from "@intx/types/runtime"; import { withFileMutationLock } from "./file-mutation-lock.js"; import { applyLineRangeEdit, parseEditFileMode } from "./edit-file-line-range.js"; +import { formatChangeDiff } from "./change-diff.js"; function mutationPath(call: ToolCall): string | undefined { if (call.name !== "edit_file" && call.name !== "write_file") return undefined; @@ -10,18 +11,29 @@ function mutationPath(call: ToolCall): string | undefined { return typeof path === "string" && path.length > 0 ? path : undefined; } +function withDiff(result: ToolResult, path: string, before: string, after: string): ToolResult { + const diff = formatChangeDiff(path, before, after); + if (diff === undefined) return result; + return { ...result, content: `${result.content}\n\n${diff}` }; +} + export function verifyPlugin(): ToolPlugin { return { middleware: (next) => async (call, signal) => { const lockedPath = mutationPath(call); const run = async (): Promise => { + // edit_file already read the file here pre-PR (to validate old_string + // uniqueness downstream) — reusing it for the diff is free. write_file + // did not: this pre-write read is a genuine extra read added by the + // diff feature, since write_file has no other source for "before". let before: string | undefined; - if (call.name === "edit_file") { + if (call.name === "edit_file" || call.name === "write_file") { const path = String(call.arguments.path ?? ""); try { before = await readFile(path, "utf8"); } catch { - // File may not exist yet; edit will likely fail downstream + // File may not exist yet (new file / edit will likely fail downstream). + before = call.name === "write_file" ? "" : undefined; } } @@ -39,6 +51,7 @@ export function verifyPlugin(): ToolPlugin { isError: true, }; } + return withDiff(result, path, before ?? "", actual); } catch (err) { return { callId: call.id, @@ -73,6 +86,7 @@ export function verifyPlugin(): ToolPlugin { isError: true, }; } + return withDiff(result, path, before, actual); } catch (err) { return { callId: call.id,