Improve Antigravity retrieval: retired Flash alias and offline fallback - #3119
Improve Antigravity retrieval: retired Flash alias and offline fallback#3119Yuxin-Qiao wants to merge 4 commits into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 866938c57c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.hasSharedOAuthCredentials(context: context) | ||
| { | ||
| return [app, cli, ide, oauth] | ||
| return [app, cli, ide, oauth, offline] |
There was a problem hiding this comment.
In auto mode with selected, environment, or shared OAuth credentials, the offline strategy is placed after OAuth, but AntigravityOAuthFetchStrategy.shouldFallback always returns false. Consequently, expired credentials or any OAuth request failure terminate ProviderFetchPipeline.fetch before it considers existing offline conversation data, defeating the new fallback precisely when live probes and OAuth both fail.
Useful? React with 👍 / 👎.
| private func scheduleDebouncedTokenPublicationSync() { | ||
| self.sharedSpendDashboardObservationDebounceTask?.cancel() | ||
| self.sharedSpendDashboardObservationDebounceTask = Task { @MainActor [weak self] in |
There was a problem hiding this comment.
If an observed configuration change schedules the one-shot withObservationTracking re-registration and a Codex or independent-token publication arrives within the 250 ms debounce window, this shared task is cancelled. The replacement token-sync task only calls applySharedSpendDashboardConfiguration and never re-enters observeSharedSpendDashboardConfiguration, so subsequent settings or provider-configuration changes are no longer observed until the publisher is restarted; use separate debounce tasks or re-register observation in this path.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
P2
|
Codex review: needs real behavior proof before merge. Reviewed August 23, 2026, 6:06 AM ET / 10:06 UTC. ClawSweeper reviewWhat this changesThe PR adds Antigravity local-data fallback behavior and caps concurrent Codex spend-dashboard scans. Regression provenancePossible regression — suspected (reviewed change). No predecessor PR is attributed. Merge readiness⛔ Blocked until stronger real behavior proof is added - 7 items remain Keep open: the new concurrency limit drops completed Codex account results when more than three accounts are scanned. Priority: P1 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherCodexBar gathers provider usage through ordered fetch strategies for menu display and scans configured Codex accounts for the spend dashboard. Antigravity can fall back from live/OAuth probes to local metadata, while dashboard results are aggregated into account rows and totals. flowchart LR
A[Provider settings] --> B[Antigravity fetch pipeline]
B --> C[Live and OAuth probes]
C --> D[Offline conversation store]
D --> E[Menu usage snapshot]
F[Codex accounts] --> G[Bounded dashboard scans]
G --> H[Spend dashboard rows]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Retain every result consumed during bounded task admission, add a four-account regression test, and provide a redacted final-head offline-fallback trace. Do we have a high-confidence way to reproduce the issue? Yes. With four pending accounts, the fourth admission executes Is this the best way to solve the issue? No. The concurrency cap is appropriate, but it must retain admission-time results and receive a multi-account regression test. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 27c7f334e3c4. LabelsLabel changes:
Label justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (8 earlier review cycles)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79d2596eed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| updatedAt: Date(), | ||
| identity: ProviderIdentitySnapshot( | ||
| providerID: .antigravity, | ||
| accountEmail: AntigravitySelectedAccountGuard.selectedAccountEmail(context: context), |
There was a problem hiding this comment.
When an OAuth account remains selected while the data source is explicitly set to CLI, these conversation files still belong to the ambient HOME/GEMINI_CLI_HOME and contain no verified account identity, but the fallback stamps them with the selected OAuth account's email. If the local files were produced by another signed-in account, the menu therefore renders that account's conversation count under the selected account; leave accountEmail unset unless ownership can be derived from the offline store.
AGENTS.md reference: AGENTS.md:L46-L46
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
merge-risk: 🚨 auth-provider
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 988ba883db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.geminiHomeDirectory(home: home, env: env) | ||
| .appendingPathComponent("antigravity-cli", isDirectory: true) | ||
| .appendingPathComponent("conversations", isDirectory: true) |
There was a problem hiding this comment.
For the default agy layout, the repository's own process fixtures launch the server with --app_data_dir ~/.gemini/antigravity (Tests/CodexBarTests/AntigravityStatusProbeTests.swift:98-100), but this resolver searches the CLI installation tree at ~/.gemini/antigravity-cli/conversations. Consequently, existing conversation databases in the app-data directory never make this strategy available, so the new offline fallback is skipped unless users happen to duplicate data under the executable directory.
Useful? React with 👍 / 👎.
| lhs.hideNativeCodexCostWhenOpenCodexPresent == rhs.hideNativeCodexCostWhenOpenCodexPresent && | ||
| lhs.hiddenSourceIDs == rhs.hiddenSourceIDs && | ||
| lhs.preferredCurrencyCode == rhs.preferredCurrencyCode |
There was a problem hiding this comment.
When currency, hidden-source filters, or the native-Codex hide flag changes while a load is in flight, update takes the new display-only fast path, but these comparisons make sameSourceOwnership false. The running request then reaches handleBuiltRequest, treats the change as an ownership mismatch, and starts another load, including another 365-day Codex scan/provider capture; this defeats the advertised no-scan fast path precisely during initial or manual refreshes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f97bda00e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (index, account) in pendingAccounts.enumerated() { | ||
| group.addTask { |
There was a problem hiding this comment.
When a user has many visible managed/profile Codex accounts, this loop immediately creates one task per account, and each task performs a 365-day snapshot scan followed by an activity-cache load; a forced refresh can therefore launch an unbounded number of filesystem-heavy scans at once, saturating disk/CPU and making refresh latency and UI responsiveness worse than the previous sequential implementation. Limit the number of active account scans while preserving the configured result order.
Useful? React with 👍 / 👎.
…l data exists Fix P2 from Codex review on steipete#3119: AntigravityOAuthFetchStrategy.shouldFallback now returns true when offline conversation data is present, so expired credentials do not terminate the pipeline before AntigravityOfflineFetchStrategy.
|
Fixed P2: OAuth now falls back to offline when local data exists Change
Local proof (head 5106df4) $ swiftformat Sources/CodexBarCore/Providers/Antigravity/AntigravityProviderDescriptor.swift
0/1 files formatted.
$ swiftlint --strict
Done linting! Found 0 violations, 0 serious in 1974 files.
$ git log --oneline -2
5106df422 fix(antigravity): allow OAuth errors to fallback to offline when local data exists
0f97bda00 fix(test): seed pinned claude spend publication before first snapshotOffline store unit tests already exist ( @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
rating: 🦪 silver shellfish
…l data exists Fix P2 from Codex review on steipete#3119: AntigravityOAuthFetchStrategy.shouldFallback now checks hasOfflineData, so expired credentials do not block offline.
5106df4 to
175a92c
Compare
clawsweeper
Bot
removed
the
merge-risk: 🚨 session-state
…+ proof - Offline snapshot now has nil accountEmail (P1) - OfflineStore also counts $HOME/.gemini/antigravity and .../conversations (P2) - SpendDashboardController bounds Codex scans to 3 concurrent (P2) - Add AntigravityOfflineFallbackProofTests covering app-data and nil email
|
Added P1/P2 fixes + real behavior proof for dfada2c. See PR body for redacted trace. @clawsweeper re-review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfada2c73d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if pendingCount >= 3 { | ||
| _ = try await group.next() | ||
| pendingCount -= 1 |
There was a problem hiding this comment.
When four or more Codex accounts are pending, group.next() removes a completed result from the task group but discards its tuple. Each account admitted beyond the first three therefore drops one earlier account's snapshot and failure state, leaving only three results to contribute rows and totals regardless of how many scans succeeded. Store each throttling result in results before launching the next task, or otherwise schedule bounded work without consuming results early.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
the
merge-risk: 🚨 other
clawsweeper
Bot
removed
the
merge-risk: 🚨 auth-provider
1 participant
Summary
~/.gemini/antigravity-cli/conversations/*.db(GEMINI_CLI_HOME override) with tokscale cache fallback, and AntigravityOfflineFetchStrategy as terminal offline probe in auto/cli pipelinesP1/P2 fixes (review 5370820306, head dfada2c)
AntigravityOfflineFetchStrategynow setsaccountEmail: nilinstead of stampingselectedAccountEmail; ambient$HOME/.geminidata is no longer mis-attributed to the selected OAuth identity (Sources/CodexBarCore/Providers/Antigravity/AntigravityProviderDescriptor.swift:836)AntigravityOfflineStorenow counts~/.gemini/antigravity/*.dband~/.gemini/antigravity/conversations/*.dbin addition toantigravity-cli/conversations(the repo launches CLI with--app_data_dir ~/.gemini/antigravity, so the old path missed the real store). AddedappDataDirectoryhelper and summed DBs (Sources/CodexBarCore/Providers/Antigravity/AntigravityOfflineStore.swift)SpendDashboardControllernow caps concurrent Codex account scans to 3 viawithThrowingTaskGroupthrottling (pendingCount >=3 -> await group.next()), preserving index ordering (Sources/CodexBar/SpendDashboardController.swift:471)AntigravityOAuthFetchStrategy.shouldFallbacknow returnshasOfflineData(HOME/GEMINI_CLI_HOME) instead offalse, so expired credentials reachoffline(fix for Improve Antigravity retrieval: retired Flash alias and offline fallback #3119#discussion_r3830474646, already in 175a92c, retained)Real behavior proof (redacted, after fix, head dfada2c)
Rebased onto current
origin/main(27c7f33) to avoid duplicateofflinedefinition;swiftlint --strict0 violations,swiftformat0/4 formatted.Additional Swift proof (new tests, same logic, run on
dfada2c73):CI on
dfada2c73will re-runlint-build-test+ bothswift-test-macosshards +build-linux-*(previous5106df422failed due to duplicateofflineafter1cf98b330landed on main; rebased fix avoids it).@clawsweeper re-review