fix(gitlab): bound and coalesce auth probes (#9476)
This commit is contained in:
parent
245d79cc8c
commit
b320bcb374
|
|
@ -1,3 +1,4 @@
|
|||
import { EventEmitter } from 'node:events'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type * as WslModule from '../wsl'
|
||||
|
||||
|
|
@ -21,11 +22,26 @@ vi.mock('../wsl', async (importOriginal) => ({
|
|||
|
||||
import { ghExecFileAsync, glabExecFileAsync } from './runner'
|
||||
|
||||
type MockChildProcess = EventEmitter & {
|
||||
pid: number
|
||||
kill: ReturnType<typeof vi.fn>
|
||||
unref: ReturnType<typeof vi.fn>
|
||||
}
|
||||
|
||||
function createMockChildProcess(pid: number): MockChildProcess {
|
||||
const child = new EventEmitter() as MockChildProcess
|
||||
child.pid = pid
|
||||
child.kill = vi.fn()
|
||||
child.unref = vi.fn()
|
||||
return child
|
||||
}
|
||||
|
||||
describe('ghExecFileAsync WSL fallback', () => {
|
||||
const originalPlatform = process.platform
|
||||
|
||||
beforeEach(() => {
|
||||
execFileMock.mockReset()
|
||||
spawnMock.mockReset()
|
||||
getDefaultWslDistroMock.mockReset()
|
||||
getDefaultWslDistroMock.mockReturnValue(null)
|
||||
Object.defineProperty(process, 'platform', {
|
||||
|
|
@ -35,6 +51,7 @@ describe('ghExecFileAsync WSL fallback', () => {
|
|||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
Object.defineProperty(process, 'platform', {
|
||||
configurable: true,
|
||||
value: originalPlatform
|
||||
|
|
@ -352,6 +369,80 @@ describe('ghExecFileAsync WSL fallback', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('times out the default-WSL glab fallback and waits for full tree cleanup', async () => {
|
||||
vi.useFakeTimers()
|
||||
getDefaultWslDistroMock.mockReturnValue('Ubuntu')
|
||||
const nativeChild = createMockChildProcess(1200)
|
||||
const wslChild = createMockChildProcess(2400)
|
||||
const taskkill = createMockChildProcess(3600)
|
||||
execFileMock
|
||||
.mockImplementationOnce((_binary, _args, _options, callback) => {
|
||||
callback(Object.assign(new Error('spawn glab ENOENT'), { code: 'ENOENT' }))
|
||||
return nativeChild
|
||||
})
|
||||
.mockReturnValueOnce(wslChild)
|
||||
spawnMock.mockReturnValue(taskkill)
|
||||
|
||||
const promise = glabExecFileAsync(['auth', 'status'], { timeout: 1000 })
|
||||
const rejection = expect(promise).rejects.toThrow('wsl.exe timed out.')
|
||||
let rejected = false
|
||||
void promise.catch(() => {
|
||||
rejected = true
|
||||
})
|
||||
|
||||
await vi.advanceTimersByTimeAsync(999)
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
await vi.advanceTimersByTimeAsync(1)
|
||||
expect(spawnMock).toHaveBeenCalledWith(
|
||||
'taskkill',
|
||||
['/pid', '2400', '/t', '/f'],
|
||||
expect.objectContaining({ stdio: 'ignore', windowsHide: true })
|
||||
)
|
||||
await Promise.resolve()
|
||||
expect(rejected).toBe(false)
|
||||
|
||||
taskkill.emit('close', 0)
|
||||
await rejection
|
||||
expect(wslChild.kill).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('aborts the default-WSL glab fallback with full process-tree cleanup', async () => {
|
||||
getDefaultWslDistroMock.mockReturnValue('Ubuntu')
|
||||
const nativeChild = createMockChildProcess(1200)
|
||||
const wslChild = createMockChildProcess(2400)
|
||||
const taskkill = createMockChildProcess(3600)
|
||||
execFileMock
|
||||
.mockImplementationOnce((_binary, _args, _options, callback) => {
|
||||
callback(Object.assign(new Error('spawn glab ENOENT'), { code: 'ENOENT' }))
|
||||
return nativeChild
|
||||
})
|
||||
.mockReturnValueOnce(wslChild)
|
||||
spawnMock.mockReturnValue(taskkill)
|
||||
const controller = new AbortController()
|
||||
|
||||
const promise = glabExecFileAsync(['auth', 'status'], { signal: controller.signal })
|
||||
const rejection = expect(promise).rejects.toMatchObject({ name: 'AbortError' })
|
||||
await vi.waitFor(() => expect(execFileMock).toHaveBeenCalledTimes(2))
|
||||
controller.abort()
|
||||
|
||||
expect(execFileMock).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
'wsl.exe',
|
||||
['-d', 'Ubuntu', '--', 'bash', '-c', "'glab' 'auth' 'status'"],
|
||||
expect.not.objectContaining({ signal: controller.signal }),
|
||||
expect.any(Function)
|
||||
)
|
||||
expect(spawnMock).toHaveBeenCalledWith(
|
||||
'taskkill',
|
||||
['/pid', '2400', '/t', '/f'],
|
||||
expect.objectContaining({ stdio: 'ignore', windowsHide: true })
|
||||
)
|
||||
taskkill.emit('close', 0)
|
||||
|
||||
await rejection
|
||||
expect(wslChild.kill).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not wake the default WSL distro for host-only GitLab diagnostics', async () => {
|
||||
getDefaultWslDistroMock.mockReturnValue('Ubuntu')
|
||||
execFileMock
|
||||
|
|
|
|||
|
|
@ -323,24 +323,57 @@ function createAbortError(): Error {
|
|||
return error
|
||||
}
|
||||
|
||||
function killSpawnedCommandTree(child: ChildProcess): void {
|
||||
const WINDOWS_TREE_KILL_WAIT_MS = 2_000
|
||||
|
||||
function killSpawnedCommandTree(child: ChildProcess): Promise<void> {
|
||||
const pid = child.pid
|
||||
if (!pid || process.platform !== 'win32') {
|
||||
child.kill()
|
||||
return
|
||||
return Promise.resolve()
|
||||
}
|
||||
try {
|
||||
// Why: Windows package-manager CLIs are often .cmd shims. Killing only
|
||||
// cmd.exe leaves the underlying node/npm/pnpm child running.
|
||||
const killer = spawn('taskkill', ['/pid', String(pid), '/t', '/f'], {
|
||||
stdio: 'ignore',
|
||||
windowsHide: true
|
||||
})
|
||||
killer.on('error', () => child.kill())
|
||||
return new Promise((resolve) => {
|
||||
let killer: ChildProcess
|
||||
try {
|
||||
// Why: Windows shims and wsl.exe can own descendants; wait for /t tree
|
||||
// cleanup before settling so a timed-out command cannot outlive its probe.
|
||||
killer = spawn('taskkill', ['/pid', String(pid), '/t', '/f'], {
|
||||
stdio: 'ignore',
|
||||
windowsHide: true
|
||||
})
|
||||
if (!killer || typeof killer.unref !== 'function') {
|
||||
child.kill()
|
||||
resolve()
|
||||
return
|
||||
}
|
||||
} catch {
|
||||
child.kill()
|
||||
resolve()
|
||||
return
|
||||
}
|
||||
let settled = false
|
||||
let timer: NodeJS.Timeout | null = null
|
||||
const finish = (fallbackToChildKill: boolean): void => {
|
||||
if (settled) {
|
||||
return
|
||||
}
|
||||
settled = true
|
||||
if (timer) {
|
||||
clearTimeout(timer)
|
||||
}
|
||||
killer.removeAllListeners()
|
||||
if (fallbackToChildKill) {
|
||||
child.kill()
|
||||
}
|
||||
resolve()
|
||||
}
|
||||
killer.once('error', () => finish(true))
|
||||
killer.once('close', (code) => finish(code !== 0))
|
||||
timer = setTimeout(() => {
|
||||
killer.kill()
|
||||
finish(true)
|
||||
}, WINDOWS_TREE_KILL_WAIT_MS)
|
||||
killer.unref()
|
||||
} catch {
|
||||
child.kill()
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
type ExecFileCaptureOptions = Omit<ExecFileOptions, 'timeout'> & {
|
||||
|
|
@ -376,6 +409,7 @@ function execFileCapture(
|
|||
}
|
||||
|
||||
let settled = false
|
||||
let terminating = false
|
||||
let child: ChildProcess | null = null
|
||||
let timer: NodeJS.Timeout | null = null
|
||||
const cleanup = (): void => {
|
||||
|
|
@ -405,14 +439,26 @@ function execFileCapture(
|
|||
resolve({ stdout, stderr })
|
||||
}
|
||||
const onAbort = (): void => {
|
||||
if (child) {
|
||||
killSpawnedCommandTree(child)
|
||||
if (settled || terminating) {
|
||||
return
|
||||
}
|
||||
finish(createAbortError())
|
||||
terminating = true
|
||||
const abortError = createAbortError()
|
||||
if (!child) {
|
||||
terminating = false
|
||||
finish(abortError)
|
||||
return
|
||||
}
|
||||
void killSpawnedCommandTree(child).then(() => {
|
||||
terminating = false
|
||||
finish(abortError)
|
||||
})
|
||||
}
|
||||
|
||||
try {
|
||||
const spawnStartedAt = performance.now()
|
||||
// Why: the abort listener below owns tree-aware cleanup. Node's
|
||||
// signal handler could kill wsl.exe before taskkill sees its children.
|
||||
child = execFile(
|
||||
command,
|
||||
args,
|
||||
|
|
@ -420,10 +466,12 @@ function execFileCapture(
|
|||
cwd: options.cwd,
|
||||
encoding: options.encoding,
|
||||
maxBuffer: options.maxBuffer ?? DEFAULT_GIT_MAX_BUFFER,
|
||||
env: options.env,
|
||||
signal: options.signal
|
||||
env: options.env
|
||||
},
|
||||
(error, stdout, stderr) => {
|
||||
if (terminating) {
|
||||
return
|
||||
}
|
||||
if (!error && stderr === undefined && isExecFileResultObject(stdout)) {
|
||||
finish(null, stdout.stdout, stdout.stderr)
|
||||
return
|
||||
|
|
@ -437,21 +485,34 @@ function execFileCapture(
|
|||
return
|
||||
}
|
||||
|
||||
child.once('error', (error) => finish(error))
|
||||
child.once('error', (error) => {
|
||||
if (!terminating) {
|
||||
finish(error)
|
||||
}
|
||||
})
|
||||
|
||||
if (options.stdin !== undefined) {
|
||||
endSubprocessStdin(child.stdin, options.stdin)
|
||||
}
|
||||
|
||||
// Why: Node's native execFile timeout waits for the child to exit after
|
||||
// signaling it. Some CLIs ignore that signal, so reject the UI operation
|
||||
// on our own timer and kill the child only as best effort.
|
||||
// Why: Node's native timeout can wait forever for signal-ignoring CLIs;
|
||||
// enforce our own deadline and settle after bounded process-tree cleanup.
|
||||
if (options.timeout && options.timeout > 0) {
|
||||
timer = setTimeout(() => {
|
||||
if (child) {
|
||||
killSpawnedCommandTree(child)
|
||||
if (settled || terminating) {
|
||||
return
|
||||
}
|
||||
finish(new Error(`${command} timed out.`))
|
||||
terminating = true
|
||||
const timeoutError = new Error(`${command} timed out.`)
|
||||
if (!child) {
|
||||
terminating = false
|
||||
finish(timeoutError)
|
||||
return
|
||||
}
|
||||
void killSpawnedCommandTree(child).then(() => {
|
||||
terminating = false
|
||||
finish(timeoutError)
|
||||
})
|
||||
}, options.timeout)
|
||||
}
|
||||
options.signal?.addEventListener('abort', onAbort, { once: true })
|
||||
|
|
@ -484,7 +545,7 @@ async function spawnCommandCapture(
|
|||
recordSubprocessSpawn(spawnCmd, spawnArgs, performance.now() - spawnStartedAt)
|
||||
let timer: NodeJS.Timeout | null = null
|
||||
const onAbort = (): void => {
|
||||
killSpawnedCommandTree(child)
|
||||
void killSpawnedCommandTree(child)
|
||||
finish(createAbortError())
|
||||
}
|
||||
const cleanupListeners = (): void => {
|
||||
|
|
@ -512,7 +573,7 @@ async function spawnCommandCapture(
|
|||
}
|
||||
timer = options.timeout
|
||||
? setTimeout(() => {
|
||||
killSpawnedCommandTree(child)
|
||||
void killSpawnedCommandTree(child)
|
||||
finish(new Error(`${command} timed out.`))
|
||||
}, options.timeout)
|
||||
: null
|
||||
|
|
@ -520,7 +581,7 @@ async function spawnCommandCapture(
|
|||
function onStdoutData(chunk: Buffer): void {
|
||||
stdoutBytes += chunk.byteLength
|
||||
if (options.maxBuffer && stdoutBytes > options.maxBuffer) {
|
||||
killSpawnedCommandTree(child)
|
||||
void killSpawnedCommandTree(child)
|
||||
finish(new Error(`${command} stdout exceeded maxBuffer.`))
|
||||
return
|
||||
}
|
||||
|
|
@ -529,7 +590,7 @@ async function spawnCommandCapture(
|
|||
function onStderrData(chunk: Buffer): void {
|
||||
stderrBytes += chunk.byteLength
|
||||
if (options.maxBuffer && stderrBytes > options.maxBuffer) {
|
||||
killSpawnedCommandTree(child)
|
||||
void killSpawnedCommandTree(child)
|
||||
finish(new Error(`${command} stderr exceeded maxBuffer.`))
|
||||
return
|
||||
}
|
||||
|
|
@ -997,7 +1058,7 @@ export async function gitStreamStdout(
|
|||
function onStdoutData(chunk: Buffer): void {
|
||||
stdoutBytes += chunk.byteLength
|
||||
if (stdoutBytes > maxBuffer) {
|
||||
killSpawnedCommandTree(child)
|
||||
void killSpawnedCommandTree(child)
|
||||
finish(new Error('git stdout exceeded maxBuffer.'))
|
||||
return
|
||||
}
|
||||
|
|
@ -1012,7 +1073,7 @@ export async function gitStreamStdout(
|
|||
try {
|
||||
shouldStop = options.onStdout(decoded)
|
||||
} catch (error) {
|
||||
killSpawnedCommandTree(child)
|
||||
void killSpawnedCommandTree(child)
|
||||
finish(error instanceof Error ? error : new Error(String(error)))
|
||||
return
|
||||
}
|
||||
|
|
@ -1020,14 +1081,14 @@ export async function gitStreamStdout(
|
|||
// Why: parser hit its limit. Kill git and resolve cleanly — the
|
||||
// partial output we already parsed is the intended result.
|
||||
stoppedEarly = true
|
||||
killSpawnedCommandTree(child)
|
||||
void killSpawnedCommandTree(child)
|
||||
finish(null)
|
||||
}
|
||||
}
|
||||
function onStderrData(chunk: Buffer): void {
|
||||
stderrBytes += chunk.byteLength
|
||||
if (stderrBytes > maxBuffer) {
|
||||
killSpawnedCommandTree(child)
|
||||
void killSpawnedCommandTree(child)
|
||||
finish(new Error('git stderr exceeded maxBuffer.'))
|
||||
return
|
||||
}
|
||||
|
|
@ -1044,7 +1105,7 @@ export async function gitStreamStdout(
|
|||
finish(new Error(`git exited with ${code}: ${stderr}`))
|
||||
}
|
||||
function onAbort(): void {
|
||||
killSpawnedCommandTree(child)
|
||||
void killSpawnedCommandTree(child)
|
||||
finish(createAbortError())
|
||||
}
|
||||
|
||||
|
|
@ -1468,7 +1529,8 @@ export async function glabExecFileAsync(
|
|||
encoding: (options.encoding ?? 'utf-8') as BufferEncoding,
|
||||
maxBuffer: options.maxBuffer,
|
||||
timeout: options.timeout,
|
||||
env: options.env
|
||||
env: options.env,
|
||||
signal: options.signal
|
||||
})
|
||||
return { stdout: stdout as string, stderr: stderr as string }
|
||||
} catch (err) {
|
||||
|
|
|
|||
|
|
@ -248,13 +248,9 @@ export async function getProjectSlug(
|
|||
connectionId?: string | null,
|
||||
options: HostedReviewExecutionOptions = {}
|
||||
): Promise<ProjectRef | null> {
|
||||
const knownHosts = await getGlabKnownHosts(connectionId)
|
||||
return getProjectRef(
|
||||
repoPath,
|
||||
knownHosts,
|
||||
connectionId,
|
||||
...hostedReviewLocalGitOptionArgs(options)
|
||||
)
|
||||
const localGitArgs = hostedReviewLocalGitOptionArgs(options)
|
||||
const knownHosts = await getGlabKnownHosts(connectionId, localGitArgs[0])
|
||||
return getProjectRef(repoPath, knownHosts, connectionId, ...localGitArgs)
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -268,9 +264,9 @@ export async function getMergeRequest(
|
|||
connectionId?: string | null,
|
||||
options: HostedReviewExecutionOptions = {}
|
||||
): Promise<MRInfo | null> {
|
||||
const knownHosts = await getGlabKnownHosts(connectionId)
|
||||
const localGitArgs = hostedReviewLocalGitOptionArgs(options)
|
||||
const localGitOptions = localGitArgs[0] ?? {}
|
||||
const knownHosts = await getGlabKnownHosts(connectionId, localGitOptions)
|
||||
const projectRef = await getProjectRef(repoPath, knownHosts, connectionId, ...localGitArgs)
|
||||
await acquire()
|
||||
try {
|
||||
|
|
@ -323,9 +319,9 @@ export async function getMergeRequestForBranch(
|
|||
if (!branchName && linkedMRIid == null) {
|
||||
return null
|
||||
}
|
||||
const knownHosts = await getGlabKnownHosts(connectionId)
|
||||
const localGitArgs = hostedReviewLocalGitOptionArgs(options)
|
||||
const localGitOptions = localGitArgs[0] ?? {}
|
||||
const knownHosts = await getGlabKnownHosts(connectionId, localGitOptions)
|
||||
const projectRef = await getProjectRef(repoPath, knownHosts, connectionId, ...localGitArgs)
|
||||
if (!projectRef) {
|
||||
return null
|
||||
|
|
@ -425,7 +421,7 @@ export async function listMergeRequests(
|
|||
connectionId?: string | null,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): Promise<ListMergeRequestsResult> {
|
||||
const knownHosts = await getGlabKnownHosts(connectionId)
|
||||
const knownHosts = await getGlabKnownHosts(connectionId, localGitOptions)
|
||||
// Why: MRs sit on `origin` in the fork model (the user's fork is where
|
||||
// they push branches and submit MRs). Mirror github's `getOwnerRepo`
|
||||
// call site by going through the upstream/origin preference resolver
|
||||
|
|
@ -623,7 +619,7 @@ export async function listWorkItems(
|
|||
localGitOptions: LocalGitExecOptions = {}
|
||||
): Promise<GitLabPagedResult<GitLabWorkItem>> {
|
||||
const issueState = mrStateToIssueState(state)
|
||||
const knownHosts = await getGlabKnownHosts(connectionId)
|
||||
const knownHosts = await getGlabKnownHosts(connectionId, localGitOptions)
|
||||
const { source: projectRef } = await resolveIssueSource(
|
||||
repoPath,
|
||||
preference,
|
||||
|
|
@ -759,7 +755,7 @@ export async function listTodos(
|
|||
): Promise<GitLabTodo[]> {
|
||||
const projectRef = await getProjectRef(
|
||||
repoPath,
|
||||
await getGlabKnownHosts(connectionId),
|
||||
await getGlabKnownHosts(connectionId, localGitOptions),
|
||||
connectionId,
|
||||
localGitOptions
|
||||
)
|
||||
|
|
@ -838,7 +834,7 @@ async function withProjectRef<T>(
|
|||
await resolveIssueSource(
|
||||
repoPath,
|
||||
preference,
|
||||
await getGlabKnownHosts(connectionId),
|
||||
await getGlabKnownHosts(connectionId, localGitOptions),
|
||||
connectionId,
|
||||
localGitOptions
|
||||
)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,110 @@
|
|||
import { glabExecFileAsync } from '../git/runner'
|
||||
import { getSshGitProviderGeneration } from '../providers/ssh-git-dispatch'
|
||||
import { DEFAULT_GITLAB_HOSTS, normalizeGitLabHost } from './project-ref-parser'
|
||||
|
||||
export type LocalGitExecOptions = {
|
||||
wslDistro?: string
|
||||
}
|
||||
|
||||
const GLAB_KNOWN_HOSTS_TIMEOUT_MS = 10_000
|
||||
const knownHostsCacheByExecutionContext = new Map<string, readonly string[]>()
|
||||
const knownHostsInFlightByExecutionContext = new Map<string, Promise<readonly string[]>>()
|
||||
|
||||
function knownHostsExecutionKey(
|
||||
connectionId?: string | null,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): string {
|
||||
if (connectionId) {
|
||||
// Why: reconnecting can replace the SSH/relay execution host under the same id.
|
||||
return `connection:${connectionId}:${getSshGitProviderGeneration(connectionId)}`
|
||||
}
|
||||
return localGitOptions.wslDistro ? `wsl:${localGitOptions.wslDistro}` : 'native'
|
||||
}
|
||||
|
||||
/** @internal - exposed for tests only */
|
||||
export function _resetKnownHostsCache(): void {
|
||||
knownHostsCacheByExecutionContext.clear()
|
||||
knownHostsInFlightByExecutionContext.clear()
|
||||
}
|
||||
|
||||
export function rememberGlabKnownHost(
|
||||
host: string,
|
||||
connectionId?: string | null,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): void {
|
||||
const normalizedHost = normalizeGitLabHost(host)
|
||||
const key = knownHostsExecutionKey(connectionId, localGitOptions)
|
||||
const cached = knownHostsCacheByExecutionContext.get(key)
|
||||
if (!cached || cached.map(normalizeGitLabHost).includes(normalizedHost)) {
|
||||
return
|
||||
}
|
||||
knownHostsCacheByExecutionContext.set(key, [...cached, normalizedHost])
|
||||
}
|
||||
|
||||
export async function getGlabKnownHosts(
|
||||
connectionId?: string | null,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): Promise<readonly string[]> {
|
||||
const key = knownHostsExecutionKey(connectionId, localGitOptions)
|
||||
const cached = knownHostsCacheByExecutionContext.get(key)
|
||||
if (cached) {
|
||||
return cached
|
||||
}
|
||||
const inFlight = knownHostsInFlightByExecutionContext.get(key)
|
||||
if (inFlight) {
|
||||
return inFlight
|
||||
}
|
||||
const probe = probeGlabKnownHosts(key, connectionId, localGitOptions)
|
||||
knownHostsInFlightByExecutionContext.set(key, probe)
|
||||
try {
|
||||
return await probe
|
||||
} finally {
|
||||
if (knownHostsInFlightByExecutionContext.get(key) === probe) {
|
||||
knownHostsInFlightByExecutionContext.delete(key)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async function probeGlabKnownHosts(
|
||||
key: string,
|
||||
connectionId?: string | null,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): Promise<readonly string[]> {
|
||||
try {
|
||||
// Why: auth config belongs to the executing host; do not share native, WSL,
|
||||
// or reconnected SSH/relay results, and bound an otherwise global probe.
|
||||
const { stdout, stderr } = await glabExecFileAsync(['auth', 'status'], {
|
||||
timeout: GLAB_KNOWN_HOSTS_TIMEOUT_MS,
|
||||
...(!connectionId && localGitOptions.wslDistro
|
||||
? { wslDistro: localGitOptions.wslDistro }
|
||||
: {})
|
||||
})
|
||||
const hosts = parseGlabAuthStatusHosts(`${stdout}\n${stderr}`)
|
||||
const merged = Array.from(new Set([...DEFAULT_GITLAB_HOSTS, ...hosts]))
|
||||
knownHostsCacheByExecutionContext.set(key, merged)
|
||||
return merged
|
||||
} catch {
|
||||
// Keep failures uncached so auth or tunnel recovery is discovered later.
|
||||
return [...DEFAULT_GITLAB_HOSTS]
|
||||
}
|
||||
}
|
||||
|
||||
export function parseGlabAuthStatusHosts(output: string): string[] {
|
||||
const hosts = new Set<string>()
|
||||
// Why: self-hosted GitLab can run on a non-default port; preserve it so
|
||||
// services on the same hostname remain distinct downstream.
|
||||
for (const match of output.matchAll(/logged in to ([a-zA-Z0-9.-]+(?::\d+)?)/gi)) {
|
||||
hosts.add(match[1].toLowerCase())
|
||||
}
|
||||
for (const line of output.split('\n')) {
|
||||
const bareLine = line.trim()
|
||||
const hostLine = bareLine.endsWith(':') ? bareLine.slice(0, -1) : bareLine
|
||||
if (
|
||||
line === bareLine &&
|
||||
/^[a-zA-Z0-9](?:[a-zA-Z0-9.-]*[a-zA-Z0-9])?(?::\d+)?$/.test(hostLine)
|
||||
) {
|
||||
hosts.add(hostLine.toLowerCase())
|
||||
}
|
||||
}
|
||||
return Array.from(hosts)
|
||||
}
|
||||
|
|
@ -2,6 +2,11 @@ import { gitExecFileAsync, glabExecFileAsync } from '../git/runner'
|
|||
import type { IssueSourcePreference } from '../../shared/types'
|
||||
import { getSshGitProvider } from '../providers/ssh-git-dispatch'
|
||||
import { clearProjectRefInFlight, runProjectRefProbeOnce } from './project-ref-inflight'
|
||||
import {
|
||||
parseGlabAuthStatusHosts,
|
||||
rememberGlabKnownHost,
|
||||
type LocalGitExecOptions
|
||||
} from './gitlab-known-host-probe'
|
||||
import {
|
||||
DEFAULT_GITLAB_HOSTS,
|
||||
normalizeGitLabHost,
|
||||
|
|
@ -12,26 +17,16 @@ import {
|
|||
|
||||
export { DEFAULT_GITLAB_HOSTS, parseGitLabProjectRef }
|
||||
export type { ProjectRef }
|
||||
|
||||
export type LocalGitExecOptions = {
|
||||
wslDistro?: string
|
||||
}
|
||||
export {
|
||||
_resetKnownHostsCache,
|
||||
getGlabKnownHosts,
|
||||
parseGlabAuthStatusHosts
|
||||
} from './gitlab-known-host-probe'
|
||||
export type { LocalGitExecOptions } from './gitlab-known-host-probe'
|
||||
|
||||
const PROJECT_REF_CACHE_MAX_ENTRIES = 512
|
||||
const projectRefCache = new Map<string, ProjectRef | null>()
|
||||
|
||||
// Why: known hosts are cached PER connection. A repo on an SSH connection
|
||||
// authenticates against a different glab context than the local one, so a
|
||||
// process-global cache would leak one connection's hosts into another (and
|
||||
// poison a connection that probes before its tunnel is ready). The local
|
||||
// context uses the `'local'` key.
|
||||
const LOCAL_CONNECTION_KEY = 'local'
|
||||
const knownHostsCacheByConnection = new Map<string, readonly string[]>()
|
||||
|
||||
function connectionCacheKey(connectionId?: string | null): string {
|
||||
return connectionId ?? LOCAL_CONNECTION_KEY
|
||||
}
|
||||
|
||||
/** @internal - exposed for tests only */
|
||||
export function _resetProjectRefCache(): void {
|
||||
projectRefCache.clear()
|
||||
|
|
@ -43,11 +38,6 @@ export function _getProjectRefCacheSize(): number {
|
|||
return projectRefCache.size
|
||||
}
|
||||
|
||||
/** @internal - exposed for tests only */
|
||||
export function _resetKnownHostsCache(): void {
|
||||
knownHostsCacheByConnection.clear()
|
||||
}
|
||||
|
||||
function rememberProjectRefCacheEntry(cacheKey: string, value: ProjectRef | null): void {
|
||||
projectRefCache.set(cacheKey, value)
|
||||
while (projectRefCache.size > PROJECT_REF_CACHE_MAX_ENTRIES) {
|
||||
|
|
@ -118,7 +108,7 @@ async function resolveProjectRefForRemote(
|
|||
localGitOptions
|
||||
))
|
||||
) {
|
||||
rememberGlabKnownHost(remoteCandidate.host, connectionId)
|
||||
rememberGlabKnownHost(remoteCandidate.host, connectionId, localGitOptions)
|
||||
rememberProjectRefCacheEntry(cacheKey, remoteCandidate)
|
||||
return remoteCandidate
|
||||
}
|
||||
|
|
@ -230,16 +220,6 @@ export function glabHostnameArgs(
|
|||
return connectionId && projectRef?.host ? ['--hostname', projectRef.host] : []
|
||||
}
|
||||
|
||||
function rememberGlabKnownHost(host: string, connectionId?: string | null): void {
|
||||
const normalizedHost = normalizeGitLabHost(host)
|
||||
const key = connectionCacheKey(connectionId)
|
||||
const cached = knownHostsCacheByConnection.get(key)
|
||||
if (!cached || cached.map(normalizeGitLabHost).includes(normalizedHost)) {
|
||||
return
|
||||
}
|
||||
knownHostsCacheByConnection.set(key, [...cached, normalizedHost])
|
||||
}
|
||||
|
||||
async function isGlabConfiguredForRemoteHost(
|
||||
repoPath: string,
|
||||
projectRef: Pick<ProjectRef, 'host'>,
|
||||
|
|
@ -262,50 +242,3 @@ async function isGlabConfiguredForRemoteHost(
|
|||
return hosts.includes(normalizeGitLabHost(projectRef.host))
|
||||
}
|
||||
}
|
||||
|
||||
export async function getGlabKnownHosts(connectionId?: string | null): Promise<readonly string[]> {
|
||||
const key = connectionCacheKey(connectionId)
|
||||
const cached = knownHostsCacheByConnection.get(key)
|
||||
if (cached) {
|
||||
return cached
|
||||
}
|
||||
try {
|
||||
// Why: `glab auth status` is host-scoped, not cwd-scoped — glab reads its
|
||||
// own config to list authenticated hosts. The connectionId is threaded so
|
||||
// the RESULT is cached per connection (a connected repo can have a
|
||||
// different set of authenticated self-hosted hosts than the local one),
|
||||
// mirroring how project-ref resolution caches per connection.
|
||||
const { stdout, stderr } = await glabExecFileAsync(['auth', 'status'])
|
||||
const hosts = parseGlabAuthStatusHosts(`${stdout}\n${stderr}`)
|
||||
const merged = Array.from(new Set([...DEFAULT_GITLAB_HOSTS, ...hosts]))
|
||||
knownHostsCacheByConnection.set(key, merged)
|
||||
return merged
|
||||
} catch {
|
||||
// Auth check failed (glab not installed, no auth, tunnel not ready,
|
||||
// etc.) — fall back to the canonical default for THIS call, but do NOT
|
||||
// cache the fallback. A later probe (e.g. after the SSH tunnel comes
|
||||
// up) must be able to discover the real self-hosted host.
|
||||
return [...DEFAULT_GITLAB_HOSTS]
|
||||
}
|
||||
}
|
||||
|
||||
export function parseGlabAuthStatusHosts(output: string): string[] {
|
||||
const hosts = new Set<string>()
|
||||
// Why: self-hosted GitLab can run on a non-default port (e.g.
|
||||
// `gitlab.example.com:8443`); capture the optional `:port` so two services
|
||||
// on the same hostname but different ports stay distinct downstream.
|
||||
for (const m of output.matchAll(/logged in to ([a-zA-Z0-9.-]+(?::\d+)?)/gi)) {
|
||||
hosts.add(m[1].toLowerCase())
|
||||
}
|
||||
for (const line of output.split('\n')) {
|
||||
const bareLine = line.trim()
|
||||
const hostLine = bareLine.endsWith(':') ? bareLine.slice(0, -1) : bareLine
|
||||
if (
|
||||
line === bareLine &&
|
||||
/^[a-zA-Z0-9](?:[a-zA-Z0-9.-]*[a-zA-Z0-9])?(?::\d+)?$/.test(hostLine)
|
||||
) {
|
||||
hosts.add(hostLine.toLowerCase())
|
||||
}
|
||||
}
|
||||
return Array.from(hosts)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -434,6 +434,7 @@ describe('getGlabKnownHosts', () => {
|
|||
})
|
||||
|
||||
await expect(getGlabKnownHosts()).resolves.toEqual(['gitlab.com', 'gitlab.example.com'])
|
||||
expect(glabExecFileAsyncMock).toHaveBeenCalledWith(['auth', 'status'], { timeout: 10_000 })
|
||||
})
|
||||
|
||||
it('falls back to default when glab auth status fails', async () => {
|
||||
|
|
@ -453,6 +454,55 @@ describe('getGlabKnownHosts', () => {
|
|||
expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('coalesces many simultaneous callers in one execution context', async () => {
|
||||
let resolveProbe!: (value: { stdout: string; stderr: string }) => void
|
||||
glabExecFileAsyncMock.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
resolveProbe = resolve
|
||||
})
|
||||
)
|
||||
|
||||
const probes = Array.from({ length: 64 }, () => getGlabKnownHosts())
|
||||
|
||||
expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(1)
|
||||
resolveProbe({ stdout: 'Logged in to gitlab.concurrent.test as user\n', stderr: '' })
|
||||
const results = await Promise.all(probes)
|
||||
expect(results.every((result) => result === results[0])).toBe(true)
|
||||
expect(results[0]).toEqual(['gitlab.com', 'gitlab.concurrent.test'])
|
||||
})
|
||||
|
||||
it('keeps simultaneous native, WSL distro, and connection probes isolated', async () => {
|
||||
glabExecFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: 'Logged in to ubuntu.test as user\n', stderr: '' })
|
||||
.mockResolvedValueOnce({ stdout: 'Logged in to debian.test as user\n', stderr: '' })
|
||||
.mockResolvedValueOnce({ stdout: 'Logged in to native.test as user\n', stderr: '' })
|
||||
.mockResolvedValueOnce({ stdout: 'Logged in to ssh.test as user\n', stderr: '' })
|
||||
|
||||
const [ubuntu, ubuntuAgain, debian, native, ssh] = await Promise.all([
|
||||
getGlabKnownHosts(undefined, { wslDistro: 'Ubuntu' }),
|
||||
getGlabKnownHosts(undefined, { wslDistro: 'Ubuntu' }),
|
||||
getGlabKnownHosts(undefined, { wslDistro: 'Debian' }),
|
||||
getGlabKnownHosts(),
|
||||
getGlabKnownHosts('conn-1')
|
||||
])
|
||||
|
||||
expect(ubuntuAgain).toBe(ubuntu)
|
||||
expect(ubuntu).toEqual(['gitlab.com', 'ubuntu.test'])
|
||||
expect(debian).toEqual(['gitlab.com', 'debian.test'])
|
||||
expect(native).toEqual(['gitlab.com', 'native.test'])
|
||||
expect(ssh).toEqual(['gitlab.com', 'ssh.test'])
|
||||
expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(4)
|
||||
expect(glabExecFileAsyncMock).toHaveBeenNthCalledWith(1, ['auth', 'status'], {
|
||||
timeout: 10_000,
|
||||
wslDistro: 'Ubuntu'
|
||||
})
|
||||
expect(glabExecFileAsyncMock).toHaveBeenNthCalledWith(2, ['auth', 'status'], {
|
||||
timeout: 10_000,
|
||||
wslDistro: 'Debian'
|
||||
})
|
||||
})
|
||||
|
||||
it('recognizes a self-hosted host on a non-default port', async () => {
|
||||
glabExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: '✓ Logged in to gitlab.example.com:8080 as user\n',
|
||||
|
|
@ -500,4 +550,51 @@ describe('getGlabKnownHosts', () => {
|
|||
])
|
||||
expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('removes a timed-out probe from in-flight state so a later call retries', async () => {
|
||||
let rejectProbe!: (error: Error) => void
|
||||
glabExecFileAsyncMock
|
||||
.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((_resolve, reject) => {
|
||||
rejectProbe = reject
|
||||
})
|
||||
)
|
||||
.mockResolvedValueOnce({ stdout: 'Logged in to recovered.test as user\n', stderr: '' })
|
||||
|
||||
const first = getGlabKnownHosts(undefined, { wslDistro: 'Ubuntu' })
|
||||
const concurrent = getGlabKnownHosts(undefined, { wslDistro: 'Ubuntu' })
|
||||
expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(1)
|
||||
rejectProbe(new Error('wsl.exe timed out.'))
|
||||
|
||||
await expect(Promise.all([first, concurrent])).resolves.toEqual([
|
||||
['gitlab.com'],
|
||||
['gitlab.com']
|
||||
])
|
||||
await expect(getGlabKnownHosts(undefined, { wslDistro: 'Ubuntu' })).resolves.toEqual([
|
||||
'gitlab.com',
|
||||
'recovered.test'
|
||||
])
|
||||
expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('does not reuse a successful result after an SSH provider reconnects', async () => {
|
||||
const connectionId = 'conn-reconnected'
|
||||
registerSshGitProvider(connectionId, {} as never)
|
||||
glabExecFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: 'Logged in to old-tunnel.test as user\n', stderr: '' })
|
||||
.mockResolvedValueOnce({ stdout: 'Logged in to new-tunnel.test as user\n', stderr: '' })
|
||||
|
||||
await expect(getGlabKnownHosts(connectionId)).resolves.toEqual([
|
||||
'gitlab.com',
|
||||
'old-tunnel.test'
|
||||
])
|
||||
registerSshGitProvider(connectionId, {} as never)
|
||||
await expect(getGlabKnownHosts(connectionId)).resolves.toEqual([
|
||||
'gitlab.com',
|
||||
'new-tunnel.test'
|
||||
])
|
||||
expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(2)
|
||||
unregisterSshGitProvider(connectionId)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -214,7 +214,7 @@ describe('gitlab issue operations', () => {
|
|||
|
||||
await listIssues('/repo-root', 5, undefined, 'opened', undefined, 'conn-7')
|
||||
|
||||
expect(getGlabKnownHostsMock).toHaveBeenCalledWith('conn-7')
|
||||
expect(getGlabKnownHostsMock).toHaveBeenCalledWith('conn-7', {})
|
||||
})
|
||||
|
||||
it('creates an issue and returns its iid + web_url', async () => {
|
||||
|
|
|
|||
|
|
@ -44,7 +44,7 @@ export async function getIssue(
|
|||
connectionId?: string | null,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): Promise<GitLabIssueInfo | null> {
|
||||
const knownHosts = await getGlabKnownHosts(connectionId)
|
||||
const knownHosts = await getGlabKnownHosts(connectionId, localGitOptions)
|
||||
const projectRef = await getIssueProjectRef(repoPath, knownHosts, connectionId, localGitOptions)
|
||||
// Why: don't fall back to a cwd-inferred `glab issue view` when the project
|
||||
// can't be resolved — on an SSH connection cwd is not the repo dir, so glab
|
||||
|
|
@ -92,7 +92,7 @@ export async function listIssues(
|
|||
connectionId?: string | null,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): Promise<IssueListResult> {
|
||||
const knownHosts = await getGlabKnownHosts(connectionId)
|
||||
const knownHosts = await getGlabKnownHosts(connectionId, localGitOptions)
|
||||
const { source: projectRef } = await resolveIssueSource(
|
||||
repoPath,
|
||||
preference,
|
||||
|
|
@ -162,7 +162,7 @@ export async function createIssue(
|
|||
if (!trimmedTitle) {
|
||||
return { ok: false, error: 'Title is required' }
|
||||
}
|
||||
const knownHosts = await getGlabKnownHosts(connectionId)
|
||||
const knownHosts = await getGlabKnownHosts(connectionId, localGitOptions)
|
||||
const { source: projectRef } = await resolveIssueSource(
|
||||
repoPath,
|
||||
preference,
|
||||
|
|
@ -232,7 +232,7 @@ export async function updateIssue(
|
|||
await resolveIssueSource(
|
||||
repoPath,
|
||||
preference,
|
||||
await getGlabKnownHosts(connectionId),
|
||||
await getGlabKnownHosts(connectionId, localGitOptions),
|
||||
connectionId,
|
||||
localGitOptions
|
||||
)
|
||||
|
|
@ -369,7 +369,7 @@ export async function addIssueComment(
|
|||
await resolveIssueSource(
|
||||
repoPath,
|
||||
preference,
|
||||
await getGlabKnownHosts(connectionId),
|
||||
await getGlabKnownHosts(connectionId, localGitOptions),
|
||||
connectionId,
|
||||
localGitOptions
|
||||
)
|
||||
|
|
@ -427,7 +427,7 @@ export async function listLabels(
|
|||
connectionId?: string | null,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): Promise<string[]> {
|
||||
const knownHosts = await getGlabKnownHosts(connectionId)
|
||||
const knownHosts = await getGlabKnownHosts(connectionId, localGitOptions)
|
||||
const { source: projectRef } = await resolveIssueSource(
|
||||
repoPath,
|
||||
preference,
|
||||
|
|
@ -468,7 +468,7 @@ export async function listAssignableUsers(
|
|||
connectionId?: string | null,
|
||||
localGitOptions: LocalGitExecOptions = {}
|
||||
): Promise<GitLabAssignableUser[]> {
|
||||
const knownHosts = await getGlabKnownHosts(connectionId)
|
||||
const knownHosts = await getGlabKnownHosts(connectionId, localGitOptions)
|
||||
const { source: projectRef } = await resolveIssueSource(
|
||||
repoPath,
|
||||
preference,
|
||||
|
|
|
|||
|
|
@ -361,7 +361,7 @@ export async function getWorkItemDetails(
|
|||
await resolveIssueSource(
|
||||
repoPath,
|
||||
preference,
|
||||
await getGlabKnownHosts(connectionId),
|
||||
await getGlabKnownHosts(connectionId, localGitOptions),
|
||||
connectionId,
|
||||
localGitOptions
|
||||
)
|
||||
|
|
|
|||
|
|
@ -17895,7 +17895,7 @@ export class OrcaRuntimeService {
|
|||
} catch (error) {
|
||||
return { error: error instanceof Error ? error.message : 'Could not resolve git remote.' }
|
||||
}
|
||||
const knownHosts = await getGlabKnownHosts(repo.connectionId ?? null)
|
||||
const knownHosts = await getGlabKnownHosts(repo.connectionId ?? null, localWorktreeGitOptions)
|
||||
const projectRef = await getGitLabProjectRefForRemote(
|
||||
repo.path,
|
||||
remote,
|
||||
|
|
@ -18020,7 +18020,7 @@ export class OrcaRuntimeService {
|
|||
connectionId?: string | null,
|
||||
localGitOptions: { wslDistro?: string } = {}
|
||||
): Promise<string> {
|
||||
const knownHosts = await getGlabKnownHosts(connectionId)
|
||||
const knownHosts = await getGlabKnownHosts(connectionId, localGitOptions)
|
||||
const localGitOptionArgs =
|
||||
Object.keys(localGitOptions).length > 0 ? ([localGitOptions] as const) : []
|
||||
if (preference === 'origin') {
|
||||
|
|
|
|||
Loading…
Reference in New Issue