Skip to content

Commit 8e59c66

Browse files
committed
🤖 fix: git status indicator showing wrong behind count
When mux creates workspaces from bundles, origin/HEAD can be stale (pointing to a feature branch instead of main). The git status scripts were trusting symbolic-ref as the first method to find the primary branch, causing ahead/behind to be calculated against the wrong ref. Fix: Check origin/main and origin/master refs directly before falling back to origin/HEAD. This is fast (local ref lookup) and reliable. Additionally: - Changed default review base from HEAD to origin/main - Git status now uses the review base ref for ahead/behind calculations, creating consistency between the review panel and git status indicator - Added generateGitStatusScript() to support custom base refs Affected scripts: - GIT_STATUS_SCRIPT (ahead/behind calculation) - GIT_FETCH_SCRIPT (smart fetching) - useGitBranchDetails (hover card display) Also adds test coverage for parseGitShowBranchForStatus.
1 parent 306f496 commit 8e59c66

File tree

6 files changed

+238
-22
lines changed

6 files changed

+238
-22
lines changed

src/browser/components/RightSidebar/CodeReview/ReviewControls.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,10 @@ export const ReviewControls: React.FC<ReviewControlsProps> = ({
4343
lastRefreshInfo,
4444
}) => {
4545
// Global default base (used for new workspaces)
46-
const [defaultBase, setDefaultBase] = usePersistedState<string>("review-default-base", "HEAD");
46+
const [defaultBase, setDefaultBase] = usePersistedState<string>(
47+
"review-default-base",
48+
"origin/main"
49+
);
4750

4851
const handleBaseChange = (value: string) => {
4952
onFiltersChange({ ...filters, diffBase: value });

src/browser/components/RightSidebar/CodeReview/ReviewPanel.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
288288
);
289289

290290
// Global default base (shared across all workspaces)
291-
const [defaultBase] = usePersistedState<string>("review-default-base", "HEAD");
291+
const [defaultBase] = usePersistedState<string>("review-default-base", "origin/main");
292292

293293
// Persist diff base per workspace (falls back to global default)
294294
const [diffBase, setDiffBase] = usePersistedState(`review-diff-base:${workspaceId}`, defaultBase);

src/browser/components/hooks/useGitBranchDetails.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,10 +184,12 @@ export function useGitBranchDetails(
184184
# Get current branch
185185
CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo "main")
186186
187-
# Get primary branch (main or master)
188-
PRIMARY_BRANCH=$(git branch -r 2>/dev/null | grep -E 'origin/(main|master)$' | head -1 | sed 's@^.*origin/@@' || echo "main")
189-
190-
if [ -z "$PRIMARY_BRANCH" ]; then
187+
# Get primary branch (main or master) - check refs directly for reliability
188+
if git rev-parse --verify "refs/remotes/origin/main" >/dev/null 2>&1; then
189+
PRIMARY_BRANCH="main"
190+
elif git rev-parse --verify "refs/remotes/origin/master" >/dev/null 2>&1; then
191+
PRIMARY_BRANCH="master"
192+
else
191193
PRIMARY_BRANCH="main"
192194
fi
193195

src/browser/stores/GitStatusStore.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ import type { AppRouter } from "@/node/orpc/router";
33
import type { FrontendWorkspaceMetadata, GitStatus } from "@/common/types/workspace";
44
import { parseGitShowBranchForStatus } from "@/common/utils/git/parseGitStatus";
55
import {
6-
GIT_STATUS_SCRIPT,
6+
generateGitStatusScript,
77
GIT_FETCH_SCRIPT,
88
parseGitStatusScriptOutput,
99
} from "@/common/utils/git/gitStatus";
10+
import { readPersistedState } from "@/browser/hooks/usePersistedState";
1011
import { useSyncExternalStore } from "react";
1112
import { MapStore } from "./MapStore";
1213
import { isSSHRuntime } from "@/common/types/runtime";
@@ -285,6 +286,27 @@ export class GitStatusStore {
285286
);
286287
}
287288

