From 222718cf46673bb68f4081be98405428678320eb Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:54:10 -0700 Subject: [PATCH] perf(git): bound eligible route selection --- .../git-admission-waiter-queue.ts | 191 +++++++++++++++--- .../git-subprocess-admission.test.ts | 46 +++++ .../git-subprocess-admission.ts | 17 +- ...list-files-git-directory-expansion.test.ts | 8 +- .../ipc/filesystem-list-files-git-fallback.ts | 1 + src/main/ipc/filesystem-search-git.test.ts | 38 ++-- src/main/ipc/filesystem-search-git.ts | 1 + .../ipc/repos-local-clone-lifecycle.test.ts | 1 + src/main/ipc/repos/repo-clone-lifecycle.ts | 1 + src/main/runtime/orca-runtime-git.test.ts | 1 + src/main/runtime/orca-runtime.test.ts | 4 + src/main/runtime/orca-runtime.ts | 1 + .../runtime-git-generation-admission.test.ts | 159 +++++++++++++++ .../runtime-git-generation-commands.ts | 15 +- .../pull-request-linked-issue.ts | 3 +- 15 files changed, 434 insertions(+), 53 deletions(-) create mode 100644 src/main/runtime/runtime-git-generation-admission.test.ts diff --git a/src/main/git/command-runner/git-admission-waiter-queue.ts b/src/main/git/command-runner/git-admission-waiter-queue.ts index 52eec4ee655..daf3fbbb253 100644 --- a/src/main/git/command-runner/git-admission-waiter-queue.ts +++ b/src/main/git/command-runner/git-admission-waiter-queue.ts @@ -10,16 +10,78 @@ type WaiterLane = { items: AdmissionWaiter[] head: number count: number + baseEligible: boolean + headroomEligible: boolean + version: number } const TIERS = ['interactive', 'status', 'background'] as const -const createLane = (): WaiterLane => ({ items: [], head: 0, count: 0 }) +const createLane = (): WaiterLane => ({ + items: [], + head: 0, + count: 0, + baseEligible: false, + headroomEligible: false, + version: 0 +}) type TierLanes = Record> const createTierLanes = (): TierLanes => ({ interactive: new Map(), status: new Map(), background: new Map() }) +type Candidate = { lane: WaiterLane; waiter: AdmissionWaiter; version: number } + +class CandidateHeap { + private readonly items: Candidate[] = [] + + push(candidate: Candidate): void { + this.items.push(candidate) + let index = this.items.length - 1 + while (index > 0) { + const parent = Math.floor((index - 1) / 2) + if (this.items[parent].waiter.id <= candidate.waiter.id) { + break + } + this.items[index] = this.items[parent] + index = parent + } + this.items[index] = candidate + } + + peek(valid: (candidate: Candidate) => boolean): Candidate | null { + // Eligibility changes invalidate by version so route updates stay O(log N). + while (this.items.length > 0 && !valid(this.items[0])) { + this.pop() + } + return this.items[0] ?? null + } + + private pop(): void { + const last = this.items.pop() + if (!last || this.items.length === 0) { + return + } + let index = 0 + while (true) { + const left = index * 2 + 1 + if (left >= this.items.length) { + break + } + const right = left + 1 + const child = + right < this.items.length && this.items[right].waiter.id < this.items[left].waiter.id + ? right + : left + if (this.items[child].waiter.id >= last.waiter.id) { + break + } + this.items[index] = this.items[child] + index = child + } + this.items[index] = last + } +} export class GitAdmissionWaiterQueue { private readonly lanes: Record = { @@ -27,6 +89,23 @@ export class GitAdmissionWaiterQueue { network: createTierLanes() } private readonly countsByBudget = new Map() + private readonly baseCandidates: Record> = + { + general: { + interactive: new CandidateHeap(), + status: new CandidateHeap(), + background: new CandidateHeap() + }, + network: { + interactive: new CandidateHeap(), + status: new CandidateHeap(), + background: new CandidateHeap() + } + } + private readonly headroomCandidates: Record = { + general: new CandidateHeap(), + network: new CandidateHeap() + } private totalCount = 0 get count(): number { @@ -46,6 +125,9 @@ export class GitAdmissionWaiterQueue { for (const key of waiter.budgetKeys) { this.countsByBudget.set(key, (this.countsByBudget.get(key) ?? 0) + 1) } + if (lane.count === 1) { + this.publishLaneHead(waiter.admissionClass, waiter.tier, lane) + } } dequeue(waiter: AdmissionWaiter): void { @@ -64,9 +146,35 @@ export class GitAdmissionWaiterQueue { this.countsByBudget.set(key, current - 1) } } + const previousHead = lane.items[lane.head] this.compact(lane) if (lane.count === 0) { + lane.version += 1 lanes.delete(waiter.route) + } else if (lane.items[lane.head] !== previousHead) { + lane.version += 1 + this.publishLaneHead(waiter.admissionClass, waiter.tier, lane) + } + } + + updateRouteEligibility( + admissionClass: AdmissionClass, + route: string | null, + baseEligible: boolean, + headroomEligible: boolean + ): void { + for (const tier of TIERS) { + const lane = this.lanes[admissionClass][tier].get(route) + if ( + !lane || + (lane.baseEligible === baseEligible && lane.headroomEligible === headroomEligible) + ) { + continue + } + lane.baseEligible = baseEligible + lane.headroomEligible = headroomEligible + lane.version += 1 + this.publishLaneHead(admissionClass, tier, lane) } } @@ -89,39 +197,66 @@ export class GitAdmissionWaiterQueue { nextFitting( admissionClass: AdmissionClass, effectiveTier: (waiter: AdmissionWaiter) => number, - slotKindFor: (waiter: AdmissionWaiter) => AdmissionSlotKind | null, + allowBase: boolean, + allowHeadroom: boolean, abort: (waiter: AdmissionWaiter) => void ): SelectedWaiter | null { - let selected: (SelectedWaiter & { tier: number }) | null = null - for (const rawTier of TIERS) { - 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 + while (true) { + const candidates: SelectedWaiter[] = [] + if (allowBase) { + // Aging preserves order within a raw tier, so only its oldest eligible head can win. + for (const tier of TIERS) { + const candidate = this.peekValid(this.baseCandidates[admissionClass][tier], 'base') + if (candidate) { + candidates.push({ waiter: candidate.waiter, slotKind: 'base' }) } - 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 (allowHeadroom) { + const candidate = this.peekValid(this.headroomCandidates[admissionClass], 'headroom') + if (candidate) { + candidates.push({ waiter: candidate.waiter, slotKind: 'headroom' }) + } + } + const selected = candidates.sort( + (left, right) => + effectiveTier(left.waiter) - effectiveTier(right.waiter) || + left.waiter.id - right.waiter.id || + (left.slotKind === 'base' ? -1 : 1) + )[0] + if (!selected || !selected.waiter.signal?.aborted) { + return selected ?? null + } + abort(selected.waiter) } - return selected + } + + private publishLaneHead( + admissionClass: AdmissionClass, + tier: GitAdmissionTier, + lane: WaiterLane + ): void { + const waiter = lane.items[lane.head] + if (!waiter || waiter.state !== 'queued') { + return + } + const candidate = { lane, waiter, version: lane.version } + if (lane.baseEligible) { + this.baseCandidates[admissionClass][tier].push(candidate) + } + if (tier === 'interactive' && lane.headroomEligible) { + this.headroomCandidates[admissionClass].push(candidate) + } + } + + private peekValid(heap: CandidateHeap, slotKind: AdmissionSlotKind): Candidate | null { + return heap.peek( + ({ lane, waiter, version }) => + version === lane.version && + waiter === lane.items[lane.head] && + waiter.state === 'queued' && + (slotKind === 'base' ? lane.baseEligible : lane.headroomEligible) + ) } private compact(lane: WaiterLane): void { diff --git a/src/main/git/command-runner/git-subprocess-admission.test.ts b/src/main/git/command-runner/git-subprocess-admission.test.ts index 66a9e69f05d..a66f434fad0 100644 --- a/src/main/git/command-runner/git-subprocess-admission.test.ts +++ b/src/main/git/command-runner/git-subprocess-admission.test.ts @@ -411,6 +411,52 @@ describe('GitAdmissionScheduler', () => { expect(scheduler.snapshot()).toMatchObject({ queued: 0, queuedWaiters: [] }) }) + it('selects one eligible route without scanning thousands of saturated routes', async () => { + const routeCount = 2_000 + const scheduler = new GitAdmissionScheduler({ + generalCap: routeCount, + generalHeadroom: 0, + routeCap: 1, + routeHeadroom: 0 + }) + const running = await Promise.all( + Array.from({ length: routeCount }, (_, index) => + scheduler.acquire({ ...local('background'), wslDistro: `distro-${index}` }) + ) + ) + let aborted = false + let abortedReads = 0 + const abortListeners = new Set<() => void>() + const signal = { + get aborted() { + abortedReads += 1 + return aborted + }, + addEventListener: (_event: string, listener: () => void) => abortListeners.add(listener), + removeEventListener: (_event: string, listener: () => void) => abortListeners.delete(listener) + } as unknown as AbortSignal + const queued = Array.from({ length: routeCount }, (_, index) => + scheduler + .acquire({ ...local('background'), wslDistro: `distro-${index}`, signal }) + .then((grant) => grant.release()) + ) + abortedReads = 0 + + running.at(-1)?.release() + await queued.at(-1) + + expect(abortedReads).toBeLessThanOrEqual(2) + expect(scheduler.snapshot().queued).toBe(routeCount - 1) + + aborted = true + for (const listener of abortListeners) { + listener() + } + await Promise.allSettled(queued.slice(0, -1)) + running.slice(0, -1).forEach((grant) => grant.release()) + expect(scheduler.snapshot()).toMatchObject({ queued: 0, queuedWaiters: [] }) + }) + it('captures killswitch state in each release closure', async () => { const scheduler = schedulerWithOneSlot() _resetGitAdmissionForTests(scheduler) diff --git a/src/main/git/command-runner/git-subprocess-admission.ts b/src/main/git/command-runner/git-subprocess-admission.ts index 290894af0c2..3688b599a9b 100644 --- a/src/main/git/command-runner/git-subprocess-admission.ts +++ b/src/main/git/command-runner/git-subprocess-admission.ts @@ -74,6 +74,7 @@ export class GitAdmissionScheduler { onAbort: () => this.abort(waiter) } this.waiters.enqueue(waiter) + this.refreshRouteEligibility(admissionClass, route) request.signal?.addEventListener('abort', waiter.onAbort, { once: true }) if (request.signal?.aborted) { this.abort(waiter) @@ -172,10 +173,12 @@ export class GitAdmissionScheduler { private drain(admissionClass: AdmissionClass): void { while (true) { const now = this.config.now() + const globalBudget = this.ensureBudget(admissionClass) const selected = this.waiters.nextFitting( admissionClass, (waiter) => this.effectiveTier(waiter, now), - (waiter) => this.slotKindFor(waiter), + globalBudget.baseUsed < globalBudget.baseCapacity, + globalBudget.headroomUsed < globalBudget.headroomCapacity, (waiter) => this.abort(waiter) ) if (!selected) { @@ -196,6 +199,7 @@ export class GitAdmissionScheduler { budget.headroomUsed += 1 } } + this.refreshRouteEligibility(waiter.admissionClass, waiter.route) this.waiters.dequeue(waiter) const queueWaitMs = Math.max(0, now - waiter.enqueuedAt) this.publishEvent(waiter, slotKind, 'grant', queueWaitMs) @@ -231,6 +235,7 @@ export class GitAdmissionScheduler { budget.headroomUsed -= 1 } } + this.refreshRouteEligibility(waiter.admissionClass, waiter.route) this.publishEvent(waiter, slotKind, 'release', queueWaitMs) this.pruneRouteBudgets(waiter.budgetKeys) this.drain(waiter.admissionClass) @@ -288,6 +293,16 @@ export class GitAdmissionScheduler { } } } + + private refreshRouteEligibility(admissionClass: AdmissionClass, route: string | null): void { + const budget = route ? this.ensureBudget(`route:${admissionClass}:${route}`) : null + this.waiters.updateRouteEligibility( + admissionClass, + route, + !budget || budget.baseUsed < budget.baseCapacity, + !budget || budget.headroomUsed < budget.headroomCapacity + ) + } } let scheduler = new GitAdmissionScheduler() diff --git a/src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts b/src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts index 5d7507a5615..8d3601d5baa 100644 --- a/src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts +++ b/src/main/ipc/filesystem-list-files-git-directory-expansion.test.ts @@ -60,7 +60,7 @@ describe('main Quick Open git directory expansion', () => { .mockResolvedValueOnce(primary) .mockResolvedValueOnce(ignored) - const promise = listFilesWithGit(root, [], {}) + const promise = listFilesWithGit(root, [], { wslDistro: 'Ubuntu' }) await vi.waitFor(() => expect(gitSpawnMock).toHaveBeenCalledTimes(1)) await vi.waitFor(() => expect(revParse.listenerCount('close')).toBeGreaterThan(0)) revParse.emit('close', 0, null) @@ -78,6 +78,12 @@ describe('main Quick Open git directory expansion', () => { 'scratch/notes.txt', 'src/index.ts' ]) + expect(gitSpawnMock.mock.calls[1][1]).toEqual( + expect.objectContaining({ admissionTier: 'interactive', wslDistro: 'Ubuntu' }) + ) + expect(gitSpawnMock.mock.calls[2][1]).toEqual( + expect.objectContaining({ admissionTier: 'interactive', wslDistro: 'Ubuntu' }) + ) expect(gitSpawnMock.mock.calls[2][0]).toContain('--directory') }) diff --git a/src/main/ipc/filesystem-list-files-git-fallback.ts b/src/main/ipc/filesystem-list-files-git-fallback.ts index bebab27808d..28cee2a4d50 100644 --- a/src/main/ipc/filesystem-list-files-git-fallback.ts +++ b/src/main/ipc/filesystem-list-files-git-fallback.ts @@ -132,6 +132,7 @@ export async function listFilesWithGit( // rootPath and use the output directly — no prefix stripping needed. const child = await gitSpawnAfterWindowsEnvironmentReady(['ls-files', ...args], { cwd: rootPath, + admissionTier: 'interactive', ...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {}), signal: scanController.signal, stdio: ['ignore', 'pipe', 'pipe'] diff --git a/src/main/ipc/filesystem-search-git.test.ts b/src/main/ipc/filesystem-search-git.test.ts index ae73b0aa2e2..7de098199f6 100644 --- a/src/main/ipc/filesystem-search-git.test.ts +++ b/src/main/ipc/filesystem-search-git.test.ts @@ -1,15 +1,11 @@ import { describe, expect, it, vi, beforeEach } from 'vitest' -const { spawnMock } = vi.hoisted(() => ({ - spawnMock: vi.fn() +const { gitSpawnMock } = vi.hoisted(() => ({ + gitSpawnMock: vi.fn() })) -vi.mock('child_process', () => ({ - spawn: spawnMock, - // runner.ts imports these from child_process; stubs prevent - // "missing export" errors when the mock is resolved transitively. - execFile: vi.fn(), - execFileSync: vi.fn() +vi.mock('../git/runner', () => ({ + gitSpawnAfterWindowsEnvironmentReady: gitSpawnMock })) import { searchWithGitGrep } from './filesystem-search-git' @@ -36,7 +32,7 @@ describe('filesystem-search-git', () => { it('parses git grep output and finds matches', async () => { const proc = createMockProcess() - spawnMock.mockReturnValue(proc) + gitSpawnMock.mockReturnValue(proc) const promise = searchWithGitGrep('/mock/root', { query: 'hello', rootPath: '/mock/root' }, 100) @@ -74,7 +70,7 @@ describe('filesystem-search-git', () => { it('finds multiple matches per line', async () => { const proc = createMockProcess() - spawnMock.mockReturnValue(proc) + gitSpawnMock.mockReturnValue(proc) const promise = searchWithGitGrep('/mock/root', { query: 'ab', rootPath: '/mock/root' }, 100) @@ -97,7 +93,7 @@ describe('filesystem-search-git', () => { it('respects maxResults and sets truncated', async () => { const proc = createMockProcess() - spawnMock.mockReturnValue(proc) + gitSpawnMock.mockReturnValue(proc) const promise = searchWithGitGrep('/mock/root', { query: 'x', rootPath: '/mock/root' }, 2) @@ -118,26 +114,30 @@ describe('filesystem-search-git', () => { it('passes correct flags for case-insensitive fixed-string search', async () => { const proc = createMockProcess() - spawnMock.mockReturnValue(proc) + gitSpawnMock.mockReturnValue(proc) const promise = searchWithGitGrep( '/mock/root', { query: 'test', rootPath: '/mock/root', caseSensitive: false, useRegex: false }, - 100 + 100, + { wslDistro: 'Ubuntu' } ) setTimeout(() => proc.emit('close'), 10) await promise - const gitArgs = spawnMock.mock.calls[0][1] as string[] + const gitArgs = gitSpawnMock.mock.calls[0][0] as string[] expect(gitArgs).toContain('-i') expect(gitArgs).toContain('--fixed-strings') expect(gitArgs).not.toContain('--extended-regexp') + expect(gitSpawnMock.mock.calls[0][1]).toEqual( + expect.objectContaining({ admissionTier: 'interactive', wslDistro: 'Ubuntu' }) + ) }) it('passes correct flags for regex whole-word search', async () => { const proc = createMockProcess() - spawnMock.mockReturnValue(proc) + gitSpawnMock.mockReturnValue(proc) const promise = searchWithGitGrep( '/mock/root', @@ -154,7 +154,7 @@ describe('filesystem-search-git', () => { setTimeout(() => proc.emit('close'), 10) await promise - const gitArgs = spawnMock.mock.calls[0][1] as string[] + const gitArgs = gitSpawnMock.mock.calls[0][0] as string[] expect(gitArgs).toContain('-w') expect(gitArgs).toContain('--extended-regexp') expect(gitArgs).not.toContain('-i') @@ -163,7 +163,7 @@ describe('filesystem-search-git', () => { it('returns empty result when git grep spawn fails', async () => { const proc = createMockProcess() - spawnMock.mockReturnValue(proc) + gitSpawnMock.mockReturnValue(proc) const promise = searchWithGitGrep('/mock/root', { query: 'test', rootPath: '/mock/root' }, 100) @@ -180,7 +180,7 @@ describe('filesystem-search-git', () => { try { const proc = createMockProcess() - spawnMock.mockReturnValue(proc) + gitSpawnMock.mockReturnValue(proc) const promise = searchWithGitGrep('/mock/root', { query: 'ok', rootPath: '/mock/root' }, 100) @@ -207,7 +207,7 @@ describe('filesystem-search-git', () => { it('skips lines without null separator', async () => { const proc = createMockProcess() - spawnMock.mockReturnValue(proc) + gitSpawnMock.mockReturnValue(proc) const promise = searchWithGitGrep('/mock/root', { query: 'ok', rootPath: '/mock/root' }, 100) diff --git a/src/main/ipc/filesystem-search-git.ts b/src/main/ipc/filesystem-search-git.ts index 0bff6336f66..da54799cad1 100644 --- a/src/main/ipc/filesystem-search-git.ts +++ b/src/main/ipc/filesystem-search-git.ts @@ -32,6 +32,7 @@ export async function searchWithGitGrep( const gitArgs = buildGitGrepArgs(args.query, args) const child = await gitSpawnAfterWindowsEnvironmentReady(gitArgs, { cwd: rootPath, + admissionTier: 'interactive', ...(localGitOptions.wslDistro ? { wslDistro: localGitOptions.wslDistro } : {}), stdio: ['ignore', 'pipe', 'pipe'] }) diff --git a/src/main/ipc/repos-local-clone-lifecycle.test.ts b/src/main/ipc/repos-local-clone-lifecycle.test.ts index b6f851590ba..395bfec8001 100644 --- a/src/main/ipc/repos-local-clone-lifecycle.test.ts +++ b/src/main/ipc/repos-local-clone-lifecycle.test.ts @@ -249,6 +249,7 @@ describe('repos:add + repos:clone', () => { expect(gitSpawnMock).toHaveBeenCalledWith( ['clone', '--progress', '--', 'https://example.com/orca.git', join(destination, 'orca')], expect.objectContaining({ + admissionTier: 'interactive', env: expect.objectContaining({ GIT_TERMINAL_PROMPT: '0', GCM_INTERACTIVE: 'never' diff --git a/src/main/ipc/repos/repo-clone-lifecycle.ts b/src/main/ipc/repos/repo-clone-lifecycle.ts index 19ce962057b..920c231aa7b 100644 --- a/src/main/ipc/repos/repo-clone-lifecycle.ts +++ b/src/main/ipc/repos/repo-clone-lifecycle.ts @@ -145,6 +145,7 @@ export function registerRepoCloneHandlers(mainWindow: BrowserWindow, store: Stor ['clone', '--progress', '--', args.url, clonePath], { cwd: args.destination, + admissionTier: 'interactive', // Why: without this, an auth-needing clone pops Git Credential Manager's OAuth window on Windows, unclosable in a restricted env (issue #7652). env: nonInteractiveGitEnv(), signal: pendingController.signal, diff --git a/src/main/runtime/orca-runtime-git.test.ts b/src/main/runtime/orca-runtime-git.test.ts index 2cf98bc84cd..897a8ad9530 100644 --- a/src/main/runtime/orca-runtime-git.test.ts +++ b/src/main/runtime/orca-runtime-git.test.ts @@ -371,6 +371,7 @@ describe('RuntimeGitCommands', () => { }) expect(mocks.getStagedCommitContext).toHaveBeenCalledWith(worktreePath, { + admissionTier: 'interactive', wslDistro: 'Ubuntu' }) expect(prepareForCodexLaunch).toHaveBeenCalledWith({ diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 850c9acbb3a..dce88d3564f 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -9295,6 +9295,10 @@ describe('OrcaRuntimeService', () => { try { const repo = await runtime.cloneRepo('https://example.com/repo-badge-color.git', '/tmp') + expect(spawnSpy).toHaveBeenCalledWith( + expect.arrayContaining(['clone']), + expect.objectContaining({ admissionTier: 'interactive' }) + ) expect(repo.badgeColor).toBe(DEFAULT_REPO_BADGE_COLOR) expect(added).toEqual([ expect.objectContaining({ diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index a73d6b6eb42..a38452eb3c8 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -22194,6 +22194,7 @@ export class OrcaRuntimeService { ['clone', '--progress', '--', trimmedUrl, clonePath], { cwd: trimmedDestination, + admissionTier: 'interactive', // Why: without the non-interactive guard, a clone that needs GitHub // auth makes Git Credential Manager pop its "Connect to GitHub" OAuth // window on Windows; in a network-restricted env the browser/device diff --git a/src/main/runtime/runtime-git-generation-admission.test.ts b/src/main/runtime/runtime-git-generation-admission.test.ts new file mode 100644 index 00000000000..2569ae47749 --- /dev/null +++ b/src/main/runtime/runtime-git-generation-admission.test.ts @@ -0,0 +1,159 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { GlobalSettings } from '../../shared/global-settings-types' +import type { RuntimeGitCommandHost, RuntimeGitTarget } from './runtime-git-command-target' + +const mocks = vi.hoisted(() => ({ + getStagedCommitContext: vi.fn(), + gitExecFileAsync: vi.fn(), + getPullRequestDraftContext: vi.fn(), + loadPullRequestLinkedIssue: vi.fn(), + getSshGitProvider: vi.fn(), + prepareLocalCommitMessageAgentEnv: vi.fn(), + generateCommitMessageFromContext: vi.fn(), + generatePullRequestFieldsFromContext: vi.fn(), + resolveHostedReviewBodyForGeneration: vi.fn() +})) + +vi.mock('../git/status', () => ({ getStagedCommitContext: mocks.getStagedCommitContext })) +vi.mock('../git/runner', () => ({ gitExecFileAsync: mocks.gitExecFileAsync })) +vi.mock('../text-generation/pull-request-context', () => ({ + getPullRequestDraftContext: mocks.getPullRequestDraftContext +})) +vi.mock('../source-control/pull-request-linked-issue', () => ({ + loadPullRequestLinkedIssue: mocks.loadPullRequestLinkedIssue +})) +vi.mock('../providers/ssh-git-dispatch', () => ({ + getSshGitProvider: mocks.getSshGitProvider, + SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE: 'provider unavailable' +})) +vi.mock('../text-generation/commit-message-agent-environment', () => ({ + prepareLocalCommitMessageAgentEnv: mocks.prepareLocalCommitMessageAgentEnv +})) +vi.mock('../text-generation/commit-message-text-generation', () => ({ + generateCommitMessageFromContext: mocks.generateCommitMessageFromContext, + generatePullRequestFieldsFromContext: mocks.generatePullRequestFieldsFromContext +})) +vi.mock('../source-control/pull-request-template', () => ({ + resolveHostedReviewBodyForGeneration: mocks.resolveHostedReviewBodyForGeneration +})) + +import { RuntimeGitGenerationCommands } from './runtime-git-generation-commands' + +const params = { agentId: 'codex' as const, model: 'gpt-5.5' } +const settingsOverride = { sourceControlAiResolvedParams: params } +const pullRequestContext = { + base: 'main', + branch: 'feature/admission', + commitSummary: 'abc123 feat: test', + changeSummary: 'M README.md', + patch: '+hello', + currentTitle: '', + currentBody: '', + currentDraft: false +} + +function makeTarget(path: string, overrides: Partial = {}): RuntimeGitTarget { + return { + worktree: { + id: 'wt-1', + repoId: 'repo-1', + path, + git: { path, branch: 'main', isBare: false, isMainWorktree: false, head: 'a'.repeat(40) } + } as RuntimeGitTarget['worktree'], + ...overrides + } +} + +function makeCommands(target: RuntimeGitTarget): RuntimeGitGenerationCommands { + const host: RuntimeGitCommandHost = { + resolveRuntimeGitTarget: async () => target, + getRuntimeSettings: () => ({}) as GlobalSettings + } + return new RuntimeGitGenerationCommands(host) +} + +describe('RuntimeGitGenerationCommands admission', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.gitExecFileAsync.mockResolvedValue({ stdout: '', stderr: '' }) + mocks.loadPullRequestLinkedIssue.mockResolvedValue(null) + mocks.resolveHostedReviewBodyForGeneration.mockImplementation(async ({ body }) => body) + mocks.prepareLocalCommitMessageAgentEnv.mockResolvedValue({ ok: true, env: {} }) + mocks.generateCommitMessageFromContext.mockResolvedValue({ success: true, message: 'feat' }) + mocks.generatePullRequestFieldsFromContext.mockResolvedValue({ success: true, fields: {} }) + }) + + it('marks local staged commit context as interactive without dropping WSL routing', async () => { + mocks.getStagedCommitContext.mockResolvedValue({ + branch: 'main', + stagedSummary: 'M README.md', + stagedPatch: '+hello' + }) + const commands = makeCommands( + makeTarget('C:\\repo', { localGitOptions: { wslDistro: 'Ubuntu' } }) + ) + + await commands.generateRuntimeCommitMessage('id:wt-1', settingsOverride) + + expect(mocks.getStagedCommitContext).toHaveBeenCalledWith('C:\\repo', { + wslDistro: 'Ubuntu', + admissionTier: 'interactive' + }) + }) + + it('marks local pull-request context and linked-issue reads as interactive', async () => { + mocks.getPullRequestDraftContext.mockImplementation(async (execute) => { + await execute(['fetch', 'origin', 'main'], { timeout: 123 }) + return pullRequestContext + }) + const commands = makeCommands( + makeTarget('C:\\repo', { localGitOptions: { wslDistro: 'Ubuntu' } }) + ) + + await commands.generateRuntimePullRequestFields( + 'id:wt-1', + { base: 'main', title: '', body: '', draft: false }, + settingsOverride + ) + + expect(mocks.gitExecFileAsync).toHaveBeenCalledWith(['fetch', 'origin', 'main'], { + cwd: 'C:\\repo', + wslDistro: 'Ubuntu', + timeout: 123, + admissionTier: 'interactive' + }) + expect(mocks.loadPullRequestLinkedIssue).toHaveBeenCalledWith( + expect.objectContaining({ + connectionId: undefined, + localGitOptions: { wslDistro: 'Ubuntu', admissionTier: 'interactive' } + }) + ) + }) + + it('keeps SSH pull-request context reads on the remote provider', async () => { + const exec = vi.fn().mockResolvedValue({ stdout: '', stderr: '' }) + mocks.getSshGitProvider.mockReturnValue({ exec, executeCommitMessagePlan: vi.fn() }) + mocks.getPullRequestDraftContext.mockImplementation(async (execute) => { + await execute(['log', '--oneline']) + return pullRequestContext + }) + const commands = makeCommands( + makeTarget('/remote/repo', { + connectionId: 'conn-1', + localGitOptions: { wslDistro: 'Ubuntu' } + }) + ) + + await commands.generateRuntimePullRequestFields( + 'id:wt-1', + { base: 'main', title: '', body: '', draft: false }, + settingsOverride + ) + + expect(exec).toHaveBeenCalledWith(['log', '--oneline'], '/remote/repo') + expect(mocks.gitExecFileAsync).not.toHaveBeenCalled() + expect(mocks.loadPullRequestLinkedIssue).toHaveBeenCalledWith( + expect.objectContaining({ connectionId: 'conn-1', localGitOptions: {} }) + ) + }) +}) diff --git a/src/main/runtime/runtime-git-generation-commands.ts b/src/main/runtime/runtime-git-generation-commands.ts index 0bbabc39e28..868b97acd89 100644 --- a/src/main/runtime/runtime-git-generation-commands.ts +++ b/src/main/runtime/runtime-git-generation-commands.ts @@ -89,7 +89,10 @@ export class RuntimeGitGenerationCommands { let context: CommitMessageDraftContext | null try { - context = await getStagedCommitContext(target.worktree.path, localGitOptionsForTarget(target)) + context = await getStagedCommitContext(target.worktree.path, { + ...localGitOptionsForTarget(target), + admissionTier: 'interactive' + }) } catch (error) { console.error('[runtime-git] Failed to read staged commit context:', error) return { success: false, error: 'Failed to read staged changes.' } @@ -166,7 +169,12 @@ export class RuntimeGitGenerationCommands { provider: input.provider, repoPath: target.worktree.path, connectionId: target.connectionId, - localGitOptions: localGitOptionsForTarget(target) + localGitOptions: target.connectionId + ? {} + : { + ...localGitOptionsForTarget(target), + admissionTier: 'interactive' + } }) let context: Awaited> try { @@ -189,7 +197,8 @@ export class RuntimeGitGenerationCommands { gitExecFileAsync(argv, { cwd: target.worktree.path, ...localGitOptionsForTarget(target), - ...options + ...options, + admissionTier: 'interactive' }), { base: input.base, diff --git a/src/main/source-control/pull-request-linked-issue.ts b/src/main/source-control/pull-request-linked-issue.ts index 94ea1432473..90c61930d91 100644 --- a/src/main/source-control/pull-request-linked-issue.ts +++ b/src/main/source-control/pull-request-linked-issue.ts @@ -2,6 +2,7 @@ import type { HostedReviewProvider } from '../../shared/hosted-review' import type { PullRequestLinkedIssue } from '../../shared/pull-request-generation' import { isLinkedIssueNumber } from '../../shared/source-control-ai-action-variables' import type { WorkspaceLinkedItem } from '../../shared/worktree/types' +import type { GitRuntimeOptions } from '../git/git-runtime-options' import { getIssue as getGitHubIssue } from '../github/issues' import { getIssue as getGitLabIssue } from '../gitlab/issues' @@ -11,7 +12,7 @@ export type PullRequestLinkedIssueMeta = { linkedWorkItem?: WorkspaceLinkedItem | null } -type LocalGitOptions = { wslDistro?: string } +type LocalGitOptions = Pick function inferIssueProvider( meta: PullRequestLinkedIssueMeta,