Skip to content

fix(antigravity): honor explicit CLI overrides - #3847

Closed
steipete wants to merge 1 commit into
mainfrom
triage/20260921-antigravity-scoped
Closed

steipete wants to merge 1 commit into
mainfrom
triage/20260921-antigravity-scoped

Conversation

@steipete

Copy link
Copy Markdown
Owner

An unusable ANTIGRAVITY_CLI_PATH could fall through to another agy and launch it during a background refresh. Antigravity now treats any explicitly set override as authoritative: an empty, missing, or non-executable target stops CLI discovery. Unsetting the variable restores normal discovery; other providers retain their existing behavior.

Adopts the override fix and resolver coverage from @hhh2210's #3689. The shared resolver's final optional return is simplified without changing behavior, keeping production code net negative. Documentation separates CLI-only quota refresh from Add Account's OAuth-client requirement. The broader diagnostic-category change from #3689 is not adopted, and saved-account credential staging remains unresolved.

Verification

  • Red on unchanged production code: CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --jobs 2 --filter AntigravityBinaryLocatorTests — 3 tests, 4 failed assertions across all invalid-override cases; both positive-control tests passed.
  • Green: CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --jobs 2 --filter 'Antigravity|PathBuilderTests|ArkcliBinaryLocatorTests|KiroBinaryLocatorTests|ProviderArchitectureGatekeeperTests'572 tests in 45 suites passed; portable target: 18 tests in 3 suites passed; 2 opt-in screenshot tests skipped, 0 failures. Includes the override regression, fallback controls, Antigravity warm/account guards, and architecture gatekeeper.
  • make check — passed; Found 0 violations, 0 serious in 2548 files.
  • git diff --check — passed.
  • Independent Codex autoreview — scoped clean through P2, no actionable findings.

Synthetic fixtures only; no live accounts, browser cookies, Keychain reads, or running app relaunches. Full sharded tests were excluded for the shared triage host.

Production: 1 file changed, 8 insertions(+), 9 deletions(-) (net -1). Tests: 2 files changed, 114 insertions(+), 1 deletion(-) (net +113).

Closes #3689
Refs #3780
Refs #3662
Refs #3146

Treat an unusable ANTIGRAVITY_CLI_PATH as authoritative so background
refreshes cannot discover and launch a different agy. Preserve other
providers' lookup behavior and simplify the final optional fallback.

