Improve apply_patch diff UI and validation

package.json

@@ -26,7 +26,7 @@
     "dev": "node scripts/dev-server.mjs",
     "postinstall": "node scripts/fix-node-pty-permissions.mjs",
     "start": "node dist/cli.js serve",
-    "test": "tsx src/config.test.ts && tsx src/ui/card-types.test.ts && tsx src/apply-patch.test.ts && tsx src/process-platform.test.ts && tsx src/process-sessions.test.ts && tsx src/roots.test.ts && tsx src/skills.test.ts && tsx src/workspaces.test.ts && tsx src/review-checkpoints.test.ts && tsx src/oauth-store.test.ts && tsx src/cli.test.ts",
+    "test": "tsx src/config.test.ts && tsx src/ui/card-types.test.ts && tsx src/ui/patch-display.test.ts && tsx src/apply-patch.test.ts && tsx src/process-platform.test.ts && tsx src/process-sessions.test.ts && tsx src/roots.test.ts && tsx src/skills.test.ts && tsx src/workspaces.test.ts && tsx src/review-checkpoints.test.ts && tsx src/oauth-store.test.ts && tsx src/cli.test.ts",
     "typecheck": "tsc -p tsconfig.json --noEmit"
   },
   "keywords": [],
@@ -39,6 +39,7 @@
     "@modelcontextprotocol/sdk": "^1.29.0",
     "@pierre/diffs": "^1.2.5",
     "better-sqlite3": "^12.10.0",
+    "diff": "^8.0.3",
     "drizzle-orm": "^0.45.2",
     "express": "^5.2.1",
     "react": "^19.2.6",

src/apply-patch.test.ts

@@ -2,7 +2,7 @@ import assert from "node:assert/strict";
 import { chmod, mkdtemp, readFile, stat, symlink, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
-import { applyPatch, parsePatch, replaceFile } from "./apply-patch.js";
+import { applyPatch, isSamePatchFile, parsePatch, replaceFile } from "./apply-patch.js";
 
 const root = await mkdtemp(join(tmpdir(), "devspace-apply-patch-"));
 const replacement = join(root, "replacement.txt");
@@ -11,6 +11,17 @@ await writeFile(replacement, "old\n");
 await writeFile(replacementTemporary, "new\n");
 await replaceFile(replacementTemporary, replacement, true, "win32");
 assert.equal(await readFile(replacement, "utf8"), "new\n");
+
+const sameIdentity = async (): Promise<{ dev: number; ino: number }> => ({ dev: 1, ino: 2 });
+const differentIdentity = async (path: string): Promise<{ dev: number; ino: number }> => ({
+  dev: 1,
+  ino: path.endsWith("foo.txt") ? 3 : 2,
+});
+assert.equal(await isSamePatchFile("/tmp/Foo.txt", "/tmp/Foo.txt"), true);
+assert.equal(await isSamePatchFile("/tmp/Foo.txt", "/tmp/foo.txt", sameIdentity), true);
+assert.equal(await isSamePatchFile("/tmp/Foo.txt", "/tmp/bar.txt", sameIdentity), false);
+assert.equal(await isSamePatchFile("/tmp/Foo.txt", "/tmp/foo.txt", differentIdentity), false);
+
 await writeFile(join(root, "alpha.txt"), "one\ntwo\nthree\n");
 await writeFile(join(root, "remove.txt"), "remove me\n");
 await writeFile(join(root, "windows.txt"), "first\r\nsecond\r\n");
@@ -124,7 +135,52 @@ await assert.rejects(
   ),
   /could not find hunk context/,
 );
