From ac8d365296e45e4e75641881acc13655ffac48e5 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sat, 4 Jul 2026 13:37:06 -0700 Subject: [PATCH] Fix STA-1292: prevent Windows worktree-create hang (disable interactive credential prompt + timeout create-path git) (#7301) Co-authored-by: Orca --- src/main/git/runner.test.ts | 71 +++++++++++++++++++++ src/main/git/runner.ts | 48 ++++++++++++-- src/main/git/worktree.test.ts | 41 +++++++++--- src/main/git/worktree.ts | 13 +++- src/main/ipc/worktree-remote.ts | 8 ++- src/main/runtime/fetch-remote-cache.test.ts | 12 +++- src/main/runtime/orca-runtime.test.ts | 5 +- src/main/runtime/orca-runtime.ts | 14 +++- 8 files changed, 187 insertions(+), 25 deletions(-) diff --git a/src/main/git/runner.test.ts b/src/main/git/runner.test.ts index 24a031bbc..57b04831a 100644 --- a/src/main/git/runner.test.ts +++ b/src/main/git/runner.test.ts @@ -3,12 +3,30 @@ // and stderr extraction from execFile rejections (err.message is unreliable). import { afterEach, describe, expect, it, vi } from 'vitest' import { + appendGitConfigEnv, extractExecError, isTransientGhError, + nonInteractiveGitEnv, parseRetryAfterMs, + promptGuardGitEnv, redirectPortedHostnameToEnv } from './runner' +// Reads git config injected via the GIT_CONFIG_COUNT/KEY/VALUE env protocol +// back into a plain key→value map so tests can assert on it directly. +function readGitConfigEnv(env: NodeJS.ProcessEnv): Record { + const count = Number.parseInt(env.GIT_CONFIG_COUNT ?? '0', 10) + const config: Record = {} + for (let i = 0; i < count; i++) { + const key = env[`GIT_CONFIG_KEY_${i}`] + const value = env[`GIT_CONFIG_VALUE_${i}`] + if (key !== undefined && value !== undefined) { + config[key] = value + } + } + return config +} + describe('redirectPortedHostnameToEnv', () => { it('moves a ported --hostname into GITLAB_HOST and strips the flag', () => { const { args, options } = redirectPortedHostnameToEnv( @@ -147,3 +165,56 @@ describe('extractExecError', () => { }) }) }) + +describe('appendGitConfigEnv', () => { + it('injects entries starting at count 0 when none exist', () => { + const env = appendGitConfigEnv({ PATH: '/usr/bin' }, [['credential.interactive', 'false']]) + expect(env.GIT_CONFIG_COUNT).toBe('1') + expect(env.GIT_CONFIG_KEY_0).toBe('credential.interactive') + expect(env.GIT_CONFIG_VALUE_0).toBe('false') + expect(env.PATH).toBe('/usr/bin') + }) + + it('composes with an existing count instead of clobbering caller config', () => { + const env = appendGitConfigEnv( + { GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'core.quotePath', GIT_CONFIG_VALUE_0: 'false' }, + [['credential.guiPrompt', 'false']] + ) + expect(env.GIT_CONFIG_COUNT).toBe('2') + // Existing entry preserved. + expect(env.GIT_CONFIG_KEY_0).toBe('core.quotePath') + expect(env.GIT_CONFIG_VALUE_0).toBe('false') + // New entry appended at the next index. + expect(env.GIT_CONFIG_KEY_1).toBe('credential.guiPrompt') + expect(env.GIT_CONFIG_VALUE_1).toBe('false') + }) +}) + +describe('promptGuardGitEnv credential-interactivity disable (STA-1292)', () => { + it('disables the GCM GUI prompt without nuking the credential helper', () => { + const env = promptGuardGitEnv({ PATH: '/usr/bin' }) + // GCM: never show the GUI, but still serve cached credentials. + expect(env.GCM_INTERACTIVE).toBe('never') + const config = readGitConfigEnv(env) + expect(config['credential.interactive']).toBe('false') + expect(config['credential.guiPrompt']).toBe('false') + // Regression guard: we must NOT clear the helper — that would break + // cached-credential auth for private repos. + expect(config['credential.helper']).toBeUndefined() + // Existing prompt guards remain intact. + expect(env.GIT_TERMINAL_PROMPT).toBe('0') + }) +}) + +describe('nonInteractiveGitEnv credential-interactivity disable (STA-1292)', () => { + it('carries the credential-interactivity disable through from promptGuardGitEnv', () => { + const env = nonInteractiveGitEnv({ PATH: '/usr/bin' }) + expect(env.GCM_INTERACTIVE).toBe('never') + const config = readGitConfigEnv(env) + expect(config['credential.interactive']).toBe('false') + expect(config['credential.guiPrompt']).toBe('false') + expect(config['credential.helper']).toBeUndefined() + // Its own BatchMode SSH guard is still applied and unaffected. + expect(env.GIT_SSH_COMMAND).toBe('ssh -o BatchMode=yes') + }) +}) diff --git a/src/main/git/runner.ts b/src/main/git/runner.ts index 325d92220..ec0d1d7b8 100644 --- a/src/main/git/runner.ts +++ b/src/main/git/runner.ts @@ -526,13 +526,47 @@ export function gitOptionalLocksDisabledEnv( } } -function promptGuardGitEnv(env: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEnv { - return { - ...env, - GIT_TERMINAL_PROMPT: '0', - GIT_ASKPASS: env.GIT_ASKPASS ?? '', - SSH_ASKPASS: env.SSH_ASKPASS ?? '' - } +/** + * Append git config entries through the GIT_CONFIG_COUNT / GIT_CONFIG_KEY_n / + * GIT_CONFIG_VALUE_n env protocol (git >= 2.31), composing with any count + * already present in `env` so we never clobber config a caller injected the + * same way. + */ +export function appendGitConfigEnv( + env: NodeJS.ProcessEnv, + entries: readonly (readonly [key: string, value: string])[] +): NodeJS.ProcessEnv { + const parsed = Number.parseInt(env.GIT_CONFIG_COUNT ?? '', 10) + const base = Number.isInteger(parsed) && parsed > 0 ? parsed : 0 + const next = { ...env } + entries.forEach(([key, value], index) => { + next[`GIT_CONFIG_KEY_${base + index}`] = key + next[`GIT_CONFIG_VALUE_${base + index}`] = value + }) + next.GIT_CONFIG_COUNT = String(base + entries.length) + return next +} + +export function promptGuardGitEnv(env: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEnv { + return appendGitConfigEnv( + { + ...env, + GIT_TERMINAL_PROMPT: '0', + GIT_ASKPASS: env.GIT_ASKPASS ?? '', + SSH_ASKPASS: env.SSH_ASKPASS ?? '', + // Why: Git Credential Manager ignores GIT_TERMINAL_PROMPT / GIT_ASKPASS and + // pops a GUI on first auth — the Windows worktree-create hang (STA-1292). + // `never` suppresses the prompt while still serving cached credentials. + GCM_INTERACTIVE: 'never' + }, + // Why: disable only the *interactive* credential prompt, NOT the helper + // itself — an empty credential.helper would break cached-credential auth for + // private repos. Harmless on macOS/Linux (no GCM) and on the SSH path. + [ + ['credential.interactive', 'false'], + ['credential.guiPrompt', 'false'] + ] + ) } /** diff --git a/src/main/git/worktree.test.ts b/src/main/git/worktree.test.ts index 5bc272b98..43269a065 100644 --- a/src/main/git/worktree.test.ts +++ b/src/main/git/worktree.test.ts @@ -25,7 +25,8 @@ import { listWorktrees, moveWorktree, parseWorktreeList, - removeWorktree + removeWorktree, + WORKTREE_ADD_TIMEOUT_MS } from './worktree' describe('listWorktrees in-flight sharing', () => { @@ -482,7 +483,7 @@ describe('addWorktree', () => { '/repo-feature', 'refs/remotes/origin/main' ], - { cwd: '/repo' } + { cwd: '/repo', timeout: WORKTREE_ADD_TIMEOUT_MS } ], [ [ @@ -507,10 +508,30 @@ describe('addWorktree', () => { }) expect(gitExecFileAsyncMock.mock.calls).toEqual([ - [['worktree', 'add', '/repo-feature', 'feature/test'], { cwd: '/repo' }] + [ + ['worktree', 'add', '/repo-feature', 'feature/test'], + { cwd: '/repo', timeout: WORKTREE_ADD_TIMEOUT_MS } + ] ]) }) + it('bounds the worktree add call with a positive timeout (STA-1292 OneDrive stall guard)', async () => { + // Why: without a timeout, a OneDrive cloud-placeholder checkout can stall + // `git worktree add` for minutes. Assert the runner receives a non-zero + // timeout so a stuck create fails fast instead of hanging forever. + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree add + + await addWorktree('/repo', '/repo-feature', 'feature/test', 'feature/test', false, false, { + checkoutExistingBranch: true + }) + + const worktreeAddCall = gitExecFileAsyncMock.mock.calls.find( + ([argv]) => Array.isArray(argv) && argv[0] === 'worktree' && argv[1] === 'add' + ) + expect(worktreeAddCall?.[1]).toMatchObject({ timeout: WORKTREE_ADD_TIMEOUT_MS }) + expect(WORKTREE_ADD_TIMEOUT_MS).toBeGreaterThan(0) + }) + it('does not write branch base config when no base branch is provided', async () => { gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree add gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: 'true\n' }) // push.autoSetupRemote already set @@ -520,7 +541,7 @@ describe('addWorktree', () => { expect(gitExecFileAsyncMock.mock.calls).toEqual([ [ ['worktree', 'add', '--no-track', '-b', 'feature/no-base', '/repo-feature'], - { cwd: '/repo' } + { cwd: '/repo', timeout: WORKTREE_ADD_TIMEOUT_MS } ], [['config', '--get', 'push.autoSetupRemote'], { cwd: '/repo-feature' }] ]) @@ -602,7 +623,7 @@ describe('addWorktree', () => { '/repo-feature', 'refs/remotes/origin/main' ], - { cwd: '/repo' } + { cwd: '/repo', timeout: WORKTREE_ADD_TIMEOUT_MS } ], [ [ @@ -640,7 +661,7 @@ describe('addWorktree', () => { '/repo-feature', 'refs/remotes/origin/main' ], - { cwd: '/repo' } + { cwd: '/repo', timeout: WORKTREE_ADD_TIMEOUT_MS } ], [ [ @@ -679,7 +700,7 @@ describe('addWorktree', () => { '/repo-feature', 'refs/remotes/origin/main' ], - { cwd: '/repo' } + { cwd: '/repo', timeout: WORKTREE_ADD_TIMEOUT_MS } ], [ [ @@ -719,7 +740,7 @@ describe('addWorktree', () => { '/repo-feature', 'refs/remotes/origin/main' ], - { cwd: '/repo' } + { cwd: '/repo', timeout: WORKTREE_ADD_TIMEOUT_MS } ] ]) }) @@ -769,7 +790,7 @@ describe('addWorktree', () => { '/repo-feature', 'refs/remotes/origin/main' ], - { cwd: '/repo' } + { cwd: '/repo', timeout: WORKTREE_ADD_TIMEOUT_MS } ], [ [ @@ -1283,7 +1304,7 @@ describe('addWorktree', () => { '/repo-feature', 'refs/heads/main' ], - { cwd: '/repo' } + { cwd: '/repo', timeout: WORKTREE_ADD_TIMEOUT_MS } ]) }) diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index 5a2e875a4..eb30e13ad 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -68,6 +68,12 @@ type LocalBaseRefRefreshability = const SPARSE_CHECKOUT_DETECTION_CONCURRENCY = 8 +// Why: bound `git worktree add` so a OneDrive cloud-placeholder base path can't +// stall its checkout writes for minutes (STA-1292) — a stuck create then fails +// fast instead of spinning forever. Generous enough not to kill a legit large +// checkout (mirrors the SYNC runner's cloud-placeholder floor, #7225). +export const WORKTREE_ADD_TIMEOUT_MS = 180_000 + function gitExecOptions( cwd: string, options: GitWorktreeExecOptions = {} @@ -878,7 +884,12 @@ async function performAddWorktree( args.push(effectiveBase) } } - await gitExecFileAsync(args, gitExecOptions(repoPath, options)) + await gitExecFileAsync(args, { + ...gitExecOptions(repoPath, options), + // Why: bound the checkout so a OneDrive cloud-placeholder stall (STA-1292) + // fails fast rather than hanging worktree creation indefinitely. + timeout: WORKTREE_ADD_TIMEOUT_MS + }) if (options.checkoutExistingBranch) { return localBaseRefRefresh ? { localBaseRefRefresh } : {} diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index c1003a106..8193d6d74 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -118,6 +118,9 @@ import { const SSH_WORKTREE_CREATE_FETCH_FRESHNESS_MS = 30_000 const SSH_WORKTREE_CREATE_FETCH_CACHE_MAX = 512 +// Why: bound the create-path fallback `git fetch origin` so a Windows +// credential-manager GUI hang (STA-1292) can't wedge worktree creation forever. +const CREATE_BASE_FALLBACK_FETCH_TIMEOUT_MS = 60_000 const sshWorktreeCreateFetchInflight = new Map>() const sshWorktreeCreateFetchCompletedAt = new Map() const sshWorktreeCreateFetchQueueTail = new Map>() @@ -1974,7 +1977,10 @@ export async function createLocalWorktree( } } else { if (!(await hasLocalCommitObjectWithOptions(repo.path, baseBranch, localWorktreeGitOptions))) { - legacyFetchPromise = gitExecFileAsync(['fetch', 'origin'], localGitExecOptions) + legacyFetchPromise = gitExecFileAsync(['fetch', 'origin'], { + ...localGitExecOptions, + timeout: CREATE_BASE_FALLBACK_FETCH_TIMEOUT_MS + }) .then(() => undefined) .catch(() => undefined) emitCreateWorktreeProgress(mainWindow, 'fetching', args.creationId) diff --git a/src/main/runtime/fetch-remote-cache.test.ts b/src/main/runtime/fetch-remote-cache.test.ts index 514594eb6..fce2a5e6d 100644 --- a/src/main/runtime/fetch-remote-cache.test.ts +++ b/src/main/runtime/fetch-remote-cache.test.ts @@ -36,6 +36,12 @@ function exactBaseRefreshOptions(cwd: string): { return { cwd, timeout: 60_000, useConfiguredSshCommandForNetwork: true } } +// Why (STA-1292): the broad create-path fetch must carry a timeout so a Windows +// credential-manager GUI hang can't wedge worktree creation forever. +function fullRemoteFetchOptions(cwd: string): { cwd: string; timeout: number } { + return { cwd, timeout: 60_000 } +} + function mockFetchResults(results: (Promise | unknown)[]): void { let fetchIndex = 0 gitExecFileAsyncMock.mockImplementation((argv: string[]) => { @@ -299,7 +305,7 @@ describe('OrcaRuntimeService.fetchRemoteWithCache', () => { ['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'], exactBaseRefreshOptions('/repo/h') ], - [['fetch', 'origin'], { cwd: '/repo/h' }] + [['fetch', 'origin'], fullRemoteFetchOptions('/repo/h')] ]) }) @@ -341,7 +347,7 @@ describe('OrcaRuntimeService.fetchRemoteWithCache', () => { ([argv]) => Array.isArray(argv) && argv[0] === 'fetch' ) expect(fetchCalls).toEqual([ - [['fetch', 'origin'], { cwd: '/repo/i' }], + [['fetch', 'origin'], fullRemoteFetchOptions('/repo/i')], [ ['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'], exactBaseRefreshOptions('/repo/i') @@ -387,7 +393,7 @@ describe('OrcaRuntimeService.fetchRemoteWithCache', () => { ([argv]) => Array.isArray(argv) && argv[0] === 'fetch' ) expect(fetchCalls).toEqual([ - [['fetch', 'origin'], { cwd: '/repo/i-fail' }], + [['fetch', 'origin'], fullRemoteFetchOptions('/repo/i-fail')], [ ['fetch', '--no-tags', 'origin', '+refs/heads/main:refs/remotes/origin/main'], exactBaseRefreshOptions('/repo/i-fail') diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index a005e79a8..a8ef74053 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -4880,7 +4880,10 @@ describe('OrcaRuntimeService', () => { ['rev-parse', '--path-format=absolute', '--git-common-dir'], wslGitOptions ) - expect(asyncGitSpy).toHaveBeenCalledWith(['fetch', 'origin'], wslGitOptions) + expect(asyncGitSpy).toHaveBeenCalledWith(['fetch', 'origin'], { + ...wslGitOptions, + timeout: 60_000 + }) expect(syncGitSpy).toHaveBeenCalledWith( ['rev-list', '--left-right', '--count', 'HEAD...origin/main'], { cwd: TEST_WORKTREE_PATH, wslDistro: 'Ubuntu' } diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 216235198..8c53bbd99 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -14075,7 +14075,13 @@ export class OrcaRuntimeService { } const promise = this.enqueueRemoteFetch(key, () => - gitExecFileAsync(['fetch', remote], { cwd: repoPath, ...gitOptions }) + gitExecFileAsync(['fetch', remote], { + cwd: repoPath, + ...gitOptions, + // Why: cap the create-path base-ref fetch so a stuck first-auth on + // Windows (GCM prompt) fails fast instead of hanging creation (STA-1292). + timeout: REMOTE_FETCH_TIMEOUT_MS + }) .then((): RemoteFetchResult => { // Why (§3.3 Lifecycle): timestamp on success ONLY. Writing on rejection // would make the freshness cache lie about the last known remote state. @@ -14134,7 +14140,7 @@ export class OrcaRuntimeService { // Why: exact remote-base refresh is the network gate for worktree // creation, so honor repo SSH routing and bound custom wrappers. useConfiguredSshCommandForNetwork: true, - timeout: 60_000 + timeout: REMOTE_FETCH_TIMEOUT_MS } ) .then((): RemoteFetchResult => { @@ -22520,6 +22526,10 @@ const PTY_CONTROLLER_LIST_TIMEOUT_MS = 3000 // clicks and successive coordinator dispatches feel snappy, while still being // short enough that a genuinely-changed remote is observed on the next action. const FETCH_FRESHNESS_MS = 30_000 +// Why: bound create-path remote fetches so a Windows credential-manager GUI hang +// (STA-1292) can't wedge worktree creation forever; parity with the exact-base +// refresh sibling's timeout. +const REMOTE_FETCH_TIMEOUT_MS = 60_000 const REMOTE_FETCH_CACHE_MAX = 512 const DRIFT_PROBE_SUBJECT_LIMIT = 5