From b2a128929597e4374e3f903b821fded640c5542b Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 3 Jun 2026 09:49:56 +0000 Subject: [PATCH] fix: clarify apply-patch delete-file misuse --- scripts/src/apply-patch-v2.ts | 27 +++++++++++++++++++++- scripts/ssh-argv-guidance-contract-test.ts | 19 +++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/scripts/src/apply-patch-v2.ts b/scripts/src/apply-patch-v2.ts index fca0c65d..204650f2 100644 --- a/scripts/src/apply-patch-v2.ts +++ b/scripts/src/apply-patch-v2.ts @@ -188,8 +188,13 @@ export function parseApplyPatchV2(patchText: string): PatchParseResult { continue; } if (line.startsWith(deleteFileMarker)) { - hunks.push({ kind: "delete", path: validatePatchPath(line.slice(deleteFileMarker.length), index + 1) }); + const filePath = validatePatchPath(line.slice(deleteFileMarker.length), index + 1); index += 1; + const nextContentIndex = nextNonBlankLineIndex(lines, index, lines.length - 1); + if (nextContentIndex < lines.length - 1 && !isFileHeader(lines[nextContentIndex] ?? "")) { + throw invalidDeleteFileLineError(lines[nextContentIndex] ?? "", nextContentIndex + 1, filePath); + } + hunks.push({ kind: "delete", path: filePath }); continue; } if (line.startsWith(updateFileMarker)) { @@ -240,6 +245,26 @@ function invalidAddFileLineError(lineText: string, line: number, filePath: strin ); } +function invalidDeleteFileLineError(lineText: string, line: number, filePath: string): ApplyPatchV2Error { + const details = { line, path: filePath, text: lineText }; + if (lineText.trimStart().startsWith("@@")) { + return new ApplyPatchV2Error( + "invalid delete file line: remove @@ for Delete File. Delete File sections contain only the `*** Delete File: ` header; do not include hunk markers or file content.", + details, + ); + } + return new ApplyPatchV2Error( + "invalid delete file line: Delete File sections contain only the `*** Delete File: ` header. Remove old file content lines such as -old line.", + details, + ); +} + +function nextNonBlankLineIndex(lines: string[], start: number, endExclusive: number): number { + let index = start; + while (index < endExclusive && (lines[index] ?? "").trim().length === 0) index += 1; + return index; +} + export function deriveUpdatedContent(filePath: string, originalContent: string, chunks: UpdateChunk[]): PatchUpdateResult { const originalLines = splitContentLines(originalContent); const replacements = computeReplacements(filePath, originalLines, chunks); diff --git a/scripts/ssh-argv-guidance-contract-test.ts b/scripts/ssh-argv-guidance-contract-test.ts index e695a689..640684f1 100644 --- a/scripts/ssh-argv-guidance-contract-test.ts +++ b/scripts/ssh-argv-guidance-contract-test.ts @@ -923,6 +923,25 @@ export async function runSshArgvGuidanceContract(): Promise { ); assertCondition(!addFileWithHunkMarkerV2.commands.some((command) => command.startsWith("write-b64") || command.startsWith("delete")), "v2 Add File parse failures must not touch remote files", addFileWithHunkMarkerV2.commands); + const deleteFileWithHunkMarkerV2 = await applyPatchV2FixtureAttempt([ + "*** Begin Patch", + "*** Delete File: bad-delete.txt", + "@@", + "-content", + "*** End Patch", + "", + ].join("\n"), { "bad-delete.txt": "content\n" }, { stderrOutput: true }); + assertCondition(deleteFileWithHunkMarkerV2.exitCode === 1 && deleteFileWithHunkMarkerV2.error === null, "v2 CLI path should return a visible parse error for Delete File @@ misuse", deleteFileWithHunkMarkerV2); + assertCondition(deleteFileWithHunkMarkerV2.stdout === "", "v2 Delete File parse failures should keep Codex-style empty stdout", deleteFileWithHunkMarkerV2); + assertCondition( + deleteFileWithHunkMarkerV2.stderr.includes("remove @@ for Delete File") + && deleteFileWithHunkMarkerV2.stderr.includes("do not include hunk markers or file content"), + "v2 Delete File @@ misuse should tell agents exactly how to repair the patch", + deleteFileWithHunkMarkerV2.stderr, + ); + assertCondition(deleteFileWithHunkMarkerV2.files["bad-delete.txt"] === "content\n", "v2 Delete File parse failures must not delete remote files", deleteFileWithHunkMarkerV2); + assertCondition(!deleteFileWithHunkMarkerV2.commands.some((command) => command.startsWith("delete")), "v2 Delete File parse failures must not issue remote delete operations", deleteFileWithHunkMarkerV2.commands); + const sequentialCompoundV2 = await applyPatchV2Fixture([ "*** Begin Patch", "*** Update File: sequence.txt",