diff --git a/src/main/git/repo.ts b/src/main/git/repo.ts index c3c4d7420..a8536765d 100644 --- a/src/main/git/repo.ts +++ b/src/main/git/repo.ts @@ -152,8 +152,14 @@ function hasGitRef(path: string, ref: string): boolean { /** * Resolve the default base ref for new worktrees. * Prefer the remote primary branch over a potentially stale local branch. + * + * Why: returns `null` when no candidate ref is resolvable. Previously this + * fell through to a hardcoded `'origin/main'` even when that ref did not + * exist, which silently handed `git worktree add` a bad ref and produced + * an opaque git error. Callers now fail loudly with a useful message, or + * degrade gracefully for non-creation uses (e.g. hosted URL building). */ -export function getDefaultBaseRef(path: string): string { +export function getDefaultBaseRef(path: string): string | null { try { const ref = gitExecFileSync(['symbolic-ref', '--quiet', 'refs/remotes/origin/HEAD'], { cwd: path @@ -179,14 +185,14 @@ export function getDefaultBaseRef(path: string): string { return 'master' } - return 'origin/main' + return null } -export async function getBaseRefDefault(path: string): Promise { +export async function getBaseRefDefault(path: string): Promise { return getDefaultBaseRefAsync(path) } -async function getDefaultBaseRefAsync(path: string): Promise { +async function getDefaultBaseRefAsync(path: string): Promise { try { const { stdout } = await gitExecFileAsync( ['symbolic-ref', '--quiet', 'refs/remotes/origin/HEAD'], @@ -213,7 +219,7 @@ async function getDefaultBaseRefAsync(path: string): Promise { return 'master' } - return 'origin/main' + return null } export async function searchBaseRefs(path: string, query: string, limit = 25): Promise { @@ -318,7 +324,11 @@ export function getRemoteFileUrl( return null } - const defaultBranch = getDefaultBaseRef(repoPath).replace(/^origin\//, '') + const defaultBaseRef = getDefaultBaseRef(repoPath) + if (!defaultBaseRef) { + return null + } + const defaultBranch = defaultBaseRef.replace(/^origin\//, '') const browseUrl = info.browseFile(relativePath, { committish: defaultBranch }) if (!browseUrl) { return null diff --git a/src/main/ipc/repos.ts b/src/main/ipc/repos.ts index da965eb3c..2785bd5cd 100644 --- a/src/main/ipc/repos.ts +++ b/src/main/ipc/repos.ts @@ -49,32 +49,38 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v return store.getRepos() }) - ipcMain.handle('repos:add', async (_event, args: { path: string; kind?: 'git' | 'folder' }): Promise<{ repo: Repo } | { error: string }> => { - const repoKind = args.kind === 'folder' ? 'folder' : 'git' - if (repoKind === 'git' && !isGitRepo(args.path)) { - return { error: `Not a valid git repository: ${args.path}` } - } + ipcMain.handle( + 'repos:add', + async ( + _event, + args: { path: string; kind?: 'git' | 'folder' } + ): Promise<{ repo: Repo } | { error: string }> => { + const repoKind = args.kind === 'folder' ? 'folder' : 'git' + if (repoKind === 'git' && !isGitRepo(args.path)) { + return { error: `Not a valid git repository: ${args.path}` } + } - // Check if already added - const existing = store.getRepos().find((r) => r.path === args.path) - if (existing) { - return { repo: existing } - } + // Check if already added + const existing = store.getRepos().find((r) => r.path === args.path) + if (existing) { + return { repo: existing } + } - const repo: Repo = { - id: randomUUID(), - path: args.path, - displayName: getRepoName(args.path), - badgeColor: REPO_COLORS[store.getRepos().length % REPO_COLORS.length], - addedAt: Date.now(), - kind: repoKind - } + const repo: Repo = { + id: randomUUID(), + path: args.path, + displayName: getRepoName(args.path), + badgeColor: REPO_COLORS[store.getRepos().length % REPO_COLORS.length], + addedAt: Date.now(), + kind: repoKind + } - store.addRepo(repo) - await rebuildAuthorizedRootsCache(store) - notifyReposChanged(mainWindow) - return { repo } - }) + store.addRepo(repo) + await rebuildAuthorizedRootsCache(store) + notifyReposChanged(mainWindow) + return { repo } + } + ) ipcMain.handle( 'repos:addRemote', @@ -347,14 +353,17 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v ipcMain.handle('repos:getBaseRefDefault', async (_event, args: { repoId: string }) => { const repo = store.getRepo(args.repoId) if (!repo || isFolderRepo(repo)) { - return 'origin/main' + // Why: folder-mode repos have no git state to resolve a base ref from. + // Return null so the renderer can decline to use a fabricated default + // (e.g. avoid running a branch compare against a ref that doesn't exist). + return null } // Why: remote repos need the relay to resolve symbolic-ref on the // remote host where the git data lives. if (repo.connectionId) { const provider = getSshGitProvider(repo.connectionId) if (!provider) { - return 'origin/main' + return null } try { const result = await provider.exec( @@ -366,9 +375,11 @@ export function registerRepoHandlers(mainWindow: BrowserWindow, store: Store): v return ref.replace(/^refs\/remotes\//, '') } } catch { - // Fall through to default + // Fall through — no symbolic-ref on the remote. } - return 'origin/main' + // Why: don't fabricate 'origin/main'. Let the renderer surface "no + // default" and prompt the user to pick a base branch. + return null } return getBaseRefDefault(repo.path) }) diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 27bb922e5..7ff55ce1b 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -79,6 +79,11 @@ export async function createRemoteWorktree( const remotePath = `${repo.path}/../${sanitizedName}` // Determine base branch + // Why: previously fell back to a hardcoded 'origin/main' when + // symbolic-ref failed. That silently handed addWorktree a ref that may + // not exist on the remote (e.g. repos whose primary branch is master or + // develop), producing an opaque git error. Fail here with a clear + // message so the UI can surface it and prompt the user to pick a base. let baseBranch = args.baseBranch || repo.worktreeBaseRef if (!baseBranch) { try { @@ -88,9 +93,14 @@ export async function createRemoteWorktree( ) baseBranch = stdout.trim() } catch { - baseBranch = 'origin/main' + // Fall through — baseBranch stays unset. } } + if (!baseBranch) { + throw new Error( + 'Could not resolve a default base ref for this repo. Pick a base branch explicitly and try again.' + ) + } // Fetch latest const remote = baseBranch.includes('/') ? baseBranch.split('/')[0] : 'origin' @@ -226,8 +236,20 @@ export async function createLocalWorktree( ) } - // Determine base branch + // Determine base branch. + // + // Why: getDefaultBaseRef may return null when none of origin/HEAD, + // origin/main, origin/master, local main, or local master exist. In that + // case we must not fall back to a hardcoded 'origin/main' — passing a + // non-existent ref to `git worktree add` produces an opaque error. Fail + // here with a clear message so the UI can prompt the user to pick a base + // branch explicitly. const baseBranch = args.baseBranch || repo.worktreeBaseRef || getDefaultBaseRef(repo.path) + if (!baseBranch) { + throw new Error( + 'Could not resolve a default base ref for this repo. Pick a base branch explicitly and try again.' + ) + } const setupScript = getEffectiveHooks(repo)?.scripts.setup // Why: `ask` is a pre-create choice gate, not a post-create side effect. // Resolve it before mutating git state so missing UI input cannot strand diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 80432f019..f02e8c756 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -259,6 +259,31 @@ describe('registerWorktreeHandlers', () => { }) }) + it('throws a clear error when no default base ref can be resolved', async () => { + // Why: guard against regressing to a silent 'origin/main' fallback. When + // getDefaultBaseRef returns null (e.g. a fresh repo with no origin/HEAD, + // no origin/main, no origin/master, and no local main/master), we must + // fail loudly with a message that prompts the user to pick a base + // branch, not hand a non-existent ref to `git worktree add`. + getDefaultBaseRefMock.mockReturnValue(null) + store.getRepo.mockReturnValue({ + id: 'repo-1', + path: '/workspace/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0, + worktreeBaseRef: null + }) + + await expect( + handlers['worktrees:create'](null, { + repoId: 'repo-1', + name: 'improve-dashboard' + }) + ).rejects.toThrow(/Could not resolve a default base ref/) + expect(addWorktreeMock).not.toHaveBeenCalled() + }) + it('creates an issue-command runner for an existing repo/worktree pair', async () => { const result = await handlers['hooks:createIssueCommandRunner'](null, { repoId: 'repo-1', diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 39fa74659..d5a4ce73d 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -51,6 +51,21 @@ vi.mock('../ipc/filesystem-auth', () => ({ invalidateAuthorizedRootsCache: invalidateAuthorizedRootsCacheMock })) +// Why: the CLI create-worktree path calls getDefaultBaseRef to resolve a +// fallback base branch. Real resolution shells out to `git` against the +// test's fabricated repo path, which has no refs, so we stub it to a +// predictable 'origin/main'. The runtime no longer silently fabricates this +// default, so tests that want the legacy behavior must express it via the mock. +vi.mock('../git/repo', async (importOriginal) => { + const actual = (await importOriginal()) as Record + return { + ...actual, + getDefaultBaseRef: vi.fn().mockReturnValue('origin/main'), + getBranchConflictKind: vi.fn().mockResolvedValue(null), + getGitUsername: vi.fn().mockReturnValue('') + } +}) + afterEach(() => { vi.mocked(listWorktrees).mockResolvedValue(MOCK_GIT_WORKTREES) vi.mocked(addWorktree).mockReset() diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index adafb4a10..3ad0588f5 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -676,6 +676,15 @@ export class OrcaRuntimeService { const workspaceRoot = wslHome ? join(wslHome, 'orca', 'workspaces') : settings.workspaceDir worktreePath = ensurePathWithinWorkspace(worktreePath, workspaceRoot) const baseBranch = args.baseBranch || repo.worktreeBaseRef || getDefaultBaseRef(repo.path) + if (!baseBranch) { + // Why: getDefaultBaseRef returns null when no suitable ref exists. + // Don't fabricate 'origin/main' — passing it to addWorktree would + // produce an opaque git failure. Surface a clear error so the CLI + // caller can pick an explicit --base ref. + throw new Error( + 'Could not resolve a default base ref for this repo. Pass an explicit --base and try again.' + ) + } const remote = baseBranch.includes('/') ? baseBranch.split('/')[0] : 'origin' try { diff --git a/src/preload/api-types.d.ts b/src/preload/api-types.d.ts index cd2e43a5e..734a18e0a 100644 --- a/src/preload/api-types.d.ts +++ b/src/preload/api-types.d.ts @@ -294,7 +294,7 @@ export type PreloadApi = { }) => Promise<{ repo: Repo } | { error: string }> onCloneProgress: (callback: (data: { phase: string; percent: number }) => void) => () => void getGitUsername: (args: { repoId: string }) => Promise - getBaseRefDefault: (args: { repoId: string }) => Promise + getBaseRefDefault: (args: { repoId: string }) => Promise searchBaseRefs: (args: { repoId: string; query: string; limit?: number }) => Promise onChanged: (callback: () => void) => () => void } diff --git a/src/preload/index.d.ts b/src/preload/index.d.ts index b01d8cf05..5d7fa8049 100644 --- a/src/preload/index.d.ts +++ b/src/preload/index.d.ts @@ -37,7 +37,7 @@ type ReposApi = { cloneAbort: () => Promise onCloneProgress: (callback: (data: { phase: string; percent: number }) => void) => () => void getGitUsername: (args: { repoId: string }) => Promise - getBaseRefDefault: (args: { repoId: string }) => Promise + getBaseRefDefault: (args: { repoId: string }) => Promise searchBaseRefs: (args: { repoId: string; query: string; limit?: number }) => Promise onChanged: (callback: () => void) => () => void } diff --git a/src/preload/index.ts b/src/preload/index.ts index 52b204f56..e295ba8ef 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -210,7 +210,7 @@ const api = { getGitUsername: (args: { repoId: string }): Promise => ipcRenderer.invoke('repos:getGitUsername', args), - getBaseRefDefault: (args: { repoId: string }): Promise => + getBaseRefDefault: (args: { repoId: string }): Promise => ipcRenderer.invoke('repos:getBaseRefDefault', args), searchBaseRefs: (args: { repoId: string; query: string; limit?: number }): Promise => diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 4f93e11d4..a733feead 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -173,7 +173,10 @@ function SourceControlInner(): React.JSX.Element { const [scope, setScope] = useState('all') const [collapsedSections, setCollapsedSections] = useState>(new Set()) const [baseRefDialogOpen, setBaseRefDialogOpen] = useState(false) - const [defaultBaseRef, setDefaultBaseRef] = useState('origin/main') + // Why: start null rather than 'origin/main' so branch compare doesn't fire + // with a fabricated ref before the IPC resolves. effectiveBaseRef stays + // falsy until we have a real answer from the main process. + const [defaultBaseRef, setDefaultBaseRef] = useState(null) const [filterQuery, setFilterQuery] = useState('') const filterInputRef = useRef(null) @@ -238,8 +241,11 @@ function SourceControlInner(): React.JSX.Element { } }) .catch(() => { + // Why: leave defaultBaseRef null on failure instead of fabricating + // 'origin/main'. effectiveBaseRef stays falsy, so branch compare and + // PR fetch skip running against a ref that may not exist. if (!stale) { - setDefaultBaseRef('origin/main') + setDefaultBaseRef(null) } }) diff --git a/src/renderer/src/components/settings/BaseRefPicker.tsx b/src/renderer/src/components/settings/BaseRefPicker.tsx index ce2f0ea8c..a0246e8ff 100644 --- a/src/renderer/src/components/settings/BaseRefPicker.tsx +++ b/src/renderer/src/components/settings/BaseRefPicker.tsx @@ -16,7 +16,10 @@ export function BaseRefPicker({ onSelect, onUsePrimary }: BaseRefPickerProps): React.JSX.Element { - const [defaultBaseRef, setDefaultBaseRef] = useState('origin/main') + // Why: null until the IPC resolves (or when the repo has no default base ref + // available). We avoid seeding with 'origin/main' because that would display + // a fabricated default in repos that don't actually have origin/main. + const [defaultBaseRef, setDefaultBaseRef] = useState(null) const [baseRefQuery, setBaseRefQuery] = useState('') const [baseRefResults, setBaseRefResults] = useState([]) const [isSearchingBaseRefs, setIsSearchingBaseRefs] = useState(false) @@ -32,7 +35,7 @@ export function BaseRefPicker({ } } catch { if (!stale) { - setDefaultBaseRef('origin/main') + setDefaultBaseRef(null) } } } @@ -93,11 +96,15 @@ export function BaseRefPicker({
-
{effectiveBaseRef}
+
+ {effectiveBaseRef ?? 'No default base ref'} +

{currentBaseRef ? 'Pinned for this repo' - : `Following primary branch (${defaultBaseRef})`} + : defaultBaseRef + ? `Following primary branch (${defaultBaseRef})` + : 'Pick a base branch below'}

{onUsePrimary && (