Skip to content

Antigravity CLI (agy) as an unattended review-flow reviewer - #479

Merged
realtonyyoung merged 23 commits into
mainfrom
claude-tyoung/ai-1414-agy-unattended-reviewer
Aug 7, 2026
Merged

realtonyyoung merged 23 commits into
mainfrom
claude-tyoung/ai-1414-agy-unattended-reviewer

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

Adds agy — the Antigravity CLI — as an unattended review-flow reviewer, via the codebase's first exec-per-turn hosted-agent runtime.

Fixes AI-1414

Why this one is shaped differently

Every other hosted runtime here wraps a single long-lived child (PTY or ACP). agy speaks neither protocol: each turn is a separate agy -p … --output-format stream-json --conversation <id> process that exits when the turn ends, and between turns there is no process at all. So the runtime is a phase machine over a turn queue, translating agy's NDJSON into AcpEventEnvelopes and feeding the existing AcpTranscriptForwarder unchanged.

Server-side: no changes. Routing was already vendor-neutral.

Containment

A daemon-owned worktree plus an empty per-launch HOME. That single choice does two jobs: no kcap plugin means no capture hooks, so recording stays single-lane; and no global mcp_config.json means nested review flows can't be launched. ADC env auth (AGY_ADC_AUTH=1 + GOOGLE_CLOUD_PROJECT + inherited credentials) is what lets that HOME stay empty.

Enforced by a real-process test, not a model-layer refusal — live run observed: zero kcap log/watcher entries keyed on the conversation id, zero processes carrying it, agy's Library writes landing under the relocated home, and the operator's own agy store untouched, with a positive control proving that state does exist in the contained tree.

The reviewer deliberately does not pass --dangerously-skip-permissions. Measured on 1.1.10: without it, an absolute out-of-workspace view_file is refused (state: "ERROR", tool_info.error = {TOOL_ERROR, "User denied permission for read_file(…)"}, content never read); with it, the same read succeeds. That flag is the entire read boundary, and a reviewer only reads.

Gating

Consent (KCAP_ANTIGRAVITY_UNATTENDED_REVIEWER=1) → POSIX-only → binary present → minimum version floor (KCAP_ANTIGRAVITY_MIN_CLI_VERSION, default 1.1.10), through the existing CliVersionAllowed range grammar rather than a third version mechanism.

A floor rather than a per-build affirmation, unlike Kiro and Gemini, because agy auto-updates on a cadence the operator neither controls nor predicts — it moved 1.1.8 → 1.1.10 mid-development. kcap daemon reviewer affirm --vendor antigravity therefore refuses with a coded antigravity_reviewer_not_affirmable rather than silently succeeding at a no-op. An unidentifiable version still refuses: a build we can't name can't be shown to meet a floor.

The floor is applied at both advertisement and the launch boundary, off one shared decision — an explicit vendor request that bypasses advertisement can't slip a below-floor build through.

Notable details

  • ServiceEnvironment.ReviewerConsentKeys gains the Antigravity flag. Without it a service-installed daemon silently drops the operator's consent from its unit and the reviewer could never be enabled.
  • Per-turn PID records. The durable record is rewritten on every turn spawn and cleared on confirmed exit, so rounds 2+ stay reapable — a one-shot record would only ever name turn 1's pid.
  • Teardown is measured, not assumed. The per-launch HOME holds the reviewer's conversation JSONL (the caller's diff and findings), so it's deleted only on confirmed quiescence; unconfirmed skips and logs, leaving the epoch sweep to collect it, because deleting under a live child leaves it writing into an unlinked path.
  • A denial ends the turn — no closing agent_response, empty result.response. Anything waiting on a post-denial summary hangs rather than fails, so waits are bounded.

Testing

209 Antigravity tests (204 pass, 5 skipped — the env-gated live certs). Full suite sits at the current origin/main baseline of 49 failures, verified by set-diffing against a detached baseline worktree; the two branch-only entries are the known load-dependent PTY class and pass in isolation. Daemon and CLI AOT publishes are both clean.

The end-to-end live cert (AntigravityReviewerLiveCertTests) is written and proven to skip at ~13 ms with zero spend, but has not been run live — it needs a daemon advertising antigravity, which means the consent flag captured into a supervised daemon's unit and a restart. That's an operator action, noted rather than simulated.

Probe record: docs/probes/2026-08-06-agy-reviewer/findings.md.

🤖 Generated with Claude Code

@linear-code

linear-code Bot commented Aug 7, 2026

Copy link
Copy Markdown

AI-1414

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add Antigravity CLI (agy) as an unattended exec-per-turn review-flow reviewer

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Adds agy (Antigravity CLI) as a new unattended review-flow reviewer vendor, via the codebase's
 first exec-per-turn hosted-agent runtime.
• Translates agy NDJSON (--output-format stream-json) into AcpEventEnvelopes and forwards via
 existing ACP transcript pipeline.
• Enforces containment with a daemon-owned worktree plus an empty per-launch HOME, verified by
 real-process tests.
• Gates availability by consent → POSIX-only → binary present → minimum CLI version floor (not
 per-build affirmation).
Diagram

graph TD
  Orchestrator["AgentOrchestrator"] --> Factory["AntigravityHostedAgentRuntimeFactory"] --> Runtime["AntigravityHostedAgentRuntime"]
  Runtime -->|"spawns per turn"| AgyProc[("agy turn process")]
  AgyProc -->|"NDJSON stdout"| Ndjson["AntigravityNdjson"] --> Forwarder["AcpTranscriptForwarder"]
  Factory -->|"creates/deletes"| Home[("Isolated HOME")]
  Factory -->|"gate check"| Capability["Reviewer gate ladder"]
  subgraph Legend
    direction LR
    _svc(["Service"]) ~~~ _proc(["Per-turn child"]) ~~~ _mod["Module"]
  end
Loading
High-Level Assessment

