Fix STA-1292: prevent Windows worktree-create hang (disable interactive credential prompt + timeout create-path git) (#7301)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
parent
d7d7067ae1
commit
ac8d365296
|
|
@ -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<string, string> {
|
||||
const count = Number.parseInt(env.GIT_CONFIG_COUNT ?? '0', 10)
|
||||
const config: Record<string, string> = {}
|
||||
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')
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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']
|
||||
]
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -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 }
|
||||
])
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -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 } : {}
|
||||
|
|
|
|||
|
|
@ -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<string, Promise<void>>()
|
||||
const sshWorktreeCreateFetchCompletedAt = new Map<string, number>()
|
||||
const sshWorktreeCreateFetchQueueTail = new Map<string, Promise<void>>()
|
||||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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> | 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')
|
||||
|
|
|
|||
|
|
@ -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' }
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue