Repository navigation
Host Antigravity CLI agents: posture, queued input, and the reviewer split - #482
Merged
Merged
Conversation
Lifts the interactive-launch refusal and passes --dangerously-skip-permissions on the hosted arm only. The refusal claimed an inherited HOME lets agy's capture hooks spawn a watcher that holds the turn's stdout open, wedging every turn. Measured: four piped `agy -p ... --output-format stream-json | cat` runs under a real HOME, on a kcap predating the descriptor fix, all reached EOF in 6-16s, with the hook log and the real-HOME transcript present as positive controls. The wedge does not reproduce. What those runs did show is that each was also recorded by the hook lane as its own watcher session, while this runtime is already the transcript source (it implements IAcpTranscriptSource and the factory returns it as Transcript) - so an inherited HOME would duplicate capture rather than add it. The per-launch isolated HOME is therefore kept for hosted launches too. A hosted agent exists to do work: without the flag, agy soft-denies shell and out-of-workspace operations while still exiting 0, so it merely looks broken. Measured on agy 1.1.10, that flag is the read boundary and the daemon-owned worktree is not - with it, an absolute out-of-workspace view_file succeeds; without it the same read is refused with a typed tool_info.error. The comments say so rather than claiming worktree containment. The precedent cited is Claude (--permission-mode bypassPermissions, single-axis, already shipped) and deliberately not Codex, whose posture is two axes and still sits on a sandbox with prompting off. A reviewer reads an owned worktree and nothing else, so it keeps the soft-deny. Both directions are pinned as whole argv vectors in one test. The vendor ladder (consent, platform, recorded build minimum) still applies to every launch including an interactive one, since the containment it protects is the isolated home both shapes rely on; a test pins that too.
Task 3 lifted the interactive-launch refusal, but StartAsync still ran the whole reviewer ladder for both shapes. A hosted agy launch on a daemon that had never set KCAP_ANTIGRAVITY_UNATTENDED_REVIEWER therefore failed with antigravity_unattended_reviewer_disabled - a review complaint about a launch that is not a review, and a consent gate the owner explicitly declined for hosted agy (ship on by default, like Claude/Codex/Cursor). Consent is reviewer-only because its own justification is cross-principal: an unattended reviewer runs under the daemon user's authority and returns what it read to whoever requested the review, who need not be the operator. A hosted launch has no counterpart - the server's DaemonRegistry is keyed (TeamId, OwnerUserId, Name), GetDaemonsForRepoAsync takes the caller's own normalized user id, and the launch hub resolves the daemon with that same id, so the launcher IS the daemon's owner (verified in kcap-server, read-only). Platform, binary presence and the recorded build minimum still gate BOTH shapes: they protect the per-launch isolated HOME, which a hosted launch depends on exactly as a review does. One ladder takes the launch shape as a parameter (LaunchRefusal(bool reviewFlow)) rather than splitting into two that must agree - a shape this file has already paid for once. Advertisement keeps the full ladder including consent, since advertising IS the offer to review unattended and the server refuses an unadvertised reviewer. The daemon now seeds this vendor's version floor whenever agy resolves, not from the reviewer consent event. Without that the fix would be worthless: seeded from consent, a consent-less daemon records no minimum, so every hosted launch would refuse as version_no_minimum instead - the same gate removed from the front of the ladder and reinstated behind it. SeedVersionFloor is a separate name rather than SeedReviewerAffirmation(..., enabled: true), because a literal true between two config.XUnattendedReviewerEnabled siblings reads as an oversight and would be tidied back. Denial text, coded tokens unchanged (they are load-bearing for tests, the server's failure reasons and the README): - version_no_minimum no longer tells the operator to restart with the reviewer flag set. That is wrong for a hosted launch and, after the seeding change, stale for a reviewer too. - unsupported_platform, version_below_minimum, auth_unavailable and requires_owned_worktree now describe the containment rather than the reviewer. requires_owned_worktree in particular guards on ctx.Work, not IsReviewFlow, so it must not call the launch a review. - The launch log line drops "reviewer" and carries reviewFlow instead. - Left alone: binary_missing, version_unresolved, version_incomparable, launch_timeout and launch_failed were already shape-neutral, and the consent refusal itself is now correct because only a review can reach it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A turn is one `agy -p` process and between turns there is no process at all, so mid-turn input cannot steer the running turn: it has to become the NEXT turn. That already worked in order, and never coalesced - the tests here pin it, since a concatenated pair of user messages is unrecoverable rather than untidy - but the over-cap half was broken in the way that is hardest to see. The pending-turns channel used BoundedChannelFullMode.DropWrite, under which TryWrite returns TRUE and discards the item. So the "queue full" branch was unreachable: an over-cap message vanished with not even a log line, let alone anything the user could see. FullMode.Wait (still TryWrite, never WriteAsync) makes the rejection observable without blocking - blocking would stall the daemon's serial command lane behind the very stuck turn that filled the queue. An over-cap send now faults for EVERY caller, not just one that asked for a write ack (the ack-less path is every server-driven SendInput, i.e. exactly the caller that would otherwise never learn), and emits a system_note onto the transcript, which is the only surface the person who typed the message sees. One note per rejected message: each is a separately lost message. A TERMINAL runtime stays as it was - its transcript channel is already completed, so no note could be delivered, and the session ending is its own signal. `kcap agent attach` against a runtime that emits no terminal output attached the client to a blank screen that never repaints, and only admitted the problem if the user typed - indistinguishable from a wedged daemon. It is now refused by name, identifying the vendor and pointing at the dashboard. Enforced in the daemon, not the CLI: attach sends a full agent id verbatim without ever fetching the agent table, so the client cannot know the vendor. That covers hosted agy and the ACP vendors alike, which have the same empty output buffer. The raw-input NotSupportedException path stays covered as defence in depth for a runtime whose two answers disagree.
StartAsync serves both a hosted agent and a review-flow reviewer, but it injected the kcap-flow-result MCP channel unconditionally — and validated that channel's inputs unconditionally too, so a hosted launch could be refused with antigravity_reviewer_result_channel_incomplete for a channel it has no flow to report to, and the definition's MCP allowlist could refuse it for a definition it does not have. Both the injection and its fail-closed validation now sit behind ctx.IsReviewFlow, in one BuildReviewFlowMcp helper — the same split AcpHostedAgentRuntimeFactory.ValidateAndBuildReviewFlowMcp already draws. The hosted arm forwards ctx.McpServers, mirroring that factory's hosted arm; nothing populates it today, so a hosted launch's surface is empty deliberately rather than by a drop. The isolated home still gets a valid mcp_config.json either way — empty for hosted, which is what replaces the operator's global config and keeps the surface closed. Tests pin both directions: a hosted launch with no result-channel inputs succeeds with an exactly-empty config; a review missing them still fails closed with the coded reason, injects the channel with its command and both env vars, materializes an auto-approvable allowlist entry, refuses a flow-starting one, and never forwards a caller-supplied server.
…rejection faking liveness Six review findings on the hosted-Antigravity branch. 1. The one production line the consent-less-seeding commit exists to change was untested: swapping SeedVersionFloor back to SeedReviewerAffirmation(..., config.AntigravityUnattendedReviewerEnabled, ...) survived the whole Antigravity and DaemonRunner suites. Nothing invokes DaemonRunner.RunAsync from a test (it builds the entire DI host), and the existing test called SeedVersionFloor by hand -- pinning the directory shape but not the conditionality, which is the half that changed. The three seeding calls now sit in SeedReviewerFloors(stateDir, config), which RunAsync invokes and the tests drive: the hosted-launch test goes through it, and a new sibling asserts the asymmetry directly (antigravity records a floor without consent; kiro and gemini do not, with all three binaries resolvable so the siblings' absence is the consent condition and not a missing stub). Both mutants are red. 2. Antigravity's floor is no longer anchored to a consent event, so a pre-consent build stays the minimum across an upgrade and a later downgrade to it is admitted. Accepted deliberately -- hosted launches need a floor without consent, the window is bounded to builds at or above the pre-consent one, and the affirm verb is the remedy -- and recorded as an explicit per-vendor exception on ReviewerVersionStore, whose contract it bends, with a pointer from DaemonReviewerCommand and an operator-facing note in the README. The gate is not weakened and consent does not re-seed. 3. BuildTurnPsi's doc cited Claude as precedent for widening a hosted launch. Claude's split runs the other way: ClaudeLauncher.BuildArgs adds --permission-mode bypassPermissions only inside its IsReviewFlow branch, and DisablesApprovalPrompts is IsReviewFlow && Work == OwnedWorktree. A maintainer "aligning with the precedent" would move the flag onto the reviewer arm -- the one direction this split exists to prevent. Rewritten to claim only what is true: Claude is the precedent for a single-axis no-prompt posture existing at all (unlike Codex's Sandbox x Approval), and explicitly not for which launch kind receives it. 4. LaunchKind.Review silently took the hosted arm. The lift keys on !IsReviewFlow, true for Default AND Review, and only PtyHostedAgentRuntimeFactory builds the review MCP config and prompt -- so a PR review would have run with --dangerously-skip-permissions, an empty MCP surface and no review prompt. Refused up front with antigravity_pr_review_unsupported, ahead of the containment ladder since no install or affirmation could clear it. The shipped UI cannot request it (the server hard-codes Claude, and the dialog has no vendor picker), but the hub takes vendor and kind as independent client arguments and rejects only Codex+Review, so it is wire-reachable. The factory doc no longer claims the context carries two launch shapes. 5. A rejected input advanced the liveness clock: the full-queue notice went out through EmitEnvelope, which advances first. The queue is full only because a turn is wedged, so a user's retries against a stuck agent refreshed the very attestation that would report it idle -- bounded for a reviewer by its TTL arm, unbounded for a hosted agent. Daemon-authored notices now go through EmitDaemonNotice, which writes identically but does not advance. Agent output is unchanged, pinned in both directions. 6. Dropped a racy decorative assertion whose comment claimed it proved "a note per rejection, not per send" -- notes.Count == 2 already carries that, and HandleInit resolves the conversation-id TCS before emitting session_started with RunContinuationsAsynchronously, so it was a load-sensitive false red rather than a detector. Antigravity 248/0, *Reviewer* 372/0, DaemonRunner 53/0 (5 skipped, all live-cert gated). Three CodexLauncher Prepare_* failures under *Launch* are inherited -- reproduced identically on a detached cf63758 worktree. Daemon AOT publish clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR Summary by QodoEnable hosted Antigravity (agy) agents with correct posture and launch gating
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
The queue-full rejection logged at Warning, so an operator saw that an input was refused — but the transcript note is the only thing the person who typed it ever sees (the faulted send reaches the daemon log, never their screen), and that note is best-effort: the Terminal check releases the state lock before the emit, so a TerminateAsync landing in the gap completes the transcript writer and the note is discarded. The authoritative record therefore said "rejected" while staying silent about the sender having been told nothing. Write now reports whether the envelope landed, and the rejection path escalates a miss to Warning itself. Deliberately not raised inside Write: an envelope arriving after the channel closed is the ordinary shape of teardown — the turn worker drains agy's last lines while TerminateAsync completes the writer — so warning there would fire on every clean stop and train people to ignore the level. The race itself is left open. Closing it means holding the state lock across a channel write, and no foreign call under that lock is one of this class's standing invariants. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The launch test read the per-launch mcp_config.json with File.ReadAllTextAsync. That file lives in the child's own config dir, so agy may rewrite it, and a write-denying open is mandatory sharing on Windows — invisible on macOS and Linux, red only on the Windows CI leg. Reads via WatchCommand.ReadAllTextSharedAsync now, the same helper the watcher uses for agent-written transcripts. Also shortens the two longest doc comments this branch added. Both kept the facts that are not recoverable from the code — that the antigravity floor is seeded from the binary resolving rather than from consent, and that Claude's own posture split runs the opposite way so nobody "aligns" this flag onto the reviewer arm — and lost the surrounding essay. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes
agyusable as a hosted dashboard agent, alongside the unattended reviewer that landed in #479. One factory now serves both, so most of this is getting that split right.Fixes AI-1412
Server-side half: kurrent-io/kcap-server#1347 (merged).
The split
--dangerously-skip-permissionsKCAP_ANTIGRAVITY_UNATTENDED_REVIEWERconsentkcap-flow-resultMCP + result-channel validationHOMEThe flag is the entire read boundary; the owned worktree is not one. Measured on 1.1.10: with it, an absolute out-of-workspace
view_filesucceeds; without it, refused with a typedtool_info.error. Claude is the precedent for a single-axis no-prompt posture existing at all (Codex's is two axes, so its no-prompt mode still sits on a sandbox) — but explicitly not for which launch kind gets it, since Claude widens its reviewer and prompts on interactive, the mirror image of this.Consent is reviewer-only. Its justification is cross-principal exposure — a reviewer returns what it read to whoever requested the review. A hosted launch has no such exposure:
DaemonRegistryis keyed(TeamId, OwnerUserId, Name)andCapacitorHubresolves the daemon from the caller's own id, so the launcher is the owner. Hosted agy ships on by default, per the owner's decision.The isolated HOME stays for hosted too, and not for the reason originally supposed. Four piped runs under a real HOME reached EOF in 6–16s, so the "capture hooks wedge the turn's stdout" premise doesn't hold. What the probe did find: every run was also captured by the hook lane as a second watcher session, while the runtime already forwards its own transcript. An inherited HOME duplicates capture rather than adding it.
Also here
BoundedChannelFullMode.DropWrite, whereTryWritereturns true and silently discards — so the existing "queue full" branch was unreachable dead code and an over-cap message vanished without a log line. Over-cap now faults the send and writes asystem_note, one per rejected message.kcap agent attachrefused by name, daemon-side on!EmitsTerminalOutput— the client sends an id verbatim and cannot know the vendor. Replaces a blank-screen hang; the remove-the-refusal mutant fails by timeout, reproducing it.LaunchKind.Reviewis wire-reachable independently of vendor (CapacitorHubtakesvendorandkindas separate client arguments and rejects only the Codex pair), and would otherwise have got the hosted arm with no review tools and no error.Testing
253 Antigravity, 377 across all reviewer suites, 53
DaemonRunner— all green; daemon AOT clean. Every guard mutation-verified.Two worth calling out. The consent fix's one production line was untested — reverting it passed 248/0, because nothing invokes
DaemonRunner.RunAsyncand the test called the seeding helper directly, pinning the directory but not the conditionality. Hoisted intoSeedReviewerFloorsso the asymmetry is the thing under test. And one mutant "survived" six runs having never compiled: a scripted edit left the source mtime not newer than the last build output, so MSBuild skipped it.One accepted trade is documented in
ReviewerVersionStore: antigravity's floor is seeded from the binary resolving rather than from a consent event, because hosted launches need one without consent. That leaves a narrow window where a downgrade below the pre-consent build is admitted; the remedy iskcap daemon reviewer affirm --vendor antigravity.🤖 Generated with Claude Code