Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 24 additions & 10 deletions src/main/agent-hooks/remote-hook-service-installers.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { tokenizeCommandLine } from '../../shared/agent-command-line-entrypoint'
import { qoderHookService } from '../qoder/hook-service'
import { describe, expect, it, vi } from 'vitest'
import { parse as parseJsonc } from 'jsonc-parser'
Expand Down Expand Up @@ -347,23 +348,36 @@ describe('remote hook service installers', () => {
}
for (const eventName of ['PreInvocation', 'PostInvocation', 'Stop']) {
const command = antigravityConfig['orca-status'][eventName]?.[0]?.command
expect(command).toContain('/home/dev/.orca/agent-hooks/antigravity-hook.sh')
expect(command).toContain(`ORCA_ANTIGRAVITY_EVENT='${eventName}'`)
expect(tokenizeCommandLine(command ?? '').slice(0, 2)).toEqual(['/bin/sh', '-c'])
expect(tokenizeCommandLine(command ?? '')[2]).toContain(
'/home/dev/.orca/agent-hooks/antigravity-hook.sh'
)
expect(tokenizeCommandLine(command ?? '')[2]).toContain(
`ORCA_ANTIGRAVITY_EVENT='${eventName}'`
)
}
for (const eventName of ['PreToolUse', 'PostToolUse']) {
const definition = antigravityConfig['orca-status'][eventName]?.[0]
const command = definition?.hooks?.[0]?.command
expect(definition?.matcher).toBe('*')
expect(command).toContain('/home/dev/.orca/agent-hooks/antigravity-hook.sh')
expect(command).toContain(`ORCA_ANTIGRAVITY_EVENT='${eventName}'`)
expect(tokenizeCommandLine(command ?? '')[2]).toContain(
'/home/dev/.orca/agent-hooks/antigravity-hook.sh'
)
expect(tokenizeCommandLine(command ?? '')[2]).toContain(
`ORCA_ANTIGRAVITY_EVENT='${eventName}'`
)
}
// Why: #2426 was an SSH report — a remote host missing the script must still answer the gate, not deny every tool.
expect(antigravityConfig['orca-status'].PreToolUse[0].hooks?.[0]?.command).toContain(
`printf '%s\\n' '{"decision":"ask"}'`
)
expect(antigravityConfig['orca-status'].PostToolUse[0].hooks?.[0]?.command).not.toContain(
'{"decision"'
)
expect(
tokenizeCommandLine(
antigravityConfig['orca-status'].PreToolUse[0].hooks?.[0]?.command ?? ''
)[2]
).toContain(`printf '%s\\n' '{"decision":"ask"}'`)
expect(
tokenizeCommandLine(
antigravityConfig['orca-status'].PostToolUse[0].hooks?.[0]?.command ?? ''
)[2]
).not.toContain('{"decision"')

const ampPlugin = amp.fs.files.get('/home/dev/.config/amp/plugins/orca-agent-status.ts')
expect(ampPlugin).toContain('/hook/amp')
Expand Down
3 changes: 2 additions & 1 deletion src/main/antigravity/hook-script.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
buildPosixHookPayloadCapture,
POSIX_HOOK_JSON_STDIN,
buildPosixHookSpoolLines,
buildWindowsHookEnvironmentGuardLines,
buildWindowsHookStdinDrainEpilogue,
Expand Down Expand Up @@ -55,7 +56,7 @@ 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'),
...buildPosixHookPayloadCapture('empty-object', POSIX_HOOK_JSON_STDIN),
...buildPosixHookSpoolLines('antigravity', 'ORCA_ANTIGRAVITY_EVENT'),
'if [ -n "$ORCA_AGENT_HOOK_ENDPOINT" ] && [ -r "$ORCA_AGENT_HOOK_ENDPOINT" ]; then',
' . "$ORCA_AGENT_HOOK_ENDPOINT" 2>/dev/null || :',
Expand Down
13 changes: 8 additions & 5 deletions src/main/antigravity/hook-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ vi.mock('os', async () => {
}
})

import { tokenizeCommandLine } from '../../shared/agent-command-line-entrypoint'
import { AntigravityHookService } from './hook-service'
import { POSIX_HOOK_STDIN_READER } from '../agent-hooks/hook-stdin-contract'
import { POSIX_HOOK_JSON_STDIN_READER } from '../agent-hooks/hook-stdin-contract'
import { createManagedCommandMatcher } from '../agent-hooks/installer-utils'

