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 <event> 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 <command>` 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 <neil@stably.ai> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Brennan Benson
GitHub
parent
fe53387747
commit
d0d257189c
|
|
@ -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')
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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
|
||||
// <event> 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
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
|
|
|
|||
|
|
@ -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 <event> 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<string, { hooks?: { command?: string }[] }[]> }
|
||||
|
||||
// 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<string, unknown>; body: string }) => void
|
||||
const receivedPromise = new Promise<{ headers: Record<string, unknown>; 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<void>((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<number>((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<void>((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')
|
||||
|
|
|
|||
|
|
@ -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 <command>` 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')
|
||||
|
|
|
|||
Loading…
Reference in New Issue