diff --git a/src/main/rate-limits/codex-fetcher.test.ts b/src/main/rate-limits/codex-fetcher.test.ts index cc37c1335..cdeb1fa7f 100644 --- a/src/main/rate-limits/codex-fetcher.test.ts +++ b/src/main/rate-limits/codex-fetcher.test.ts @@ -1,3 +1,4 @@ +import { EventEmitter } from 'node:events' import { beforeEach, describe, expect, it, vi } from 'vitest' const { childSpawnMock, resolveCodexCommandMock, ptySpawnMock } = vi.hoisted(() => ({ @@ -24,6 +25,20 @@ function makeDisposable() { return { dispose: vi.fn() } } +function makeRpcChild() { + const child = new EventEmitter() as EventEmitter & { + stdout: EventEmitter + stderr: EventEmitter + stdin: { write: ReturnType } + kill: ReturnType + } + child.stdout = new EventEmitter() + child.stderr = new EventEmitter() + child.stdin = { write: vi.fn() } + child.kill = vi.fn() + return child +} + describe('fetchCodexRateLimits', () => { beforeEach(() => { vi.useFakeTimers() @@ -57,4 +72,41 @@ describe('fetchCodexRateLimits', () => { term.kill.mock.invocationCallOrder[0] ) }) + + it('falls back to the PTY status reader when RPC exits before returning usage', async () => { + const rpcChild = makeRpcChild() + const ptyHandlers: { onData?: (data: string) => void } = {} + + childSpawnMock.mockReturnValue(rpcChild) + ptySpawnMock.mockReturnValue({ + onData: vi.fn((callback) => { + ptyHandlers.onData = callback + return makeDisposable() + }), + onExit: vi.fn(() => makeDisposable()), + write: vi.fn(), + kill: vi.fn() + }) + + const resultPromise = fetchCodexRateLimits() + rpcChild.emit('close') + await vi.advanceTimersByTimeAsync(0) + + expect(ptySpawnMock).toHaveBeenCalled() + const onPtyData = ptyHandlers.onData + if (!onPtyData) { + throw new Error('PTY data handler was not registered') + } + onPtyData('>') + onPtyData('5h limit: 7%\nWeekly limit: 12%\n') + await vi.advanceTimersByTimeAsync(500) + + await expect(resultPromise).resolves.toMatchObject({ + provider: 'codex', + session: { usedPercent: 7 }, + weekly: { usedPercent: 12 }, + status: 'ok', + error: null + }) + }) }) diff --git a/src/main/rate-limits/codex-fetcher.ts b/src/main/rate-limits/codex-fetcher.ts index 0c1cd50a3..486e0e748 100644 --- a/src/main/rate-limits/codex-fetcher.ts +++ b/src/main/rate-limits/codex-fetcher.ts @@ -428,7 +428,12 @@ export async function fetchCodexRateLimits( ): Promise { // Path A: try RPC first try { - return await fetchViaRpc(options) + const rpcResult = await fetchViaRpc(options) + if (rpcResult.status === 'ok' || rpcResult.status === 'unavailable') { + return rpcResult + } + // Why: app-server can fail independently of the interactive CLI. Keep the + // status-bar useful by trying the older /status PTY reader on RPC errors. } catch { // RPC failed — fall through to PTY } diff --git a/src/main/text-generation/commit-message-text-generation.test.ts b/src/main/text-generation/commit-message-text-generation.test.ts index e9e5b5f42..709e254b5 100644 --- a/src/main/text-generation/commit-message-text-generation.test.ts +++ b/src/main/text-generation/commit-message-text-generation.test.ts @@ -432,7 +432,7 @@ describe('generateCommitMessageFromContext', () => { }) expect(spawnMock).toHaveBeenCalledWith( 'C:\\Windows\\System32\\cmd.exe', - ['/d', '/s', '/c', '"C:/tools/agent.cmd"'], + ['/d', '/c', 'C:/tools/agent.cmd'], expect.objectContaining({ cwd: 'C:\\repo', windowsHide: true diff --git a/src/main/win32-utils.test.ts b/src/main/win32-utils.test.ts index cf4efb4cd..f80fbe687 100644 --- a/src/main/win32-utils.test.ts +++ b/src/main/win32-utils.test.ts @@ -44,9 +44,9 @@ describe('getSpawnArgsForWindows', () => { '--foo' ]) expect(spawnCmd).toBe('C:\\Windows\\System32\\cmd.exe') - // Why: /d disables AutoRun; /s preserves quoted command-line parsing; - // /c runs the quoted batch command and exits. - expect(spawnArgs).toEqual(['/d', '/s', '/c', '"C:\\tools\\codex.cmd" "login" "--foo"']) + // Why: /d disables AutoRun; /c runs the batch command and exits. + // Separate argv entries avoid cmd.exe seeing Node-escaped quotes. + expect(spawnArgs).toEqual(['/d', '/c', 'C:\\tools\\codex.cmd', 'login', '--foo']) }) } finally { if (originalComSpec === undefined) { @@ -80,6 +80,14 @@ describe('getSpawnArgsForWindows', () => { ) }) }) + + it('rejects unsafe command paths for .cmd scripts on win32', () => { + withPlatform('win32', () => { + expect(() => getSpawnArgsForWindows('C:\\bad&path\\agent.cmd', ['login'])).toThrow( + 'UNSAFE_WINDOWS_BATCH_ARGUMENTS' + ) + }) + }) }) describe('isPermissionError', () => { diff --git a/src/main/win32-utils.ts b/src/main/win32-utils.ts index edddf2eef..7f577e55f 100644 --- a/src/main/win32-utils.ts +++ b/src/main/win32-utils.ts @@ -35,13 +35,6 @@ function hasUnsafeWindowsBatchSyntax(value: string): boolean { return /[&|<>^"%!\r\n]/.test(value) } -function quoteWindowsBatchToken(value: string): string { - if (hasUnsafeWindowsBatchSyntax(value)) { - throw new UnsafeWindowsBatchArgumentsError() - } - return `"${value}"` -} - /** Check whether an error is a Windows permission error (EACCES or EPERM). */ export function isPermissionError(error: unknown): boolean { return ( @@ -132,8 +125,16 @@ export function getSpawnArgsForWindows( args: string[] ): { spawnCmd: string; spawnArgs: string[] } { if (isWindowsBatchScript(command)) { - const commandLine = [command, ...args].map(quoteWindowsBatchToken).join(' ') - return { spawnCmd: getCmdExePath(), spawnArgs: ['/d', '/s', '/c', commandLine] } + for (const value of [command, ...args]) { + if (hasUnsafeWindowsBatchSyntax(value)) { + throw new UnsafeWindowsBatchArgumentsError() + } + } + + // Why: when Node passes a pre-quoted command line as one argv entry, + // cmd.exe sees literal escaped quotes on Windows and refuses to run .cmd + // shims. Separate argv entries let Node quote spaces without breaking cmd. + return { spawnCmd: getCmdExePath(), spawnArgs: ['/d', '/c', command, ...args] } } return { spawnCmd: command, spawnArgs: args } }