From 5204a56873526c4dcddb1eb3e1b9aba2b8aabeca Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sat, 13 Jun 2026 00:15:32 -0700 Subject: [PATCH] Cap hung read-path git commands with a default timeout (#5308) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A git subprocess that never exits — waiting on a credential/stdin prompt, or stuck on a wedged NFS/network filesystem under its cwd — left gitExecFileAsync pending forever. On the headless `serve` runtime these read-path calls (worktree list, status, rev-parse) pile up with no timeout, the libuv subprocess slots stay held, and the runtime stops answering every client. Opening a large non-git folder (e.g. a home dir) as a worktree and starting an agent in it reproduced this. Give gitExecFileAsync a 60s default timeout when the caller sets none, while exempting legitimately long-running network subcommands (clone/fetch/pull/push/remote/submodule/lfs) so large-repo transfers aren't killed mid-flight. Explicit caller timeouts (including an explicit 0 opt-out) are honored verbatim. This also breaks the reconnect boot-loop: a re-hung worktree's git calls now self-resolve at the ceiling instead of looping forever. Co-authored-by: Orca --- src/main/git/runner-command-exec.test.ts | 47 +++++++++++++++- src/main/git/runner-default-timeout.test.ts | 55 ++++++++++++++++++ src/main/git/runner.ts | 62 ++++++++++++++++++++- 3 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 src/main/git/runner-default-timeout.test.ts diff --git a/src/main/git/runner-command-exec.test.ts b/src/main/git/runner-command-exec.test.ts index 30278404e90..05d011b7e0b 100644 --- a/src/main/git/runner-command-exec.test.ts +++ b/src/main/git/runner-command-exec.test.ts @@ -13,7 +13,7 @@ vi.mock('node:child_process', () => ({ spawn: spawnMock })) -import { commandExecFileAsync, gitExecFileAsync } from './runner' +import { commandExecFileAsync, gitExecFileAsync, GIT_DEFAULT_TIMEOUT_MS } from './runner' type MockChildProcess = EventEmitter & { stdout: EventEmitter @@ -210,4 +210,49 @@ describe('runner execFile timeout handling', () => { await rejection expect(child.kill).toHaveBeenCalled() }) + + // Issue #5308: a hung read-path git command (non-git folder on a wedged NFS + // mount, or git waiting on a credential prompt) with NO explicit timeout must + // still self-resolve at the default ceiling instead of hanging forever and + // wedging the serve runtime for all clients. + it('rejects a hung read-path git command at the default timeout when none is set', async () => { + const child = createMockChildProcess(1234) + execFileMock.mockReturnValue(child) + + const promise = gitExecFileAsync(['worktree', 'list', '--porcelain', '-z'], { + cwd: '/home5/Brian' + }) + const rejection = expect(promise).rejects.toThrow('git timed out.') + await vi.advanceTimersByTimeAsync(GIT_DEFAULT_TIMEOUT_MS) + + await rejection + expect(child.kill).toHaveBeenCalled() + }) + + // Long-running network subcommands opt out of the default ceiling so large + // clones/fetches aren't killed mid-transfer. + it('does not apply the default timeout to network subcommands', async () => { + const child = createMockChildProcess(1234) + let execCallback: ((err: Error | null, stdout: string, stderr: string) => void) | undefined + execFileMock.mockImplementation((_cmd, _args, _opts, cb) => { + execCallback = cb + return child + }) + + let settled = false + const promise = gitExecFileAsync(['fetch', '--prune'], { cwd: '/repo' }) + .catch(() => undefined) + .finally(() => { + settled = true + }) + // Advance well past the read-path ceiling; fetch must still be running. + await vi.advanceTimersByTimeAsync(GIT_DEFAULT_TIMEOUT_MS * 5) + expect(settled).toBe(false) + expect(child.kill).not.toHaveBeenCalled() + + // Let the fetch "finish" so the test doesn't leak a pending promise. + execCallback?.(null, '', '') + await promise + expect(settled).toBe(true) + }) }) diff --git a/src/main/git/runner-default-timeout.test.ts b/src/main/git/runner-default-timeout.test.ts new file mode 100644 index 00000000000..3184d3d73e5 --- /dev/null +++ b/src/main/git/runner-default-timeout.test.ts @@ -0,0 +1,55 @@ +// Reproduction harness for issue #5308: a hung git subprocess on the +// worktree-resolution / status read path (e.g. a non-git folder on a wedged +// NFS mount, or git waiting on a credential/stdin prompt) must not hang +// forever. Without a default timeout the promise never settles, the libuv +// subprocess slot is held, and the serve runtime stops answering every client. +import { describe, expect, it } from 'vitest' +import { resolveGitDefaultTimeoutMs, GIT_DEFAULT_TIMEOUT_MS } from './runner' + +describe('resolveGitDefaultTimeoutMs', () => { + it('honors an explicit caller timeout verbatim', () => { + expect(resolveGitDefaultTimeoutMs(['status'], 1234)).toBe(1234) + // Explicit 0 means "caller opted out" — never override it. + expect(resolveGitDefaultTimeoutMs(['status'], 0)).toBe(0) + }) + + it('applies the default timeout to read-path commands with no explicit value', () => { + // These are the commands on the hot worktree-resolution / status path that + // wedged the runtime in #5308. They should never run unbounded. + for (const args of [ + ['worktree', 'list', '--porcelain', '-z'], + ['status', '--porcelain'], + ['rev-parse', '--show-toplevel'], + ['symbolic-ref', '--short', 'HEAD'], + ['config', '--get', 'user.name'] + ]) { + expect(resolveGitDefaultTimeoutMs(args, undefined)).toBe(GIT_DEFAULT_TIMEOUT_MS) + } + }) + + it('exempts long-running network subcommands from the default timeout', () => { + // Clone/fetch/pull/push are legitimately slow and must not be capped by the + // generic read-path default — capping them would break large-repo workflows. + for (const args of [ + ['clone', 'https://example.com/repo.git'], + ['fetch', '--prune'], + ['pull', '--rebase', 'origin', 'main'], + ['push', 'origin', 'HEAD'], + ['remote', 'update'], + ['submodule', 'update', '--init'], + ['lfs', 'pull'] + ]) { + expect(resolveGitDefaultTimeoutMs(args, undefined)).toBeUndefined() + } + }) + + it('looks past global flags to find the subcommand', () => { + // `git -c key=val -C /path fetch` is still a fetch. + expect( + resolveGitDefaultTimeoutMs(['-c', 'gc.auto=0', '-C', '/repo', 'fetch'], undefined) + ).toBeUndefined() + expect( + resolveGitDefaultTimeoutMs(['-c', 'core.pager=cat', 'status', '--porcelain'], undefined) + ).toBe(GIT_DEFAULT_TIMEOUT_MS) + }) +}) diff --git a/src/main/git/runner.ts b/src/main/git/runner.ts index 91f92c8ac94..0f88fc4b50d 100644 --- a/src/main/git/runner.ts +++ b/src/main/git/runner.ts @@ -480,6 +480,66 @@ export function gitOptionalLocksDisabledEnv( } } +// Why: a git subprocess that never exits (waiting on a credential/stdin +// prompt, or a wedged NFS/network filesystem under the cwd) otherwise leaves +// gitExecFileAsync pending forever. On the headless `serve` runtime these +// read-path calls pile up and the whole runtime stops answering clients +// (issue #5308). A generous ceiling kills a truly-hung process without +// tripping on normal local git work. +export const GIT_DEFAULT_TIMEOUT_MS = 60_000 + +// Subcommands that legitimately run longer than the read-path ceiling — they +// move data over the network or recurse into submodules/LFS. Capping these +// would break large-repo clones/fetches, so they opt out of the default. +const GIT_UNBOUNDED_SUBCOMMANDS = new Set([ + 'clone', + 'fetch', + 'pull', + 'push', + 'remote', + 'submodule', + 'lfs' +]) + +// Global flags that precede the subcommand: skip them (and their values, for +// the ones that take an argument) to find the real subcommand. +const GIT_GLOBAL_FLAGS_WITH_VALUE = new Set(['-c', '-C', '--git-dir', '--work-tree', '--namespace']) + +function findGitSubcommand(args: string[]): string | undefined { + for (let i = 0; i < args.length; i++) { + const arg = args[i] + if (GIT_GLOBAL_FLAGS_WITH_VALUE.has(arg)) { + i++ // skip the flag's value + continue + } + if (arg.startsWith('-')) { + continue // valueless global flag (e.g. --no-pager) + } + return arg + } + return undefined +} + +/** + * Resolve the effective timeout for a git invocation. An explicit caller + * timeout (including an explicit 0 opt-out) always wins; otherwise read-path + * commands get GIT_DEFAULT_TIMEOUT_MS and long-running network subcommands + * stay unbounded. + */ +export function resolveGitDefaultTimeoutMs( + args: string[], + explicitTimeout: number | undefined +): number | undefined { + if (explicitTimeout !== undefined) { + return explicitTimeout + } + const subcommand = findGitSubcommand(args) + if (subcommand && GIT_UNBOUNDED_SUBCOMMANDS.has(subcommand)) { + return undefined + } + return GIT_DEFAULT_TIMEOUT_MS +} + /** * Async git command execution. Drop-in replacement for * `execFileAsync('git', args, { cwd, encoding, ... })`. @@ -500,7 +560,7 @@ export async function gitExecFileAsync( cwd: resolved.cwd, encoding: (options.encoding ?? 'utf-8') as BufferEncoding, maxBuffer: options.maxBuffer, - timeout: options.timeout, + timeout: resolveGitDefaultTimeoutMs(args, options.timeout), env: options.env }) return { stdout: stdout as string, stderr: stderr as string }