From c3ab805d12ff15372efe7d77fbb232cd8b2440ab Mon Sep 17 00:00:00 2001 From: Kaynan Sampaio de Camargo <33468632+kaynansc@users.noreply.github.com> Date: Mon, 13 Jul 2026 00:26:22 -0700 Subject: [PATCH] fix(agent-hooks): drain stdin before hook script early exits so agents never hit EPIPE (#8430) * Fix hook scripts to drain stdin before any early-exit path Generated agent hook scripts and missing-script launchers could exit successfully before consuming the payload written to their stdin, leaving the writer with a broken pipe (EPIPE/ERROR_BROKEN_PIPE) once the reader closed early. Capture stdin (or drain it via a shared epilogue/fast-path guard) before any whole-script success exit across all POSIX, batch, PowerShell, and Git Bash launcher variants, and add a cross-agent lifecycle test suite plus a live Electron verification script to guard the contract going forward. * Harden hook scripts against unreadable managed scripts and add a Claude/ - Extend the POSIX launcher guard to also require `[ -r ]`, not just `-f`/`-x`, so an executable-but-unreadable managed script still drains stdin instead of erroring or silently misbehaving. - Add a verifier case (`verifyClaudeDevinSkip`) that spins up a local HTTP server and confirms the Claude hook never forwards a request that Devin already imported, catching accidental double-forwarding. - Update installer-utils tests and stdin-lifecycle docs to match the new readable-file guard and the added verification case. * Fix hook-launcher verification to derive script paths from the installed Extract the quoted path from the launcher's `if [ -f '...'` clause instead of reconstructing it via join(home, ...), so missing/failing-script test cases can't silently fall through to the real script if the install layout changes. --------- Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com> --- .../verify-agent-hook-stdin-lifecycle.mjs | 394 ++++++++++++++++++ config/tsconfig.cli.json | 1 + docs/reference/agent-hook-stdin-lifecycle.md | 198 +++++++++ .../agent-hook-memory-sftp.test-fixture.ts | 87 ++++ src/main/agent-hooks/hook-stdin-contract.ts | 37 ++ src/main/agent-hooks/installer-utils.test.ts | 115 ++++- src/main/agent-hooks/installer-utils.ts | 61 ++- .../managed-hook-stdin-lifecycle.test.ts | 366 ++++++++++++++++ .../agent-hooks/managed-hook-timeout.test.ts | 88 +--- .../remote-hook-service-installers.test.ts | 18 +- src/main/antigravity/hook-service.test.ts | 4 +- src/main/antigravity/hook-service.ts | 26 +- src/main/claude/hook-service.test.ts | 8 +- src/main/claude/hook-service.ts | 18 +- .../codex/hook-service-wsl-runtime.test.ts | 36 +- src/main/codex/hook-service.ts | 20 +- .../command-code-managed-script.ts | 15 +- src/main/command-code/hook-service.test.ts | 2 +- src/main/copilot/hook-service.test.ts | 14 +- src/main/copilot/hook-service.ts | 20 +- src/main/cursor/hook-service.ts | 15 +- src/main/devin/hook-service.test.ts | 9 +- src/main/devin/hook-service.ts | 15 +- src/main/devin/hook-settings.ts | 7 +- src/main/droid/hook-service.ts | 15 +- src/main/gemini/hook-service.ts | 15 +- src/main/grok/hook-service.ts | 15 +- src/main/kimi/hook-service.ts | 6 +- 28 files changed, 1380 insertions(+), 245 deletions(-) create mode 100644 config/scripts/verify-agent-hook-stdin-lifecycle.mjs create mode 100644 docs/reference/agent-hook-stdin-lifecycle.md create mode 100644 src/main/agent-hooks/agent-hook-memory-sftp.test-fixture.ts create mode 100644 src/main/agent-hooks/hook-stdin-contract.ts create mode 100644 src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts diff --git a/config/scripts/verify-agent-hook-stdin-lifecycle.mjs b/config/scripts/verify-agent-hook-stdin-lifecycle.mjs new file mode 100644 index 000000000..06ee97347 --- /dev/null +++ b/config/scripts/verify-agent-hook-stdin-lifecycle.mjs @@ -0,0 +1,394 @@ +#!/usr/bin/env node + +import { spawn } from 'node:child_process' +import { + accessSync, + chmodSync, + constants as fsConstants, + mkdtempSync, + readFileSync, + rmSync, + statSync, + symlinkSync, + writeFileSync +} from 'node:fs' +import { createServer } from 'node:http' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +const MANAGED_SCRIPTS = [ + ['antigravity-hook.sh', 'antigravity'], + ['claude-hook.sh', 'claude'], + ['codex-hook.sh', 'codex'], + ['command-code-hook.sh', 'command-code'], + ['copilot-hook.sh', 'copilot'], + ['cursor-hook.sh', 'cursor'], + ['devin-hook.sh', 'devin'], + ['droid-hook.sh', 'droid'], + ['gemini-hook.sh', 'gemini'], + ['grok-hook.sh', 'grok'], + ['kimi-hook.sh', 'kimi'], + ['openclaude-hook.sh', 'claude'] +] + +const REQUIRED_JSON_STDOUT = new Set(['antigravity-hook.sh', 'copilot-hook.sh', 'gemini-hook.sh']) + +function parseArgs(argv) { + const result = { home: process.env.HOME ?? '', minMtime: 0 } + for (let index = 0; index < argv.length; index += 1) { + const value = argv[index] + if (value === '--home') { + result.home = argv[index + 1] ?? '' + index += 1 + } else if (value === '--min-mtime') { + result.minMtime = Number(argv[index + 1] ?? 0) + index += 1 + } else { + throw new Error(['Unknown argument: ', value].join('')) + } + } + if (!result.home) { + throw new Error('Pass --home or set HOME to the isolated Electron home directory') + } + return result +} + +function withoutOrcaEnvironment(extra = {}) { + return { + ...Object.fromEntries(Object.entries(process.env).filter(([key]) => !key.startsWith('ORCA_'))), + ...extra + } +} + +function runShell(command, payload, env) { + return new Promise((resolve, reject) => { + const child = spawn('/bin/sh', ['-c', command], { + env, + stdio: ['pipe', 'pipe', 'pipe'] + }) + const stdout = [] + const stderr = [] + const stdinErrors = [] + const timeout = setTimeout(() => { + child.kill('SIGKILL') + reject(new Error(['Timed out running: ', command.slice(0, 120)].join(''))) + }, 10_000) + child.stdout.on('data', (chunk) => stdout.push(chunk)) + child.stderr.on('data', (chunk) => stderr.push(chunk)) + child.stdin.on('error', (error) => stdinErrors.push(error)) + child.on('error', (error) => { + clearTimeout(timeout) + reject(error) + }) + child.on('close', (exitCode) => { + clearTimeout(timeout) + resolve({ + exitCode, + stdinErrors, + stdout: Buffer.concat(stdout).toString('utf8'), + stderr: Buffer.concat(stderr).toString('utf8') + }) + }) + child.stdin.end(payload) + }) +} + +function assertSuccessfulWrite(result, label) { + if (result.exitCode !== 0) { + throw new Error( + [label, ' exited ', String(result.exitCode), ': ', result.stderr.slice(0, 500)].join('') + ) + } + if (result.stdinErrors.length > 0) { + throw new Error( + [ + label, + ' produced stdin errors: ', + result.stdinErrors.map((error) => error.message).join(', ') + ].join('') + ) + } +} + +function assertProtocolStdout(fileName, stdout) { + if (!REQUIRED_JSON_STDOUT.has(fileName)) { + return + } + const firstLine = stdout.trim().split(/\r?\n/, 1)[0] + try { + JSON.parse(firstLine) + } catch { + throw new Error([fileName, ' did not emit protocol JSON: ', stdout.slice(0, 200)].join('')) + } +} + +function readGeneratedScripts(home, minMtime) { + const hooksDir = join(home, '.orca', 'agent-hooks') + return MANAGED_SCRIPTS.map(([fileName, source]) => { + const path = join(hooksDir, fileName) + const stats = statSync(path) + if (!stats.isFile()) { + throw new Error([fileName, ' is not a regular file'].join('')) + } + try { + accessSync(path, fsConstants.R_OK | fsConstants.X_OK) + } catch { + throw new Error([fileName, ' is not readable and executable'].join('')) + } + if (minMtime > 0 && stats.mtimeMs < minMtime) { + throw new Error([fileName, ' predates the Electron launch'].join('')) + } + const body = readFileSync(path, 'utf8') + const captureIndex = body.indexOf('payload=$(cat)') + const firstExitIndex = body.indexOf('exit 0') + if (captureIndex < 0 || firstExitIndex <= captureIndex) { + throw new Error([fileName, ' can exit before capturing stdin'].join('')) + } + return { body, fileName, path, source } + }) +} + +function findStrings(value, matches = []) { + if (typeof value === 'string') { + matches.push(value) + return matches + } + if (Array.isArray(value)) { + for (const child of value) { + findStrings(child, matches) + } + return matches + } + if (value && typeof value === 'object') { + for (const child of Object.values(value)) { + findStrings(child, matches) + } + } + return matches +} + +function nextRequest(server) { + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + server.removeListener('request', onRequest) + reject(new Error('Generated hook did not reach the loopback server')) + }, 8_000) + const onRequest = (request, response) => { + const chunks = [] + request.on('data', (chunk) => chunks.push(chunk)) + request.on('end', () => { + clearTimeout(timeout) + response.writeHead(200, { 'Content-Type': 'application/json' }) + response.end('{}') + resolve({ + body: Buffer.concat(chunks).toString('utf8'), + headers: request.headers, + url: request.url + }) + }) + } + server.once('request', onRequest) + }) +} + +async function verifyNoOpWrites(scripts, home, payload) { + const commandCodeBin = mkdtempSync(join(tmpdir(), 'orca-hook-command-code-bin-')) + symlinkSync('/bin/cat', join(commandCodeBin, 'cat')) + try { + for (const script of scripts) { + const path = + script.fileName === 'command-code-hook.sh' + ? commandCodeBin + : (process.env.PATH ?? '/usr/bin:/bin') + const result = await runShell( + ['/bin/sh ', JSON.stringify(script.path)].join(''), + payload, + withoutOrcaEnvironment({ + HOME: home, + PATH: path, + ORCA_AGENT_HOOK_ENDPOINT: '' + }) + ) + assertSuccessfulWrite(result, [script.fileName, ' no-op'].join('')) + assertProtocolStdout(script.fileName, result.stdout) + } + } finally { + rmSync(commandCodeBin, { recursive: true, force: true }) + } +} + +async function verifyClaudeDevinSkip(scripts, home, payload) { + const claude = scripts.find((script) => script.fileName === 'claude-hook.sh') + let unexpectedRequests = 0 + const server = createServer((_request, response) => { + unexpectedRequests += 1 + response.writeHead(200, { 'Content-Type': 'application/json' }) + response.end('{}') + }) + await new Promise((resolve, reject) => { + server.once('error', reject) + server.listen(0, '127.0.0.1', resolve) + }) + try { + const address = server.address() + if (!address || typeof address === 'string') { + throw new Error('Claude skip verifier did not receive a TCP port') + } + const result = await runShell( + ['/bin/sh ', JSON.stringify(claude.path)].join(''), + payload, + withoutOrcaEnvironment({ + DEVIN_PROJECT_DIR: join(home, 'devin-project'), + HOME: home, + ORCA_AGENT_HOOK_ENDPOINT: '', + ORCA_AGENT_HOOK_PORT: String(address.port), + ORCA_AGENT_HOOK_TOKEN: 'electron-verification-token', + ORCA_PANE_KEY: 'electron-verification-pane' + }) + ) + assertSuccessfulWrite(result, 'Claude Devin-import skip') + if (unexpectedRequests !== 0) { + throw new Error('Claude forwarded a hook imported by Devin') + } + } finally { + await new Promise((resolve) => server.close(resolve)) + } +} + +async function verifyForwarding(scripts, home, payload) { + const server = createServer() + await new Promise((resolve, reject) => { + server.once('error', reject) + server.listen(0, '127.0.0.1', resolve) + }) + try { + const address = server.address() + if (!address || typeof address === 'string') { + throw new Error('Loopback verifier did not receive a TCP port') + } + for (const script of scripts) { + const requestPromise = nextRequest(server) + const result = await runShell( + ['/bin/sh ', JSON.stringify(script.path)].join(''), + payload, + withoutOrcaEnvironment({ + HOME: home, + ORCA_AGENT_HOOK_ENDPOINT: '', + ORCA_AGENT_HOOK_PORT: String(address.port), + ORCA_AGENT_HOOK_TOKEN: 'electron-verification-token', + ORCA_PANE_KEY: 'electron-verification-pane', + ORCA_TAB_ID: 'electron-verification-tab', + ORCA_WORKTREE_ID: 'electron-verification-worktree', + ORCA_AGENT_HOOK_ENV: 'test', + ORCA_AGENT_HOOK_VERSION: '1', + ORCA_ANTIGRAVITY_EVENT: 'PostInvocation', + ORCA_COPILOT_HOOK_EVENT: 'PostToolUse' + }) + ) + assertSuccessfulWrite(result, [script.fileName, ' forwarding'].join('')) + assertProtocolStdout(script.fileName, result.stdout) + const request = await requestPromise + const form = new URLSearchParams(request.body) + if (request.url !== ['/hook/', script.source].join('')) { + throw new Error( + [script.fileName, ' posted to ', String(request.url), ' instead of ', script.source].join( + '' + ) + ) + } + if (request.headers['x-orca-agent-hook-token'] !== 'electron-verification-token') { + throw new Error([script.fileName, ' lost the hook token header'].join('')) + } + if (form.get('payload') !== payload) { + throw new Error([script.fileName, ' changed the forwarded payload'].join('')) + } + if (form.get('paneKey') !== 'electron-verification-pane') { + throw new Error([script.fileName, ' changed the forwarded pane key'].join('')) + } + } + } finally { + await new Promise((resolve) => server.close(resolve)) + } +} + +// Why: rewrite from the path embedded in the installed command, not a +// reconstructed join(home, ...). That way missing/failing-script cases cannot +// silently re-run the real script if the install layout changes. +function rewriteLauncherScriptPath(command, nextPath) { + const match = /if \[ -f '([^']+)'/.exec(command) + if (!match) { + throw new Error('Installed launcher command did not reference a quoted script path') + } + return command.replaceAll(match[1], nextPath) +} + +async function verifyInstalledLauncher(home, payload) { + const settingsPath = join(home, '.claude', 'settings.json') + const settings = JSON.parse(readFileSync(settingsPath, 'utf8')) + const command = findStrings(settings).find( + (value) => value.includes('claude-hook.sh') && value.includes('if [ -f ') + ) + if (!command || !command.includes('] && [ -r ') || !command.includes('else cat >/dev/null')) { + throw new Error('Electron did not install the guarded Claude launcher') + } + const scratch = mkdtempSync(join(tmpdir(), 'orca-hook-launcher-')) + try { + const missingPath = join(scratch, 'missing-hook.sh') + const missingResult = await runShell( + rewriteLauncherScriptPath(command, missingPath), + payload, + withoutOrcaEnvironment({ HOME: home }) + ) + assertSuccessfulWrite(missingResult, 'installed missing-script launcher') + + const failingPath = join(scratch, 'failing-hook.sh') + writeFileSync(failingPath, '#!/bin/sh\ncat >/dev/null\nexit 7\n', 'utf8') + chmodSync(failingPath, 0o755) + const failingResult = await runShell( + rewriteLauncherScriptPath(command, failingPath), + payload, + withoutOrcaEnvironment({ HOME: home }) + ) + if (failingResult.exitCode !== 7 || failingResult.stdinErrors.length > 0) { + throw new Error('Installed launcher did not preserve a running script failure') + } + } finally { + rmSync(scratch, { recursive: true, force: true }) + } +} + +async function main() { + const args = parseArgs(process.argv.slice(2)) + const payload = JSON.stringify({ + hook_event_name: 'PostToolUse', + tool_name: 'shell', + tool_output: 'x'.repeat(1_200_000) + }) + const scripts = readGeneratedScripts(args.home, args.minMtime) + await verifyNoOpWrites(scripts, args.home, payload) + await verifyClaudeDevinSkip(scripts, args.home, payload) + await verifyForwarding(scripts, args.home, payload) + await verifyInstalledLauncher(args.home, payload) + process.stdout.write( + [ + JSON.stringify( + { + forwardingPayloadBytes: Buffer.byteLength(payload), + claudeDevinSkipCases: 1, + launcherCases: 2, + noOpScripts: scripts.length, + forwardedScripts: scripts.length, + status: 'passed' + }, + null, + 2 + ), + '\n' + ].join('') + ) +} + +main().catch((error) => { + console.error(error instanceof Error ? error.stack : error) + process.exitCode = 1 +}) diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index 5c1ec10b7..a8e5dcad2 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -3,6 +3,7 @@ "include": [ "../src/cli/**/*", "../src/shared/**/*", + "../src/main/agent-hooks/hook-stdin-contract.ts", "../src/main/agent-hooks/installer-utils.ts", "../src/main/agent-hooks/installer-utils-remote.ts", "../src/main/agent-hooks/managed-agent-hook-controls.ts", diff --git a/docs/reference/agent-hook-stdin-lifecycle.md b/docs/reference/agent-hook-stdin-lifecycle.md new file mode 100644 index 000000000..9f875b3d0 --- /dev/null +++ b/docs/reference/agent-hook-stdin-lifecycle.md @@ -0,0 +1,198 @@ +# Agent Hook Stdin Lifecycle + +## Problem + +Orca installs agent hooks in global agent configuration, so a managed hook can +run in an Orca pane, an SSH/WSL runtime, a detached session, or an ordinary +terminal with no Orca environment. Hook runners write an event payload to the +child process's stdin after spawn. + +The generated scripts currently inspect Orca environment variables before they +read stdin. When a guard exits first, the hook runner can still be writing to a +pipe whose reader has closed. POSIX reports `EPIPE`; Windows reports +`ERROR_BROKEN_PIPE`. The same failure exists when a managed launcher finds that +its script was removed or is no longer executable. + +This is an ownership bug rather than an agent-specific parsing bug. Every +process that accepts a hook payload owns the read end until it reaches EOF, +including processes that decide the event is irrelevant. + +## Goals + +- Make successful hook no-ops consume stdin to EOF on macOS, Linux, Windows, + WSL, and SSH hosts. +- Encode stdin ownership once per platform instead of copying ad hoc drains + into every early-exit branch. +- Preserve payload bytes, hook output, transport timeouts, exit-code behavior, + and provider-specific event metadata on the forwarding path. +- Cover every generated script and missing-script launcher with behavioral + regression tests. + +## Non-goals + +- Do not redesign hook configuration schemas, endpoint discovery, relay HTTP + payloads, or event selection. +- Do not make hook payloads unbounded; existing agent and config timeouts remain + the outer lifecycle bound. +- Do not suppress failures from a script that exists and actually runs. Only a + deliberate no-op path exits successfully after consuming stdin. +- Do not change plugin-based integrations that do not receive piped hook stdin. + +## Contract + +1. A generated hook script consumes its input exactly once. +2. No whole-script success exit may occur before that consumption completes. +3. A launcher that declines to start a missing, unreadable, or non-executable + managed script becomes the stdin owner and drains to EOF before returning + success. +4. A launcher propagates the exit code of a managed script that was started. +5. Output required by an agent protocol may be emitted before stdin is read, + but the process must still retain the read end until EOF. +6. Drain commands are platform-qualified where the platform searches the + working directory implicitly. + +## Design + +### POSIX generated scripts + +Capture stdin near the start of the generated script, after any protocol output +that must be immediate and before endpoint refresh, environment guards, or +provider-specific skips: + +```sh +payload=$(cat) +if [ -z "$payload" ]; then + exit 0 +fi +``` + +Antigravity is the one semantic exception: events without payload still post an +empty object, so its shared capture policy maps empty input to `{}` instead of +exiting. Claude's Devin-import skip happens after capture. Command Code captures +before ancestor/endpoint recovery so its comparatively expensive discovery +cannot leave the writer blocked. + +The common payload-capture fragments live in +`src/main/agent-hooks/hook-stdin-contract.ts`. Templates choose the required or +empty-object policy instead of spelling the lifecycle independently. + +### Windows batch generated scripts + +Batch scripts stream stdin directly into system `curl.exe`; buffering arbitrary +JSON in an environment variable would corrupt metacharacters and hit size +limits. Their posting path therefore remains streaming. + +All environment guard failures jump to one epilogue: + +```bat +if "%ORCA_AGENT_HOOK_PORT%"=="" goto :orca_agent_hook_drain_stdin +... +exit /b 0 +:orca_agent_hook_drain_stdin +"%SystemRoot%\System32\more.com" >nul 2>nul +exit /b 0 +``` + +`more.com` is qualified because Windows searches the current working directory +for executables. Shared guard and epilogue builders keep labels and commands +identical across templates. Command Code's existing endpoint-discovery labels +remain subroutines and the drain epilogue is placed after them. + +### PowerShell generated scripts + +PowerShell captures with `[Console]::In.ReadToEnd()` before endpoint and +environment guards. Copilot then parses the captured value only on the posting +path. This mirrors POSIX ownership without starting an additional process. + +### Managed launchers + +- POSIX `/bin/sh` launchers require a regular, readable, executable file. The + rejected path drains with `cat`; the started-script branch keeps propagating + status. +- Encoded PowerShell launchers use `Test-Path -PathType Leaf`. A missing script + calls `[Console]::In.ReadToEnd()` and exits zero; an existing script preserves + `$LASTEXITCODE`. +- Codex's cmd.exe fast path remains PowerShell-free. It rejects missing paths + and directories, using the same qualified `more.com` drain for both. +- Claude's Git Bash fast path uses a POSIX file guard and drain while continuing + to execute the `.cmd` directly rather than interpreting it as shell source. +- Agent-specific direct launchers, including Antigravity event wrappers and + Copilot's PowerShell-file command, adopt the same missing-file behavior. + +## Data Flow + +```text +hook runner spawns command + -> runner writes payload and closes stdin + -> launcher starts managed script + -> script consumes payload + -> refreshes endpoint / evaluates guards + -> posts or exits zero + -> OR launcher cannot start script + -> launcher drains payload + -> exits zero +``` + +Local, WSL, and SSH installs serialize the same POSIX template. Windows local +installs use the batch or PowerShell template. No host assumes another host's +path syntax or shell. + +## Failure Semantics + +- Missing Orca environment: consume input, exit zero, emit only protocol-required + output. +- Empty payload: consume EOF, then follow the agent's existing empty-event rule. +- Missing/unreadable/non-executable script: launcher consumes input and exits + zero. +- Endpoint parse/read failure: preserve the existing fail-open behavior after + stdin ownership has been satisfied. +- Existing script returns nonzero: propagate its status; do not drain again or + disguise the script failure. +- Hook runner never closes stdin: the existing config-level timeout terminates + the hook. Reading to EOF does not introduce an unbounded lifecycle beyond that + already required by normal payload parsing. + +## Verification + +Add one cross-agent lifecycle suite rather than per-service string-position +assertions. + +- Generate every SSH-compatible POSIX managed script, spawn it with Orca + environment removed, write a payload larger than pipe buffers, and assert: + exit zero, no stdin error, and required protocol output remains valid. +- Exercise the Claude Devin skip independently because it is a second no-op + condition before endpoint forwarding. +- Exercise POSIX, encoded PowerShell, cmd.exe, and Git Bash missing-script + launchers with a large payload and verify zero write errors. +- On Windows CI, install and execute every local batch/PowerShell managed script + with missing Orca environment and a large payload. +- Keep structural assertions for shared batch guard/epilogue generation, but do + not use substring placement as the primary regression gate. +- Preserve existing successful-post, timeout, quoting, SSH install, WSL, and + nonzero-exit propagation tests. + +For a macOS smoke test, launch the real Electron app with an isolated `HOME` +and `ORCA_DEV_USER_DATA_PATH`, record the launch time, then run: + +```sh +node config/scripts/verify-agent-hook-stdin-lifecycle.mjs \ + --home "$ISOLATED_HOME" \ + --min-mtime "$LAUNCH_START_MS" +``` + +The verifier rejects stale files, then exercises the 12 POSIX scripts written +by that Electron launch with a payload larger than pipe buffers. It covers +successful no-ops, loopback forwarding without payload changes, required JSON +stdout, Claude's Devin skip, and both branches of the installed missing-script +launcher. + +## Rollout And Compatibility + +The generated scripts are rewritten by existing install/update flows, so no +config migration is needed. Commands use POSIX `sh` primitives, Windows inbox +executables, and PowerShell features available on supported Windows releases. +No Git behavior or provider-specific review behavior changes. + +The change should ship atomically across templates and launchers. A partial +rollout would leave global hooks with platform- or agent-dependent pipe safety, +which is the inconsistency this design removes. diff --git a/src/main/agent-hooks/agent-hook-memory-sftp.test-fixture.ts b/src/main/agent-hooks/agent-hook-memory-sftp.test-fixture.ts new file mode 100644 index 000000000..1dfaabf6c --- /dev/null +++ b/src/main/agent-hooks/agent-hook-memory-sftp.test-fixture.ts @@ -0,0 +1,87 @@ +import type { SFTPWrapper } from 'ssh2' + +export type AgentHookMemoryFileSystem = { + files: Map + dirs: Set + modes: Map +} + +export function createAgentHookMemorySftp(initialFiles: Record = {}): { + sftp: SFTPWrapper + fs: AgentHookMemoryFileSystem +} { + const fs: AgentHookMemoryFileSystem = { + files: new Map(Object.entries(initialFiles)), + dirs: new Set(['/']), + modes: new Map() + } + const missing = (path: string): { code: number; message: string } => ({ + code: 2, + message: `ENOENT ${path}` + }) + const sftp = { + readFile: (path: string, _encoding: string, done: (error: unknown, data?: string) => void) => { + const data = fs.files.get(path) + if (data === undefined) { + done(missing(path)) + return + } + done(null, data) + }, + writeFile: ( + path: string, + content: string, + options: string | { mode?: number }, + done: (error: unknown) => void + ) => { + fs.files.set(path, content) + if (typeof options !== 'string' && options.mode !== undefined) { + fs.modes.set(path, options.mode) + } + done(null) + }, + rename: (source: string, target: string, done: (error: unknown) => void) => { + const content = fs.files.get(source) + if (content === undefined) { + done(missing(source)) + return + } + fs.files.set(target, content) + fs.files.delete(source) + const mode = fs.modes.get(source) + if (mode !== undefined) { + fs.modes.set(target, mode) + fs.modes.delete(source) + } + done(null) + }, + unlink: (path: string, done: (error: unknown) => void) => { + fs.files.delete(path) + fs.modes.delete(path) + done(null) + }, + chmod: (path: string, mode: number, done: (error: unknown) => void) => { + fs.modes.set(path, mode) + done(null) + }, + stat: (path: string, done: (error: unknown, stats?: { mode: number }) => void) => { + if (!fs.files.has(path)) { + done(missing(path)) + return + } + done(null, { mode: fs.modes.get(path) ?? 0o100644 }) + }, + readdir: (path: string, done: (error: unknown, entries?: { filename: string }[]) => void) => { + if (!fs.dirs.has(path)) { + done(missing(path)) + return + } + done(null, []) + }, + mkdir: (path: string, done: (error: unknown) => void) => { + fs.dirs.add(path) + done(null) + } + } as unknown as SFTPWrapper + return { sftp, fs } +} diff --git a/src/main/agent-hooks/hook-stdin-contract.ts b/src/main/agent-hooks/hook-stdin-contract.ts new file mode 100644 index 000000000..a54a8e55f --- /dev/null +++ b/src/main/agent-hooks/hook-stdin-contract.ts @@ -0,0 +1,37 @@ +export type PosixHookEmptyPayloadPolicy = 'exit' | 'empty-object' + +export const POSIX_HOOK_STDIN_DRAIN_COMMAND = 'cat >/dev/null 2>&1 || :' + +// Why: every POSIX hook must own stdin before any no-op exit; sharing this +// prelude prevents agent templates from inventing different drain semantics. +export function buildPosixHookPayloadCapture( + emptyPayloadPolicy: PosixHookEmptyPayloadPolicy = 'exit' +): string[] { + const emptyPayloadLines = + emptyPayloadPolicy === 'empty-object' ? [" payload='{}'"] : [' exit 0'] + return ['payload=$(cat)', 'if [ -z "$payload" ]; then', ...emptyPayloadLines, 'fi'] +} + +export const WINDOWS_HOOK_STDIN_DRAIN_LABEL = 'orca_agent_hook_drain_stdin' +export const WINDOWS_HOOK_STDIN_DRAIN_COMMAND = '"%SystemRoot%\\System32\\more.com" >nul 2>nul' + +// Why: batch payloads stream directly to curl and cannot be buffered safely in +// environment variables, so guard failures share one EOF-draining epilogue. +export function buildWindowsHookEnvironmentGuardLines(): string[] { + const drainTarget = `goto :${WINDOWS_HOOK_STDIN_DRAIN_LABEL}` + return [ + `if "%ORCA_AGENT_HOOK_PORT%"=="" ${drainTarget}`, + `if "%ORCA_AGENT_HOOK_TOKEN%"=="" ${drainTarget}`, + `if "%ORCA_PANE_KEY%"=="" ${drainTarget}` + ] +} + +export function buildWindowsHookStdinDrainEpilogue(): string[] { + return [ + `:${WINDOWS_HOOK_STDIN_DRAIN_LABEL}`, + // Why: qualify the inbox reader because Windows searches the worktree for + // executables before PATH and hook payloads must not reach repo-local code. + WINDOWS_HOOK_STDIN_DRAIN_COMMAND, + 'exit /b 0' + ] +} diff --git a/src/main/agent-hooks/installer-utils.test.ts b/src/main/agent-hooks/installer-utils.test.ts index ba0dad4a0..66d4134f0 100644 --- a/src/main/agent-hooks/installer-utils.test.ts +++ b/src/main/agent-hooks/installer-utils.test.ts @@ -160,10 +160,8 @@ describe('createManagedCommandMatcher', () => { }) it('matches the guarded launcher form so wrapped commands sweep correctly', () => { - // Why: wrapPosixHookCommand wraps the launcher in `if [ -x ... ]; then ...; fi` - // so a stale entry no-ops instead of returning exit 127. The sweep on - // install() must still recognize the guarded form as managed, otherwise - // repeated installs would accumulate one guarded + one unguarded entry. + // Why: older guarded launchers used a single `-x` check. The sweep must + // still recognize them or reinstalling would retain a stale duplicate. expect( match( 'if [ -x "/Users/alice/Library/Application Support/Orca/agent-hooks/claude-hook.sh" ]; then /bin/sh "/Users/alice/Library/Application Support/Orca/agent-hooks/claude-hook.sh"; fi' @@ -176,6 +174,17 @@ describe('createManagedCommandMatcher', () => { expect(match(command)).toBe(true) }) + it('matches PowerShell and POSIX variants across Copilot platform switches', () => { + const matchPosix = createManagedCommandMatcher('copilot-hook.sh') + const matchPowerShell = createManagedCommandMatcher('copilot-hook.ps1') + + expect(matchPosix("& 'C:\\Users\\alice\\.orca\\agent-hooks\\copilot-hook.ps1'")).toBe(true) + expect( + matchPosix(wrapWindowsHookCommand('C:\\Users\\alice\\.orca\\agent-hooks\\copilot-hook.ps1')) + ).toBe(true) + expect(matchPowerShell("/bin/sh '/home/alice/.orca/agent-hooks/copilot-hook.sh'")).toBe(true) + }) + it('matches the legacy per-userData script path AND the new shared ~/.orca path', () => { // Why: install() must sweep old per-userData commands when migrating to // the shared ~/.orca script path, or stale launchers keep failing. @@ -285,13 +294,15 @@ describe('writeManagedScript', () => { describe('wrapPosixHookCommand', () => { it('produces a guarded command that no-ops when the script is missing', () => { const cmd = wrapPosixHookCommand('/does/not/exist.sh') - expect(cmd).toBe("if [ -x '/does/not/exist.sh' ]; then /bin/sh '/does/not/exist.sh'; fi") + expect(cmd).toBe( + "if [ -f '/does/not/exist.sh' ] && [ -r '/does/not/exist.sh' ] && [ -x '/does/not/exist.sh' ]; then /bin/sh '/does/not/exist.sh'; else cat >/dev/null 2>&1 || :; fi" + ) }) it('preserves spaces in the script path (Library/Application Support case)', () => { // Why: Electron's userData on macOS lives under "Application Support" with - // a space. The guard must keep the path quoted so `[ -x ]` and `/bin/sh` - // each see one argument. + // a space. The guard must keep the path quoted so each file test and + // `/bin/sh` see one argument. const cmd = wrapPosixHookCommand('/Users/a/Library/Application Support/Orca/agent-hooks/x.sh') expect(cmd).toContain("'/Users/a/Library/Application Support/Orca/agent-hooks/x.sh'") }) @@ -302,7 +313,7 @@ describe('wrapPosixHookCommand', () => { // /bin/sh as a single argument. const cmd = wrapPosixHookCommand("/path/with'quote/x.sh") expect(cmd).toBe( - "if [ -x '/path/with'\\''quote/x.sh' ]; then /bin/sh '/path/with'\\''quote/x.sh'; fi" + "if [ -f '/path/with'\\''quote/x.sh' ] && [ -r '/path/with'\\''quote/x.sh' ] && [ -x '/path/with'\\''quote/x.sh' ]; then /bin/sh '/path/with'\\''quote/x.sh'; else cat >/dev/null 2>&1 || :; fi" ) }) @@ -311,7 +322,7 @@ describe('wrapPosixHookCommand', () => { ORCA_COPILOT_HOOK_EVENT: 'UserPromptSubmit' }) expect(cmd).toBe( - "if [ -x '/does/not/exist.sh' ]; then ORCA_COPILOT_HOOK_EVENT='UserPromptSubmit' /bin/sh '/does/not/exist.sh'; fi" + "if [ -f '/does/not/exist.sh' ] && [ -r '/does/not/exist.sh' ] && [ -x '/does/not/exist.sh' ]; then ORCA_COPILOT_HOOK_EVENT='UserPromptSubmit' /bin/sh '/does/not/exist.sh'; else cat >/dev/null 2>&1 || :; fi" ) }) @@ -324,6 +335,35 @@ describe('wrapPosixHookCommand', () => { } ) + it.skipIf(process.platform === 'win32')( + 'drains stdin when a directory occupies the managed script path', + () => { + const scriptPath = join(tmpDir, 'directory-hook.sh') + mkdirSync(scriptPath) + const result = spawnSync('/bin/sh', ['-c', wrapPosixHookCommand(scriptPath)], { + input: Buffer.alloc(1_000_000, 'x') + }) + + expect(result.error).toBeUndefined() + expect(result.status).toBe(0) + } + ) + + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'drains stdin when the managed script is executable but unreadable', + () => { + const scriptPath = join(tmpDir, 'unreadable-hook.sh') + writeFileSync(scriptPath, '#!/bin/sh\nexit 0\n', 'utf-8') + chmodSync(scriptPath, 0o111) + const result = spawnSync('/bin/sh', ['-c', wrapPosixHookCommand(scriptPath)], { + input: Buffer.alloc(1_000_000, 'x') + }) + + expect(result.error).toBeUndefined() + expect(result.status).toBe(0) + } + ) + // Why: commit 4d618795 explicitly switched from `&& ... || true` (which // swallowed non-zero exits) to `if ... then ... fi` (which preserves the // script's exit code). This test guards against a future regression that @@ -350,13 +390,27 @@ function decodeWindowsHookCommand(command: string): string { return Buffer.from(encodedCommand!, 'base64').toString('utf16le') } +function expectedDecodedWindowsHookCommand(scriptPath: string): string { + const quoted = `'${scriptPath.replaceAll("'", "''")}'` + return `if (Test-Path -LiteralPath ${quoted} -PathType Leaf) { & ${quoted}; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null; exit 0` +} + describe('wrapWindowsHookCommand', () => { it('invokes the .cmd through an encoded PowerShell command', () => { const command = wrapWindowsHookCommand('C:\\Users\\alice\\.orca\\agent-hooks\\codex-hook.cmd') expect(command).toMatch(qualifiedWindowsPowerShellCommand) expect(command).not.toMatch(/^powershell\b/i) expect(decodeWindowsHookCommand(command)).toBe( - "& 'C:\\Users\\alice\\.orca\\agent-hooks\\codex-hook.cmd'; exit $LASTEXITCODE" + expectedDecodedWindowsHookCommand('C:\\Users\\alice\\.orca\\agent-hooks\\codex-hook.cmd') + ) + }) + + it('scopes environment variables inside the encoded launcher', () => { + const command = wrapWindowsHookCommand('C:\\hooks\\copilot-hook.ps1', { + ORCA_COPILOT_HOOK_EVENT: 'UserPromptSubmit' + }) + expect(decodeWindowsHookCommand(command)).toContain( + "$env:ORCA_COPILOT_HOOK_EVENT = 'UserPromptSubmit'; if (Test-Path" ) }) @@ -367,7 +421,9 @@ describe('wrapWindowsHookCommand', () => { const cmd = wrapWindowsHookCommand('C:\\Users\\Jorge Silva\\.orca\\agent-hooks\\codex-hook.cmd') expect(cmd).toMatch(qualifiedWindowsPowerShellCommand) expect(decodeWindowsHookCommand(cmd)).toBe( - "& 'C:\\Users\\Jorge Silva\\.orca\\agent-hooks\\codex-hook.cmd'; exit $LASTEXITCODE" + expectedDecodedWindowsHookCommand( + 'C:\\Users\\Jorge Silva\\.orca\\agent-hooks\\codex-hook.cmd' + ) ) }) @@ -376,7 +432,7 @@ describe('wrapWindowsHookCommand', () => { expect(cmd).not.toContain('%ORCA_TEST%') expect(cmd).not.toContain('^') expect(decodeWindowsHookCommand(cmd)).toBe( - "& 'C:\\Users\\%ORCA_TEST%\\a^b\\codex-hook.cmd'; exit $LASTEXITCODE" + expectedDecodedWindowsHookCommand('C:\\Users\\%ORCA_TEST%\\a^b\\codex-hook.cmd') ) }) @@ -398,31 +454,52 @@ describe('wrapWindowsHookCommand', () => { }) describe('wrapWindowsCmdHookCommand', () => { - it('returns a bare .cmd path when cmd.exe can invoke it safely', () => { + it('keeps the safe-path launcher PowerShell-free and drains non-file paths', () => { const scriptPath = 'C:\\Users\\alice\\.orca\\agent-hooks\\codex-hook.cmd' - expect(wrapWindowsCmdHookCommand(scriptPath)).toBe(scriptPath) + const command = wrapWindowsCmdHookCommand(scriptPath) + expect(command).toContain(`if exist "${scriptPath}\\."`) + expect(command).toContain(`if exist "${scriptPath}" (call "${scriptPath}")`) + expect(command).toContain('"%SystemRoot%\\System32\\more.com" >nul 2>nul') + expect(command).not.toMatch(/powershell/i) }) + it.skipIf(process.platform !== 'win32')( + 'drains stdin when a directory occupies the managed script path', + () => { + const scriptPath = 'directory-hook.cmd' + mkdirSync(join(tmpDir, scriptPath)) + const result = spawnSync('cmd.exe', ['/d', '/c', wrapWindowsCmdHookCommand(scriptPath)], { + cwd: tmpDir, + input: Buffer.alloc(1_000_000, 'x') + }) + + expect(result.error).toBeUndefined() + expect(result.status).toBe(0) + } + ) + it('falls back to the encoded launcher when cmd.exe would split or expand the path', () => { const scriptPath = 'C:\\Users\\Jane Doe\\%ORCA_TEST%\\codex-hook.cmd' const command = wrapWindowsCmdHookCommand(scriptPath) expect(command).toMatch(qualifiedWindowsPowerShellCommand) - expect(decodeWindowsHookCommand(command)).toBe(`& '${scriptPath}'; exit $LASTEXITCODE`) + expect(decodeWindowsHookCommand(command)).toBe(expectedDecodedWindowsHookCommand(scriptPath)) }) }) describe('wrapWindowsGitBashHookCommand', () => { - it('returns a forward-slash .cmd path when Git Bash can execute it safely', () => { + it('guards the forward-slash fast path and drains when missing', () => { expect( wrapWindowsGitBashHookCommand('C:\\Users\\alice\\.orca\\agent-hooks\\claude-hook.cmd') - ).toBe('C:/Users/alice/.orca/agent-hooks/claude-hook.cmd') + ).toBe( + "if [ -f 'C:/Users/alice/.orca/agent-hooks/claude-hook.cmd' ]; then 'C:/Users/alice/.orca/agent-hooks/claude-hook.cmd'; else cat >/dev/null 2>&1 || :; fi" + ) }) it('falls back to the encoded launcher when bash would split the path', () => { const scriptPath = 'C:\\Users\\Jane Doe\\.orca\\agent-hooks\\claude-hook.cmd' const command = wrapWindowsGitBashHookCommand(scriptPath) expect(command).toMatch(qualifiedWindowsPowerShellCommand) - expect(decodeWindowsHookCommand(command)).toBe(`& '${scriptPath}'; exit $LASTEXITCODE`) + expect(decodeWindowsHookCommand(command)).toBe(expectedDecodedWindowsHookCommand(scriptPath)) }) it('falls back to the encoded launcher when bash metacharacters are present', () => { @@ -430,7 +507,7 @@ describe('wrapWindowsGitBashHookCommand', () => { const command = wrapWindowsGitBashHookCommand(scriptPath) expect(command).toMatch(qualifiedWindowsPowerShellCommand) expect(command).not.toContain('& bob') - expect(decodeWindowsHookCommand(command)).toBe(`& '${scriptPath}'; exit $LASTEXITCODE`) + expect(decodeWindowsHookCommand(command)).toBe(expectedDecodedWindowsHookCommand(scriptPath)) }) }) diff --git a/src/main/agent-hooks/installer-utils.ts b/src/main/agent-hooks/installer-utils.ts index 86bb58607..6b9e93eda 100644 --- a/src/main/agent-hooks/installer-utils.ts +++ b/src/main/agent-hooks/installer-utils.ts @@ -13,6 +13,10 @@ import { dirname, join } from 'node:path' import { randomUUID } from 'node:crypto' import type { AgentHookSource } from '../../shared/agent-hook-relay' import { grantDirAcl, isPermissionError } from '../win32-utils' +import { + POSIX_HOOK_STDIN_DRAIN_COMMAND, + WINDOWS_HOOK_STDIN_DRAIN_COMMAND +} from './hook-stdin-contract' export type HookCommandConfig = { type: 'command' @@ -86,12 +90,13 @@ export function readHooksJson(configPath: string): HooksConfig | null { export function createManagedCommandMatcher( scriptFileName: string ): (command: string | undefined) => boolean { - const scriptStem = scriptFileName.replace(/\.(?:cmd|sh)$/, '') - // Why: local Windows installs use .cmd, while SSH/POSIX installs and older - // entries use .sh. A platform switch should still sweep stale Orca hooks. + const scriptStem = scriptFileName.replace(/\.(?:cmd|ps1|sh)$/, '') + // Why: local Windows installs use .cmd or Copilot's .ps1, while SSH/POSIX + // installs use .sh. A platform switch must still sweep stale Orca hooks. const needles = [ `agent-hooks/${scriptFileName}`, `agent-hooks/${scriptStem}.cmd`, + `agent-hooks/${scriptStem}.ps1`, `agent-hooks/${scriptStem}.sh` ] return (command) => { @@ -123,24 +128,26 @@ export function getSharedManagedScriptPath(scriptFileName: string): string { return join(homedir(), '.orca', 'agent-hooks', scriptFileName) } +function quotePosixShellString(value: string): string { + return `'${value.replaceAll("'", "'\\''")}'` +} + // Why: a stale managed hook entry (left over after the user wiped userData, // switched dev↔prod installs, or had a partial install fail) used to fire // `/bin/sh ""` on every tool call, which exits 127 and surfaces // as `PreToolUse hook (failed) error: hook exited with code 127` in the agent -// transcript. Wrapping the launcher in `if [ -x ... ]; then ...; fi` makes a -// missing/non-executable script a silent no-op so a broken install never -// poisons the user's session. Failures inside the script itself are -// unaffected — only the missing-script case short-circuits. +// transcript. Guarding for a regular readable executable file makes a broken +// install a silent no-op without hiding failures from a script that starts. export function wrapPosixHookCommand(scriptPath: string, env: Record = {}): string { // Why: POSIX single-quote escape so $, `, ", and \ in scriptPath are taken // literally — avoids a shell-injection footgun if a future caller passes an // arbitrary path. - const quoted = `'${scriptPath.replaceAll("'", "'\\''")}'` + const quoted = quotePosixShellString(scriptPath) const envPrefix = Object.entries(env) .map(([key, value]) => `${key}='${value.replaceAll("'", "'\\''")}'`) .join(' ') const invocation = envPrefix ? `${envPrefix} /bin/sh ${quoted}` : `/bin/sh ${quoted}` - return `if [ -x ${quoted} ]; then ${invocation}; fi` + return `if [ -f ${quoted} ] && [ -r ${quoted} ] && [ -x ${quoted} ]; then ${invocation}; else ${POSIX_HOOK_STDIN_DRAIN_COMMAND}; fi` } function quotePowerShellString(value: string): string { @@ -154,11 +161,17 @@ function getWindowsPowerShellExecutablePath(): string { return `${systemRoot.replaceAll('\\', '/')}/System32/WindowsPowerShell/v1.0/powershell.exe` } -export function wrapWindowsHookCommand(scriptPath: string): string { - // Why: most Windows agents run hooks through Git Bash or another shell that - // mangles a raw backslash path. Codex has its own cmd.exe-safe fast path; the - // shared wrapper keeps the encoded launcher for every other agent. - const command = `& ${quotePowerShellString(scriptPath)}; exit $LASTEXITCODE` +export function wrapWindowsHookCommand( + scriptPath: string, + env: Record = {} +): string { + // Why: the encoded launcher protects paths across Windows hook shells and + // owns stdin when a stale config points at a missing managed script. + const quoted = quotePowerShellString(scriptPath) + const envPrefix = Object.entries(env) + .map(([key, value]) => `$env:${key} = ${quotePowerShellString(value)}; `) + .join('') + const command = `${envPrefix}if (Test-Path -LiteralPath ${quoted} -PathType Leaf) { & ${quoted}; exit $LASTEXITCODE }; [Console]::In.ReadToEnd() | Out-Null; exit 0` const encodedCommand = Buffer.from(command, 'utf16le').toString('base64') return `${getWindowsPowerShellExecutablePath()} -NoProfile -ExecutionPolicy Bypass -EncodedCommand ${encodedCommand}` } @@ -166,19 +179,23 @@ export function wrapWindowsHookCommand(scriptPath: string): string { export const WINDOWS_CMD_SAFE_PATH = /^[A-Za-z0-9_.:\\~-]+$/ export function wrapWindowsCmdHookCommand(scriptPath: string): string { - // Why: skip the ~360ms PowerShell interpreter startup when the path is safe - // for cmd.exe to invoke directly. The fallback keeps the encoded PowerShell - // launcher for paths with spaces or metacharacters. - return WINDOWS_CMD_SAFE_PATH.test(scriptPath) ? scriptPath : wrapWindowsHookCommand(scriptPath) + // Why: keep the safe-path fast path PowerShell-free while making a stale + // config entry own stdin; the `path\.` sentinel also rejects a directory + // that happens to occupy the managed .cmd path, including an empty one. + return WINDOWS_CMD_SAFE_PATH.test(scriptPath) + ? `if exist "${scriptPath}\\." (${WINDOWS_HOOK_STDIN_DRAIN_COMMAND}) else if exist "${scriptPath}" (call "${scriptPath}") else (${WINDOWS_HOOK_STDIN_DRAIN_COMMAND})` + : wrapWindowsHookCommand(scriptPath) } export const WINDOWS_GIT_BASH_SAFE_PATH = /^[A-Za-z0-9_.:/~-]+$/ export function wrapWindowsGitBashHookCommand(scriptPath: string): string { const bashPath = scriptPath.replaceAll('\\', '/') - // Why: Claude Code's Windows hook runner is Git Bash/MSYS. It can execute a - // bare .cmd via forward slashes, but shell metacharacters must stay encoded. - return WINDOWS_GIT_BASH_SAFE_PATH.test(bashPath) ? bashPath : wrapWindowsHookCommand(scriptPath) + // Why: Claude's Git Bash runner can execute a forward-slash .cmd directly; + // unsafe paths stay encoded and the fast path gains a missing-file drain. + return WINDOWS_GIT_BASH_SAFE_PATH.test(bashPath) + ? `if [ -f ${quotePosixShellString(bashPath)} ]; then ${quotePosixShellString(bashPath)}; else ${POSIX_HOOK_STDIN_DRAIN_COMMAND}; fi` + : wrapWindowsHookCommand(scriptPath) } export function buildWindowsAgentHookPostCommand(source: AgentHookSource): string { @@ -300,7 +317,7 @@ export function writeManagedScript(scriptPath: string, content: string): void { try { writeScriptWithAclRetry(tmpPath, content) // Why: chmod before rename so the canonical path is never visible in a - // non-executable state; wrapPosixHookCommand's `[ -x ]` guard would + // unreadable/non-executable state; wrapPosixHookCommand's guards would // silently skip the hook in that window. if (process.platform !== 'win32') { chmodSync(tmpPath, 0o755) diff --git a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts new file mode 100644 index 000000000..6a627227f --- /dev/null +++ b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts @@ -0,0 +1,366 @@ +// Why: stdin ownership is a cross-agent process contract; one executable +// matrix catches an unread early exit without duplicating template assertions. +import { describe, expect, it, vi } from 'vitest' +import { spawn } from 'node:child_process' +import { mkdtempSync, readFileSync, readdirSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import type { SFTPWrapper } from 'ssh2' +import type * as osModule from 'node:os' + +const { homedirMock } = vi.hoisted(() => ({ + homedirMock: vi.fn<() => string>() +})) + +vi.mock('electron', () => ({ + app: { + getPath: () => '/tmp/orca-user-data' + } +})) + +vi.mock('os', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + homedir: homedirMock.mockImplementation(actual.homedir) + } +}) + +import { AntigravityHookService } from '../antigravity/hook-service' +import { ClaudeHookService } from '../claude/hook-service' +import { CodexHookService } from '../codex/hook-service' +import { CommandCodeHookService } from '../command-code/hook-service' +import { CopilotHookService } from '../copilot/hook-service' +import { CursorHookService } from '../cursor/hook-service' +import { DevinHookService } from '../devin/hook-service' +import { DroidHookService } from '../droid/hook-service' +import { GeminiHookService } from '../gemini/hook-service' +import { GrokHookService } from '../grok/hook-service' +import { KimiHookService } from '../kimi/hook-service' +import { openClaudeHookService } from '../openclaude/hook-service' +import { + wrapPosixHookCommand, + wrapWindowsCmdHookCommand, + wrapWindowsGitBashHookCommand, + wrapWindowsHookCommand +} from './installer-utils' +import { createAgentHookMemorySftp } from './agent-hook-memory-sftp.test-fixture' + +const REMOTE_HOME = '/home/dev' +const LARGE_PAYLOAD = Buffer.alloc(1_000_000, 'x') +const REMOTE_INSTALLERS = [ + { + agent: 'antigravity', + install: (sftp: SFTPWrapper) => new AntigravityHookService().installRemote(sftp, REMOTE_HOME) + }, + { + agent: 'claude', + install: (sftp: SFTPWrapper) => new ClaudeHookService().installRemote(sftp, REMOTE_HOME) + }, + { + agent: 'openclaude', + install: (sftp: SFTPWrapper) => openClaudeHookService.installRemote(sftp, REMOTE_HOME) + }, + { + agent: 'codex', + install: (sftp: SFTPWrapper) => new CodexHookService().installRemote(sftp, REMOTE_HOME) + }, + { + agent: 'command-code', + install: (sftp: SFTPWrapper) => new CommandCodeHookService().installRemote(sftp, REMOTE_HOME) + }, + { + agent: 'copilot', + install: (sftp: SFTPWrapper) => new CopilotHookService().installRemote(sftp, REMOTE_HOME) + }, + { + agent: 'cursor', + install: (sftp: SFTPWrapper) => new CursorHookService().installRemote(sftp, REMOTE_HOME) + }, + { + agent: 'devin', + install: (sftp: SFTPWrapper) => new DevinHookService().installRemote(sftp, REMOTE_HOME) + }, + { + agent: 'droid', + install: (sftp: SFTPWrapper) => new DroidHookService().installRemote(sftp, REMOTE_HOME) + }, + { + agent: 'gemini', + install: (sftp: SFTPWrapper) => new GeminiHookService().installRemote(sftp, REMOTE_HOME) + }, + { + agent: 'grok', + install: (sftp: SFTPWrapper) => new GrokHookService().installRemote(sftp, REMOTE_HOME) + }, + { + agent: 'kimi', + install: (sftp: SFTPWrapper) => new KimiHookService().installRemote(sftp, REMOTE_HOME) + } +] as const + +const LOCAL_INSTALLERS = [ + { agent: 'antigravity', install: () => new AntigravityHookService().install() }, + { agent: 'claude', install: () => new ClaudeHookService().install() }, + { agent: 'openclaude', install: () => openClaudeHookService.install() }, + { agent: 'codex', install: () => new CodexHookService().install() }, + { agent: 'command-code', install: () => new CommandCodeHookService().install() }, + { agent: 'copilot', install: () => new CopilotHookService().install() }, + { agent: 'cursor', install: () => new CursorHookService().install() }, + { agent: 'devin', install: () => new DevinHookService().install() }, + { agent: 'droid', install: () => new DroidHookService().install() }, + { agent: 'gemini', install: () => new GeminiHookService().install() }, + { agent: 'grok', install: () => new GrokHookService().install() }, + { agent: 'kimi', install: () => new KimiHookService().install() } +] as const + +type HookRun = { + exitCode: number | null + stdinErrors: NodeJS.ErrnoException[] +} + +function runHookProcess( + executable: string, + args: string[], + env: NodeJS.ProcessEnv +): Promise { + return new Promise((resolve, reject) => { + const child = spawn(executable, args, { env, stdio: ['pipe', 'ignore', 'ignore'] }) + const stdinErrors: NodeJS.ErrnoException[] = [] + const timeout = setTimeout(() => { + child.kill('SIGKILL') + reject(new Error('hook did not finish after stdin closed')) + }, 10_000) + child.on('error', (error) => { + clearTimeout(timeout) + reject(error) + }) + child.stdin.on('error', (error: NodeJS.ErrnoException) => stdinErrors.push(error)) + child.on('close', (exitCode) => { + clearTimeout(timeout) + resolve({ exitCode, stdinErrors }) + }) + child.stdin.end(LARGE_PAYLOAD) + }) +} + +function hookEnvironment(extraEnv: NodeJS.ProcessEnv = {}): NodeJS.ProcessEnv { + const env = Object.fromEntries( + Object.entries(process.env).filter(([key]) => !key.startsWith('ORCA_')) + ) + return { + ...env, + HOME: REMOTE_HOME, + ORCA_AGENT_HOOK_ENDPOINT: '', + ...extraEnv + } +} + +function runPosixHook(command: string, extraEnv: NodeJS.ProcessEnv = {}): Promise { + return runHookProcess('/bin/sh', ['-c', command], hookEnvironment(extraEnv)) +} + +async function generatePosixScripts(): Promise> { + const scripts = new Map() + for (const entry of REMOTE_INSTALLERS) { + const memory = createAgentHookMemorySftp() + const status = await entry.install(memory.sftp) + expect(status.state, `${entry.agent} install status`).toBe('installed') + const generated = [...memory.fs.files.entries()].filter( + ([path]) => path.includes('/.orca/agent-hooks/') && path.endsWith('.sh') + ) + expect(generated, `${entry.agent} generated scripts`).toHaveLength(1) + scripts.set(entry.agent, generated[0][1]) + } + return scripts +} + +function withPlatform(platform: NodeJS.Platform, run: () => T): T { + const original = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: platform }) + try { + return run() + } finally { + if (original) { + Object.defineProperty(process, 'platform', original) + } + } +} + +describe('Windows managed hook stdin structure', () => { + it('routes every batch guard to a shared drain epilogue', () => { + const home = mkdtempSync(join(tmpdir(), 'orca-hook-stdin-windows-')) + homedirMock.mockReturnValue(home) + const previousGrokHome = process.env.GROK_HOME + const previousKimiHome = process.env.KIMI_CODE_HOME + delete process.env.GROK_HOME + delete process.env.KIMI_CODE_HOME + try { + withPlatform('win32', () => { + for (const entry of LOCAL_INSTALLERS) { + expect(entry.install().state, `${entry.agent} install status`).toBe('installed') + } + }) + const hooksDir = join(home, '.orca', 'agent-hooks') + const fileNames = readdirSync(hooksDir) + const mainBatchScripts = fileNames.filter( + (name) => name.endsWith('-hook.cmd') && !name.startsWith('antigravity-') + ) + mainBatchScripts.push('antigravity-hook.cmd') + expect(mainBatchScripts).toHaveLength(10) + for (const fileName of mainBatchScripts) { + const script = readFileSync(join(hooksDir, fileName), 'utf8') + expect(script, `${fileName} port guard`).toContain( + 'if "%ORCA_AGENT_HOOK_PORT%"=="" goto :orca_agent_hook_drain_stdin' + ) + expect(script, `${fileName} token guard`).toContain( + 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" goto :orca_agent_hook_drain_stdin' + ) + expect(script, `${fileName} pane guard`).toContain( + 'if "%ORCA_PANE_KEY%"=="" goto :orca_agent_hook_drain_stdin' + ) + expect(script, `${fileName} drain epilogue`).toContain( + [ + ':orca_agent_hook_drain_stdin', + '"%SystemRoot%\\System32\\more.com" >nul 2>nul', + 'exit /b 0' + ].join('\r\n') + ) + } + + const copilot = readFileSync(join(hooksDir, 'copilot-hook.ps1'), 'utf8') + expect(copilot.indexOf('[Console]::In.ReadToEnd()')).toBeLessThan( + copilot.indexOf('if (-not $env:ORCA_AGENT_HOOK_PORT') + ) + const kimi = readFileSync(join(hooksDir, 'kimi-hook.sh'), 'utf8') + expect(kimi.indexOf('payload=$(cat)')).toBeLessThan(kimi.indexOf('exit 0')) + } finally { + homedirMock.mockImplementation(() => process.env.HOME ?? tmpdir()) + if (previousGrokHome === undefined) { + delete process.env.GROK_HOME + } else { + process.env.GROK_HOME = previousGrokHome + } + if (previousKimiHome === undefined) { + delete process.env.KIMI_CODE_HOME + } else { + process.env.KIMI_CODE_HOME = previousKimiHome + } + rmSync(home, { recursive: true, force: true }) + } + }) + + it.skipIf(process.platform !== 'win32')( + 'executes every local script and missing-script launcher without a broken writer', + async () => { + const home = mkdtempSync(join(tmpdir(), 'orca-hook-stdin-windows-live-')) + homedirMock.mockReturnValue(home) + try { + for (const entry of LOCAL_INSTALLERS) { + expect(entry.install().state, `${entry.agent} install status`).toBe('installed') + } + const hooksDir = join(home, '.orca', 'agent-hooks') + const mainScripts = readdirSync(hooksDir).filter( + (name) => + name === 'antigravity-hook.cmd' || + name.endsWith('-hook.ps1') || + name.endsWith('-hook.sh') || + (name.endsWith('-hook.cmd') && !name.startsWith('antigravity-')) + ) + expect(mainScripts).toHaveLength(12) + for (const fileName of mainScripts) { + const scriptPath = join(hooksDir, fileName) + const executable = fileName.endsWith('.cmd') + ? 'cmd.exe' + : fileName.endsWith('.ps1') + ? join( + process.env.SystemRoot ?? 'C:\\Windows', + 'System32', + 'WindowsPowerShell', + 'v1.0', + 'powershell.exe' + ) + : process.env.KIMI_SHELL_PATH || 'bash.exe' + const args = fileName.endsWith('.cmd') + ? ['/d', '/c', scriptPath] + : fileName.endsWith('.ps1') + ? ['-NoProfile', '-ExecutionPolicy', 'Bypass', '-File', scriptPath] + : [scriptPath] + const result = await runHookProcess(executable, args, hookEnvironment()) + expect(result.exitCode, `${fileName} exit code`).toBe(0) + expect(result.stdinErrors, `${fileName} stdin errors`).toHaveLength(0) + } + + const missingScript = 'C:\\missing\\orca-hook.cmd' + const launcherCases = [ + { + name: 'encoded PowerShell', + executable: 'cmd.exe', + args: ['/d', '/c', wrapWindowsHookCommand(missingScript)] + }, + { + name: 'cmd fast path', + executable: 'cmd.exe', + args: ['/d', '/c', wrapWindowsCmdHookCommand(missingScript)] + }, + { + name: 'Git Bash fast path', + executable: process.env.KIMI_SHELL_PATH || 'bash.exe', + args: ['-lc', wrapWindowsGitBashHookCommand(missingScript)] + } + ] + for (const launcher of launcherCases) { + const result = await runHookProcess(launcher.executable, launcher.args, hookEnvironment()) + expect(result.exitCode, `${launcher.name} exit code`).toBe(0) + expect(result.stdinErrors, `${launcher.name} stdin errors`).toHaveLength(0) + } + } finally { + homedirMock.mockImplementation(() => process.env.HOME ?? tmpdir()) + rmSync(home, { recursive: true, force: true }) + } + } + ) +}) + +describe.skipIf(process.platform === 'win32')('managed hook stdin lifecycle', () => { + it('captures stdin before every possible whole-script success exit', async () => { + const scripts = await generatePosixScripts() + for (const [agent, script] of scripts) { + const captureIndex = script.indexOf('payload=$(cat)') + const firstExitIndex = script.indexOf('exit 0') + expect(captureIndex, `${agent} payload capture`).toBeGreaterThanOrEqual(0) + expect(firstExitIndex, `${agent} first success exit`).toBeGreaterThan(captureIndex) + } + }) + + it('accepts a large payload without Orca environment or a broken writer', async () => { + const scripts = await generatePosixScripts() + for (const [agent, script] of scripts) { + const extraEnv = + agent === 'command-code' + ? { + ORCA_AGENT_HOOK_PORT: '1', + ORCA_AGENT_HOOK_TOKEN: 'test-token', + ORCA_PANE_KEY: 'test-pane' + } + : {} + const result = await runPosixHook(script, extraEnv) + expect(result.exitCode, `${agent} exit code`).toBe(0) + expect(result.stdinErrors, `${agent} stdin errors`).toHaveLength(0) + } + }) + + it('drains before Claude skips hooks imported by Devin', async () => { + const script = (await generatePosixScripts()).get('claude') + expect(script).toBeDefined() + const result = await runPosixHook(script!, { DEVIN_PROJECT_DIR: '/tmp/devin-project' }) + expect(result.exitCode).toBe(0) + expect(result.stdinErrors).toHaveLength(0) + }) + + it('drains a large payload when the configured script is missing', async () => { + const result = await runPosixHook(wrapPosixHookCommand('/missing/orca-hook.sh')) + expect(result.exitCode).toBe(0) + expect(result.stdinErrors).toHaveLength(0) + }) +}) diff --git a/src/main/agent-hooks/managed-hook-timeout.test.ts b/src/main/agent-hooks/managed-hook-timeout.test.ts index dab3302a8..a215210a5 100644 --- a/src/main/agent-hooks/managed-hook-timeout.test.ts +++ b/src/main/agent-hooks/managed-hook-timeout.test.ts @@ -41,93 +41,7 @@ import { DevinHookService } from '../devin/hook-service' import { DroidHookService } from '../droid/hook-service' import { KimiHookService } from '../kimi/hook-service' import { openClaudeHookService } from '../openclaude/hook-service' - -type FakeFs = { - files: Map - dirs: Set - modes: Map -} - -function createFakeSftp(initialFiles: Record = {}): { - sftp: SFTPWrapper - fs: FakeFs -} { - const fs: FakeFs = { - files: new Map(Object.entries(initialFiles)), - dirs: new Set(['/']), - modes: new Map() - } - const noEntryError = (path: string): { code: number; message: string } => ({ - code: 2, - message: `ENOENT ${path}` - }) - - const sftp = { - readFile: (path: string, _enc: string, cb: (err: unknown, data?: string) => void): void => { - const v = fs.files.get(path) - if (v === undefined) { - cb(noEntryError(path)) - return - } - cb(null, v) - }, - writeFile: ( - path: string, - content: string, - options: string | { mode?: number }, - cb: (err: unknown) => void - ): void => { - fs.files.set(path, content) - if (typeof options !== 'string' && options.mode !== undefined) { - fs.modes.set(path, options.mode) - } - cb(null) - }, - rename: (src: string, dst: string, cb: (err: unknown) => void): void => { - const v = fs.files.get(src) - if (v === undefined) { - cb(noEntryError(src)) - return - } - fs.files.set(dst, v) - fs.files.delete(src) - const mode = fs.modes.get(src) - if (mode !== undefined) { - fs.modes.set(dst, mode) - fs.modes.delete(src) - } - cb(null) - }, - unlink: (path: string, cb: (err: unknown) => void): void => { - fs.files.delete(path) - fs.modes.delete(path) - cb(null) - }, - chmod: (path: string, mode: number, cb: (err: unknown) => void): void => { - fs.modes.set(path, mode) - cb(null) - }, - stat: (path: string, cb: (err: unknown, stats?: { mode: number }) => void): void => { - if (!fs.files.has(path)) { - cb(noEntryError(path)) - return - } - cb(null, { mode: fs.modes.get(path) ?? 0o100644 }) - }, - readdir: (path: string, cb: (err: unknown, list?: { filename: string }[]) => void): void => { - if (fs.dirs.has(path)) { - cb(null, []) - return - } - cb(noEntryError(path)) - }, - mkdir: (path: string, cb: (err: unknown) => void): void => { - fs.dirs.add(path) - cb(null) - } - } as unknown as SFTPWrapper - return { sftp, fs } -} +import { createAgentHookMemorySftp as createFakeSftp } from './agent-hook-memory-sftp.test-fixture' const REMOTE_HOME = '/home/dev' diff --git a/src/main/agent-hooks/remote-hook-service-installers.test.ts b/src/main/agent-hooks/remote-hook-service-installers.test.ts index 90726a2d7..8489048d0 100644 --- a/src/main/agent-hooks/remote-hook-service-installers.test.ts +++ b/src/main/agent-hooks/remote-hook-service-installers.test.ts @@ -244,7 +244,7 @@ describe('remote hook service installers', () => { ]) { const command = hooks.hooks[eventName]?.[0]?.hooks?.[0]?.command expect(command).toContain('/home/dev/.orca/agent-hooks/codex-hook.sh') - expect(command).toMatch(/^if \[ -x /) + expect(command).toMatch(/^if \[ -f /) } expect(fs.files.get('/home/dev/.orca/agent-hooks/codex-hook.sh')).toContain('#!/bin/sh') expect(fs.modes.get('/home/dev/.orca/agent-hooks/codex-hook.sh')).toBe(0o755) @@ -338,7 +338,7 @@ describe('remote hook service installers', () => { for (const eventName of ['BeforeAgent', 'AfterAgent', 'AfterTool', 'BeforeTool']) { const command = geminiConfig.hooks[eventName]?.[0]?.hooks?.[0]?.command expect(command).toContain('/home/dev/.orca/agent-hooks/gemini-hook.sh') - expect(command).toMatch(/^if \[ -x /) + expect(command).toMatch(/^if \[ -f /) } expect(geminiConfig.hooks.PreToolUse).toBeUndefined() @@ -398,7 +398,7 @@ describe('remote hook service installers', () => { const definition = commandCodeConfig.hooks[eventName]?.[0] const command = definition?.hooks?.[0]?.command expect(command).toContain('/home/dev/.orca/agent-hooks/command-code-hook.sh') - expect(command).toMatch(/^if \[ -x /) + expect(command).toMatch(/^if \[ -f /) } expect(commandCodeConfig.hooks.PreToolUse?.[0]?.matcher).toBe('.*') expect(commandCodeConfig.hooks.PostToolUse?.[0]?.matcher).toBe('.*') @@ -421,7 +421,7 @@ describe('remote hook service installers', () => { const definition = grokConfig.hooks[eventName]?.[0] const command = definition?.hooks?.[0]?.command expect(command).toContain('/home/dev/.orca/agent-hooks/grok-hook.sh') - expect(command).toMatch(/^if \[ -x /) + expect(command).toMatch(/^if \[ -f /) } // Why: Grok tool matchers are real regexes; bare `*` is invalid match-all. expect(grokConfig.hooks.PreToolUse?.[0]?.matcher).toBe('.*') @@ -443,14 +443,14 @@ describe('remote hook service installers', () => { const definition = devinConfig.hooks[eventName]?.[0] const command = definition?.hooks?.[0]?.command expect(command).toContain('/home/dev/.orca/agent-hooks/devin-hook.sh') - expect(command).toMatch(/^if \[ -x /) + expect(command).toMatch(/^if \[ -f /) } for (const eventName of ['PreToolUse', 'PostToolUse', 'PermissionRequest']) { const definition = devinConfig.hooks[eventName]?.[0] const command = definition?.hooks?.[0]?.command expect(definition?.matcher).toBeUndefined() expect(command).toContain('/home/dev/.orca/agent-hooks/devin-hook.sh') - expect(command).toMatch(/^if \[ -x /) + expect(command).toMatch(/^if \[ -f /) } expect(devin.fs.files.get('/home/dev/.orca/agent-hooks/devin-hook.sh')).toContain('/hook/devin') }) @@ -506,9 +506,9 @@ describe('remote hook service installers', () => { ]) { expect(config).toContain(`event = "${eventName}"`) } - // The command points at the POSIX managed script via the `[ -x ]` guard. + // The command points at the POSIX managed script via the regular-file guard. expect(config).toContain('/home/dev/.orca/agent-hooks/kimi-hook.sh') - expect(config).toMatch(/command = "if \[ -x /) + expect(config).toMatch(/command = "if \[ -f /) expect(fs.files.get('/home/dev/.orca/agent-hooks/kimi-hook.sh')).toContain('/hook/kimi') }) @@ -749,7 +749,7 @@ describe('remote hook service installers', () => { const definition = config.hooks[eventName]?.[0] const command = definition?.hooks?.[0]?.command expect(command).toContain('/home/dev/.orca/agent-hooks/droid-hook.sh') - expect(command).toMatch(/^if \[ -x /) + expect(command).toMatch(/^if \[ -f /) } // Tool/permission events carry a `*` matcher; lifecycle events do not. expect(config.hooks.PreToolUse?.[0]?.matcher).toBe('*') diff --git a/src/main/antigravity/hook-service.test.ts b/src/main/antigravity/hook-service.test.ts index 67750cedf..fffe15052 100644 --- a/src/main/antigravity/hook-service.test.ts +++ b/src/main/antigravity/hook-service.test.ts @@ -16,6 +16,7 @@ vi.mock('os', async () => { }) import { AntigravityHookService } from './hook-service' +import { createManagedCommandMatcher } from '../agent-hooks/installer-utils' const ANTIGRAVITY_SCRIPT_FILE_NAME = process.platform === 'win32' ? 'antigravity-hook.cmd' : 'antigravity-hook.sh' @@ -165,10 +166,9 @@ describe('AntigravityHookService', () => { const definition = config['orca-status'][eventName][0] const command = eventName === 'PostToolUse' ? definition.hooks?.[0]?.command : definition.command - expect(command).toContain(wrapperFileName) + expect(createManagedCommandMatcher(wrapperFileName)(command)).toBe(true) expect(command).not.toContain('cmd /d /s /c') expect(command).not.toContain('ORCA_ANTIGRAVITY_EVENT') - expect(command).not.toContain('"') const wrapper = readFileSync(join(homeDir, '.orca', 'agent-hooks', wrapperFileName), 'utf8') expect(wrapper).toContain(`set "ORCA_ANTIGRAVITY_EVENT=${eventName}"`) diff --git a/src/main/antigravity/hook-service.ts b/src/main/antigravity/hook-service.ts index c8e30632a..e83160a35 100644 --- a/src/main/antigravity/hook-service.ts +++ b/src/main/antigravity/hook-service.ts @@ -14,6 +14,7 @@ import { readHooksJson, removeManagedCommands, wrapPosixHookCommand, + wrapWindowsCmdHookCommand, writeHooksJson, writeManagedScript, type HookDefinition, @@ -24,6 +25,12 @@ import { writeHooksJsonRemote, writeManagedScriptRemote } from '../agent-hooks/installer-utils-remote' +import { + buildPosixHookPayloadCapture, + buildWindowsHookEnvironmentGuardLines, + buildWindowsHookStdinDrainEpilogue, + WINDOWS_HOOK_STDIN_DRAIN_COMMAND +} from '../agent-hooks/hook-stdin-contract' const ANTIGRAVITY_HOOK_BUNDLE_NAME = 'orca-status' @@ -76,7 +83,7 @@ function getWindowsWrapperScriptPath(event: AntigravityEvent): string { function getManagedCommand(scriptPath: string, event: AntigravityEvent): string { if (process.platform === 'win32') { - return getWindowsWrapperScriptPath(event) + return wrapWindowsCmdHookCommand(getWindowsWrapperScriptPath(event)) } return wrapPosixHookCommand(scriptPath, { ORCA_ANTIGRAVITY_EVENT: event.eventName }) } @@ -92,11 +99,10 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { ' echo {}', ')', 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', - 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', - 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', - 'if "%ORCA_PANE_KEY%"=="" exit /b 0', + ...buildWindowsHookEnvironmentGuardLines(), buildWindowsAntigravityHookPostCommand(), 'exit /b 0', + ...buildWindowsHookStdinDrainEpilogue(), '' ].join('\r\n') } @@ -114,18 +120,15 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { ' printf "{}\\n"', ' ;;', 'esac', + // Why: some Antigravity events arrive without stdin but still need a + // status post, so the shared capture maps empty input to an object. + ...buildPosixHookPayloadCapture('empty-object'), 'if [ -n "$ORCA_AGENT_HOOK_ENDPOINT" ] && [ -r "$ORCA_AGENT_HOOK_ENDPOINT" ]; then', ' . "$ORCA_AGENT_HOOK_ENDPOINT" 2>/dev/null || :', 'fi', 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', - 'payload=$(cat)', - 'if [ -z "$payload" ]; then', - // Why: some Antigravity hook events can arrive without stdin. Still post - // the event name so Orca shows a status row instead of silently dropping it. - " payload='{}'", - 'fi', // Timeout caps best-effort hook posts if the local listener stalls. // Why: pipe payload to curl's stdin (`payload@-`) instead of an inline // `payload=$VALUE` arg, so tens-of-KB tool output stays off the curl @@ -162,6 +165,9 @@ function getWindowsWrapperScript(eventName: string): string { ') else (', ' echo {}', ')', + // Why: when the shared core script is missing, this wrapper becomes the + // stdin owner and must finish the agent's payload write before returning. + WINDOWS_HOOK_STDIN_DRAIN_COMMAND, 'exit /b 0', '' ].join('\r\n') diff --git a/src/main/claude/hook-service.test.ts b/src/main/claude/hook-service.test.ts index 6400efe73..1c07cf389 100644 --- a/src/main/claude/hook-service.test.ts +++ b/src/main/claude/hook-service.test.ts @@ -247,8 +247,8 @@ describe('ClaudeHookService.installRemote', () => { expect(settings).toBeTruthy() const parsed = JSON.parse(settings!) // Why: every load-bearing event must be present and point at the - // remote-shaped script path with the `if [ -x ... ]; then ... fi` - // wrapper applied. Drift in any of these is a real bug — Claude + // remote-shaped script path with the guarded launcher applied. Drift in + // any of these is a real bug — Claude // Code rejects unknown shapes silently and the agent-hooks pipeline // goes dark. for (const event of [ @@ -266,7 +266,7 @@ describe('ClaudeHookService.installRemote', () => { expect(parsed.hooks[event]).toBeTruthy() const cmd = parsed.hooks[event][0].hooks[0].command as string expect(cmd).toContain('/home/dev/.orca/agent-hooks/claude-hook.sh') - expect(cmd).toMatch(/^if \[ -x /) + expect(cmd).toMatch(/^if \[ -f /) } // Managed script body const script = fs.files.get('/home/dev/.orca/agent-hooks/claude-hook.sh') @@ -355,7 +355,7 @@ describe('OpenClaudeHookService-compatible install', () => { const command = parsed.hooks[event][0].hooks[0].command as string expect(isOpenClaudeManagedCommand(command)).toBe(true) if (process.platform !== 'win32') { - expect(command).toMatch(/^if \[ -x /) + expect(command).toMatch(/^if \[ -f /) } } expect( diff --git a/src/main/claude/hook-service.ts b/src/main/claude/hook-service.ts index 758daec38..980c9604b 100644 --- a/src/main/claude/hook-service.ts +++ b/src/main/claude/hook-service.ts @@ -11,6 +11,12 @@ import { writeHooksJsonRemote, writeManagedScriptRemote } from '../agent-hooks/installer-utils-remote' +import { + buildPosixHookPayloadCapture, + buildWindowsHookEnvironmentGuardLines, + buildWindowsHookStdinDrainEpilogue, + WINDOWS_HOOK_STDIN_DRAIN_LABEL +} from '../agent-hooks/hook-stdin-contract' import { applyManagedHooks, CLAUDE_EVENTS, @@ -50,7 +56,7 @@ function getManagedScript( ? [ // Why: Devin imports .claude hooks by default. Skip Orca's managed // Claude hook there so status posts stay attributed to Devin. - 'if not "%DEVIN_PROJECT_DIR%"=="" exit /b 0' + `if not "%DEVIN_PROJECT_DIR%"=="" goto :${WINDOWS_HOOK_STDIN_DRAIN_LABEL}` ] : []), // Why: the endpoint file holds the *live* port/token for this Orca @@ -60,9 +66,7 @@ function getManagedScript( // reaches the current server. Falls through to PTY env if the file // is missing (first run / pre-endpoint-file / running outside Orca). 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', - 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', - 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', - 'if "%ORCA_PANE_KEY%"=="" exit /b 0', + ...buildWindowsHookEnvironmentGuardLines(), // 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 @@ -70,12 +74,14 @@ function getManagedScript( // curl works the same here as for the POSIX/Codex hooks. buildWindowsAgentHookCurlPostCommand('claude'), 'exit /b 0', + ...buildWindowsHookStdinDrainEpilogue(), '' ].join('\r\n') } return [ '#!/bin/sh', + ...buildPosixHookPayloadCapture(), ...(options.skipWhenDevinImportsClaude ? [ // Why: Devin imports .claude hooks by default. Skip Orca's managed @@ -105,10 +111,6 @@ function getManagedScript( 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', - 'payload=$(cat)', - 'if [ -z "$payload" ]; then', - ' exit 0', - 'fi', // Why: worktreeId embeds a filesystem path, so hand-building JSON in POSIX // shell is not safe once a path contains quotes or newlines. Post the raw // hook payload plus metadata as form fields and let the receiver parse it. diff --git a/src/main/codex/hook-service-wsl-runtime.test.ts b/src/main/codex/hook-service-wsl-runtime.test.ts index 232725e5f..d6b06f851 100644 --- a/src/main/codex/hook-service-wsl-runtime.test.ts +++ b/src/main/codex/hook-service-wsl-runtime.test.ts @@ -66,6 +66,10 @@ function getManagedTrustEntry( } } +function expectedManagedCommand(scriptPath: string): string { + return `if [ -f '${scriptPath}' ] && [ -r '${scriptPath}' ]; then /bin/sh '${scriptPath}'; else cat >/dev/null 2>&1 || :; fi` +} + describe('Codex WSL runtime hook install', () => { it('plans WSL hook files with Linux command and trust paths', () => { const runtimeHome = @@ -133,7 +137,7 @@ describe('Codex WSL runtime hook install', () => { trustConfigPath: '/old/home/hooks.json' } expect(_internals.installManagedHooksIntoWslRuntime(oldPlan).state).toBe('installed') - const oldCommand = `if [ -r '${oldPlan.commandScriptPath}' ]; then /bin/sh '${oldPlan.commandScriptPath}'; fi` + const oldCommand = expectedManagedCommand(oldPlan.commandScriptPath) const oldKey = computeTrustKey(getManagedTrustEntry(oldPlan, oldCommand)) const newPlan = { @@ -142,7 +146,7 @@ describe('Codex WSL runtime hook install', () => { trustConfigPath: '/new/home/hooks.json' } expect(_internals.installManagedHooksIntoWslRuntime(newPlan).state).toBe('installed') - const newCommand = `if [ -r '${newPlan.commandScriptPath}' ]; then /bin/sh '${newPlan.commandScriptPath}'; fi` + const newCommand = expectedManagedCommand(newPlan.commandScriptPath) const newKey = computeTrustKey(getManagedTrustEntry(newPlan, newCommand)) const trustEntries = readHookTrustEntries(plan.tomlPath) @@ -150,6 +154,30 @@ describe('Codex WSL runtime hook install', () => { expect(trustEntries.has(newKey)).toBe(true) }) + it.skipIf(process.platform === 'win32')( + 'drains stdin when the WSL runtime script is missing', + () => { + const basePlan = createTestPlan() + const plan = { + ...basePlan, + commandScriptPath: join(dirname(basePlan.configPath), 'missing-codex-hook.sh') + } + writeFileSync(plan.configPath, '{"hooks":{}}\n', 'utf-8') + writeFileSync(plan.tomlPath, '', 'utf-8') + + expect(_internals.installManagedHooksIntoWslRuntime(plan).state).toBe('installed') + const installed = JSON.parse(readFileSync(plan.configPath, 'utf-8')) as HooksConfig + const command = installed.hooks.UserPromptSubmit[0]?.hooks?.[0]?.command + expect(command).toBe(expectedManagedCommand(plan.commandScriptPath)) + + const result = spawnSync('/bin/sh', ['-c', command!], { + input: Buffer.alloc(1_000_000, 'x') + }) + expect(result.error).toBeUndefined() + expect(result.status).toBe(0) + } + ) + it('sweeps all managed WSL trust for disable or confirmed absence', () => { // Why: disable and confirmed absence intentionally pass []. Transient // unavailability must NOT use this path — last known-good trust remains. @@ -351,9 +379,7 @@ describe('Codex WSL runtime hook install', () => { const installed = JSON.parse(readFileSync(plan.configPath, 'utf-8')) as HooksConfig expect(Object.keys(installed.hooks).sort()).toEqual([...managedEvents].sort()) const managedCommand = installed.hooks.UserPromptSubmit[0]?.hooks?.[0]?.command - expect(managedCommand).toBe( - `if [ -r '${plan.commandScriptPath}' ]; then /bin/sh '${plan.commandScriptPath}'; fi` - ) + expect(managedCommand).toBe(expectedManagedCommand(plan.commandScriptPath)) expect(installed.hooks.UserPromptSubmit[1]?.hooks?.[0]?.command).toBe(userCommand) expect(readFileSync(plan.scriptPath, 'utf-8')).toContain('command -v curl.exe') diff --git a/src/main/codex/hook-service.ts b/src/main/codex/hook-service.ts index 0ad81dede..b09a94f5e 100644 --- a/src/main/codex/hook-service.ts +++ b/src/main/codex/hook-service.ts @@ -25,6 +25,12 @@ import { writeManagedScriptRemote, writeTextFileRemoteAtomic } from '../agent-hooks/installer-utils-remote' +import { + buildPosixHookPayloadCapture, + buildWindowsHookEnvironmentGuardLines, + buildWindowsHookStdinDrainEpilogue, + POSIX_HOOK_STDIN_DRAIN_COMMAND +} from '../agent-hooks/hook-stdin-contract' import { computeTrustKey, computeTrustedHash, @@ -136,8 +142,8 @@ export type { CodexWslRuntimeHookInstallPlan } function wrapReadablePosixHookCommand(scriptPath: string): string { const quoted = `'${scriptPath.replaceAll("'", "'\\''")}'` // Why: WSL runtime hooks are written from Windows through UNC, where the - // executable bit is not reliable. /bin/sh only needs the script to be readable. - return `if [ -r ${quoted} ]; then /bin/sh ${quoted}; fi` + // executable bit is not reliable; a missing script must still own stdin. + return `if [ -f ${quoted} ] && [ -r ${quoted} ]; then /bin/sh ${quoted}; else ${POSIX_HOOK_STDIN_DRAIN_COMMAND}; fi` } function getSystemConfigPath(): string { @@ -781,17 +787,17 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { // surviving PTY reach the current server even though its env points at // the prior Orca's coordinates. 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', - 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', - 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', - 'if "%ORCA_PANE_KEY%"=="" exit /b 0', + ...buildWindowsHookEnvironmentGuardLines(), buildWindowsAgentHookCurlPostCommand('codex'), 'exit /b 0', + ...buildWindowsHookStdinDrainEpilogue(), '' ].join('\r\n') } return [ '#!/bin/sh', + ...buildPosixHookPayloadCapture(), // Why: see claude/hook-service.ts for rationale. Sourcing refreshes // PORT/TOKEN/ENV/VERSION from the current Orca so a surviving PTY keeps // reporting after a restart. @@ -823,10 +829,6 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', - 'payload=$(cat)', - 'if [ -z "$payload" ]; then', - ' exit 0', - 'fi', 'post_codex_hook() {', ' curl_bin="$1"', ' connect_timeout="${2:-0.5}"', diff --git a/src/main/command-code/command-code-managed-script.ts b/src/main/command-code/command-code-managed-script.ts index cc625c605..ca45f2fa5 100644 --- a/src/main/command-code/command-code-managed-script.ts +++ b/src/main/command-code/command-code-managed-script.ts @@ -1,4 +1,9 @@ import { buildWindowsAgentHookPostCommand } from '../agent-hooks/installer-utils' +import { + buildPosixHookPayloadCapture, + buildWindowsHookEnvironmentGuardLines, + buildWindowsHookStdinDrainEpilogue +} from '../agent-hooks/hook-stdin-contract' export type CommandCodeManagedScriptTarget = 'local' | 'posix' @@ -11,9 +16,7 @@ export function buildCommandCodeManagedScript( 'setlocal', 'if "%ORCA_AGENT_HOOK_PORT%"=="" if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" if not "%ORCA_AGENT_HOOK_PORT%"=="" call :sourceEndpointByPort', - 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', - 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', - 'if "%ORCA_PANE_KEY%"=="" exit /b 0', + ...buildWindowsHookEnvironmentGuardLines(), buildWindowsAgentHookPostCommand('command-code'), 'exit /b 0', ':sourceEndpointByPort', @@ -25,12 +28,14 @@ export function buildCommandCodeManagedScript( 'if not "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', 'for /f "tokens=2 delims==" %%P in (\'findstr /b /c:"set ORCA_AGENT_HOOK_PORT=" "%~1" 2^>nul\') do if "%%P"=="%ORCA_AGENT_HOOK_PORT%" call "%~1" 2>nul', 'exit /b 0', + ...buildWindowsHookStdinDrainEpilogue(), '' ].join('\r\n') } return [ '#!/bin/sh', + ...buildPosixHookPayloadCapture(), '__orca_read_ancestor_var() {', ' __orca_name="$1"', ' __orca_pid="${PPID:-}"', @@ -116,10 +121,6 @@ export function buildCommandCodeManagedScript( 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', - 'payload=$(cat)', - 'if [ -z "$payload" ]; then', - ' exit 0', - 'fi', // Timeout caps best-effort hook posts if the local listener stalls. // Why: pipe payload to curl's stdin (`payload@-`) instead of an inline // `payload=$VALUE` arg, so tens-of-KB tool output stays off the curl diff --git a/src/main/command-code/hook-service.test.ts b/src/main/command-code/hook-service.test.ts index 4b8b4bfce..6e2945163 100644 --- a/src/main/command-code/hook-service.test.ts +++ b/src/main/command-code/hook-service.test.ts @@ -58,7 +58,7 @@ describe('CommandCodeHookService', () => { expect(config.hooks.PreToolUse[0].hooks[0].command).toContain(join(homeDir, '.orca')) } if (process.platform !== 'win32') { - expect(config.hooks.PreToolUse[0].hooks[0].command).toMatch(/^if \[ -x /) + expect(config.hooks.PreToolUse[0].hooks[0].command).toMatch(/^if \[ -f /) } }) diff --git a/src/main/copilot/hook-service.test.ts b/src/main/copilot/hook-service.test.ts index 68ae69629..a9260460d 100644 --- a/src/main/copilot/hook-service.test.ts +++ b/src/main/copilot/hook-service.test.ts @@ -88,12 +88,16 @@ describe('CopilotHookService', () => { expect(firstPromptHook.type).toBe('command') expect(firstPromptHook.timeoutSec).toBe(5) if (process.platform === 'win32') { - expect(firstPromptHook.powershell).toContain('agent-hooks') - expect(firstPromptHook.powershell).toContain('copilot-hook.ps1') - expect(firstPromptHook.powershell).toContain('ORCA_COPILOT_HOOK_EVENT') - expect(firstPromptHook.powershell).toContain('UserPromptSubmit') + const powershell = firstPromptHook.powershell as string + expect(powershell).toContain('-EncodedCommand') + const encoded = powershell.match(/ -EncodedCommand (\S+)$/)?.[1] + const decoded = Buffer.from(encoded!, 'base64').toString('utf16le') + expect(decoded).toContain('agent-hooks') + expect(decoded).toContain('copilot-hook.ps1') + expect(decoded).toContain("$env:ORCA_COPILOT_HOOK_EVENT = 'UserPromptSubmit'") } else { - expect(firstPromptHook.bash).toContain('if [ -x ') + expect(firstPromptHook.bash).toContain('if [ -f ') + expect(firstPromptHook.bash).toContain('] && [ -x ') expect(firstPromptHook.bash).toContain('.orca/agent-hooks/copilot-hook.sh') expect(firstPromptHook.bash).toContain("ORCA_COPILOT_HOOK_EVENT='UserPromptSubmit'") } diff --git a/src/main/copilot/hook-service.ts b/src/main/copilot/hook-service.ts index a61588ebf..6a8206d0b 100644 --- a/src/main/copilot/hook-service.ts +++ b/src/main/copilot/hook-service.ts @@ -12,6 +12,7 @@ import { readHooksJson, removeManagedCommands, wrapPosixHookCommand, + wrapWindowsHookCommand, writeHooksJson, writeManagedScript, type HookDefinition @@ -21,6 +22,7 @@ import { writeHooksJsonRemote, writeManagedScriptRemote } from '../agent-hooks/installer-utils-remote' +import { buildPosixHookPayloadCapture } from '../agent-hooks/hook-stdin-contract' // Why: Copilot's user-level hook files can use VS Code-compatible PascalCase // names, which match the event vocabulary already normalized by Orca's hook @@ -61,14 +63,11 @@ function getManagedScriptPath(): string { return getSharedManagedScriptPath(getManagedScriptFileName()) } -function quotePowerShellPath(path: string): string { - return `'${path.replaceAll("'", "''")}'` -} - function getManagedCommand(scriptPath: string, eventName: string): string { - return process.platform === 'win32' - ? `$env:ORCA_COPILOT_HOOK_EVENT = '${eventName}'; powershell.exe -NoProfile -ExecutionPolicy Bypass -File ${quotePowerShellPath(scriptPath)}` - : wrapPosixHookCommand(scriptPath, { ORCA_COPILOT_HOOK_EVENT: eventName }) + if (process.platform !== 'win32') { + return wrapPosixHookCommand(scriptPath, { ORCA_COPILOT_HOOK_EVENT: eventName }) + } + return wrapWindowsHookCommand(scriptPath, { ORCA_COPILOT_HOOK_EVENT: eventName }) } function getManagedHookDefinition(command: string): HookDefinition { @@ -118,6 +117,7 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { if (target === 'local' && process.platform === 'win32') { return [ "Write-Output '{}'", + '$inputData = [Console]::In.ReadToEnd()', // Why: endpoint.cmd is cmd syntax, not PowerShell. Parse its `set KEY=...` // lines so surviving PTYs can refresh to the current Orca server. 'if ($env:ORCA_AGENT_HOOK_ENDPOINT -and (Test-Path -LiteralPath $env:ORCA_AGENT_HOOK_ENDPOINT)) {', @@ -130,7 +130,6 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { ' } catch {}', '}', 'if (-not $env:ORCA_AGENT_HOOK_PORT -or -not $env:ORCA_AGENT_HOOK_TOKEN -or -not $env:ORCA_PANE_KEY) { exit 0 }', - '$inputData = [Console]::In.ReadToEnd()', 'if ([string]::IsNullOrWhiteSpace($inputData)) { exit 0 }', 'try {', ' $payload = $inputData | ConvertFrom-Json', @@ -154,6 +153,7 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { return [ '#!/bin/sh', "printf '{}\\n'", + ...buildPosixHookPayloadCapture(), // Why: Copilot consumes stdout for some hooks, so stdout is emitted before // endpoint refresh, stdin parsing, or the network POST can fail. 'if [ -n "$ORCA_AGENT_HOOK_ENDPOINT" ] && [ -r "$ORCA_AGENT_HOOK_ENDPOINT" ]; then', @@ -162,10 +162,6 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', - 'payload=$(cat)', - 'if [ -z "$payload" ]; then', - ' exit 0', - 'fi', // Why: pipe payload to curl's stdin (`payload@-`) instead of an inline // `payload=$VALUE` arg, so tens-of-KB tool output stays off the curl // command line (EDR command-line false positives). Wire body is identical. diff --git a/src/main/cursor/hook-service.ts b/src/main/cursor/hook-service.ts index 9c5d19e8f..b1ccdbf12 100644 --- a/src/main/cursor/hook-service.ts +++ b/src/main/cursor/hook-service.ts @@ -20,6 +20,11 @@ import { writeHooksJsonRemote, writeManagedScriptRemote } from '../agent-hooks/installer-utils-remote' +import { + buildPosixHookPayloadCapture, + buildWindowsHookEnvironmentGuardLines, + buildWindowsHookStdinDrainEpilogue +} from '../agent-hooks/hook-stdin-contract' // Why: cursor-agent exposes a declarative hooks.json surface at // ~/.cursor/hooks.json (https://cursor.com/docs/hooks) with camelCase event @@ -76,17 +81,17 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { // surviving PTY reach the current server even though its env points at // the prior Orca's coordinates. 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', - 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', - 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', - 'if "%ORCA_PANE_KEY%"=="" exit /b 0', + ...buildWindowsHookEnvironmentGuardLines(), buildWindowsAgentHookPostCommand('cursor'), 'exit /b 0', + ...buildWindowsHookStdinDrainEpilogue(), '' ].join('\r\n') } return [ '#!/bin/sh', + ...buildPosixHookPayloadCapture(), // Why: see claude/hook-service.ts for rationale. Sourcing refreshes // PORT/TOKEN/ENV/VERSION from the current Orca so a surviving PTY keeps // reporting after a restart. @@ -96,10 +101,6 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', - 'payload=$(cat)', - 'if [ -z "$payload" ]; then', - ' exit 0', - 'fi', // Why: worktreeId embeds a filesystem path, so hand-building JSON in POSIX // shell is not safe once a path contains quotes or newlines. Post the raw // hook payload plus metadata as form fields and let the receiver parse it. diff --git a/src/main/devin/hook-service.test.ts b/src/main/devin/hook-service.test.ts index 6bc105ffd..98a839938 100644 --- a/src/main/devin/hook-service.test.ts +++ b/src/main/devin/hook-service.test.ts @@ -126,9 +126,12 @@ describe('DevinHookService', () => { Object.defineProperty(process, 'platform', { value: 'win32' }) try { const scriptPath = 'C:\\Users\\Ada Lovelace\\.orca\\agent-hooks\\devin-hook.cmd' - expect(getDevinManagedCommand(scriptPath)).toBe( - 'cmd /d /s /c ""C:\\Users\\Ada Lovelace\\.orca\\agent-hooks\\devin-hook.cmd""' - ) + const command = getDevinManagedCommand(scriptPath) + const encoded = command.match(/ -EncodedCommand (\S+)$/)?.[1] + expect(encoded).toBeDefined() + const decoded = Buffer.from(encoded!, 'base64').toString('utf16le') + expect(decoded).toContain(`Test-Path -LiteralPath '${scriptPath}' -PathType Leaf`) + expect(decoded).toContain('[Console]::In.ReadToEnd() | Out-Null') } finally { Object.defineProperty(process, 'platform', { value: previous }) } diff --git a/src/main/devin/hook-service.ts b/src/main/devin/hook-service.ts index b0d63b96e..8b4cc4390 100644 --- a/src/main/devin/hook-service.ts +++ b/src/main/devin/hook-service.ts @@ -10,6 +10,11 @@ import { writeHooksJsonRemote, writeManagedScriptRemote } from '../agent-hooks/installer-utils-remote' +import { + buildPosixHookPayloadCapture, + buildWindowsHookEnvironmentGuardLines, + buildWindowsHookStdinDrainEpilogue +} from '../agent-hooks/hook-stdin-contract' import { applyDevinManagedHooks, DEVIN_EVENTS, @@ -41,17 +46,17 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { // reaches the current server. Falls through to PTY env if the file // is missing (first run / pre-endpoint-file / running outside Orca). 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', - 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', - 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', - 'if "%ORCA_PANE_KEY%"=="" exit /b 0', + ...buildWindowsHookEnvironmentGuardLines(), buildWindowsAgentHookPostCommand('devin'), 'exit /b 0', + ...buildWindowsHookStdinDrainEpilogue(), '' ].join('\r\n') } return [ '#!/bin/sh', + ...buildPosixHookPayloadCapture(), // Why: the endpoint file holds the *live* port/token for this Orca // install. PTYs that survive an Orca restart have stale PORT/TOKEN // baked into their env from the old instance — sourcing the file here @@ -72,10 +77,6 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', - 'payload=$(cat)', - 'if [ -z "$payload" ]; then', - ' exit 0', - 'fi', // Why: worktreeId embeds a filesystem path, so hand-building JSON in POSIX // shell is not safe once a path contains quotes or newlines. Post the raw // hook payload plus metadata as form fields and let the receiver parse it. diff --git a/src/main/devin/hook-settings.ts b/src/main/devin/hook-settings.ts index a27f47523..674ddb6c2 100644 --- a/src/main/devin/hook-settings.ts +++ b/src/main/devin/hook-settings.ts @@ -6,6 +6,7 @@ import { getSharedManagedScriptPath, removeManagedCommands, wrapPosixHookCommand, + wrapWindowsCmdHookCommand, type HookDefinition, type HooksConfig } from '../agent-hooks/installer-utils' @@ -51,9 +52,9 @@ export function getDevinRemoteConfigPath(remoteHome: string): string { export function getDevinManagedCommand(scriptPath: string): string { if (process.platform === 'win32') { - // Why: Devin runs hooks through the platform shell on Windows; invoking the - // .cmd via cmd.exe preserves spaces in the shared ~/.orca script path. - return `cmd /d /s /c ""${scriptPath.replaceAll('"', '""')}""` + // Why: keep safe paths on cmd.exe's fast path; the fallback protects spaced + // paths, and both forms drain stdin for a stale missing-script entry. + return wrapWindowsCmdHookCommand(scriptPath) } return wrapPosixHookCommand(scriptPath) } diff --git a/src/main/droid/hook-service.ts b/src/main/droid/hook-service.ts index 946f5ac88..508f24e98 100644 --- a/src/main/droid/hook-service.ts +++ b/src/main/droid/hook-service.ts @@ -21,6 +21,11 @@ import { writeHooksJsonRemote, writeManagedScriptRemote } from '../agent-hooks/installer-utils-remote' +import { + buildPosixHookPayloadCapture, + buildWindowsHookEnvironmentGuardLines, + buildWindowsHookStdinDrainEpilogue +} from '../agent-hooks/hook-stdin-contract' // Why: SessionStart is installed (not just listened for) so that resuming a // droid session via `droid --resume` resets the per-pane prompt/tool caches @@ -77,27 +82,23 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { '@echo off', 'setlocal', 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', - 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', - 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', - 'if "%ORCA_PANE_KEY%"=="" exit /b 0', + ...buildWindowsHookEnvironmentGuardLines(), buildWindowsAgentHookPostCommand('droid'), 'exit /b 0', + ...buildWindowsHookStdinDrainEpilogue(), '' ].join('\r\n') } return [ '#!/bin/sh', + ...buildPosixHookPayloadCapture(), 'if [ -n "$ORCA_AGENT_HOOK_ENDPOINT" ] && [ -r "$ORCA_AGENT_HOOK_ENDPOINT" ]; then', ' . "$ORCA_AGENT_HOOK_ENDPOINT" 2>/dev/null || :', 'fi', 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', - 'payload=$(cat)', - 'if [ -z "$payload" ]; then', - ' exit 0', - 'fi', // Timeout caps best-effort hook posts if the local listener stalls. // Why: pipe payload to curl's stdin (`payload@-`) instead of an inline // `payload=$VALUE` arg, so tens-of-KB tool output stays off the curl diff --git a/src/main/gemini/hook-service.ts b/src/main/gemini/hook-service.ts index 423df12f8..e078476ed 100644 --- a/src/main/gemini/hook-service.ts +++ b/src/main/gemini/hook-service.ts @@ -21,6 +21,11 @@ import { writeHooksJsonRemote, writeManagedScriptRemote } from '../agent-hooks/installer-utils-remote' +import { + buildPosixHookPayloadCapture, + buildWindowsHookEnvironmentGuardLines, + buildWindowsHookStdinDrainEpilogue +} from '../agent-hooks/hook-stdin-contract' // Why: Gemini CLI fires `BeforeAgent` when a turn starts and `AfterAgent` when // it completes. `AfterTool` marks the resumption of model work after a tool @@ -65,11 +70,10 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { // surviving PTY reach the current server even though its env points at // the prior Orca's coordinates. 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', - 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', - 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', - 'if "%ORCA_PANE_KEY%"=="" exit /b 0', + ...buildWindowsHookEnvironmentGuardLines(), buildWindowsAgentHookPostCommand('gemini'), 'exit /b 0', + ...buildWindowsHookStdinDrainEpilogue(), '' ].join('\r\n') } @@ -80,6 +84,7 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { // to return. Emit `{}` first so the agent never stalls parsing our output, // even if the env-var guards below cause an early exit. 'printf "{}\\n"', + ...buildPosixHookPayloadCapture(), // Why: see claude/hook-service.ts for rationale. Sourcing refreshes // PORT/TOKEN/ENV/VERSION from the current Orca so a surviving PTY keeps // reporting after a restart. @@ -89,10 +94,6 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', - 'payload=$(cat)', - 'if [ -z "$payload" ]; then', - ' exit 0', - 'fi', // Why: worktreeId embeds a filesystem path, so hand-building JSON in POSIX // shell is not safe once a path contains quotes or newlines. Post the raw // hook payload plus metadata as form fields and let the receiver parse it. diff --git a/src/main/grok/hook-service.ts b/src/main/grok/hook-service.ts index c88e1254f..564565d0d 100644 --- a/src/main/grok/hook-service.ts +++ b/src/main/grok/hook-service.ts @@ -20,6 +20,11 @@ import { writeHooksJsonRemote, writeManagedScriptRemote } from '../agent-hooks/installer-utils-remote' +import { + buildPosixHookPayloadCapture, + buildWindowsHookEnvironmentGuardLines, + buildWindowsHookStdinDrainEpilogue +} from '../agent-hooks/hook-stdin-contract' // Why: Grok's tool-event matcher is a real regex (see Grok hooks docs). Bare // `*` is not a valid "match all" pattern and can fail to load/match, so tool @@ -115,9 +120,7 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { '@echo off', 'setlocal', 'if defined ORCA_AGENT_HOOK_ENDPOINT if exist "%ORCA_AGENT_HOOK_ENDPOINT%" call "%ORCA_AGENT_HOOK_ENDPOINT%" 2>nul', - 'if "%ORCA_AGENT_HOOK_PORT%"=="" exit /b 0', - 'if "%ORCA_AGENT_HOOK_TOKEN%"=="" exit /b 0', - 'if "%ORCA_PANE_KEY%"=="" exit /b 0', + ...buildWindowsHookEnvironmentGuardLines(), 'set "ORCA_GROK_HOME=%GROK_HOME%"', `if not "%GROK_HOME:~${GROK_HOME_ENVELOPE_MAX_LENGTH},1%"=="" set "ORCA_GROK_HOME="`, // Why: a trailing backslash escapes curl's closing argv quote on Windows, @@ -125,22 +128,20 @@ function getManagedScript(target: 'local' | 'posix' = 'local'): string { 'if "%ORCA_GROK_HOME:~-1%"=="\\" set "ORCA_GROK_HOME=%ORCA_GROK_HOME%."', WINDOWS_GROK_HOOK_POST_COMMAND, 'exit /b 0', + ...buildWindowsHookStdinDrainEpilogue(), '' ].join('\r\n') } return [ '#!/bin/sh', + ...buildPosixHookPayloadCapture(), 'if [ -n "$ORCA_AGENT_HOOK_ENDPOINT" ] && [ -r "$ORCA_AGENT_HOOK_ENDPOINT" ]; then', ' . "$ORCA_AGENT_HOOK_ENDPOINT" 2>/dev/null || :', 'fi', 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', - 'payload=$(cat)', - 'if [ -z "$payload" ]; then', - ' exit 0', - 'fi', 'grok_home=', `if [ -n "\${GROK_HOME:-}" ] && [ "\${#GROK_HOME}" -le ${GROK_HOME_ENVELOPE_MAX_LENGTH} ]; then`, ' grok_home=$GROK_HOME', diff --git a/src/main/kimi/hook-service.ts b/src/main/kimi/hook-service.ts index 459c0994e..94f5d11ea 100644 --- a/src/main/kimi/hook-service.ts +++ b/src/main/kimi/hook-service.ts @@ -23,6 +23,7 @@ import { writeManagedScriptRemote, writeTextFileRemoteAtomic } from '../agent-hooks/installer-utils-remote' +import { buildPosixHookPayloadCapture } from '../agent-hooks/hook-stdin-contract' import { applyManagedKimiHooks, KIMI_HOOK_EVENTS, @@ -59,6 +60,7 @@ function getManagedCommand(scriptPath: string): string { function getManagedScript(): string { return [ '#!/bin/sh', + ...buildPosixHookPayloadCapture(), // Why: refresh PORT/TOKEN/ENV/VERSION from the current Orca install so a PTY // that survived an Orca restart still reaches the live listener. See // claude/hook-service.ts for the full rationale. @@ -68,10 +70,6 @@ function getManagedScript(): string { 'if [ -z "$ORCA_AGENT_HOOK_PORT" ] || [ -z "$ORCA_AGENT_HOOK_TOKEN" ] || [ -z "$ORCA_PANE_KEY" ]; then', ' exit 0', 'fi', - 'payload=$(cat)', - 'if [ -z "$payload" ]; then', - ' exit 0', - 'fi', // Why: worktreeId embeds a filesystem path, so hand-building JSON in POSIX // shell is not safe once a path contains quotes or newlines. Post the raw // hook payload plus metadata as form fields and let the receiver parse it.