diff --git a/src/main/rate-limits/codex-fetcher-probe-shutdown.test.ts b/src/main/rate-limits/codex-fetcher-probe-shutdown.test.ts index 0a75c040ca4..ef4a36ac5ad 100644 --- a/src/main/rate-limits/codex-fetcher-probe-shutdown.test.ts +++ b/src/main/rate-limits/codex-fetcher-probe-shutdown.test.ts @@ -265,4 +265,74 @@ describe('fetchCodexRateLimits probe shutdown', () => { Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) } }) + + it.each([ + { failAt: 1, phase: 'initial request' }, + { failAt: 2, phase: 'initialized notification' }, + { failAt: 3, phase: 'rate-limit request' } + ])('keeps the home lock when the $phase stdin write throws', async ({ failAt, phase }) => { + const originalPlatform = process.platform + Object.defineProperty(process, 'platform', { configurable: true, value: 'darwin' }) + const firstChild = makeRpcChild() + let writeCount = 0 + firstChild.stdin.write.mockImplementation((line: string) => { + writeCount += 1 + if (writeCount === failAt) { + throw new Error(`${phase} stdin failed`) + } + const message = JSON.parse(line) as { id?: number; method?: string } + if (message.method === 'initialize') { + setTimeout(() => { + firstChild.stdout.emit( + 'data', + Buffer.from(`${JSON.stringify({ jsonrpc: '2.0', id: message.id, result: {} })}\n`) + ) + }, 0) + } + }) + firstChild.stdin.end = vi.fn() + firstChild.kill = vi.fn((signal?: string) => { + if (signal === 'SIGKILL') { + firstChild.exitCode = 1 + firstChild.emit('exit', 1, 'SIGKILL') + } + return true + }) + const secondChild = makeRpcChild() + childSpawnMock.mockReturnValueOnce(firstChild).mockReturnValueOnce(secondChild) + respondToRpcRateLimitRead(secondChild, { + primary: { usedPercent: 3 }, + secondary: { usedPercent: 4 } + }) + const home = `/managed/home-stdin-error-${failAt}` + + try { + const first = fetchCodexRateLimits({ + allowPtyFallback: false, + codexHomePath: home + }) + await vi.advanceTimersByTimeAsync(0) + expect(firstChild.kill).toHaveBeenCalledWith('SIGTERM') + + const second = fetchCodexRateLimits({ + allowPtyFallback: false, + codexHomePath: home + }) + await vi.advanceTimersByTimeAsync(4_999) + expect(childSpawnMock).toHaveBeenCalledTimes(1) + + await vi.advanceTimersByTimeAsync(1) + expect(firstChild.kill).toHaveBeenCalledWith('SIGKILL') + await vi.advanceTimersByTimeAsync(1) + expect(childSpawnMock).toHaveBeenCalledTimes(2) + await vi.advanceTimersByTimeAsync(1) + await expect(first).resolves.toMatchObject({ + status: 'error', + error: `${phase} stdin failed` + }) + await expect(second).resolves.toMatchObject({ status: 'ok' }) + } finally { + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + } + }) }) diff --git a/src/main/rate-limits/codex-fetcher.ts b/src/main/rate-limits/codex-fetcher.ts index bc7d198796f..32f57f7bde2 100644 --- a/src/main/rate-limits/codex-fetcher.ts +++ b/src/main/rate-limits/codex-fetcher.ts @@ -722,9 +722,17 @@ async function fetchViaRpc(options?: FetchCodexRateLimitsOptions): Promise { it('sends no signal during the drain window on Windows where kill is always forceful', async () => { const child = makeFakeChild() - const done = terminateCodexProbeChild(child, { platform: 'win32' }) + const killWindowsProcessTree = vi.fn(async () => {}) + const done = terminateCodexProbeChild(child, { platform: 'win32', killWindowsProcessTree }) expect(child.stdin.end).toHaveBeenCalledTimes(1) expect(child.kill).not.toHaveBeenCalled() await vi.advanceTimersByTimeAsync(CODEX_PROBE_SHUTDOWN_DRAIN_MS) + expect(killWindowsProcessTree).toHaveBeenCalledWith(child.pid) expect(child.kill).toHaveBeenCalledTimes(1) expect(child.kill).toHaveBeenCalledWith() @@ -102,6 +105,34 @@ describe('terminateCodexProbeChild', () => { await done }) + it('waits for the Windows descendant tree kill before releasing the probe', async () => { + const child = makeFakeChild() + let finishTreeKill!: () => void + const treeKill = new Promise((resolve) => { + finishTreeKill = resolve + }) + const killWindowsProcessTree = vi.fn(() => treeKill) + let settled = false + const done = terminateCodexProbeChild(child, { + platform: 'win32', + drainMs: 0, + killWindowsProcessTree + }).then(() => { + settled = true + }) + + await vi.advanceTimersByTimeAsync(0) + expect(killWindowsProcessTree).toHaveBeenCalledWith(child.pid) + child.exit() + await Promise.resolve() + expect(settled).toBe(false) + + finishTreeKill() + await done + expect(settled).toBe(true) + expect(child.kill).not.toHaveBeenCalled() + }) + it('does nothing for a child that already exited', async () => { const child = makeFakeChild() child.exit() diff --git a/src/main/rate-limits/codex-probe-termination.ts b/src/main/rate-limits/codex-probe-termination.ts index a3b421465d6..e5a2f966542 100644 --- a/src/main/rate-limits/codex-probe-termination.ts +++ b/src/main/rate-limits/codex-probe-termination.ts @@ -1,3 +1,5 @@ +import { terminateWindowsProcessTree, type WindowsTreeKiller } from '../windows-process-tree-kill' + // Why: Codex OAuth uses rotating refresh tokens. Hard-killing a probe's // app-server mid-refresh can strand auth.json between rotations and permanently // invalidate the stored credential, so termination always requests shutdown @@ -7,6 +9,7 @@ export const CODEX_PROBE_SHUTDOWN_DRAIN_MS = 5_000 const HARD_KILL_EXIT_WAIT_MS = 1_000 export type TerminatableProbeChild = { + pid?: number exitCode: number | null signalCode?: NodeJS.Signals | null kill: (signal?: NodeJS.Signals) => boolean @@ -21,6 +24,7 @@ export type TerminateCodexProbeOptions = { hardKillWaitMs?: number // Why: injectable so the Windows branch is testable from POSIX CI hosts. platform?: NodeJS.Platform + killWindowsProcessTree?: WindowsTreeKiller } function hasExited(child: TerminatableProbeChild): boolean { @@ -82,6 +86,18 @@ export async function terminateCodexProbeChild( if (await waitForExit(child, options?.drainMs ?? CODEX_PROBE_SHUTDOWN_DRAIN_MS)) { return } + if (platform === 'win32' && child.pid) { + try { + // npm-installed Codex runs beneath cmd.exe; killing only that wrapper can + // leave app-server alive after the credential-home lock is released. + await (options?.killWindowsProcessTree ?? terminateWindowsProcessTree)(child.pid) + } catch { + // The direct-child fallback still applies if an injected killer rejects. + } + } + if (hasExited(child)) { + return + } try { if (platform === 'win32') { child.kill()