diff --git a/package-lock.json b/package-lock.json index 6c1f889..c226ef4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,6 +16,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", diff --git a/package.json b/package.json index 8b1e6e0..e16711b 100644 --- a/package.json +++ b/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", diff --git a/src/apply-patch.test.ts b/src/apply-patch.test.ts index 202dda0..c6f0869 100644 --- a/src/apply-patch.test.ts +++ b/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/); diff --git a/src/apply-patch.ts b/src/apply-patch.ts index ed1832d..32ec4a4 100644 --- a/src/apply-patch.ts +++ b/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; +type FileIdentityReader = (path: string) => Promise; + 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 { + 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 { const actions = parsePatch(patch); const results: AppliedPatchFile[] = []; const patches: string[] = []; + const staged = new Map(); + + const readStagedOptional = async (absolute: string, displayPath: string): Promise => { + 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 => { + 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 { - 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 { 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; diff --git a/src/server.ts b/src/server.ts index 622d4c8..da24131 100644 --- a/src/server.ts +++ b/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() diff --git a/src/ui/card-types.test.ts b/src/ui/card-types.test.ts index 6a98ff4..eb47e9a 100644 --- a/src/ui/card-types.test.ts +++ b/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); diff --git a/src/ui/card-types.ts b/src/ui/card-types.ts index f8a136e..1e3c940 100644 --- a/src/ui/card-types.ts +++ b/src/ui/card-types.ts @@ -16,6 +16,8 @@ export type ToolName = export type HostContext = NonNullable>; +export type PatchOperation = "add" | "update" | "delete" | "move"; + export interface ToolResultCard { tool: ToolName; workspaceId?: string; @@ -26,6 +28,7 @@ export interface ToolResultCard { files?: Array<{ path?: string; previousPath?: string; + operation?: PatchOperation; type?: string; additions?: number; removals?: number; @@ -86,7 +89,11 @@ export function isWriteTool(tool: ToolName): boolean { } export function isEditTool(tool: ToolName): boolean { - return tool === "edit" || tool === "apply_patch"; + return tool === "edit"; +} + +export function isPatchTool(tool: ToolName): boolean { + return tool === "apply_patch"; } export function isSearchTool(tool: ToolName): boolean { @@ -139,6 +146,7 @@ export function isExpandableCard(card: ToolResultCard): boolean { } if (isReviewTool(card.tool)) return Boolean(card.files?.length || card.payload?.patch); + if (isPatchTool(card.tool)) return Boolean(card.payload?.patch); return Boolean(card.payload); } diff --git a/src/ui/patch-display.test.ts b/src/ui/patch-display.test.ts new file mode 100644 index 0000000..5583ace --- /dev/null +++ b/src/ui/patch-display.test.ts @@ -0,0 +1,70 @@ +import assert from "node:assert/strict"; +import { getPatchDisplayParts } from "./patch-display.js"; + +assert.deepEqual(getPatchDisplayParts({}), { + title: "Apply Patch", + tone: "edit", +}); + +assert.deepEqual( + getPatchDisplayParts({ files: [{ path: "created.ts", operation: "add" }] }), + { + title: "Write File", + iconOperation: "add", + tone: "write", + }, +); + +assert.deepEqual( + getPatchDisplayParts({ + files: [ + { path: "a.ts", operation: "add" }, + { path: "b.ts", operation: "add" }, + ], + }), + { + title: "Write Files", + iconOperation: "add", + tone: "write", + }, +); + +assert.deepEqual( + getPatchDisplayParts({ + files: [ + { path: "created.ts", operation: "add" }, + { path: "edited.ts", operation: "update" }, + ], + }), + { + title: "Write & Edit Files", + tone: "edit", + }, +); + +assert.deepEqual( + getPatchDisplayParts({ + files: [ + { path: "same.ts", operation: "add" }, + { path: "same.ts", operation: "update" }, + ], + }), + { + title: "Write & Edit File", + tone: "edit", + }, +); + +assert.deepEqual( + getPatchDisplayParts({ + files: [ + { path: "edited.ts", operation: "update" }, + { path: "moved.ts", previousPath: "old.ts", operation: "move" }, + { path: "removed.ts", operation: "delete" }, + ], + }), + { + title: "Edit, Move & Delete Files", + tone: "edit", + }, +); diff --git a/src/ui/patch-display.ts b/src/ui/patch-display.ts new file mode 100644 index 0000000..9263233 --- /dev/null +++ b/src/ui/patch-display.ts @@ -0,0 +1,74 @@ +import type { PatchOperation, ToolResultCard } from "./card-types.js"; + +export interface PatchDisplayParts { + title: string; + iconOperation?: PatchOperation; + tone: "edit" | "write"; +} + +const patchOperationLabels: Record = { + add: "Write", + update: "Edit", + delete: "Delete", + move: "Move", +}; + +export function getPatchDisplayParts(card: Pick): PatchDisplayParts { + const files = card.files ?? []; + const operations = patchOperations(files); + + if (operations.length === 0) { + return { title: "Apply Patch", tone: "edit" }; + } + + const singleOperation = operations.length === 1 ? operations[0] : undefined; + + const display: PatchDisplayParts = { + title: patchTitle(operations, countChangedFiles(files)), + tone: singleOperation === "add" ? "write" : "edit", + }; + if (singleOperation) display.iconOperation = singleOperation; + return display; +} + +function patchOperations(files: NonNullable): PatchOperation[] { + const operations = new Set(); + for (const file of files) { + if (file.operation) operations.add(file.operation); + } + return [...operations]; +} + +function countChangedFiles(files: NonNullable): number { + const paths = new Set(); + let unnamedFiles = 0; + + for (const file of files) { + const path = file.path ?? file.previousPath; + if (path) { + paths.add(path); + } else { + unnamedFiles += 1; + } + } + + return paths.size + unnamedFiles; +} + +function patchTitle(operations: PatchOperation[], fileCount: number): string { + if (operations.length === 1) { + return `${patchOperationLabels[operations[0]]} ${fileNoun(fileCount)}`; + } + + return `${joinTitleParts(operations.map((operation) => patchOperationLabels[operation]))} ${fileNoun(fileCount)}`; +} + +function fileNoun(fileCount: number): "File" | "Files" { + return fileCount === 1 ? "File" : "Files"; +} + +function joinTitleParts(parts: string[]): string { + if (parts.length <= 1) return parts[0] ?? ""; + if (parts.length === 2) return `${parts[0]} & ${parts[1]}`; + return `${parts.slice(0, -1).join(", ")} & ${parts.at(-1)}`; +} diff --git a/src/ui/workspace-app.tsx b/src/ui/workspace-app.tsx index 177a91e..edbb620 100644 --- a/src/ui/workspace-app.tsx +++ b/src/ui/workspace-app.tsx @@ -8,6 +8,7 @@ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { isEditTool, isExpandableCard, + isPatchTool, isReadTool, isReviewTool, isSearchTool, @@ -18,9 +19,11 @@ import { payloadText, summaryNumber, type HostContext, + type PatchOperation, type ToolName, type ToolResultCard, } from "./card-types.js"; +import { getPatchDisplayParts } from "./patch-display.js"; import "./workspace-app.css"; interface ToolDisplay { @@ -261,17 +264,17 @@ async function renderPayloadIfNeeded(): Promise { return; } - if (isReviewTool(card.tool)) { - const visibleFileCount = reviewFilesExpanded - ? undefined - : Math.max(3, (card.files ?? []).slice(0, 3).length); + if (isReviewTool(card.tool) || isPatchTool(card.tool)) { + const visibleFileCount = isReviewTool(card.tool) && !reviewFilesExpanded + ? Math.max(3, (card.files ?? []).slice(0, 3).length) + : undefined; if (currentPayload) { currentPayload.update({ card, hostContext, errorMessage, visibleFileCount }); return; } - renderStatus(target, "Loading review..."); + renderStatus(target, isReviewTool(card.tool) ? "Loading review..." : "Loading diff..."); const { mountReviewPayload } = await import("./review-payload.js"); if (target !== currentPayloadContainer || !card) return; @@ -340,7 +343,7 @@ function renderSummaryBadge(card: ToolResultCard): HTMLElement { return stats; } - if (isEditTool(card.tool) || isWriteTool(card.tool)) { + if (isPatchTool(card.tool) || isEditTool(card.tool) || isWriteTool(card.tool)) { const stats = element("span", { className: "stats" }); stats.setAttribute("aria-label", "Diff statistics"); stats.append( @@ -490,6 +493,24 @@ function formatAgentsFilesForPayload( .join("\n\n"); } +function getPatchToolDisplay(card: ToolResultCard, label: string): ToolDisplay { + const display = getPatchDisplayParts(card); + + return { + icon: patchIcon(display.iconOperation), + title: display.title, + label, + tone: display.tone, + }; +} + +function patchIcon(operation: PatchOperation | undefined): string { + if (operation === "add") return filePlusIcon(); + if (operation === "delete") return fileIcon(); + if (operation === "move") return filesIcon(); + return editIcon(); +} + function getToolDisplay(card: ToolResultCard): ToolDisplay { const label = getToolLabel(card); @@ -503,7 +524,7 @@ function getToolDisplay(card: ToolResultCard): ToolDisplay { case "edit": return { icon: editIcon(), title: "Edit File", label, tone: "edit" }; case "apply_patch": - return { icon: editIcon(), title: "Apply Patch", label, tone: "edit" }; + return getPatchToolDisplay(card, label); case "grep": return { icon: searchIcon(), title: "Grep", label, tone: "search" }; case "glob":