Skip to content

Commit 553e429

Browse files
authored
fix(code): fix branch vs default diff for unpushed changes (#1874)
## Problem when a user has committed but unpushed changes, they do not show up in the "branch vs [default]" diff because we use the github APIs and compare to the _remote_ branch <!-- Who is this for and what problem does it solve? --> <!-- Closes #ISSUE_ID --> ## Changes updates to use local instead of assuming remote for this case <!-- What did you change and why? --> <!-- If there are frontend changes, include screenshots. --> ## How did you test this? manually repro'd and tested after fix
1 parent d9b137b commit 553e429

9 files changed

Lines changed: 294 additions & 16 deletions

File tree

apps/code/src/main/services/git/schemas.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,12 @@ export const getBranchChangedFilesInput = z.object({
416416
});
417417
export const getBranchChangedFilesOutput = z.array(changedFileSchema);
418418

419+
export const getLocalBranchChangedFilesInput = z.object({
420+
directoryPath: z.string(),
421+
branch: z.string(),
422+
});
423+
export const getLocalBranchChangedFilesOutput = z.array(changedFileSchema);
424+
419425
export const generateCommitMessageInput = z.object({
420426
directoryPath: z.string(),
421427
conversationContext: z.string().optional(),

apps/code/src/main/services/git/service.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ const execFileAsync = promisify(execFile);
88
import { execGh } from "@posthog/git/gh";
99
import {
1010
getAllBranches,
11+
getBranchDiffPatchesByPath,
12+
getChangedFilesBetweenBranches,
1113
getChangedFilesDetailed,
1214
getCommitConventions,
1315
getCommitsBetweenBranches,
@@ -1237,6 +1239,39 @@ export class GitService extends TypedEventEmitter<GitServiceEvents> {
12371239
});
12381240
}
12391241

1242+
public async getLocalBranchChangedFiles(
1243+
directoryPath: string,
1244+
branch: string,
1245+
): Promise<ChangedFile[]> {
1246+
await this.fetchIfStale(directoryPath);
1247+
1248+
const defaultBranch = await getDefaultBranch(directoryPath);
1249+
if (!defaultBranch) return [];
1250+
1251+
const files = await getChangedFilesBetweenBranches(
1252+
directoryPath,
1253+
defaultBranch,
1254+
branch,
1255+
{ excludePatterns: [".claude", "CLAUDE.local.md"] },
1256+
);
1257+
if (files.length === 0) return [];
1258+
1259+
const patchByPath = await getBranchDiffPatchesByPath(
1260+
directoryPath,
1261+
defaultBranch,
1262+
branch,
1263+
);
1264+
1265+
return files.map((f) => ({
1266+
path: f.path,
1267+
status: f.status,
1268+
originalPath: f.originalPath,
1269+
linesAdded: f.linesAdded,
1270+
linesRemoved: f.linesRemoved,
1271+
patch: patchByPath.get(f.path),
1272+
}));
1273+
}
1274+
12401275
public async generateCommitMessage(
12411276
directoryPath: string,
12421277
conversationContext?: string,

apps/code/src/main/trpc/routers/git.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ import {
4444
getGitSyncStatusOutput,
4545
getLatestCommitInput,
4646
getLatestCommitOutput,
47+
getLocalBranchChangedFilesInput,
48+
getLocalBranchChangedFilesOutput,
4749
getPrChangedFilesInput,
4850
getPrChangedFilesOutput,
4951
getPrDetailsByUrlInput,
@@ -367,6 +369,16 @@ export const gitRouter = router({
367369
getService().getBranchChangedFiles(input.repo, input.branch),
368370
),
369371

372+
getLocalBranchChangedFiles: publicProcedure
373+
.input(getLocalBranchChangedFilesInput)
374+
.output(getLocalBranchChangedFilesOutput)
375+
.query(({ input }) =>
376+
getService().getLocalBranchChangedFiles(
377+
input.directoryPath,
378+
input.branch,
379+
),
380+
),
381+
370382
generateCommitMessage: publicProcedure
371383
.input(generateCommitMessageInput)
372384
.output(generateCommitMessageOutput)

apps/code/src/renderer/features/code-review/components/DiffStatsBadge.tsx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import { Tooltip } from "@components/ui/Tooltip";
22
import {
3-
useBranchChangedFiles,
3+
useLocalBranchChangedFiles,
44
usePrChangedFiles,
55
} from "@features/git-interaction/hooks/useGitQueries";
66
import {
77
computeDiffStats,
88
type DiffStats,
99
} from "@features/git-interaction/utils/diffStats";
10+
import { useCwd } from "@features/sidebar/hooks/useCwd";
1011
import { useCloudChangedFiles } from "@features/task-detail/hooks/useCloudChangedFiles";
1112
import { useWorkspace } from "@features/workspace/hooks/useWorkspace";
1213
import { GitDiff } from "@phosphor-icons/react";
@@ -44,16 +45,16 @@ function CloudDiffStatsBadge({ task }: { task: Task }) {
4445

4546
function LocalDiffStatsBadge({ task }: { task: Task }) {
4647
const taskId = task.id;
48+
const repoPath = useCwd(taskId);
4749
const {
4850
effectiveSource,
49-
repoSlug,
5051
linkedBranch,
5152
prUrl,
5253
diffStats: localDiffStats,
5354
} = useEffectiveDiffSource(taskId);
5455

55-
const { data: branchFiles } = useBranchChangedFiles(
56-
effectiveSource === "branch" ? repoSlug : null,
56+
const { data: branchFiles } = useLocalBranchChangedFiles(
57+
effectiveSource === "branch" ? (repoPath ?? null) : null,
5758
effectiveSource === "branch" ? linkedBranch : null,
5859
);
5960
const { data: prFiles } = usePrChangedFiles(

apps/code/src/renderer/features/code-review/components/ReviewPage.tsx

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { useDiffViewerStore } from "@features/code-editor/stores/diffViewerStore";
22
import {
3-
useBranchChangedFiles,
3+
useLocalBranchChangedFiles,
44
usePrChangedFiles,
55
} from "@features/git-interaction/hooks/useGitQueries";
66
import { usePrDetails } from "@features/git-interaction/hooks/usePrDetails";
@@ -52,7 +52,6 @@ export function ReviewPage({ task }: ReviewPageProps) {
5252
prUrl,
5353
linkedBranch,
5454
defaultBranch,
55-
repoSlug,
5655
branchSourceAvailable,
5756
prSourceAvailable,
5857
} = useEffectiveDiffSource(taskId);
@@ -113,7 +112,7 @@ export function ReviewPage({ task }: ReviewPageProps) {
113112
<BranchReviewPage
114113
task={task}
115114
branch={linkedBranch as string}
116-
repoSlug={repoSlug}
115+
repoPath={repoPath}
117116
defaultBranch={defaultBranch}
118117
isReviewOpen={isReviewOpen}
119118
effectiveSource={effectiveSource}
@@ -211,7 +210,7 @@ export function ReviewPage({ task }: ReviewPageProps) {
211210
function BranchReviewPage({
212211
task,
213212
branch,
214-
repoSlug,
213+
repoPath,
215214
defaultBranch,
216215
isReviewOpen,
217216
effectiveSource,
@@ -222,7 +221,7 @@ function BranchReviewPage({
222221
}: {
223222
task: Task;
224223
branch: string;
225-
repoSlug: string | null;
224+
repoPath: string | null;
226225
defaultBranch: string | null;
227226
isReviewOpen: boolean;
228227
effectiveSource: ResolvedDiffSource;
@@ -233,10 +232,11 @@ function BranchReviewPage({
233232
}) {
234233
const taskId = task.id;
235234

236-
const { data: files = EMPTY_BRANCH_FILES, isLoading } = useBranchChangedFiles(
237-
isReviewOpen ? repoSlug : null,
238-
isReviewOpen ? branch : null,
239-
);
235+
const { data: files = EMPTY_BRANCH_FILES, isLoading } =
236+
useLocalBranchChangedFiles(
237+
isReviewOpen ? repoPath : null,
238+
isReviewOpen ? branch : null,
239+
);
240240

241241
const allPaths = useMemo(() => files.map((f) => f.path), [files]);
242242

@@ -249,7 +249,7 @@ function BranchReviewPage({
249249
linesAdded={reviewState.linesAdded}
250250
linesRemoved={reviewState.linesRemoved}
251251
isLoading={
252-
(isLoading || (!repoSlug && isReviewOpen)) && files.length === 0
252+
(isLoading || (!repoPath && isReviewOpen)) && files.length === 0
253253
}
254254
isEmpty={files.length === 0}
255255
allExpanded={reviewState.collapsedFiles.size === 0}

apps/code/src/renderer/features/git-interaction/hooks/useGitQueries.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,3 +187,24 @@ export function useBranchChangedFiles(
187187
),
188188
);
189189
}
190+
191+
export function useLocalBranchChangedFiles(
192+
directoryPath: string | null,
193+
branch: string | null,
194+
) {
195+
const trpc = useTRPC();
196+
return useQuery(
197+
trpc.git.getLocalBranchChangedFiles.queryOptions(
198+
{
199+
directoryPath: directoryPath as string,
200+
branch: branch as string,
201+
},
202+
{
203+
enabled: !!directoryPath && !!branch,
204+
staleTime: 30_000,
205+
refetchOnMount: "always",
206+
retry: 1,
207+
},
208+
),
209+
);
210+
}

apps/code/src/renderer/features/git-interaction/utils/gitCacheKeys.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,7 @@ export function invalidateGitBranchQueries(repoPath: string) {
2323
queryClient.invalidateQueries(trpc.git.getLatestCommit.queryFilter(input));
2424
queryClient.invalidateQueries(trpc.git.getPrStatus.queryFilter(input));
2525
queryClient.invalidateQueries(trpc.git.getFileAtHead.pathFilter());
26+
queryClient.invalidateQueries(
27+
trpc.git.getLocalBranchChangedFiles.pathFilter(),
28+
);
2629
}

packages/git/src/queries.test.ts

Lines changed: 150 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1-
import { mkdtemp, rm, writeFile } from "node:fs/promises";
1+
import { mkdtemp, rm, unlink, writeFile } from "node:fs/promises";
22
import { tmpdir } from "node:os";
33
import path from "node:path";
44
import { afterEach, describe, expect, it } from "vitest";
55
import { createGitClient } from "./client";
6-
import { detectDefaultBranch } from "./queries";
6+
import {
7+
detectDefaultBranch,
8+
getBranchDiffPatchesByPath,
9+
splitUnifiedDiffByFile,
10+
} from "./queries";
711

812
async function setupRepo(defaultBranch = "main"): Promise<string> {
913
const dir = await mkdtemp(path.join(tmpdir(), "posthog-code-queries-"));
@@ -94,3 +98,147 @@ describe("detectDefaultBranch", () => {
9498
await rm(remoteDir, { recursive: true, force: true });
9599
});
96100
});
101+
102+
describe("splitUnifiedDiffByFile", () => {
103+
it("returns an empty map for empty input", () => {
104+
expect(splitUnifiedDiffByFile("")).toEqual(new Map());
105+
});
106+
107+
it("splits a two-file diff keyed by post-image path", () => {
108+
const raw = [
109+
"diff --git a/one.txt b/one.txt",
110+
"index 0000000..1111111 100644",
111+
"--- a/one.txt",
112+
"+++ b/one.txt",
113+
"@@ -1 +1 @@",
114+
"-hello",
115+
"+hello world",
116+
"diff --git a/two.txt b/two.txt",
117+
"new file mode 100644",
118+
"--- /dev/null",
119+
"+++ b/two.txt",
120+
"@@ -0,0 +1 @@",
121+
"+brand new",
122+
"",
123+
].join("\n");
124+
125+
const result = splitUnifiedDiffByFile(raw);
126+
127+
expect([...result.keys()]).toEqual(["one.txt", "two.txt"]);
128+
expect(result.get("one.txt")).toContain("diff --git a/one.txt b/one.txt");
129+
expect(result.get("one.txt")).toContain("+hello world");
130+
expect(result.get("two.txt")).toContain("diff --git a/two.txt b/two.txt");
131+
expect(result.get("two.txt")).toContain("+brand new");
132+
});
133+
134+
it("keys renames by the post-rename (b/) path", () => {
135+
const raw = [
136+
"diff --git a/old.txt b/new.txt",
137+
"similarity index 100%",
138+
"rename from old.txt",
139+
"rename to new.txt",
140+
"",
141+
].join("\n");
142+
143+
const result = splitUnifiedDiffByFile(raw);
144+
expect(result.has("new.txt")).toBe(true);
145+
expect(result.has("old.txt")).toBe(false);
146+
expect(result.get("new.txt")).toContain("rename from old.txt");
147+
});
148+
149+
it("handles binary diffs", () => {
150+
const raw = [
151+
"diff --git a/image.png b/image.png",
152+
"Binary files a/image.png and b/image.png differ",
153+
"",
154+
].join("\n");
155+
156+
const result = splitUnifiedDiffByFile(raw);
157+
expect(result.get("image.png")).toContain("Binary files");
158+
});
159+
});
160+
161+
describe("getBranchDiffPatchesByPath", () => {
162+
let repoDir: string | undefined;
163+
164+
afterEach(async () => {
165+
if (repoDir) {
166+
await rm(repoDir, { recursive: true, force: true });
167+
repoDir = undefined;
168+
}
169+
});
170+
171+
async function setupBranchWithCommits(): Promise<{
172+
repoDir: string;
173+
remoteDir: string;
174+
}> {
175+
const workDir = await mkdtemp(path.join(tmpdir(), "posthog-code-branch-"));
176+
const remoteDir = await mkdtemp(path.join(tmpdir(), "posthog-code-bare-"));
177+
178+
const remoteGit = createGitClient(remoteDir);
179+
await remoteGit.init(["--bare", "--initial-branch", "main"]);
180+
181+
const git = createGitClient(workDir);
182+
await git.init(["--initial-branch", "main"]);
183+
await git.addConfig("user.name", "Test");
184+
await git.addConfig("user.email", "test@example.com");
185+
await git.addConfig("commit.gpgsign", "false");
186+
await git.addRemote("origin", remoteDir);
187+
188+
await writeFile(path.join(workDir, "file.txt"), "line1\nline2\n");
189+
await git.add(["file.txt"]);
190+
await git.commit("initial");
191+
await git.push(["origin", "main"]);
192+
193+
await git.checkoutLocalBranch("feature");
194+
await writeFile(path.join(workDir, "file.txt"), "line1\nchanged\n");
195+
await writeFile(path.join(workDir, "added.txt"), "new file\n");
196+
await git.add(["file.txt", "added.txt"]);
197+
await git.commit("feature work, not pushed");
198+
199+
return { repoDir: workDir, remoteDir };
200+
}
201+
202+
it("returns per-file patches for commits not yet pushed", async () => {
203+
const { repoDir: workDir, remoteDir } = await setupBranchWithCommits();
204+
repoDir = workDir;
205+
206+
try {
207+
const patches = await getBranchDiffPatchesByPath(
208+
workDir,
209+
"main",
210+
"feature",
211+
);
212+
213+
expect(patches.has("file.txt")).toBe(true);
214+
expect(patches.has("added.txt")).toBe(true);
215+
expect(patches.get("file.txt")).toContain("-line2");
216+
expect(patches.get("file.txt")).toContain("+changed");
217+
expect(patches.get("added.txt")).toContain("+new file");
218+
} finally {
219+
await rm(remoteDir, { recursive: true, force: true });
220+
}
221+
});
222+
223+
it("returns deletions keyed by their path", async () => {
224+
const { repoDir: workDir, remoteDir } = await setupBranchWithCommits();
225+
repoDir = workDir;
226+
227+
try {
228+
const git = createGitClient(workDir);
229+
await unlink(path.join(workDir, "file.txt"));
230+
await git.add(["file.txt"]);
231+
await git.commit("delete file.txt");
232+
233+
const patches = await getBranchDiffPatchesByPath(
234+
workDir,
235+
"main",
236+
"feature",
237+
);
238+
239+
expect(patches.get("file.txt")).toContain("deleted file mode");
240+
} finally {
241+
await rm(remoteDir, { recursive: true, force: true });
242+
}
243+
});
244+
});

0 commit comments

Comments
 (0)