From 92be3e9dff4d020b940d8a97d39f5dc2e4446bbc Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Fri, 6 Nov 2026 07:03:46 +0100 Subject: [PATCH] fix(diff): select renamed patches and read dangling symlinks --- .../checkout-diff-content.e2e.test.ts | 34 +++++++- packages/server/src/server/diff/git.test.ts | 83 +++++++++++++++++++ packages/server/src/server/diff/git.ts | 28 +++++-- 3 files changed, 135 insertions(+), 10 deletions(-) diff --git a/packages/server/src/server/daemon-e2e/checkout-diff-content.e2e.test.ts b/packages/server/src/server/daemon-e2e/checkout-diff-content.e2e.test.ts index 006b3b04af..b297d07b5a 100644 --- a/packages/server/src/server/daemon-e2e/checkout-diff-content.e2e.test.ts +++ b/packages/server/src/server/daemon-e2e/checkout-diff-content.e2e.test.ts @@ -461,6 +461,7 @@ test("SDK diff journey publishes a tag tree, commits a one-file refresh and shar }); const initial = states.at(-1); if (initial?.phase !== "ready") throw new Error("Expected ready"); + const initialStateCount = states.length; await second.connect(); startGitCommandMetrics(); const duplicateStates: import("@getpaseo/client").DiffState[] = []; @@ -487,7 +488,8 @@ test("SDK diff journey publishes a tag tree, commits a one-file refresh and shar if (refreshed?.phase !== "ready") throw new Error("Expected ready"); expect(refreshed.files[0]).not.toBe(initial.files[0]); expect(refreshed.files[1]).toBe(initial.files[1]); - expect(states.filter((state) => state.phase === "loading")).toHaveLength(1); + expect(states.slice(initialStateCount).map((state) => state.phase)).not.toContain("loading"); + expect(states.map((state) => state.phase)).not.toContain("error"); await duplicate.release(); await observation.release(); } finally { @@ -521,3 +523,33 @@ test("commit-file RPC keeps the existing read ceiling for patches larger than on rmSync(cwd, { recursive: true, force: true }); } }, 30000); + +test("commit-file RPC selects renamed content when the source path becomes a directory", async () => { + const ctx = await createDaemonTestContext(); + const cwd = mkdtempSync(join(tmpdir(), "commit-rename-directory-")); + try { + git(cwd, "init", "-b", "main"); + git(cwd, "config", "user.name", "Test"); + git(cwd, "config", "user.email", "test@example.com"); + writeFileSync(join(cwd, "a.txt"), "original line\n".repeat(16)); + git(cwd, "add", "."); + git(cwd, "-c", "commit.gpgsign=false", "commit", "-m", "base"); + git(cwd, "mv", "a.txt", "z.txt"); + writeFileSync(join(cwd, "z.txt"), "original line\n".repeat(16) + "renamed line\n"); + mkdirSync(join(cwd, "a.txt")); + writeFileSync(join(cwd, "a.txt", "child.txt"), "replacement line\n"); + git(cwd, "add", "."); + git(cwd, "-c", "commit.gpgsign=false", "commit", "-m", "rename"); + const sha = git(cwd, "rev-parse", "HEAD").trim(); + const { getCommitFileDiff } = await import("../../utils/checkout-git.js"); + const expected = await getCommitFileDiff({ cwd, sha, path: "z.txt" }); + const actual = await ctx.client.getCommitFileDiff(cwd, sha, "z.txt"); + expect(expected?.hunks.flatMap((hunk) => hunk.lines.map((line) => line.content))).toContain( + "renamed line", + ); + expect(actual.file?.hunks).toEqual(expected?.hunks); + } finally { + await ctx.cleanup(); + rmSync(cwd, { recursive: true, force: true }); + } +}, 30000); diff --git a/packages/server/src/server/diff/git.test.ts b/packages/server/src/server/diff/git.test.ts index 970a32852b..12d3fab427 100644 --- a/packages/server/src/server/diff/git.test.ts +++ b/packages/server/src/server/diff/git.test.ts @@ -291,3 +291,86 @@ test.skipIf(process.platform === "win32")( } }, ); + +test.each(["worktree", "index", "rev"] as const)( + "%s rename reads select the destination when the old path becomes a directory", + async (headKind) => { + const cwd = await mkdtemp(join(tmpdir(), "diff-rename-directory-")); + const git = (...args: string[]) => execFileSync("git", args, { cwd, encoding: "utf8" }).trim(); + const { mkdir } = await import("node:fs/promises"); + try { + git("init", "-b", "main"); + git("config", "user.name", "Test"); + git("config", "user.email", "test@example.com"); + await writeFile(join(cwd, "a.txt"), "original line\n".repeat(16)); + git("add", "."); + git("-c", "commit.gpgsign=false", "commit", "-m", "base"); + const base = git("rev-parse", "HEAD"); + git("mv", "a.txt", "z.txt"); + await writeFile(join(cwd, "z.txt"), "original line\n".repeat(16) + "renamed line\n"); + await mkdir(join(cwd, "a.txt")); + await writeFile(join(cwd, "a.txt", "child.txt"), "replacement line\n"); + git("add", "."); + if (headKind === "rev") git("-c", "commit.gpgsign=false", "commit", "-m", "rename"); + const head = headKind === "rev" ? { kind: "rev" as const, rev: "HEAD" } : { kind: headKind }; + const comparison = await listComparison({ + cwd, + base: { kind: "rev", rev: base }, + head, + highlight: false, + }); + try { + const file = comparison.files.find((candidate) => candidate.entry.path === "z.txt"); + if (!file) throw new Error("Missing rename"); + expect(file.entry.oldPath).toBe("a.txt"); + const record = await comparison.read(file); + if (record.kind !== "ready") throw new Error("Expected readable rename"); + const lines = record.body.hunks.flatMap((hunk) => hunk.lines.map((line) => line.content)); + expect(lines).toContain("renamed line"); + expect(lines).not.toContain("replacement line"); + } finally { + await comparison.close(); + } + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }, +); + +test.skipIf(process.platform === "win32")( + "a dangling untracked symlink exposes its target text alongside tracked changes", + async () => { + const cwd = await mkdtemp(join(tmpdir(), "diff-dangling-link-")); + const git = (...args: string[]) => execFileSync("git", args, { cwd, encoding: "utf8" }); + const { symlink } = await import("node:fs/promises"); + try { + git("init", "-b", "main"); + git("config", "user.name", "Test"); + git("config", "user.email", "test@example.com"); + await writeFile(join(cwd, "a.txt"), "before\n"); + git("add", "."); + git("-c", "commit.gpgsign=false", "commit", "-m", "base"); + await writeFile(join(cwd, "a.txt"), "after\n"); + await symlink("not-created-yet", join(cwd, "link")); + const comparison = await listComparison({ + cwd, + base: { kind: "rev", rev: "HEAD" }, + head: { kind: "worktree" }, + highlight: false, + }); + try { + expect(comparison.files.map((file) => file.entry.path)).toEqual(["a.txt", "link"]); + const record = await comparison.read(comparison.files[1]); + if (record.kind !== "ready") throw new Error("Expected link body"); + expect(record.body.hunks.flatMap((hunk) => hunk.lines)).toContainEqual({ + type: "add", + content: "not-created-yet", + }); + } finally { + await comparison.close(); + } + } finally { + await rm(cwd, { recursive: true, force: true }); + } + }, +); diff --git a/packages/server/src/server/diff/git.ts b/packages/server/src/server/diff/git.ts index d2304e4347..59e6649c99 100644 --- a/packages/server/src/server/diff/git.ts +++ b/packages/server/src/server/diff/git.ts @@ -253,7 +253,13 @@ async function readComparisonList( const command = file.source.kind === "untracked" ? ["diff", "--no-index", ...GIT_PATCH_FORMAT_ARGS, "--", "/dev/null", file.entry.path] - : [...args, "--patch", "--", ...paths.map((path) => `:(literal)${path}`)]; + : [ + ...args, + ...(file.entry.oldPath !== null ? ["--diff-filter=RC"] : []), + "--patch", + "--", + ...paths.map((path) => `:(literal)${path}`), + ]; const result = await runGitCommand(command, { ...options, acceptExitCodes: [0, 1], @@ -262,7 +268,9 @@ async function readComparisonList( const bytes = Buffer.byteLength(result.stdout); if (result.truncated || bytes > remainingBytes || bytes > maxPatchBytes) return { kind: "too_large" }; - const parsed = parseDiff(result.stdout)[0]; + const parsed = parseDiff(result.stdout).find( + (candidate) => candidate.path === file.entry.path, + ); if (!parsed) return { kind: "ready", body: { path: file.entry.path, hunks: [] }, bytes }; let highlighted = parsed; if (compare.highlight !== false) { @@ -320,13 +328,15 @@ async function listUntrackedFiles(options: GitCommandOptions): Promise