mirror of
https://github.com/stablyai/orca.git
synced 2026-10-16 08:03:09 +00:00
Cap hung read-path git commands with a default timeout (#5308)
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 <help@stably.ai>
This commit is contained in:
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
+61
-1
@@ -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 }
|
||||
|
||||
Reference in New Issue
Block a user