Given agy’s exec-per-turn protocol (no PTY/ACP long-lived child) and the existing ACP envelope pipeline, a dedicated phase-machine runtime that translates NDJSON into AcpEventEnvelopes is the right integration point and keeps AcpTranscriptForwarder unchanged. Using a minimum-version floor (via existing CliVersionAllowed range grammar) instead of per-build affirmation is consistent with agy’s observed auto-update behavior and avoids operational churn while still failing closed.

Files changed (29) +6120 / -24

Enhancement (9) +2518 / -4
AntigravityNdjson.csParse agy NDJSON and map to ACP transcript envelopes +345/-0

Parse agy NDJSON and map to ACP transcript envelopes

• Introduces an AOT-friendly NDJSON parser for agy 'init'/'step_update'/'result' events and a step accumulator for text deltas, usage, and tool lifecycle. Produces 'AcpEventEnvelope's without emitting 'session_ended' (server owns session termination).

src/Capacitor.Cli.Daemon/Acp/AntigravityNdjson.cs

AntigravityReviewerCapability.csAdd unattended capability gate for Antigravity reviewer +114/-0

Add unattended capability gate for Antigravity reviewer

• Implements pure decision logic for enabling the reviewer: consent flag, POSIX-only requirement, and minimum CLI version floor. Provides coded denial reasons and reuses 'DaemonRunner.CliVersionAllowed' for version comparison.

src/Capacitor.Cli.Daemon/Acp/AntigravityReviewerCapability.cs

AntigravityReviewerHome.csCreate and manage isolated per-launch HOME for agy reviewer +281/-0

Create and manage isolated per-launch HOME for agy reviewer

• Creates an owner-only per-launch HOME that writes only an injected 'mcp_config.json', verifies the kcap plugin directory is absent, and prevents path escape (e.g., via daemon 'GEMINI_CLI_HOME'). Adds safe deletion (no symlink traversal) and stale-home sweeping by daemon epoch.

src/Capacitor.Cli.Daemon/Acp/AntigravityReviewerHome.cs

IAgyTurnProcess.csIntroduce per-turn process abstraction for agy exec-per-turn runtime +67/-0

Introduce per-turn process abstraction for agy exec-per-turn runtime

• Defines the minimal lifecycle/IO interface for one 'agy -p' turn, including NDJSON line streaming, termination, and idempotent disposal contracts. Enables runtime testing without spawning real processes.

src/Capacitor.Cli.Daemon/Acp/IAgyTurnProcess.cs

DaemonRunner.csWire Antigravity config/env overrides and register runtime factory +43/-3

Wire Antigravity config/env overrides and register runtime factory

• Binds new 'KCAP_ANTIGRAVITY_*' env overrides into config, registers the Antigravity runtime factory, and sweeps stale Antigravity reviewer homes at daemon startup. Updates unattended vendor list computation to accept 'DaemonConfig' so the floor is consistently applied.

src/Capacitor.Cli.Daemon/DaemonRunner.cs

AntigravityHostedAgentRuntime.csAdd exec-per-turn hosted agent runtime with explicit phase machine +1034/-0

Add exec-per-turn hosted agent runtime with explicit phase machine

• Implements the exec-per-turn runtime: queues turns, spawns a new child per turn, translates NDJSON into transcript envelopes, and maintains logical liveness between turns. Encodes critical concurrency/termination invariants (single terminal signal, lock ordering, EOF-without-result => Terminal).

src/Capacitor.Cli.Daemon/Services/AntigravityHostedAgentRuntime.cs

AntigravityHostedAgentRuntimeFactory.csAdd factory that owns agy argv/env, HOME containment, and launch handshake +602/-0

Add factory that owns agy argv/env, HOME containment, and launch handshake

• Implements the unattended gate ladder (including binary presence + version floor), constructs per-turn 'ProcessStartInfo', creates per-launch isolated HOME (with injected MCP config), and enforces that 'StartAsync' does not return until conversation id is resolved. Refuses non-review-flow launches and borrowed worktree requests.

src/Capacitor.Cli.Daemon/Services/AntigravityHostedAgentRuntimeFactory.cs

DaemonReviewerCommand.csRefuse 'affirm' for Antigravity (non-affirmable floor-based gate) +30/-0

Refuse 'affirm' for Antigravity (non-affirmable floor-based gate)

• Adds a NonAffirmableReviewer table so 'kcap daemon reviewer affirm --vendor antigravity' fails with a coded explanation rather than succeeding as a no-op or being treated as an unknown vendor.

src/Capacitor.Cli/Commands/DaemonReviewerCommand.cs

ReviewerVendors.csSwitch canonical vendor token from 'agy' to 'antigravity' +2/-1

Switch canonical vendor token from 'agy' to 'antigravity'

• Updates the canonical list of user-facing reviewer vendor tokens to include 'antigravity' instead of 'agy'. Downstream tests update expected token lists accordingly.

src/Capacitor.Cli/Commands/ReviewerVendors.cs

Bug fix (1) +13 / -0
AgentOrchestrator.csWire per-turn PID recording callbacks for exec-per-turn runtime +13/-0

Wire per-turn PID recording callbacks for exec-per-turn runtime

• Adds per-turn PID record/clear callbacks for Antigravity runtime so round 2+ processes are durably tracked and reapable after daemon death. Mirrors the intent of existing PID recording for long-lived runtimes but at per-turn cadence.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs

Tests (15) +3248 / -12
ReviewerVendorPreferenceTests.csUpdate integration expectations for vendor token list +2/-1

Update integration expectations for vendor token list

• Updates expected warning output to list 'antigravity' instead of 'agy' among known vendor tokens.

test/Capacitor.Cli.Tests.Integration/ReviewerVendorPreferenceTests.cs

AntigravityContainmentTests.csAdd gated real-process containment enforcement test +347/-0

Add gated real-process containment enforcement test

• Runs a real agy turn under the production per-launch HOME and asserts containment using filesystem/process-table observations (no kcap logs/watchers keyed on conversation id; operator agy store untouched). Gated behind env vars to avoid CI dependency on agy and credentials.

