Skip to content

Commit 4f30163

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 ad0eb2e commit 4f30163

File tree

6 files changed

+229
-22
lines changed

6 files changed

+229
-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: 7 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";
@@ -297,9 +298,13 @@ export class GitStatusStore {
297298
}
298299

299300
try {
301+
// Use global default base ref for git status (defaults to origin/main)
302+
const baseRef = readPersistedState<string>("review-default-base", "origin/main");
303+
const script = generateGitStatusScript(baseRef);
304+
300305
const result = await this.client.workspace.executeBash({
301306
workspaceId: metadata.id,
302-
script: GIT_STATUS_SCRIPT,
307+
script,
303308
options: { timeout_secs: 5 },
304309
});
305310

src/common/utils/git/gitStatus.ts

Lines changed: 137 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,30 +4,125 @@
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 with fallback to auto-detection
15+
// Otherwise fall back to auto-detection directly
16+
if (baseRef?.startsWith("origin/")) {
17+
// Script that tries provided base first, falls back to auto-detection
18+
return `
19+
# Try provided base ref first, fall back to auto-detection if it doesn't exist
20+
PREFERRED_BASE="${baseRef}"
21+
BASE_REF=""
22+
23+
if git rev-parse --verify "$PREFERRED_BASE" >/dev/null 2>&1; then
24+
BASE_REF="$PREFERRED_BASE"
25+
else
26+
# Provided base doesn't exist, auto-detect primary branch
27+
if git rev-parse --verify "refs/remotes/origin/main" >/dev/null 2>&1; then
28+
BASE_REF="origin/main"
29+
elif git rev-parse --verify "refs/remotes/origin/master" >/dev/null 2>&1; then
30+
BASE_REF="origin/master"
31+
fi
32+
fi
33+
34+
# Final fallback
35+
if [ -z "$BASE_REF" ]; then
36+
echo "ERROR: Could not find base ref"
37+
exit 1
38+
fi
39+
40+
# Get show-branch output for ahead/behind counts
41+
SHOW_BRANCH=$(git show-branch --sha1-name HEAD "$BASE_REF" 2>/dev/null)
42+
43+
if [ $? -ne 0 ]; then
44+
echo "ERROR: git show-branch failed"
45+
exit 1
46+
fi
47+
48+
# Check for dirty (uncommitted changes)
49+
DIRTY_COUNT=$(git status --porcelain 2>/dev/null | wc -l | tr -d ' ')
50+
51+
# Compute line deltas vs merge-base
52+
MERGE_BASE=$(git merge-base HEAD "$BASE_REF" 2>/dev/null || echo "")
53+
54+
OUTGOING_STATS="0 0"
55+
if [ -n "$MERGE_BASE" ]; then
56+
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 }')
57+
if [ -z "$OUTGOING_STATS" ]; then
58+
OUTGOING_STATS="0 0"
59+
fi
60+
fi
61+
62+
INCOMING_STATS="0 0"
63+
if [ -n "$MERGE_BASE" ]; then
64+
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 }')
65+
if [ -z "$INCOMING_STATS" ]; then
66+
INCOMING_STATS="0 0"
67+
fi
68+
fi
69+
70+
# Output sections (BASE_REF without origin/ prefix for consistency)
71+
echo "---PRIMARY---"
72+
echo "$BASE_REF" | sed 's@^origin/@@'
73+
echo "---SHOW_BRANCH---"
74+
echo "$SHOW_BRANCH"
75+
echo "---DIRTY---"
76+
echo "$DIRTY_COUNT"
77+
echo "---LINE_DELTA---"
78+
echo "$OUTGOING_STATS $INCOMING_STATS"
79+
`;
80+
}
81+
82+
// Auto-detect primary branch
83+
return `
1184
# Get primary branch - try multiple methods
85+
# Note: symbolic-ref can return stale values when cloned from bundles,
86+
# but we should trust it if it's a sensible default branch name.
1287
PRIMARY_BRANCH=""
1388
1489
# Method 1: symbolic-ref (fastest)
15-
PRIMARY_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')
90+
SYMREF_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')
91+
92+
# Accept symbolic-ref if it's a known default branch name (not a feature branch)
93+
# Common default branches: main, master, develop, trunk, default, release
94+
case "$SYMREF_BRANCH" in
95+
main|master|develop|trunk|default|release)
96+
PRIMARY_BRANCH="$SYMREF_BRANCH"
97+
;;
98+
esac
1699
17-
# Method 2: remote show origin (fallback)
100+
# Method 2: check for main or master directly (fast, reliable fallback)
101+
# This handles cases where symbolic-ref points to a stale feature branch
18102
if [ -z "$PRIMARY_BRANCH" ]; then
19-
PRIMARY_BRANCH=$(git remote show origin 2>/dev/null | grep 'HEAD branch' | cut -d' ' -f5)
103+
if git rev-parse --verify "refs/remotes/origin/main" >/dev/null 2>&1; then
104+
PRIMARY_BRANCH="main"
105+
elif git rev-parse --verify "refs/remotes/origin/master" >/dev/null 2>&1; then
106+
PRIMARY_BRANCH="master"
107+
fi
108+
fi
109+
110+
# Method 3: If symbolic-ref gave us something and main/master don't exist,
111+
# trust the symbolic-ref value (handles non-standard default branches)
112+
if [ -z "$PRIMARY_BRANCH" ] && [ -n "$SYMREF_BRANCH" ]; then
113+
if git rev-parse --verify "refs/remotes/origin/$SYMREF_BRANCH" >/dev/null 2>&1; then
114+
PRIMARY_BRANCH="$SYMREF_BRANCH"
115+
fi
20116
fi
21117
22-
# Method 3: check for main or master
118+
# Method 4: remote show origin (slow, network call, last resort)
23119
if [ -z "$PRIMARY_BRANCH" ]; then
24-
PRIMARY_BRANCH=$(git branch -r 2>/dev/null | grep -E 'origin/(main|master)$' | head -1 | sed 's@^.*origin/@@')
120+
PRIMARY_BRANCH=$(git remote show origin 2>/dev/null | grep 'HEAD branch' | cut -d' ' -f5)
25121
fi
26122
27-
# Exit if we can't determine primary branch
123+
# Final fallback: assume main
28124
if [ -z "$PRIMARY_BRANCH" ]; then
29-
echo "ERROR: Could not determine primary branch"
30-
exit 1
125+
PRIMARY_BRANCH="main"
31126
fi
32127
33128
# Get show-branch output for ahead/behind counts
@@ -74,6 +169,13 @@ echo "$DIRTY_COUNT"
74169
echo "---LINE_DELTA---"
75170
echo "$OUTGOING_STATS $INCOMING_STATS"
76171
`;
172+
}
173+
174+
/**
175+
* @deprecated Use generateGitStatusScript() instead
176+
* Bash script to get git status for a workspace (auto-detects primary branch).
177+
*/
178+
export const GIT_STATUS_SCRIPT = generateGitStatusScript();
77179

