mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 08:03:39 +00:00
test(agent-hooks): address review — abandon stdin in the guard test, cover blank payloads
- The missing-pane case closed stdin, so a hook that read before the env guard would still pass on an EOF the real caller never sends. Abandon the pipe instead (#11549). - Cover whitespace-only payloads, which the trim() in the ingest already accepts. - Trim comments to the repo's concise-comment rule; the drift history lives in the PR.
This commit is contained in:
@@ -1,14 +1,7 @@
|
||||
// Why (#15117): Antigravity spent three months posting hook status through Windows
|
||||
// PowerShell 5.1 after every other agent had moved to curl.exe. Nothing caught it, because
|
||||
// the drift was invisible to per-agent tests: the shared builder took only `(source)` and had
|
||||
// no way to add the extra form field Antigravity needs, so it forked a private copy of the
|
||||
// then-correct PowerShell command (#2100). When the shared builder switched to curl (#6402),
|
||||
// every agent that *called* it was fixed for free and the private copy silently was not.
|
||||
//
|
||||
// This suite asserts the invariant that drift violated, across every agent at once: a managed
|
||||
// Windows .cmd hook posts through curl.exe and spawns no interpreter. It generates the scripts
|
||||
// under a mocked win32 platform rather than executing them, so it guards the POSIX CI legs too
|
||||
// — the drift would otherwise only be visible on a Windows runner.
|
||||
// Why (#15117): an agent holding a private copy of the shared post command missed the move to
|
||||
// curl for three months, invisible to per-agent tests. Assert the invariant across every agent
|
||||
// at once: a managed Windows .cmd hook posts through curl.exe and spawns no interpreter.
|
||||
// Generated under a mocked win32 platform, not executed, so the POSIX CI legs guard it too.
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { mkdtempSync, readFileSync, readdirSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
@@ -7,14 +7,10 @@ import {
|
||||
import { buildWindowsAgentHookPostCommand } from '../agent-hooks/installer-utils'
|
||||
import { ANTIGRAVITY_PRE_TOOL_USE_DECISION } from './hook-events'
|
||||
|
||||
// Why (#15117): Antigravity was the last agent posting hook status through Windows
|
||||
// PowerShell 5.1. Its ~300ms cold start per event is what made the console the agent
|
||||
// allocates for each hook visible; curl.exe (Win10 1803+) finishes in ~10ms and, unlike
|
||||
// PowerShell, does not recode the UTF-8 payload through the console code page.
|
||||
// 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', [
|
||||
// Why: Antigravity alone takes its event name from the wrapper's env rather than the piped
|
||||
// payload. Before `extraFormLines` existed this forced a private copy of the shared command,
|
||||
// which is how this script missed the fleet-wide move to curl for three months (#15117).
|
||||
// Why: Antigravity alone takes its event name from the wrapper's env, not the piped payload.
|
||||
' --data-urlencode "hook_event_name=%ORCA_ANTIGRAVITY_EVENT%" ^'
|
||||
])
|
||||
|
||||
@@ -22,9 +18,8 @@ export function getManagedScript(target: 'local' | 'posix' = 'local'): string {
|
||||
if (target === 'local' && process.platform === 'win32') {
|
||||
return [
|
||||
'@echo off',
|
||||
// Why (#9358/#9941): a bare `setlocal` inherits delayed expansion from the caller, and
|
||||
// every `!` in a percent-expanded value on the curl line is then eaten as a delayed
|
||||
// reference — silently mangling paneKey and dropping worktreeId. `!` is legal in a path.
|
||||
// Why (#9358/#9941): inherited delayed expansion eats `!` out of the percent-expanded
|
||||
// curl args, mangling paneKey and dropping worktreeId. `!` is legal in a Windows path.
|
||||
'setlocal DisableDelayedExpansion',
|
||||
'if /I "%ORCA_ANTIGRAVITY_EVENT%"=="Stop" (',
|
||||
' echo {"decision":""}',
|
||||
|
||||
@@ -98,8 +98,6 @@ describe('AntigravityHookService', () => {
|
||||
)
|
||||
expect(script).toContain('/hook/antigravity')
|
||||
if (process.platform === 'win32') {
|
||||
// Why (#15117): PowerShell's ~300ms cold start per hook event is what made the
|
||||
// per-event console visible; curl.exe keeps the flash imperceptible.
|
||||
expect(script).not.toContain('powershell.exe')
|
||||
expect(script).toContain('%SystemRoot%\\System32\\curl.exe')
|
||||
expect(script).toContain('hook_event_name=%ORCA_ANTIGRAVITY_EVENT%')
|
||||
|
||||
@@ -1,10 +1,5 @@
|
||||
// Why (#15117): Antigravity was the last agent whose Windows hook posted status through
|
||||
// Windows PowerShell 5.1. The agent spawns each hook itself, so Orca cannot suppress the
|
||||
// console it allocates — but PowerShell's ~300ms cold start is what made that console
|
||||
// linger long enough to be seen, roughly every 2-6s for a whole session. Moving to
|
||||
// curl.exe removes the interpreter from the hot path. Shape assertions alone would not
|
||||
// catch a curl line that posts nothing, so this suite pipes a real payload through the
|
||||
// installed wrappers and follows it to a listener.
|
||||
// Why (#15117): shape assertions cannot catch a curl line that posts nothing, so this suite
|
||||
// pipes a real payload through the installed wrappers and follows it to a live listener.
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { spawn } from 'node:child_process'
|
||||
import { createServer, type Server } from 'node:http'
|
||||
@@ -99,7 +94,9 @@ type HookRun = { exitCode: number | null; stdout: string; stderr: string; timedO
|
||||
function runWrapper(
|
||||
wrapperPath: string,
|
||||
env: NodeJS.ProcessEnv,
|
||||
stdinPayload: string = PAYLOAD
|
||||
// Why: `null` abandons stdin instead of closing it — the shape a caller outside an Orca
|
||||
// pane produces, and the only way to prove the env guard exits before reading (#11549).
|
||||
stdinPayload: string | null = PAYLOAD
|
||||
): Promise<HookRun> {
|
||||
return new Promise((resolve, reject) => {
|
||||
// Why: mirror how Antigravity spawns the hook — `cmd /c <bare .cmd path>`, the exact
|
||||
@@ -131,7 +128,12 @@ function runWrapper(
|
||||
// Why: curl exits once the POST is written; give the listener a beat to finish reading it.
|
||||
setTimeout(() => resolve({ exitCode, stdout, stderr, timedOut }), 250)
|
||||
})
|
||||
child.stdin.end(Buffer.from(stdinPayload, 'utf8'))
|
||||
// Why: an abandoned pipe raises EPIPE once the child exits; swallow it so the run still
|
||||
// resolves on the child's own terms.
|
||||
child.stdin.on('error', () => {})
|
||||
if (stdinPayload !== null) {
|
||||
child.stdin.end(Buffer.from(stdinPayload, 'utf8'))
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -273,7 +275,8 @@ describe.skipIf(process.platform !== 'win32')('Antigravity Windows hook payload
|
||||
// so the guard must exit before the read — otherwise the console lingers indefinitely.
|
||||
const result = await runWrapper(
|
||||
join(home, '.orca', 'agent-hooks', 'antigravity-pre-tool-use.cmd'),
|
||||
hookEnvironment({ USERPROFILE: home, HOME: home })
|
||||
hookEnvironment({ USERPROFILE: home, HOME: home }),
|
||||
null
|
||||
)
|
||||
|
||||
expect(result.timedOut).toBe(false)
|
||||
|
||||
@@ -26,20 +26,25 @@ describe('shared agent-hook-listener', () => {
|
||||
// Why (#15117): the Windows script posts stdin verbatim through curl, so an event that
|
||||
// fires without a payload arrives as `payload=""` rather than the `{}` the POSIX script
|
||||
// substitutes. Both transports must still produce the status transition.
|
||||
it('accepts an Antigravity event whose payload arrived empty', () => {
|
||||
const started = normalizeHookPayload(
|
||||
state,
|
||||
'antigravity',
|
||||
{
|
||||
paneKey: PANE_KEY,
|
||||
tabId: 'tab-1',
|
||||
worktreeId: 'wt',
|
||||
hook_event_name: 'PreInvocation',
|
||||
payload: ''
|
||||
},
|
||||
'production'
|
||||
)
|
||||
expect(started?.payload).toMatchObject({ state: 'working', agentType: 'antigravity' })
|
||||
it('accepts an Antigravity event whose payload is empty or whitespace-only', () => {
|
||||
for (const payload of ['', ' ', '\r\n']) {
|
||||
const started = normalizeHookPayload(
|
||||
state,
|
||||
'antigravity',
|
||||
{
|
||||
paneKey: PANE_KEY,
|
||||
tabId: 'tab-1',
|
||||
worktreeId: 'wt',
|
||||
hook_event_name: 'PreInvocation',
|
||||
payload
|
||||
},
|
||||
'production'
|
||||
)
|
||||
expect(started?.payload, `payload ${JSON.stringify(payload)}`).toMatchObject({
|
||||
state: 'working',
|
||||
agentType: 'antigravity'
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
// Why (#15117): this is the shape Windows actually produces — curl omits the form field
|
||||
|
||||
Reference in New Issue
Block a user