-assert.equal(await readFile(join(root, "should-not-exist.txt"), "utf8"), "staged\n");
+await assert.rejects(readFile(join(root, "should-not-exist.txt"), "utf8"), /ENOENT/);
+assert.equal(await readFile(join(root, "moved/alpha.txt"), "utf8"), "ONE\nchanged\nthree\n");
+
+const splitHunkRoot = await mkdtemp(join(tmpdir(), "devspace-apply-patch-split-hunk-"));
+await writeFile(
+  join(splitHunkRoot, "long.txt"),
+  Array.from({ length: 20 }, (_, index) => String(index + 1)).join("\n") + "\n",
+);
+const splitHunkResult = await applyPatch(
+  splitHunkRoot,
+  `*** Begin Patch
+*** Update File: long.txt
+@@
+ 1
+-2
++two
+ 3
+@@
+ 17
+-18
++eighteen
+ 19
+*** End Patch`,
+);
+assert.equal(splitHunkResult.patch.match(/^@@ /gm)?.length, 2);
+assert.equal(
+  await readFile(join(splitHunkRoot, "long.txt"), "utf8"),
+  [
+    "1", "two", "3", "4", "5", "6", "7", "8", "9", "10",
+    "11", "12", "13", "14", "15", "16", "17", "eighteen", "19", "20",
+  ].join("\n") + "\n",
+);
+
+const trailingSpaceRoot = await mkdtemp(join(tmpdir(), "devspace-apply-patch-trailing-space-"));
+await writeFile(join(trailingSpaceRoot, "spaces.txt"), "old\n");
+const trailingSpaceResult = await applyPatch(
+  trailingSpaceRoot,
+  `*** Begin Patch
+*** Update File: spaces.txt
+@@
+-old
++new${"   "}
+*** End Patch`,
+);
+assert.equal(trailingSpaceResult.patch.endsWith("+new   "), true);
+assert.equal(await readFile(join(trailingSpaceRoot, "spaces.txt"), "utf8"), "new   \n");
 
 assert.throws(() => parsePatch("*** Begin Patch\n*** End Patch"), /contains no file actions/);
 assert.throws(() => parsePatch("*** Add File: bad.txt\n+x"), /missing .* marker/);

src/apply-patch.ts

@@ -1,8 +1,9 @@
 import { randomUUID } from "node:crypto";
-import { constants } from "node:fs";
-import { access, mkdir, readFile, realpath, rename, rm, stat, writeFile } from "node:fs/promises";
+import { constants, type Stats } from "node:fs";
+import { access, lstat, mkdir, readFile, realpath, rename, rm, stat, writeFile } from "node:fs/promises";
 import { dirname, isAbsolute, relative, resolve } from "node:path";
 import { TextDecoder } from "node:util";
+import { createTwoFilesPatch, FILE_HEADERS_ONLY } from "diff";
 
 export type PatchOperation = "add" | "update" | "delete" | "move";
 
@@ -40,6 +41,10 @@ interface TextFile {
   mode?: number;
 }
 
+type StagedTextFile = TextFile | null;
+type FileIdentity = Pick<Stats, "dev" | "ino">;
+type FileIdentityReader = (path: string) => Promise<FileIdentity>;
+
 function patchError(message: string): Error {
   return new Error(`Invalid patch: ${message}`);
 }
@@ -314,26 +319,61 @@ export async function replaceFile(
   await rm(backup, { force: true });
 }
 