const ANTIGRAVITY_SCRIPT_FILE_NAME =
Expand Down Expand Up @@ -86,10 +87,12 @@ describe('AntigravityHookService', () => {
if (process.platform === 'win32') {
expect(config['orca-status'].PreInvocation[0].command).not.toContain('ORCA_ANTIGRAVITY_EVENT')
} else {
expect(config['orca-status'].PreInvocation[0].command).toContain(
"ORCA_ANTIGRAVITY_EVENT='PreInvocation'"
expect(
tokenizeCommandLine(config['orca-status'].PreInvocation[0].command ?? '')[2]
).toContain("ORCA_ANTIGRAVITY_EVENT='PreInvocation'")
expect(tokenizeCommandLine(config['orca-status'].Stop[0].command ?? '')[2]).toContain(
"ORCA_ANTIGRAVITY_EVENT='Stop'"
)
expect(config['orca-status'].Stop[0].command).toContain("ORCA_ANTIGRAVITY_EVENT='Stop'")
}

const script = readFileSync(
Expand All @@ -106,7 +109,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(`payload=$(${POSIX_HOOK_JSON_STDIN_READER})`)
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
Expand Down
5 changes: 4 additions & 1 deletion src/main/antigravity/hook-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
ANTIGRAVITY_PRE_TOOL_USE_DECISION,
type AntigravityEvent
} from './hook-events'
import { quotePosixShellString } from '../agent-hooks/posix-hook-command'
import { getManagedScript, getWindowsWrapperScript } from './hook-script'
import {
buildInstalledConfig,
Expand Down Expand Up @@ -51,12 +52,14 @@ function getWindowsWrapperScriptPath(event: AntigravityEvent): string {
}

function getPosixManagedCommand(scriptPath: string, event: AntigravityEvent): string {
return wrapPosixHookCommand(
const command = wrapPosixHookCommand(
scriptPath,
{ ORCA_ANTIGRAVITY_EVENT: event.eventName },
// Why: a missing managed script must not brick tools; the guard answers PreToolUse itself instead of staying silent.
event.eventName === 'PreToolUse' ? { fallbackStdout: ANTIGRAVITY_PRE_TOOL_USE_DECISION } : {}
)
// ACP hosts tokenize the command into argv; the shell must be the executable, not `if`.
return `/bin/sh -c ${quotePosixShellString(command)}`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The contract test that scans every managed command for bare $VAR references still builds Antigravity's command from the unwrapped wrapPosixHookCommand(...), so it no longer scans the string this PR actually installs. Low stakes today — neither form contains $ — but the fixture has silently drifted from production.

Technical details
# Antigravity contract fixture no longer mirrors the installer

## Affected sites
- `src/main/agent-hooks/managed-hook-command-contract.test.ts:56-64` — `antigravityPosixCommands` returns `wrapPosixHookCommand(...)` directly, without the `/bin/sh -c` wrapper added here.
- `src/main/antigravity/hook-service.ts:54-62` — production now returns `/bin/sh -c ${quotePosixShellString(command)}`.

## Required outcome
- The `antigravity` builder in `buildersByAgent` should scan the same string the installer emits, so the no-bare-variable contract applies to the real command.

## Suggested approach (optional)
- Export `getPosixManagedCommand` (or a small `wrapAntigravityPosixHookCommand`) from `hook-service.ts` and call it from the test; inlining the one-line `/bin/sh -c ${quotePosixShellString(...)}` wrapper in the fixture also works.

}

function getManagedCommand(scriptPath: string, event: AntigravityEvent): string {
Expand Down
136 changes: 136 additions & 0 deletions src/main/antigravity/posix-hook-invocation.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
import { afterEach, describe, expect, it, vi } from 'vitest'
import { mkdtempSync, readFileSync, rmSync } from 'node:fs'
import { tmpdir } from 'node:os'
import type * as OsModule from 'node:os'
import { join } from 'node:path'
import { createServer } from 'node:http'
import { z } from 'zod'
import { tokenizeCommandLine } from '../../shared/agent-command-line-entrypoint'
import { spawnProcess } from '../../shared/child-process/run-process'
import { AntigravityHookService } from './hook-service'

const { homeMock } = vi.hoisted(() => ({ homeMock: vi.fn<() => string>() }))
vi.mock('node:os', async (importOriginal) => ({
...(await importOriginal<typeof OsModule>()),
homedir: homeMock
}))

const hooksSchema = z.object({
'orca-status': z.record(
z.string(),
z.array(
z.object({
command: z.string().optional(),
hooks: z.array(z.object({ command: z.string() })).optional()
})
)
)
})

function installedCommand(home: string, event: string): string {
homeMock.mockReturnValue(home)
expect(new AntigravityHookService().install().state).toBe('installed')
const config = hooksSchema.parse(
JSON.parse(readFileSync(join(home, '.gemini', 'config', 'hooks.json'), 'utf8'))
)
const hook = config['orca-status'][event]?.[0]
const command = hook?.command ?? hook?.hooks?.[0]?.command
if (!command) {
throw new Error(`Missing ${event} hook`)
}
return command
}

function invoke(command: string, input: string, env: NodeJS.ProcessEnv) {
const [program, ...args] = tokenizeCommandLine(command)
if (!program) {
throw new Error('Missing executable')
}
expect(program).toBe('/bin/sh')
return new Promise<{ code: number | null; stdout: string; elapsedMs: number }>(
(resolve, reject) => {
const start = Date.now()
const child = spawnProcess({ program, args, env })
let stdout = ''
child.stdout.on('data', (chunk: Buffer) => {
stdout += chunk.toString()
})
child.stdin.on('error', () => {})
const timer = setTimeout(() => child.kill('SIGKILL'), 8000)
child.on('error', reject)
child.on('close', (code) => {
clearTimeout(timer)
resolve({ code, stdout: stdout.trim(), elapsedMs: Date.now() - start })
})
// agy may leave stdin open after sending one JSON value.
if (input) {
child.stdin.write(Buffer.from(input))
}
}
)
}

let home = ''
afterEach(() => {
if (home) {
rmSync(home, { recursive: true, force: true })
}
})

describe.skipIf(process.platform === 'win32')('Antigravity POSIX hook executable contract', () => {
it('delivers UTF-8 JSON with open stdin through a directly spawned command', async () => {
home = mkdtempSync(join(tmpdir(), "orca-agy-hook-'quoted-"))
const payload = JSON.stringify({
tool_name: 'run_command',
tool_input: { command: 'echo café 日本語 😀' }
})
const posts: URLSearchParams[] = []
const server = createServer((req, res) => {
let body = ''
req.setEncoding('utf8')
req.on('data', (chunk: string) => {
body += chunk
})
req.on('end', () => {
posts.push(new URLSearchParams(body))
res.end('{}')
})
})
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve))
const address = server.address()
if (typeof address !== 'object' || address === null) {
throw new Error('Missing listener port')
}
try {
const result = await invoke(installedCommand(home, 'PreToolUse'), payload, {
...process.env,
ORCA_AGENT_HOOK_ENDPOINT: '',
ORCA_AGENT_HOOK_PORT: String(address.port),
ORCA_AGENT_HOOK_TOKEN: 'test-only-token',
ORCA_PANE_KEY: 'test-pane'
})
expect(result.code).toBe(0)
expect(result.stdout).toBe('{"decision":"ask"}')
expect(result.elapsedMs).toBeLessThan(8000)
expect(posts).toHaveLength(1)
expect(posts[0]?.get('payload')).toBe(payload)
expect(posts[0]?.get('hook_event_name')).toBe('PreToolUse')
} finally {
server.close()
}
}, 10000)

it('returns after a bounded wait when stdin has no bytes and remains open', async () => {
home = mkdtempSync(join(tmpdir(), 'orca-agy-empty-hook-'))
const result = await invoke(installedCommand(home, 'PreInvocation'), '', {
...process.env,
ORCA_AGENT_HOOK_ENDPOINT: '',
ORCA_AGENT_HOOK_PORT: '',
ORCA_AGENT_HOOK_TOKEN: '',
ORCA_PANE_KEY: ''
})
expect(result.code).toBe(0)
expect(result.stdout).toBe('{}')
expect(result.elapsedMs).toBeLessThan(8000)
}, 10000)
})
Loading