289+
/**
290+
* Get the review base ref for a workspace.
291+
* Falls back to global default, then to undefined (auto-detect).
292+
*/
293+
private getReviewBaseRef(workspaceId: string): string | undefined {
294+
// Try workspace-specific base first
295+
const workspaceBase = readPersistedState<string>(`review-diff-base:${workspaceId}`, "");
296+
if (workspaceBase?.startsWith("origin/")) {
297+
return workspaceBase;
298+
}
299+
300+
// Fall back to global default
301+
const globalDefault = readPersistedState<string>("review-default-base", "origin/main");
302+
if (globalDefault?.startsWith("origin/")) {
303+
return globalDefault;
304+
}
305+
306+
// Let the script auto-detect
307+
return undefined;
308+
}
309+
288310
/**
289311
* Check git status for a single workspace.
290312
*/
@@ -297,9 +319,13 @@ export class GitStatusStore {
297319
}
298320

299321
try {
322+
// Get the review base ref to use for this workspace
323+
const baseRef = this.getReviewBaseRef(metadata.id);
324+
const script = generateGitStatusScript(baseRef);
325+
300326
const result = await this.client.workspace.executeBash({
301327
workspaceId: metadata.id,
302-
script: GIT_STATUS_SCRIPT,
328+
script,
303329
options: { timeout_secs: 5 },
304330
});
305331

src/common/utils/git/gitStatus.ts

Lines changed: 125 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,30 +4,113 @@
44
*/
55

66
/**
7-
* Bash script to get git status for a workspace.
8-
* Returns structured output with primary branch, show-branch, and dirty status.
7+
* Generate bash script to get git status for a workspace.
8+
* Returns structured output with base ref, show-branch, and dirty status.
9+
*
10+
* @param baseRef - The ref to compare against (e.g., "origin/main", "origin/develop").
11+
* If not provided, auto-detects origin/main or origin/master.
912
*/
10-
export const GIT_STATUS_SCRIPT = `
13+
export function generateGitStatusScript(baseRef?: string): string {
14+
// If baseRef is provided and looks valid, use it directly
15+
// Otherwise fall back to auto-detection
16+
if (baseRef?.startsWith("origin/")) {
17+
// Simple script when base is explicitly provided
18+
return `
19+
# Use provided base ref
20+
BASE_REF="${baseRef}"
21+
22+
# Verify the ref exists
23+
if ! git rev-parse --verify "$BASE_REF" >/dev/null 2>&1; then
24+
echo "ERROR: Base ref $BASE_REF does not exist"
25+
exit 1
26+
fi
27+
28+
# Get show-branch output for ahead/behind counts
29+
SHOW_BRANCH=$(git show-branch --sha1-name HEAD "$BASE_REF" 2>/dev/null)
30+
31+
if [ $? -ne 0 ]; then
32+
echo "ERROR: git show-branch failed"
33+
exit 1
34+
fi
35+
36+
# Check for dirty (uncommitted changes)
37+
DIRTY_COUNT=$(git status --porcelain 2>/dev/null | wc -l | tr -d ' ')
38+
39+
# Compute line deltas vs merge-base
40+
MERGE_BASE=$(git merge-base HEAD "$BASE_REF" 2>/dev/null || echo "")
41+
42+
OUTGOING_STATS="0 0"
43+
if [ -n "$MERGE_BASE" ]; then
44+
OUTGOING_STATS=$(git diff --numstat "$MERGE_BASE" 2>/dev/null | awk '{ if ($1 == "-" || $2 == "-") next; add += $1; del += $2 } END { printf "%d %d", add+0, del+0 }')
45+
if [ -z "$OUTGOING_STATS" ]; then
46+
OUTGOING_STATS="0 0"
47+
fi
48+
fi
49+
50+
INCOMING_STATS="0 0"
51+
if [ -n "$MERGE_BASE" ]; then
52+
INCOMING_STATS=$(git diff --numstat "$MERGE_BASE" "$BASE_REF" 2>/dev/null | awk '{ if ($1 == "-" || $2 == "-") next; add += $1; del += $2 } END { printf "%d %d", add+0, del+0 }')
53+
if [ -z "$INCOMING_STATS" ]; then
54+
INCOMING_STATS="0 0"
55+
fi
56+
fi
57+
58+
# Output sections (BASE_REF without origin/ prefix for consistency)
59+
echo "---PRIMARY---"
60+
echo "$BASE_REF" | sed 's@^origin/@@'
61+
echo "---SHOW_BRANCH---"
62+
echo "$SHOW_BRANCH"
63+
echo "---DIRTY---"
64+
echo "$DIRTY_COUNT"
65+
echo "---LINE_DELTA---"
66+
echo "$OUTGOING_STATS $INCOMING_STATS"
67+
`;
68+
}
69+
70+
// Auto-detect primary branch
71+
return `
1172
# Get primary branch - try multiple methods
73+
# Note: symbolic-ref can return stale values when cloned from bundles,
74+
# but we should trust it if it's a sensible default branch name.
1275
PRIMARY_BRANCH=""
1376
1477
# Method 1: symbolic-ref (fastest)
15-
PRIMARY_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')
78+
SYMREF_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')
1679
17-
# Method 2: remote show origin (fallback)
80+
# Accept symbolic-ref if it's a known default branch name (not a feature branch)
81+
# Common default branches: main, master, develop, trunk, default, release
82+
case "$SYMREF_BRANCH" in
83+
main|master|develop|trunk|default|release)
84+
PRIMARY_BRANCH="$SYMREF_BRANCH"
85+
;;
86+
esac
87+
88+
# Method 2: check for main or master directly (fast, reliable fallback)
89+
# This handles cases where symbolic-ref points to a stale feature branch
1890
if [ -z "$PRIMARY_BRANCH" ]; then
19-
PRIMARY_BRANCH=$(git remote show origin 2>/dev/null | grep 'HEAD branch' | cut -d' ' -f5)
91+
if git rev-parse --verify "refs/remotes/origin/main" >/dev/null 2>&1; then
92+
PRIMARY_BRANCH="main"
93+
elif git rev-parse --verify "refs/remotes/origin/master" >/dev/null 2>&1; then
94+
PRIMARY_BRANCH="master"
95+
fi
96+
fi
97+
98+
# Method 3: If symbolic-ref gave us something and main/master don't exist,
99+
# trust the symbolic-ref value (handles non-standard default branches)
100+
if [ -z "$PRIMARY_BRANCH" ] && [ -n "$SYMREF_BRANCH" ]; then
101+
if git rev-parse --verify "refs/remotes/origin/$SYMREF_BRANCH" >/dev/null 2>&1; then
102+
PRIMARY_BRANCH="$SYMREF_BRANCH"
103+
fi
20104
fi
21105
22-
# Method 3: check for main or master
106+
# Method 4: remote show origin (slow, network call, last resort)
23107
if [ -z "$PRIMARY_BRANCH" ]; then
24-
PRIMARY_BRANCH=$(git branch -r 2>/dev/null | grep -E 'origin/(main|master)$' | head -1 | sed 's@^.*origin/@@')
108+
PRIMARY_BRANCH=$(git remote show origin 2>/dev/null | grep 'HEAD branch' | cut -d' ' -f5)
25109
fi
26110
27-
# Exit if we can't determine primary branch
111+
# Final fallback: assume main
28112
if [ -z "$PRIMARY_BRANCH" ]; then
29-
echo "ERROR: Could not determine primary branch"
30-
exit 1
113+
PRIMARY_BRANCH="main"
31114
fi
32115
33116
# Get show-branch output for ahead/behind counts
@@ -74,6 +157,13 @@ echo "$DIRTY_COUNT"
74157
echo "---LINE_DELTA---"
75158
echo "$OUTGOING_STATS $INCOMING_STATS"
76159
`;
160+
}
161+
162+
/**
163+
* @deprecated Use generateGitStatusScript() instead
164+
* Bash script to get git status for a workspace (auto-detects primary branch).
165+
*/
166+
export const GIT_STATUS_SCRIPT = generateGitStatusScript();
77167

78168
/**
79169
* Parse the output from GIT_STATUS_SCRIPT.
@@ -142,10 +232,32 @@ export SSH_ASKPASS=echo
142232
export GIT_SSH_COMMAND="\${GIT_SSH_COMMAND:-ssh} -o BatchMode=yes -o StrictHostKeyChecking=accept-new"
143233
144234
# Get primary branch name
145-
PRIMARY_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')
235+
# Note: symbolic-ref can return stale values, but trust it if it's a sensible name
236+
PRIMARY_BRANCH=""
237+
SYMREF_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')
238+
239+
# Accept symbolic-ref if it's a known default branch name
240+
case "$SYMREF_BRANCH" in
241+
main|master|develop|trunk|default|release)
242+
PRIMARY_BRANCH="$SYMREF_BRANCH"
243+
;;
244+
esac
245+
246+
# Fallback: check for main or master directly
146247
if [ -z "$PRIMARY_BRANCH" ]; then
147-
PRIMARY_BRANCH=$(git remote show origin 2>/dev/null | grep 'HEAD branch' | cut -d' ' -f5)
248+
if git rev-parse --verify "refs/remotes/origin/main" >/dev/null 2>&1; then
249+
PRIMARY_BRANCH="main"
250+
elif git rev-parse --verify "refs/remotes/origin/master" >/dev/null 2>&1; then
251+
PRIMARY_BRANCH="master"
252+
fi
148253
fi
254+
255+
# Trust symbolic-ref if main/master don't exist (non-standard default branches)
256+
if [ -z "$PRIMARY_BRANCH" ] && [ -n "$SYMREF_BRANCH" ]; then
257+
PRIMARY_BRANCH="$SYMREF_BRANCH"
258+
fi
259+
260+
# Final fallback
149261
if [ -z "$PRIMARY_BRANCH" ]; then
150262
PRIMARY_BRANCH="main"
151263
fi

src/common/utils/git/parseGitStatus.test.ts

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { parseGitRevList } from "./parseGitStatus";
1+
import { parseGitRevList, parseGitShowBranchForStatus } from "./parseGitStatus";
22

33
// Base result shape with zero line deltas (parseGitRevList doesn't compute these)
44
const base = {
@@ -39,3 +39,76 @@ describe("parseGitRevList", () => {
3939
expect(parseGitRevList("\t")).toBe(null);
4040
});
4141
});
42+
43+
describe("parseGitShowBranchForStatus", () => {
44+
test("parses 2-branch output correctly", () => {
45+
const output = `! [HEAD] feat: add feature
46+
! [origin/main] fix: bug fix
47+
--
48+
+ [cf9cbfb7] feat: add feature
49+
++ [306f4968] fix: bug fix`;
50+
51+
const result = parseGitShowBranchForStatus(output);
52+
expect(result).not.toBeNull();
53+
expect(result!.ahead).toBe(1);
54+
expect(result!.behind).toBe(0);
55+
});
56+
57+
test("parses 2-branch output with behind commits", () => {
58+
const output = `! [HEAD] feat: add feature
59+
! [origin/main] latest on main
60+
--
61+
+ [cf9cbfb7] feat: add feature
62+
+ [1] behind commit 1
63+
+ [2] behind commit 2
64+
+ [3] behind commit 3
65+
++ [base] common ancestor`;
66+
67+
const result = parseGitShowBranchForStatus(output);
68+
expect(result).not.toBeNull();
69+
expect(result!.ahead).toBe(1);
70+
expect(result!.behind).toBe(3);
71+
});
72+
73+
test("parses 2-branch output with many behind commits", () => {
74+
const output = `! [HEAD] feat: add feature
75+
! [origin/main] latest on main
76+
--
77+
+ [cf9cbfb7] feat: add feature
78+
+ [1] behind commit 1
79+
+ [2] behind commit 2
80+
+ [3] behind commit 3
81+
+ [4] behind commit 4
82+
+ [5] behind commit 5
83+
+ [6] behind commit 6
84+
+ [7] behind commit 7
85+
+ [8] behind commit 8
86+
+ [9] behind commit 9
87+
++ [base] common ancestor`;
88+
89+
const result = parseGitShowBranchForStatus(output);
90+
expect(result).not.toBeNull();
91+
expect(result!.ahead).toBe(1);
92+
expect(result!.behind).toBe(9);
93+
});
94+
95+
test("handles 3-branch output (misuse case)", () => {
96+
// This tests what happens if 3-branch output is accidentally fed to the parser
97+
// The parser uses columns 0 and 1, ignoring column 2
98+
const output = `! [HEAD] feat: add feature
99+
! [origin/main] fix: bug fix
100+
! [origin/feature] feat: add feature
101+
---
102+
+ + [cf9cbfb7] feat: add feature
103+
++ [306f4968] fix: bug fix`;
104+
105+
// With 3 columns: "+ +" means col0='+', col1=' ', col2='+'
106+
// Parser sees col0='+', col1=' ' -> ahead
107+
// With 3 columns: " ++" means col0=' ', col1='+', col2='+'
108+
// Parser sees col0=' ', col1='+' -> behind
109+
const result = parseGitShowBranchForStatus(output);
110+
expect(result).not.toBeNull();
111+
expect(result!.ahead).toBe(1); // "+ +" has col0='+', col1=' '
112+
expect(result!.behind).toBe(1); // " ++" has col0=' ', col1='+'
113+
});
114+
});

0 commit comments

Comments
 (0)