diff --git a/src/main/git/repo-default-base-timeout.test.ts b/src/main/git/repo-default-base-timeout.test.ts new file mode 100644 index 00000000000..b7c1518bc77 --- /dev/null +++ b/src/main/git/repo-default-base-timeout.test.ts @@ -0,0 +1,46 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { gitExecFileAsyncMock } = vi.hoisted(() => ({ + gitExecFileAsyncMock: vi.fn() +})) + +vi.mock('./runner', () => ({ + gitExecFileAsync: gitExecFileAsyncMock, + gitExecFileSync: vi.fn() +})) + +import { getBaseRefDefault } from './repo' + +describe('getBaseRefDefault async subprocess bounds', () => { + beforeEach(() => { + gitExecFileAsyncMock.mockReset() + }) + + it('bounds every local probe and degrades a timeout to no default', async () => { + gitExecFileAsyncMock.mockRejectedValue(new Error('git timed out.')) + + await expect(getBaseRefDefault('/repo')).resolves.toBeNull() + + expect(gitExecFileAsyncMock).toHaveBeenCalled() + for (const [, options] of gitExecFileAsyncMock.mock.calls) { + expect(options).toEqual({ cwd: '/repo', timeout: 15_000 }) + } + }) + + it('preserves the same timeout when routing probes through WSL', async () => { + gitExecFileAsyncMock.mockRejectedValue(new Error('git timed out.')) + + await expect( + getBaseRefDefault('\\\\wsl.localhost\\Ubuntu\\repo', { wslDistro: 'Ubuntu' }) + ).resolves.toBeNull() + + expect(gitExecFileAsyncMock).toHaveBeenCalled() + for (const [, options] of gitExecFileAsyncMock.mock.calls) { + expect(options).toEqual({ + cwd: '\\\\wsl.localhost\\Ubuntu\\repo', + timeout: 15_000, + wslDistro: 'Ubuntu' + }) + } + }) +}) diff --git a/src/main/git/repo.ts b/src/main/git/repo.ts index f649017cdeb..fa1c23ebf0d 100644 --- a/src/main/git/repo.ts +++ b/src/main/git/repo.ts @@ -15,6 +15,13 @@ type LocalGitExecOptions = { wslDistro?: string } +type LocalDefaultBaseRefGitOptions = { + cwd: string + wslDistro?: string +} + +const DEFAULT_BASE_REF_PROBE_TIMEOUT_MS = 15_000 + type GitRepoProbeResult = 'repo' | 'not-repo' | 'indeterminate' type GitMarkerScanResult = { status: 'valid'; rootPath: string } | { status: 'absent' | 'invalid' } @@ -651,13 +658,23 @@ export async function resolveDefaultBaseRefViaExec(exec: GitExec): Promise hasGitRefViaExec(exec, ref)) } +export function resolveDefaultBaseRefWithLocalGit( + options: LocalDefaultBaseRefGitOptions +): Promise { + return resolveDefaultBaseRefViaExec((argv) => + gitExecFileAsync(argv, { + ...options, + // Why: async avoids main-thread stalls, but dead local/WSL filesystems still need a bound. + timeout: DEFAULT_BASE_REF_PROBE_TIMEOUT_MS + }) + ) +} + async function getDefaultBaseRefAsync( path: string, options: LocalGitExecOptions = {} ): Promise { - return resolveDefaultBaseRefViaExec((argv) => - gitExecFileAsync(argv, gitExecOptions(path, options)) - ) + return resolveDefaultBaseRefWithLocalGit(gitExecOptions(path, options)) } /** diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 3f50a5ce056..e0fd8ee8fbc 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -27,7 +27,11 @@ import type { import { getPRForBranch } from '../github/client' import { listWorktrees, addWorktree, addSparseWorktree } from '../git/worktree' import type { AddWorktreeOptions, AddWorktreeResult } from '../git/worktree' -import { getBranchConflictKind, resolveDefaultBaseRefViaExec } from '../git/repo' +import { + getBranchConflictKind, + resolveDefaultBaseRefViaExec, + resolveDefaultBaseRefWithLocalGit +} from '../git/repo' import { resolveLocalGitUsername } from '../git/git-username' import { hasCommitObjectViaGitExec } from '../git/commit-object-ref' import { resolveWorktreeCreateBase } from '../worktree-create-base' @@ -1999,8 +2003,7 @@ export async function createLocalWorktree( const baseBranch = await resolveWorktreeCreateBase({ requestedBaseBranch: args.baseBranch, repoWorktreeBaseRef: repo.worktreeBaseRef, - resolveDefaultBaseRef: () => - resolveDefaultBaseRefViaExec((argv) => gitExecFileAsync(argv, localGitExecOptions)), + resolveDefaultBaseRef: () => resolveDefaultBaseRefWithLocalGit(localGitExecOptions), isBaseUsable: async (baseBranchCandidate) => { if (runtime) { const remoteTrackingBase = await runtime.resolveRemoteTrackingBase( diff --git a/src/main/ipc/worktrees-windows.test.ts b/src/main/ipc/worktrees-windows.test.ts index 7c6d0572da1..73d10495b1f 100644 --- a/src/main/ipc/worktrees-windows.test.ts +++ b/src/main/ipc/worktrees-windows.test.ts @@ -10,6 +10,7 @@ const { removeWorktreeMock, resolveLocalGitUsernameMock, getDefaultBaseRefMock, + resolveDefaultBaseRefWithLocalGitMock, resolveDefaultBaseRefViaExecMock, getBranchConflictKindMock, getPRForBranchMock, @@ -38,6 +39,7 @@ const { removeWorktreeMock: vi.fn(), resolveLocalGitUsernameMock: vi.fn(), getDefaultBaseRefMock: vi.fn(), + resolveDefaultBaseRefWithLocalGitMock: vi.fn(), resolveDefaultBaseRefViaExecMock: vi.fn(), getBranchConflictKindMock: vi.fn(), getPRForBranchMock: vi.fn(), @@ -84,6 +86,7 @@ vi.mock('../git/runner', () => ({ vi.mock('../git/repo', () => ({ getDefaultBaseRef: getDefaultBaseRefMock, + resolveDefaultBaseRefWithLocalGit: resolveDefaultBaseRefWithLocalGitMock, resolveDefaultBaseRefViaExec: resolveDefaultBaseRefViaExecMock, getBranchConflictKind: getBranchConflictKindMock })) @@ -164,6 +167,7 @@ describe('registerWorktreeHandlers – Windows path handling', () => { removeWorktreeMock.mockReset() resolveLocalGitUsernameMock.mockReset() getDefaultBaseRefMock.mockReset() + resolveDefaultBaseRefWithLocalGitMock.mockReset() resolveDefaultBaseRefViaExecMock.mockReset() getBranchConflictKindMock.mockReset() getPRForBranchMock.mockReset() @@ -230,6 +234,7 @@ describe('registerWorktreeHandlers – Windows path handling', () => { store.setWorktreeMeta.mockReturnValue({}) resolveLocalGitUsernameMock.mockResolvedValue('') getDefaultBaseRefMock.mockReturnValue('origin/main') + resolveDefaultBaseRefWithLocalGitMock.mockResolvedValue('origin/main') resolveDefaultBaseRefViaExecMock.mockResolvedValue('origin/main') getBranchConflictKindMock.mockResolvedValue(null) getPRForBranchMock.mockResolvedValue(null) diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index ade839b50fb..db5bf3b1a4a 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -28,7 +28,8 @@ const { removeWorktreeMock, forceDeleteLocalBranchMock, resolveLocalGitUsernameMock, - getDefaultBaseRefMock, + getBaseRefDefaultMock, + resolveDefaultBaseRefWithLocalGitMock, resolveDefaultBaseRefViaExecMock, getDefaultRemoteMock, getBranchConflictKindMock, @@ -77,7 +78,8 @@ const { removeWorktreeMock: vi.fn(), forceDeleteLocalBranchMock: vi.fn(), resolveLocalGitUsernameMock: vi.fn(), - getDefaultBaseRefMock: vi.fn(), + getBaseRefDefaultMock: vi.fn(), + resolveDefaultBaseRefWithLocalGitMock: vi.fn(), resolveDefaultBaseRefViaExecMock: vi.fn(), getDefaultRemoteMock: vi.fn(), getBranchConflictKindMock: vi.fn(), @@ -130,7 +132,8 @@ vi.mock('../git/runner', () => ({ })) vi.mock('../git/repo', () => ({ - getDefaultBaseRef: getDefaultBaseRefMock, + getBaseRefDefault: getBaseRefDefaultMock, + resolveDefaultBaseRefWithLocalGit: resolveDefaultBaseRefWithLocalGitMock, resolveDefaultBaseRefViaExec: resolveDefaultBaseRefViaExecMock, getDefaultRemote: getDefaultRemoteMock, getBranchConflictKind: getBranchConflictKindMock @@ -300,7 +303,8 @@ describe('registerWorktreeHandlers', () => { removeWorktreeMock, forceDeleteLocalBranchMock, resolveLocalGitUsernameMock, - getDefaultBaseRefMock, + getBaseRefDefaultMock, + resolveDefaultBaseRefWithLocalGitMock, resolveDefaultBaseRefViaExecMock, getDefaultRemoteMock, getBranchConflictKindMock, @@ -401,7 +405,8 @@ describe('registerWorktreeHandlers', () => { ]) store.getAllWorktreeLineage.mockReturnValue({}) resolveLocalGitUsernameMock.mockResolvedValue('') - getDefaultBaseRefMock.mockReturnValue('origin/main') + getBaseRefDefaultMock.mockResolvedValue('origin/main') + resolveDefaultBaseRefWithLocalGitMock.mockResolvedValue('origin/main') resolveDefaultBaseRefViaExecMock.mockResolvedValue('origin/main') getDefaultRemoteMock.mockResolvedValue('origin') getBranchConflictKindMock.mockResolvedValue(null) @@ -525,6 +530,7 @@ describe('registerWorktreeHandlers', () => { await handlers['worktrees:prefetchCreateBase'](null, { repoId: 'repo-1' }) + expect(getBaseRefDefaultMock).toHaveBeenCalledWith('/workspace/repo') expect(runtimeStub.resolveRemoteTrackingBase).toHaveBeenCalledWith( '/workspace/repo', 'origin/master' @@ -2050,12 +2056,6 @@ describe('registerWorktreeHandlers', () => { it('routes local worktree creation through the selected WSL project runtime', async () => { mockSelectedWslProjectRuntime() - resolveDefaultBaseRefViaExecMock.mockImplementation( - async (exec: (args: string[]) => Promise<{ stdout: string }>) => { - await exec(['symbolic-ref', '--quiet', 'refs/remotes/origin/HEAD']) - return 'origin/main' - } - ) listWorktreesMock.mockResolvedValue([ { path: '/workspace/repo', @@ -2087,10 +2087,10 @@ describe('registerWorktreeHandlers', () => { false, { wslDistro: 'Ubuntu' } ) - expect(gitExecFileAsyncMock).toHaveBeenCalledWith( - ['symbolic-ref', '--quiet', 'refs/remotes/origin/HEAD'], - { cwd: '/workspace/repo', wslDistro: 'Ubuntu' } - ) + expect(resolveDefaultBaseRefWithLocalGitMock).toHaveBeenCalledWith({ + cwd: '/workspace/repo', + wslDistro: 'Ubuntu' + }) expect(getBranchConflictKindMock).toHaveBeenCalledWith( '/workspace/repo', 'improve-dashboard', @@ -4980,7 +4980,7 @@ describe('registerWorktreeHandlers', () => { // 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`. - resolveDefaultBaseRefViaExecMock.mockResolvedValue(null) + resolveDefaultBaseRefWithLocalGitMock.mockResolvedValue(null) store.getRepo.mockReturnValue({ id: 'repo-1', path: '/workspace/repo', diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 1f0fb590010..b8d9eee175f 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -45,7 +45,7 @@ import { runHook, shouldRunSetupForCreate } from '../hooks' -import { getBranchConflictKind, getDefaultBaseRef } from '../git/repo' +import { getBaseRefDefault, getBranchConflictKind } from '../git/repo' import type { OrchestrationDb } from './orchestration/db' import type { MessagePriority, MessageRow, MessageType } from './orchestration/types' import { @@ -521,16 +521,23 @@ vi.mock('../github/issues', async (importOriginal) => { } }) -// 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. +// Why: CLI worktree creation resolves a default against fabricated repo paths +// in these tests, so keep the async resolver deterministic. vi.mock('../git/repo', async (importOriginal) => { const actual = (await importOriginal()) as Record + const actualGetBaseRefDefault = actual.getBaseRefDefault as ( + path: string, + options?: { wslDistro?: string } + ) => Promise return { ...actual, - getDefaultBaseRef: vi.fn().mockReturnValue('origin/main'), + // Why: fabricated local test repos need a deterministic default, while + // WSL coverage must still exercise the real async Git-options path. + getBaseRefDefault: vi + .fn() + .mockImplementation((path: string, options?: { wslDistro?: string }) => + options?.wslDistro ? actualGetBaseRefDefault(path, options) : Promise.resolve('origin/main') + ), getBranchConflictKind: vi.fn().mockResolvedValue(null) } }) @@ -2857,7 +2864,7 @@ describe('OrcaRuntimeService', () => { suggestLocalBaseRefUpdate: true } ) - expect(getDefaultBaseRef).toHaveBeenCalled() + expect(getBaseRefDefault).toHaveBeenCalled() } finally { getReposSpy.mockRestore() gitSpy.mockRestore() @@ -5363,7 +5370,7 @@ describe('OrcaRuntimeService', () => { it('treats SSH worktree drift as unknown without local git probes', async () => { vi.mocked(listWorktrees).mockClear() - vi.mocked(getDefaultBaseRef).mockClear() + vi.mocked(getBaseRefDefault).mockClear() const remoteStore = { ...store, getRepos: () => [ @@ -5399,7 +5406,7 @@ describe('OrcaRuntimeService', () => { } expect(gitProvider.listWorktrees).toHaveBeenCalledWith('/remote/repo') - expect(getDefaultBaseRef).not.toHaveBeenCalled() + expect(getBaseRefDefault).not.toHaveBeenCalled() expect(listWorktrees).not.toHaveBeenCalled() }) @@ -5465,7 +5472,7 @@ describe('OrcaRuntimeService', () => { }) expect(asyncGitSpy).toHaveBeenCalledWith( ['symbolic-ref', '--quiet', 'refs/remotes/origin/HEAD'], - wslGitOptions + { ...wslGitOptions, timeout: 15_000 } ) expect(asyncGitSpy).toHaveBeenCalledWith(['remote'], wslGitOptions) expect(asyncGitSpy).toHaveBeenCalledWith( @@ -26473,6 +26480,7 @@ describe('OrcaRuntimeService', () => { }) expect(gitSpy).toHaveBeenCalledWith(['symbolic-ref', '--quiet', 'refs/remotes/origin/HEAD'], { cwd: TEST_REPO_PATH, + timeout: 15_000, wslDistro: 'Ubuntu' }) expect(gitSpy).toHaveBeenCalledWith( @@ -27140,6 +27148,7 @@ describe('OrcaRuntimeService', () => { }) expect(gitSpy).toHaveBeenCalledWith(['symbolic-ref', '--quiet', 'refs/remotes/origin/HEAD'], { cwd: TEST_REPO_PATH, + timeout: 15_000, wslDistro: 'Ubuntu' }) expect(getBranchConflictKind).toHaveBeenCalledWith( diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index e183aff463e..d14528cedfa 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -600,7 +600,6 @@ import type { } from '../../shared/github-project-types' import { getBaseRefDefault, - getDefaultBaseRef, getDefaultRemote, getBranchConflictKind, isGitRepo, @@ -611,6 +610,7 @@ import { parseAndFilterSearchRefDetails, parseRemoteCount, resolveDefaultBaseRefViaExec, + resolveDefaultBaseRefWithLocalGit, buildSearchBaseRefsArgv, isForEachRefExcludeUnsupportedError, mergeBaseRefSearchResultGroups, @@ -14264,8 +14264,8 @@ export class OrcaRuntimeService { repoWorktreeBaseRef: repo.worktreeBaseRef, resolveDefaultBaseRef: () => hasLocalWorktreeGitOptions - ? resolveDefaultBaseRefViaExec((argv) => gitExecFileAsync(argv, localGitExecOptions)) - : Promise.resolve(getDefaultBaseRef(repo.path)), + ? resolveDefaultBaseRefWithLocalGit(localGitExecOptions) + : getBaseRefDefault(repo.path), isBaseUsable: async (baseBranchCandidate) => { const remoteTrackingBase = await this.resolveRemoteTrackingBase( repo.path, @@ -14296,10 +14296,8 @@ export class OrcaRuntimeService { } }) 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. + // Why: a null default means no suitable ref exists; fail clearly instead + // of handing Git a fabricated origin/main ref. throw new Error( 'Could not resolve a default base ref for this repo. Pass an explicit --base and try again.' ) diff --git a/src/main/worktree-create-base-prefetch.ts b/src/main/worktree-create-base-prefetch.ts index 42add57be66..8653c6866eb 100644 --- a/src/main/worktree-create-base-prefetch.ts +++ b/src/main/worktree-create-base-prefetch.ts @@ -2,7 +2,7 @@ import { isFolderRepo } from '../shared/repo-kind' import type { Repo } from '../shared/types' import { hasLocalCommitObject, isFullGitObjectId } from './git/commit-object-ref' import { hasWorktreeBaseCommitRef } from './git/worktree-base-ref-probe' -import { getDefaultBaseRef } from './git/repo' +import { getBaseRefDefault } from './git/repo' import { getSshGitProvider } from './providers/ssh-git-dispatch' import { prefetchRemoteWorktreeCreateBase } from './ipc/worktree-remote' import { resolveWorktreeCreateBase } from './worktree-create-base' @@ -48,7 +48,7 @@ async function prefetchLocalWorktreeCreateBase( const resolvedBaseBranch = await resolveWorktreeCreateBase({ requestedBaseBranch: baseBranch, repoWorktreeBaseRef: repo.worktreeBaseRef, - resolveDefaultBaseRef: async () => getDefaultBaseRef(repo.path), + resolveDefaultBaseRef: () => getBaseRefDefault(repo.path), isBaseUsable: async (baseBranchCandidate) => { const remoteTrackingBase = await runtime.resolveRemoteTrackingBase( repo.path,