fix: reject stray lines between file sections instead of silently dropping them

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.

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++;
 			}
 

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<string, string>): Promise<string> {
+	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<boolean> {
+	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);
+	});
+});