fix(daemon): release antigravity model lock on pre-spawn early exit - #5467
Open
tomsen02 wants to merge 1 commit into
Open
fix(daemon): release antigravity model lock on pre-spawn early exit#5467tomsen02 wants to merge 1 commit into
tomsen02 wants to merge 1 commit into
Conversation
The global antigravity model lock is acquired before buildArgs/spawn but its release is only handed off to the child-exit/watcher path after a successful spawn. Every early exit in between — the pre-spawn cancel check, the Windows command-line budget guards, a buildArgs throw, the missing-binary AGENT_UNAVAILABLE path, and a spawn failure — returned without releasing. A run cancelled while awaiting the lock therefore resumed once the prior holder released, became the holder, hit the cancel check, and returned without releasing — poisoning the chain so every later concrete-model antigravity run blocked forever. Add an idempotent, handoff-guarded release across the acquire->spawn region: each early exit calls it, and it becomes a no-op once ownership transfers to the child/watch path. This preserves the invariant that the lock is only handed off when the child lifecycle owns it, so it never reopens the settings.json cross-talk race (263fd2f) by releasing while a slow-cold-start agy still holds the file. Fixes nexu-io#5466
Contributor
|
Thanks @tomsen02 — the reproduction and red→green test seam are very clear here. I’ve queued this up for review and will follow along from here. |
Contributor
|
🧪 Queued for QA validation — this PR changes a live run/cancel path, so it’ll get a manual QA pass before merge. Nothing needed from you right now; we’ll update here once that validation happens. Thanks again for the thorough reproduction. 🙏 |
Contributor
Contributor
Author
Contributor
|
Thanks for the follow-up, @tomsen02. I’ve refreshed the review request on the current head and flagged this again for a human review pass. From what I can see, you do not need to rebase right now. |
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
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.
Fixes #5466
Why
Following up on the lock-ownership gap I reported in #5466. Maintainer confirmed the bug and prescribed the fix shape in the issue thread ("an idempotent early-exit release around the acquire→spawn region, guarded so it stops firing once ownership has been transferred to the child/watch path"); this PR implements exactly that.
The pain: for an Antigravity concrete-model run,
startChatRunacquires the process-global model lock beforebuildArgs/spawn, but only hands the release off to the child-exit/watcher path after a successful spawn. Every early exit in between returns without releasing:buildArgsthrow,AGENT_UNAVAILABLEpath,A run cancelled while it is still waiting on the lock resumes once the prior holder releases, becomes the holder, hits the cancel check, and returns without releasing. The chain is then poisoned: every later concrete-model Antigravity run blocks forever on
await acquireAntigravityModelLock(), and only a daemon restart recovers it.What users will see
No visible change on the happy path. The fix removes a hang: if you cancel an Antigravity run (with a concrete model like
Gemini 3.1 Pro (High)) while a previous Antigravity run is still holding the model lock, your next Antigravity run no longer hangs indefinitely with no output.Surface area
Bug fix verification
apps/daemon/tests/antigravity-model-lock-cancel-release.test.tsmain, green on this branch? yes. The spec drives three real runs over the production HTTP API (POST /api/runs,POST /api/runs/:id/cancel) against a PATH-injected fakeagy: A holds the lock (itsagystalls until agofile appears), B waits on the lock and is cancelled while waiting, A exits so B resumes and (pre-fix) poisons the lock, then C can never acquire it. Assertion: C reaches a terminal status.expected false to be true— C never terminal (lock poisoned).agy/test processes left behind.Validation
pnpm --filter @open-design/daemon typecheck(bothtsconfig.jsonandtsconfig.tests.json) — cleanpnpm guard— 78/78 passpnpm --filter @open-design/daemon exec vitest run tests/antigravity-model-lock-cancel-release.test.ts— green, 3/3run-retry-runtime+run-retry-policy— 22/22 passNotes
antigravityModelLockHandedOff): once ownership transfers to the watcher + child-exit fallback it becomes a no-op, so it never double-releases nor reopens thesettings.jsoncross-talk race (263fd2f) by releasing while a slow-cold-startagystill holds the file.antigravityModelLockReleaseis null), so other agents are unaffected.