diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index 3ac8a99637..47d2aa76ea 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -539,10 +539,129 @@ describe("checkout git utilities", () => { }); expect(diff.structured).toContainEqual( - expect.objectContaining({ path: "renamed.txt", oldPath: "file.txt" }), + expect.objectContaining({ + path: "renamed.txt", + oldPath: "file.txt", + additions: 0, + deletions: 0, + hunks: [], + }), ); }); + it.each([ + { mode: "uncommitted", oldPath: "to-rename.txt", path: "renamed.txt" }, + { mode: "base", oldPath: "to-rename.txt", path: "renamed.txt" }, + { mode: "uncommitted", oldPath: "old [file].txt", path: "new [file].txt" }, + ] as const)( + "shows only the edited line of a rename ($mode, $path)", + async ({ mode, oldPath, path }) => { + const original = Array.from({ length: 12 }, (_, index) => `line ${index + 1}`); + commitFile(repoDir, oldPath, original.join("\n") + "\n", "file to rename"); + if (mode === "base") { + execFileSync("git", ["checkout", "-b", "feature/rename"], { cwd: repoDir }); + } + execFileSync("git", ["mv", oldPath, path], { cwd: repoDir }); + const edited = [...original]; + edited[5] = "edited line 6"; + writeFileSync(join(repoDir, path), edited.join("\n") + "\n"); + execFileSync("git", ["add", path], { cwd: repoDir }); + + expect( + execFileSync("git", ["diff", "--name-status", "HEAD"], { cwd: repoDir }).toString(), + ).toMatch(/R\d+\t/); + if (mode === "base") { + execFileSync("git", ["-c", "commit.gpgsign=false", "commit", "-m", "rename and edit"], { + cwd: repoDir, + }); + } + + const diff = await getCheckoutDiff(repoDir, { + mode, + ...(mode === "base" ? { baseRef: "main" } : {}), + includeStructured: true, + }); + + expect(diff.structured).toEqual([ + expect.objectContaining({ + path, + oldPath, + isNew: false, + isDeleted: false, + additions: 1, + deletions: 1, + status: "ok", + }), + ]); + expect(diff.diff).toContain(`rename from ${oldPath}`); + expect(diff.diff).toContain(`rename to ${path}`); + expect(diff.diff).toContain("-line 6\n+edited line 6"); + expect(diff.diff).not.toContain("new file mode"); + }, + ); + + it("includes a changed copy source only once in the diff", async () => { + const original = + Array.from({ length: 12 }, (_, index) => `line ${index + 1}`).join("\n") + "\n"; + commitFile(repoDir, "source.txt", original, "copy source"); + execFileSync("git", ["config", "diff.renames", "copies"], { cwd: repoDir }); + writeFileSync(join(repoDir, "copy.txt"), original); + writeFileSync(join(repoDir, "source.txt"), original.replace("line 6", "edited line 6")); + execFileSync("git", ["add", "copy.txt", "source.txt"], { cwd: repoDir }); + expect( + execFileSync("git", ["diff", "--name-status", "HEAD"], { cwd: repoDir }).toString(), + ).toContain("C100\tsource.txt\tcopy.txt"); + + const diff = await getCheckoutDiff(repoDir, { + mode: "uncommitted", + includeStructured: true, + }); + expect(diff.structured).toEqual([ + expect.objectContaining({ + path: "copy.txt", + oldPath: "source.txt", + additions: 0, + deletions: 0, + }), + expect.objectContaining({ path: "source.txt", additions: 1, deletions: 1 }), + ]); + expect(diff.diff.match(/^diff --git a\/source.txt b\/source.txt$/gm)).toHaveLength(1); + expect(diff.diff).toContain("copy from source.txt"); + expect(diff.diff).toContain("copy to copy.txt"); + }); + + it("keeps a small copy patch when its edited source exceeds the patch limit", async () => { + const original = + Array.from({ length: 12 }, (_, index) => `line ${index + 1}`).join("\n") + "\n"; + commitFile(repoDir, "source.txt", original, "copy source"); + execFileSync("git", ["config", "diff.renames", "copies"], { cwd: repoDir }); + writeFileSync(join(repoDir, "copy.txt"), original); + writeFileSync(join(repoDir, "source.txt"), original + "added source line\n".repeat(80_000)); + execFileSync("git", ["add", "copy.txt", "source.txt"], { cwd: repoDir }); + expect( + execFileSync("git", ["diff", "--name-status", "HEAD"], { cwd: repoDir }).toString(), + ).toContain("C100\tsource.txt\tcopy.txt"); + + const diff = await getCheckoutDiff(repoDir, { + mode: "uncommitted", + includeStructured: true, + }); + expect(diff.structured).toEqual([ + expect.objectContaining({ + path: "copy.txt", + oldPath: "source.txt", + additions: 0, + deletions: 0, + hunks: [], + status: "ok", + }), + expect.objectContaining({ path: "source.txt", status: "too_large", hunks: [] }), + ]); + expect(diff.diff).toContain("copy from source.txt"); + expect(diff.diff).toContain("copy to copy.txt"); + expect(diff.diff).not.toContain("+added source line"); + }); + it("reads the origin URL once when collecting facts for an origin-tracking branch", async () => { setupRemoteTrackingMain(repoDir, tempDir); diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index f663a52a63..9b31fd6665 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -721,12 +721,19 @@ async function getTrackedDiffTextForPath(input: { cwd: string; refsForDiff: CheckoutDiffRefs; path: string; + oldPath?: string; ignoreWhitespace: boolean; }): Promise<{ path: string; text: string; truncated: boolean }> { + const paths = input.oldPath ? [input.oldPath, input.path] : [input.path]; const result = await runGitCommand( buildGitDiffArgs({ ignoreWhitespace: input.ignoreWhitespace, - extra: [...getCheckoutDiffRefArgs(input.refsForDiff), "--", `:(literal)${input.path}`], + extra: [ + ...(input.oldPath ? ["--diff-filter=RC"] : []), + ...getCheckoutDiffRefArgs(input.refsForDiff), + "--", + ...paths.map((path) => `:(literal)${path}`), + ], }), { cwd: input.cwd, @@ -735,9 +742,22 @@ async function getTrackedDiffTextForPath(input: { }, ); + // A source path can participate in another rename/copy in the same request. + // Return only the requested destination; the filter keeps source edits out of its budget. + let text = result.stdout; + if (input.oldPath && !result.truncated) { + text = result.stdout + .split(/(?=^diff --git )/m) + .filter((section) => { + const hunkStart = section.indexOf("\n@@ "); + const header = hunkStart < 0 ? section : section.slice(0, hunkStart); + return parseDiff(header)[0]?.path === input.path; + }) + .join(""); + } return { path: input.path, - text: result.stdout, + text, truncated: result.truncated, }; } @@ -3314,26 +3334,29 @@ async function processTrackedChanges( let trackedDiffText = ""; let trackedDiffBytes = 0; - const renamedPaths = new Set( - trackedChanges.filter((change) => change.oldPath).map((change) => change.path), + const oldPathByPath = new Map( + trackedChanges + .filter((change) => change.oldPath) + .map((change) => [change.path, change.oldPath]), ); for (let start = 0; start < trackedDiffPaths.length; start += TRACKED_DIFF_BATCH_SIZE) { const paths = trackedDiffPaths.slice(start, start + TRACKED_DIFF_BATCH_SIZE); - // Keep rename/copy path filtering identical to the existing per-file command. + // Rename/copy detection needs both paths. const ordinary = await getTrackedDiffTexts({ cwd, refsForDiff, - paths: paths.filter((path) => !renamedPaths.has(path)), + paths: paths.filter((path) => !oldPathByPath.has(path)), ignoreWhitespace, }); const renamed = await Promise.all( paths - .filter((path) => renamedPaths.has(path)) + .filter((path) => oldPathByPath.has(path)) .map((path) => getTrackedDiffTextForPath({ cwd, refsForDiff, path, + oldPath: oldPathByPath.get(path), ignoreWhitespace, }), ),