diff --git a/packages/server/src/server/git-filename-diffs.e2e.test.ts b/packages/server/src/server/git-filename-diffs.e2e.test.ts index 343e1c81c8..c32ee040b8 100644 --- a/packages/server/src/server/git-filename-diffs.e2e.test.ts +++ b/packages/server/src/server/git-filename-diffs.e2e.test.ts @@ -5,6 +5,44 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { createTestPaseoDaemon, DaemonClient } from "./test-utils/index.js"; +test("a rename-only commit returns zero added or deleted lines through the client", async () => { + const cwd = mkdtempSync(join(tmpdir(), "commit-rename-diff-")); + const git = (...args: string[]) => execFileSync("git", args, { cwd, encoding: "utf8" }).trim(); + const daemon = await createTestPaseoDaemon({ agentClients: {} }); + const client = new DaemonClient({ url: `ws://127.0.0.1:${daemon.port}/ws` }); + try { + git("init", "-b", "main"); + git("config", "user.name", "Test"); + git("config", "user.email", "test@example.com"); + writeFileSync(join(cwd, "old.txt"), "same\n"); + git("add", "."); + git("-c", "commit.gpgsign=false", "commit", "-m", "initial"); + git("mv", "old.txt", "new.txt"); + git("-c", "commit.gpgsign=false", "commit", "-m", "rename"); + const sha = git("rev-parse", "HEAD"); + + await client.connect(); + const history = await client.listCheckoutCommits(cwd); + expect(history.commits.find((commit) => commit.sha === sha)?.files).toEqual([ + { path: "new.txt", additions: 0, deletions: 0, status: "renamed" }, + ]); + expect(await client.getCommitFileDiff(cwd, sha, "new.txt")).toEqual({ + file: { + path: "new.txt", + isNew: false, + isDeleted: false, + additions: 0, + deletions: 0, + hunks: [], + }, + }); + } finally { + await client.close(); + await daemon.close(); + rmSync(cwd, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } +}); + test.skipIf(process.platform === "win32")( "checkout and commit diffs preserve filenames that Git quotes", async () => { diff --git a/packages/server/src/utils/checkout-git.commit-file-diff.test.ts b/packages/server/src/utils/checkout-git.commit-file-diff.test.ts index fbb22fe3b9..576e9b0c80 100644 --- a/packages/server/src/utils/checkout-git.commit-file-diff.test.ts +++ b/packages/server/src/utils/checkout-git.commit-file-diff.test.ts @@ -3,7 +3,8 @@ import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "fs" import { tmpdir } from "os"; import { join } from "path"; import { afterEach, describe, expect, it } from "vitest"; -import { getCommitFileDiff } from "./checkout-git.js"; +import { getCommitFileDiff, listCheckoutCommits } from "./checkout-git.js"; +import { startGitCommandMetrics, stopGitCommandMetrics } from "./run-git-command.js"; const tempDirs: string[] = []; @@ -44,6 +45,75 @@ function headSha(repoDir: string): string { } describe("getCommitFileDiff", () => { + it.each(["new.txt", "with spaces.txt"])("preserves a rename-only commit to %s", async (name) => { + const repoDir = initRepo(); + commitFile(repoDir, "old.txt", "same\n", "initial"); + git(["mv", "old.txt", name], repoDir); + git(["-c", "commit.gpgsign=false", "commit", "-m", "rename"], repoDir); + const sha = headSha(repoDir); + + const history = await listCheckoutCommits({ cwd: repoDir }); + expect(history.commits.find((commit) => commit.sha === sha)?.files).toEqual([ + { path: name, status: "renamed", additions: 0, deletions: 0 }, + ]); + expect(await getCommitFileDiff({ cwd: repoDir, sha, path: name })).toEqual({ + path: name, + isNew: false, + isDeleted: false, + additions: 0, + deletions: 0, + hunks: [], + }); + }); + + it.each(["new.txt", "目录.txt"])( + "preserves the edits in a committed rename to %s", + async (name) => { + const repoDir = initRepo(); + commitFile(repoDir, "old.txt", "same\nkeep\nlast\n", "initial"); + git(["mv", "old.txt", name], repoDir); + writeFileSync(join(repoDir, name), "same\nKEEP\nlast\n"); + git(["add", "."], repoDir); + git(["-c", "commit.gpgsign=false", "commit", "-m", "rename and edit"], repoDir); + const sha = headSha(repoDir); + + const history = await listCheckoutCommits({ cwd: repoDir }); + expect(history.commits.find((commit) => commit.sha === sha)?.files).toEqual([ + { path: name, status: "renamed", additions: 1, deletions: 1 }, + ]); + const file = await getCommitFileDiff({ cwd: repoDir, sha, path: name }); + expect(file).toMatchObject({ + path: name, + isNew: false, + isDeleted: false, + additions: 1, + deletions: 1, + }); + expect(file?.hunks.flatMap((hunk) => hunk.lines)).toEqual( + expect.arrayContaining([ + expect.objectContaining({ type: "remove", content: "keep" }), + expect.objectContaining({ type: "add", content: "KEEP" }), + ]), + ); + }, + ); + + it.skipIf(process.platform === "win32")("preserves a rename to a quoted filename", async () => { + const repoDir = initRepo(); + const name = 'new "draft".txt'; + commitFile(repoDir, "old.txt", "same\n", "initial"); + git(["mv", "old.txt", name], repoDir); + git(["-c", "commit.gpgsign=false", "commit", "-m", "rename"], repoDir); + expect(await getCommitFileDiff({ cwd: repoDir, sha: headSha(repoDir), path: name })).toEqual({ + path: name, + isNew: false, + isDeleted: false, + additions: 0, + deletions: 0, + hunks: [], + }); + }); + it("returns the parsed diff for a modified file in a commit", async () => { const repoDir = initRepo(); commitFile(repoDir, "foo.txt", "a\nb\nc\n", "initial"); @@ -78,6 +148,78 @@ describe("getCommitFileDiff", () => { expect(file?.deletions).toBe(0); }); + it.each([ + ["exact", "line1\nline2\nline3\nline4\nline5\nline6\n"], + ["edited", "line1\nline2\nline3\nline4\nline5\nCHANGED\n"], + ])("shows an added %s copy as a new file", async (_kind, addedContent) => { + const repoDir = initRepo(); + commitFile(repoDir, "source.txt", "line1\nline2\nline3\nline4\nline5\nline6\n", "initial"); + commitFile(repoDir, "added.txt", addedContent, "add similar file"); + const sha = headSha(repoDir); + + const history = await listCheckoutCommits({ cwd: repoDir }); + expect(history.commits.find((commit) => commit.sha === sha)?.files).toEqual([ + { path: "added.txt", status: "added", additions: 6, deletions: 0 }, + ]); + const file = await getCommitFileDiff({ cwd: repoDir, sha, path: "added.txt" }); + expect(file).toMatchObject({ + path: "added.txt", + isNew: true, + isDeleted: false, + additions: 6, + deletions: 0, + }); + expect( + file?.hunks + .flatMap((hunk) => hunk.lines) + .filter((line) => line.type !== "header") + .map(({ type, content }) => ({ type, content })), + ).toEqual( + addedContent + .trimEnd() + .split("\n") + .map((content) => ({ type: "add", content })), + ); + }); + + it("shares commit rename discovery across concurrent and later file requests", async () => { + const repoDir = initRepo(); + commitFile(repoDir, "old.txt", "same\n", "initial"); + git(["mv", "old.txt", "new.txt"], repoDir); + commitFile(repoDir, "added.txt", "added\n", "rename and add"); + const sha = headSha(repoDir); + + startGitCommandMetrics(); + try { + const [renamed, added] = await Promise.all([ + getCommitFileDiff({ cwd: repoDir, sha, path: "new.txt" }), + getCommitFileDiff({ cwd: repoDir, sha, path: "added.txt" }), + ]); + expect(renamed).toMatchObject({ isNew: false, additions: 0, deletions: 0 }); + expect(added).toMatchObject({ isNew: true, additions: 1, deletions: 0 }); + expect(await getCommitFileDiff({ cwd: repoDir, sha, path: "new.txt" })).toEqual(renamed); + } finally { + const metrics = stopGitCommandMetrics(); + expect(metrics.commands.filter(({ args }) => args.includes("--name-status"))).toHaveLength(1); + } + }); + + it("rediscovers renames when a symbolic commit ref moves", async () => { + const repoDir = initRepo(); + commitFile(repoDir, "old.txt", "same\n", "initial"); + expect(await getCommitFileDiff({ cwd: repoDir, sha: "HEAD", path: "old.txt" })).toMatchObject({ + isNew: true, + additions: 1, + }); + git(["mv", "old.txt", "new.txt"], repoDir); + git(["-c", "commit.gpgsign=false", "commit", "-m", "rename"], repoDir); + expect(await getCommitFileDiff({ cwd: repoDir, sha: "HEAD", path: "new.txt" })).toMatchObject({ + isNew: false, + additions: 0, + deletions: 0, + }); + }); + it("returns null for a path not changed in the commit", async () => { const repoDir = initRepo(); commitFile(repoDir, "foo.txt", "a\n", "initial"); @@ -89,6 +231,59 @@ describe("getCommitFileDiff", () => { expect(file).toBeNull(); }); + it("does not follow an unchanged file back to an earlier commit", async () => { + const repoDir = initRepo(); + commitFile(repoDir, "foo.txt", "same\n", "initial"); + commitFile(repoDir, "other.txt", "other\n", "unrelated change"); + expect( + await getCommitFileDiff({ cwd: repoDir, sha: headSha(repoDir), path: "foo.txt" }), + ).toBeNull(); + }); + + it("returns the added file in a root commit and the deleted file in its deletion commit", async () => { + const repoDir = initRepo(); + commitFile(repoDir, "foo.txt", "same\n", "initial"); + expect( + await getCommitFileDiff({ cwd: repoDir, sha: headSha(repoDir), path: "foo.txt" }), + ).toMatchObject({ + isNew: true, + isDeleted: false, + additions: 1, + deletions: 0, + }); + git(["rm", "foo.txt"], repoDir); + git(["-c", "commit.gpgsign=false", "commit", "-m", "delete"], repoDir); + expect( + await getCommitFileDiff({ cwd: repoDir, sha: headSha(repoDir), path: "foo.txt" }), + ).toMatchObject({ + isNew: false, + isDeleted: true, + additions: 0, + deletions: 1, + }); + }); + + it("preserves a rename introduced by a merge against its first parent", async () => { + const repoDir = initRepo(); + commitFile(repoDir, "old.txt", "same\n", "initial"); + git(["checkout", "-b", "feature"], repoDir); + git(["mv", "old.txt", "new.txt"], repoDir); + git(["-c", "commit.gpgsign=false", "commit", "-m", "rename"], repoDir); + git(["checkout", "main"], repoDir); + commitFile(repoDir, "main.txt", "main\n", "advance main"); + git(["merge", "--no-ff", "feature", "-m", "merge feature"], repoDir); + expect( + await getCommitFileDiff({ cwd: repoDir, sha: headSha(repoDir), path: "new.txt" }), + ).toMatchObject({ + path: "new.txt", + isNew: false, + isDeleted: false, + additions: 0, + deletions: 0, + hunks: [], + }); + }); + it("returns a merge commit diff against its first parent", async () => { const repoDir = initRepo(); commitFile(repoDir, "README.md", "base\n", "initial"); diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index 64148c26e4..ce00702908 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -2643,6 +2643,60 @@ export async function listCheckoutCommits({ return { baseRef: comparisonBaseRef, commits }; } +const commitRenamesCache = new TTLCache>>({ + max: 100, + ttl: 60_000, + checkAgeOnGet: true, +}); + +function getCommitRenames(cwd: string, sha: string): Promise> { + // Symbolic refs and abbreviated IDs can resolve to a different commit later. + if (!/^(?:[0-9a-f]{40}|[0-9a-f]{64})$/i.test(sha)) { + return readCommitRenames(cwd, sha); + } + const key = JSON.stringify([cwd, sha]); + const cached = commitRenamesCache.get(key); + if (cached) { + return cached; + } + const lookup = readCommitRenames(cwd, sha).catch((error: unknown) => { + if (commitRenamesCache.get(key) === lookup) { + commitRenamesCache.delete(key); + } + throw error; + }); + // Store the pending lookup too, so concurrent file requests share the scan. + commitRenamesCache.set(key, lookup); + return lookup; +} + +// Discover renames across the commit before narrowing the patch to a file. +// --follow also detects copies, which would disagree with history's -M-only list. +async function readCommitRenames(cwd: string, sha: string): Promise> { + const result = await runGitCommand( + [ + "show", + sha, + "--format=", + "--diff-merges=first-parent", + "--name-status", + "-z", + "-M", + "--diff-filter=R", + ], + { cwd, envOverlay: READ_ONLY_GIT_ENV }, + ); + if (result.truncated) { + throw new Error("Commit renames exceeded the git output limit"); + } + const renames = new Map(); + const fields = result.stdout.split("\0"); + for (let index = 0; index + 2 < fields.length; index += 3) { + renames.set(fields[index + 2]!, fields[index + 1]!); + } + return renames; +} + /** * Fetches the unified diff of a single file as introduced by one commit and * parses it into the same {@link ParsedDiffFile} shape the diff subscription @@ -2665,8 +2719,20 @@ export async function getCommitFileDiff({ path: string; }): Promise { const worktreeRoot = await requireGitWorktreeRoot(cwd); + const renames = await getCommitRenames(worktreeRoot, sha); + const oldPath = renames.get(path); + const paths = oldPath === undefined ? [path] : [oldPath, path]; const { stdout } = await runGitCommand( - ["show", sha, "--format=", "--diff-merges=first-parent", ...GIT_PATCH_FORMAT_ARGS, "--", path], + [ + "show", + sha, + "--format=", + "--diff-merges=first-parent", + "-M", + ...GIT_PATCH_FORMAT_ARGS, + "--", + ...paths.map((filePath) => `:(literal)${filePath}`), + ], { cwd: worktreeRoot, envOverlay: READ_ONLY_GIT_ENV, @@ -2682,8 +2748,8 @@ export async function getCommitFileDiff({ getNewFileContent: (file) => readGitFileContentAtRef(worktreeRoot, sha, file.path), }); - // `--` scopes the diff to a single pathspec, so there is at most one real - // entry. Pick by path to drop any stray header-only section the parser emits. + // Including the rename source lets Git pair it with the destination. Select + // the requested entry if another change also matches the source path. const file = parsedFiles.find((candidate) => candidate.path === path) ?? null; if (!file) { return null;