From 2588f0aee8a8d4e4aa46462464d7bb9c86fa18e1 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 23 Aug 2026 14:01:38 -0700 Subject: [PATCH] fix(windows): stabilize interactive login fallback Force service tests onto the no-console branch, preserve windowsHide for noninteractive Claude commands, and keep the change scoped to managed account sign-in. --- src/cli/handlers/skills.ts | 7 +---- ...aude-account-service-login-process.test.ts | 16 ++++++++++++ .../claude-windows-interactive-login.test.ts | 26 +++++++++++++++++++ src/main/claude-accounts/service.ts | 7 +++-- .../codex-windows-interactive-login.test.ts | 12 +++++++++ 5 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/cli/handlers/skills.ts b/src/cli/handlers/skills.ts index cbd3c6d4cfa..4e8db5dbdae 100644 --- a/src/cli/handlers/skills.ts +++ b/src/cli/handlers/skills.ts @@ -15,7 +15,6 @@ import { UnsafeWindowsBatchArgumentsError, WINDOWS_BATCH_UNSAFE_CHARACTERS_LABEL } from '../../shared/windows-batch-spawn' -import { openWindowsInteractiveStdio } from '../../shared/windows-console-stdio' import { isSkillsCliAgentKeyShaped, toSkillsCliAgentKeys } from '../../shared/skills-cli-agent-keys' import { buildAgentFeatureSkillInstallArgs, @@ -144,14 +143,10 @@ function runNpxSkills(args: string[]): Promise { // Why: npx is an `#!/usr/bin/env node` script, so resolving it off PATH is // not enough — without node alongside it the child exits 127 with no // 'error' event and the message below never fires. - const consoleStdio = openWindowsInteractiveStdio() const child = spawn(spawnCmd, spawnArgs, { - stdio: consoleStdio === 'inherit' ? 'inherit' : consoleStdio.stdio, + stdio: 'inherit', env: { ...process.env, PATH: buildNpxPath(resolved) } }) - if (consoleStdio !== 'inherit') { - consoleStdio.dispose() - } // Why: a missing npx/Node on a headless host surfaces as a raw spawn ENOENT; // wrap it so the CLI reports an actionable message like every other failure here. child.once('error', (error) => { 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 1549acd494d..60095f49e09 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 @@ -517,7 +517,17 @@ describe('ClaudeAccountService credential capture', () => { const destroyStdin = vi.spyOn(child.stdin, 'destroy') const taskkill = new EventEmitter() const spawnMock = vi.fn((command: string) => (command === 'taskkill.exe' ? taskkill : child)) + const buildInteractiveLoginSpawn = vi.fn(() => ({ + command: getCmdExePath(), + args: ['/d', '/c', 'start', '', '/wait', 'claude', 'auth', 'login', '--claudeai'], + stdio: 'ignore' as const, + windowsHide: true, + dispose: vi.fn() + })) vi.doMock('node:child_process', () => ({ spawn: spawnMock })) + vi.doMock('../../shared/windows-interactive-login-spawn', () => ({ + buildWindowsHostInteractiveLoginSpawn: buildInteractiveLoginSpawn + })) try { const { ClaudeAccountService } = await import('./service') @@ -549,6 +559,11 @@ describe('ClaudeAccountService credential capture', () => { const addPromise = service.addAccount() await vi.waitFor(() => { + expect(buildInteractiveLoginSpawn).toHaveBeenCalledWith('claude', [ + 'auth', + 'login', + '--claudeai' + ]) expect(spawnMock).toHaveBeenCalledWith( getCmdExePath(), ['/d', '/c', 'start', '', '/wait', 'claude', 'auth', 'login', '--claudeai'], @@ -571,6 +586,7 @@ describe('ClaudeAccountService credential capture', () => { expect(service.cancelPendingLogin()).toBe(false) } finally { vi.doUnmock('node:child_process') + vi.doUnmock('../../shared/windows-interactive-login-spawn') } }) }) 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 2076658d3d9..76b28e1862f 100644 --- a/src/main/claude-accounts/claude-windows-interactive-login.test.ts +++ b/src/main/claude-accounts/claude-windows-interactive-login.test.ts @@ -35,7 +35,27 @@ describe('Claude Windows host interactive login', () => { queueMicrotask(() => child.emit('exit', 0)) return child }) + const buildInteractiveLoginSpawn = vi.fn(() => ({ + command: getCmdExePath(), + args: [ + '/d', + '/c', + 'start', + '', + '/wait', + 'C:\\Tools\\claude.cmd', + 'auth', + 'login', + '--claudeai' + ], + stdio: 'ignore' as const, + windowsHide: true, + dispose: vi.fn() + })) vi.doMock('node:child_process', () => ({ spawn: spawnMock })) + vi.doMock('../../shared/windows-interactive-login-spawn', () => ({ + buildWindowsHostInteractiveLoginSpawn: buildInteractiveLoginSpawn + })) try { const { ClaudeAccountService } = await import('./service') @@ -58,6 +78,11 @@ describe('Claude Windows host interactive login', () => { 1000 ) + expect(buildInteractiveLoginSpawn).toHaveBeenCalledWith('C:\\Tools\\claude.cmd', [ + 'auth', + 'login', + '--claudeai' + ]) expect(spawnMock).toHaveBeenCalledWith( getCmdExePath(), expect.arrayContaining(['start', '', '/wait']), @@ -69,6 +94,7 @@ describe('Claude Windows host interactive login', () => { expect(child.kill).not.toHaveBeenCalled() } finally { vi.doUnmock('node:child_process') + vi.doUnmock('../../shared/windows-interactive-login-spawn') } }) }) diff --git a/src/main/claude-accounts/service.ts b/src/main/claude-accounts/service.ts index 113707181d1..4e4427337e2 100644 --- a/src/main/claude-accounts/service.ts +++ b/src/main/claude-accounts/service.ts @@ -1106,11 +1106,10 @@ export class ClaudeAccountService { ? interactiveLogin.stdio : [options?.keepStdinOpen ? 'pipe' : 'ignore', 'pipe', 'pipe'], shell: spawnConfig.shell, - // On Windows this is wsl.exe or a cmd invocation; without the flag it - // opens a console and steals foreground on every managed login (#10488). - windowsHide: true, + // Hide noninteractive commands and the fallback wrapper; direct console + // login handles need the owning window to remain visible. + windowsHide: interactiveLogin?.windowsHide ?? true, windowsVerbatimArguments: spawnConfig.windowsVerbatimArguments, - windowsHide: interactiveLogin?.windowsHide, env: spawnConfig.env, // Why: Claude auth can leave browser/login descendants alive after denial. // A process group lets cancellation terminate the whole POSIX login tree. diff --git a/src/main/codex-accounts/codex-windows-interactive-login.test.ts b/src/main/codex-accounts/codex-windows-interactive-login.test.ts index 57a1d9a3163..ed2b9a34f06 100644 --- a/src/main/codex-accounts/codex-windows-interactive-login.test.ts +++ b/src/main/codex-accounts/codex-windows-interactive-login.test.ts @@ -46,6 +46,13 @@ describe('Codex Windows host interactive login', () => { queueMicrotask(() => child.emit('close', 0)) return child }) + const buildInteractiveLoginSpawn = vi.fn(() => ({ + command: getCmdExePath(), + args: ['/d', '/c', 'start', '', '/wait', 'C:\\Tools\\codex.cmd', 'login'], + stdio: 'ignore' as const, + windowsHide: true, + dispose: vi.fn() + })) vi.doMock('node:child_process', () => ({ execFileSync: vi.fn(), spawn: spawnMock @@ -53,6 +60,9 @@ describe('Codex Windows host interactive login', () => { vi.doMock('../codex-cli/command', () => ({ resolveCodexCommand: () => 'C:\\Tools\\codex.cmd' })) + vi.doMock('../../shared/windows-interactive-login-spawn', () => ({ + buildWindowsHostInteractiveLoginSpawn: buildInteractiveLoginSpawn + })) try { const { CodexAccountService } = await import('./service') @@ -67,6 +77,7 @@ describe('Codex Windows host interactive login', () => { } ).runCodexLogin(testState.fakeHomeDir) + expect(buildInteractiveLoginSpawn).toHaveBeenCalledWith('C:\\Tools\\codex.cmd', ['login']) expect(spawnMock).toHaveBeenCalledWith( getCmdExePath(), expect.arrayContaining(['start', '', '/wait']), @@ -80,6 +91,7 @@ describe('Codex Windows host interactive login', () => { Object.defineProperty(process, 'platform', originalPlatform) vi.doUnmock('node:child_process') vi.doUnmock('../codex-cli/command') + vi.doUnmock('../../shared/windows-interactive-login-spawn') } }) })