Repository navigation
fix(antigravity): bound POSIX hook stdin so agy cannot hang loading - #15821
brennanb2025 wants to merge 2 commits into
Conversation
Antigravity can omit stdin or leave the pipe open until the hook exits. Unbound cat never reached curl, so the CLI waited on the hook and the sidebar stayed working. Capture through a watchdog-killed cat after the Orca env guard, without changing the shared POSIX stdin contract.
📝 WalkthroughWalkthroughThe Antigravity POSIX hook now validates its environment before reading stdin. It captures stdin through a background reader with a one-second watchdog, terminates the reader, and uses 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c633c48-630f-47d1-8f25-8271e14f692c
📒 Files selected for processing (5)
config/scripts/verify-agent-hook-stdin-lifecycle.mjssrc/main/agent-hooks/managed-hook-stdin-lifecycle.test.tssrc/main/antigravity/hook-script.tssrc/main/antigravity/hook-service.test.tssrc/main/antigravity/posix-hook-payload-delivery.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
| it('exits when a payload line is written but stdin is never closed', async () => { | ||
| const { scriptPath, port, posts } = await installAndListen() | ||
| const result = await runScript( | ||
| scriptPath, | ||
| { ...orcaEnv(port), ORCA_ANTIGRAVITY_EVENT: 'Stop' }, | ||
| ABANDONED_LINE_PAYLOAD, | ||
| false | ||
| ) | ||
|
|
||
| expect(result.timedOut).toBe(false) | ||
| expect(result.exitCode).toBe(0) | ||
| expect(result.elapsedMs).toBeLessThan(EXIT_BUDGET_MS) | ||
| expect(result.stdout.trim()).toBe('{"decision":""}') | ||
| expect(posts.some((post) => post.hookEventName === 'Stop' && post.token === HOOK_TOKEN)).toBe( | ||
| true | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Assert the payload for the open-stdin case.
This test verifies only the event name and token. If the watchdog terminates the reader before it consumes ABANDONED_LINE_PAYLOAD, the hook can post {} and this test still passes.
Assert one post and compare its payload to ABANDONED_LINE_PAYLOAD.trimEnd().
🧰 Tools
🪛 GitHub Check: tests node 24 4/16
[failure] 224-224: src/main/antigravity/posix-hook-payload-delivery.test.ts > Antigravity POSIX hook payload delivery > exits when a payload line is written but stdin is never closed
AssertionError: expected true to be false // Object.is equality
- Expected
- Received
- false
- true
❯ src/main/antigravity/posix-hook-payload-delivery.test.ts:224:29
🪛 GitHub Check: tests node 26 4/16
[failure] 224-224: src/main/antigravity/posix-hook-payload-delivery.test.ts > Antigravity POSIX hook payload delivery > exits when a payload line is written but stdin is never closed
AssertionError: expected true to be false // Object.is equality
- Expected
- Received
- false
- true
❯ src/main/antigravity/posix-hook-payload-delivery.test.ts:224:29
|
Codex revision complete. What changed:
What stayed and why:
Non-vacuity proof:
Verification:
|
|
I ran the generated POSIX scripts from the current investigation branch and this PR (
The slow-write case is a 54-byte payload, not a volume stress test: it checks scheduling/transport delay within the hook's existing 10-second host budget. Killing The spool difference reflects newer code on the target branch: For this comparison I preserved the proposed shell script and supplied the same The shared |
|
The revised implementation is now in draft #21634, stacked on #21628. It reuses JSON-completion reading, bounds the no-interpreter fallback, rejects timed-out capture instead of forwarding a prefix, and preserves current offline spooling. Eight real-process tests pass, including delayed Unicode input, abandoned/open stdin, missing interpreters, offline storage, and descendant cleanup. Before/after loopback evidence is attached to the draft. The installed agy still returns generation HTTP 401, so this verifies the generated hook processes, not a successful live agent turn. Linux/SSH and actual Windows validation remain pending. |
|
Complete-JSON reading landed in #24596. Any remaining hang in its fallback should be fixed in the current shared reader, so the older watchdog implementation can be retired while that work remains tracked. |
2 participants
STA-4700
Problem
Antigravity (
agy) waits for each managed hook process to exit before continuing its execution loop. The POSIXantigravity-hook.shcaptured stdin with unboundedcat. When the agent omits stdin or leaves the pipe open until the hook exits,catnever sees EOF, the hook never reachescurl/exit 0, and both the CLI and Orca’s sidebar stay in a loading/working state.Reproduction
Spawn the generated POSIX script with Orca env present and leave stdin open (do not send EOF). Pre-fix: the process hung until killed (
timedOut: trueat ~4s). Closed-and-written stdin still completed in ~0.3s and posted the payload.A live
agy --printfrom a throwaway HOME hit Google OAuth, so causation from the live TUI is inferred from this hang plus the documented no-stdin / abandoned-pipe cases (#15117, #11549). The hook-process hang itself was observed on the installed script.Fix
In
src/main/antigravity/hook-script.tsonly (sharedhook-stdin-contract.tsunchanged):askon PreToolUse, empty decision on Stop).cat(fd 3 so a backgroundcatdoes not inherit/dev/null;command -p sleepso emptyPATHcannot fire the kill immediately).printf | curl payload@-.Tests
Acceptance in
posix-hook-payload-delivery.test.tsagainst a real loopback listener:timedOut: truecatrestored)timedOut: trueagainAlso: env-missing abandoned stdin exits in <500ms; lifecycle suite still covers large payloads and empty
PATH.Follow-ups (not this PR)
curl --data-urlencode payload@-is not bounded by--max-time(curl reads@-to EOF before the transfer timer).#11549only covers env-missing.wrapPosixHookCommandmissing-script drain is still unboundedcat.Verified
pnpm vitest run --config config/vitest.config.tson the antigravity POSIX delivery, hook-service, Windows delivery (skipped on macOS), and managed-hook stdin lifecycle suitespnpm typechecknode config/scripts/check-changed-code-quality.mjsnode config/scripts/check-react-doctor-changed.mjsCould not verify
payload@-hang on this machine