From 455ff742cc8365ef9ff54f5aa327eed4e9c0cd34 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 20 Sep 2026 21:02:59 -0700 Subject: [PATCH] fix(antigravity): stop waiting on abandoned hook input (#21634) * fix(antigravity): bound hook stdin without forwarding timed-out payloads * fix(cli): include bounded Antigravity hook reader in typecheck --- config/tsconfig.cli.json | 1 + src/main/agent-hooks/hook-stdin-contract.ts | 115 ++++++---- .../managed-hook-stdin-lifecycle.test.ts | 11 +- .../agent-hooks/posix-hook-bounded-stdin.ts | 32 +++ src/main/antigravity/hook-script.ts | 12 +- src/main/antigravity/hook-service.test.ts | 3 +- .../posix-hook-payload-delivery.test.ts | 197 ++++++++++++++++++ 7 files changed, 322 insertions(+), 49 deletions(-) create mode 100644 src/main/agent-hooks/posix-hook-bounded-stdin.ts create mode 100644 src/main/antigravity/posix-hook-payload-delivery.test.ts diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index 43eac7cb024..b268e886463 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -8,6 +8,7 @@ "../src/main/claude/hook-script.ts", "../src/main/claude/claude-session-end-hook-capability.ts", "../src/main/agent-hooks/hook-stdin-contract.ts", + "../src/main/agent-hooks/posix-hook-bounded-stdin.ts", "../src/main/agent-hooks/hook-post-command.ts", "../src/main/agent-hooks/hook-config-write-path.ts", "../src/main/agent-hooks/hooks-json-read.ts", diff --git a/src/main/agent-hooks/hook-stdin-contract.ts b/src/main/agent-hooks/hook-stdin-contract.ts index eeec395b578..eb1380e64a3 100644 --- a/src/main/agent-hooks/hook-stdin-contract.ts +++ b/src/main/agent-hooks/hook-stdin-contract.ts @@ -20,58 +20,66 @@ export const POSIX_HOOK_JSON_STDIN_IDLE_TIMEOUT_SECONDS = 1.5 // // Three invariants this script must hold, because the shell chains a second // reader behind it and a reader that consumed bytes cannot be retried: -// 1. A non-zero exit implies stdin was never read, so the `||` fallback still -// sees the whole stream. Everything after the imports is therefore guarded. +// 1. Default-mode failures imply stdin was never read. Strict mode reserves +// exit 65 for incomplete consumed input, which its launcher never retries. // 2. Decoding is incremental. A multi-byte character straddling two reads must // not raise, or a CJK/emoji payload falls through to `cat` and hangs. // 3. The payload is emitted unchanged. Re-serialising would rewrite non-ASCII // as \uXXXX and reorder keys behind the agent's back. -const POSIX_HOOK_JSON_STDIN_PYTHON = [ - 'import codecs, json, os, select', - 'text = ""', - 'try:', - ' decoder = codecs.getincrementaldecoder("utf-8")("replace")', - ` timeout = ${POSIX_HOOK_JSON_STDIN_FIRST_BYTE_TIMEOUT_SECONDS}.0`, - ' while 1:', - ' if not select.select([0], [], [], timeout)[0]:', - ' text += decoder.decode(b"", True)', - ' break', - ' chunk = os.read(0, 65536)', - ' if not chunk:', - ' text += decoder.decode(b"", True)', - ' break', - ` timeout = ${POSIX_HOOK_JSON_STDIN_IDLE_TIMEOUT_SECONDS}`, - ' text += decoder.decode(chunk)', - // raw_decode does not skip leading whitespace, so a padded payload would - // otherwise never complete and would wait out the idle timeout. - ' value = text.lstrip()', - ' if not value:', - ' continue', - ' try:', - ' end = json.JSONDecoder().raw_decode(value)[1]', - ' except ValueError:', - ' continue', - ' text = value[:end]', - ' break', - 'except Exception:', - ' pass', - 'try:', - // os.write skips the locale-dependent stdout encoder, which raises under - // LC_ALL=C for a non-ASCII payload. - ' data = text.encode("utf-8")', - ' written = 0', - ' while written < len(data):', - ' written += os.write(1, data[written:])', - 'except Exception:', - ' pass' -].join('\n') +function jsonStdinPython( + rejectIncomplete = false, + idleTimeoutSeconds = POSIX_HOOK_JSON_STDIN_IDLE_TIMEOUT_SECONDS +): string { + return [ + 'import codecs, json, os, select', + ...(rejectIncomplete ? ['import sys', 'complete = False'] : []), + 'text = ""', + 'try:', + ' decoder = codecs.getincrementaldecoder("utf-8")("replace")', + ` timeout = ${POSIX_HOOK_JSON_STDIN_FIRST_BYTE_TIMEOUT_SECONDS}.0`, + ' while 1:', + ' if not select.select([0], [], [], timeout)[0]:', + ' text += decoder.decode(b"", True)', + ' break', + ' chunk = os.read(0, 65536)', + ' if not chunk:', + ' text += decoder.decode(b"", True)', + ' break', + ` timeout = ${idleTimeoutSeconds}`, + ' text += decoder.decode(chunk)', + // raw_decode does not skip leading whitespace, so a padded payload would + // otherwise never complete and would wait out the idle timeout. + ' value = text.lstrip()', + ' if not value:', + ' continue', + ' try:', + ' end = json.JSONDecoder().raw_decode(value)[1]', + ' except ValueError:', + ' continue', + ' text = value[:end]', + ...(rejectIncomplete ? [' complete = True'] : []), + ' break', + 'except Exception:', + ' pass', + ...(rejectIncomplete ? ['if text.strip() and not complete:', ' sys.exit(65)'] : []), + 'try:', + // os.write skips the locale-dependent stdout encoder, which raises under + // LC_ALL=C for a non-ASCII payload. + ' data = text.encode("utf-8")', + ' written = 0', + ' while written < len(data):', + ' written += os.write(1, data[written:])', + 'except Exception:', + ' pass' + ].join('\n') +} // Why a variable rather than two inline copies: the script is embedded twice in // the reader chain, and `-c '<600 chars>'` twice is an EDR oversized-command-line // signal as well as unreadable in the generated hook. const POSIX_HOOK_JSON_STDIN_PYTHON_VAR = 'orca_hook_json_stdin_py' export const POSIX_HOOK_JSON_STDIN_PRELUDE: readonly string[] = [ - `${POSIX_HOOK_JSON_STDIN_PYTHON_VAR}='${POSIX_HOOK_JSON_STDIN_PYTHON}'` + `${POSIX_HOOK_JSON_STDIN_PYTHON_VAR}='${jsonStdinPython()}'` ] const jsonStdinInterpreter = (name: string): string => @@ -101,6 +109,29 @@ export const POSIX_HOOK_JSON_STDIN: PosixHookStdinReader = { prelude: POSIX_HOOK_JSON_STDIN_PRELUDE } +export function createStrictPosixHookJsonStdinReader( + fallback: PosixHookStdinReader +): PosixHookStdinReader { + return { + prelude: [ + `${POSIX_HOOK_JSON_STDIN_PYTHON_VAR}='${jsonStdinPython(true, POSIX_HOOK_JSON_STDIN_FIRST_BYTE_TIMEOUT_SECONDS)}'`, + ...fallback.prelude, + 'orca_read_hook_json() {', + ...['python3', 'python'].flatMap((name) => [ + ` ${jsonStdinInterpreter(name)}`, + ' case $? in', + ' 0) return 0 ;;', + // Exit 65 consumed incomplete input; another reader would lose its prefix. + ' 65) return 65 ;;', + ' esac' + ]), + ` ${fallback.reader}`, + '}' + ], + reader: `${POSIX_HOOK_JSON_STDIN_HOME_GUARD}; orca_read_hook_json` + } +} + // 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( diff --git a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts index cd6c336e751..2420116a33c 100644 --- a/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts +++ b/src/main/agent-hooks/managed-hook-stdin-lifecycle.test.ts @@ -1,3 +1,4 @@ +import { POSIX_HOOK_BOUNDED_STDIN } from './posix-hook-bounded-stdin' // Why: stdin ownership is a cross-agent process contract; one executable // matrix catches an unread early exit without duplicating template assertions. // Exception (#11549): Windows batch hooks give up stdin ownership on the @@ -64,6 +65,7 @@ import { KimiHookService } from '../kimi/hook-service' import { openClaudeHookService } from '../openclaude/hook-service' import { wrapPosixHookCommand, wrapWindowsHookCommand } from './installer-utils' import { + createStrictPosixHookJsonStdinReader, POSIX_HOOK_JSON_STDIN_PRELUDE, POSIX_HOOK_JSON_STDIN_READER, POSIX_HOOK_STDIN_READER, @@ -563,11 +565,14 @@ 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 captureScript = script.replace(POSIX_HOOK_BOUNDED_STDIN.prelude.join('\n'), '') + const strictReader = createStrictPosixHookJsonStdinReader(POSIX_HOOK_BOUNDED_STDIN) const captureIndex = Math.max( - script.indexOf(`payload=$(${POSIX_HOOK_STDIN_READER})`), - script.indexOf(`payload=$(${POSIX_HOOK_JSON_STDIN_READER})`) + captureScript.indexOf(`payload=$(${POSIX_HOOK_STDIN_READER})`), + captureScript.indexOf(`payload=$(${POSIX_HOOK_JSON_STDIN_READER})`), + captureScript.indexOf(`payload=$(${strictReader.reader})`) ) - const firstExitIndex = script.indexOf('exit 0') + const firstExitIndex = captureScript.indexOf('exit 0') expect(captureIndex, `${agent} payload capture`).toBeGreaterThanOrEqual(0) expect(firstExitIndex, `${agent} first success exit`).toBeGreaterThan(captureIndex) // Why: the JSON reader dereferences a variable the prelude sets, so a script diff --git a/src/main/agent-hooks/posix-hook-bounded-stdin.ts b/src/main/agent-hooks/posix-hook-bounded-stdin.ts new file mode 100644 index 00000000000..8efea7624c7 --- /dev/null +++ b/src/main/agent-hooks/posix-hook-bounded-stdin.ts @@ -0,0 +1,32 @@ +import type { PosixHookStdinReader } from './hook-stdin-contract' + +export const POSIX_HOOK_EOF_TIMEOUT_SECONDS = 5 + +// A timed-out EOF reader must fail capture rather than forward a truncated payload. +export const POSIX_HOOK_BOUNDED_STDIN: PosixHookStdinReader = { + reader: 'orca_read_hook_to_eof', + prelude: [ + 'orca_read_hook_to_eof() (', + ' exec 3<&0', + ' if command -p cat /dev/null 2>&1; then', + ' command -p cat <&3 2>/dev/null &', + ' else', + ' command cat <&3 2>/dev/null &', + ' fi', + ' orca_reader=$!', + ' (', + ` command -p sleep ${POSIX_HOOK_EOF_TIMEOUT_SECONDS} &`, + ' orca_sleeper=$!', + ' trap \'kill "$orca_sleeper" 2>/dev/null; wait "$orca_sleeper" 2>/dev/null; exit 0\' TERM', + ' wait "$orca_sleeper"', + ' kill "$orca_reader" 2>/dev/null', + ' ) >/dev/null 2>&1 &', + ' orca_watchdog=$!', + ' wait "$orca_reader" 2>/dev/null', + ' orca_read_status=$?', + ' kill "$orca_watchdog" 2>/dev/null', + ' wait "$orca_watchdog" 2>/dev/null', + ' exit "$orca_read_status"', + ')' + ] +} diff --git a/src/main/antigravity/hook-script.ts b/src/main/antigravity/hook-script.ts index fb27ad63494..fa1151437fc 100644 --- a/src/main/antigravity/hook-script.ts +++ b/src/main/antigravity/hook-script.ts @@ -1,5 +1,6 @@ +import { POSIX_HOOK_BOUNDED_STDIN } from '../agent-hooks/posix-hook-bounded-stdin' import { - buildPosixHookPayloadCapture, + createStrictPosixHookJsonStdinReader, buildPosixHookSpoolLines, buildWindowsHookEnvironmentGuardLines, buildWindowsHookStdinDrainEpilogue, @@ -8,6 +9,8 @@ import { import { buildWindowsAgentHookPostCommand } from '../agent-hooks/installer-utils' import { ANTIGRAVITY_PRE_TOOL_USE_DECISION } from './hook-events' +const ANTIGRAVITY_POSIX_STDIN = createStrictPosixHookJsonStdinReader(POSIX_HOOK_BOUNDED_STDIN) + // Why (#15117): PowerShell cost ~300ms of startup per event, which is what made the console // the agent allocates for each hook last long enough to see. const WINDOWS_ANTIGRAVITY_HOOK_POST_COMMAND = buildWindowsAgentHookPostCommand('antigravity', [ @@ -55,7 +58,12 @@ export function getManagedScript(target: 'local' | 'posix' = 'local'): string { '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'), + ...ANTIGRAVITY_POSIX_STDIN.prelude, + `if payload=$(${ANTIGRAVITY_POSIX_STDIN.reader}); then`, + ` [ -n "$payload" ] || payload='{}'`, + 'else', + ' exit 0', + 'fi', ...buildPosixHookSpoolLines('antigravity', 'ORCA_ANTIGRAVITY_EVENT'), 'if [ -n "$ORCA_AGENT_HOOK_ENDPOINT" ] && [ -r "$ORCA_AGENT_HOOK_ENDPOINT" ]; then', ' . "$ORCA_AGENT_HOOK_ENDPOINT" 2>/dev/null || :', diff --git a/src/main/antigravity/hook-service.test.ts b/src/main/antigravity/hook-service.test.ts index 6adcdc2448f..a0deb253188 100644 --- a/src/main/antigravity/hook-service.test.ts +++ b/src/main/antigravity/hook-service.test.ts @@ -17,7 +17,6 @@ vi.mock('os', async () => { }) import { AntigravityHookService } from './hook-service' -import { POSIX_HOOK_STDIN_READER } from '../agent-hooks/hook-stdin-contract' import { createManagedCommandMatcher } from '../agent-hooks/installer-utils' const ANTIGRAVITY_SCRIPT_FILE_NAME = @@ -149,7 +148,7 @@ describe('AntigravityHookService', () => { expect(script).toContain('setlocal DisableDelayedExpansion') } else { expect(script).toContain('hook_event_name=${ORCA_ANTIGRAVITY_EVENT}') - expect(script).toContain(`payload=$(${POSIX_HOOK_STDIN_READER})`) + expect(script).toContain('if payload=$(') expect(script).toContain("payload='{}'") expect(script).not.toContain('if [ -z "$payload" ]; then\n exit 0\nfi') // Why: payload is piped to curl via stdin (`payload@-`) so it never lands diff --git a/src/main/antigravity/posix-hook-payload-delivery.test.ts b/src/main/antigravity/posix-hook-payload-delivery.test.ts new file mode 100644 index 00000000000..75b3685f182 --- /dev/null +++ b/src/main/antigravity/posix-hook-payload-delivery.test.ts @@ -0,0 +1,197 @@ +import { createServer } from 'node:http' +import { mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { spawnProcess } from '../../shared/child-process/run-process' +import { getManagedScript } from './hook-script' + +const PAYLOAD = JSON.stringify({ hook_event_name: 'Stop', result: 'café 日本語 😀'.repeat(4000) }) + +async function runHook(options: { + chunks: string[] + close: boolean + gapMs?: number + noPython?: boolean + offline?: boolean +}) { + const root = mkdtempSync(join(tmpdir(), 'orca-agy-stdin-')) + const posts: string[] = [] + const server = createServer((req, res) => { + let body = '' + req.setEncoding('utf8') + req.on('data', (part) => { + body += part + }) + req.on('end', () => { + posts.push(new URLSearchParams(body).get('payload') ?? '') + res.end('{}') + }) + }) + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + const address = server.address() + if (!address || typeof address === 'string') { + throw new Error('listener unavailable') + } + const endpoint = join(root, 'endpoint.sh') + writeFileSync(endpoint, '# isolated endpoint\n') + const env = { + ...process.env, + ORCA_ANTIGRAVITY_EVENT: 'Stop', + ORCA_PANE_KEY: 'probe-pane', + ORCA_AGENT_HOOK_ENDPOINT: endpoint, + ORCA_AGENT_HOOK_PORT: options.offline ? '' : String(address.port), + ORCA_AGENT_HOOK_TOKEN: options.offline ? '' : 'isolated-token' + } + let script = getManagedScript('posix') + if (options.noPython) { + // Simulate unavailable interpreters without changing the host's binaries or PATH. + script = script.replaceAll('command -p python', 'command -p orca_missing_python') + } + const child = spawnProcess({ program: '/bin/sh', args: ['-c', script], env, detached: true }) + const started = Date.now() + let stdout = '' + let stderr = '' + let timedOut = false + child.stdout.on('data', (part) => { + stdout += part + }) + child.stderr.on('data', (part) => { + stderr += part + }) + child.stdin.on('error', () => {}) + const timeout = setTimeout(() => { + timedOut = true + if (child.pid) { + process.kill(-child.pid, 'SIGKILL') + } + }, 8000) + try { + const exited = new Promise((resolve, reject) => { + child.once('error', reject) + child.once('close', resolve) + }) + for (const [index, chunk] of options.chunks.entries()) { + if (index > 0) { + await new Promise((resolve) => setTimeout(resolve, options.gapMs ?? 0)) + } + child.stdin.write(chunk) + } + if (options.close) { + child.stdin.end() + } + const code = await exited + let descendantsRemain = false + if (child.pid) { + try { + process.kill(-child.pid, 0) + descendantsRemain = true + process.kill(-child.pid, 'SIGKILL') + } catch { + // The hook's process group is gone. + } + } + const spoolDir = join(root, 'spool') + let spooled: string[] = [] + try { + spooled = readdirSync(spoolDir).map((name) => readFileSync(join(spoolDir, name), 'utf8')) + } catch { + /* no spool */ + } + return { + code, + timedOut, + stdout, + stderr, + posts, + spooled, + descendantsRemain, + elapsed: Date.now() - started + } + } finally { + clearTimeout(timeout) + child.stdin.destroy() + await new Promise((resolve) => server.close(() => resolve())) + rmSync(root, { recursive: true, force: true }) + } +} + +describe.skipIf(process.platform === 'win32')('Antigravity POSIX payload capture', () => { + it('posts complete JSON without requiring EOF', async () => { + const result = await runHook({ chunks: [PAYLOAD], close: false }) + expect(result).toMatchObject({ + code: 0, + timedOut: false, + descendantsRemain: false, + stderr: '', + posts: [PAYLOAD] + }) + }, 10000) + + it.each([false, true])( + 'preserves delayed chunks (missing Python: %s)', + async (noPython) => { + const result = await runHook({ + chunks: [PAYLOAD.slice(0, 25), PAYLOAD.slice(25)], + gapMs: 1500, + close: true, + noPython + }) + expect(result).toMatchObject({ + code: 0, + timedOut: false, + descendantsRemain: false, + stderr: '', + posts: [PAYLOAD] + }) + }, + 10000 + ) + + it.each([false, true])( + 'rejects incomplete input rather than posting it (missing Python: %s)', + async (noPython) => { + const result = await runHook({ chunks: ['{"unfinished":'], close: false, noPython }) + expect(result).toMatchObject({ + code: 0, + timedOut: false, + descendantsRemain: false, + stderr: '', + posts: [], + spooled: [] + }) + expect(result.stdout.trim()).toBe('{"decision":""}') + }, + 10000 + ) + + it.each([false, true])( + 'exits on abandoned stdin (missing Python: %s)', + async (noPython) => { + const result = await runHook({ chunks: [], close: false, noPython }) + expect(result).toMatchObject({ + code: 0, + timedOut: false, + descendantsRemain: false, + stderr: '' + }) + }, + 10000 + ) + + it('preserves offline event spooling', async () => { + const result = await runHook({ chunks: [PAYLOAD], close: true, offline: true }) + expect(result).toMatchObject({ + code: 0, + timedOut: false, + descendantsRemain: false, + stderr: '', + posts: [] + }) + expect(result.spooled).toHaveLength(1) + expect(JSON.parse(result.spooled[0])).toMatchObject({ + payload: JSON.parse(PAYLOAD), + hookEventName: 'Stop' + }) + }, 10000) +})