fix(review): detect remote default branch via ls-remote fallback#609
Merged
backnotprop merged 1 commit intomainfrom Apr 24, 2026
Merged
fix(review): detect remote default branch via ls-remote fallback#609backnotprop merged 1 commit intomainfrom
backnotprop merged 1 commit intomainfrom
Conversation
backnotprop
force-pushed
the
fix/default-branch-detection
branch
from
509897c to
ee05c9e
Compare
getDefaultBranch relied on `symbolic-ref refs/remotes/origin/HEAD` which is commonly unset in worktrees, manual remote setups, and long-lived repos. When absent, the code fell through to local `main`/`master`, silently losing the upstream-preference behavior. Adds `detectRemoteDefaultBranch()` — queries the remote via `git ls-remote --symref origin HEAD` to find the actual default branch name (works for any name: develop, trunk, etc.). The call is fire-and-forget at server startup: non-blocking, runs concurrently while the browser loads. By the time the user opens Branch diff or PR Diff (usually 1-2 seconds later), the result has arrived and `currentBase` has been silently upgraded to the upstream ref. If offline or slow, the 5-second timeout fires and the local fallback sticks. If the user has already switched bases manually, their choice is never overwritten. `ReviewGitRuntime.runGit` gains an optional `timeoutMs` param. Bun kills the process via setTimeout + proc.kill(). Pi uses spawnSync's native `timeout` option. Both treat timeout as a non-zero exit. For provenance purposes, this commit was AI assisted.
backnotprop
force-pushed
the
fix/default-branch-detection
branch
from
ee05c9e to
cbfa0c7
Compare
backnotprop
added a commit
that referenced
this pull request
Apr 24, 2026
Pi's reviewRuntime.runGit used spawnSync, which blocked the Node.js event loop during git operations. The ls-remote call from #609 could freeze the HTTP server for up to 5 seconds on slow networks. Now mirrors the Bun server's async pattern: spawn with piped stdio, kill timer for timeouts, stream-based stdout/stderr collection. For provenance purposes, this commit was AI assisted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
origin/HEADisn't set (common in worktrees, manual remotes, long-lived repos), the base picker defaulted to localmaininstead of upstreamorigin/main. This made the "prefer upstream" behavior from feat(review): custom base branch for code review (#599) #602 silently ineffective for many users.git ls-remote --symref origin HEADas a network-based fallback with a 3-second timeout. Detects any default branch name (develop, trunk, etc.) without hardcoded guesses.ReviewGitRuntime.runGitgains an optionaltimeoutMsparam — Bun usesproc.kill()timer, Pi usesspawnSync({ timeout }).Detection chain (in order)
symbolic-ref origin/HEAD— local, instant. Works when set.ls-remote --symref origin HEAD— network, 3s timeout. Works for any branch name.refs/remotes/origin/<detected>exists locally.main/master— no remote at all.Test plan
origin/HEADset: Branch diff / PR Diff should default toorigin/main(not localmain).origin/HEADset: behavior unchanged (step 1 catches it).main. No hang, no crash.develop):ls-remotedetects it correctly.For provenance purposes, this PR was AI assisted.