test/Capacitor.Cli.Tests.Unit/Acp/AntigravityContainmentTests.cs

AntigravityNdjsonTests.csAdd unit tests for NDJSON parsing and step accumulator +253/-0

Add unit tests for NDJSON parsing and step accumulator

• Pins parsing of verbatim-captured NDJSON fixtures and validates step aggregation/flush semantics for text, usage, and tool call/result mapping. Ensures unknown/malformed lines are tolerated (fail-open for parsing, fail-closed for behavior).

test/Capacitor.Cli.Tests.Unit/Acp/AntigravityNdjsonTests.cs

AntigravityReviewerCapabilityTests.csAdd unit tests for Antigravity gate ladder and denial reasons +175/-0

Add unit tests for Antigravity gate ladder and denial reasons

• Covers consent short-circuiting, platform refusal, version-floor comparisons (including suffixes), and denial reason text content. Ensures an unparseable floor fails closed.

test/Capacitor.Cli.Tests.Unit/Acp/AntigravityReviewerCapabilityTests.cs

AntigravityReviewerHomeTests.csAdd unit tests for reviewer HOME isolation and sweeping +64/-0

Add unit tests for reviewer HOME isolation and sweeping

• Asserts that the home contains only injected MCP server config, that the kcap plugin dir does not exist, that permissions are owner-only (POSIX), and that stale epochs are swept while current epoch is preserved.

test/Capacitor.Cli.Tests.Unit/Acp/AntigravityReviewerHomeTests.cs

AntigravityReviewerReapingTests.csAdd reaping behavior tests for exec-per-turn reviewer +195/-0

Add reaping behavior tests for exec-per-turn reviewer

• Validates that between turns the reviewer is idle-reapable, and mid-turn it is not idle-reaped, using the real orchestrator reaper logic. Also pins that launch wiring attaches the per-turn PID record seam.

test/Capacitor.Cli.Tests.Unit/AntigravityReviewerReapingTests.cs

DaemonReviewerCommandTests.csAdd tests for non-affirmable Antigravity reviewer command behavior +73/-0

Add tests for non-affirmable Antigravity reviewer command behavior

• Ensures Antigravity is recognized as non-affirmable, the match is case-insensitive, no vendor is both affirmable and non-affirmable, and 'affirm --vendor antigravity' refuses with its own explanation (not unknown-vendor text).

test/Capacitor.Cli.Tests.Unit/Commands/DaemonReviewerCommandTests.cs

DaemonRunnerAntigravityFloorTests.csAdd tests that vendor advertisement applies the Antigravity version floor +120/-0

Add tests that vendor advertisement applies the Antigravity version floor

• Uses a stub 'agy' executable to assert that ComputeUnattendedVendors applies the floor through the real factory gate ladder (not via a separate filter). Also pins that advertised capabilities include a probed CLI version for antigravity.

test/Capacitor.Cli.Tests.Unit/Daemon/DaemonRunnerAntigravityFloorTests.cs

DaemonRunnerCursorAvailabilityTests.csUpdate tests for new ComputeUnattendedVendors signature +5/-5

Update tests for new ComputeUnattendedVendors signature

• Adjusts existing tests to pass 'DaemonConfig' after ComputeUnattendedVendors was updated to require it, ensuring consistent vendor-list derivation across callers.

test/Capacitor.Cli.Tests.Unit/Daemon/DaemonRunnerCursorAvailabilityTests.cs

AntigravityActivityClockTests.csAdd tests for ActivityClock TurnInFlight semantics +289/-0

Add tests for ActivityClock TurnInFlight semantics

• Pins the runtime’s TurnInFlight behavior across settled turns, long-running turns, and terminal transitions so the orchestrator’s idle/wedge reaping decisions remain correct.

test/Capacitor.Cli.Tests.Unit/Services/AntigravityActivityClockTests.cs

AntigravityReviewerLaunchTests.csAdd tests pinning agy argv/env and conversation resume behavior +667/-0

Add tests pinning agy argv/env and conversation resume behavior

• Asserts the full argument vector (including '--disable-slash-commands' and '--print-timeout'), verifies the reviewer never passes '--dangerously-skip-permissions' or '--sandbox', and checks correct use of '--conversation' after turn 1.

test/Capacitor.Cli.Tests.Unit/Services/AntigravityReviewerLaunchTests.cs

AntigravityReviewerLiveCertTests.csAdd gated live certification tests against real agy +332/-0

Add gated live certification tests against real agy

• Gated end-to-end certification verifying real unattended rounds complete and multi-round reviews maintain one stable conversation id across distinct per-turn processes. Requires operator-provided agy + ADC environment.

test/Capacitor.Cli.Tests.Unit/Services/AntigravityReviewerLiveCertTests.cs

AntigravityRuntimeFakes.csAdd reusable fakes for exec-per-turn runtime tests +144/-0

Add reusable fakes for exec-per-turn runtime tests

• Provides FakeAgyTurnProcess and helper constructors modeling normal turns, EOF-without-result, never-ending turns, and conversation id mismatch scenarios for deterministic lifecycle testing.

test/Capacitor.Cli.Tests.Unit/Services/AntigravityRuntimeFakes.cs

AntigravityRuntimeLifecycleTests.csAdd phase-machine lifecycle tests for exec-per-turn runtime +545/-0

Add phase-machine lifecycle tests for exec-per-turn runtime

• Exercises key invariants: logical liveness between turns, ReadOutputAsync not completing while idle, EOF without terminal result driving Terminal, and deadlock-free TerminateAsync behavior under an in-flight turn.

test/Capacitor.Cli.Tests.Unit/Services/AntigravityRuntimeLifecycleTests.cs

ServiceEnvironmentTests.csExtend service env tests for Antigravity consent and AGY_ADC_AUTH +37/-6

Extend service env tests for Antigravity consent and AGY_ADC_AUTH

• Ensures the supervised-daemon environment carries the Antigravity consent flag alongside existing reviewer flags, and that AGY_ADC_AUTH is carried as a non-secret config key on all platforms including Windows.