78180
/**
79181
* Parse the output from GIT_STATUS_SCRIPT.
@@ -142,10 +244,32 @@ export SSH_ASKPASS=echo
142244
export GIT_SSH_COMMAND="\${GIT_SSH_COMMAND:-ssh} -o BatchMode=yes -o StrictHostKeyChecking=accept-new"
143245
144246
# Get primary branch name
145-
PRIMARY_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')
247+
# Note: symbolic-ref can return stale values, but trust it if it's a sensible name
248+
PRIMARY_BRANCH=""
249+
SYMREF_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')
250+
251+
# Accept symbolic-ref if it's a known default branch name
252+
case "$SYMREF_BRANCH" in
253+
main|master|develop|trunk|default|release)
254+
PRIMARY_BRANCH="$SYMREF_BRANCH"
255+
;;
256+
esac
257+
258+
# Fallback: check for main or master directly
146259
if [ -z "$PRIMARY_BRANCH" ]; then
147-
PRIMARY_BRANCH=$(git remote show origin 2>/dev/null | grep 'HEAD branch' | cut -d' ' -f5)
260+
if git rev-parse --verify "refs/remotes/origin/main" >/dev/null 2>&1; then
261+
PRIMARY_BRANCH="main"
262+
elif git rev-parse --verify "refs/remotes/origin/master" >/dev/null 2>&1; then
263+
PRIMARY_BRANCH="master"
264+
fi
265+
fi
266+
267+
# Trust symbolic-ref if main/master don't exist (non-standard default branches)
268+
if [ -z "$PRIMARY_BRANCH" ] && [ -n "$SYMREF_BRANCH" ]; then
269+
PRIMARY_BRANCH="$SYMREF_BRANCH"
148270
fi
271+
272+
# Final fallback
149273
if [ -z "$PRIMARY_BRANCH" ]; then
150274
PRIMARY_BRANCH="main"
151275
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)