diff --git a/src/index.ts b/src/index.ts index 0756bad..b8bd869 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,4 @@ -import { mkdir, readFile, realpath, rm, stat } from "node:fs/promises"; +import { mkdir, readFile, rm, stat } from "node:fs/promises"; import path from "node:path"; import type { AgentToolResult } from "@earendil-works/pi-agent-core"; import type { Model } from "@earendil-works/pi-ai"; @@ -1309,50 +1309,8 @@ function replaceApplyPatchWithEditTools(toolNames: string[]): string[] { return [...withoutExtensionManagedEditTools(toolNames), ...STANDARD_EDIT_TOOL_NAMES]; } -function isPathWithinWorkspace(workspacePath: string, candidatePath: string): boolean { - const relativePath = path.relative(workspacePath, candidatePath); - return ( - relativePath === "" || - (!relativePath.startsWith(`..${path.sep}`) && relativePath !== ".." && !path.isAbsolute(relativePath)) - ); -} - -async function findExistingAncestor(directoryPath: string, workspacePath: string): Promise { - let currentPath = directoryPath; - while (isPathWithinWorkspace(workspacePath, currentPath)) { - try { - await stat(currentPath); - return currentPath; - } catch (error) { - if (!hasErrorCode(error, "ENOENT")) { - throw error; - } - } - - const parentPath = path.dirname(currentPath); - if (parentPath === currentPath) { - break; - } - currentPath = parentPath; - } - - throw new PatchApplicationError(`Patch path escapes workspace: ${directoryPath}`); -} - -async function resolvePatchPath(cwd: string, filePath: string): Promise { - const workspacePath = await realpath(cwd); - const absolutePath = path.resolve(workspacePath, filePath); - if (!isPathWithinWorkspace(workspacePath, absolutePath)) { - throw new PatchApplicationError(`Patch path escapes workspace: ${filePath}`); - } - - const existingAncestor = await findExistingAncestor(path.dirname(absolutePath), workspacePath); - const realAncestor = await realpath(existingAncestor); - if (!isPathWithinWorkspace(workspacePath, realAncestor)) { - throw new PatchApplicationError(`Patch path escapes workspace: ${filePath}`); - } - - return absolutePath; +function resolvePatchPath(cwd: string, filePath: string): string { + return path.resolve(cwd, filePath); } function syncToolset( @@ -1453,13 +1411,13 @@ export function createApplyPatchTool(): ApplyPatchToolDefinition { .join("\n"), }, ], - details: { result }, + details: preview ? { preview, result } : { result }, }; } return { content: [{ type: "text", text: result.summaries.join("\n") }], - details: { result }, + details: preview ? { preview, result } : { result }, }; }, renderCall(args, theme, context) { @@ -1476,11 +1434,19 @@ export function createApplyPatchTool(): ApplyPatchToolDefinition { const component = new Container(); const preview = result.details?.preview; if (preview) { - const bgName = options.isPartial ? "toolPendingBg" : "toolSuccessBg"; + const bgName = options.isPartial + ? "toolPendingBg" + : result.details?.result && result.details.result.failures.length > 0 + ? "toolErrorBg" + : "toolSuccessBg"; const progress = result.details?.progress; const title = progress ? `Applying patch (${progress.applied + progress.failed}/${progress.total})` - : "Applying patch"; + : options.isPartial + ? "Applying patch" + : result.details?.result && result.details.result.failures.length > 0 + ? "Patch partially failed" + : "Applied patch"; const box = new Box(1, 1, (text: string) => applyLayeredBackground(theme, bgName, text)); box.addChild(new Text(theme.fg("toolTitle", theme.bold(title)), 0, 0)); box.addChild(new Spacer(1)); diff --git a/test/index.test.ts b/test/index.test.ts index 62fab9f..4eaa58c 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -213,36 +213,45 @@ describe("pi-apply-patch", () => { expect(await readFile(path.join(directory, "sample.txt"), "utf-8")).toBe("after\n"); }); - it("#given parent traversal path #when applying patch #then rejects outside workspace", async () => { + it("#given parent traversal path #when applying patch #then applies outside cwd", async () => { // given const directory = await createTempDirectory(); - const outsidePath = path.join(path.dirname(directory), "outside.ts"); + const outsidePath = path.join(path.dirname(directory), `${path.basename(directory)}-outside.ts`); + const relativeOutsidePath = path.relative(directory, outsidePath); tempDirectories.push(outsidePath); await writeFile(outsidePath, "outside\n", "utf-8"); const patch = `*** Begin Patch -*** Update File: ../outside.ts +*** Update File: ${relativeOutsidePath} @@ -outside +changed *** End Patch`; - // when / then - await expect(applyPatch(directory, patch)).rejects.toThrow("escapes workspace"); - expect(await readFile(outsidePath, "utf-8")).toBe("outside\n"); + // when + await applyPatch(directory, patch); + + // then + expect(await readFile(outsidePath, "utf-8")).toBe("changed\n"); }); - it("#given absolute path outside workspace #when applying patch #then rejects outside workspace", async () => { + it("#given absolute path outside cwd #when applying patch #then applies outside cwd", async () => { // given const directory = await createTempDirectory(); + const outsidePath = path.join(path.dirname(directory), `${path.basename(directory)}-absolute.ts`); + tempDirectories.push(outsidePath); + await writeFile(outsidePath, "outside\n", "utf-8"); const patch = `*** Begin Patch -*** Update File: /etc/passwd +*** Update File: ${outsidePath} @@ --root -+toor +-outside ++changed *** End Patch`; - // when / then - await expect(applyPatch(directory, patch)).rejects.toThrow("escapes workspace"); + // when + await applyPatch(directory, patch); + + // then + expect(await readFile(outsidePath, "utf-8")).toBe("changed\n"); }); it("#given apply_patch tool execution #when started #then emits pending TUI diff update", async () => { @@ -302,6 +311,39 @@ describe("pi-apply-patch", () => { expect(rendered).not.toContain("Index:"); }); + it("#given successful apply_patch tool execution #when rendered #then final result shows diff preview", async () => { + // given + const directory = await createTempDirectory(); + await writeFile(path.join(directory, "sample.txt"), "before\n", "utf-8"); + const patch = `*** Begin Patch +*** Update File: sample.txt +@@ +-before ++after +*** End Patch`; + const tool = createApplyPatchTool(); + + // when + const result = await tool.execute("apply-patch-final-preview-test", { input: patch }, undefined, undefined, { + cwd: directory, + } as never); + const component = tool.renderResult?.( + result, + { expanded: true, isPartial: false }, + identityTheme as never, + { cwd: directory, toolCallId: "apply-patch-final-preview-test", args: { input: patch } } as never, + ); + const rendered = component?.render(120).join("\n") ?? ""; + + // then + expect(result.details?.preview).toBeDefined(); + expect(rendered).toContain("Applied patch"); + expect(rendered).toContain("• Edited sample.txt (+1 -1)"); + expect(rendered).toContain("-1 before"); + expect(rendered).toContain("+1 after"); + expect(await readFile(path.join(directory, "sample.txt"), "utf-8")).toBe("after\n"); + }); + it("#given nested cwd #when previewing absolute workspace path #then formats relative to cwd", async () => { // given const directory = await createTempDirectory(); @@ -668,22 +710,24 @@ EOF`; expect(await readFile(path.join(directory, "new.txt"), "utf-8")).toBe("no trailing newline"); }); - it("#given absolute path outside workspace #when executed #then rejects patch", async () => { + it("#given absolute path outside cwd #when executed #then applies patch", async () => { // given const directory = await createTempDirectory(); - const outsidePath = path.join(path.dirname(directory), "outside-apply-patch.txt"); + const outsidePath = path.join(path.dirname(directory), `${path.basename(directory)}-outside-apply-patch.txt`); tempDirectories.push(outsidePath); const patch = `*** Begin Patch *** Add File: ${outsidePath} +outside *** End Patch`; - // when / then - await expect(applyPatch(directory, patch)).rejects.toThrow("escapes workspace"); - await expect(readFile(outsidePath, "utf-8")).rejects.toMatchObject({ code: "ENOENT" }); + // when + await applyPatch(directory, patch); + + // then + expect(await readFile(outsidePath, "utf-8")).toBe("outside\n"); }); - it("#given symlink escaping workspace #when executed #then rejects patch", async () => { + it("#given symlink escaping cwd #when executed #then applies patch", async () => { // given const directory = await createTempDirectory(); const outsideDirectory = await createTempDirectory(); @@ -693,11 +737,11 @@ EOF`; +outside *** End Patch`; - // when / then - await expect(applyPatch(directory, patch)).rejects.toThrow("escapes workspace"); - await expect(readFile(path.join(outsideDirectory, "outside.txt"), "utf-8")).rejects.toMatchObject({ - code: "ENOENT", - }); + // when + await applyPatch(directory, patch); + + // then + expect(await readFile(path.join(outsideDirectory, "outside.txt"), "utf-8")).toBe("outside\n"); }); it("#given empty codex patch #when applying #then throws typed parse error", async () => {