feat: add Antigravity runtime provider - #2375
Conversation
yuezengwu
left a comment
There was a problem hiding this comment.
Reviewed exact head 6acfd8009535f25e8bbce10b1b06f1ddb05ed387.
Requesting changes for the following correctness blockers:
-
[P1] A timed-out turn can replay after a mutating tool already ran.
handleEventrecords non-read-only tool calls instate.sawUnsafeTool, andsettleFailurecorrectly converts that state toreplaySafety: "unsafe". However, the timeout/cancellation branch atpackages/client/src/providers/antigravity/index.ts:672-674bypassessettleFailureand unconditionally callstoken.retry(...). Ifagyexecutes a mutating tool and then stalls or misses the terminal result, First Tree resends the original delivery. On an initial turn the observed conversation ID is not adopted until the success branch, so the retry can start a fresh conversation and duplicate the external side effect. Route timeout/provider abort through the replay-safety settlement path (while keeping explicit user suspend semantics separate if necessary), preserve the exact conversation identity when it is safe to do so, and add deterministic coverage for timeout after a mutating tool, pre-provider timeout, and resumed identity/no duplicate delivery. -
The current head has reproducible PR-attributable test failures. Local exact-head runs reproduce three failures in
apps/cli/src/__tests__/capability-refresh.test.ts: the nominally all-healthy snapshots omitantigravity, so the refresher keeps polling. The focused client run also has two failures:capability-probes.test.tscontains duplicateantigravityexpectation entries andcapability-reprobe.test.tsomits the provider from the healthy snapshot. The same failures appear in Test CLI (1/2) and Test Client & Web. Please update the complete-provider fixtures and restore a green exact-head run.
There is also an unresolved accounting contract that needs evidence before this provider can be accepted: First Tree token_usage events are per-turn deltas (packages/shared/src/schemas/session-event.ts:107-128), while the handler forwards the terminal Antigravity usage snapshot verbatim on every resumed turn (index.ts:707-716). Google documents terminal usage as cumulative for a conversation session. Either compute a delta keyed to the exact conversation, or add live multi-turn evidence proving --conversation returns process-local rather than conversation-cumulative counters and encode that behavior in tests.
Verification performed on this exact head:
git diff --check 47094ba6a014555294d446a7324a089f5eb6b709...6acfd8009535f25e8bbce10b1b06f1ddb05ed387— pass.- Focused Antigravity/client suite — 145 passed, 2 failed.
- CLI capability refresher suite — 19 passed, 3 failed.
agyis not installed on the review host, so live auth/start/resume/tool/MCP/Skills/process-lifecycle behavior was not validated.
This review does not authorize merge. Because this introduces an external CLI/install path, host-local Google identity, MCP credentials/headers, Skills projection, and --dangerously-skip-permissions, an exact-head full-isolated live QA run plus human architecture/security/supply-chain acceptance is still required after the code blockers are resolved.
|
Follow-up for the review findings, now on the current PR head:
Validation on this head: Antigravity tests 25/25, capability fixture tests 100/100, full repository typecheck 9/9 tasks, and repository Biome check pass. The local full client suite still has six unrelated host-sensitive baseline failures (broker HOME/managed-skill permissions and a long authority-boundary timeout); the review host also does not have |
|
All requested code and fixture blockers are now addressed on the current PR head, and the complete GitHub CI suite is green. Please re-review the current head when available; the remaining merge block is the earlier review being attached to the superseded commit. |
|
Additional accounting hardening is now on the current head:
The focused Antigravity handler suite, client typecheck, Biome, and diff checks pass. Please review the current head together with the previously posted fixes. |
yuezengwu
left a comment
There was a problem hiding this comment.
Fresh-head re-review of exact head 231ceea7613a2c2bb67999b5faec2e1b5256d275.
The original timeout replay-safety, complete-provider fixture, and usage-baseline findings are fixed on this head. One P1 lifecycle blocker remains:
suspend() / shutdown() still bypass exact-conversation adoption and replay-safe settlement. Both lifecycle methods set sessionActive = false and increment generation before aborting the process. Consequently, after runProcess() returns, the branch at packages/client/src/providers/antigravity/index.ts:738-741 always wins for a lifecycle cancellation. That branch unconditionally calls token.retry(...); it does not call adoptObservedSessionId(...) and does not inspect state.sawUnsafeTool.
On a first Turn where agy has already emitted one exact conversation ID and a mutating tool step, this discards both pieces of evidence. start() then returns an antigravity-pending-* synthetic ID, and a later resume of that pending ID clears providerSessionId and starts a fresh provider conversation. Redelivery can therefore repeat an external side effect.
I reproduced this deterministically on the current head with a controlled provider stream: emit init(conversation-lifecycle), emit a mutating run_command, then call handler.suspend(). Observed result: synthetic pending session ID, token.retry("antigravity_turn_aborted_or_timed_out"), no replaceSessionId, and no unsafe consumed completion.
Please make lifecycle cancellation preserve the single exact observed conversation identity and prevent replay after mutating provider activity, while still rejecting late callbacks after the lifecycle fence/quarantine. Add deterministic first-Turn suspend and shutdown coverage after init + mutating tool that proves recovery cannot start a fresh conversation or duplicate delivery.
Validation on this exact head:
- focused client/Antigravity and retry/capability suites: 152/152 passed;
- CLI capability refresher: 22/22 passed;
- shared runtime provider catalog: 9/9 passed;
- client typecheck: passed;
- client/shared builds and
git diff --check: passed; - complete GitHub checks: green.
This review does not authorize merge. agy is still absent on the review host, so there is no live evidence for Google auth/start/resume, tool execution, MCP credential/header handling, Skills projection, or process lifecycle. The external CLI/install path, Google host identity, managed MCP/Skills, and --dangerously-skip-permissions remain a major architecture/security/permissions/supply-chain hard stop requiring full-isolated live QA and human acceptance after the code blocker is fixed.
|
Fresh-head review follow-up for the lifecycle P1:
Validation on this head: Antigravity handler suite 9/9, related client capability suites 106/106, CLI capability refresher 22/22, client typecheck, Biome, and |
yuezengwu
left a comment
There was a problem hiding this comment.
Requesting changes on exact head cbe3f0a859b39a88b889f9e1cfaa52887cd15b21.
[P1] Preserve ACK-none custody during preemption and route retirement
The new lifecycle branch in packages/client/src/providers/antigravity/index.ts adopts the exact conversation ID, which fixes the previous synthetic-ID problem. However, when a mutating tool was observed it now always calls settleFailure(...) after any lifecycle cancellation and terminally completes the token as consumed. suspend(reason) and shutdown(reason) do not accept or inspect HandlerShutdownOptions.
That violates the existing AgentHandler custody contract: only operator suspend and full graceful drain pass { settleProviderEntered: true }; forced concurrency preemption and route retirement deliberately leave it unset so provider-entered work stays recoverable (ACK-none). The scheduler also marks working-session preemption with ackConsumedPrefix: false. As written, Antigravity can therefore ACK and discard interrupted mutating work during preemption/retirement instead of preserving recovery custody.
The new lifecycle tests currently encode this regression: they call plain handler.suspend(...) / handler.shutdown(...) with no settlement option and expect an unsafe_replay consumed completion. Please make settlement depend on opts?.settleProviderEntered === true, preserve the exact conversation identity, and add deterministic coverage that distinguishes:
- operator suspend / graceful shutdown with the flag: terminal consumed settlement;
- plain concurrency suspend / route-retire shutdown: no completion/ACK and recoverable custody;
- exact-conversation recovery without duplicate prompt or tool delivery.
Focused validation on this head: Antigravity/client tests 154 passed; CLI capability tests 22 passed; shared provider catalog tests 9 passed; the three existing Pi custody-reference tests passed; client typecheck and git diff --check passed. These green tests do not remove this contract mismatch.
Separate release hard stop: agy is not installed on this host, so there is still no live Google auth/start/resume/tool/MCP/Skills/process-lifecycle evidence. The official CLI documentation also states that --dangerously-skip-permissions approves all tool calls, including file writes and command execution. Even after the code blocker is fixed, this external CLI/identity/credential/projection/permission boundary needs full isolated live QA plus yuezengwu human architecture, security, permissions, and supply-chain acceptance. This review is not approval to merge.
|
Fresh-head follow-up for the custody-contract review:
Validation on this head: Antigravity handler suite 11/11, client typecheck, Biome, and |
yuezengwu
left a comment
There was a problem hiding this comment.
Fresh-head re-review of exact head c7a9c03c8f6988e2fd15ef66ef4b93fb1372b832.
The new option-aware handler branch is directionally correct: settleProviderEntered: true now performs terminal unsafe settlement, while plain suspend/shutdown leaves the DeliveryToken retryable. However, one P1 remains at the real SessionRuntime boundary:
[P1] First-turn preemption still loses the exact Antigravity conversation and can replay the mutating delivery in a fresh conversation.
On lifecycle cancellation, the handler records the observed ID in pendingLifecycleSessionId and returns it from start() (packages/client/src/providers/antigravity/index.ts:744-769,1061-1081). But working-session preemption first invalidates the route transition. When the canceled start() later returns, SessionRuntime.routeMessage() hits the stale-transition guard and calls discardStaleRouteTransition() before assigning entry.claudeSessionId (packages/client/src/runtime/session-runtime.ts:2320-2325). The canceled unestablished entry is then dropped (:3064-3074,3124-3133), and this path never records an eviction resume mapping. The recovered inbox row therefore has no exact provider session handle and the replacement handler starts a new Antigravity conversation. The existing generic runtime regression at session-manager-edge-coverage.test.ts:4527-4573 explicitly expects that fresh start() behavior after a stale canceled producer.
The new handler test does not cover this boundary. It directly reuses the same handler and calls resume() with a different synthetic message, m-recovery / recover without replay (handler.test.ts:566-577), rather than allowing SessionRuntime/coordinator recovery to reset and redeliver the original inbox row. Its assertion that the original prompt is absent is therefore vacuous. In production, token.retry(messages, ...) preserves the original row for recovery; even where an established exact ID survives, sending that original mutation prompt into --conversation again is not evidence of no duplicate provider/tool delivery.
Please add a deterministic SessionRuntime-level test that exercises both working concurrency preemption and forced route retirement during a first Antigravity turn after init + a mutating tool event. It should prove: no ACK, server-faithful recovery of the same original inbox row, exact conversation identity retained across retired and replacement handlers, and no duplicate prompt/tool delivery. The implementation needs a provider-safe recovery/continuation contract rather than testing a manually substituted message.
Fresh-head validation:
- focused Antigravity/retry/capability suites: all passed; the initial 5 s aggregate run had one host-sensitive Codex resolver timeout, and its full 59-test file passed with a 15 s timeout;
- runtime edge/start suites: 162/162 passed;
- CLI capability: 22/22; shared provider catalog: 9/9;
- repository typecheck: 9/9 tasks; serial repository build: 5/5 tasks;
- Biome completed with existing warnings;
git diff --checkpassed; - all GitHub checks are green.
This review still does not authorize merge. agy is absent on this host, so there is no live Google auth/start/resume/tool/MCP/Skills/process-lifecycle evidence. Official CLI behavior also makes --dangerously-skip-permissions an all-tool permission grant. The external installer/CLI, Google host identity, MCP credentials/headers, Skills projection, and permission boundary still require full-isolated live QA plus yuezengwu human architecture, security, permissions, and supply-chain acceptance.
|
Follow-up for the fresh-head lifecycle review, now on
Validation on this head:
Please fresh-review the current head. Live |
yuezengwu
left a comment
There was a problem hiding this comment.
Fresh-head review of exact head 9e7ed113e68ee8803cba2cf328778cb73d1f9bb5.
Requesting changes for two remaining custody/replay blockers and the current required-check regressions:
-
[P1] The provider continuation is a new user turn, not an interrupted-turn resume primitive.
ANTIGRAVITY_CONTINUATION_PROMPTexplicitly acknowledges that Antigravity has no documented operation for resuming an interrupted turn, thenresume()passes that text throughrunTurn()as a new prompt (packages/client/src/providers/antigravity/index.ts:73-81, 1135-1154). Google's headless CLI contract says--conversationstarts a new process, prompts areuserevents, and each prompt executes a full turn; it also rejectscontrol_request/control_responserather than offering an interrupt-resume control (official docs). Therefore this prompt can only advise the model not to repeat prior tool effects; it cannot prove that an already-executed tool whose terminal event was lost will not be executed again. The new tests mock the recovered process and return success without asking a real provider to reconcile an interrupted tool, so the single observed tool event is a fixture property, not an exactly-once guarantee. Please keep the original process/turn alive, make provider-entered turns ineligible for forced retirement until they settle, or use a provider-supported authoritative continuation/idempotency boundary; a natural-language instruction is not sufficient custody proof. -
[P1] Provider-turn retries still drop the continuation and resend the original delivery. When a retryable failure has
sawProviderActivity,settleFailure()retries the token and callsfailSessionForRecovery()(index.ts:652-667). That runtime method persists onlyclaudeSessionId, notentry.providerContinuation(packages/client/src/runtime/session-runtime.ts:1784-1800). The recovered inbox row therefore reachesresume()withoutopts.continuation, takes the normal branch atindex.ts:1147-1149, and serializes the original user content into the same conversation again. This bypasses the new lifecycle-only continuation path and recreates the duplicate-delivery risk for transient/provider-capacity failures after aninitor other activity. Carry exact-message continuation custody through this recovery path as well, and add a SessionRuntime regression for provider activity -> retryable failure -> exact-row recovery that proves the original content is not submitted twice and no ACK occurs before successful recovery. -
The exact head fails the required Client test job. Both GitHub Test Client & Web and a clean local full Client run report the same two failures:
contracts-entry.test.tsrejects the unallowlistedHandlerResumeOptions/ProviderContinuationexports, andsession-runtime-authority-boundary.test.tsreportsSessionProjectionAuthority.activateLiveSessionreturning private nested-containerevictedMappings(ledger-identity-escape). Please restore both contract/ownership guardrails rather than treating them as incidental CI failures.
Fresh validation performed:
- full Client suite: 219 files passed, 2 failed; 2881 tests passed, 2 failed, 7 skipped;
- focused Antigravity/SessionRuntime suite: 200/200 passed;
- repository typecheck: 9/9 tasks passed; Client typecheck/build passed;
- repository Biome check and
git diff --checkpassed; agyis absent on this review host.
This review does not authorize merge. Even after the code blockers are fixed, the external agy install/CLI, host-local Google identity, MCP credentials/headers, managed Skills projection, and --dangerously-skip-permissions boundary still require an exact-head isolated live QA run plus yuezengwu's human architecture, security, permissions, and supply-chain acceptance.
|
Pushed dfa64a2 as a narrow CI-contract repair on top of reviewed head 9e7ed11. This commit only addresses the two confirmed blockers:
Validation on the exact repaired head:
Scope remains intentionally narrow. This does not resolve or approve the larger Antigravity continuation/exactly-once architecture questions, and it does not lift any architecture-review or live-QA hard stop. |
|
Live follow-up: GitHub CI for dfa64a2 has now completed successfully, including Test Client & Web and the aggregate Test job. The PR remains blocked / changes requested; this CI repair does not supersede the required fresh-head architecture review or live QA. |
yuezengwu
left a comment
There was a problem hiding this comment.
I re-reviewed exact head dfa64a2007ef784cead04fedada594b8e73444d0 from zero. The narrow CI repair is correct: the public contract allowlist/occupancy now includes HandlerResumeOptions and ProviderContinuation, and activateLiveSession() returns a field-by-field continuation snapshot instead of leaking the stored nested identity. The two prior CI failures are resolved locally and in GitHub Actions.
Two P1 correctness blockers remain unchanged because this commit does not modify the affected runtime paths:
-
The recovery instruction is a new natural-language user turn, not an authoritative interrupted-turn resume.
ANTIGRAVITY_CONTINUATION_PROMPTexplicitly acknowledges that Antigravity has no documented headless operation for resuming an interrupted turn without a new user event, andresume()sends that text through normalrunTurn(). The current headless protocol documents each stdin prompt as auserevent that executes a full turn. If a mutating tool completed before the terminal event was lost, “do not repeat” is advisory and cannot establish exactly-once behavior. The mock recovery tests only prove that their fixture chooses not to invoke the tool again. This needs either continued custody of the original live process/turn until a terminal outcome, or a provider-supported authoritative interrupted-turn/idempotency primitive—not a second prompt. -
A retryable failure after provider activity still drops continuation custody and redelivers the original request. In
settleFailure(),sawProviderActivitytakes the generic retry branch, callstoken.retry(...), thenfailSessionForRecovery(reason, providerSessionId).SessionRuntime.failSessionForRecovery()records only the provider session ID andlastActivity; it does not recordproviderContinuation. The recovered row therefore reachesresume()withoutopts.continuation, takes the normal branch, and serializes the original message again into the same conversation. Please propagate exact-message continuation through this provider-turn retry path and add a SessionRuntime-level regression covering: provider activity -> retryable failure -> same-row recovery, no second original prompt, and no ACK until recovery succeeds.
Validation on this exact head:
- focused runtime/contracts suite: 5 files, 223/223 passed;
- full
@first-tree/clientsuite: 221 files passed, 2 skipped; 2883 tests passed, 7 skipped; pnpm check,pnpm typecheck,pnpm build, andgit diff --checkpassed (existing non-failing lint/build warnings remain);- all current GitHub checks are green;
agyis not installed on this host, so there is still no exact-head live evidence for Google auth, start/resume, mutating tools, MCP/Skills projection, credential handling, or process lifecycle.
Separately from the code blockers, external agy installation/CLI, Google identity, managed MCP headers/credentials, Skills projection, and --dangerously-skip-permissions remain a major architecture/security/permissions/supply-chain hard stop. They require full isolated live QA and yuezengwu's explicit human acceptance before merge.
Protocol reference: https://www.antigravity.google/docs/cli/headless/
yuezengwu
left a comment
There was a problem hiding this comment.
Fresh-head review of exact head 3d0eb11880163ecd43f58fd6d25cd85fd467ed31.
The new provider-turn liveness fence is directionally correct: concurrency/LRU/idle retirement no longer interrupts an active Antigravity process, forced recovery retirement waits for the provider turn to settle, and the single-row provider-failure path now carries an exact ProviderContinuation. The previous contracts/authority CI regressions also remain fixed, and all current GitHub checks are green. Two P1 custody gaps remain:
-
The continuation is still an advisory second user turn, not an authoritative interrupted-turn or idempotency primitive.
ANTIGRAVITY_CONTINUATION_PROMPTexplicitly acknowledges the missing protocol operation, andresume()sends it through the normalrunTurn()path (packages/client/src/providers/antigravity/index.ts:75-83,1162-1181). Google's current headless contract says--conversationstarts a new process, every stdin prompt is auserevent that executes a full turn, andcontrol_request/control_responseterminate as unsupported. The new liveness fence fixes host-initiated preemption while the original process is alive, but a retryable provider failure after activity still replaces the ambiguous failed turn with this new natural-language turn. If the provider completed a tool effect before losing the terminal event, “do not repeat” cannot prove exactly-once behavior. The regression fixture makes the recovered mock return success without invoking the tool; that validates plumbing, not the provider guarantee. This path needs a provider-supported authoritative continuation/idempotency boundary, or it must fail closed instead of auto-continuing an ambiguous provider-entered turn. -
Coalesced queued turns still discard continuation custody and replay original content.
runQueued()combines every queued message into one provider prompt (index.ts:1023-1036), but both retry and lifecycle continuation construction are gated onmessages.length === 1(:667-681and:776-794). With two injected inbox rows, aninit/provider-activity event followed by a retryable failure callsfailSessionForRecovery(..., continuation = undefined). Recovery retains the conversation ID but has no exact-message continuation, soresume()takes its normal branch and serializes an original row into the same conversation again (:1174-1181). The new SessionRuntime regressions all use one row and do not exercise this production batch path. Please either stop coalescing provider-custody rows, define an authoritative aggregate continuation identity, or fail closed; add a two-row provider-activity -> retryable-failure regression proving no original content is resubmitted and no ACK occurs before a safe terminal outcome.
Fresh validation:
- focused Antigravity/SessionRuntime/provider-policy/contracts suite: 259/259 passed;
- full
@first-tree/clientsuite: 221 files passed, 2 skipped; 2,884 tests passed, 7 skipped; - Client typecheck/build and
git diff --check: passed; repository Biome completed with existing non-failing warnings/info; - all current GitHub checks: pass/skipping only;
agyis absent on this host, so no live Google auth/start/resume/tool/MCP/Skills/process-lifecycle validation was possible.
This review does not authorize merge. Independently of the code blockers, the external agy install/CLI, host-local Google identity, MCP credentials/headers, managed Skills projection, and --dangerously-skip-permissions all-tool grant remain a major architecture/security/permissions/supply-chain hard stop requiring exact-head isolated live QA plus yuezengwu's explicit human acceptance.
|
Follow-up for the two fresh-head P1s, now on exact head
Validation on this exact head:
CI has started for |
yuezengwu
left a comment
There was a problem hiding this comment.
Fresh-head review of exact head af3a7b0a741d6d69d0d49e33b3ecf40d7ff27cf8.
The two previous P1s are substantially addressed: the advisory natural-language continuation turn is removed, provider-entered failures are classified unsafe_replay, and the queue now preserves one inbox row per provider turn. The focused and full local client suites pass on this head.
One P1 fail-closed gap remains:
An unsafe turn is replayable when the required terminal notice cannot be posted. settleFailure() records ambiguity only in the handler-local ambiguousProviderTurnKeys and then calls token.complete(... consumed/unsafe_replay) (packages/client/src/providers/antigravity/index.ts:660-683). If posting the required runtime failure notice fails, SessionRuntime.completeDeliveryTurn() deliberately returns retry and moves the exact row back to recovery (packages/client/src/runtime/session-runtime.ts:1985-2003). The Antigravity handler then returns without persisting the ambiguity. A same-handler redelivery reaches the active-session inject() path, which never checks ambiguousProviderTurnKeys (index.ts:1192-1196); a replacement/restart is worse because shutdown() clears the set (:1214-1232). The recovered row can therefore be serialized into a new provider turn after the original provider attempt already had unobservable effects. The new tests all use a token whose complete() returns settled, so they do not exercise this required-notice/ACK-failure branch.
Please make the no-replay fence durable in the inbox/session custody path until the terminal notice and ACK settle, and apply it to every redelivery route (inject, resume, handler replacement, and restart). Add a SessionRuntime-level regression covering provider activity -> unsafe_replay -> terminal notice failure (or ACK retry) -> exact-row redelivery, proving no second agy process/prompt is started and the row can only retry the terminal disposition.
Fresh validation on this exact head:
- focused Antigravity/provider-policy/capability/contracts/runtime suite: 15 files, 416/416 passed;
- full
@first-tree/clientsuite: 221 files passed, 2 skipped; 2,885 tests passed, 7 skipped; - client typecheck, client build, and
git diff --checkpassed; worktree remained clean; - GitHub
Test ServerandTest Client & Webare currently failed on one timeout each; the client timeout passed in the full local run and neither failing file is changed by this PR, but the exact-head required checks are not green; agyis absent on this host, so there is no live Google auth/start/resume/tool/MCP/Skills/process-lifecycle evidence.
This review does not authorize merge. Independently of the code blocker, external agy installation/CLI, host-local Google identity, managed MCP credentials/headers, Skills projection, and --dangerously-skip-permissions remain a major architecture/security/permissions/supply-chain hard stop requiring exact-head isolated live QA plus yuezengwu's explicit human acceptance.
|
Follow-up for the unsafe-replay notice/ACK custody P1, now on exact head
Validation on this exact head:
The live evidence gap remains unchanged: this host has no |
|
All contributors are covered by the First Tree CLA. |
09ca454 to
38ec464
Compare
|
recheck |
|
Follow-up for the remaining non-P1 hardening, now on exact head
Local validation on this exact head:
GitHub CI is green for this exact head, including This does not lift the prior P1 custody review, merge authorization, or the required isolated |
Summary
agywith the documentedstream-jsonstdin/stdout protocol, preserve the provider conversation ID, and resume exact conversations with fail-closed protocol and identity checks.Validation
corepack pnpm typecheckcorepack pnpm check(passes; existing repository lint warnings remain)Notes
The implementation follows the official headless mode and resume command documentation. The local validation environment does not have
agyinstalled, so Google authentication and live provider execution remain host-level follow-up validation.