Skip to content
Merged
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
2 changes: 1 addition & 1 deletion docs/PRODUCT.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
131 changes: 131 additions & 0 deletions src/agent/apply-patch-diff.test.ts
Original file line number Diff line number Diff line change
@@ -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<AgentTool[]> {
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 });
}
});
});
68 changes: 68 additions & 0 deletions src/plugins/change-diff.test.ts
Original file line number Diff line number Diff line change
@@ -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");
});
});
Loading
Loading