fix rebase race by fetching to private ref with timeouts

Concurrent fetches can replace FETCH_HEAD and remote-tracking refs between
fetch and rebase, causing the rebase to fail. Fetch to a temporary private
ref instead, use --no-write-fetch-head when available (Git 2.29+), and
serialize FETCH_HEAD access for older versions. Add process termination
barriers to ensure proper cleanup and extend timeouts for SSH operations.
This commit is contained in:
Jinjing
2026-08-24 11:35:14 -07:00
parent 4e7ad768d2
commit c74496bf17
22 changed files with 1295 additions and 110 deletions
+9 -8
View File
@@ -33,13 +33,14 @@ authority.
## Current Capabilities
| Capability | Preferred behavior | Compatibility behavior |
| ----------------------- | ------------------------------------------------- | --------------------------------------------------------------------------------------- |
| `worktree-list-z` | NUL-delimited worktree paths with `prunable` marks | Line-block parser for Git before `worktree list -z` (2.36); the `prunable`/`locked` annotations still parse on Git 2.312.35, and a path-existence probe restores `prunable` detection for Git before 2.31 |
| `rev-parse-path-format` | Absolute repo metadata paths | Resolve legacy relative output against the scanned repo |
| `for-each-ref-exclude` | Exclude remote HEAD before the output limit | Request extra refs, then filter remote HEAD in Orca |
| `merge-tree-write-tree` | Derive real-merge conflicts and no-op tree proofs | Omit the conflict summary and keep conservative branch cleanup behavior before Git 2.38 |
| `merge-tree-merge-base` | Supply the already-resolved merge base | Use the older two-commit `merge-tree --write-tree` form |
| Capability | Preferred behavior | Compatibility behavior |
| --------------------------- | ----------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `fetch-no-write-fetch-head` | Fetch a private rebase ref without changing worktree-local `FETCH_HEAD` | Serialize all Orca fetch/pull operations per worktree Git directory before Git 2.29 |
| `worktree-list-z` | NUL-delimited worktree paths with `prunable` marks | Line-block parser for Git before `worktree list -z` (2.36); the `prunable`/`locked` annotations still parse on Git 2.312.35, and a path-existence probe restores `prunable` detection for Git before 2.31 |
| `rev-parse-path-format` | Absolute repo metadata paths | Resolve legacy relative output against the scanned repo |
| `for-each-ref-exclude` | Exclude remote HEAD before the output limit | Request extra refs, then filter remote HEAD in Orca |
| `merge-tree-write-tree` | Derive real-merge conflicts and no-op tree proofs | Omit the conflict summary and keep conservative branch cleanup behavior before Git 2.38 |
| `merge-tree-merge-base` | Supply the already-resolved merge base | Use the older two-commit `merge-tree --write-tree` form |
## Why Not `simple-git`
@@ -54,7 +55,7 @@ capability problem.
## CI Contract
PR checks run the capability contract against real Git 2.25.5, 2.38.1, and
2.49.1 binaries. This spans the core-workflow baseline, the transitional
2.49.1 binaries. This spans the pre-2.29 serialized `FETCH_HEAD` fallback, the transitional
`merge-tree --write-tree` behavior before `--merge-base`, and current Git.
Keep the unit tests alongside that matrix. They cover concurrent probes,
+43 -10
View File
@@ -1,11 +1,13 @@
import { randomUUID } from 'node:crypto'
import { normalizeGitErrorMessage } from '../../shared/git-remote-error'
import { isNoWriteFetchHeadUnsupportedError } from '../../shared/git-fetch-head-capability'
import {
REBASE_SOURCE_FETCH_TIMEOUT_MS,
resolveGitRemoteRebaseSource
} from '../../shared/git-rebase-source'
import type { GitRuntimeOptions } from './git-runtime-options'
import { gitOptionsForWorktree } from './git-runtime-options'
import { withLocalGitCapabilityCacheForExecution } from './git-capability-state'
import { gitExecFileAsync } from './runner'
import { runWithGitReadCacheInvalidation } from './status'
@@ -15,41 +17,72 @@ export async function gitPullRebaseFromBase(
options: GitRuntimeOptions = {}
): Promise<void> {
await runWithGitReadCacheInvalidation(async () => {
const operationOptions = {
...gitOptionsForWorktree(worktreePath, options),
terminationBarrier: true,
captureWslLoginShellOutput: true
}
let rebaseRef: string | null = null
try {
const source = await resolveGitRemoteRebaseSource(
(args) => gitExecFileAsync(args, gitOptionsForWorktree(worktreePath, options)),
(args) => gitExecFileAsync(args, operationOptions),
baseRef
)
let forkPoint: string | null = null
let hasHead = true
try {
const { stdout } = await gitExecFileAsync(
['merge-base', '--fork-point', `refs/remotes/${source.displayName}`, 'HEAD'],
gitOptionsForWorktree(worktreePath, options)
operationOptions
)
forkPoint = stdout.trim() || null
} catch {
// A first fetch or an unhelpful reflog falls back to Git's merge-base behavior.
try {
await gitExecFileAsync(['rev-parse', '--verify', 'HEAD'], operationOptions)
} catch {
hasHead = false
}
}
// Why: concurrent fetches can replace FETCH_HEAD and remote-tracking refs between fetch and rebase.
rebaseRef = `refs/orca/rebase/${randomUUID()}`
await gitExecFileAsync(
['fetch', source.remoteName, `+refs/heads/${source.branchName}:${rebaseRef}`],
{ ...gitOptionsForWorktree(worktreePath, options), timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS }
const fetchArgs = [source.remoteName, `+refs/heads/${source.branchName}:${rebaseRef}`]
await withLocalGitCapabilityCacheForExecution(
{ cwd: worktreePath, ...options },
(capabilities) =>
capabilities.runWithFallback(
'fetch-no-write-fetch-head',
() =>
gitExecFileAsync(['fetch', '--no-write-fetch-head', ...fetchArgs], {
...operationOptions,
timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS
}),
() =>
gitExecFileAsync(['fetch', ...fetchArgs], {
...operationOptions,
timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS
}),
isNoWriteFetchHeadUnsupportedError
)
)
await gitExecFileAsync(
forkPoint ? ['rebase', '--onto', rebaseRef, forkPoint] : ['rebase', rebaseRef],
gitOptionsForWorktree(worktreePath, options)
hasHead
? forkPoint
? ['rebase', '--onto', rebaseRef, forkPoint]
: ['rebase', rebaseRef]
: ['merge', '--ff-only', rebaseRef],
operationOptions
)
} catch (error) {
throw new Error(normalizeGitErrorMessage(error, 'pull'))
} finally {
if (rebaseRef) {
try {
await gitExecFileAsync(
['update-ref', '-d', rebaseRef],
gitOptionsForWorktree(worktreePath, options)
const { signal: _abortedSignal, ...cleanupOptions } = gitOptionsForWorktree(
worktreePath,
options
)
await gitExecFileAsync(['update-ref', '-d', rebaseRef], cleanupOptions)
} catch {
// Cleanup must not hide the fetch or rebase result.
}
+81 -15
View File
@@ -9,11 +9,19 @@ vi.mock('./runner', () => ({
}))
import { REBASE_SOURCE_FETCH_TIMEOUT_MS } from '../../shared/git-rebase-source'
import { clearGitCapabilityStateForTests } from './git-capability-state'
import { gitFastForward, gitFetch, gitPull, gitPullRebaseFromBase, gitPush } from './remote'
const REBASE_OPERATION_OPTIONS = {
cwd: '/repo',
terminationBarrier: true,
captureWslLoginShellOutput: true
}
describe('git remote operations', () => {
beforeEach(() => {
gitExecFileAsyncMock.mockReset()
clearGitCapabilityStateForTests()
})
it('pushes to origin when no upstream is configured', async () => {
@@ -524,21 +532,29 @@ describe('git remote operations', () => {
await gitPullRebaseFromBase('/repo', 'upstream/main')
expect(gitExecFileAsyncMock.mock.calls).toEqual([
[['remote'], { cwd: '/repo' }],
[['check-ref-format', '--branch', 'main'], { cwd: '/repo' }],
[['merge-base', '--fork-point', 'refs/remotes/upstream/main', 'HEAD'], { cwd: '/repo' }],
[['remote'], REBASE_OPERATION_OPTIONS],
[['check-ref-format', '--branch', 'main'], REBASE_OPERATION_OPTIONS],
[
['fetch', 'upstream', expect.stringMatching(/^\+refs\/heads\/main:refs\/orca\/rebase\//)],
{ cwd: '/repo', timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS }
['merge-base', '--fork-point', 'refs/remotes/upstream/main', 'HEAD'],
REBASE_OPERATION_OPTIONS
],
[
[
'fetch',
'--no-write-fetch-head',
'upstream',
expect.stringMatching(/^\+refs\/heads\/main:refs\/orca\/rebase\//)
],
{ ...REBASE_OPERATION_OPTIONS, timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS }
],
[
['rebase', '--onto', expect.stringMatching(/^refs\/orca\/rebase\//), 'fork-point'],
{ cwd: '/repo' }
REBASE_OPERATION_OPTIONS
],
[['update-ref', '-d', expect.stringMatching(/^refs\/orca\/rebase\//)], { cwd: '/repo' }]
])
const fetchRefspec = gitExecFileAsyncMock.mock.calls[3][0][2]
const fetchRefspec = gitExecFileAsyncMock.mock.calls[3][0][3]
const rebasedRef = gitExecFileAsyncMock.mock.calls[4][0][2]
const deletedRef = gitExecFileAsyncMock.mock.calls[5][0][2]
expect(fetchRefspec).toBe(`+refs/heads/main:${rebasedRef}`)
@@ -559,7 +575,7 @@ describe('git remote operations', () => {
expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(
5,
['rebase', '--onto', expect.stringMatching(/^refs\/orca\/rebase\//), 'fork-point'],
{ cwd: '/repo' }
REBASE_OPERATION_OPTIONS
)
})
@@ -568,6 +584,7 @@ describe('git remote operations', () => {
.mockResolvedValueOnce({ stdout: 'upstream\n', stderr: '' })
.mockResolvedValueOnce({ stdout: '', stderr: '' })
.mockRejectedValueOnce(new Error('missing remote-tracking ref'))
.mockResolvedValueOnce({ stdout: 'head\n', stderr: '' })
.mockResolvedValueOnce({ stdout: '', stderr: '' })
.mockResolvedValueOnce({ stdout: '', stderr: '' })
.mockResolvedValueOnce({ stdout: '', stderr: '' })
@@ -575,24 +592,53 @@ describe('git remote operations', () => {
await expect(gitPullRebaseFromBase('/repo', 'upstream/main')).resolves.toBeUndefined()
expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(
4,
['fetch', 'upstream', expect.stringMatching(/^\+refs\/heads\/main:refs\/orca\/rebase\//)],
{ cwd: '/repo', timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS }
5,
[
'fetch',
'--no-write-fetch-head',
'upstream',
expect.stringMatching(/^\+refs\/heads\/main:refs\/orca\/rebase\//)
],
{ ...REBASE_OPERATION_OPTIONS, timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS }
)
})
it('fast-forwards an unborn branch from the fetched private ref', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'upstream\n', stderr: '' })
.mockResolvedValueOnce({ stdout: '', stderr: '' })
.mockRejectedValueOnce(new Error('unborn HEAD'))
.mockRejectedValueOnce(new Error('unborn HEAD'))
.mockResolvedValueOnce({ stdout: '', stderr: '' })
.mockResolvedValueOnce({ stdout: '', stderr: '' })
.mockResolvedValueOnce({ stdout: '', stderr: '' })
await gitPullRebaseFromBase('/repo', 'upstream/main')
const fetchedRef = gitExecFileAsyncMock.mock.calls[4][0][3]
expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(
6,
['merge', '--ff-only', fetchedRef.slice(fetchedRef.indexOf(':') + 1)],
REBASE_OPERATION_OPTIONS
)
})
it('removes the private ref when rebase fails', async () => {
const controller = new AbortController()
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'upstream\n', stderr: '' })
.mockResolvedValueOnce({ stdout: '', stderr: '' })
.mockResolvedValueOnce({ stdout: 'fork-point\n', stderr: '' })
.mockResolvedValueOnce({ stdout: '', stderr: '' })
.mockRejectedValueOnce(new Error('fatal: rebase conflict'))
.mockImplementationOnce(async () => {
controller.abort()
throw new Error('fatal: rebase conflict')
})
.mockResolvedValueOnce({ stdout: '', stderr: '' })
await expect(gitPullRebaseFromBase('/repo', 'upstream/main')).rejects.toThrow(
'fatal: rebase conflict'
)
await expect(
gitPullRebaseFromBase('/repo', 'upstream/main', { signal: controller.signal })
).rejects.toThrow('fatal: rebase conflict')
const rebasedRef = gitExecFileAsyncMock.mock.calls[4][0][2]
expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(6, ['update-ref', '-d', rebasedRef], {
@@ -600,6 +646,26 @@ describe('git remote operations', () => {
})
})
it('serializes the private fetch when Git cannot avoid writing FETCH_HEAD', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'upstream\n', stderr: '' })
.mockResolvedValueOnce({ stdout: '', stderr: '' })
.mockResolvedValueOnce({ stdout: 'fork-point\n', stderr: '' })
.mockRejectedValueOnce(new Error("error: unknown option `no-write-fetch-head'"))
.mockResolvedValueOnce({ stdout: '', stderr: '' })
.mockResolvedValueOnce({ stdout: '', stderr: '' })
.mockResolvedValueOnce({ stdout: '', stderr: '' })
await gitPullRebaseFromBase('/repo', 'upstream/main')
const preferredRefspec = gitExecFileAsyncMock.mock.calls[3][0][3]
expect(gitExecFileAsyncMock).toHaveBeenNthCalledWith(
5,
['fetch', 'upstream', preferredRefspec],
{ ...REBASE_OPERATION_OPTIONS, timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS }
)
})
it('normalizes pull authentication errors to a friendly message', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: 'feature\n', stderr: '' })
+44
View File
@@ -218,6 +218,50 @@ describe('runner execFile timeout handling', () => {
expect(child.kill).toHaveBeenCalled()
})
it.skipIf(process.platform === 'win32')(
'keeps a barrier Git abort pending until the process group closes',
async () => {
const child = createMockChildProcess(1234)
spawnMock.mockImplementation((command: string) => {
if (command !== 'ps') {
return child
}
const probe = createMockChildProcess(4321)
queueMicrotask(() => probe.emit('close', 0, null))
return probe
})
const processKill = vi.spyOn(process, 'kill').mockImplementation(() => true)
const controller = new AbortController()
try {
const pending = gitExecFileAsync(['status'], {
cwd: '/repo',
signal: controller.signal,
terminationBarrier: true
})
let settled = false
void pending.then(
() => {
settled = true
},
() => {
settled = true
}
)
controller.abort()
child.emit('close', 0, null)
await vi.advanceTimersByTimeAsync(1_999)
expect(settled).toBe(false)
await vi.advanceTimersByTimeAsync(1)
expect(processKill).toHaveBeenCalledWith(-1234, 'SIGKILL')
await expect(pending).rejects.toMatchObject({ name: 'AbortError' })
} finally {
processKill.mockRestore()
}
}
)
it('rejects gh executions that never call back using the default timeout', async () => {
const child = createMockChildProcess(1234)
execFileMock.mockReturnValue(child)
@@ -253,6 +253,34 @@ describe('WSL direct Git reads', () => {
})
})
it('fences parsed output from a barrier Git login-shell fallback', async () => {
await withPlatform('win32', async () => {
seedWslGitReadEnvironmentForTests(DISTRO, LOGIN_ENVIRONMENT)
spawnMock.mockImplementation((_command, args) => {
const child = createMockChild()
queueMicrotask(() => {
const fenced = fencedProbeStdout(args?.[5], 'fork-point\n')
const echoedMarker = fenced.match(/__ORCA_WSL_CAPTURE_BEGIN_[^_]+__/)?.[0] ?? ''
child.stdout.emit('data', Buffer.from(`${echoedMarker}shell trace\n${fenced}`))
child.emit('close', 0, null)
})
return child
})
await expect(
gitExecFileAsync(['merge-base', '--fork-point', 'upstream/main', 'HEAD'], {
cwd: String.raw`C:\repo`,
env: { GIT_CONFIG_GLOBAL: '/home/user/custom.gitconfig' },
wslDistro: DISTRO,
captureWslLoginShellOutput: true,
terminationBarrier: true
})
).resolves.toEqual({ stdout: 'fork-point\n', stderr: '' })
expect(spawnMock.mock.calls[0]?.[1]?.[5]).toContain('__ORCA_WSL_CAPTURE_BEGIN_')
})
})
it('ignores unchanged ambient host Git variables when selecting the direct path', async () => {
await withPlatform('win32', async () => {
const originalAskpass = process.env.GIT_ASKPASS
+110 -16
View File
@@ -44,6 +44,11 @@ import {
} from '../../shared/wsl-login-shell-command'
import { UNTRANSLATED_GIT_OUTPUT_ENV } from '../../shared/git-output-locale'
import { endSubprocessStdin } from '../../shared/subprocess-stdin-write'
import { runProcess } from '../../shared/child-process/run-process'
import {
resolveGitFetchHeadCommand,
runWithGitFetchHeadLock
} from '../../shared/git-fetch-head-lock'
import {
disableWslGitReadEnvironment,
getWslGitReadEnvironment,
@@ -418,6 +423,8 @@ type GitExecOptions = {
wslDistro?: string
preferWslDirectGit?: boolean
useConfiguredSshCommandForNetwork?: boolean
terminationBarrier?: boolean
captureWslLoginShellOutput?: boolean
}
type CommandExecOptions = {
@@ -605,6 +612,49 @@ function killSpawnedCommandTree(child: ChildProcess): Promise<void> {
type ExecFileCaptureOptions = Omit<ExecFileOptions, 'timeout'> & {
timeout?: number
stdin?: string
terminationBarrier?: boolean
}
const GIT_TERMINATION_BARRIER_FALLBACK_TIMEOUT_MS = 2_147_000_000
async function execFileCaptureToTermination(
command: string,
args: string[],
options: ExecFileCaptureOptions
): Promise<{ stdout: string | Buffer; stderr: string | Buffer }> {
const result = await runProcess({
program: command,
args,
cwd: typeof options.cwd === 'string' ? options.cwd : undefined,
env: options.env,
timeoutMs: options.timeout ?? GIT_TERMINATION_BARRIER_FALLBACK_TIMEOUT_MS,
maxOutputBytes: options.maxBuffer ?? DEFAULT_GIT_MAX_BUFFER,
signal: options.signal,
terminationBarrier: true,
...(options.stdin === undefined ? {} : { input: options.stdin })
})
const stdout = options.encoding === 'buffer' ? Buffer.from(result.stdout) : result.stdout
const stderr = options.encoding === 'buffer' ? Buffer.from(result.stderr) : result.stderr
if (result.code === 0 && !result.timedOut && !options.signal?.aborted) {
return { stdout, stderr }
}
const error = new Error(
result.timedOut
? `${command} timed out.`
: options.signal?.aborted
? 'The operation was aborted.'
: result.stderr.trim() || `${command} exited with ${result.code}.`
)
if (options.signal?.aborted) {
error.name = 'AbortError'
}
throw Object.assign(error, {
code: result.code,
killed: result.timedOut || result.signal !== null || options.signal?.aborted === true,
signal: result.signal,
stdout,
stderr
})
}
function emptyExecFileOutput(options: ExecFileCaptureOptions): string | Buffer {
@@ -1077,7 +1127,7 @@ async function buildNetworkSshPolicyEnv(options: GitExecOptions): Promise<{
* Async git command execution. Drop-in replacement for
* `execFileAsync('git', args, { cwd, encoding, ... })`.
*/
export async function gitExecFileAsync(
async function gitExecFileAsyncUnlocked(
args: string[],
options: GitExecOptions
): Promise<{ stdout: string; stderr: string }> {
@@ -1090,7 +1140,7 @@ export async function gitExecFileAsync(
signal: options.signal
})
}
let resolved = resolveGitCommand(args, options)
let resolved = resolveGitCommand(args, options, false, options.captureWslLoginShellOutput)
const environmentReady = prepareWindowsHostGitEnvironment(
resolved,
options.env,
@@ -1098,33 +1148,52 @@ export async function gitExecFileAsync(
)
const env = environmentReady ? await environmentReady : options.env
const effectiveOptions = env === options.env ? options : { ...options, env }
resolved = resolveGitCommand(args, effectiveOptions)
resolved = resolveGitCommand(
args,
effectiveOptions,
false,
effectiveOptions.captureWslLoginShellOutput
)
const policy = effectiveOptions.useConfiguredSshCommandForNetwork
? await buildNetworkSshPolicyEnv(effectiveOptions)
: { env: nonInteractiveGitEnv(effectiveOptions.env), mode: 'default' as const }
const capture = (
command: ResolvedCommand
): Promise<{ stdout: string | Buffer; stderr: string | Buffer }> =>
execFileCapture(command.binary, command.args, {
cwd: command.cwd,
encoding: (options.encoding ?? 'utf-8') as BufferEncoding,
maxBuffer: options.maxBuffer,
timeout: options.timeout,
stdin: options.stdin,
env: policy.env,
signal: options.signal
})
(options.terminationBarrier ? execFileCaptureToTermination : execFileCapture)(
command.binary,
command.args,
{
cwd: command.cwd,
encoding: (options.encoding ?? 'utf-8') as BufferEncoding,
maxBuffer: options.maxBuffer,
timeout: options.timeout,
stdin: options.stdin,
env: policy.env,
signal: options.signal,
terminationBarrier: options.terminationBarrier
}
)
let result: { stdout: string | Buffer; stderr: string | Buffer }
try {
result = await capture(resolved)
} catch (error) {
if (directWslGitExitCode(error, resolved) !== null && !options.signal?.aborted) {
const wasMissing = invalidateMissingDirectWslGit(error, resolved)
result = await capture(resolveGitCommand(args, effectiveOptions, true))
const fallback = resolveGitCommand(
args,
effectiveOptions,
true,
effectiveOptions.captureWslLoginShellOutput
)
result = await capture(fallback)
// Why: matching failures can be normal Git control flow; only a successful login retry proves the direct environment was insufficient.
disableDirectWslGitAfterSuccessfulFallback(wasMissing, resolved)
const { stdout, stderr } = result
return { stdout: stdout as string, stderr: stderr as string }
return {
stdout: readCapturedGitString(stdout as string, fallback),
stderr: stderr as string
}
}
if (options.useConfiguredSshCommandForNetwork && error && typeof error === 'object') {
Object.assign(error, { gitSshPolicyMode: policy.mode })
@@ -1132,11 +1201,22 @@ export async function gitExecFileAsync(
throw error
}
const { stdout, stderr } = result
return { stdout: stdout as string, stderr: stderr as string }
return { stdout: readCapturedGitString(stdout as string, resolved), stderr: stderr as string }
}
)
}
export function gitExecFileAsync(
args: string[],
options: GitExecOptions
): Promise<{ stdout: string; stderr: string }> {
const run = () => gitExecFileAsyncUnlocked(args, options)
const command = resolveGitFetchHeadCommand(args, options.cwd)
return command.needsLock
? runWithGitFetchHeadLock(command.cwd, options.signal, run, command.gitDir)
: run()
}
/**
* Async command execution with the same WSL cwd translation as repo-scoped git.
* Keep this for fixed binary+argv call sites; never pass shell fragments.
@@ -1222,7 +1302,7 @@ function readCapturedGitBuffer(stdout: Buffer, resolved: ResolvedCommand): Buffe
if (!captured) {
return stdout
}
const beginIndex = stdout.indexOf(captured.beginMarker, 0, 'utf8')
const beginIndex = stdout.lastIndexOf(captured.beginMarker, undefined, 'utf8')
if (beginIndex === -1) {
return stdout
}
@@ -1231,6 +1311,20 @@ function readCapturedGitBuffer(stdout: Buffer, resolved: ResolvedCommand): Buffe
return endIndex === -1 ? stdout.subarray(payloadStart) : stdout.subarray(payloadStart, endIndex)
}
function readCapturedGitString(stdout: string, resolved: ResolvedCommand): string {
const captured = resolved.captured
if (!captured) {
return stdout
}
const beginIndex = stdout.lastIndexOf(captured.beginMarker)
if (beginIndex === -1) {
return stdout
}
const payloadStart = beginIndex + captured.beginMarker.length
const endIndex = stdout.indexOf(captured.endMarker, payloadStart)
return endIndex === -1 ? stdout.slice(payloadStart) : stdout.slice(payloadStart, endIndex)
}
/** Result of a streamed git command; `stoppedEarly` is true when onStdout asked to stop before the child exited. */
export type GitStreamResult = { stoppedEarly: boolean }
@@ -1,6 +1,7 @@
import { describe, expect, it, beforeEach } from 'vitest'
import { SshGitProvider } from './ssh-git-provider'
import { createMockMux, type MockMultiplexer } from './ssh-git-provider-test-harness'
import { REBASE_FROM_BASE_RPC_TIMEOUT_MS } from '../../shared/git-rebase-source'
describe('SshGitProvider', () => {
let mux: MockMultiplexer
@@ -101,10 +102,14 @@ describe('SshGitProvider', () => {
it('rebaseFromBase sends git.rebaseFromBase request', async () => {
await provider.rebaseFromBase('/home/user/repo', 'upstream/main')
expect(mux.request).toHaveBeenCalledWith('git.rebaseFromBase', {
worktreePath: '/home/user/repo',
baseRef: 'upstream/main'
})
expect(mux.request).toHaveBeenCalledWith(
'git.rebaseFromBase',
{
worktreePath: '/home/user/repo',
baseRef: 'upstream/main'
},
{ timeoutMs: REBASE_FROM_BASE_RPC_TIMEOUT_MS }
)
})
it('fetchRemote sends git.fetch request', async () => {
+6 -1
View File
@@ -33,6 +33,7 @@ import {
import { InFlightPromiseDedupe, stableInFlightKey } from '../../shared/in-flight-promise-dedupe'
import { gitExecMutatesRepository } from '../../shared/git-exec-mutation'
import { GitStatusReadLeaseOwner } from '../git/git-status-read-lease-owner'
import { REBASE_FROM_BASE_RPC_TIMEOUT_MS } from '../../shared/git-rebase-source'
import { GitUpstreamStatusReadOwner } from '../git/git-upstream-status-read-owner'
type NonInteractiveExecQueueEntry = {
@@ -565,7 +566,11 @@ export class SshGitProvider implements IGitProvider {
async rebaseFromBase(worktreePath: string, baseRef: string): Promise<void> {
await this.runWithGitReadInvalidation(async () => {
await this.mux.request('git.rebaseFromBase', { worktreePath, baseRef })
await this.mux.request(
'git.rebaseFromBase',
{ worktreePath, baseRef },
{ timeoutMs: REBASE_FROM_BASE_RPC_TIMEOUT_MS }
)
})
}
+87 -4
View File
@@ -3,7 +3,7 @@
* sync, fast-forward, and the narrow review-head fetches for GitHub pull
* requests and GitLab merge requests.
*/
import { describe, expect, it, beforeEach, afterEach } from 'vitest'
import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest'
import * as fs from 'node:fs/promises'
import * as path from 'node:path'
import { mkdtempSync, writeFileSync } from 'node:fs'
@@ -14,16 +14,20 @@ import { gitInit, gitCommit, type MockDispatcher } from './git-handler-test-setu
import {
createGitHandlerRelay,
createGitTempDir,
removeGitTempDir
removeGitTempDir,
type GitSpyTarget
} from './git-handler-test-harness'
describe('GitHandler', () => {
let dispatcher: MockDispatcher
let tmpDir: string
let gitTarget: GitSpyTarget
beforeEach(() => {
tmpDir = createGitTempDir()
;({ dispatcher } = createGitHandlerRelay())
const relay = createGitHandlerRelay()
dispatcher = relay.dispatcher
gitTarget = relay.handler as unknown as GitSpyTarget
})
afterEach(async () => {
@@ -199,7 +203,10 @@ describe('GitHandler', () => {
execFileSync('git', ['reset', '--hard', forkPoint], { cwd: producerDir, stdio: 'pipe' })
writeFileSync(path.join(producerDir, 'replacement.txt'), 'replacement')
gitCommit(producerDir, 'replacement remote commit')
execFileSync('git', ['push', '--force', 'origin', branch], { cwd: producerDir, stdio: 'pipe' })
execFileSync('git', ['push', '--force', 'origin', branch], {
cwd: producerDir,
stdio: 'pipe'
})
await dispatcher.callRequest('git.rebaseFromBase', {
worktreePath: tmpDir,
@@ -223,6 +230,82 @@ describe('GitHandler', () => {
}
}, 15_000)
it('fast-forwards an unborn branch from the selected remote base', async () => {
const bareDir = mkdtempSync(path.join(tmpdir(), 'relay-git-unborn-bare-'))
const producerDir = mkdtempSync(path.join(tmpdir(), 'relay-git-unborn-producer-'))
try {
execFileSync('git', ['init', '--bare'], { cwd: bareDir, stdio: 'pipe' })
gitInit(producerDir)
writeFileSync(path.join(producerDir, 'base.txt'), 'base')
gitCommit(producerDir, 'base')
const branch = execFileSync('git', ['branch', '--show-current'], {
cwd: producerDir,
encoding: 'utf-8'
}).trim()
execFileSync('git', ['remote', 'add', 'origin', bareDir], {
cwd: producerDir,
stdio: 'pipe'
})
execFileSync('git', ['push', 'origin', branch], { cwd: producerDir, stdio: 'pipe' })
gitInit(tmpDir)
execFileSync('git', ['remote', 'add', 'origin', bareDir], { cwd: tmpDir, stdio: 'pipe' })
await dispatcher.callRequest('git.rebaseFromBase', {
worktreePath: tmpDir,
baseRef: `origin/${branch}`
})
await expect(fs.access(path.join(tmpDir, 'base.txt'))).resolves.toBeUndefined()
expect(execFileSync('git', ['rev-parse', '--verify', 'HEAD'], { cwd: tmpDir })).toBeTruthy()
} finally {
await Promise.all([
fs.rm(bareDir, { recursive: true, force: true }),
fs.rm(producerDir, { recursive: true, force: true })
])
}
})
it('cancels the active rebase fetch and still removes its private ref', async () => {
const controller = new AbortController()
let rejectFetch!: (error: Error) => void
const fetchStarted = new Promise<void>((resolve) => {
vi.spyOn(gitTarget, 'git').mockImplementation(async (args, _cwd, options) => {
if (args[0] === 'remote') {
return { stdout: 'origin\n', stderr: '' }
}
if (args[0] === 'merge-base') {
return { stdout: 'fork-point\n', stderr: '' }
}
if (args[0] === 'fetch') {
resolve()
return new Promise((_resolve, reject) => {
rejectFetch = reject
options?.signal?.addEventListener('abort', () => reject(new Error('aborted')), {
once: true
})
})
}
return { stdout: '', stderr: '' }
})
})
const request = dispatcher.callRequest(
'git.rebaseFromBase',
{ worktreePath: tmpDir, baseRef: 'origin/main' },
{ isStale: () => false, signal: controller.signal }
)
await fetchStarted
controller.abort()
await expect(request).rejects.toThrow('aborted')
const calls = vi.mocked(gitTarget.git).mock.calls
expect(calls.some(([args]) => args[0] === 'rebase')).toBe(false)
const cleanup = calls.find(([args]) => args[0] === 'update-ref')
expect(cleanup?.[2]?.signal).toBeUndefined()
expect(rejectFetch).toBeTypeOf('function')
})
it('fetches the explicit publish target remote', async () => {
const bareDir = mkdtempSync(path.join(tmpdir(), 'relay-git-fork-bare-'))
try {
+56
View File
@@ -0,0 +1,56 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const { runProcessMock } = vi.hoisted(() => ({ runProcessMock: vi.fn() }))
vi.mock('../shared/child-process/run-process', () => ({ runProcess: runProcessMock }))
import { createGitHandlerRelay } from './git-handler-test-harness'
type GitTerminationTarget = {
git(
args: string[],
cwd: string,
options: { signal?: AbortSignal; terminationBarrier: true; timeout?: number }
): Promise<{ stdout: string; stderr: string }>
}
describe('GitHandler termination barrier', () => {
beforeEach(() => runProcessMock.mockReset())
it('rejects a zero-exit result that crossed its timeout', async () => {
runProcessMock.mockResolvedValue({
code: 0,
signal: null,
stdout: '',
stderr: '',
timedOut: true
})
const { handler } = createGitHandlerRelay()
const target = handler as unknown as GitTerminationTarget
await expect(
target.git(['status'], '/repo', { terminationBarrier: true, timeout: 1 })
).rejects.toThrow('git status timed out.')
})
it('rejects a zero-exit result after caller cancellation', async () => {
runProcessMock.mockResolvedValue({
code: 0,
signal: null,
stdout: '',
stderr: '',
timedOut: false
})
const controller = new AbortController()
controller.abort()
const { handler } = createGitHandlerRelay()
const target = handler as unknown as GitTerminationTarget
await expect(
target.git(['status'], '/repo', {
signal: controller.signal,
terminationBarrier: true
})
).rejects.toMatchObject({ name: 'AbortError' })
})
})
+124 -26
View File
@@ -61,7 +61,10 @@ import {
import { upstreamOnlyCommitsArePatchEquivalent } from '../shared/git-upstream-status'
import { assertGitPushTargetShape } from '../shared/git-push-target-validation'
import { getPublishTargetStatus, type GitCommandRunner } from '../shared/git-publish-target-status'
import { isNoWriteFetchHeadUnsupportedError } from '../shared/git-fetch-head-capability'
import { resolveGitFetchHeadCommand, runWithGitFetchHeadLock } from '../shared/git-fetch-head-lock'
import {
REBASE_FROM_BASE_OPERATION_TIMEOUT_MS,
REBASE_SOURCE_FETCH_TIMEOUT_MS,
resolveGitRemoteRebaseSource
} from '../shared/git-rebase-source'
@@ -100,10 +103,12 @@ import { endSubprocessStdin } from '../shared/subprocess-stdin-write'
import { clearGitStatusLineStatsCache } from '../shared/git-status-line-stats-cache'
import { invalidateGitBranchLineTotalInFlight } from '../shared/git-branch-line-total'
import { streamRelayGitStdout } from './git-stdout-stream'
import { runProcess } from '../shared/child-process/run-process'
const execFileAsync = promisify(execFile)
const MAX_GIT_BUFFER = 10 * 1024 * 1024
const BULK_CHUNK_SIZE = 100
const GIT_REBASE_PROCESS_FALLBACK_TIMEOUT_MS = 2_147_000_000
function resolveSubmoduleStatusArea(
params: Record<string, unknown>
@@ -182,6 +187,47 @@ function execFileWithStdin(
})
}
async function runGitToTermination(
args: string[],
options: ExecFileOptions,
stdin: string | undefined
): Promise<{ stdout: string; stderr: string }> {
const result = await runProcess({
program: 'git',
args,
cwd: typeof options.cwd === 'string' ? options.cwd : undefined,
env: options.env,
timeoutMs:
typeof options.timeout === 'number'
? options.timeout
: GIT_REBASE_PROCESS_FALLBACK_TIMEOUT_MS,
maxOutputBytes: typeof options.maxBuffer === 'number' ? options.maxBuffer : MAX_GIT_BUFFER,
signal: options.signal,
terminationBarrier: true,
...(stdin === undefined ? {} : { input: stdin })
})
if (result.code === 0 && !result.timedOut && !options.signal?.aborted) {
return { stdout: result.stdout, stderr: result.stderr }
}
const error = new Error(
result.timedOut
? `git ${args[0] ?? 'command'} timed out.`
: options.signal?.aborted
? 'The operation was aborted.'
: result.stderr.trim() || `git ${args[0] ?? 'command'} failed.`
)
if (options.signal?.aborted) {
error.name = 'AbortError'
}
throw Object.assign(error, {
code: result.code,
killed: result.timedOut || result.signal !== null || options.signal?.aborted === true,
signal: result.signal,
stdout: result.stdout,
stderr: result.stderr
})
}
export class GitHandler {
private dispatcher: RelayDispatcher
private readonly gitDiffReadDedupe = new InFlightPromiseDedupe<unknown>()
@@ -252,7 +298,7 @@ export class GitHandler {
this.dispatcher.onRequest('git.push', (p) => this.push(p))
this.dispatcher.onRequest('git.pull', (p) => this.pull(p))
this.dispatcher.onRequest('git.fastForward', (p) => this.fastForward(p))
this.dispatcher.onRequest('git.rebaseFromBase', (p) => this.rebaseFromBase(p))
this.dispatcher.onRequest('git.rebaseFromBase', (p, context) => this.rebaseFromBase(p, context))
this.dispatcher.onRequest('git.branchDiff', (p, context) => this.branchDiff(p, context))
this.dispatcher.onRequest('git.commitDiff', (p, context) => this.commitDiff(p, context))
this.dispatcher.onRequest('git.listWorktrees', (p, context) => this.listWorktrees(p, context))
@@ -333,25 +379,36 @@ export class GitHandler {
nonInteractive?: boolean
stdin?: string
timeout?: number
terminationBarrier?: boolean
}
): Promise<{ stdout: string; stderr: string }> {
const env = opts?.nonInteractive ? buildRelayUnattendedGitEnv() : buildRelayGitEnv()
if (opts?.disableOptionalLocks) {
env.GIT_OPTIONAL_LOCKS = '0'
const expandedCwd = expandTilde(cwd)
const run = async (): Promise<{ stdout: string; stderr: string }> => {
const env = opts?.nonInteractive ? buildRelayUnattendedGitEnv() : buildRelayGitEnv()
if (opts?.disableOptionalLocks) {
env.GIT_OPTIONAL_LOCKS = '0'
}
const execOptions = {
cwd: expandedCwd,
env,
encoding: 'utf-8',
maxBuffer: opts?.maxBuffer ?? MAX_GIT_BUFFER,
timeout: opts?.timeout,
signal: opts?.signal
} satisfies ExecFileOptions
if (opts?.terminationBarrier) {
return runGitToTermination(args, execOptions, opts.stdin)
}
if (opts?.stdin !== undefined) {
return execFileWithStdin('git', args, execOptions, opts.stdin)
}
const { stdout, stderr } = await execFileAsync('git', args, execOptions)
return { stdout: String(stdout), stderr: String(stderr) }
}
const execOptions = {
cwd: expandTilde(cwd),
env,
encoding: 'utf-8',
maxBuffer: opts?.maxBuffer ?? MAX_GIT_BUFFER,
timeout: opts?.timeout,
signal: opts?.signal
} satisfies ExecFileOptions
if (opts?.stdin !== undefined) {
return execFileWithStdin('git', args, execOptions, opts.stdin)
}
const { stdout, stderr } = await execFileAsync('git', args, execOptions)
return { stdout: String(stdout), stderr: String(stderr) }
const command = resolveGitFetchHeadCommand(args, expandedCwd)
return command.needsLock
? runWithGitFetchHeadLock(command.cwd, opts?.signal, run, command.gitDir)
: run()
}
private async gitBuffer(args: string[], cwd: string): Promise<Buffer> {
@@ -1132,37 +1189,76 @@ export class GitHandler {
await this.pullWithArgs(params, ['--ff-only'])
}
private async rebaseFromBase(params: Record<string, unknown>) {
private async rebaseFromBase(params: Record<string, unknown>, context?: RequestContext) {
this.clearGitMutationReadCaches()
const worktreePath = params.worktreePath as string
const baseRef = params.baseRef as string
let rebaseRef: string | null = null
const controller = new AbortController()
const abortFromContext = () => controller.abort()
if (context?.signal?.aborted) {
controller.abort()
} else {
context?.signal?.addEventListener('abort', abortFromContext, { once: true })
}
const timeout = setTimeout(() => controller.abort(), REBASE_FROM_BASE_OPERATION_TIMEOUT_MS)
try {
try {
const source = await resolveGitRemoteRebaseSource(
((args) => this.git(args, worktreePath)) as GitCommandRunner,
((args) =>
this.git(args, worktreePath, {
signal: controller.signal,
terminationBarrier: true
})) as GitCommandRunner,
baseRef
)
let forkPoint: string | null = null
let hasHead = true
try {
const { stdout } = await this.git(
['merge-base', '--fork-point', `refs/remotes/${source.displayName}`, 'HEAD'],
worktreePath
worktreePath,
{ signal: controller.signal, terminationBarrier: true }
)
forkPoint = stdout.trim() || null
} catch {
// A first fetch or an unhelpful reflog falls back to Git's merge-base behavior.
try {
await this.git(['rev-parse', '--verify', 'HEAD'], worktreePath, {
signal: controller.signal,
terminationBarrier: true
})
} catch {
hasHead = false
}
}
// Why: concurrent fetches can replace FETCH_HEAD and remote-tracking refs between fetch and rebase.
rebaseRef = `refs/orca/rebase/${randomUUID()}`
await this.git(
['fetch', source.remoteName, `+refs/heads/${source.branchName}:${rebaseRef}`],
worktreePath,
{ timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS }
const fetchArgs = [source.remoteName, `+refs/heads/${source.branchName}:${rebaseRef}`]
await this.gitCapabilities.runWithFallback(
'fetch-no-write-fetch-head',
() =>
this.git(['fetch', '--no-write-fetch-head', ...fetchArgs], worktreePath, {
timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS,
signal: controller.signal,
terminationBarrier: true
}),
() =>
this.git(['fetch', ...fetchArgs], worktreePath, {
timeout: REBASE_SOURCE_FETCH_TIMEOUT_MS,
signal: controller.signal,
terminationBarrier: true
}),
isNoWriteFetchHeadUnsupportedError
)
await this.git(
forkPoint ? ['rebase', '--onto', rebaseRef, forkPoint] : ['rebase', rebaseRef],
worktreePath
hasHead
? forkPoint
? ['rebase', '--onto', rebaseRef, forkPoint]
: ['rebase', rebaseRef]
: ['merge', '--ff-only', rebaseRef],
worktreePath,
{ signal: controller.signal, terminationBarrier: true }
)
} catch (error) {
throw new Error(normalizeGitErrorMessage(error, 'pull'))
@@ -1175,6 +1271,8 @@ export class GitHandler {
// Cleanup must not hide the fetch or rebase result.
}
}
clearTimeout(timeout)
context?.signal?.removeEventListener('abort', abortFromContext)
this.clearGitMutationReadCaches()
}
}
@@ -0,0 +1,74 @@
import { EventEmitter } from 'node:events'
import type { ChildProcess } from 'node:child_process'
import { afterEach, describe, expect, it, vi } from 'vitest'
const { spawnMock } = vi.hoisted(() => ({ spawnMock: vi.fn() }))
vi.mock('node:child_process', () => ({ spawn: spawnMock }))
import { forceTerminateProcessTree } from './process-tree-termination'
function mockProcess(pid: number): ChildProcess {
const child = new EventEmitter() as EventEmitter & {
pid: number
kill: ReturnType<typeof vi.fn>
}
child.pid = pid
child.kill = vi.fn((_signal?: NodeJS.Signals | number) => true)
return child as unknown as ChildProcess
}
async function withWindows(run: () => Promise<void>): Promise<void> {
const original = process.platform
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
try {
await run()
} finally {
Object.defineProperty(process, 'platform', { configurable: true, value: original })
}
}
describe('forceTerminateProcessTree', () => {
afterEach(() => {
spawnMock.mockReset()
vi.useRealTimers()
})
it('waits for Windows taskkill tree completion', async () => {
await withWindows(async () => {
const child = mockProcess(1234)
const taskkill = mockProcess(5678)
spawnMock.mockReturnValue(taskkill)
let settled = false
const pending = forceTerminateProcessTree(child)
void pending.then(() => {
settled = true
})
await Promise.resolve()
expect(settled).toBe(false)
expect(spawnMock).toHaveBeenCalledWith(
'taskkill',
['/pid', '1234', '/t', '/f'],
expect.objectContaining({ shell: false, windowsHide: true })
)
taskkill.emit('close', 0)
await expect(pending).resolves.toBe(true)
expect(child.kill).not.toHaveBeenCalled()
})
})
it('falls back to the root when Windows tree termination fails', async () => {
await withWindows(async () => {
const child = mockProcess(1234)
const taskkill = mockProcess(5678)
spawnMock.mockReturnValue(taskkill)
const pending = forceTerminateProcessTree(child)
taskkill.emit('close', 1)
await expect(pending).resolves.toBe(false)
expect(child.kill).toHaveBeenCalledWith('SIGKILL')
})
})
})
@@ -0,0 +1,150 @@
import { spawn as nodeSpawn, type ChildProcess } from 'node:child_process'
const PROBE_INTERVAL_MS = 25
const SUBPROCESS_TIMEOUT_MS = 2_000
const MAX_PS_OUTPUT_BYTES = 8 * 1024 * 1024
export function signalProcessTree(child: ChildProcess, signal?: NodeJS.Signals): Promise<boolean> {
if (!child.pid) {
killRoot(child, signal)
return Promise.resolve(true)
}
if (process.platform === 'win32') {
return taskkillTree(child, signal)
}
try {
process.kill(-child.pid, signal)
return Promise.resolve(true)
} catch {
return Promise.resolve(!processGroupExists(child.pid))
}
}
export async function forceTerminateProcessTree(child: ChildProcess): Promise<boolean> {
const signaled = await signalProcessTree(child, 'SIGKILL')
if (!signaled) {
return false
}
if (process.platform !== 'win32' && child.pid) {
await waitForPosixProcessGroupQuiescence(child.pid)
}
return true
}
function taskkillTree(child: ChildProcess, signal?: NodeJS.Signals): Promise<boolean> {
return new Promise((resolve) => {
let killer: ChildProcess
try {
killer = nodeSpawn('taskkill', ['/pid', String(child.pid), '/t', '/f'], {
stdio: 'ignore',
windowsHide: true,
shell: false
})
} catch {
killRoot(child, signal)
resolve(false)
return
}
let settled = false
const finish = (fallback: boolean): void => {
if (settled) {
return
}
settled = true
clearTimeout(timer)
if (fallback) {
killRoot(child, signal)
}
resolve(!fallback)
}
killer.once('error', () => finish(true))
killer.once('close', (code) => finish(code !== 0))
const timer = setTimeout(() => {
killer.kill()
finish(true)
}, SUBPROCESS_TIMEOUT_MS)
timer.unref?.()
})
}
async function waitForPosixProcessGroupQuiescence(processGroupId: number): Promise<void> {
while (true) {
const states = await readPosixProcessGroupStates(processGroupId)
if (
states ? states.every((state) => state.startsWith('Z')) : !processGroupExists(processGroupId)
) {
return
}
await new Promise<void>((resolve) => setTimeout(resolve, PROBE_INTERVAL_MS))
}
}
function readPosixProcessGroupStates(processGroupId: number): Promise<string[] | null> {
return new Promise((resolve) => {
let probe: ChildProcess
try {
probe = nodeSpawn('ps', ['-axo', 'pgid=,state='], {
stdio: ['ignore', 'pipe', 'ignore'],
windowsHide: true,
shell: false
})
} catch {
resolve(null)
return
}
let output = ''
let truncated = false
let settled = false
const finish = (states: string[] | null): void => {
if (settled) {
return
}
settled = true
clearTimeout(timer)
resolve(states)
}
probe.stdout?.on('data', (chunk: Buffer | string) => {
const text = chunk.toString()
if (output.length + text.length > MAX_PS_OUTPUT_BYTES) {
truncated = true
return
}
output += text
})
probe.stdout?.on('error', () => {})
probe.once('error', () => finish(null))
probe.once('close', (code) => {
if (code !== 0 || truncated) {
finish(null)
return
}
const states = output.split('\n').flatMap((line) => {
const match = line.trim().match(/^(\d+)\s+(\S+)/)
return match && Number(match[1]) === processGroupId ? [match[2]] : []
})
finish(states)
})
const timer = setTimeout(() => {
probe.kill()
finish(null)
}, SUBPROCESS_TIMEOUT_MS)
timer.unref?.()
})
}
function processGroupExists(processGroupId: number): boolean {
try {
process.kill(-processGroupId, 0)
return true
} catch (error) {
return (error as NodeJS.ErrnoException).code !== 'ESRCH'
}
}
function killRoot(child: ChildProcess, signal?: NodeJS.Signals): void {
try {
child.kill(signal)
} catch {
/* already gone */
}
}
+40 -2
View File
@@ -1,4 +1,7 @@
import { describe, expect, it } from 'vitest'
import { mkdtemp, readFile, rm } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import * as path from 'node:path'
import { resolveSpawn, runProcess, runProcessSync } from './run-process'
import { WINDOWS_ARGUMENT_CORPUS } from './__fixtures__/windows-argument-corpus'
@@ -126,6 +129,42 @@ describe('abort', () => {
// Not a timeout: the caller asked it to stop.
expect(result.timedOut).toBe(false)
}, 20_000)
it.skipIf(process.platform === 'win32')(
'kills the process group and waits for confirmed root exit with a termination barrier',
async () => {
const root = await mkdtemp(path.join(tmpdir(), 'run-process-barrier-'))
const marker = path.join(root, 'descendant-state')
const descendantScript =
`printf ready > "$1";trap 'printf signaled > "$1"' TERM;` + `while :;do sleep 1;done`
const controller = new AbortController()
const pending = runProcess({
program: process.execPath,
args: [
'-e',
`const {spawn}=require('node:child_process');` +
`spawn('/bin/sh',${JSON.stringify(['-c', descendantScript, 'sh', marker])},{stdio:'ignore'});` +
`setInterval(()=>{},1000)`
],
timeoutMs: 60_000,
signal: controller.signal,
terminationBarrier: true
})
try {
await expect
.poll(() => readFile(marker, 'utf8').catch(() => ''), { timeout: 10_000 })
.toBe('ready')
controller.abort()
await pending
await expect.poll(() => readFile(marker, 'utf8')).toBe('signaled')
} finally {
controller.abort()
await pending
await rm(root, { recursive: true, force: true })
}
},
30_000
)
})
describe('stdin delivery failures', () => {
@@ -151,8 +190,7 @@ describe('a signal that is already aborted', () => {
controller.abort()
const startedAt = Date.now()
const result = await runProcess({
program: process.execPath,
args: ['-e', 'setInterval(() => {}, 1000)'],
program: path.join(tmpdir(), 'orca-must-not-spawn'),
timeoutMs: 30_000,
signal: controller.signal
})
+71 -23
View File
@@ -5,6 +5,7 @@ import {
type SpawnOptions as NodeSpawnOptions
} from 'node:child_process'
import { buildWindowsCmdShimCommandLine, isCmdInterpretedProgram } from './windows-command-line'
import { forceTerminateProcessTree, signalProcessTree } from './process-tree-termination'
/**
* The single place Orca starts a child process.
@@ -39,6 +40,8 @@ export type ProcessSpec = {
maxOutputBytes?: number
/** Kills the process when aborted; the result still reports the exit. */
signal?: AbortSignal
/** Kill the whole process tree and do not settle until the root confirms exit. */
terminationBarrier?: boolean
}
export type ProcessResult = {
@@ -89,7 +92,8 @@ export function resolveSpawn(spec: ProcessSpec, platform: NodeJS.Platform): Reso
windowsHide: true,
// Why never `shell: true`: it concatenates arguments without escaping (Node
// itself warns DEP0190) and it silently makes windowsHide a no-op.
shell: false
shell: false,
...(spec.terminationBarrier && platform !== 'win32' ? { detached: true } : {})
}
if (platform !== 'win32' || !isCmdInterpretedProgram(spec.program)) {
@@ -155,6 +159,9 @@ function createOutputSink(maxBytes: number): {
* the process could not be started at all.
*/
export function runProcess(spec: ProcessSpec): Promise<ProcessResult> {
if (spec.signal?.aborted) {
return Promise.resolve({ code: null, signal: null, stdout: '', stderr: '', timedOut: false })
}
const maxOutputBytes = spec.maxOutputBytes ?? DEFAULT_MAX_OUTPUT_BYTES
return new Promise<ProcessResult>((resolve, reject) => {
@@ -170,6 +177,10 @@ export function runProcess(spec: ProcessSpec): Promise<ProcessResult> {
const stderr = createOutputSink(maxOutputBytes)
let timedOut = false
let settled = false
let barrierStopping = false
let barrierReady = false
let initialBarrierTermination: Promise<boolean> | undefined
let deferredClose: { code: number | null; signal: NodeJS.Signals | null } | null = null
const settle = (act: () => void): void => {
if (settled) {
@@ -196,6 +207,11 @@ export function runProcess(spec: ProcessSpec): Promise<ProcessResult> {
let graceTimer: ReturnType<typeof setTimeout> | undefined
const resolveFromClose = (code: number | null, signal: NodeJS.Signals | null): void =>
settle(() =>
resolve({ code, signal, stdout: stdout.text(), stderr: stderr.text(), timedOut })
)
/**
* Stop the child, then settle whether or not it complies.
*
@@ -205,18 +221,53 @@ export function runProcess(spec: ProcessSpec): Promise<ProcessResult> {
* promise, so a single unkillable child would wedge every one of them.
*/
const stopAndSettle = (): void => {
terminate(child)
graceTimer ??= setTimeout(() => {
terminate(child, 'SIGKILL')
settle(() =>
resolve({
code: null,
signal: null,
stdout: stdout.text(),
stderr: stderr.text(),
timedOut
if (spec.terminationBarrier) {
barrierStopping = true
initialBarrierTermination ??= signalProcessTree(child)
if (process.platform === 'win32') {
void initialBarrierTermination.then((terminated) => {
if (!terminated) {
return
}
barrierReady = true
if (deferredClose) {
resolveFromClose(deferredClose.code, deferredClose.signal)
}
})
)
}
} else {
terminate(child)
}
graceTimer ??= setTimeout(() => {
if (spec.terminationBarrier) {
const initialTermination = initialBarrierTermination ?? Promise.resolve(false)
if (process.platform === 'win32') {
void initialTermination.then((terminated) => {
if (!terminated) {
terminate(child, 'SIGKILL')
}
barrierReady = true
if (deferredClose) {
resolveFromClose(deferredClose.code, deferredClose.signal)
}
})
return
}
void Promise.all([initialTermination, forceTerminateProcessTree(child)]).then(
([initialTerminated, forceTerminated]) => {
if (!initialTerminated || !forceTerminated) {
return
}
barrierReady = true
if (deferredClose) {
resolveFromClose(deferredClose.code, deferredClose.signal)
}
}
)
return
}
terminate(child, 'SIGKILL')
resolveFromClose(null, null)
}, PROCESS_EXIT_GRACE_MS)
graceTimer.unref?.()
}
@@ -239,11 +290,13 @@ export function runProcess(spec: ProcessSpec): Promise<ProcessResult> {
}
child.once('error', (error) => settle(() => reject(error)))
child.once('close', (code, signal) =>
settle(() =>
resolve({ code, signal, stdout: stdout.text(), stderr: stderr.text(), timedOut })
)
)
child.once('close', (code, signal) => {
if (barrierStopping && !barrierReady) {
deferredClose = { code, signal }
return
}
resolveFromClose(code, signal)
})
// Why close rather than leave open: a child that reads stdin (a hook
// draining its payload, a CLI probing for a TTY) otherwise blocks until the
@@ -252,12 +305,7 @@ export function runProcess(spec: ProcessSpec): Promise<ProcessResult> {
})
}
/**
* Best-effort termination of a captured child.
*
* Deliberately root-only: descendant reaping is the job-object owner's
* responsibility, not every caller's.
*/
/** Best-effort root termination, or whole-tree termination for barrier callers. */
function terminate(child: ChildProcess, signal?: NodeJS.Signals): void {
try {
child.kill(signal)
+10 -1
View File
@@ -1,5 +1,5 @@
import { execFile } from 'node:child_process'
import { mkdtemp, rename, rm, writeFile } from 'node:fs/promises'
import { mkdtemp, readFile, rename, rm, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { promisify } from 'node:util'
@@ -9,6 +9,7 @@ import {
isUnsupportedMergeTreeWriteTreeError
} from './git-merge-tree-capability'
import { isForEachRefExcludeUnsupportedError } from './git-ref-command-capabilities'
import { isNoWriteFetchHeadUnsupportedError } from './git-fetch-head-capability'
import {
hasUnsupportedRevParsePathFormatEcho,
isUnsupportedWorktreeListZError
@@ -151,6 +152,14 @@ describeBinaryCompatibility('real Git binary compatibility', () => {
})
it('recognizes ref and merge-tree compatibility boundaries', async () => {
const fetchHeadPath = join(repoPath, '.git', 'FETCH_HEAD')
await writeFile(fetchHeadPath, 'sentinel\n')
await expectPreferredOrRecognizedFallback(
['fetch', '--no-write-fetch-head', '.', '+HEAD:refs/orca/compat/no-write-fetch-head'],
supports(2, 29),
isNoWriteFetchHeadUnsupportedError
)
await expect(readFile(fetchHeadPath, 'utf-8')).resolves.toBe('sentinel\n')
await expectPreferredOrRecognizedFallback(
['for-each-ref', '--format=%(refname)', '--exclude=refs/remotes/**/HEAD', '--count=10'],
supports(2, 42),
+1
View File
@@ -3,6 +3,7 @@
export const GIT_CAPABILITY_RETRY_INTERVAL_MS = 30 * 60_000
export type GitCapability =
| 'fetch-no-write-fetch-head'
| 'for-each-ref-exclude'
| 'merge-tree-merge-base'
| 'merge-tree-write-tree'
@@ -0,0 +1,18 @@
import { describe, expect, it } from 'vitest'
import { isNoWriteFetchHeadUnsupportedError } from './git-fetch-head-capability'
describe('isNoWriteFetchHeadUnsupportedError', () => {
it.each([
new Error("error: unknown option `no-write-fetch-head'"),
{ stderr: "error: unrecognized option '--no-write-fetch-head'" },
{ stdout: "fatal: invalid option: 'no-write-fetch-head'" }
])('recognizes old-Git option rejection shapes', (error) => {
expect(isNoWriteFetchHeadUnsupportedError(error)).toBe(true)
})
it('does not classify an ordinary fetch failure as unsupported', () => {
expect(
isNoWriteFetchHeadUnsupportedError({ stderr: 'fatal: could not read from remote repository' })
).toBe(false)
})
})
+16
View File
@@ -0,0 +1,16 @@
function gitErrorText(error: unknown): string {
if (typeof error !== 'object' || error === null) {
return error instanceof Error ? error.message : String(error)
}
return ['message', 'stderr', 'stdout']
.map((key) => (error as Record<string, unknown>)[key])
.filter((value): value is string => typeof value === 'string')
.join('\n')
}
export function isNoWriteFetchHeadUnsupportedError(error: unknown): boolean {
const output = gitErrorText(error)
return /(?:unknown|invalid|unrecognized) option(?::\s*|\s+)[`']?(?:--?)?no-write-fetch-head[`']?(?:\s|$)/i.test(
output
)
}
+145
View File
@@ -0,0 +1,145 @@
import { describe, expect, it } from 'vitest'
import { mkdtemp, mkdir, rm, symlink } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import * as path from 'node:path'
import { resolveGitFetchHeadCommand, runWithGitFetchHeadLock } from './git-fetch-head-lock'
describe('runWithGitFetchHeadLock', () => {
it.each([
{ args: ['fetch', 'origin'], expected: true },
{ args: ['pull', '--rebase'], expected: true },
{ args: ['-c', 'maintenance.auto=false', 'fetch', 'origin'], expected: true },
{ args: ['fetch', '--no-write-fetch-head', 'origin'], expected: false },
{ args: ['fetch', '--no-write-fetch-head', '--write-fetch-head'], expected: true },
{ args: ['rev-parse', 'fetch'], expected: false }
])('classifies FETCH_HEAD operations: $args', ({ args, expected }) => {
expect(resolveGitFetchHeadCommand(args, '/repo').needsLock).toBe(expected)
})
it('resolves -C and --git-dir before deriving the lock identity', () => {
expect(resolveGitFetchHeadCommand(['-C', 'repo', 'fetch'], '/tmp')).toMatchObject({
cwd: '/tmp/repo',
needsLock: true
})
expect(resolveGitFetchHeadCommand(['--git-dir=/repo/.git', 'fetch'], '/tmp')).toMatchObject({
gitDir: '/repo/.git',
needsLock: true
})
})
it('serializes FETCH_HEAD users in one worktree without blocking another', async () => {
const root = await mkdtemp(path.join(tmpdir(), 'fetch-head-lock-isolation-'))
const repo = path.join(root, 'repo')
const otherRepo = path.join(root, 'other')
await Promise.all([
mkdir(path.join(repo, '.git'), { recursive: true }),
mkdir(path.join(otherRepo, '.git'), { recursive: true })
])
let releaseFirst!: () => void
const firstGate = new Promise<void>((resolve) => {
releaseFirst = resolve
})
let markFirstStarted!: () => void
const firstStarted = new Promise<void>((resolve) => {
markFirstStarted = resolve
})
const order: string[] = []
const first = runWithGitFetchHeadLock(repo, undefined, async () => {
order.push('first:start')
markFirstStarted()
await firstGate
order.push('first:end')
})
await firstStarted
const second = runWithGitFetchHeadLock(repo, undefined, async () => {
order.push('second')
})
const other = runWithGitFetchHeadLock(otherRepo, undefined, async () => {
order.push('other')
})
try {
await other
expect(order).toEqual(['first:start', 'other'])
releaseFirst()
await Promise.all([first, second])
expect(order).toEqual(['first:start', 'other', 'first:end', 'second'])
} finally {
releaseFirst()
await Promise.allSettled([first, second, other])
await rm(root, { recursive: true, force: true })
}
})
it('does not run a queued operation after cancellation', async () => {
const root = await mkdtemp(path.join(tmpdir(), 'fetch-head-lock-cancel-'))
const repo = path.join(root, 'repo')
await mkdir(path.join(repo, '.git'), { recursive: true })
let releaseFirst!: () => void
const firstGate = new Promise<void>((resolve) => {
releaseFirst = resolve
})
let markFirstStarted!: () => void
const firstStarted = new Promise<void>((resolve) => {
markFirstStarted = resolve
})
const first = runWithGitFetchHeadLock(repo, undefined, async () => {
markFirstStarted()
await firstGate
})
await firstStarted
const controller = new AbortController()
const queued = runWithGitFetchHeadLock(repo, controller.signal, async () => 'ran')
try {
controller.abort()
await expect(queued).rejects.toMatchObject({ name: 'AbortError' })
} finally {
releaseFirst()
await Promise.allSettled([first, queued])
await rm(root, { recursive: true, force: true })
}
})
it('serializes root, nested, and symlink aliases of one worktree', async () => {
const root = await mkdtemp(path.join(tmpdir(), 'fetch-head-lock-'))
const repo = path.join(root, 'repo')
const nested = path.join(repo, 'nested')
const alias = path.join(root, 'alias')
await mkdir(path.join(repo, '.git'), { recursive: true })
await mkdir(nested)
await symlink(repo, alias)
let release!: () => void
const gate = new Promise<void>((resolve) => {
release = resolve
})
let markFirstStarted!: () => void
const firstStarted = new Promise<void>((resolve) => {
markFirstStarted = resolve
})
const order: string[] = []
try {
const first = runWithGitFetchHeadLock(repo, undefined, async () => {
order.push('root')
markFirstStarted()
await gate
})
await firstStarted
const nestedRun = runWithGitFetchHeadLock(nested, undefined, async () => {
order.push('nested')
})
const aliasRun = runWithGitFetchHeadLock(alias, undefined, async () => {
order.push('alias')
})
await new Promise((resolve) => setTimeout(resolve, 20))
expect(order).toEqual(['root'])
release()
await Promise.all([first, nestedRun, aliasRun])
expect(order[0]).toBe('root')
expect(new Set(order.slice(1))).toEqual(new Set(['nested', 'alias']))
} finally {
await rm(root, { recursive: true, force: true })
}
})
})
+171
View File
@@ -0,0 +1,171 @@
import * as path from 'node:path'
import { readFile, realpath, stat } from 'node:fs/promises'
type GitFetchHeadLockLane = {
tail: Promise<void>
release: () => void
}
const lanes = new Map<string, GitFetchHeadLockLane>()
const GLOBAL_OPTIONS_WITH_VALUE = new Set([
'-c',
'-C',
'--git-dir',
'--work-tree',
'--namespace',
'--super-prefix',
'--config-env',
'--exec-path'
])
type GitFetchHeadCommand = { needsLock: boolean; cwd: string; gitDir?: string }
export function resolveGitFetchHeadCommand(
args: readonly string[],
initialCwd: string
): GitFetchHeadCommand {
let cwd = initialCwd
let gitDir: string | undefined
let subcommandIndex = -1
for (let index = 0; index < args.length; index += 1) {
const arg = args[index]
if (arg === '-C' && args[index + 1]) {
cwd = path.resolve(cwd, args[index + 1])
index += 1
continue
}
if (arg.startsWith('-C') && arg.length > 2) {
cwd = path.resolve(cwd, arg.slice(2))
continue
}
if (arg === '--git-dir' && args[index + 1]) {
gitDir = path.resolve(cwd, args[index + 1])
index += 1
continue
}
if (arg.startsWith('--git-dir=')) {
gitDir = path.resolve(cwd, arg.slice('--git-dir='.length))
continue
}
if (GLOBAL_OPTIONS_WITH_VALUE.has(arg)) {
index += 1
continue
}
if (arg.startsWith('-')) {
continue
}
subcommandIndex = index
break
}
const subcommand = args[subcommandIndex]
if (subcommand === 'pull') {
return { needsLock: true, cwd, gitDir }
}
if (subcommand !== 'fetch') {
return { needsLock: false, cwd, gitDir }
}
let writesFetchHead = true
for (const arg of args.slice(subcommandIndex + 1)) {
if (arg === '--no-write-fetch-head') {
writesFetchHead = false
} else if (arg === '--write-fetch-head') {
writesFetchHead = true
}
}
return { needsLock: writesFetchHead, cwd, gitDir }
}
async function fetchHeadPath(
worktreePath: string,
signal: AbortSignal | undefined,
explicitGitDir?: string
): Promise<string> {
let current = await realpath(worktreePath).catch(() => path.resolve(worktreePath))
let gitDir = explicitGitDir
while (!gitDir) {
const dotGitPath = path.join(current, '.git')
try {
const metadata = await stat(dotGitPath)
if (metadata.isDirectory()) {
gitDir = dotGitPath
break
}
const contents = await readFile(dotGitPath, { encoding: 'utf-8', signal })
const match = contents.match(/^gitdir:\s*(.+)\s*$/m)
if (match) {
gitDir = path.resolve(current, match[1])
break
}
} catch {
if (signal?.aborted) {
throw abortError()
}
}
const parent = path.dirname(current)
if (parent === current) {
gitDir = path.join(current, '.git')
break
}
current = parent
}
const canonicalGitDir = await realpath(gitDir).catch(() => path.resolve(gitDir))
return path.join(canonicalGitDir, 'FETCH_HEAD')
}
function abortError(): Error {
const error = new Error('The operation was aborted.')
error.name = 'AbortError'
return error
}
async function waitForPredecessor(
predecessor: Promise<void>,
signal: AbortSignal | undefined
): Promise<void> {
if (!signal) {
await predecessor.catch(() => undefined)
return
}
if (signal.aborted) {
throw abortError()
}
let rejectAbort!: (error: Error) => void
const aborted = new Promise<never>((_resolve, reject) => {
rejectAbort = reject
})
const onAbort = () => rejectAbort(abortError())
signal.addEventListener('abort', onAbort, { once: true })
try {
await Promise.race([predecessor.catch(() => undefined), aborted])
} finally {
signal.removeEventListener('abort', onAbort)
}
}
export async function runWithGitFetchHeadLock<T>(
worktreePath: string,
signal: AbortSignal | undefined,
run: () => Promise<T>,
explicitGitDir?: string
): Promise<T> {
const key = await fetchHeadPath(worktreePath, signal, explicitGitDir)
const predecessor = lanes.get(key)?.tail ?? Promise.resolve()
let release!: () => void
const current = new Promise<void>((resolve) => {
release = resolve
})
const lane = { tail: predecessor.catch(() => undefined).then(() => current), release }
lanes.set(key, lane)
void lane.tail.then(() => {
if (lanes.get(key) === lane) {
lanes.delete(key)
}
})
try {
await waitForPredecessor(predecessor, signal)
return await run()
} finally {
lane.release()
}
}
+2
View File
@@ -1,5 +1,7 @@
// Why: a stalled remote must fail the rebase fetch, not hang the rebase; client and relay share one bound.
export const REBASE_SOURCE_FETCH_TIMEOUT_MS = 60_000
export const REBASE_FROM_BASE_OPERATION_TIMEOUT_MS = REBASE_SOURCE_FETCH_TIMEOUT_MS + 60_000
export const REBASE_FROM_BASE_RPC_TIMEOUT_MS = REBASE_FROM_BASE_OPERATION_TIMEOUT_MS + 5_000
export type GitCommandRunner = (args: string[]) => Promise<{ stdout: string }>