fix: clean up account login timeout listeners (#3745)

This commit is contained in:
Neil 2026-05-30 08:32:51 -07:00 committed by GitHub
parent 3d0c7b7fc4
commit f933c396c5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 158 additions and 19 deletions

View File

@ -1,8 +1,10 @@
/* eslint-disable max-lines -- test suite covers Claude capture and rollback edge cases */
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { EventEmitter } from 'node:events'
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { PassThrough } from 'node:stream'
import {
deleteActiveClaudeKeychainCredentialsStrict,
readActiveClaudeKeychainCredentials,
@ -803,4 +805,56 @@ describe('ClaudeAccountService credential capture', () => {
wslDistro: 'Ubuntu'
})
})
it('removes command listeners when Claude sign-in times out', async () => {
vi.resetModules()
vi.useFakeTimers()
const child = new EventEmitter() as EventEmitter & {
stdout: PassThrough
stderr: PassThrough
kill: () => void
}
child.stdout = new PassThrough()
child.stderr = new PassThrough()
child.kill = vi.fn()
const spawnMock = vi.fn(() => child)
vi.doMock('node:child_process', () => ({ spawn: spawnMock }))
try {
const { ClaudeAccountService } = await import('./service')
const service = new ClaudeAccountService(
createService() as never,
createService() as never,
createService() as never
)
const commandPromise = (
service as unknown as {
runClaudeCommand(
args: string[],
configDir: { windowsPath: string; linuxPath: string | null; wslDistro: string | null },
timeoutMs: number
): Promise<string>
}
).runClaudeCommand(
['login'],
{ windowsPath: '/tmp/claude-auth', linuxPath: null, wslDistro: null },
1000
)
const rejection = expect(commandPromise).rejects.toThrow(
'Claude sign-in took too long to finish.'
)
await vi.advanceTimersByTimeAsync(1000)
await rejection
expect(child.kill).toHaveBeenCalledTimes(1)
expect(child.stdout.listenerCount('data')).toBe(0)
expect(child.stderr.listenerCount('data')).toBe(0)
expect(child.listenerCount('error')).toBe(0)
expect(child.listenerCount('close')).toBe(0)
} finally {
vi.useRealTimers()
vi.doUnmock('node:child_process')
}
})
})

View File

@ -914,25 +914,35 @@ export class ClaudeAccountService {
output = output.slice(-MAX_COMMAND_OUTPUT_CHARS)
}
}
let timeout: ReturnType<typeof setTimeout> | null = null
const cleanupListeners = (): void => {
if (timeout) {
clearTimeout(timeout)
timeout = null
}
child.stdout.off('data', appendOutput)
child.stderr.off('data', appendOutput)
child.off('error', onError)
child.off('close', onClose)
}
const settle = (callback: () => void): void => {
if (settled) {
return
}
settled = true
clearTimeout(timeout)
cleanupListeners()
callback()
}
const timeout = setTimeout(() => {
const timeoutError = new Error('Claude sign-in took too long to finish.')
timeout = setTimeout(() => {
child.kill()
settle(() => rejectPromise(new Error('Claude sign-in took too long to finish.')))
settle(() => rejectPromise(timeoutError))
}, timeoutMs)
child.stdout.on('data', appendOutput)
child.stderr.on('data', appendOutput)
child.on('error', (error) => {
const onError = (error: Error): void => {
settle(() => rejectPromise(error))
})
child.on('close', (code) => {
}
const onClose = (code: number | null): void => {
settle(() => {
if (code === 0 || options?.allowFailure) {
resolvePromise(output)
@ -947,7 +957,12 @@ export class ClaudeAccountService {
)
)
})
})
}
child.stdout.on('data', appendOutput)
child.stderr.on('data', appendOutput)
child.on('error', onError)
child.on('close', onClose)
})
}

View File

@ -993,4 +993,59 @@ describe('CodexAccountService config sync', () => {
expect(rateLimits.refreshForCodexAccountChange).toHaveBeenCalledTimes(2)
})
it('removes command listeners when Codex login times out', async () => {
vi.resetModules()
vi.useFakeTimers()
const child = new EventEmitter() as EventEmitter & {
stdout: PassThrough
stderr: PassThrough
kill: () => void
}
child.stdout = new PassThrough()
child.stderr = new PassThrough()
child.kill = vi.fn()
const spawnMock = vi.fn(() => child)
vi.doMock('node:child_process', () => ({
execFileSync: vi.fn(),
spawn: spawnMock
}))
vi.doMock('../codex-cli/command', () => ({
resolveCodexCommand: () => 'codex'
}))
try {
const settings = createSettings()
const store = createStore(settings)
const rateLimits = createRateLimits()
const runtimeHome = createRuntimeHome()
const { CodexAccountService } = await import('./service')
const service = new CodexAccountService(
store as never,
rateLimits as never,
runtimeHome as never
)
const loginPromise = (
service as unknown as {
runCodexLogin(managedHomePath: string): Promise<void>
}
).runCodexLogin(testState.fakeHomeDir)
const rejection = expect(loginPromise).rejects.toThrow(
'Codex sign-in took too long to finish.'
)
await vi.advanceTimersByTimeAsync(120_000)
await rejection
expect(child.kill).toHaveBeenCalledTimes(1)
expect(child.stdout.listenerCount('data')).toBe(0)
expect(child.stderr.listenerCount('data')).toBe(0)
expect(child.listenerCount('error')).toBe(0)
expect(child.listenerCount('close')).toBe(0)
} finally {
vi.useRealTimers()
vi.doUnmock('node:child_process')
vi.doUnmock('../codex-cli/command')
}
})
})

View File

@ -734,26 +734,36 @@ export class CodexAccountService {
}
}
let timeout: ReturnType<typeof setTimeout> | null = null
const cleanupListeners = (): void => {
if (timeout) {
clearTimeout(timeout)
timeout = null
}
child.stdout.off('data', appendOutput)
child.stderr.off('data', appendOutput)
child.off('error', onError)
child.off('close', onClose)
}
const settle = (callback: () => void): void => {
if (settled) {
return
}
settled = true
clearTimeout(timeout)
cleanupListeners()
callback()
}
const timeout = setTimeout(() => {
const timeoutError = new Error('Codex sign-in took too long to finish. Please try again.')
timeout = setTimeout(() => {
child.kill()
settle(() => {
rejectPromise(new Error('Codex sign-in took too long to finish. Please try again.'))
rejectPromise(timeoutError)
})
}, LOGIN_TIMEOUT_MS)
child.stdout.on('data', appendOutput)
child.stderr.on('data', appendOutput)
child.on('error', (error) => {
const onError = (error: Error): void => {
settle(() => {
const isEnoent = (error as NodeJS.ErrnoException).code === 'ENOENT'
// Why: ENOENT can mean either the codex binary doesn't exist OR the
@ -767,9 +777,9 @@ export class CodexAccountService {
: error.message
rejectPromise(new Error(message))
})
})
}
child.on('close', (code) => {
const onClose = (code: number | null): void => {
settle(() => {
if (code === 0) {
resolvePromise()
@ -784,7 +794,12 @@ export class CodexAccountService {
)
)
})
})
}
child.stdout.on('data', appendOutput)
child.stderr.on('data', appendOutput)
child.on('error', onError)
child.on('close', onClose)
})
}