fix(git): close final admission gaps

This commit is contained in:
Brennan Benson
2026-08-27 16:27:42 -07:00
parent d1fb05a9a7
commit c9d290a299
11 changed files with 255 additions and 52 deletions
@@ -14,19 +14,17 @@ type WaiterLane = {
const TIERS = ['interactive', 'status', 'background'] as const
const createLane = (): WaiterLane => ({ items: [], head: 0, count: 0 })
type TierLanes = Record<GitAdmissionTier, Map<string | null, WaiterLane>>
const createTierLanes = (): TierLanes => ({
interactive: new Map(),
status: new Map(),
background: new Map()
})
export class GitAdmissionWaiterQueue {
private readonly lanes: Record<AdmissionClass, Record<GitAdmissionTier, WaiterLane>> = {
general: {
interactive: createLane(),
status: createLane(),
background: createLane()
},
network: {
interactive: createLane(),
status: createLane(),
background: createLane()
}
private readonly lanes: Record<AdmissionClass, TierLanes> = {
general: createTierLanes(),
network: createTierLanes()
}
private readonly countsByBudget = new Map<string, number>()
private totalCount = 0
@@ -36,7 +34,12 @@ export class GitAdmissionWaiterQueue {
}
enqueue(waiter: AdmissionWaiter): void {
const lane = this.lanes[waiter.admissionClass][waiter.tier]
const lanes = this.lanes[waiter.admissionClass][waiter.tier]
let lane = lanes.get(waiter.route)
if (!lane) {
lane = createLane()
lanes.set(waiter.route, lane)
}
lane.items.push(waiter)
lane.count += 1
this.totalCount += 1
@@ -46,7 +49,11 @@ export class GitAdmissionWaiterQueue {
}
dequeue(waiter: AdmissionWaiter): void {
const lane = this.lanes[waiter.admissionClass][waiter.tier]
const lanes = this.lanes[waiter.admissionClass][waiter.tier]
const lane = lanes.get(waiter.route)
if (!lane) {
return
}
lane.count -= 1
this.totalCount -= 1
for (const key of waiter.budgetKeys) {
@@ -58,6 +65,9 @@ export class GitAdmissionWaiterQueue {
}
}
this.compact(lane)
if (lane.count === 0) {
lanes.delete(waiter.route)
}
}
hasBudget(key: string): boolean {
@@ -68,8 +78,9 @@ export class GitAdmissionWaiterQueue {
return (['general', 'network'] as const)
.flatMap((admissionClass) =>
TIERS.flatMap((tier) => {
const lane = this.lanes[admissionClass][tier]
return lane.items.slice(lane.head).filter((waiter) => waiter.state === 'queued')
return [...this.lanes[admissionClass][tier].values()].flatMap((lane) =>
lane.items.slice(lane.head).filter((waiter) => waiter.state === 'queued')
)
})
)
.sort((left, right) => left.id - right.id)
@@ -83,29 +94,31 @@ export class GitAdmissionWaiterQueue {
): SelectedWaiter | null {
let selected: (SelectedWaiter & { tier: number }) | null = null
for (const rawTier of TIERS) {
const lane = this.lanes[admissionClass][rawTier]
for (let index = lane.head; index < lane.items.length; index += 1) {
const waiter = lane.items[index]
if (waiter.state !== 'queued') {
continue
for (const lane of this.lanes[admissionClass][rawTier].values()) {
for (let index = lane.head; index < lane.items.length; index += 1) {
const waiter = lane.items[index]
if (waiter.state !== 'queued') {
continue
}
if (waiter.signal?.aborted) {
abort(waiter)
continue
}
const slotKind = slotKindFor(waiter)
if (slotKind) {
const tier = effectiveTier(waiter)
if (
!selected ||
tier < selected.tier ||
(tier === selected.tier && waiter.id < selected.waiter.id)
) {
selected = { waiter, slotKind, tier }
}
}
// Every waiter in this route lane has the same budget shape, so a
// blocked head proves the rest cannot fit either.
break
}
if (waiter.signal?.aborted) {
abort(waiter)
continue
}
const slotKind = slotKindFor(waiter)
if (!slotKind) {
continue
}
const tier = effectiveTier(waiter)
if (
!selected ||
tier < selected.tier ||
(tier === selected.tier && waiter.id < selected.waiter.id)
) {
selected = { waiter, slotKind, tier }
}
break
}
}
return selected
@@ -4,6 +4,7 @@ import { execFileCapture } from './exec-file-capture'
import { resolveGitCommand } from './git-command-resolution'
import { DEFAULT_GIT_MAX_BUFFER, type GitExecOptions } from './git-exec-options'
import { promptGuardGitEnv } from './git-process-env'
import { acquireGitAdmission } from './git-subprocess-admission'
export type GitSshPolicyMode =
| 'default'
@@ -131,6 +132,18 @@ export async function buildNetworkSshPolicyEnv(options: GitExecOptions): Promise
// Why fenced: a login-shell banner here reads as a user-configured sshCommand,
// which skips the BatchMode fallback below and disarms the no-prompt guard.
const resolved = resolveGitCommand(['config', '--get', 'core.sshCommand'], options, true, true)
const probeArgs = ['config', '--get', 'core.sshCommand']
const grant = await acquireGitAdmission({
args: probeArgs,
cwd: options.cwd,
wslDistro: options.wslDistro,
tier: options.admissionTier,
signal: options.signal
})
let reportTerminated: () => void = () => {}
const terminated = new Promise<void>((resolve) => {
reportTerminated = resolve
})
let configuredCommand = ''
try {
const { stdout } = await execFileCapture(resolved.binary, resolved.args, {
@@ -139,12 +152,15 @@ export async function buildNetworkSshPolicyEnv(options: GitExecOptions): Promise
maxBuffer: DEFAULT_GIT_MAX_BUFFER,
timeout: CORE_SSH_COMMAND_PROBE_TIMEOUT_MS,
env: promptEnv,
signal: options.signal
signal: options.signal,
onChildTerminated: reportTerminated
})
const payload = resolved.captured?.readStdout(String(stdout)) ?? String(stdout)
configuredCommand = payload.trim()
} catch {
configuredCommand = ''
} finally {
void terminated.then(grant.release)
}
if (!configuredCommand) {
@@ -369,6 +369,48 @@ describe('GitAdmissionScheduler', () => {
expect(scheduler.snapshot()).toMatchObject({ queued: 0, queuedWaiters: [] })
})
it('skips a 4k saturated-route lane without rescanning its blocked prefix', async () => {
const scheduler = new GitAdmissionScheduler({
generalCap: 2,
generalHeadroom: 0,
routeCap: 1,
routeHeadroom: 0
})
const routedRunning = await scheduler.acquire({
...local('background'),
wslDistro: 'Ubuntu'
})
const localRunning = await scheduler.acquire(local('background'))
let abortedReads = 0
const signal = {
get aborted() {
abortedReads += 1
return false
},
addEventListener: () => {},
removeEventListener: () => {}
} as unknown as AbortSignal
const count = 4_000
const routed = Array.from({ length: count }, () =>
scheduler
.acquire({ ...local('background'), wslDistro: 'Ubuntu', signal })
.then((grant) => grant.release())
)
const localWork = Array.from({ length: count }, () =>
scheduler.acquire({ ...local('background'), signal }).then((grant) => grant.release())
)
abortedReads = 0
localRunning.release()
await Promise.all(localWork)
expect(scheduler.snapshot().queued).toBe(count)
routedRunning.release()
await Promise.all(routed)
expect(abortedReads).toBeLessThan(30_000)
expect(scheduler.snapshot()).toMatchObject({ queued: 0, queuedWaiters: [] })
})
it('captures killswitch state in each release closure', async () => {
const scheduler = schedulerWithOneSlot()
_resetGitAdmissionForTests(scheduler)
+34 -1
View File
@@ -22,7 +22,10 @@ import {
translateWslOutputPaths,
wslAwareSpawn
} from './runner'
import { _resetGitAdmissionForTests } from './command-runner/git-subprocess-admission'
import {
GitAdmissionScheduler,
_resetGitAdmissionForTests
} from './command-runner/git-subprocess-admission'
afterEach(() => _resetGitAdmissionForTests())
@@ -417,6 +420,36 @@ describe('runner execFile timeout handling', () => {
)
})
it('admits the core.sshCommand probe before spawning it', async () => {
const scheduler = new GitAdmissionScheduler({ generalCap: 1, generalHeadroom: 0 })
_resetGitAdmissionForTests(scheduler)
const blocker = await scheduler.acquire({ args: ['status'], cwd: '/repo', tier: 'status' })
const calls: string[][] = []
execFileMock.mockImplementation((_cmd, args, _opts, cb) => {
const child = createMockChildProcess(1234 + calls.length)
calls.push(args)
cb(null, '', '')
queueMicrotask(() => child.emit('close', 0, null))
return child
})
const pending = gitExecFileAsync(['fetch', '--no-write-fetch-head', 'origin'], {
cwd: '/repo',
env: {},
useConfiguredSshCommandForNetwork: true
})
await Promise.resolve()
expect(execFileMock).not.toHaveBeenCalled()
blocker.release()
await pending
expect(calls).toEqual([
['config', '--get', 'core.sshCommand'],
['fetch', '--no-write-fetch-head', 'origin']
])
})
it('replaces configured BatchMode for opted-in mergeable OpenSSH commands', async () => {
const child = createMockChildProcess(1234)
let capturedEnv: NodeJS.ProcessEnv | undefined
@@ -44,7 +44,10 @@ function oversizedResult(): GitDiffResult {
}
}
function commands(connectionId?: string): RuntimeGitCommands {
function commands(
connectionId?: string,
localGitOptions?: { wslDistro: string }
): RuntimeGitCommands {
const worktree = {
id: 'wt-1',
repoId: 'repo-1',
@@ -54,7 +57,8 @@ function commands(connectionId?: string): RuntimeGitCommands {
return new RuntimeGitCommands({
resolveRuntimeGitTarget: async () => ({
worktree,
...(connectionId ? { connectionId } : {})
...(connectionId ? { connectionId } : {}),
...(localGitOptions ? { localGitOptions } : {})
}),
getRuntimeSettings: () => ({}) as GlobalSettings
})
@@ -195,4 +199,23 @@ describe('runtime git diff transport budget', () => {
}
)
})
it('prioritizes local file diff reads without losing WSL routing', async () => {
const runtime = commands(undefined, { wslDistro: 'Ubuntu' })
await runtime.getRuntimeGitBranchDiff('id:wt-1', BRANCH_COMPARE, 'assets/logo.png')
await runtime.getRuntimeGitCommitDiff('id:wt-1', COMMIT_ARGS)
const options = { admissionTier: 'interactive', wslDistro: 'Ubuntu' }
expect(mocks.getBranchDiff).toHaveBeenLastCalledWith(
'/remote/repo',
expect.objectContaining({ filePath: 'assets/logo.png' }),
options
)
expect(mocks.getCommitDiff).toHaveBeenLastCalledWith(
'/remote/repo',
expect.objectContaining({ filePath: 'assets/logo.png' }),
options
)
})
})
+48 -2
View File
@@ -10,6 +10,7 @@ import { RuntimeGitCommands, type ResolvedRuntimeGitWorktree } from './orca-runt
const mocks = vi.hoisted(() => ({
checkoutBranch: vi.fn(),
discardChanges: vi.fn(),
listLocalBranches: vi.fn(),
getStagedCommitContext: vi.fn(),
getPullRequestDraftContext: vi.fn(),
@@ -24,6 +25,7 @@ const mocks = vi.hoisted(() => ({
vi.mock('../git/status', async () => ({
...(await vi.importActual<typeof GitStatusModule>('../git/status')),
discardChanges: mocks.discardChanges,
getStagedCommitContext: mocks.getStagedCommitContext,
getStatus: mocks.getStatus
}))
@@ -105,6 +107,7 @@ describe('RuntimeGitCommands', () => {
mocks.getSshGitProvider.mockReset()
mocks.getStatus.mockReset()
mocks.checkoutBranch.mockReset()
mocks.discardChanges.mockReset()
mocks.listLocalBranches.mockReset()
})
@@ -157,7 +160,13 @@ describe('RuntimeGitCommands', () => {
it('checks out a local branch through the resolved worktree', async () => {
const worktreePath = mkdtempSync(join(tmpdir(), 'orca-runtime-git-'))
tempDirs.push(worktreePath)
const commands = makeCommands(worktreePath)
const commands = new RuntimeGitCommands({
resolveRuntimeGitTarget: async () => ({
worktree: makeWorktree(worktreePath),
localGitOptions: { wslDistro: 'Ubuntu' }
}),
getRuntimeSettings: () => ({}) as GlobalSettings
})
mocks.checkoutBranch.mockResolvedValue(undefined)
await expect(commands.checkoutRuntimeGitBranch('id:wt-1', 'feature/x')).resolves.toEqual({
@@ -165,7 +174,10 @@ describe('RuntimeGitCommands', () => {
branch: 'feature/x'
})
expect(mocks.checkoutBranch).toHaveBeenCalledWith(worktreePath, 'feature/x', {})
expect(mocks.checkoutBranch).toHaveBeenCalledWith(worktreePath, 'feature/x', {
admissionTier: 'interactive',
wslDistro: 'Ubuntu'
})
})
it('checks out a remote branch through the SSH git provider', async () => {
@@ -202,6 +214,40 @@ describe('RuntimeGitCommands', () => {
expect(mocks.listLocalBranches).toHaveBeenCalledWith(worktreePath, {})
})
it('prioritizes a local single-file discard without losing WSL routing', async () => {
const commands = new RuntimeGitCommands({
resolveRuntimeGitTarget: async () => ({
worktree: makeWorktree('/workspace/repo'),
localGitOptions: { wslDistro: 'Ubuntu' }
}),
getRuntimeSettings: () => ({}) as GlobalSettings
})
await commands.discardRuntimeGitPath('id:wt-1', 'src/app.ts')
expect(mocks.discardChanges).toHaveBeenCalledWith('/workspace/repo', 'src/app.ts', {
admissionTier: 'interactive',
wslDistro: 'Ubuntu'
})
})
it('keeps a remote single-file discard owned by the SSH provider', async () => {
const provider = { discardChanges: vi.fn() }
mocks.getSshGitProvider.mockReturnValue(provider)
const commands = new RuntimeGitCommands({
resolveRuntimeGitTarget: async () => ({
worktree: makeWorktree('/remote/repo'),
connectionId: 'conn-1'
}),
getRuntimeSettings: () => ({}) as GlobalSettings
})
await commands.discardRuntimeGitPath('id:wt-1', 'src/app.ts')
expect(provider.discardChanges).toHaveBeenCalledWith('/remote/repo', 'src/app.ts')
expect(mocks.discardChanges).not.toHaveBeenCalled()
})
it('lists remote local branches through the SSH git provider', async () => {
const provider = {
listLocalBranches: vi.fn().mockResolvedValue({ current: 'main', branches: ['main'] })
@@ -133,7 +133,10 @@ export class RuntimeGitDiffCommands {
filePath: relativePath,
oldPath: oldRelativePath
},
localGitOptionsForTarget(target)
{
...localGitOptionsForTarget(target),
admissionTier: 'interactive'
}
),
maxContentBytes
)
@@ -171,7 +174,10 @@ export class RuntimeGitDiffCommands {
filePath: relativePath,
oldPath: oldRelativePath
},
localGitOptionsForTarget(target)
{
...localGitOptionsForTarget(target),
admissionTier: 'interactive'
}
),
maxContentBytes
)
@@ -129,7 +129,10 @@ export class RuntimeGitStagingCommands {
await provider.discardChanges(target.worktree.path, relativePath)
return { ok: true }
}
await discardChanges(target.worktree.path, relativePath, localGitOptionsForTarget(target))
await discardChanges(target.worktree.path, relativePath, {
...localGitOptionsForTarget(target),
admissionTier: 'interactive'
})
return { ok: true }
}
}
@@ -132,7 +132,10 @@ export class RuntimeGitStatusCommands {
await provider.checkoutBranch(target.worktree.path, branch)
return { ok: true, branch }
}
await checkoutBranch(target.worktree.path, branch, localGitOptionsForTarget(target))
await checkoutBranch(target.worktree.path, branch, {
...localGitOptionsForTarget(target),
admissionTier: 'interactive'
})
return { ok: true, branch }
}
@@ -3,6 +3,7 @@ import type { GlobalSettings } from '../../shared/global-settings-types'
import type { GitPushTarget } from '../../shared/worktree/types'
import type * as GitRemoteModule from '../git/remote'
import type * as GitStatusModule from '../git/status'
import type * as GitForkSyncModule from '../git/fork-sync'
import type { ResolvedRuntimeGitWorktree } from './runtime-git-command-target'
import { RuntimeGitSyncCommands } from './runtime-git-sync-commands'
@@ -11,11 +12,17 @@ const mocks = vi.hoisted(() => ({
abortRebase: vi.fn(),
commitChanges: vi.fn(),
getSshGitProvider: vi.fn(),
gitSyncForkDefaultBranch: vi.fn(),
gitFastForward: vi.fn(),
gitFetch: vi.fn(),
gitPull: vi.fn()
}))
vi.mock('../git/fork-sync', async () => ({
...(await vi.importActual<typeof GitForkSyncModule>('../git/fork-sync')),
gitSyncForkDefaultBranch: mocks.gitSyncForkDefaultBranch
}))
vi.mock('../git/status', async () => ({
...(await vi.importActual<typeof GitStatusModule>('../git/status')),
abortMerge: mocks.abortMerge,
@@ -42,6 +49,7 @@ const pushTarget = {
remoteName: 'origin',
branchName: 'main'
} satisfies GitPushTarget
const expectedUpstream = { owner: 'stablyai', repo: 'orca' }
describe('RuntimeGitSyncCommands admission', () => {
beforeEach(() => {
@@ -57,10 +65,12 @@ describe('RuntimeGitSyncCommands admission', () => {
getRuntimeSettings: () => ({}) as GlobalSettings
})
mocks.commitChanges.mockResolvedValue({ success: true })
mocks.gitSyncForkDefaultBranch.mockResolvedValue({ status: 'up-to-date' })
await commands.abortRuntimeGitMerge('id:wt-1')
await commands.abortRuntimeGitRebase('id:wt-1')
await commands.fetchRuntimeGit('id:wt-1', pushTarget)
await commands.syncRuntimeGitForkDefaultBranch('id:wt-1', expectedUpstream)
await commands.pullRuntimeGit('id:wt-1', pushTarget)
await commands.fastForwardRuntimeGit('id:wt-1', pushTarget)
await commands.commitRuntimeGit('id:wt-1', 'feat: prioritize user action')
@@ -69,6 +79,11 @@ describe('RuntimeGitSyncCommands admission', () => {
expect(mocks.abortMerge).toHaveBeenCalledWith(worktree.path, options)
expect(mocks.abortRebase).toHaveBeenCalledWith(worktree.path, options)
expect(mocks.gitFetch).toHaveBeenCalledWith(worktree.path, pushTarget, options)
expect(mocks.gitSyncForkDefaultBranch).toHaveBeenCalledWith(
worktree.path,
expectedUpstream,
options
)
expect(mocks.gitPull).toHaveBeenCalledWith(worktree.path, pushTarget, options)
expect(mocks.gitFastForward).toHaveBeenCalledWith(worktree.path, pushTarget, options)
expect(mocks.commitChanges).toHaveBeenCalledWith(
@@ -85,6 +100,7 @@ describe('RuntimeGitSyncCommands admission', () => {
commit: vi.fn().mockResolvedValue({ success: true }),
fastForwardBranch: vi.fn(),
fetchRemote: vi.fn(),
syncForkDefaultBranch: vi.fn().mockResolvedValue({ status: 'up-to-date' }),
pullBranch: vi.fn()
}
mocks.getSshGitProvider.mockReturnValue(provider)
@@ -99,6 +115,7 @@ describe('RuntimeGitSyncCommands admission', () => {
await commands.abortRuntimeGitMerge('id:wt-1')
await commands.abortRuntimeGitRebase('id:wt-1')
await commands.fetchRuntimeGit('id:wt-1', pushTarget)
await commands.syncRuntimeGitForkDefaultBranch('id:wt-1', expectedUpstream)
await commands.pullRuntimeGit('id:wt-1', pushTarget)
await commands.fastForwardRuntimeGit('id:wt-1', pushTarget)
await commands.commitRuntimeGit('id:wt-1', 'feat: keep execution remote')
@@ -106,12 +123,14 @@ describe('RuntimeGitSyncCommands admission', () => {
expect(provider.abortMerge).toHaveBeenCalledWith(worktree.path)
expect(provider.abortRebase).toHaveBeenCalledWith(worktree.path)
expect(provider.fetchRemote).toHaveBeenCalledWith(worktree.path, pushTarget)
expect(provider.syncForkDefaultBranch).toHaveBeenCalledWith(worktree.path, expectedUpstream)
expect(provider.pullBranch).toHaveBeenCalledWith(worktree.path, pushTarget)
expect(provider.fastForwardBranch).toHaveBeenCalledWith(worktree.path, pushTarget)
expect(provider.commit).toHaveBeenCalledWith(worktree.path, 'feat: keep execution remote')
expect(mocks.abortMerge).not.toHaveBeenCalled()
expect(mocks.abortRebase).not.toHaveBeenCalled()
expect(mocks.gitFetch).not.toHaveBeenCalled()
expect(mocks.gitSyncForkDefaultBranch).not.toHaveBeenCalled()
expect(mocks.gitPull).not.toHaveBeenCalled()
expect(mocks.gitFastForward).not.toHaveBeenCalled()
expect(mocks.commitChanges).not.toHaveBeenCalled()
@@ -95,11 +95,10 @@ export class RuntimeGitSyncCommands {
}
return provider.syncForkDefaultBranch(target.worktree.path, expectedUpstream)
}
return gitSyncForkDefaultBranch(
target.worktree.path,
expectedUpstream,
localGitOptionsForTarget(target)
)
return gitSyncForkDefaultBranch(target.worktree.path, expectedUpstream, {
...localGitOptionsForTarget(target),
admissionTier: 'interactive'
})
}
async pullRuntimeGit(