Skip to content

Commit 7b186e8

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 7b186e8

File tree

4 files changed

+112
-33
lines changed

4 files changed

+112
-33
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: 51 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,14 @@
99
*
1010
* @param baseRef - The ref to compare against (e.g., "origin/main").
1111
* If not provided or not an origin/ ref, auto-detects.
12+
* "origin/HEAD" is treated as auto-detect because it can be stale
13+
* (e.g. in repos cloned from bundles).
1214
*/
1315
export function generateGitStatusScript(baseRef?: string): string {
14-
// Extract branch name if it's an origin/ ref, otherwise empty for auto-detect
15-
const preferredBranch = baseRef?.startsWith("origin/") ? baseRef.replace(/^origin\//, "") : "";
16+
// Extract branch name if it's an origin/ ref, otherwise empty for auto-detect.
17+
// Note: origin/HEAD is ignored because it may point at a stale feature branch.
18+
const rawPreferredBranch = baseRef?.startsWith("origin/") ? baseRef.replace(/^origin\//, "") : "";
19+
const preferredBranch = rawPreferredBranch === "HEAD" ? "" : rawPreferredBranch;
1620

1721
return `
1822
# Determine primary branch to compare against
@@ -23,22 +27,46 @@ PREFERRED_BRANCH="${preferredBranch}"
2327
if [ -n "$PREFERRED_BRANCH" ]; then
2428
if git rev-parse --verify "refs/remotes/origin/$PREFERRED_BRANCH" >/dev/null 2>&1; then
2529
PRIMARY_BRANCH="$PREFERRED_BRANCH"
30+
elif [ "$PREFERRED_BRANCH" = "main" ] && git rev-parse --verify "refs/remotes/origin/master" >/dev/null 2>&1; then
31+
PRIMARY_BRANCH="master"
32+
elif [ "$PREFERRED_BRANCH" = "master" ] && git rev-parse --verify "refs/remotes/origin/main" >/dev/null 2>&1; then
33+
PRIMARY_BRANCH="main"
2634
fi
2735
fi
2836
2937
# Fall back to auto-detection
3038
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/@@')
39+
# symbolic-ref can be stale (e.g., when cloned from a bundle)
40+
SYMREF_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')
41+
42+
# Trust symbolic-ref only if it looks like a default branch name
43+
case "$SYMREF_BRANCH" in
44+
main|master|develop|trunk|default|release)
45+
if git rev-parse --verify "refs/remotes/origin/$SYMREF_BRANCH" >/dev/null 2>&1; then
46+
PRIMARY_BRANCH="$SYMREF_BRANCH"
47+
fi
48+
;;
49+
esac
50+
51+
# Prefer origin/main or origin/master if present (handles stale origin/HEAD)
52+
if [ -z "$PRIMARY_BRANCH" ]; then
53+
if git rev-parse --verify "refs/remotes/origin/main" >/dev/null 2>&1; then
54+
PRIMARY_BRANCH="main"
55+
elif git rev-parse --verify "refs/remotes/origin/master" >/dev/null 2>&1; then
56+
PRIMARY_BRANCH="master"
57+
fi
58+
fi
3359
34-
# Method 2: remote show origin (fallback)
60+
# Fallback: ask origin (may require network)
3561
if [ -z "$PRIMARY_BRANCH" ]; then
3662
PRIMARY_BRANCH=$(git remote show origin 2>/dev/null | grep 'HEAD branch' | cut -d' ' -f5)
3763
fi
3864
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/@@')
65+
# Last resort: use symbolic-ref if it points to an existing remote-tracking ref
66+
if [ -z "$PRIMARY_BRANCH" ] && [ -n "$SYMREF_BRANCH" ]; then
67+
if git rev-parse --verify "refs/remotes/origin/$SYMREF_BRANCH" >/dev/null 2>&1; then
68+
PRIMARY_BRANCH="$SYMREF_BRANCH"
69+
fi
4270
fi
4371
fi
4472
@@ -48,8 +76,10 @@ if [ -z "$PRIMARY_BRANCH" ]; then
4876
exit 1
4977
fi
5078
79+
BASE_REF="origin/$PRIMARY_BRANCH"
80+
5181
# Get show-branch output for ahead/behind counts
52-
SHOW_BRANCH=$(git show-branch --sha1-name HEAD "origin/$PRIMARY_BRANCH" 2>/dev/null)
82+
SHOW_BRANCH=$(git show-branch --sha1-name HEAD "$BASE_REF" 2>/dev/null)
5383
5484
if [ $? -ne 0 ]; then
5585
echo "ERROR: git show-branch failed"
@@ -62,7 +92,7 @@ DIRTY_COUNT=$(git status --porcelain 2>/dev/null | wc -l | tr -d ' ')
6292
# Compute line deltas (additions/deletions) vs merge-base with origin's primary branch.
6393
#
6494
# 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 "")
95+
MERGE_BASE=$(git merge-base HEAD "$BASE_REF" 2>/dev/null || echo "")
6696
6797
# Outgoing: local changes vs merge-base (working tree vs base, includes uncommitted changes)
6898
OUTGOING_STATS="0 0"
@@ -76,7 +106,7 @@ fi
76106
# Incoming: remote primary branch changes vs merge-base
77107
INCOMING_STATS="0 0"
78108
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 }')
109+
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 }')
80110
if [ -z "$INCOMING_STATS" ]; then
81111
INCOMING_STATS="0 0"
82112
fi
@@ -153,10 +183,10 @@ export function parseGitStatusScriptOutput(output: string): ParsedGitStatusOutpu
153183
* (e.g., IDE or user already fetched).
154184
*
155185
* 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)
186+
* 1. ls-remote --symref origin HEAD to get default branch + SHA (no lock, network only)
187+
* 2. rev-parse to check local remote-tracking SHA (no lock)
188+
* 3. If local already matches: skip fetch (no lock needed)
189+
* 4. If not: fetch updates (lock, but rare)
160190
*/
161191
export const GIT_FETCH_SCRIPT = `
162192
# Disable ALL prompts
@@ -165,19 +195,13 @@ export GIT_ASKPASS=echo
165195
export SSH_ASKPASS=echo
166196
export GIT_SSH_COMMAND="\${GIT_SSH_COMMAND:-ssh} -o BatchMode=yes -o StrictHostKeyChecking=accept-new"
167197
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
198+
# Determine remote default branch + SHA via ls-remote (no lock, network only)
199+
REMOTE_INFO=$(git ls-remote --symref origin HEAD 2>/dev/null || echo "")
200+
PRIMARY_BRANCH=$(printf '%s\n' "$REMOTE_INFO" | awk '$1=="ref:" && $3=="HEAD" {sub("^refs/heads/","",$2); print $2; exit}')
201+
REMOTE_SHA=$(printf '%s\n' "$REMOTE_INFO" | awk '$2=="HEAD" && $1!="ref:" {print $1; exit}')
176202
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"
203+
if [ -z "$PRIMARY_BRANCH" ] || [ -z "$REMOTE_SHA" ]; then
204+
echo "SKIP: Could not get remote HEAD"
181205
exit 0
182206
fi
183207

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

Lines changed: 54 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,56 @@ describe("parseGitRevList", () => {
3939
expect(parseGitRevList("\t")).toBe(null);
4040
});
4141
});
42+
43+
function makeBehindCommitLines(count: number): string {
44+
return Array.from({ length: count }, (_, i) => ` + [${i + 1}] behind commit ${i + 1}`).join("\n");
45+
}
46+
47+
describe("parseGitShowBranchForStatus", () => {
48+
test("parses 2-branch output correctly", () => {
49+
const output = `! [HEAD] feat: add feature
50+
! [origin/main] fix: bug fix
51+
--
52+
+ [cf9cbfb7] feat: add feature
53+
++ [306f4968] fix: bug fix`;
54+
55+
const result = parseGitShowBranchForStatus(output);
56+
expect(result).not.toBeNull();
57+
expect(result!.ahead).toBe(1);
58+
expect(result!.behind).toBe(0);
59+
});
60+
61+
test("parses 2-branch output with behind commits", () => {
62+
const output = `! [HEAD] feat: add feature
63+
! [origin/main] latest on main
64+
--
65+
+ [cf9cbfb7] feat: add feature
66+
${makeBehindCommitLines(9)}
67+
++ [base] common ancestor`;
68+
69+
const result = parseGitShowBranchForStatus(output);
70+
expect(result).not.toBeNull();
71+
expect(result!.ahead).toBe(1);
72+
expect(result!.behind).toBe(9);
73+
});
74+
75+
test("handles 3-branch output (misuse case)", () => {
76+
// This tests what happens if 3-branch output is accidentally fed to the parser.
77+
// The parser uses columns 0 and 1, ignoring column 2.
78+
const output = `! [HEAD] feat: add feature
79+
! [origin/main] fix: bug fix
80+
! [origin/feature] feat: add feature
81+
---
82+
+ + [cf9cbfb7] feat: add feature
83+
++ [306f4968] fix: bug fix`;
84+
85+
// With 3 columns: "+ +" means col0='+', col1=' ', col2='+'
86+
// Parser sees col0='+', col1=' ' -> ahead
87+
// With 3 columns: " ++" means col0=' ', col1='+', col2='+'
88+
// Parser sees col0=' ', col1='+' -> behind
89+
const result = parseGitShowBranchForStatus(output);
90+
expect(result).not.toBeNull();
91+
expect(result!.ahead).toBe(1); // "+ +" has col0='+', col1=' '
92+
expect(result!.behind).toBe(1); // " ++" has col0=' ', col1='+'
93+
});
94+
});

0 commit comments

Comments
 (0)