mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 08:03:39 +00:00
fix(windows): close interactive login cleanup races
This commit is contained in:
@@ -518,13 +518,18 @@ describe('ClaudeAccountService credential capture', () => {
|
||||
const taskkill = new EventEmitter()
|
||||
const spawnMock = vi.fn((command: string) => (command === 'taskkill.exe' ? taskkill : child))
|
||||
const cleanupInteractiveLogin = vi.fn()
|
||||
let publishTerminationPid: (pid: number) => void = () => {}
|
||||
const buildInteractiveLoginSpawn = vi.fn(() => ({
|
||||
command: getCmdExePath(),
|
||||
args: ['/d', '/c', 'start', '', '/wait', 'claude', 'auth', 'login', '--claudeai'],
|
||||
stdio: 'ignore' as const,
|
||||
windowsHide: true,
|
||||
cleanup: cleanupInteractiveLogin,
|
||||
getTerminationPid: () => 9876
|
||||
getTerminationPid: () => null,
|
||||
waitForTerminationPid: () =>
|
||||
new Promise<number>((resolve) => {
|
||||
publishTerminationPid = resolve
|
||||
})
|
||||
}))
|
||||
vi.doMock('node:child_process', () => ({ spawn: spawnMock }))
|
||||
vi.doMock('../../shared/windows-interactive-login-spawn', () => ({
|
||||
@@ -576,11 +581,19 @@ describe('ClaudeAccountService credential capture', () => {
|
||||
expect(service.cancelPendingLogin()).toBe(true)
|
||||
const rejection = expect(addPromise).rejects.toThrow('Claude sign-in was cancelled.')
|
||||
expect(child.kill).not.toHaveBeenCalled()
|
||||
expect(spawnMock).toHaveBeenCalledWith(
|
||||
expect(spawnMock).not.toHaveBeenCalledWith(
|
||||
'taskkill.exe',
|
||||
['/pid', '9876', '/t', '/f'],
|
||||
expect.objectContaining({ stdio: 'ignore', windowsHide: true })
|
||||
expect.anything(),
|
||||
expect.anything()
|
||||
)
|
||||
publishTerminationPid(9876)
|
||||
await vi.waitFor(() => {
|
||||
expect(spawnMock).toHaveBeenCalledWith(
|
||||
'taskkill.exe',
|
||||
['/pid', '9876', '/t', '/f'],
|
||||
expect.objectContaining({ stdio: 'ignore', windowsHide: true })
|
||||
)
|
||||
})
|
||||
expect(destroyStdin).not.toHaveBeenCalled()
|
||||
taskkill.emit('close', 0)
|
||||
await rejection
|
||||
|
||||
@@ -136,7 +136,7 @@ describe('Claude Windows host interactive login', () => {
|
||||
)
|
||||
const rejection = expect(login).rejects.toThrow('Claude sign-in took too long to finish.')
|
||||
|
||||
await vi.advanceTimersByTimeAsync(1000)
|
||||
await vi.advanceTimersByTimeAsync(3_000)
|
||||
|
||||
await rejection
|
||||
expect(child.kill).toHaveBeenCalledOnce()
|
||||
|
||||
@@ -1107,7 +1107,7 @@ export class ClaudeAccountService {
|
||||
: [options?.keepStdinOpen ? 'pipe' : 'ignore', 'pipe', 'pipe'],
|
||||
shell: spawnConfig.shell,
|
||||
// Why: hide only the outer wrapper; start creates the visible login console.
|
||||
windowsHide: interactiveLogin?.windowsHide ?? true,
|
||||
windowsHide: true,
|
||||
windowsVerbatimArguments: spawnConfig.windowsVerbatimArguments,
|
||||
env: spawnConfig.env,
|
||||
// Why: Claude auth can leave browser/login descendants alive after denial.
|
||||
@@ -1181,8 +1181,7 @@ export class ClaudeAccountService {
|
||||
return
|
||||
}
|
||||
terminationPending = true
|
||||
const windowsTerminationPid = interactiveLogin?.getTerminationPid?.() ?? child.pid
|
||||
if (process.platform === 'win32' && windowsTerminationPid) {
|
||||
const killWindowsTree = (windowsTerminationPid: number): void => {
|
||||
const taskkill = spawn(
|
||||
'taskkill.exe',
|
||||
['/pid', String(windowsTerminationPid), '/t', '/f'],
|
||||
@@ -1209,9 +1208,27 @@ export class ClaudeAccountService {
|
||||
}, WINDOWS_TASKKILL_TIMEOUT_MS)
|
||||
taskkill.once('error', () => finishTaskkill(false))
|
||||
taskkill.once('close', (code) => finishTaskkill(code === 0))
|
||||
}
|
||||
if (process.platform === 'win32') {
|
||||
const resolveTerminationPid = interactiveLogin?.waitForTerminationPid
|
||||
? interactiveLogin.waitForTerminationPid()
|
||||
: Promise.resolve(interactiveLogin?.getTerminationPid?.() ?? child.pid ?? null)
|
||||
void resolveTerminationPid
|
||||
.then((windowsTerminationPid) => {
|
||||
if (windowsTerminationPid) {
|
||||
killWindowsTree(windowsTerminationPid)
|
||||
return
|
||||
}
|
||||
child.kill()
|
||||
afterKill()
|
||||
})
|
||||
.catch(() => {
|
||||
child.kill()
|
||||
afterKill()
|
||||
})
|
||||
return
|
||||
}
|
||||
if (process.platform !== 'win32' && child.pid) {
|
||||
if (child.pid) {
|
||||
try {
|
||||
process.kill(-child.pid)
|
||||
afterKill()
|
||||
|
||||
@@ -1749,7 +1749,7 @@ export class CodexAccountService {
|
||||
? spawnConfig.interactiveLogin.stdio
|
||||
: ['ignore', 'pipe', 'pipe'],
|
||||
// Why: hide the outer wrapper only. A dedicated login console stays visible.
|
||||
windowsHide: spawnConfig.interactiveLogin?.windowsHide ?? true,
|
||||
windowsHide: true,
|
||||
env: spawnConfig.env
|
||||
})
|
||||
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
import type { StdioOptions } from 'node:child_process'
|
||||
import { closeSync as fsCloseSync, openSync as fsOpenSync } from 'node:fs'
|
||||
|
||||
export const WINDOWS_CONSOLE_INPUT_DEVICE = '\\\\.\\CONIN$'
|
||||
@@ -14,6 +13,12 @@ type OpenWindowsConsoleInputDeps = {
|
||||
closeSync?: (fd: number) => void
|
||||
}
|
||||
|
||||
type WindowsInteractiveChildStdio = [
|
||||
number | 'inherit',
|
||||
typeof process.stderr | 'inherit',
|
||||
'inherit'
|
||||
]
|
||||
|
||||
/** Opens the real Windows console input device instead of Electron's inherited stdin. */
|
||||
export function openWindowsConsoleInput(
|
||||
deps: OpenWindowsConsoleInputDeps = {}
|
||||
@@ -49,7 +54,7 @@ export function openWindowsConsoleInput(
|
||||
export function stdioForWindowsInteractiveChild(
|
||||
json: boolean,
|
||||
deps: OpenWindowsConsoleInputDeps = {}
|
||||
): { stdio: StdioOptions; dispose: () => void } {
|
||||
): { stdio: WindowsInteractiveChildStdio; dispose: () => void } {
|
||||
const opened = openWindowsConsoleInput(deps)
|
||||
return opened === 'inherit'
|
||||
? {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { existsSync, writeFileSync } from 'node:fs'
|
||||
import { getCmdExePath } from './windows-batch-spawn'
|
||||
import { buildWindowsHostInteractiveLoginSpawn } from './windows-interactive-login-spawn'
|
||||
@@ -17,6 +17,17 @@ function encodedValue(value: string): string {
|
||||
return `Read-OrcaValue '${Buffer.from(value).toString('base64')}'`
|
||||
}
|
||||
|
||||
function pidFilePathFromSpawnArgs(args: string[]): string {
|
||||
const script = Buffer.from(args[11] ?? '', 'base64').toString('utf16le')
|
||||
const encodedPath = script.match(
|
||||
/WriteAllText\(\(Read-OrcaValue '([^']+)'\), \[string\]\$PID\)/
|
||||
)?.[1]
|
||||
if (!encodedPath) {
|
||||
throw new Error('PID relay path is missing')
|
||||
}
|
||||
return Buffer.from(encodedPath, 'base64').toString('utf8')
|
||||
}
|
||||
|
||||
describe('buildWindowsHostInteractiveLoginSpawn', () => {
|
||||
it('relays a batch login through a visible, PID-addressable console', () => {
|
||||
const spawn = withWindows(() =>
|
||||
@@ -42,11 +53,7 @@ describe('buildWindowsHostInteractiveLoginSpawn', () => {
|
||||
expect(script).toContain(encodedValue(getCmdExePath()))
|
||||
expect(script).toContain(encodedValue('C:\\Tools\\claude.cmd'))
|
||||
expect(script).toContain(encodedValue('--claudeai'))
|
||||
const pidPathBase64 = script.match(
|
||||
/WriteAllText\(\(Read-OrcaValue '([^']+)'\), \[string\]\$PID\)/
|
||||
)?.[1]
|
||||
expect(pidPathBase64).toBeTruthy()
|
||||
const pidFilePath = Buffer.from(pidPathBase64!, 'base64').toString('utf8')
|
||||
const pidFilePath = pidFilePathFromSpawnArgs(spawn.args)
|
||||
|
||||
expect(spawn.stdio).toBe('ignore')
|
||||
expect(spawn.windowsHide).toBe(true)
|
||||
@@ -66,4 +73,25 @@ describe('buildWindowsHostInteractiveLoginSpawn', () => {
|
||||
expect(script).toContain(encodedValue('login'))
|
||||
spawn.cleanup()
|
||||
})
|
||||
|
||||
it('waits for the relay PID when cancellation races console startup', async () => {
|
||||
vi.useFakeTimers()
|
||||
const spawn = withWindows(() =>
|
||||
buildWindowsHostInteractiveLoginSpawn('C:\\Tools\\claude.exe', ['auth', 'login'])
|
||||
)
|
||||
try {
|
||||
const pendingPid = spawn.waitForTerminationPid()
|
||||
expect(spawn.getTerminationPid()).toBeNull()
|
||||
|
||||
await vi.advanceTimersByTimeAsync(100)
|
||||
const pidFilePath = pidFilePathFromSpawnArgs(spawn.args)
|
||||
writeFileSync(pidFilePath, '8642')
|
||||
await vi.advanceTimersByTimeAsync(25)
|
||||
|
||||
await expect(pendingPid).resolves.toBe(8642)
|
||||
} finally {
|
||||
spawn.cleanup()
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -11,8 +11,12 @@ export type WindowsHostInteractiveLoginSpawn = {
|
||||
windowsHide: boolean
|
||||
cleanup: () => void
|
||||
getTerminationPid: () => number | null
|
||||
waitForTerminationPid: () => Promise<number | null>
|
||||
}
|
||||
|
||||
const PID_RELAY_WAIT_TIMEOUT_MS = 2_000
|
||||
const PID_RELAY_POLL_INTERVAL_MS = 25
|
||||
|
||||
function encodeUtf8(value: string): string {
|
||||
return Buffer.from(value, 'utf8').toString('base64')
|
||||
}
|
||||
@@ -41,6 +45,25 @@ function readPidFile(pidFilePath: string): number | null {
|
||||
}
|
||||
}
|
||||
|
||||
function waitForPidFile(pidFilePath: string): Promise<number | null> {
|
||||
const current = readPidFile(pidFilePath)
|
||||
if (current !== null) {
|
||||
return Promise.resolve(current)
|
||||
}
|
||||
const deadline = Date.now() + PID_RELAY_WAIT_TIMEOUT_MS
|
||||
return new Promise((resolve) => {
|
||||
const poll = (): void => {
|
||||
const pid = readPidFile(pidFilePath)
|
||||
if (pid !== null || Date.now() >= deadline) {
|
||||
resolve(pid)
|
||||
return
|
||||
}
|
||||
setTimeout(poll, PID_RELAY_POLL_INTERVAL_MS)
|
||||
}
|
||||
setTimeout(poll, PID_RELAY_POLL_INTERVAL_MS)
|
||||
})
|
||||
}
|
||||
|
||||
export function buildWindowsHostInteractiveLoginSpawn(
|
||||
command: string,
|
||||
args: string[]
|
||||
@@ -69,6 +92,7 @@ export function buildWindowsHostInteractiveLoginSpawn(
|
||||
stdio: 'ignore',
|
||||
windowsHide: true,
|
||||
cleanup: () => rmSync(pidFilePath, { force: true }),
|
||||
getTerminationPid: () => readPidFile(pidFilePath)
|
||||
getTerminationPid: () => readPidFile(pidFilePath),
|
||||
waitForTerminationPid: () => waitForPidFile(pidFilePath)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user