Skip to content

Commit 51a1dde

Browse files
committed
🤖 fix: ignore stale origin/HEAD in git status scripts
#1414 unified the review base with git status, but fallback branch detection still trusted refs/remotes/origin/HEAD. Workspaces created from bundles can inherit a stale origin/HEAD (e.g. pointing at a feature branch), causing ahead/behind to be computed against the wrong remote ref. This follow-up: - Treats origin/HEAD as "auto-detect" even if configured as the base ref - Makes generateGitStatusScript() ignore suspicious origin/HEAD values and prefer origin/main or origin/master - Adds main↔master fallback when a configured origin/<branch> does not exist - Updates GIT_FETCH_SCRIPT to derive the remote default branch via `git ls-remote --symref origin HEAD` - Updates Sidebar story mocks to detect the status script via output markers - Adds unit tests for parseGitShowBranchForStatus
1 parent 00d3554 commit 51a1dde

File tree

4 files changed

+126
-31
lines changed

4 files changed

+126
-31
lines changed

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/stories/App.sidebar.stories.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ function createGitStatusExecutor(gitStatus?: Map<string, GitStatusFixture>) {
113113
}
114114

115115
// GitStatusStore consolidated status script
116-
if (script.includes("PRIMARY_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD")) {
116+
if (script.includes("---PRIMARY---") && script.includes("---SHOW_BRANCH---")) {
117117
const output = createGitStatusOutput(status);
118118
return Promise.resolve({ success: true as const, output, exitCode: 0, wall_duration_ms: 50 });
119119
}

src/common/utils/git/gitStatus.ts

Lines changed: 45 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,22 +23,46 @@ PREFERRED_BRANCH="${preferredBranch}"
2323
if [ -n "$PREFERRED_BRANCH" ]; then
2424
if git rev-parse --verify "refs/remotes/origin/$PREFERRED_BRANCH" >/dev/null 2>&1; then
2525
PRIMARY_BRANCH="$PREFERRED_BRANCH"
26+
elif [ "$PREFERRED_BRANCH" = "main" ] && git rev-parse --verify "refs/remotes/origin/master" >/dev/null 2>&1; then
27+
PRIMARY_BRANCH="master"
28+
elif [ "$PREFERRED_BRANCH" = "master" ] && git rev-parse --verify "refs/remotes/origin/main" >/dev/null 2>&1; then
29+
PRIMARY_BRANCH="main"
2630
fi
2731
fi
2832
2933
# Fall back to auto-detection
3034
if [ -z "$PRIMARY_BRANCH" ]; then
31-
# Method 1: symbolic-ref (fastest)
32-
PRIMARY_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')
35+
# symbolic-ref can be stale (e.g., when cloned from a bundle)
36+
SYMREF_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')
37+
38+
# Trust symbolic-ref only if it looks like a default branch name
39+
case "$SYMREF_BRANCH" in
40+
main|master|develop|trunk|default|release)
41+
if git rev-parse --verify "refs/remotes/origin/$SYMREF_BRANCH" >/dev/null 2>&1; then
42+
PRIMARY_BRANCH="$SYMREF_BRANCH"
43+
fi
44+
;;
45+
esac
46+
47+
# Prefer origin/main or origin/master if present (handles stale origin/HEAD)
48+
if [ -z "$PRIMARY_BRANCH" ]; then
49+
if git rev-parse --verify "refs/remotes/origin/main" >/dev/null 2>&1; then
50+
PRIMARY_BRANCH="main"
51+
elif git rev-parse --verify "refs/remotes/origin/master" >/dev/null 2>&1; then
52+
PRIMARY_BRANCH="master"
53+
fi
54+
fi
3355
34-
# Method 2: remote show origin (fallback)
56+
# Fallback: ask origin (may require network)
3557
if [ -z "$PRIMARY_BRANCH" ]; then
3658
PRIMARY_BRANCH=$(git remote show origin 2>/dev/null | grep 'HEAD branch' | cut -d' ' -f5)
3759
fi
3860
39-
# Method 3: check for main or master
40-
if [ -z "$PRIMARY_BRANCH" ]; then
41-
PRIMARY_BRANCH=$(git branch -r 2>/dev/null | grep -E 'origin/(main|master)$' | head -1 | sed 's@^.*origin/@@')
61+
# Last resort: use symbolic-ref if it points to an existing remote-tracking ref
62+
if [ -z "$PRIMARY_BRANCH" ] && [ -n "$SYMREF_BRANCH" ]; then
63+
if git rev-parse --verify "refs/remotes/origin/$SYMREF_BRANCH" >/dev/null 2>&1; then
64+
PRIMARY_BRANCH="$SYMREF_BRANCH"
65+
fi
4266
fi
4367
fi
4468
@@ -48,8 +72,10 @@ if [ -z "$PRIMARY_BRANCH" ]; then
4872
exit 1
4973
fi
5074
75+
BASE_REF="origin/$PRIMARY_BRANCH"
76+
5177
# Get show-branch output for ahead/behind counts
52-
SHOW_BRANCH=$(git show-branch --sha1-name HEAD "origin/$PRIMARY_BRANCH" 2>/dev/null)
78+
SHOW_BRANCH=$(git show-branch --sha1-name HEAD "$BASE_REF" 2>/dev/null)
5379
5480
if [ $? -ne 0 ]; then
5581
echo "ERROR: git show-branch failed"
@@ -62,7 +88,7 @@ DIRTY_COUNT=$(git status --porcelain 2>/dev/null | wc -l | tr -d ' ')
6288
# Compute line deltas (additions/deletions) vs merge-base with origin's primary branch.
6389
#
6490
# We emit *only* totals to keep output tiny (avoid output truncation in large repos).
65-
MERGE_BASE=$(git merge-base HEAD "origin/$PRIMARY_BRANCH" 2>/dev/null || echo "")
91+
MERGE_BASE=$(git merge-base HEAD "$BASE_REF" 2>/dev/null || echo "")
6692
6793
# Outgoing: local changes vs merge-base (working tree vs base, includes uncommitted changes)
6894
OUTGOING_STATS="0 0"
@@ -76,7 +102,7 @@ fi
76102
# Incoming: remote primary branch changes vs merge-base
77103
INCOMING_STATS="0 0"
78104
if [ -n "$MERGE_BASE" ]; then
79-
INCOMING_STATS=$(git diff --numstat "$MERGE_BASE" "origin/$PRIMARY_BRANCH" 2>/dev/null | awk '{ if ($1 == "-" || $2 == "-") next; add += $1; del += $2 } END { printf "%d %d", add+0, del+0 }')
105+
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 }')
80106
if [ -z "$INCOMING_STATS" ]; then
81107
INCOMING_STATS="0 0"
82108
fi
@@ -153,10 +179,10 @@ export function parseGitStatusScriptOutput(output: string): ParsedGitStatusOutpu
153179
* (e.g., IDE or user already fetched).
154180
*
155181
* Flow:
156-
* 1. ls-remote to get remote SHA (no lock, network only)
157-
* 2. cat-file to check if SHA exists locally (no lock)
158-
* 3. If local: skip fetch (no lock needed)
159-
* 4. If not local: fetch to get new commits (lock, but rare)
182+
* 1. ls-remote --symref origin HEAD to get default branch + SHA (no lock, network only)
183+
* 2. rev-parse to check local remote-tracking SHA (no lock)
184+
* 3. If local already matches: skip fetch (no lock needed)
185+
* 4. If not: fetch updates (lock, but rare)
160186
*/
161187
export const GIT_FETCH_SCRIPT = `
162188
# Disable ALL prompts
@@ -165,19 +191,13 @@ export GIT_ASKPASS=echo
165191
export SSH_ASKPASS=echo
166192
export GIT_SSH_COMMAND="\${GIT_SSH_COMMAND:-ssh} -o BatchMode=yes -o StrictHostKeyChecking=accept-new"
167193
168-
# Get primary branch name
169-
PRIMARY_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')
170-
if [ -z "$PRIMARY_BRANCH" ]; then
171-
PRIMARY_BRANCH=$(git remote show origin 2>/dev/null | grep 'HEAD branch' | cut -d' ' -f5)
172-
fi
173-
if [ -z "$PRIMARY_BRANCH" ]; then
174-
PRIMARY_BRANCH="main"
175-
fi
194+
# Determine remote default branch + SHA via ls-remote (no lock, network only)
195+
REMOTE_INFO=$(git ls-remote --symref origin HEAD 2>/dev/null || echo "")
196+
PRIMARY_BRANCH=$(printf '%s\n' "$REMOTE_INFO" | awk '$1=="ref:" && $3=="HEAD" {sub("^refs/heads/","",$2); print $2; exit}')
197+
REMOTE_SHA=$(printf '%s\n' "$REMOTE_INFO" | awk '$2=="HEAD" && $1!="ref:" {print $1; exit}')
176198
177-
# Check remote SHA via ls-remote (no lock, network only)
178-
REMOTE_SHA=$(git ls-remote origin "refs/heads/$PRIMARY_BRANCH" 2>/dev/null | cut -f1)
179-
if [ -z "$REMOTE_SHA" ]; then
180-
echo "SKIP: Could not get remote SHA"
199+
if [ -z "$PRIMARY_BRANCH" ] || [ -z "$REMOTE_SHA" ]; then
200+
echo "SKIP: Could not get remote HEAD"
181201
exit 0
182202
fi
183203

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)