test/Capacitor.Cli.Tests.Unit/Services/ServiceEnvironmentTests.cs

Documentation (2) +290 / -4
README.mdDocument unattended Antigravity reviews +55/-4

Document unattended Antigravity reviews

• Adds setup instructions for enabling Antigravity unattended reviews, including ADC auth requirements, POSIX-only constraint, and minimum-version floor behavior. Updates service install guidance to include Antigravity consent and ADC variables.

README.md

findings.mdAdd probe findings on containment/auth/read boundary +235/-0

Add probe findings on containment/auth/read boundary

• Adds measured observations backing the design: NDJSON wire shapes, containment evidence, and permission boundary behavior (notably avoiding '--dangerously-skip-permissions'). Serves as the empirical basis for containment and gating decisions.

docs/probes/2026-08-06-agy-reviewer/findings.md

Other (2) +51 / -4
DaemonConfig.csAdd Antigravity reviewer configuration surface +40/-0

Add Antigravity reviewer configuration surface

• Adds config properties for Antigravity path/model, unattended consent flag, minimum CLI version floor, and launch/turn timeouts, with documented rationale for defaults and fail-closed behavior.

src/Capacitor.Cli.Daemon/DaemonConfig.cs

ServiceEnvironment.csCarry Antigravity consent and AGY_ADC_AUTH into supervised daemon units +11/-4

Carry Antigravity consent and AGY_ADC_AUTH into supervised daemon units

• Adds 'KCAP_ANTIGRAVITY_UNATTENDED_REVIEWER' to the always-carried consent keys and 'AGY_ADC_AUTH' to the always-carried Google config keys (treated as a non-secret boolean switch).

src/Capacitor.Cli/Services/ServiceEnvironment.cs

@qodo-code-review

qodo-code-review Bot commented Aug 7, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Manual ValueKind check in TryParseLine ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The new NDJSON parser directly checks JsonElement.ValueKind instead of using the project's
JsonElementExtensions, violating the standard JSON-handling requirement. This increases the chance
of inconsistent parsing behavior vs. other JSON codepaths and adds avoidable manual branching.
Code

src/Capacitor.Cli.Daemon/Acp/AntigravityNdjson.cs[101]

+            if (root.ValueKind != JsonValueKind.Object) return null;
Evidence
PR Compliance ID 3 requires using JsonElementExtensions instead of manual JsonValueKind
branching. The added code explicitly checks root.ValueKind != JsonValueKind.Object, which is a
direct manual ValueKind check in new JSON handling logic.

CLAUDE.md: Use JsonElementExtensions Instead of Manual JSON ValueKind Checks
src/Capacitor.Cli.Daemon/Acp/AntigravityNdjson.cs[99-103]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`AntigravityNdjson.TryParseLine` manually checks `root.ValueKind` instead of relying on the project’s `JsonElementExtensions` helpers, which is disallowed by the compliance checklist.

## Issue Context
The code already uses `root.Str(...)` / `root.Obj(...)` (from `JsonElementExtensions`) for other JSON reads, so the explicit `ValueKind` check can be removed/refactored to align with the standard pattern.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Acp/AntigravityNdjson.cs[99-108]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Cleanup gated on stale exit ✓ Resolved 🐞 Bug ☼ Reliability
Description
AntigravityHostedAgentRuntime latches _turnExitConfirmed from process.HasExited before calling
process.DisposeAsync, and uses that flag to decide whether to clear the PID record and (later)
whether it’s safe to delete the isolated reviewer HOME. If the grace wait returns without confirming
exit and DisposeAsync subsequently kills the still-running process, the flag remains false, causing
stale PID records and leaving transcript-bearing HOME directories behind until the next daemon-epoch
sweep.
Code

src/Capacitor.Cli.Daemon/Services/AntigravityHostedAgentRuntime.cs[R705-708]

+            // Asked BEFORE that disposal, which is the only point this is a truthful question: a
+            // disposed process object reports HasExited true, so the same read afterwards would
+            // manufacture a confirmation. DisposeAsync's cleanup gate is the consumer.
+            _turnExitConfirmed = process.HasExited;
Evidence
The runtime uses a pre-disposal HasExited snapshot to decide PID-record clearing and later HOME
deletion eligibility, but the concrete IAgyTurnProcess implementation will kill the process during
DisposeAsync if it is still running. Because WaitForExitAsync is explicitly allowed to time out
silently, the pre-disposal snapshot can remain false even though disposal performs the termination,
leading to skipped cleanup and stale bookkeeping.

src/Capacitor.Cli.Daemon/Services/AntigravityHostedAgentRuntime.cs[674-726]
src/Capacitor.Cli.Daemon/Services/AntigravityHostedAgentRuntime.cs[1007-1026]
src/Capacitor.Cli.Daemon/Services/AntigravityHostedAgentRuntimeFactory.cs[529-545]
src/Capacitor.Cli.Daemon/Services/AntigravityHostedAgentRuntimeFactory.cs[561-568]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`AntigravityHostedAgentRuntime.ProcessTurnAsync` snapshots `_turnExitConfirmed = process.HasExited` before `process.DisposeAsync()`, but `AgyTurnProcess.DisposeAsync()` will kill the process tree if it’s still running. This means a turn can be forcibly terminated during disposal while `_turnExitConfirmed` remains `false`, which then (a) skips `PidCallbacks.Clear()` and (b) causes `AntigravityHostedAgentRuntime.DisposeAsync()` to skip deleting the isolated HOME (leaving transcript-bearing directories behind).

### Issue Context
This is most likely when the “exit confirmation grace” wait doesn’t confirm exit (timeout or OS lag) even though the process is about to exit or will be killed as part of disposal. The runtime should base cleanup decisions on the final termination outcome (after any forced termination), not on a pre-disposal snapshot.

### Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AntigravityHostedAgentRuntime.cs[674-726]
- src/Capacitor.Cli.Daemon/Services/AntigravityHostedAgentRuntime.cs[1007-1026]
- src/Capacitor.Cli.Daemon/Services/AntigravityHostedAgentRuntimeFactory.cs[529-590]

### Implementation direction
- After `WaitForExitAsync(ExitConfirmationGrace)`, if `process.HasExited` is still false, explicitly terminate and then re-wait with a bounded timeout to confirm exit (or treat as unconfirmed and enter Terminal).
- Set `_turnExitConfirmed` from the *post-termination* observation (after the extra wait), so `PidCallbacks.Clear()` and HOME deletion gating reflect the final state.
- Consider updating `AgyTurnProcess.DisposeAsync()` to optionally wait briefly for exit after `Kill(...)` (bounded), so callers can reliably confirm termination before making cleanup decisions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread src/Capacitor.Cli.Daemon/Acp/AntigravityNdjson.cs Outdated
Comment thread src/Capacitor.Cli.Daemon/Services/AntigravityHostedAgentRuntime.cs
realtonyyoung and others added 22 commits August 7, 2026 07:25
…rvice units

Adds the config surface later tasks read to spawn agy as an unattended
review-flow reviewer: DaemonConfig.AntigravityPath (defaulting to "agy",
matching a standard install's PATH entry) plus model, consent, and two
timeout knobs, each with an env override in DaemonRunner. Also adds
AGY_ADC_AUTH to ServiceEnvironment's always-carried Google config keys —
it's a boolean auth-mode switch, not a credential, so it's carried on
every platform including Windows.

AI-1414
…ation, GEMINI_CLI_HOME containment check

Review findings: the first test lacked the Windows guard its two siblings have
(CreateOwnerOnly throws unconditionally on Windows); Create's delete-then-create
sequence didn't verify the directory was actually empty before writing, so a
partially-failed best-effort Delete could silently inherit a previous
reviewer's brain/conversations state; and WriteMcpConfig's path resolution
honors the daemon process's own GEMINI_CLI_HOME, which could write the
reviewer's result channel outside the isolated home entirely. All three are
now guarded/verified rather than assumed, matching KiroReviewerHome's stated
philosophy.
AntigravityHostedAgentRuntime implements IHostedAgentRuntime/IAcpTranscriptSource
over agy's exec-per-turn shape (no long-lived child between turns), via an
explicit Starting/Executing/Idle/Terminal phase machine: EOF without a
terminal `result` drives Terminal (never Idle), HasExited reports logical
liveness, ReadOutputAsync parks on one constructor-owned TaskCompletionSource,
and TerminateAsync takes the state lock (never the turn gate) so a long-running
turn can never deadlock a stop. Adds IAgyTurnProcess as the injectable
per-turn spawn seam, and AntigravityRuntimeLifecycleTests pinning all four
rules plus the FakeRuntime/FakeTurn helpers a sibling plan reuses.
…ks, vacuous tests

Critical: AcpSessionId could read "" if a consumer (the orchestrator's
StartAcpForwardingAsync) reads it right after the initial prompt is sent,
since SendUserInputAsync/SendUserInputAndWaitForWriteAsync both return
before turn 1's `init` line is ever parsed. Adds WaitForConversationIdAsync,
a barrier completed from HandleInit and faulted on Terminal (never hangs),
documented as a hard requirement for any factory. Also makes
_conversationId/_cwd/_sessionStartedEmitted volatile for cross-thread
visibility, matching _current/_phase.

Important fixes:
- Every turn's IAgyTurnProcess is now disposed on every exit path (a
  try/finally wrapping ProcessTurnAsync's body) — previously leaked pipes/
  handles every round.
- Closed a Terminate-racing-spawn orphan: publishing _current and checking
  for an already-Terminal runtime now happens atomically under the same
  _stateLock TerminateAsync captures _current under, so exactly one side
  always takes responsibility for reaping a just-spawned child.
- De-vacuated five tests that called WaitForTurnIdleAsync immediately after
  SendUserInputAsync — that hand-off is asynchronous and can return before
  the turn even starts. Tests now await WaitForConversationIdAsync (or an
  explicit per-turn spawn signal for the two-turn conversation-id test)
  first, and assert the resolved conversation id positively rather than
  only a negative.
- Lifted FakeTurn/FakeAgyTurnProcess/FakeRuntime out of the test class into
  a shared AntigravityRuntimeFakes.cs so a sibling plan can actually
  reference the pinned helpers.
- Added FakeTurn.ChangedConversationId and a dedicated test exercising the
  mismatch → reap → Terminal path, previously uncovered.

Minor: a full pending-turns queue now logs at Warning with a running
dropped-count (matching AcpHostedAgentRuntime), distinct from the terminal
case which stays at Debug; _transcript is SingleWriter=false since
EnterTerminal's TryComplete can race the worker's own TryWrite.

Re-ran the TerminateAsync-takes-the-gate mutation check against the
restructured ProcessTurnAsync — still hangs with the mutant, still green
after the inverse revert.
…st, interface contract

MEDIUM: WaitForConversationIdAsync only resolved (success via HandleInit)
or faulted (via EnterTerminal) — but a turn can settle cleanly to Idle
(no EnterTerminal call at all) having never seen a non-empty
conversation_id on its init line, e.g. a malformed/missing init whose
transcript still reaches a terminal result. That left the barrier stuck
forever on an otherwise-healthy runtime, reopening the exact failure mode
it was added to close. Added a second fault call site (shared via
FaultConversationIdBarrierIfUnresolved) in ProcessTurnAsync's
clean-success tail, documented on rule (e), with a dedicated
FakeTurn.NormalWithoutConversationId regression test.

IMPORTANT: added a deterministic test for the Terminate-races-spawn
orphan fix (Terminate_racing_a_still_running_spawn_reaps_the_process_it_
never_got_to_track) using an entered/release TaskCompletionSource pair to
park the worker inside the spawn closure, guaranteeing TerminateAsync
wins the publish-vs-capture race, then asserting TerminateCalls == 1 &&
DisposeCalls == 1 on the process it never got to track. Also made
Terminate_during_a_long_turn_completes_rather_than_deadlocking's "the
turn has started" guarantee structural via an onSpawn signal instead of
relying on scheduling timing.

Hardening: documented dispose-idempotency and terminate-after-dispose as
required contracts on IAgyTurnProcess, since the real implementation
(a later task) is built against this interface. FaultConversationIdBarrierIfUnresolved
also immediately observes any exception it sets, so an unawaited barrier
never risks TaskScheduler.UnobservedTaskException under a host that
escalates it.

Re-ran the TerminateAsync-takes-the-gate mutation check once more against
the barrier/terminate-path changes — still hangs with the mutant, still
green (14/14) after the inverse revert.
Builds every turn child's argv and environment from one pure builder, creates
and disposes the per-launch isolated HOME, and holds the ordering the
orchestrator depends on: StartAsync does not return until turn 1's `init` has
resolved the conversation id, so a transcript can never bind to "".

Wires the runtime into AgentActivityClock, which the reviewer reaper now makes
every decision from. TurnInFlight is bracketed around the whole turn and also
cleared on entry to Terminal: between turns this runtime has no process at all,
so a flag left held would produce a reviewer that is never idle-reaped and
nothing external would ever contradict it. The clock is attached before the
first turn, since one assigned later makes every stamp in the launch a no-op.

An interactive launch is refused rather than accepted: an inherited HOME lets
agy's own capture hooks fire, and the watcher they spawn can hold this runtime's
stdout open after a turn exits, which would wedge every later turn.
Which cancellation type surfaces on a shutdown mid-launch depends on where the
cancel is observed, so an exact-type assertion passed under the whole-suite
filter and failed deterministically when the class ran alone.
Adds AntigravityReviewerCapability — the one place the gate ladder for this
vendor is defined: consent, platform, and a minimum CLI version FLOOR.

The floor is where Antigravity diverges from Kiro and Gemini. Those require an
operator to affirm each installed build; agy updates itself, so an affirmation
gate would park the reviewer on a cadence the operator neither controls nor can
predict. Anything at or above the floor is accepted, and the floor itself is
operator-settable (KCAP_ANTIGRAVITY_MIN_CLI_VERSION) so a false negative does
not wait for a release. There is deliberately no affirm verb for this vendor,
and no refusal points at one; `kcap daemon reviewer affirm --vendor antigravity`
refuses with its own coded explanation rather than as an unknown vendor.

An unidentifiable build stays a distinct arm from an old one: the operator's
next action differs. The comparison is the existing CliVersionAllowed, asked
twice — once against ">=0.0" for parseability, once against the floor — rather
than adding a second version parser.

Wiring: DI registration, the advertisement arm (so the capability payload
carries a probed CLI version instead of the generic arm's null), the crash-path
home sweep beside Kiro's, the reviewer consent flag in the service unit
(without it a supervised daemon silently drops the opt-in), and the advisory
vendor token, corrected from agy to antigravity.

The floor is applied at the advertisement seam, which only ever narrows — a
vendor the factory's own ladder already withheld keeps its own reason.
Three defects, all in the same shape: a bound that expired and a bound that
settled were indistinguishable.

1. DisposeAsync deleted the reviewer HOME — which holds the reviewer's own
   conversation JSONL, i.e. the caller's diff, source excerpts and findings —
   with no confirmed-exit gate. TerminateAsync's timeout, the turn worker's
   join and the process handle's disposal are all best-effort and swallowed,
   so "nothing of ours can still be writing" was asserted, never established.
   Kill(entireProcessTree: true) is not atomic against a grandchild forked
   between tree enumeration and signal, and agy's children include its MCP
   stdio servers, so a survivor racing the delete is a real shape: files are
   unlinked, agy mkdirs the tree back and keeps writing review context into a
   home nobody will now dispose. Follows the ACP runtime's gate, including its
   reasoning that unconfirmed means SKIP the deletion, not force it — deleting
   under a live reviewer is worse than leaving it, and the epoch-keyed startup
   sweep collects it on the next boot. The skip logs; a retained
   transcript-bearing home must never be silent.

2. RunTurnWorkerAsync's catch (OperationCanceledException) claimed owner
   cancellation without checking it. ProcessTurnAsync individually catches
   every IAgyTurnProcess call EXCEPT its bounded exit-confirmation wait, whose
   contract ("returns silently on timeout") does not forbid an implementation
   propagating its own cancellation — and this runtime is built against the
   interface, not against AgyTurnProcess. Such an exception exited the loop
   without EnterTerminal: _terminalTcs never completes, ReadOutputAsync parks
   forever, FinalizeAgentRunAsync never fires. Now filtered on the owner token,
   so the unexpected case falls through to the handler that does go Terminal.

3. The two turn-gate-flag tests were non-deterministic detectors of
   EnterTerminal's own SetTurnInFlight(false): the worker's finally clears the
   same flag, so whether the mutant was caught was a scheduling coin-flip
   (measured 1, 1, 0, 2, 2, 1 failures across six runs — a one-in-six chance of
   surviving CI). Both now park the worker where its finally structurally
   cannot have run — inside a read loop that ignores cancellation, and inside a
   held-open process disposal — so EnterTerminal is the only possible clear.
   Re-measured: 6/6 red under the same mutant.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Section 6 of the design mandates the exec-per-turn cadence over the existing
rewrite seam — record on turn spawn, clear on confirmed turn exit — so the
fail-closed PID-record contract applies per turn. It was never implemented: the
wiring at the launch path is gated on the ACP reconnect shape, which this
runtime is not, so it got only the one-shot record of turn 1's pid.

Consequence: every round after the first spawns a differently-pid'd child that
never enters the durable record, and the env-marker fallback cannot cover it
either — the startup scan is gated on Linux, while this reviewer is POSIX-only
meaning macOS in practice. A daemon SIGKILL during round 2+ left a turn child
reapable by neither mechanism. It self-clears via --print-timeout, which is why
this was a gap rather than a leak.

AgyPidRecordCallbacks mirrors AcpPidRecordCallbacks — one immutable bundle, so
there is no window with a real recorder and no clearer. Record runs before the
_current publish, so a refusal leaves nothing published to reconcile, and a
throw fails the turn: the child is reaped and the runtime goes terminal rather
than running untracked. Clear runs off the same single HasExited read the
disposal gate uses, so an unconfirmed survivor keeps the record that is the only
thing left able to reap it.

Left null the runtime records nothing, which is the deliberate pre-wiring state
rather than a fail-open hole: turn 1 spawns inside the factory's StartAsync,
before the orchestrator can wire anything, and the one-shot record covers
exactly that turn.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The shared AntigravityReviewerCapability.Decide/DenialReason shipped without a
caller: the factory kept its own inline ladder, pinned byte-identical by test,
and the minimum-version floor was applied separately at the ADVERTISEMENT seam
by DaemonRunner.ApplyAntigravityVersionFloor.

That left a real gap, documented at the time and closed here: advertisement is
what stops a launch being attempted, but an explicit vendor: "antigravity"
request reaches the factory without consulting it — so a below-floor build could
still be launched, because the launch boundary's defence-in-depth check could
not see the floor.

ReviewerRefusal now takes every verdict and every text from the shared decision,
and is read by both seams. Consent and platform are decided WITHOUT a probe: the
decision short-circuits both before it looks at a version, but C# evaluates
arguments first, so an inline probe would spawn agy for a daemon that switched
the reviewer off — the exact thing the consent arm's short-circuit exists to
prevent. The one arm the shared decision cannot express, a binary that does not
resolve at all, stays in the factory and stays AHEAD of the probe, so an
operator with no agy is told it is missing rather than sent to check a version
that was never going to resolve.

A resolveVersion seam mirrors the one the affirmation-gated reviewers already
have; production resolves the real probed version. Without it every launch test
would refuse as version_unresolved on any host without agy installed.

ApplyAntigravityVersionFloor and its threading are deleted rather than left to
agree with the factory by coincidence, and the byte-identical pin goes with them
— with one ladder it had nothing left to compare. The DaemonRunner tests now
exercise the real factory over a stub binary in both directions, since a fake
factory answers SupportsUnattended from a field and would prove nothing about
where the floor lives.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The confirmed-exit gate asked HasExited off _current and then disposed a second
read of the same volatile field. Two reads can observe different values, so the
gate could judge a process this method never disposed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… record the probe

A model-layer refusal is not containment evidence, so this runs a real agy
turn under the production per-launch HOME and observes the filesystem and the
process table: no kcap hook fired for the conversation, no watcher process,
agy's Library writes landed inside the relocated HOME, and the operator's own
agy conversation store gained nothing. Gated on KCAP_ANTIGRAVITY_REVIEWER_LIVE
plus a project, so CI skips in ~15ms.

The positive control earned its place immediately: the first live run failed
with containment working perfectly, because a default EnumerationOptions skips
Hidden and .NET calls every Unix dot-entry hidden — hiding the entire .gemini
subtree, which is where agy writes all of its conversation state.
Adds the gated live cert for the unattended Antigravity reviewer, driving
the production launch factory against a real agy.

Two cases. The first is the positive control: one review-flow launch whose
round completes with nothing to answer, asserted as a terminal result
rather than a returned launch -- StartAsync resolves at turn 1's init, so a
reviewer that reported a conversation and then died mid-turn would satisfy
a launch-only assertion.

The second is the load-bearing one. agy has no long-lived process, so a
multi-round review is separate invocations resumed with --conversation.
That shape is invisible from a single round and regresses silently: the
review still works, it just lands as one kcap session per round. The cert
pins all three observable halves -- one stable conversation id, a distinct
pid per round, and the resume flag adjacent to that id -- because each
alone is satisfiable by a broken build.

The turn-source seam only observes: it records the psi and then constructs
exactly what production's default constructs, so the argv assertions run
against the vector the OS actually received. The version probe and binary
resolution are left to production, since seaming the floor would certify a
build the gate would have refused.

Gated behind KCAP_ANTIGRAVITY_REVIEWER_LIVE=1, with the skip as the first
statement executed. Past the gate the harness fails loudly on a missing
GOOGLE_CLOUD_PROJECT rather than skipping: the launch path inherits the ADC
variables rather than re-stamping them, so its absence presents downstream
as a launch timeout that names the wrong culprit.

README: the reviewer section already stated the agy-CLI-not-IDE
prerequisite, the opt-in, the version floor and the ADC setup correctly.
Corrected one claim -- "no affirm verb" -- since the verb does exist and
refuses for this vendor, so an operator who tries it meets a coded error
the docs did not mention.
…ent OS

The Windows CI leg failed two tests that have nothing to do with Windows:
A_missing_binary_is_withheld_with_the_path_it_looked_for and
A_below_floor_build_is_refused_at_the_launch_boundary_too both got the platform
refusal, because the ladder short-circuits on UnsupportedPlatform before reaching
the arm each of them asserts.

This is the second time this repository has paid for the same mistake.
AntigravityReviewerCapability.Decide already takes the platform as a parameter,
and its comment records why: an earlier Kiro revision read the ambient OS inside a
method claiming to be pure, and a dozen consent and version tests short-circuited
on the Windows leg. Collapsing the factory's ladder onto that decision reintroduced
the ambient read one layer up, where no test could reach past it.

The seam also makes the Windows arm itself assertable from POSIX, which it was not
before, so it now has a test rather than only a CI leg.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The house rule says to use JsonElementExtensions instead of checking a JSON
value kind, and AntigravityNdjson.TryParseLine's top-level guard did not.
The helper genuinely had no primitive for it: Str/Num/Obj/Arr all answer
about a NAMED PROPERTY, so a caller asking whether the element itself is an
object -- a document root, typically -- had nothing to reach for and dropped
to a raw comparison. Six other places in this repository did the same, which
is what makes this a missing primitive rather than one call site's style.

So IsObject is added and the four existing accessors are expressed through
it, keeping one definition of the concept. The other six sites are left
alone deliberately; they are unrelated to this change.

The guard also turned out to be untested: deleting it left the whole suite
green, because the four existing parse cases are a blank line, a non-JSON
line and two objects -- none of them valid JSON that is not an object. That
case now has a test, and it is the distinction that matters at the read
loop: null means "nothing to read" and is dropped, while Unknown is a real
event a future handler could act on.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ProcessTurnAsync latched _turnExitConfirmed from process.HasExited and only
then called process.DisposeAsync() -- which kills the process tree. So a turn
whose child was still running at teardown recorded "unconfirmed" even though
this very method went on to kill it, and two consequences gated on that flag
were skipped: the durable PID record was left naming a dead process, and the
per-launch HOME -- the reviewer's conversation JSONL, meaning the caller's
diff, source excerpts and findings -- was left on disk until the next
daemon-epoch sweep.

The fail-safe direction is right and is unchanged: unconfirmed still means
SKIP the deletion, never force it, because deleting under a live reviewer
leaves it writing into an unlinked path. The bug was that the question was
asked too early, so the fail-safe fired on children that had in fact exited.
The teardown now terminates a still-running child explicitly, bounded, and
reads HasExited off that post-termination observation -- which can only ever
turn a NO into a YES, so a genuine survivor still reports unconfirmed. The
sibling test whose child ignores the kill pins that direction and stays green.

The early read stays early for the reason it always was: a disposed process
object reports HasExited true, so asking after disposal manufactures the
confirmation. The bound is 2s, deliberately under DisposeAsync's 5s
turn-worker join budget, since this wait runs inside the worker and a longer
one could push a worker that was about to join past that budget -- producing
the very unconfirmed outcome it exists to avoid.

AgyTurnProcess.DisposeAsync deliberately does NOT wait after its own kill.
That was tried: Kill(entireProcessTree: true) is SIGKILL on POSIX, which no
child can catch or defer, so the mutant deleting the wait survived 6 of 6
runs against a real child. An unpinnable guard is worse than none, so the
contract is written down instead -- disposal is not a source of exit
evidence, and a caller that needs one terminates first. The real-process test
pins what disposal does owe its caller: it kills a running child, which
matters when every review round is its own process.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… a bound

Two review findings, both about honesty rather than behaviour.

The real-process disposal test asserted HasExited on the very next line after
disposal. SIGKILL is delivered asynchronously and disposal does not block for it,
so that read races the kernel and a loaded CI box can lose. It passed 14
consecutive local runs, which is exactly the sample size that makes this class of
flake look solved — and this branch has already been reddened twice by
load-dependent timing. It now waits on the observer handle first, deliberately not
on the child handle, which disposal has already closed. The assertion is unchanged,
so a mutant that skips the kill still fails it.

The TurnExitForceGrace comment claimed a 2s wait "can never" push the turn worker
past DisposeAsync's 5s join budget. That is not something it can guarantee: the
worker may already have spent arbitrary time in the turn, and the budget runs from
DisposeAsync's call rather than from that line. Reworded to claim only that this
wait will not itself be what blows the budget, and to record that overshooting is
not a correctness failure — it degrades to the unconfirmed path, which skips the
HOME deletion and leaves the epoch sweep to collect it, which is the direction the
whole gate is built to fail in.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The reviewer version gate became a minimum rather than an exact match twice in
parallel. One of those was a floor read from configuration
(KCAP_ANTIGRAVITY_MIN_CLI_VERSION) and compared through the certification
gate's range grammar; the other was the shared ReviewerVersionAffirmations that
Kiro and Gemini already use. Keeping both would have left two answers to what
"this build clears the bar" means, and the configuration one directly
contradicts why the shared record is state at all: a floor an operator can set
from a shell profile is re-affirmed by their dotfiles rather than by them.

Antigravity is now a third citizen of the shared mechanism, with the same seven
decision arms as Kiro and the same remedy texts adapted to its own wording. Its
own gate keeps what is genuinely its own: the consent flag, the POSIX-only
refusal (its per-launch HOME holds review context and cannot be owner-only on
Windows), and the binary-missing arm the factory adds around the decision. The
config key, its env binding and the whole non-affirmable table are gone, so
`kcap daemon reviewer affirm --vendor antigravity` works like the others, and
the daemon seeds the record from the consent event beside Kiro's and Gemini's.

The platform seam stays a parameter at both layers — the gate and the factory —
because the Windows CI leg has twice gone red on an ambient OS read. The
ambient-reading Decide overload is deleted rather than left unused, since it
was the obvious thing to reach for next.

The record is read per decision, never snapshotted: the affirm verb runs in a
different process while the daemon is live.
@realtonyyoung
realtonyyoung force-pushed the claude-tyoung/ai-1414-agy-unattended-reviewer branch from 7c34eee to cae322b Compare August 7, 2026 11:50
ReviewerRefusal calls Decide with both versions null as a probe-free pre-check and
treats VersionUnresolved as "consent and platform are fine, keep going" — the
binary-missing check and the version probe both sit after it. That arm depends on
ReviewerVersionAffirmations.Decide testing the installed version before the
minimum, which is a property of a shared type in another assembly and was pinned
nowhere.

Reorder those two null checks — the natural "no minimum recorded, skip the
comparison" impulse — and the pre-check refuses before ever looking for the binary,
so a daemon with no agy installed is told its minimum is missing. Both halves still
read as correct on their own, which is what makes it worth a test rather than a
comment.

Mutation-verified: swapping the two lines in the shared type reds this test and
only this test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@realtonyyoung
realtonyyoung merged commit df3aebb into main Aug 7, 2026
6 checks passed
@realtonyyoung
realtonyyoung deleted the claude-tyoung/ai-1414-agy-unattended-reviewer branch August 7, 2026 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant