fix: run Codex usage RPC through Windows cmd shim correctly (#2104)

This commit is contained in:
Jinwoo Hong 2026-05-16 17:05:09 -04:00 committed by GitHub
parent 312677132e
commit 37e2be4064
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 80 additions and 14 deletions

View File

@ -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<typeof vi.fn> }
kill: ReturnType<typeof vi.fn>
}
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
})
})
})

View File

@ -428,7 +428,12 @@ export async function fetchCodexRateLimits(
): Promise<ProviderRateLimits> {
// 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
}

View File

@ -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

View File

@ -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', () => {

View File

@ -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 }
}