Adopt the scoped override fix from #3689, with synthetic red-to-green
regressions, documentation, and the 0.64.1 release note. Document current
CLI-only quota behavior separately from desktop-free OAuth setup (#3146).

Co-authored-by: hhh2210 <hzy2210@gmail.com>
@clawsweeper

clawsweeper Bot commented Sep 22, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 22, 2026
@clawsweeper

clawsweeper Bot commented Sep 22, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 22, 2026, 1:32 AM ET / 05:32 UTC.

ClawSweeper review

What this changes

The branch makes explicit Antigravity CLI paths authoritative, adds resolver regression coverage, and clarifies CLI discovery and OAuth login documentation.

Merge readiness

Ready for maintainer review

The scoped fix remains useful: current main and v0.64.0 still allow invalid overrides to fall through. No actionable patch defect was found, and the owner explicitly adopts the documented compatibility change.

Priority: P2
Reviewed head: b275fcd7ab27c72484a1c49b562a3abdef2c7752

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused patch with appropriate regression coverage, an explicitly adopted compatibility tradeoff, and no actionable correctness finding.
Proof confidence 🌊 off-meta tidepool Not applicable: This OWNER-authored PR is exempt from the external-contributor runtime-proof gate. Its reported synthetic tests exercise executable resolution; no live setup run is claimed. The patch changes no stored-data contract or cross-principal authority.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This OWNER-authored PR is exempt from the external-contributor runtime-proof gate. Its reported synthetic tests exercise executable resolution; no live setup run is claimed. The patch changes no stored-data contract or cross-principal authority.
Evidence reviewed 7 items Policy and review scope: Read the complete root AGENTS.md. No nested AGENTS.md or maintainer-notes directory exists. Applied focused-test, provider-isolation, and credential-safe inspection guidance; no builds, tests, account probes, or autoreview helpers were executed.
Pinned introduced change: The complete introduced diff contains five files. The Antigravity-only guard returns before shared discovery when an override is present; the shared minimal-fallback simplification preserves its optional result. The verified test merge has the pinned main and head parents and the same tree as the head.
Still necessary on main and latest release: Inspected pinned main and v0.64.0: both delegate Antigravity resolution directly to the shared resolver, which continues discovery after an unusable override. The requested guard is therefore absent from both inspected versions.
Findings None None.
Security None None.

How this fits together

CodexBar resolves the Antigravity executable before fetching usage through an existing local CLI server or launching a CLI process. Environment overrides and automatic discovery determine which executable that refresh can use.

flowchart TD
  A[Usage refresh] --> B{Explicit CLI path set?}
  B -->|Yes| C{Executable usable?}
  C -->|No| D[Skip CLI source]
  C -->|Yes| F[Fetch CLI quota]
  B -->|No| E[Automatic executable discovery]
  E --> F
  F --> G[Usage result]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +8/-9 (net -1); tests +114/-1 (net +113) The provider-specific guard adds focused coverage without net production growth.

Technical review

Best possible solution:

Retain the narrowly scoped authoritative override guard, its recovery guidance, and the existing account and executable-identity protections.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: on pinned main, an invalid explicit path plus an available ambient agy reaches fallback discovery. The new regression fixtures model that path; this read-only review did not execute it.

Is this the best way to solve the issue?

Yes. The provider-specific early return implements the owner's chosen override contract without changing other providers or duplicating the shared discovery pipeline.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against b99a91694d78.

Labels

Label changes:

  • add P2: This is a bounded Antigravity discovery fix without evidence of an urgent widespread outage.
  • add merge-risk: 🚨 compatibility: Invalid explicit overrides stop recovering through ambient discovery; the owner explicitly adopts this tradeoff and documents correction or removal of the variable.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This OWNER-authored PR is exempt from the external-contributor runtime-proof gate. Its reported synthetic tests exercise executable resolution; no live setup run is claimed. The patch changes no stored-data contract or cross-principal authority.

Label justifications:

  • P2: This is a bounded Antigravity discovery fix without evidence of an urgent widespread outage.
  • merge-risk: 🚨 compatibility: Invalid explicit overrides stop recovering through ambient discovery; the owner explicitly adopts this tradeoff and documents correction or removal of the variable.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This OWNER-authored PR is exempt from the external-contributor runtime-proof gate. Its reported synthetic tests exercise executable resolution; no live setup run is claimed. The patch changes no stored-data contract or cross-principal authority.

Evidence

What I checked:

  • Policy and review scope: Read the complete root AGENTS.md. No nested AGENTS.md or maintainer-notes directory exists. Applied focused-test, provider-isolation, and credential-safe inspection guidance; no builds, tests, account probes, or autoreview helpers were executed. (AGENTS.md:1, b275fcd7ab27)
  • Pinned introduced change: The complete introduced diff contains five files. The Antigravity-only guard returns before shared discovery when an override is present; the shared minimal-fallback simplification preserves its optional result. The verified test merge has the pinned main and head parents and the same tree as the head. (Sources/CodexBarCore/PathEnvironment.swift:131, b275fcd7ab27)
  • Still necessary on main and latest release: Inspected pinned main and v0.64.0: both delegate Antigravity resolution directly to the shared resolver, which continues discovery after an unusable override. The requested guard is therefore absent from both inspected versions. (Sources/CodexBarCore/PathEnvironment.swift:120, 3703a72b6b66)
  • Production callers preserve the boundary: CLI availability and fetch both resolve the executable before warm discovery, version probing, managed spawning, or print-report execution. Existing same-user, executable-identity, and selected-account checks remain unchanged. The Gemini caller only uses resolution to select migration guidance. (Sources/CodexBarCore/Providers/Antigravity/AntigravityProviderDescriptor.swift:496, b275fcd7ab27)
  • Accepted compatibility tradeoff: The captured OWNER-authored PR body explicitly adopts authoritative invalid overrides from fix(antigravity): honor CLI overrides and explain excluded reports #3689 and states that unsetting the variable restores discovery. Existing installations relying on fallback from an invalid override will lose that CLI source until the variable is corrected or removed; this exact change is intentionally adopted and documented. The broader diagnostic-category proposal and saved-account credential staging are explicitly excluded. (docs/antigravity.md:20, b275fcd7ab27)
  • Regression coverage and reported validation: The new suite covers four invalid override values, a valid override, absent-override discovery, minimal fallback hit/miss, and another provider's unchanged fallback. The captured body reports red-to-green focused tests, 572 tests in 45 suites, 18 portable tests, and clean format/lint checks; it explicitly excludes full sharded tests and live account runs. These are contributor-reported results, not reviewer executions. (Tests/CodexBarTests/AntigravityBinaryLocatorTests.swift:5, b275fcd7ab27)

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)
  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

steipete added a commit that referenced this pull request Sep 22, 2026
Treat an unusable ANTIGRAVITY_CLI_PATH as authoritative so background
refreshes cannot discover and launch a different agy. Preserve other
providers' lookup behavior and simplify the final optional fallback.

Adopt the scoped override fix from #3689, with synthetic red-to-green
regressions, documentation, and the 0.64.1 release note. Document current
CLI-only quota behavior separately from desktop-free OAuth setup (#3146).

Co-authored-by: hhh2210 <hzy2210@gmail.com>
(cherry picked from commit b275fcd)
steipete added a commit that referenced this pull request Sep 22, 2026
Treat an unusable ANTIGRAVITY_CLI_PATH as authoritative so background
refreshes cannot discover and launch a different agy. Preserve other
providers' lookup behavior and simplify the final optional fallback.

Adopt the scoped override fix from #3689, with synthetic red-to-green
regressions, documentation, and the 0.64.1 release note. Document current
CLI-only quota behavior separately from desktop-free OAuth setup (#3146).

Co-authored-by: hhh2210 <hzy2210@gmail.com>
(cherry picked from commit b275fcd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant