From d5962b96788d73370d01ccd4511914844786804e Mon Sep 17 00:00:00 2001 From: Adam Bowker Date: Thu, 23 Apr 2026 15:13:09 -0400 Subject: [PATCH] fix(code): fix branch vs default diff for unpushed changes --- apps/code/src/main/services/git/schemas.ts | 6 + apps/code/src/main/services/git/service.ts | 35 ++++ apps/code/src/main/trpc/routers/git.ts | 12 ++ .../code-review/components/DiffStatsBadge.tsx | 9 +- .../code-review/components/ReviewPage.tsx | 20 +-- .../git-interaction/hooks/useGitQueries.ts | 21 +++ .../git-interaction/utils/gitCacheKeys.ts | 3 + packages/git/src/queries.test.ts | 152 +++++++++++++++++- packages/git/src/queries.ts | 52 ++++++ 9 files changed, 294 insertions(+), 16 deletions(-) diff --git a/apps/code/src/main/services/git/schemas.ts b/apps/code/src/main/services/git/schemas.ts index a6c209495..32ce72be7 100644 --- a/apps/code/src/main/services/git/schemas.ts +++ b/apps/code/src/main/services/git/schemas.ts @@ -416,6 +416,12 @@ export const getBranchChangedFilesInput = z.object({ }); export const getBranchChangedFilesOutput = z.array(changedFileSchema); +export const getLocalBranchChangedFilesInput = z.object({ + directoryPath: z.string(), + branch: z.string(), +}); +export const getLocalBranchChangedFilesOutput = z.array(changedFileSchema); + export const generateCommitMessageInput = z.object({ directoryPath: z.string(), conversationContext: z.string().optional(), diff --git a/apps/code/src/main/services/git/service.ts b/apps/code/src/main/services/git/service.ts index 6219cb790..a40eeaa8e 100644 --- a/apps/code/src/main/services/git/service.ts +++ b/apps/code/src/main/services/git/service.ts @@ -8,6 +8,8 @@ const execFileAsync = promisify(execFile); import { execGh } from "@posthog/git/gh"; import { getAllBranches, + getBranchDiffPatchesByPath, + getChangedFilesBetweenBranches, getChangedFilesDetailed, getCommitConventions, getCommitsBetweenBranches, @@ -1237,6 +1239,39 @@ export class GitService extends TypedEventEmitter { }); } + public async getLocalBranchChangedFiles( + directoryPath: string, + branch: string, + ): Promise { + await this.fetchIfStale(directoryPath); + + const defaultBranch = await getDefaultBranch(directoryPath); + if (!defaultBranch) return []; + + const files = await getChangedFilesBetweenBranches( + directoryPath, + defaultBranch, + branch, + { excludePatterns: [".claude", "CLAUDE.local.md"] }, + ); + if (files.length === 0) return []; + + const patchByPath = await getBranchDiffPatchesByPath( + directoryPath, + defaultBranch, + branch, + ); + + return files.map((f) => ({ + path: f.path, + status: f.status, + originalPath: f.originalPath, + linesAdded: f.linesAdded, + linesRemoved: f.linesRemoved, + patch: patchByPath.get(f.path), + })); + } + public async generateCommitMessage( directoryPath: string, conversationContext?: string, diff --git a/apps/code/src/main/trpc/routers/git.ts b/apps/code/src/main/trpc/routers/git.ts index 64d6209d0..25665cbd1 100644 --- a/apps/code/src/main/trpc/routers/git.ts +++ b/apps/code/src/main/trpc/routers/git.ts @@ -44,6 +44,8 @@ import { getGitSyncStatusOutput, getLatestCommitInput, getLatestCommitOutput, + getLocalBranchChangedFilesInput, + getLocalBranchChangedFilesOutput, getPrChangedFilesInput, getPrChangedFilesOutput, getPrDetailsByUrlInput, @@ -367,6 +369,16 @@ export const gitRouter = router({ getService().getBranchChangedFiles(input.repo, input.branch), ), + getLocalBranchChangedFiles: publicProcedure + .input(getLocalBranchChangedFilesInput) + .output(getLocalBranchChangedFilesOutput) + .query(({ input }) => + getService().getLocalBranchChangedFiles( + input.directoryPath, + input.branch, + ), + ), + generateCommitMessage: publicProcedure .input(generateCommitMessageInput) .output(generateCommitMessageOutput) diff --git a/apps/code/src/renderer/features/code-review/components/DiffStatsBadge.tsx b/apps/code/src/renderer/features/code-review/components/DiffStatsBadge.tsx index 51be6e8ca..417f9d048 100644 --- a/apps/code/src/renderer/features/code-review/components/DiffStatsBadge.tsx +++ b/apps/code/src/renderer/features/code-review/components/DiffStatsBadge.tsx @@ -1,12 +1,13 @@ import { Tooltip } from "@components/ui/Tooltip"; import { - useBranchChangedFiles, + useLocalBranchChangedFiles, usePrChangedFiles, } from "@features/git-interaction/hooks/useGitQueries"; import { computeDiffStats, type DiffStats, } from "@features/git-interaction/utils/diffStats"; +import { useCwd } from "@features/sidebar/hooks/useCwd"; import { useCloudChangedFiles } from "@features/task-detail/hooks/useCloudChangedFiles"; import { useWorkspace } from "@features/workspace/hooks/useWorkspace"; import { GitDiff } from "@phosphor-icons/react"; @@ -44,16 +45,16 @@ function CloudDiffStatsBadge({ task }: { task: Task }) { function LocalDiffStatsBadge({ task }: { task: Task }) { const taskId = task.id; + const repoPath = useCwd(taskId); const { effectiveSource, - repoSlug, linkedBranch, prUrl, diffStats: localDiffStats, } = useEffectiveDiffSource(taskId); - const { data: branchFiles } = useBranchChangedFiles( - effectiveSource === "branch" ? repoSlug : null, + const { data: branchFiles } = useLocalBranchChangedFiles( + effectiveSource === "branch" ? (repoPath ?? null) : null, effectiveSource === "branch" ? linkedBranch : null, ); const { data: prFiles } = usePrChangedFiles( diff --git a/apps/code/src/renderer/features/code-review/components/ReviewPage.tsx b/apps/code/src/renderer/features/code-review/components/ReviewPage.tsx index d192f9132..b9ac5aef5 100644 --- a/apps/code/src/renderer/features/code-review/components/ReviewPage.tsx +++ b/apps/code/src/renderer/features/code-review/components/ReviewPage.tsx @@ -1,6 +1,6 @@ import { useDiffViewerStore } from "@features/code-editor/stores/diffViewerStore"; import { - useBranchChangedFiles, + useLocalBranchChangedFiles, usePrChangedFiles, } from "@features/git-interaction/hooks/useGitQueries"; import { usePrDetails } from "@features/git-interaction/hooks/usePrDetails"; @@ -52,7 +52,6 @@ export function ReviewPage({ task }: ReviewPageProps) { prUrl, linkedBranch, defaultBranch, - repoSlug, branchSourceAvailable, prSourceAvailable, } = useEffectiveDiffSource(taskId); @@ -113,7 +112,7 @@ export function ReviewPage({ task }: ReviewPageProps) { files.map((f) => f.path), [files]); @@ -249,7 +249,7 @@ function BranchReviewPage({ linesAdded={reviewState.linesAdded} linesRemoved={reviewState.linesRemoved} isLoading={ - (isLoading || (!repoSlug && isReviewOpen)) && files.length === 0 + (isLoading || (!repoPath && isReviewOpen)) && files.length === 0 } isEmpty={files.length === 0} allExpanded={reviewState.collapsedFiles.size === 0} diff --git a/apps/code/src/renderer/features/git-interaction/hooks/useGitQueries.ts b/apps/code/src/renderer/features/git-interaction/hooks/useGitQueries.ts index 9cd40da5c..c2cae954f 100644 --- a/apps/code/src/renderer/features/git-interaction/hooks/useGitQueries.ts +++ b/apps/code/src/renderer/features/git-interaction/hooks/useGitQueries.ts @@ -187,3 +187,24 @@ export function useBranchChangedFiles( ), ); } + +export function useLocalBranchChangedFiles( + directoryPath: string | null, + branch: string | null, +) { + const trpc = useTRPC(); + return useQuery( + trpc.git.getLocalBranchChangedFiles.queryOptions( + { + directoryPath: directoryPath as string, + branch: branch as string, + }, + { + enabled: !!directoryPath && !!branch, + staleTime: 30_000, + refetchOnMount: "always", + retry: 1, + }, + ), + ); +} diff --git a/apps/code/src/renderer/features/git-interaction/utils/gitCacheKeys.ts b/apps/code/src/renderer/features/git-interaction/utils/gitCacheKeys.ts index d3662af76..fb29935e5 100644 --- a/apps/code/src/renderer/features/git-interaction/utils/gitCacheKeys.ts +++ b/apps/code/src/renderer/features/git-interaction/utils/gitCacheKeys.ts @@ -23,4 +23,7 @@ export function invalidateGitBranchQueries(repoPath: string) { queryClient.invalidateQueries(trpc.git.getLatestCommit.queryFilter(input)); queryClient.invalidateQueries(trpc.git.getPrStatus.queryFilter(input)); queryClient.invalidateQueries(trpc.git.getFileAtHead.pathFilter()); + queryClient.invalidateQueries( + trpc.git.getLocalBranchChangedFiles.pathFilter(), + ); } diff --git a/packages/git/src/queries.test.ts b/packages/git/src/queries.test.ts index 5579ff4b0..6eab2e7b4 100644 --- a/packages/git/src/queries.test.ts +++ b/packages/git/src/queries.test.ts @@ -1,9 +1,13 @@ -import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { mkdtemp, rm, unlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { createGitClient } from "./client"; -import { detectDefaultBranch } from "./queries"; +import { + detectDefaultBranch, + getBranchDiffPatchesByPath, + splitUnifiedDiffByFile, +} from "./queries"; async function setupRepo(defaultBranch = "main"): Promise { const dir = await mkdtemp(path.join(tmpdir(), "posthog-code-queries-")); @@ -94,3 +98,147 @@ describe("detectDefaultBranch", () => { await rm(remoteDir, { recursive: true, force: true }); }); }); + +describe("splitUnifiedDiffByFile", () => { + it("returns an empty map for empty input", () => { + expect(splitUnifiedDiffByFile("")).toEqual(new Map()); + }); + + it("splits a two-file diff keyed by post-image path", () => { + const raw = [ + "diff --git a/one.txt b/one.txt", + "index 0000000..1111111 100644", + "--- a/one.txt", + "+++ b/one.txt", + "@@ -1 +1 @@", + "-hello", + "+hello world", + "diff --git a/two.txt b/two.txt", + "new file mode 100644", + "--- /dev/null", + "+++ b/two.txt", + "@@ -0,0 +1 @@", + "+brand new", + "", + ].join("\n"); + + const result = splitUnifiedDiffByFile(raw); + + expect([...result.keys()]).toEqual(["one.txt", "two.txt"]); + expect(result.get("one.txt")).toContain("diff --git a/one.txt b/one.txt"); + expect(result.get("one.txt")).toContain("+hello world"); + expect(result.get("two.txt")).toContain("diff --git a/two.txt b/two.txt"); + expect(result.get("two.txt")).toContain("+brand new"); + }); + + it("keys renames by the post-rename (b/) path", () => { + const raw = [ + "diff --git a/old.txt b/new.txt", + "similarity index 100%", + "rename from old.txt", + "rename to new.txt", + "", + ].join("\n"); + + const result = splitUnifiedDiffByFile(raw); + expect(result.has("new.txt")).toBe(true); + expect(result.has("old.txt")).toBe(false); + expect(result.get("new.txt")).toContain("rename from old.txt"); + }); + + it("handles binary diffs", () => { + const raw = [ + "diff --git a/image.png b/image.png", + "Binary files a/image.png and b/image.png differ", + "", + ].join("\n"); + + const result = splitUnifiedDiffByFile(raw); + expect(result.get("image.png")).toContain("Binary files"); + }); +}); + +describe("getBranchDiffPatchesByPath", () => { + let repoDir: string | undefined; + + afterEach(async () => { + if (repoDir) { + await rm(repoDir, { recursive: true, force: true }); + repoDir = undefined; + } + }); + + async function setupBranchWithCommits(): Promise<{ + repoDir: string; + remoteDir: string; + }> { + const workDir = await mkdtemp(path.join(tmpdir(), "posthog-code-branch-")); + const remoteDir = await mkdtemp(path.join(tmpdir(), "posthog-code-bare-")); + + const remoteGit = createGitClient(remoteDir); + await remoteGit.init(["--bare", "--initial-branch", "main"]); + + const git = createGitClient(workDir); + await git.init(["--initial-branch", "main"]); + await git.addConfig("user.name", "Test"); + await git.addConfig("user.email", "test@example.com"); + await git.addConfig("commit.gpgsign", "false"); + await git.addRemote("origin", remoteDir); + + await writeFile(path.join(workDir, "file.txt"), "line1\nline2\n"); + await git.add(["file.txt"]); + await git.commit("initial"); + await git.push(["origin", "main"]); + + await git.checkoutLocalBranch("feature"); + await writeFile(path.join(workDir, "file.txt"), "line1\nchanged\n"); + await writeFile(path.join(workDir, "added.txt"), "new file\n"); + await git.add(["file.txt", "added.txt"]); + await git.commit("feature work, not pushed"); + + return { repoDir: workDir, remoteDir }; + } + + it("returns per-file patches for commits not yet pushed", async () => { + const { repoDir: workDir, remoteDir } = await setupBranchWithCommits(); + repoDir = workDir; + + try { + const patches = await getBranchDiffPatchesByPath( + workDir, + "main", + "feature", + ); + + expect(patches.has("file.txt")).toBe(true); + expect(patches.has("added.txt")).toBe(true); + expect(patches.get("file.txt")).toContain("-line2"); + expect(patches.get("file.txt")).toContain("+changed"); + expect(patches.get("added.txt")).toContain("+new file"); + } finally { + await rm(remoteDir, { recursive: true, force: true }); + } + }); + + it("returns deletions keyed by their path", async () => { + const { repoDir: workDir, remoteDir } = await setupBranchWithCommits(); + repoDir = workDir; + + try { + const git = createGitClient(workDir); + await unlink(path.join(workDir, "file.txt")); + await git.add(["file.txt"]); + await git.commit("delete file.txt"); + + const patches = await getBranchDiffPatchesByPath( + workDir, + "main", + "feature", + ); + + expect(patches.get("file.txt")).toContain("deleted file mode"); + } finally { + await rm(remoteDir, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/git/src/queries.ts b/packages/git/src/queries.ts index c7b4ac43f..f64fbd513 100644 --- a/packages/git/src/queries.ts +++ b/packages/git/src/queries.ts @@ -581,6 +581,58 @@ export async function getChangedFilesBetweenBranches( ); } +/** + * Splits a unified `git diff` string into per-file patches, keyed by the `b/` + * (post-rename) path, which is the shape `ChangedFileInfo.path` uses. Each + * returned patch string begins with its own `diff --git ...` header and is a + * valid standalone unified diff. + */ +export function splitUnifiedDiffByFile(raw: string): Map { + const patches = new Map(); + if (!raw) return patches; + + const headerRegex = /^diff --git a\/.+? b\/(.+)$/gm; + const matches: Array<{ path: string; start: number }> = []; + let match = headerRegex.exec(raw); + while (match !== null) { + matches.push({ path: match[1], start: match.index }); + match = headerRegex.exec(raw); + } + + for (let i = 0; i < matches.length; i++) { + const { path, start } = matches[i]; + const end = i + 1 < matches.length ? matches[i + 1].start : raw.length; + patches.set(path, raw.slice(start, end)); + } + return patches; +} + +export async function getBranchDiffPatchesByPath( + baseDir: string, + baseBranch: string, + headBranch: string, + options?: CreateGitClientOptions, +): Promise> { + const manager = getGitOperationManager(); + return manager.executeRead( + baseDir, + async (git) => { + try { + const raw = await git.diff([ + "-M", + "--patch", + "--no-color", + `origin/${baseBranch}...${headBranch}`, + ]); + return splitUnifiedDiffByFile(raw); + } catch { + return new Map(); + } + }, + { signal: options?.abortSignal }, + ); +} + export interface DiffStats { filesChanged: number; linesAdded: number;