diff --git a/CHANGELOG.md b/CHANGELOG.md index 11e2fa4..396483f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Recognize indented file headers like Codex, and reject any other line between file sections instead of silently dropping that section while reporting success (#45). + ### Changed - Docs: install from GitHub instead of npm. diff --git a/src/index.ts b/src/index.ts index c47d4f5..8d69839 100644 --- a/src/index.ts +++ b/src/index.ts @@ -437,10 +437,35 @@ function seekSequence( return undefined; } +const FILE_HEADER_MARKERS = [ + ["add", "*** Add File: "], + ["delete", "*** Delete File: "], + ["update", "*** Update File: "], +] as const; +const MOVE_TO_MARKER = "*** Move to: "; + +// The parser and extractPatchedPaths both read headers through these two functions, so they share one +// whitespace definition (String.prototype.trim): any path the parser writes is a path consumers that +// gate writes per file (permission prompts) see, spelled the same way. +function parseFileHeader(line: string): { kind: ParsedPatch["type"]; path: string } | undefined { + const trimmed = line.trim(); + for (const [kind, marker] of FILE_HEADER_MARKERS) { + if (trimmed.startsWith(marker)) return { kind, path: trimmed.slice(marker.length) }; + } + return undefined; +} + +function parseMoveTo(line: string): string | undefined { + const trimmed = line.trimEnd(); + return trimmed.startsWith(MOVE_TO_MARKER) ? trimmed.slice(MOVE_TO_MARKER.length) : undefined; +} + export function extractPatchedPaths(patchText: string): string[] { - const normalized = stripHeredoc(normalizePatchText(patchText)); - const matches = normalized.matchAll(/^\*\*\* (?:(?:Add|Delete|Update) File|Move to): (.+)$/gm); - return Array.from(matches, (match) => match[1] ?? ""); + const lines = stripHeredoc(normalizePatchText(patchText)).split("\n"); + return lines.flatMap((line) => { + const path = parseFileHeader(line)?.path ?? parseMoveTo(line.trimStart()); + return path === undefined ? [] : [path]; + }); } function createPatchDiff(oldContent: string, newContent: string): { diff: string; added: number; removed: number } { @@ -924,19 +949,22 @@ function parsePatch(patchText: string): ParsedPatch[] { const hunks: ParsedPatch[] = []; let index = beginIndex + 1; while (index < endIndex) { - const line = lines[index] ?? ""; - if (!line.startsWith("*** ")) { + // Like Codex, file headers are recognized after trimming, and any other non-blank line between file + // sections is rejected below; skipping it would silently drop the section it introduces. + const line = (lines[index] ?? "").trim(); + if (line === "") { index++; continue; } + const header = parseFileHeader(line); - if (line.startsWith("*** Add File: ")) { - const filePath = line.slice("*** Add File: ".length); + if (header?.kind === "add") { + const filePath = header.path; index++; const contentLines: string[] = []; while (index < endIndex) { const nextLine = lines[index] ?? ""; - if (nextLine.startsWith("*** ")) { + if (nextLine.trim().startsWith("*** ")) { break; } if (!nextLine.startsWith("+")) { @@ -953,18 +981,19 @@ function parsePatch(patchText: string): ParsedPatch[] { continue; } - if (line.startsWith("*** Delete File: ")) { - hunks.push({ type: "delete", filePath: line.slice("*** Delete File: ".length) }); + if (header?.kind === "delete") { + hunks.push({ type: "delete", filePath: header.path }); index++; continue; } - if (line.startsWith("*** Update File: ")) { - const filePath = line.slice("*** Update File: ".length); + if (header?.kind === "update") { + const filePath = header.path; index++; let movePath: string | undefined; - if ((lines[index] ?? "").startsWith("*** Move to: ")) { - movePath = (lines[index] ?? "").slice("*** Move to: ".length); + const moveTo = parseMoveTo(lines[index] ?? ""); + if (moveTo !== undefined) { + movePath = moveTo; index++; } diff --git a/test/patch-headers.test.ts b/test/patch-headers.test.ts new file mode 100644 index 0000000..3ae9cd9 --- /dev/null +++ b/test/patch-headers.test.ts @@ -0,0 +1,139 @@ +import { mkdtemp, readFile, rm, stat, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { applyPatchDetailed, extractPatchedPaths, PatchParseError } from "../src/index.js"; + +const tempDirectories: string[] = []; + +async function workspace(files: Record): Promise { + const cwd = await mkdtemp(path.join(tmpdir(), "pi-apply-patch-headers-")); + tempDirectories.push(cwd); + for (const [name, content] of Object.entries(files)) { + await writeFile(path.join(cwd, name), content); + } + return cwd; +} + +async function exists(filePath: string): Promise { + return stat(filePath).then( + () => true, + () => false, + ); +} + +afterEach(async () => { + await Promise.all(tempDirectories.splice(0).map((directory) => rm(directory, { recursive: true, force: true }))); +}); + +describe("apply_patch file headers", () => { + it("#given an update header indented with spaces #when applied #then the file is updated like Codex", async () => { + // given + const cwd = await workspace({ "foo.txt": "old\n" }); + + // when + const result = await applyPatchDetailed( + cwd, + "*** Begin Patch\n *** Update File: foo.txt\n@@\n-old\n+new\n*** End Patch\n", + ); + + // then + expect(result.failures).toEqual([]); + expect(await readFile(path.join(cwd, "foo.txt"), "utf8")).toBe("new\n"); + }); + + it("#given an indented update header after a delete section #when applied #then both files change", async () => { + // given + const cwd = await workspace({ "old.txt": "x\n", "b.txt": "b1\n" }); + + // when + const result = await applyPatchDetailed( + cwd, + "*** Begin Patch\n*** Delete File: old.txt\n *** Update File: b.txt\n@@\n-b1\n+B1\n*** End Patch\n", + ); + + // then + expect(result.failures).toEqual([]); + expect(result.appliedFiles).toEqual(["old.txt", "b.txt"]); + expect(await exists(path.join(cwd, "old.txt"))).toBe(false); + expect(await readFile(path.join(cwd, "b.txt"), "utf8")).toBe("B1\n"); + }); + + it("#given an indented header right after added file content #when applied #then the next section still applies", async () => { + // given + const cwd = await workspace({ "b.txt": "b1\n" }); + + // when + const result = await applyPatchDetailed( + cwd, + "*** Begin Patch\n*** Add File: new.txt\n+hello\n *** Update File: b.txt\n@@\n-b1\n+B1\n*** End Patch\n", + ); + + // then + expect(result.failures).toEqual([]); + expect(await readFile(path.join(cwd, "new.txt"), "utf8")).toBe("hello\n"); + expect(await readFile(path.join(cwd, "b.txt"), "utf8")).toBe("B1\n"); + }); + + it("#given a stray line between file sections #when applied #then the whole patch is rejected and nothing changes", async () => { + // given + const cwd = await workspace({ "old.txt": "x\n", "b.txt": "b1\n" }); + const patch = "*** Begin Patch\n*** Delete File: old.txt\nnow update b\n@@\n-b1\n+B1\n*** End Patch\n"; + + // when + const applying = applyPatchDetailed(cwd, patch); + + // then + await expect(applying).rejects.toThrow(PatchParseError); + await expect(applyPatchDetailed(cwd, patch)).rejects.toThrow("'now update b' is not a valid hunk header"); + expect(await exists(path.join(cwd, "old.txt"))).toBe(true); + expect(await readFile(path.join(cwd, "b.txt"), "utf8")).toBe("b1\n"); + }); + + it("#given a misspelled header as the first section #when applied #then it is rejected instead of reporting no hunks", async () => { + // given + const cwd = await workspace({ "b.txt": "b1\n" }); + + // when + const applying = applyPatchDetailed(cwd, "*** Begin Patch\nUpdate File: b.txt\n@@\n-b1\n+B1\n*** End Patch\n"); + + // then + await expect(applying).rejects.toThrow("'Update File: b.txt' is not a valid hunk header"); + expect(await readFile(path.join(cwd, "b.txt"), "utf8")).toBe("b1\n"); + }); + + it("#given an indented header inside an update hunk #when applied #then it is matched as a context line like Codex", async () => { + // given + const cwd = await workspace({ "notes.md": "intro\n *** Update File: x\nend\n" }); + + // when + const result = await applyPatchDetailed( + cwd, + "*** Begin Patch\n*** Update File: notes.md\n@@\n intro\n *** Update File: x\n-end\n+END\n*** End Patch\n", + ); + + // then + expect(result.failures).toEqual([]); + expect(await readFile(path.join(cwd, "notes.md"), "utf8")).toBe("intro\n *** Update File: x\nEND\n"); + }); + + it("#given headers indented or padded with any whitespace #when applied #then the listed paths are exactly the files written", async () => { + // given + const cwd = await workspace({ "old.txt": "x\n", "b.txt": "b1\n", "c.txt": "c1\n" }); + const patch = + "*** Begin Patch\n*** Delete File: old.txt\n\u00A0*** Update File: b.txt\u00A0\n@@\n-b1\n+B1\n" + + "*** Update File: c.txt\u00A0\n*** Move to: moved.txt \n@@\n-c1\n+C1\n*** End Patch\n"; + + // when + const listed = extractPatchedPaths(patch); + const result = await applyPatchDetailed(cwd, patch); + + // then + expect(result.failures).toEqual([]); + expect(listed).toEqual(["old.txt", "b.txt", "c.txt", "moved.txt"]); + expect(result.appliedFiles).toEqual(["old.txt", "b.txt", "moved.txt"]); + expect(await readFile(path.join(cwd, "b.txt"), "utf8")).toBe("B1\n"); + expect(await readFile(path.join(cwd, "moved.txt"), "utf8")).toBe("C1\n"); + expect(await exists(path.join(cwd, "c.txt"))).toBe(false); + }); +});