fix(codex): remove redundant Windows hook launcher for Unicode profiles (#20952)

* fix(codex): reuse the Windows hook shell for Unicode profile paths

* test(codex): register Unicode hook tests in Windows CI

* test(codex): pin trust hash replacement during Windows upgrade

* test(codex): retry transient Windows teardown locks
This commit is contained in:
Neil
2026-09-16 00:30:52 -07:00
committed by GitHub
parent 78609330d1
commit d62328aa4d
9 changed files with 295 additions and 31 deletions
+2
View File
@@ -888,6 +888,8 @@ jobs:
src/shared/child-process/windows-cmd-shim-resolution.win32.test.ts
src/main/agent-hooks/windows-hook-payload-delivery.test.ts
src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts
src/main/codex/windows-hook-command.test.ts
src/main/codex/windows-hook-upgrade.test.ts
src/main/windows/windows-pty-job.win32.test.ts
src/main/windows/windows-msys-job.win32.test.ts
src/main/windows/windows-host-job.win32.test.ts
+2
View File
@@ -223,6 +223,8 @@ const WINDOWS_PACKAGE_TESTS = [
'src/shared/child-process/windows-cmd-shim-resolution.win32.test.ts',
'src/main/agent-hooks/windows-hook-payload-delivery.test.ts',
'src/main/agent-hooks/windows-direct-cmd-hook-command.test.ts',
'src/main/codex/windows-hook-command.test.ts',
'src/main/codex/windows-hook-upgrade.test.ts',
'src/main/windows/windows-pty-job.win32.test.ts',
'src/main/windows/windows-msys-job.win32.test.ts',
'src/main/windows/windows-host-job.win32.test.ts',
+2 -10
View File
@@ -707,10 +707,7 @@ describe('wrapWindowsHookCommand', () => {
describe('wrapWindowsCmdHookCommand', () => {
it('returns the bare, directly-spawnable path for a cmd-safe managed script', () => {
// Why: Codex/Antigravity/Devin launch the command as a program (argv[0]),
// not via cmd.exe, so the launcher must be a single spawnable token — a bare
// .cmd path. A cmd-builtin `if …` launcher has argv[0] = `if`, which is
// unspawnable and fails every hook with exit 1 (#8430 regression).
// Direct-spawn consumers need a launchable argv[0], not a cmd builtin such as `if`.
const scriptPath = 'C:\\Users\\alice\\.orca\\agent-hooks\\codex-hook.cmd'
const command = wrapWindowsCmdHookCommand(scriptPath)
expect(command).toBe(scriptPath)
@@ -721,12 +718,7 @@ describe('wrapWindowsCmdHookCommand', () => {
it.skipIf(process.platform !== 'win32')(
'resolves the launcher to a real executable file, not a shell fragment',
() => {
// Regression guard for #8430: Codex/Antigravity/Devin spawn the launcher as
// a program (argv[0]), so it must be an existing, launchable file. The broken
// `if exist … (call …)` form had argv[0] = `if` — a cmd builtin, not a file —
// which is unspawnable and failed every hook. The bare path is the file.
// win32-only: the real temp path is cmd-safe only with backslashes; a POSIX
// tmpDir has `/`, which routes to the encoded fallback by design.
// POSIX temp paths contain `/`, which selects the encoded fallback instead.
const scriptPath = join(tmpDir, 'codex-hook.cmd')
writeFileSync(scriptPath, '@echo off\r\nexit /b 0\r\n', 'utf-8')
const command = wrapWindowsCmdHookCommand(scriptPath)
+12 -3
View File
@@ -118,6 +118,16 @@ export {
} from './windows-powershell-hook-launcher'
export function wrapWindowsHookCommand(
scriptPath: string,
env: Record<string, string> = {},
options: { fallbackStdout?: string } = {}
): string {
return wrapWindowsPowerShellEncodedCommand(
buildWindowsHookPowerShellCommand(scriptPath, env, options)
)
}
export function buildWindowsHookPowerShellCommand(
scriptPath: string,
env: Record<string, string> = {},
// Why: POSIX wrap already answers missing-script with stdout; Windows must match so gate events cannot drift (#15462).
@@ -135,14 +145,13 @@ export function wrapWindowsHookCommand(
// Why the order: answer first (a gate event reads silence as deny), then the shared
// env guard, and only then own stdin — outside an Orca pane the caller may abandon the
// pipe, and ReadToEnd would strand the launcher there forever (#11549).
const command = `${envPrefix}if (Test-Path -LiteralPath ${quoted} -PathType Leaf) { & ${quoted}; exit $LASTEXITCODE }; ${fallback}${WINDOWS_POWERSHELL_HOOK_ENVIRONMENT_GUARD}; [Console]::In.ReadToEnd() | Out-Null; exit 0`
return wrapWindowsPowerShellEncodedCommand(command)
return `${envPrefix}if (Test-Path -LiteralPath ${quoted} -PathType Leaf) { & ${quoted}; exit $LASTEXITCODE }; ${fallback}${WINDOWS_POWERSHELL_HOOK_ENVIRONMENT_GUARD}; [Console]::In.ReadToEnd() | Out-Null; exit 0`
}
export const WINDOWS_CMD_SAFE_PATH = /^[A-Za-z0-9_.:\\~-]+$/
export function wrapWindowsCmdHookCommand(scriptPath: string): string {
// Why: Codex/Antigravity/Devin spawn the hook as argv[0], not via cmd.exe, so it must be one spawnable token; a cmd `if exist` launcher isn't (#8430).
// Direct-spawn consumers need one executable token; a cmd `if exist` fragment is not one (#8430).
return WINDOWS_CMD_SAFE_PATH.test(scriptPath) ? scriptPath : wrapWindowsHookCommand(scriptPath)
}
@@ -182,7 +182,12 @@ describe('managed hook command contract', () => {
expect(commands.length).toBeGreaterThan(0)
for (const command of commands) {
expect(command.length).toBeGreaterThan(0)
expect(findBareHookCommandVariables(command), command).toEqual([])
// Native Windows Codex evaluates PowerShell variables without Grok's dollar-byte scanner.
const scannedCommand =
agent === 'codex' && platform === 'win32' && command.startsWith('if (Test-Path')
? command.replaceAll('$LASTEXITCODE', '').replaceAll('$env:', '')
: command
expect(findBareHookCommandVariables(scannedCommand), command).toEqual([])
}
})
})
+9 -4
View File
@@ -1,8 +1,9 @@
import { join } from 'node:path'
import {
getSharedManagedScriptPath,
buildWindowsHookPowerShellCommand,
wrapPosixHookCommand,
wrapWindowsCmdHookCommand,
WINDOWS_CMD_SAFE_PATH,
writeHooksJson,
type HookDefinition
} from '../agent-hooks/installer-utils'
@@ -70,9 +71,13 @@ export function getManagedScriptPath(): string {
}
export function getManagedCommand(scriptPath: string): string {
return process.platform === 'win32'
? wrapWindowsCmdHookCommand(scriptPath)
: wrapPosixHookCommand(scriptPath)
if (process.platform !== 'win32') {
return wrapPosixHookCommand(scriptPath)
}
// Codex's default native Windows hook host is PowerShell; reuse it to avoid a second interpreter.
return WINDOWS_CMD_SAFE_PATH.test(scriptPath)
? scriptPath
: buildWindowsHookPowerShellCommand(scriptPath)
}
export type CodexManagedHookInstallMaterial = {
@@ -28,11 +28,9 @@ vi.mock('os', async (importOriginal) => {
})
import { CodexHookService } from './hook-service'
import { buildWindowsHookPowerShellCommand } from '../agent-hooks/installer-utils'
import { runExclusivelyForCodexTrustConfig } from './codex-trust-config-mutation-queue'
const WINDOWS_POWERSHELL_LAUNCHER =
/^[A-Za-z]:\/[^"]*\/System32\/WindowsPowerShell\/v1\.0\/powershell\.exe -NoProfile -EncodedCommand \S+$/
const homes = setupCodexHookHomes(homedirMock, getPathMock)
function localManagedCodexEvents(): string[] {
@@ -184,10 +182,7 @@ describe('CodexHookService', () => {
expect(Object.keys(hooksConfig)).toEqual(['hooks'])
})
// Why: #6078 — a Windows user profile path like `C:\Users\Jane Doe` used to
// be written verbatim as the hook command, so Codex split it at the space and
// the hook exited with code 1. Keep spaced paths on the encoded launcher so
// `cmd.exe /C` never sees the raw script path.
// #6078: the existing PowerShell host must still quote spaced profile paths.
it.skipIf(process.platform !== 'win32')(
'wraps the managed hook command when the profile path contains a space (#6078)',
async () => {
@@ -208,7 +203,11 @@ describe('CodexHookService', () => {
for (const eventName of localManagedCodexEvents()) {
const command = hooksConfig.hooks[eventName]?.[0]?.hooks?.[0]?.command
expect(command).toMatch(WINDOWS_POWERSHELL_LAUNCHER)
expect(command).toBe(
buildWindowsHookPowerShellCommand(
join(homedir(), '.orca', 'agent-hooks', 'codex-hook.cmd')
)
)
}
} finally {
rmSync(spaceHome, { recursive: true, force: true })
@@ -216,10 +215,9 @@ describe('CodexHookService', () => {
}
)
// Why: cmd.exe expands `%` and treats `^` as an escape even inside otherwise
// plausible paths. Keep those rare cases on the encoded launcher from #6078.
// Preserve literal-path quoting when constructing commands for shell metacharacters.
it.skipIf(process.platform !== 'win32')(
'keeps the encoded launcher when the profile path contains cmd metacharacters',
'quotes the script path when the profile contains cmd metacharacters',
async () => {
const metacharHome = join(tmpdir(), 'orca %ORCA_TEST% ^ home')
mkdirSync(metacharHome, { recursive: true })
@@ -238,7 +236,11 @@ describe('CodexHookService', () => {
for (const eventName of localManagedCodexEvents()) {
const command = hooksConfig.hooks[eventName]?.[0]?.hooks?.[0]?.command
expect(command).toMatch(WINDOWS_POWERSHELL_LAUNCHER)
expect(command).toBe(
buildWindowsHookPowerShellCommand(
join(homedir(), '.orca', 'agent-hooks', 'codex-hook.cmd')
)
)
}
} finally {
rmSync(metacharHome, { recursive: true, force: true })
@@ -268,7 +270,11 @@ describe('CodexHookService', () => {
expect(command).not.toMatch(/powershell/i)
expect(command).toMatch(/\\agent-hooks\\codex-hook\.cmd$/)
} else {
expect(command).toMatch(WINDOWS_POWERSHELL_LAUNCHER)
expect(command).toBe(
buildWindowsHookPowerShellCommand(
join(homedir(), '.orca', 'agent-hooks', 'codex-hook.cmd')
)
)
}
}
)
+146
View File
@@ -0,0 +1,146 @@
import { afterEach, describe, expect, it, vi } from 'vitest'
import { existsSync, mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { delimiter, join } from 'node:path'
import { createServer } from 'node:http'
import { runProcess } from '../../shared/child-process/run-process'
import { removeTree } from '../../shared/windows-transient-lock-removal'
import { getManagedCommand, CODEX_EVENTS } from './codex-hook-definition'
import { getManagedScript } from './codex-hook-script'
import {
createManagedCommandMatcher,
wrapWindowsCmdHookCommand
} from '../agent-hooks/installer-utils'
vi.mock('electron', () => ({ app: { getPath: () => process.cwd() } }))
afterEach(() => vi.restoreAllMocks())
describe('Codex Windows hook command', () => {
it.each(['测试用户', '홍길동', '日本語', 'rené', '测试 用户', "测试 O'Brien"])(
'uses the existing PowerShell host for %s without a second interpreter',
(profile) => {
vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
const path = `C:\\Users\\${profile}\\.orca\\agent-hooks\\codex-hook.cmd`
const command = getManagedCommand(path)
expect(command).not.toMatch(/powershell\.exe|EncodedCommand|Set-ExecutionPolicy/)
expect(command).toContain(`-LiteralPath '${path.replaceAll("'", "''")}' -PathType Leaf`)
expect(command).toContain(`[Console]::In.ReadToEnd()`)
expect(createManagedCommandMatcher('codex-hook.cmd')(command)).toBe(true)
expect(wrapWindowsCmdHookCommand(path)).toContain('-EncodedCommand')
}
)
it('preserves the existing ASCII command and POSIX launcher', () => {
vi.spyOn(process, 'platform', 'get').mockReturnValue('win32')
const path = 'C:\\Users\\alice\\.orca\\agent-hooks\\codex-hook.cmd'
expect(getManagedCommand(path)).toBe(path)
vi.spyOn(process, 'platform', 'get').mockReturnValue('linux')
expect(getManagedCommand('/home/测试/.orca/agent-hooks/codex-hook.sh')).toContain(
"[ -x '/home/测试/.orca/agent-hooks/codex-hook.sh' ]"
)
})
})
const windowsPowerShell = join(
process.env.SystemRoot ?? 'C:\\Windows',
'System32',
'WindowsPowerShell',
'v1.0',
'powershell.exe'
)
const windowsPwsh = (process.env.PATH ?? '')
.split(delimiter)
.map((directory) => join(directory, 'pwsh.exe'))
.find((file) => existsSync(file))
describe.skipIf(process.platform !== 'win32')('Codex hook delivery through PowerShell', () => {
it.each([windowsPowerShell, ...(windowsPwsh ? [windowsPwsh] : [])])(
'delivers all eight events exactly once from a Unicode profile through %s',
async (shell) => {
const root = mkdtempSync(join(tmpdir(), 'orca-codex-cjk-'))
const home = join(root, "测试 사용자 O'Brien")
mkdirSync(home)
const scriptPath = join(home, 'codex-hook.cmd')
writeFileSync(scriptPath, getManagedScript())
const posts: URLSearchParams[] = []
const tokens: unknown[] = []
const server = createServer((req, res) => {
const chunks: Buffer[] = []
req.on('data', (chunk) => chunks.push(chunk))
req.on('end', () => {
tokens.push(req.headers['x-orca-agent-hook-token'])
posts.push(new URLSearchParams(Buffer.concat(chunks).toString('utf8')))
res.writeHead(204).end()
})
})
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve))
const address = server.address()
if (!address || typeof address === 'string') {
throw new Error('Missing listener port')
}
const env = {
...Object.fromEntries(
Object.entries(process.env).filter(([key]) => !key.startsWith('ORCA_'))
),
ORCA_BACKGROUND_LAUNCH: '1',
ORCA_AGENT_HOOK_PORT: String(address.port),
ORCA_AGENT_HOOK_TOKEN: 'unicode-test-token',
ORCA_PANE_KEY: 'unicode-tab:unicode-leaf',
ORCA_WORKTREE_ID: 'C:\\folder workspace\\测试 & repo'
}
const payloads = CODEX_EVENTS.map((hook_event_name) =>
JSON.stringify({
hook_event_name,
prompt: '测试 한국어 😀 " \\ \n & %PATH% ! $HOME '.repeat(7000)
})
)
const invoke = (command: string, input: string) =>
runProcess({
program: shell,
args: ['-NoProfile', '-Command', command],
input,
env,
timeoutMs: 10_000,
terminationBarrier: true
})
try {
for (let offset = 0; offset < payloads.length; offset += 4) {
const results = await Promise.all(
payloads
.slice(offset, offset + 4)
.map((payload) => invoke(getManagedCommand(scriptPath), payload))
)
for (const result of results) {
expect(result).toMatchObject({ code: 0, stdout: '', stderr: '', timedOut: false })
}
}
expect(posts).toHaveLength(CODEX_EVENTS.length)
expect(tokens).toEqual(CODEX_EVENTS.map(() => 'unicode-test-token'))
expect(posts.map((post) => post.get('payload')).sort()).toEqual([...payloads].sort())
for (const post of posts) {
expect(post.get('paneKey')).toBe(env.ORCA_PANE_KEY)
expect(post.get('worktreeId')).toBe(env.ORCA_WORKTREE_ID)
}
await new Promise<void>((resolve) => server.close(() => resolve()))
expect(await invoke(getManagedCommand(scriptPath), payloads[0])).toMatchObject({
code: 0,
stdout: '',
stderr: '',
timedOut: false
})
rmSync(scriptPath)
expect(await invoke(getManagedCommand(scriptPath), payloads[0])).toMatchObject({
code: 0,
stdout: '',
stderr: '',
timedOut: false
})
expect(posts).toHaveLength(CODEX_EVENTS.length)
} finally {
await new Promise<void>((resolve) => server.close(() => resolve()))
await removeTree(root)
}
},
30_000
)
})
@@ -0,0 +1,97 @@
import { describe, expect, it, vi } from 'vitest'
import { mkdirSync, readFileSync, writeFileSync } from 'node:fs'
import { join } from 'node:path'
import type * as Os from 'node:os'
import { setupCodexHookHomes } from './hook-service-test-harness'
const { getPathMock, homedirMock } = vi.hoisted(() => ({
getPathMock: vi.fn<(name: string) => string>(),
homedirMock: vi.fn<() => string>()
}))
vi.mock('electron', () => ({ app: { getPath: getPathMock } }))
vi.mock('os', async (importOriginal) => ({
...(await importOriginal<typeof Os>()),
homedir: homedirMock
}))
import { CodexHookService } from './hook-service'
import { CODEX_EVENTS, CODEX_EVENT_LABEL, getManagedCommand } from './codex-hook-definition'
import { readHooksJson, wrapWindowsHookCommand } from '../agent-hooks/installer-utils'
import {
computeTrustedHash,
getCodexExplicitHomeHookSourcePath,
upsertHookTrustEntries
} from './config-toml-trust'
const homes = setupCodexHookHomes(homedirMock, getPathMock)
describe.skipIf(process.platform !== 'win32')('Unicode Windows hook upgrade', () => {
it('replaces all encoded commands and trust hashes while preserving user hooks on reinstall', async () => {
const home = join(homes.tmpHome, '测试 用户')
mkdirSync(home)
homedirMock.mockReturnValue(home)
const runtimeHome = join(homes.userDataDir, 'codex-runtime-home', 'home')
const configPath = join(runtimeHome, 'hooks.json')
const tomlPath = join(runtimeHome, 'config.toml')
const scriptPath = join(home, '.orca', 'agent-hooks', 'codex-hook.cmd')
const oldCommand = wrapWindowsHookCommand(scriptPath)
const userHome = join(home, '.codex')
mkdirSync(userHome)
const userConfig = JSON.stringify({
hooks: { Stop: [{ hooks: [{ type: 'command', command: 'user-hook' }] }] }
})
writeFileSync(join(userHome, 'hooks.json'), userConfig)
mkdirSync(runtimeHome, { recursive: true })
writeFileSync(
configPath,
JSON.stringify({
hooks: Object.fromEntries(
CODEX_EVENTS.map((event) => [
event,
[{ hooks: [{ type: 'command', command: oldCommand, timeout: 10 }] }]
])
)
})
)
upsertHookTrustEntries(
tomlPath,
CODEX_EVENTS.map((event) => ({
sourcePath: getCodexExplicitHomeHookSourcePath(configPath),
eventLabel: CODEX_EVENT_LABEL[event],
groupIndex: 0,
handlerIndex: 0,
command: oldCommand,
timeoutSec: 10
}))
)
const service = new CodexHookService()
expect(service.getStatus().state).not.toBe('installed')
for (let pass = 0; pass < 2; pass++) {
expect((await service.install()).state).toBe('installed')
expect(service.getStatus().state).toBe('installed')
const hooks = readHooksJson(configPath)?.hooks
const trust = readFileSync(tomlPath, 'utf8')
for (const event of CODEX_EVENTS) {
const commands = hooks?.[event]?.flatMap((group) => group.hooks ?? []) ?? []
expect(
commands.filter((hook) => hook.command === getManagedCommand(scriptPath))
).toHaveLength(1)
expect(commands.some((hook) => hook.command === oldCommand)).toBe(false)
const entry = {
sourcePath: getCodexExplicitHomeHookSourcePath(configPath),
eventLabel: CODEX_EVENT_LABEL[event],
groupIndex: 0,
handlerIndex: 0,
command: getManagedCommand(scriptPath),
timeoutSec: 10
}
expect(trust).toContain(computeTrustedHash(entry))
expect(trust).not.toContain(computeTrustedHash({ ...entry, command: oldCommand }))
}
expect(
hooks?.Stop?.some((group) => group.hooks?.some((hook) => hook.command === 'user-hook'))
).toBe(true)
expect(readFileSync(join(userHome, 'hooks.json'), 'utf8')).toBe(userConfig)
}
})
})