+export async function isSamePatchFile(
+  source: string,
+  destination: string,
+  readIdentity: FileIdentityReader = lstat,
+): Promise<boolean> {
+  if (source === destination) return true;
+  if (source.toLowerCase() !== destination.toLowerCase()) return false;
+
+  try {
+    const [sourceIdentity, destinationIdentity] = await Promise.all([
+      readIdentity(source),
+      readIdentity(destination),
+    ]);
+    return sourceIdentity.dev === destinationIdentity.dev && sourceIdentity.ino === destinationIdentity.ino;
+  } catch (error) {
+    const code = (error as NodeJS.ErrnoException).code;
+    if (code === "ENOENT" || code === "ENOTDIR") return false;
+    throw error;
+  }
+}
+
 export async function applyPatch(root: string, patch: string): Promise<ApplyPatchResult> {
   const actions = parsePatch(patch);
   const results: AppliedPatchFile[] = [];
   const patches: string[] = [];
+  const staged = new Map<string, StagedTextFile>();
+
+  const readStagedOptional = async (absolute: string, displayPath: string): Promise<StagedTextFile> => {
+    if (staged.has(absolute)) return staged.get(absolute) ?? null;
+    const file = await readOptionalTextFile(absolute, displayPath);
+    staged.set(absolute, file);
+    return file;
+  };
+
+  const readStagedRequired = async (absolute: string, displayPath: string): Promise<TextFile> => {
+    const file = await readStagedOptional(absolute, displayPath);
+    if (!file) throw patchError(`file does not exist: ${displayPath}`);
+    return file;
+  };
 
   for (const action of actions) {
     if (action.kind === "add") {
       const absolute = await resolveConfinedPath(root, action.path);
-      const original = await readOptionalTextFile(absolute, action.path);
-      await writeTextFile(absolute, action.content, original?.mode);
+      const original = await readStagedOptional(absolute, action.path);
+      staged.set(absolute, { content: action.content, mode: original?.mode });
       patches.push(unifiedFilePatch(action.path, action.path, original?.content ?? null, action.content));
       results.push({ path: action.path, operation: "add" });
       continue;
     }
 
     const absolute = await resolveConfinedPath(root, action.path);
-    const file = await readRequiredTextFile(absolute, action.path);
+    const file = await readStagedRequired(absolute, action.path);
 
     if (action.kind === "delete") {
-      await rm(absolute);
+      staged.set(absolute, null);
       patches.push(unifiedFilePatch(action.path, action.path, file.content, null));
       results.push({ path: action.path, operation: "delete" });
       continue;
@@ -342,30 +382,33 @@ export async function applyPatch(root: string, patch: string): Promise<ApplyPatc
     const updated = applyHunks(action.path, file.content, action.hunks);
     if (action.moveTo) {
       const destination = await resolveConfinedPath(root, action.moveTo);
-      if (destination !== absolute) await readOptionalTextFile(destination, action.moveTo);
-      await writeTextFile(destination, updated, file.mode);
-      if (destination !== absolute) await rm(absolute);
+      const samePatchFile = await isSamePatchFile(absolute, destination);
+      if (!samePatchFile) await readStagedOptional(destination, action.moveTo);
+      if (samePatchFile) staged.delete(absolute);
+      staged.set(destination, { content: updated, mode: file.mode });
+      if (!samePatchFile) staged.set(absolute, null);
       patches.push(unifiedFilePatch(action.path, action.moveTo, file.content, updated));
       results.push({ path: action.moveTo, previousPath: action.path, operation: "move" });
     } else {
-      await writeTextFile(absolute, updated, file.mode);
+      staged.set(absolute, { content: updated, mode: file.mode });
       patches.push(unifiedFilePatch(action.path, action.path, file.content, updated));
       results.push({ path: action.path, operation: "update" });
     }
   }
 
+  for (const [absolute, file] of staged) {
+    if (file) await writeTextFile(absolute, file.content, file.mode);
+  }
+
+  for (const [absolute, file] of staged) {
+    if (!file) await rm(absolute, { force: true });
+  }
+
   const unifiedPatch = patches.filter(Boolean).join("\n");
   const stats = countPatchStats(unifiedPatch);
   return { files: results, patch: unifiedPatch, ...stats };
 }
 
-async function readRequiredTextFile(absolute: string, displayPath: string): Promise<TextFile> {
-  if (!(await fileExists(absolute))) throw patchError(`file does not exist: ${displayPath}`);
-  const metadata = await stat(absolute);
-  if (!metadata.isFile()) throw patchError(`path is not a regular file: ${displayPath}`);
-  return { content: await readUtf8Text(absolute, displayPath), mode: metadata.mode };
-}
-
 async function readOptionalTextFile(absolute: string, displayPath: string): Promise<TextFile | null> {
   if (!(await fileExists(absolute))) return null;
   const metadata = await stat(absolute);
@@ -397,73 +440,40 @@ async function writeTextFile(destination: string, content: string, mode?: number
   }
 }
 
-function fileLines(content: string): string[] {
-  if (content.length === 0) return [];
-  const normalized = content.replace(/\r\n/g, "\n");
-  const lines = normalized.split("\n");
-  if (normalized.endsWith("\n")) lines.pop();
-  return lines;
-}
-
-function hunkRange(start: number, count: number): string {
-  return count === 0 ? "0,0" : `${start},${count}`;
-}
-
 function unifiedFilePatch(
   oldPath: string,
   newPath: string,
   oldContent: string | null,
   newContent: string | null,
 ): string {
-  const oldLines = fileLines(oldContent ?? "");
-  const newLines = fileLines(newContent ?? "");
-  let prefix = 0;
-  while (
-    prefix < oldLines.length &&
-    prefix < newLines.length &&
-    oldLines[prefix] === newLines[prefix]
-  ) {
-    prefix += 1;
-  }
-
-  let suffix = 0;
-  while (
-    suffix < oldLines.length - prefix &&
-    suffix < newLines.length - prefix &&
-    oldLines[oldLines.length - 1 - suffix] === newLines[newLines.length - 1 - suffix]
-  ) {
-    suffix += 1;
-  }
-
-  const contextBefore = Math.min(3, prefix);
-  const contextAfter = Math.min(3, suffix);
-  const oldChanged = oldLines.slice(prefix, oldLines.length - suffix);
-  const newChanged = newLines.slice(prefix, newLines.length - suffix);
-  const before = oldLines.slice(prefix - contextBefore, prefix);
-  const after = oldLines.slice(oldLines.length - suffix, oldLines.length - suffix + contextAfter);
-  const oldCount = contextBefore + oldChanged.length + contextAfter;
-  const newCount = contextBefore + newChanged.length + contextAfter;
-  const oldStart = oldContent === null ? 0 : prefix - contextBefore + 1;
-  const newStart = newContent === null ? 0 : prefix - contextBefore + 1;
-  const displayOld = oldContent === null ? "/dev/null" : `a/${oldPath}`;
-  const displayNew = newContent === null ? "/dev/null" : `b/${newPath}`;
+  const oldFileName = oldContent === null ? "/dev/null" : `a/${oldPath}`;
+  const newFileName = newContent === null ? "/dev/null" : `b/${newPath}`;
+  const body = createTwoFilesPatch(
+    oldFileName,
+    newFileName,
+    oldContent ?? "",
+    newContent ?? "",
+    "",
+    "",
+    { context: 3, headerOptions: FILE_HEADERS_ONLY },
+  );
 
   return [
     `diff --git a/${oldPath} b/${newPath}`,
     oldContent === null ? "new file mode 100644" : undefined,
     newContent === null ? "deleted file mode 100644" : undefined,
-    `--- ${displayOld}`,
-    `+++ ${displayNew}`,
-    `@@ -${hunkRange(oldStart, oldCount)} +${hunkRange(newStart, newCount)} @@`,
-    ...before.map((line) => ` ${line}`),
-    ...oldChanged.map((line) => `-${line}`),
-    ...newChanged.map((line) => `+${line}`),
-    ...after.map((line) => ` ${line}`),
+    stripFinalNewline(body),
   ]
     .filter((line): line is string => line !== undefined)
     .join("\n");
 }
 
+function stripFinalNewline(value: string): string {
+  if (value.endsWith("\r\n")) return value.slice(0, -2);
+  if (value.endsWith("\n")) return value.slice(0, -1);
+  return value;
+}
+
 function countPatchStats(patch: string): { additions: number; removals: number } {
   let additions = 0;
   let removals = 0;

src/server.ts

@@ -1082,7 +1082,7 @@ function createMcpServer(
       {
         title: "Apply patch",
         description:
-          "Apply one Codex-style patch inside an open workspace. Supports adding, overwriting, updating, deleting, and moving files. Earlier successful file changes remain if a later patch action fails. Use this for all file modifications. Paths must be relative to the workspace. Call open_workspace first and pass workspaceId.",
+          "Apply one Codex-style patch inside an open workspace. Supports adding, overwriting, updating, deleting, and moving files. Use this for all file modifications. Paths must be relative to the workspace. Call open_workspace first and pass workspaceId.",
         inputSchema: {
           workspaceId: z
             .string()

src/ui/card-types.test.ts

@@ -1,6 +1,8 @@
 import assert from "node:assert/strict";
 import {
   isEditTool,
+  isExpandableCard,
+  isPatchTool,
   isShellTool,
   isToolName,
 } from "./card-types.js";
@@ -9,8 +11,15 @@ for (const tool of ["apply_patch", "exec_command", "write_stdin"]) {
   assert.equal(isToolName(tool), true, `${tool} should be a recognized card tool`);
 }
 
-assert.equal(isEditTool("apply_patch"), true);
+assert.equal(isPatchTool("apply_patch"), true);
+assert.equal(isEditTool("apply_patch"), false);
 assert.equal(isShellTool("exec_command"), true);
 assert.equal(isShellTool("write_stdin"), true);
 assert.equal(isEditTool("exec_command"), false);
 assert.equal(isShellTool("apply_patch"), false);
+
+assert.equal(
+  isExpandableCard({ tool: "apply_patch", payload: { patch: "diff --git a/a b/a" } }),
+  true,
+);
+assert.equal(isExpandableCard({ tool: "apply_patch" }), false);