Skip to content

Commit cbfa0c7

Browse files
committed
fix(review): detect remote default branch asynchronously at startup
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.
1 parent d102c5f commit cbfa0c7

4 files changed

Lines changed: 67 additions & 6 deletions

File tree

apps/pi-extension/server/serverReview.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
type DiffType,
2727
type GitCommandResult,
2828
type GitContext,
29+
detectRemoteDefaultBranch,
2930
getFileContentsForDiff as getFileContentsForDiffCore,
3031
getGitContext as getGitContextCore,
3132
gitAddFile as gitAddFileCore,
@@ -106,11 +107,12 @@ export interface ReviewServerResult {
106107
export const reviewRuntime: ReviewGitRuntime = {
107108
async runGit(
108109
args: string[],
109-
options?: { cwd?: string },
110+
options?: { cwd?: string; timeoutMs?: number },
110111
): Promise<GitCommandResult> {
111112
const result = spawnSync("git", args, {
112113
cwd: options?.cwd,
113114
encoding: "utf-8",
115+
...(options?.timeoutMs ? { timeout: options.timeoutMs } : {}),
114116
});
115117
return {
116118
stdout: result.stdout ?? "",
@@ -207,6 +209,14 @@ export async function startReviewServer(options: {
207209
// the reviewer is currently looking at. Honors an explicit initialBase from
208210
// the caller — e.g. programmatic Pi callers can request a non-detected base.
209211
let currentBase = options.initialBase || options.gitContext?.defaultBranch || "main";
212+
let baseEverSwitched = false;
213+
214+
// Fire-and-forget: query the remote for its actual default branch.
215+
if (options.gitContext && !options.initialBase && !isPRMode) {
216+
detectRemoteDefaultBranch(reviewRuntime, options.gitContext.cwd).then((remote) => {
217+
if (remote && !baseEverSwitched) currentBase = remote;
218+
});
219+
}
210220

211221
// Agent jobs — background process manager (late-binds serverUrl via getter)
212222
let serverUrl = "";
@@ -547,6 +557,7 @@ export async function startReviewServer(options: {
547557
currentGitRef = result.label;
548558
currentDiffType = newType;
549559
currentBase = base;
560+
baseEverSwitched = true;
550561
currentError = result.error;
551562

552563
// Recompute gitContext for the effective cwd so the client's

packages/server/git.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,20 +36,27 @@ export type {
3636

3737
async function runGit(
3838
args: string[],
39-
options?: { cwd?: string },
39+
options?: { cwd?: string; timeoutMs?: number },
4040
): Promise<GitCommandResult> {
4141
const proc = Bun.spawn(["git", ...args], {
4242
cwd: options?.cwd,
4343
stdout: "pipe",
4444
stderr: "pipe",
4545
});
4646

47+
let timer: ReturnType<typeof setTimeout> | undefined;
48+
if (options?.timeoutMs) {
49+
timer = setTimeout(() => proc.kill(), options.timeoutMs);
50+
}
51+
4752
const [stdout, stderr, exitCode] = await Promise.all([
4853
new Response(proc.stdout).text(),
4954
new Response(proc.stderr).text(),
5055
proc.exited,
5156
]);
5257

58+
if (timer) clearTimeout(timer);
59+
5360
return { stdout, stderr, exitCode };
5461
}
5562

packages/server/review.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,9 @@
1111

1212
import { isRemoteSession, getServerHostname, getServerPort } from "./remote";
1313
import type { Origin } from "@plannotator/shared/agents";
14-
import { type DiffType, type GitContext, runVcsDiff, getVcsFileContentsForDiff, canStageFiles, stageFile, unstageFile, resolveVcsCwd, validateFilePath, getVcsContext } from "./vcs";
15-
import { parseWorktreeDiffType } from "@plannotator/shared/review-core";
14+
import { type DiffType, type GitContext, runVcsDiff, getVcsFileContentsForDiff, canStageFiles, stageFile, unstageFile, resolveVcsCwd, validateFilePath, getVcsContext, gitRuntime } from "./vcs";
15+
import { parseWorktreeDiffType, detectRemoteDefaultBranch, resolveBaseBranch } from "@plannotator/shared/review-core";
1616
import type { AgentJobInfo } from "@plannotator/shared/agent-jobs";
17-
import { resolveBaseBranch } from "@plannotator/shared/review-core";
1817
import { getRepoInfo } from "./repo";
1918
import { handleImage, handleUpload, handleAgents, handleServerReady, handleDraftSave, handleDraftLoad, handleDraftDelete, handleFavicon, type OpencodeClient } from "./shared-handlers";
2019
import { contentHash, deleteDraft } from "./draft";
@@ -144,6 +143,17 @@ export async function startReviewServer(
144143
// the reviewer is currently looking at. Honors an explicit initialBase from
145144
// the caller — e.g. programmatic Pi callers can request a non-detected base.
146145
let currentBase = options.initialBase || gitContext?.defaultBranch || "main";
146+
let baseEverSwitched = false;
147+
148+
// Fire-and-forget: query the remote for its actual default branch. If it
149+
// arrives before the user interacts, quietly upgrade currentBase from the
150+
// local fallback (e.g. "main") to the upstream ref (e.g. "origin/main").
151+
// Non-blocking — the server is already listening by the time this resolves.
152+
if (gitContext && !options.initialBase && !isPRMode) {
153+
detectRemoteDefaultBranch(gitRuntime, gitContext.cwd).then((remote) => {
154+
if (remote && !baseEverSwitched) currentBase = remote;
155+
});
156+
}
147157

148158
// Agent jobs — background process manager (late-binds serverUrl via getter)
149159
let serverUrl = "";
@@ -496,6 +506,7 @@ export async function startReviewServer(
496506
currentGitRef = result.label;
497507
currentDiffType = newDiffType;
498508
currentBase = base;
509+
baseEverSwitched = true;
499510
currentError = result.error;
500511

501512
// Recompute gitContext for the effective cwd so the client's

packages/shared/review-core.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export interface GitCommandResult {
5959
export interface ReviewGitRuntime {
6060
runGit: (
6161
args: string[],
62-
options?: { cwd?: string },
62+
options?: { cwd?: string; timeoutMs?: number },
6363
) => Promise<GitCommandResult>;
6464
readTextFile: (path: string) => Promise<string | null>;
6565
}
@@ -111,6 +111,38 @@ export async function getDefaultBranch(
111111
return "master";
112112
}
113113

114+
/**
115+
* Query the remote for its default branch via `ls-remote --symref`. Returns
116+
* `origin/<name>` if the remote answers and the tracking ref exists locally,
117+
* otherwise `null`. Designed to run in the background at server startup — the
118+
* caller fires it with `.then()` and uses the result if/when it arrives.
119+
*
120+
* Timeout-guarded: if the network is slow or absent, the promise resolves
121+
* (with `null`) once the timeout fires. Never throws.
122+
*/
123+
export async function detectRemoteDefaultBranch(
124+
runtime: ReviewGitRuntime,
125+
cwd?: string,
126+
): Promise<string | null> {
127+
try {
128+
const lsRemote = await runtime.runGit(
129+
["ls-remote", "--symref", "origin", "HEAD"],
130+
{ cwd, timeoutMs: 5000 },
131+
);
132+
if (lsRemote.exitCode !== 0) return null;
133+
const match = lsRemote.stdout.match(/^ref:\s+refs\/heads\/(\S+)\s+HEAD/m);
134+
if (!match) return null;
135+
const remoteBranch = `origin/${match[1]}`;
136+
const refExists = await runtime.runGit(
137+
["show-ref", "--verify", "--quiet", `refs/remotes/${remoteBranch}`],
138+
{ cwd },
139+
);
140+
return refExists.exitCode === 0 ? remoteBranch : null;
141+
} catch {
142+
return null;
143+
}
144+
}
145+
114146
export async function listBranches(
115147
runtime: ReviewGitRuntime,
116148
cwd?: string,

0 commit comments

Comments
 (0)