diff --git a/src/main/claude-accounts/claude-account-service-login-process.test.ts b/src/main/claude-accounts/claude-account-service-login-process.test.ts index 69bc3793608..43e051a95fd 100644 --- a/src/main/claude-accounts/claude-account-service-login-process.test.ts +++ b/src/main/claude-accounts/claude-account-service-login-process.test.ts @@ -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((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 diff --git a/src/main/claude-accounts/claude-windows-interactive-login.test.ts b/src/main/claude-accounts/claude-windows-interactive-login.test.ts index cdacdc9e187..69ca24c21a8 100644 --- a/src/main/claude-accounts/claude-windows-interactive-login.test.ts +++ b/src/main/claude-accounts/claude-windows-interactive-login.test.ts @@ -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() diff --git a/src/main/claude-accounts/service.ts b/src/main/claude-accounts/service.ts index 72aea7a04c7..88e3712fde8 100644 --- a/src/main/claude-accounts/service.ts +++ b/src/main/claude-accounts/service.ts @@ -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() diff --git a/src/main/codex-accounts/service.ts b/src/main/codex-accounts/service.ts index 2a4ef384f80..5fd0a5ef649 100644 --- a/src/main/codex-accounts/service.ts +++ b/src/main/codex-accounts/service.ts @@ -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 }) diff --git a/src/shared/windows-console-input.ts b/src/shared/windows-console-input.ts index 0bc84780b5b..0b846276a17 100644 --- a/src/shared/windows-console-input.ts +++ b/src/shared/windows-console-input.ts @@ -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' ? { diff --git a/src/shared/windows-interactive-login-spawn.test.ts b/src/shared/windows-interactive-login-spawn.test.ts index a89cfbd8e58..07cde964095 100644 --- a/src/shared/windows-interactive-login-spawn.test.ts +++ b/src/shared/windows-interactive-login-spawn.test.ts @@ -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() + } + }) }) diff --git a/src/shared/windows-interactive-login-spawn.ts b/src/shared/windows-interactive-login-spawn.ts index b2e3f19d7ff..1068607e38f 100644 --- a/src/shared/windows-interactive-login-spawn.ts +++ b/src/shared/windows-interactive-login-spawn.ts @@ -11,8 +11,12 @@ export type WindowsHostInteractiveLoginSpawn = { windowsHide: boolean cleanup: () => void getTerminationPid: () => number | null + waitForTerminationPid: () => Promise } +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 { + 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) } }