fix(vcs): preserve batched patch boundaries (#25787)

This commit is contained in:
Luke Parker
2026-05-05 00:34:06 +00:00
committed by GitHub
parent 4b65b1e053
commit 6a5e329427
4 changed files with 65 additions and 17 deletions
+3 -1
View File
@@ -85,7 +85,9 @@ const fileFromPatchChunk = (chunk: string) => {
} }
const splitGitPatch = (patch: Git.Patch) => { const splitGitPatch = (patch: Git.Patch) => {
const starts = [...patch.text.matchAll(/^diff --git /gm)].map((match) => match.index) const starts = [...patch.text.matchAll(/(?:^|\n)diff --git /g)].map((match) =>
match[0].startsWith("\n") ? match.index + 1 : match.index,
)
const chunks = starts.map((start, index) => patch.text.slice(start, starts[index + 1] ?? patch.text.length)) const chunks = starts.map((start, index) => patch.text.slice(start, starts[index + 1] ?? patch.text.length))
if (!patch.truncated) return chunks if (!patch.truncated) return chunks
return chunks.slice(0, -1) return chunks.slice(0, -1)
@@ -1,5 +1,6 @@
import { $ } from "bun" import { $ } from "bun"
import { afterEach, describe, expect, test } from "bun:test" import { afterEach, describe, expect, test } from "bun:test"
import { parsePatch } from "diff"
import { Effect } from "effect" import { Effect } from "effect"
import fs from "fs/promises" import fs from "fs/promises"
import path from "path" import path from "path"
@@ -288,6 +289,32 @@ describe("Vcs diff", () => {
}) })
}) })
test(
"diff('git') keeps carriage returns inside patch hunks",
async () => {
await using tmp = await tmpdir({ git: true })
await fs.writeFile(path.join(tmp.path, "file.txt"), "keep\nsame\rdiff --git inside\ndelete\n", "utf-8")
await $`git add .`.cwd(tmp.path).quiet()
await $`git commit --no-gpg-sign -m "add file"`.cwd(tmp.path).quiet()
await fs.writeFile(path.join(tmp.path, "file.txt"), "keep\nadd\nsame\rdiff --git inside\n", "utf-8")
await withVcsOnly(tmp.path, async () => {
const diff = await AppRuntime.runPromise(
Effect.gen(function* () {
const vcs = yield* Vcs.Service
return yield* vcs.diff("git")
}),
)
const file = diff.find((item) => item.file === "file.txt")
expect(file?.patch).toContain(" same\rdiff --git inside")
expect(file?.patch).toContain("-delete")
expect(() => parsePatch(file?.patch ?? "")).not.toThrow()
})
},
20_000,
)
test("diff('branch') returns changes against default branch", async () => { test("diff('branch') returns changes against default branch", async () => {
await using tmp = await tmpdir({ git: true }) await using tmp = await tmpdir({ git: true })
await $`git branch -M main`.cwd(tmp.path).quiet() await $`git branch -M main`.cwd(tmp.path).quiet()
@@ -34,4 +34,20 @@ describe("session diff", () => {
expect(text(view, "deletions")).toBe("one\n") expect(text(view, "deletions")).toBe("one\n")
expect(text(view, "additions")).toBe("two\n") expect(text(view, "additions")).toBe("two\n")
}) })
test("ignores malformed persisted patches", () => {
const diff = {
file: "a.ts",
patch:
"diff --git a/a.ts b/a.ts\nindex ff4ceb2..65a1de0 100644\n--- a/a.ts\n+++ b/a.ts\n@@ -1,3 +1,3 @@\n keep\n+add\n same\r",
additions: 1,
deletions: 1,
status: "modified" as const,
}
const view = normalize(diff)
expect(view.patch).toBe(diff.patch)
expect(text(view, "deletions")).toBe("")
expect(text(view, "additions")).toBe("")
})
}) })
+19 -16
View File
@@ -27,26 +27,29 @@ const cache = new Map<string, FileDiffMetadata>()
function patch(diff: ReviewDiff) { function patch(diff: ReviewDiff) {
if (typeof diff.patch === "string") { if (typeof diff.patch === "string") {
const [patch] = parsePatch(diff.patch) try {
const [patch] = parsePatch(diff.patch)
const beforeLines = []
const afterLines = []
const beforeLines = [] for (const hunk of patch.hunks) {
const afterLines = [] for (const line of hunk.lines) {
if (line.startsWith("-")) {
for (const hunk of patch.hunks) { beforeLines.push(line.slice(1))
for (const line of hunk.lines) { } else if (line.startsWith("+")) {
if (line.startsWith("-")) { afterLines.push(line.slice(1))
beforeLines.push(line.slice(1)) } else {
} else if (line.startsWith("+")) { // context line (starts with ' ')
afterLines.push(line.slice(1)) beforeLines.push(line.slice(1))
} else { afterLines.push(line.slice(1))
// context line (starts with ' ') }
beforeLines.push(line.slice(1))
afterLines.push(line.slice(1))
} }
} }
}
return { before: beforeLines.join("\n"), after: afterLines.join("\n"), patch: diff.patch } return { before: beforeLines.join("\n"), after: afterLines.join("\n"), patch: diff.patch }
} catch {
return { before: "", after: "", patch: diff.patch }
}
} }
return { return {
before: "before" in diff && typeof diff.before === "string" ? diff.before : "", before: "before" in diff && typeof diff.before === "string" ? diff.before : "",