mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 16:03:49 +00:00
perf(git): bound eligible route selection
This commit is contained in:
@@ -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<GitAdmissionTier, Map<string | null, WaiterLane>>
|
||||
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<AdmissionClass, TierLanes> = {
|
||||
@@ -27,6 +89,23 @@ export class GitAdmissionWaiterQueue {
|
||||
network: createTierLanes()
|
||||
}
|
||||
private readonly countsByBudget = new Map<string, number>()
|
||||
private readonly baseCandidates: Record<AdmissionClass, Record<GitAdmissionTier, CandidateHeap>> =
|
||||
{
|
||||
general: {
|
||||
interactive: new CandidateHeap(),
|
||||
status: new CandidateHeap(),
|
||||
background: new CandidateHeap()
|
||||
},
|
||||
network: {
|
||||
interactive: new CandidateHeap(),
|
||||
status: new CandidateHeap(),
|
||||
background: new CandidateHeap()
|
||||
}
|
||||
}
|
||||
private readonly headroomCandidates: Record<AdmissionClass, CandidateHeap> = {
|
||||
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 {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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')
|
||||
})
|
||||
|
||||
|
||||
@@ -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']
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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']
|
||||
})
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -371,6 +371,7 @@ describe('RuntimeGitCommands', () => {
|
||||
})
|
||||
|
||||
expect(mocks.getStagedCommitContext).toHaveBeenCalledWith(worktreePath, {
|
||||
admissionTier: 'interactive',
|
||||
wslDistro: 'Ubuntu'
|
||||
})
|
||||
expect(prepareForCodexLaunch).toHaveBeenCalledWith({
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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> = {}): 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: {} })
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -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<ReturnType<typeof getPullRequestDraftContext>>
|
||||
try {
|
||||
@@ -189,7 +197,8 @@ export class RuntimeGitGenerationCommands {
|
||||
gitExecFileAsync(argv, {
|
||||
cwd: target.worktree.path,
|
||||
...localGitOptionsForTarget(target),
|
||||
...options
|
||||
...options,
|
||||
admissionTier: 'interactive'
|
||||
}),
|
||||
{
|
||||
base: input.base,
|
||||
|
||||
@@ -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<GitRuntimeOptions, 'wslDistro' | 'admissionTier'>
|
||||
|
||||
function inferIssueProvider(
|
||||
meta: PullRequestLinkedIssueMeta,
|
||||
|
||||
Reference in New Issue
Block a user