From d0d257189cb080f9e0a6ddee69aef8b636fa2469 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 25 Jun 2026 20:53:58 -0700 Subject: [PATCH] Speed up Windows Codex/Claude hooks without dropping events (#6402) Windows runs managed hooks through a shell, and #6078 wrapped the launcher in `powershell -EncodedCommand` to survive spaces in profile paths. Combined with the inner PowerShell Invoke-WebRequest post, every hook spawned two PowerShell processes (~300ms startup each), so a hook took ~650ms+ and fired up to 6x per turn. Codex 0.140 renders that as lingering "Running hook" rows. The earlier RC worked around it by deleting SessionStart/UserPromptSubmit/Stop, which loses lifecycle status fidelity. Keep all six Codex events and make them fast instead: - Codex runs hooks as `cmd.exe /C ` and forwards our string verbatim when it has no spaces/quotes, so emit the bare .cmd path for cmd-safe profiles (zero shell startup) and fall back to the encoded PowerShell launcher only for spaced/metachar paths (#6078 robustness). - Replace the inner PowerShell post with curl.exe (Windows 10 1803+), posting the same form fields as the POSIX hook and reading the raw payload from stdin via `--data-urlencode payload@-` so UTF-8 (e.g. CJK) survives without code-page translation. Result: the common Codex case is 0 PowerShell (~70-150ms vs ~650ms); spaced-path profiles drop to 1. Claude runs hooks through Git Bash, so its launcher must stay PowerShell-encoded (a bare path is what breaks it), but its inner post moves to curl.exe too, cutting Claude from two PowerShell startups to one. Validated against the real Codex 0.140 binary (interactive TUI + exec): all managed hooks fire, render briefly, and clear with no lingering rows; the listener receives every post in 1-6ms. Co-authored-by: Neil Co-authored-by: Claude Opus 4.8 --- src/main/agent-hooks/installer-utils.test.ts | 27 +++++ src/main/agent-hooks/installer-utils.ts | 24 +++++ src/main/claude/hook-service.test.ts | 26 +++++ src/main/claude/hook-service.ts | 9 +- src/main/codex/hook-service.test.ts | 101 ++++++++++++++++++- src/main/codex/hook-service.ts | 20 +++- 6 files changed, 199 insertions(+), 8 deletions(-) diff --git a/src/main/agent-hooks/installer-utils.test.ts b/src/main/agent-hooks/installer-utils.test.ts index a5e157b79c2..4d92d3e689e 100644 --- a/src/main/agent-hooks/installer-utils.test.ts +++ b/src/main/agent-hooks/installer-utils.test.ts @@ -15,6 +15,7 @@ import { join } from 'path' import { spawnSync } from 'child_process' import { buildWindowsAgentHookPostCommand, + buildWindowsAgentHookCurlPostCommand, createManagedCommandMatcher, getSharedManagedScriptPath, hookDefinitionHasManagedCommand, @@ -402,3 +403,29 @@ describe('buildWindowsAgentHookPostCommand', () => { expect(command).not.toContain('Invoke-WebRequest') }) }) + +describe('buildWindowsAgentHookCurlPostCommand', () => { + it('posts form fields via curl.exe and reads the payload from stdin', () => { + const command = buildWindowsAgentHookCurlPostCommand('codex') + + // Why: the fast path must not spawn a second PowerShell — that startup cost + // is the regression this replaces. + expect(command).not.toMatch(/powershell/i) + expect(command).toContain('%SystemRoot%\\System32\\curl.exe') + expect(command).toContain('http://127.0.0.1:%ORCA_AGENT_HOOK_PORT%/hook/codex') + expect(command).toContain('-H "Content-Type: application/x-www-form-urlencoded"') + expect(command).toContain('-H "X-Orca-Agent-Hook-Token: %ORCA_AGENT_HOOK_TOKEN%"') + expect(command).toContain('--data-urlencode "paneKey=%ORCA_PANE_KEY%"') + expect(command).toContain('--data-urlencode "worktreeId=%ORCA_WORKTREE_ID%"') + // Why: `payload@-` makes curl read raw bytes from stdin and urlencode them, + // so UTF-8 prompts survive without a code-page conversion. + expect(command).toContain('--data-urlencode "payload@-"') + // Why: same dead-listener bound as the POSIX hook so a stalled server can't + // hold up the agent. + expect(command).toContain('--connect-timeout 0.5 --max-time 1.5') + }) + + it('targets the requested hook source endpoint', () => { + expect(buildWindowsAgentHookCurlPostCommand('grok')).toContain('/hook/grok') + }) +}) diff --git a/src/main/agent-hooks/installer-utils.ts b/src/main/agent-hooks/installer-utils.ts index 446a28f7391..12770d80857 100644 --- a/src/main/agent-hooks/installer-utils.ts +++ b/src/main/agent-hooks/installer-utils.ts @@ -176,6 +176,30 @@ export function buildWindowsAgentHookPostCommand(source: AgentHookSource): strin ].join('\r\n') } +// Why: status hooks fire up to 6× per turn; spawning PowerShell per post adds +// ~300ms of interpreter startup each, which Codex 0.140's synchronous "Running +// hook" rows make visible. curl.exe (Windows 10 1803+) posts the same +// form fields as the POSIX hook and reads the raw payload from stdin via +// `--data-urlencode payload@-`, so UTF-8 (e.g. CJK prompts) survives byte-for- +// byte without the code-page translation that forced the PowerShell post. +export function buildWindowsAgentHookCurlPostCommand(source: AgentHookSource): string { + return [ + '"%SystemRoot%\\System32\\curl.exe" -sS -X POST', + `"http://127.0.0.1:%ORCA_AGENT_HOOK_PORT%/hook/${source}"`, + '--connect-timeout 0.5 --max-time 1.5', + '-H "Content-Type: application/x-www-form-urlencoded"', + '-H "X-Orca-Agent-Hook-Token: %ORCA_AGENT_HOOK_TOKEN%"', + '--data-urlencode "paneKey=%ORCA_PANE_KEY%"', + '--data-urlencode "tabId=%ORCA_TAB_ID%"', + '--data-urlencode "launchToken=%ORCA_AGENT_LAUNCH_TOKEN%"', + '--data-urlencode "worktreeId=%ORCA_WORKTREE_ID%"', + '--data-urlencode "env=%ORCA_AGENT_HOOK_ENV%"', + '--data-urlencode "version=%ORCA_AGENT_HOOK_VERSION%"', + '--data-urlencode "payload@-"', + '>nul 2>&1' + ].join(' ') +} + export function removeManagedCommands( definitions: HookDefinition[], isManagedCommand: (command: string | undefined) => boolean diff --git a/src/main/claude/hook-service.test.ts b/src/main/claude/hook-service.test.ts index e1dea4d4538..e67bff55ed6 100644 --- a/src/main/claude/hook-service.test.ts +++ b/src/main/claude/hook-service.test.ts @@ -215,6 +215,32 @@ describe('ClaudeHookService.install', () => { } } ) + + // Why: the launcher must stay PowerShell-encoded for Git Bash, but the hook + // POST inside the .cmd should use curl.exe so each hook spawns one + // interpreter, not two. Posting via a second PowerShell was the slow path. + it.skipIf(process.platform !== 'win32')( + 'posts from the managed .cmd via curl.exe, not a second PowerShell', + () => { + const tmpHome = mkdtempSync(join(tmpdir(), 'orca-claude-curl-')) + vi.stubEnv('HOME', tmpHome) + vi.stubEnv('USERPROFILE', tmpHome) + try { + expect(new ClaudeHookService().install().state).toBe('installed') + const script = readFileSync( + join(tmpHome, '.orca', 'agent-hooks', CLAUDE_SCRIPT_FILE_NAME), + 'utf-8' + ) + expect(script).toContain('%SystemRoot%\\System32\\curl.exe') + expect(script).toContain('--data-urlencode "payload@-"') + expect(script).toContain('/hook/claude') + expect(script).not.toMatch(/Invoke-WebRequest/i) + } finally { + vi.unstubAllEnvs() + rmSync(tmpHome, { recursive: true, force: true }) + } + } + ) }) describe('ClaudeHookService.installRemote', () => { diff --git a/src/main/claude/hook-service.ts b/src/main/claude/hook-service.ts index 38d09fecbf5..1ef948e9c57 100644 --- a/src/main/claude/hook-service.ts +++ b/src/main/claude/hook-service.ts @@ -1,7 +1,7 @@ import type { SFTPWrapper } from 'ssh2' import type { AgentHookInstallState, AgentHookInstallStatus } from '../../shared/agent-hook-types' import { - buildWindowsAgentHookPostCommand, + buildWindowsAgentHookCurlPostCommand, readHooksJson, writeHooksJson, writeManagedScript @@ -63,7 +63,12 @@ function getManagedScript( 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', 'if "%ORCA_PANE_KEY%"=="" exit /b 0', - buildWindowsAgentHookPostCommand('claude'), + // Why: post via curl.exe, not a second PowerShell. Claude's launcher is + // already an encoded PowerShell command (Git Bash needs it to survive + // spaces); a PowerShell post on top of that meant two interpreter + // startups per hook. The post runs inside the .cmd (cmd.exe context), so + // curl works the same here as for the POSIX/Codex hooks. + buildWindowsAgentHookCurlPostCommand('claude'), 'exit /b 0', '' ].join('\r\n') diff --git a/src/main/codex/hook-service.test.ts b/src/main/codex/hook-service.test.ts index 155cd51c0c8..2483550a16b 100644 --- a/src/main/codex/hook-service.test.ts +++ b/src/main/codex/hook-service.test.ts @@ -10,9 +10,12 @@ import { symlinkSync, writeFileSync } from 'fs' -import { tmpdir } from 'os' +import { homedir, tmpdir } from 'os' import type * as Os from 'os' import { join } from 'path' +import { spawn } from 'child_process' +import { createServer } from 'http' +import type { AddressInfo } from 'net' import { createManagedCommandMatcher, wrapPosixHookCommand } from '../agent-hooks/installer-utils' import { computeTrustedHash, upsertHookTrustEntriesInContent } from './config-toml-trust' @@ -211,6 +214,102 @@ describe('CodexHookService', () => { } ) + // Why: the common case — a profile path with no spaces or cmd metacharacters + // — must launch the .cmd directly with no PowerShell, restoring the pre-#6078 + // speed that Codex 0.140's synchronous "Running hook" rows expose. + it.skipIf(process.platform !== 'win32')( + 'launches the managed .cmd directly when the profile path is cmd-safe', + () => { + const status = new CodexHookService().install() + expect(status.state).toBe('installed') + + const managedCodexHome = join(userDataDir, 'codex-runtime-home', 'home') + const hooksConfig = JSON.parse( + readFileSync(join(managedCodexHome, 'hooks.json'), 'utf-8') + ) as { hooks: Record } + + // Why: the temp home is normally cmd-safe; guard so a runner whose tmpdir + // holds an exotic character still asserts the correct (fallback) branch. + const command = hooksConfig.hooks.Stop?.[0]?.hooks?.[0]?.command ?? '' + const cmdSafe = /^[A-Za-z0-9_.:\\~-]+$/.test(join(tmpHome, '.orca', 'agent-hooks')) + if (cmdSafe) { + expect(command).not.toMatch(/powershell/i) + expect(command).toMatch(/\\agent-hooks\\codex-hook\.cmd$/) + } else { + expect(command).toMatch(/^powershell -NoProfile/) + } + } + ) + + // Why: end-to-end proof the curl-based managed script posts the hook to the + // local listener with UTF-8 (CJK) payloads and a worktreeId containing spaces + // and a `&` — the cases the replaced PowerShell post and form quoting handled. + it.skipIf(process.platform !== 'win32')( + 'posts hook payloads via the curl-based managed script preserving UTF-8 and spaced metadata', + async () => { + new CodexHookService().install() + const scriptPath = join(homedir(), '.orca', 'agent-hooks', 'codex-hook.cmd') + expect(existsSync(scriptPath)).toBe(true) + + // Why: resolve when the listener has fully read the hook POST. spawnSync + // would block the event loop and starve this handler, so the child is + // spawned asynchronously while the server drains the request concurrently. + let resolveReceived: (value: { headers: Record; body: string }) => void + const receivedPromise = new Promise<{ headers: Record; body: string }>( + (resolve) => { + resolveReceived = resolve + } + ) + const server = createServer((req, res) => { + const chunks: Buffer[] = [] + req.on('data', (c: Buffer) => chunks.push(c)) + req.on('end', () => { + res.end('ok') + resolveReceived({ headers: req.headers, body: Buffer.concat(chunks).toString('utf-8') }) + }) + }) + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + const port = (server.address() as AddressInfo).port + + try { + const payload = JSON.stringify({ prompt: '你好世界', hook_event_name: 'UserPromptSubmit' }) + // Why: this suite may run inside an Orca-launched terminal whose env + // already carries ORCA_AGENT_HOOK_ENDPOINT/PORT/TOKEN. The managed + // script sources that endpoint file, so leave it out or the hook posts + // to the live Orca instead of this test's listener. + const cleanEnv = { ...process.env } + for (const key of Object.keys(cleanEnv)) { + if (key.startsWith('ORCA_')) { + delete cleanEnv[key] + } + } + const child = spawn('cmd.exe', ['/d', '/c', scriptPath], { + env: { + ...cleanEnv, + ORCA_AGENT_HOOK_PORT: String(port), + ORCA_AGENT_HOOK_TOKEN: 'tok123', + ORCA_PANE_KEY: '42:leaf-abc', + ORCA_TAB_ID: '42', + ORCA_WORKTREE_ID: 'C:\\work trees\\my repo & co', + ORCA_AGENT_HOOK_VERSION: '1' + } + }) + child.stdin.end(payload) + const exitCode = await new Promise((resolve) => child.on('close', resolve)) + expect(exitCode).toBe(0) + + const received = await receivedPromise + const params = new URLSearchParams(received.body) + expect(received.headers['x-orca-agent-hook-token']).toBe('tok123') + expect(params.get('paneKey')).toBe('42:leaf-abc') + expect(params.get('worktreeId')).toBe('C:\\work trees\\my repo & co') + expect(JSON.parse(params.get('payload') ?? '{}').prompt).toBe('你好世界') + } finally { + await new Promise((resolve) => server.close(() => resolve())) + } + } + ) + it('keeps hooks isolated by Orca userData instead of mutating system ~/.codex', () => { const systemCodexHome = join(tmpHome, '.codex') const systemHooksPath = join(systemCodexHome, 'hooks.json') diff --git a/src/main/codex/hook-service.ts b/src/main/codex/hook-service.ts index 7f2840fa89f..235b1718c3c 100644 --- a/src/main/codex/hook-service.ts +++ b/src/main/codex/hook-service.ts @@ -6,7 +6,7 @@ import type { AgentHookInstallState, AgentHookInstallStatus } from '../../shared import { buildManagedCommandHook, createManagedCommandMatcher, - buildWindowsAgentHookPostCommand, + buildWindowsAgentHookCurlPostCommand, getSharedManagedScriptPath, hookDefinitionHasManagedCommand, MANAGED_HOOK_TIMEOUT_SECONDS, @@ -130,10 +130,20 @@ function getManagedScriptPath(): string { return getSharedManagedScriptPath(getManagedScriptFileName()) } +// Why: a Windows script path is cmd-safe when it holds only characters cmd.exe +// passes through untouched (drive letter, backslash, dot, dash, underscore). +// Codex runs hooks as `cmd.exe /C ` and forwards our string verbatim +// when it has no spaces/quotes, so a bare path then launches with zero shell +// startup. Spaces or cmd metacharacters (`% ^ & ! ( )` etc.) force the encoded +// PowerShell launcher (#6078), which hides the path in base64. `~` is allowed +// because 8.3 short paths (e.g. `RUNNER~1`) are cmd-safe and common. +const WINDOWS_CMD_SAFE_PATH = /^[A-Za-z0-9_.:\\~-]+$/ + function getManagedCommand(scriptPath: string): string { - return process.platform === 'win32' - ? wrapWindowsHookCommand(scriptPath) - : wrapPosixHookCommand(scriptPath) + if (process.platform !== 'win32') { + return wrapPosixHookCommand(scriptPath) + } + return WINDOWS_CMD_SAFE_PATH.test(scriptPath) ? scriptPath : wrapWindowsHookCommand(scriptPath) } function getSystemConfigPath(): string { @@ -697,7 +707,7 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', 'if "%ORCA_PANE_KEY%"=="" exit /b 0', - buildWindowsAgentHookPostCommand('codex'), + buildWindowsAgentHookCurlPostCommand('codex'), 'exit /b 0', '' ].join('\r\n')