fix(antigravity): complete per-source failure diagnostics - #3683
Conversation
When every auto source fails, keep the most authoritative attempted source's error (app > cli > ide > oauth > offline) instead of the last one written: only .notRunning placeholders still yield to later errors. Add per-source outcome diagnostics: an auto-failure summary line in codexbar usage text output, one debug log line when a pipeline exhausts its strategies, and strategyID + outcome fields in the diagnose export. Part of steipete#3673 (workstream 1); targets the error-masking class behind steipete#3146 and steipete#3662. Source precedence is unchanged.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…r-honesty # Conflicts: # CHANGELOG.md
![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: 89586d5674
ℹ️ 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".
| guard (previous as? AntigravityStatusProbeError) == .notRunning else { | ||
| return previous |
There was a problem hiding this comment.
When Auto includes OAuth credentials and OAuth fails while no offline data exists, AntigravityOAuthFetchStrategy.shouldFallback returns false, causing ProviderFetchPipeline.fetch to return the current OAuth error immediately rather than the value folded by this resolver. Consequently, an earlier substantive app or CLI failure is still masked in this common failure path; the pipeline's terminal return needs to use the resolved lastAvailableError as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
Terminal fallback paths can bypass the first-authoritative error fold, allowing earlier app or CLI failures to remain masked.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR improves Antigravity auto-fallback error reporting and adds per-source diagnostics.
Changes:
- Preserves authoritative errors and records strategy outcomes.
- Adds CLI, logging, and diagnostic-export visibility.
- Extends tests, documentation, and changelog entries.
File summaries
| File | Reviewed changes |
|---|---|
Tests/CodexBarTests/ProviderDiagnosticExportTests.swift |
Tests diagnostic fields, legacy decoding, and redaction. |
Tests/CodexBarTests/ProviderArchitectureGatekeeperTests.swift |
Updates source anchors. |
Tests/CodexBarTests/CLIAntigravityFallbackSummaryTests.swift |
Tests ordered CLI source summaries. |
Tests/CodexBarTests/AntigravityCLIHTTPSFetchStrategyTests.swift |
Tests fallback precedence and per-source outcomes. |
Sources/CodexBarCore/Providers/ProviderFetchPlan.swift |
Tracks outcomes and logs exhaustion; terminal failures can bypass the authoritative-error fold (moderate), and log wording is provider-specific (nit). |
Sources/CodexBarCore/Providers/ProviderDiagnosticExport.swift |
Exports strategy IDs and outcomes. |
Sources/CodexBarCore/Providers/Antigravity/AntigravityProviderDescriptor.swift |
Adds authoritative error resolution; terminal paths can bypass it and mask earlier failures (moderate). |
Sources/CodexBarCLI/CLIUsageCommand.swift |
Integrates Antigravity fallback summaries. |
Sources/CodexBarCLI/CLIHelpers.swift |
Formats per-source diagnostics. |
docs/antigravity.md |
Documents diagnostic behavior. |
CHANGELOG.md |
Records the fix. |
Review details
Suppressed comments (2)
Sources/CodexBarCore/Providers/ProviderFetchPlan.swift:426
- This log message is emitted for every provider, but only Antigravity's resolver selects the most-authoritative error; the pipeline's default resolver still surfaces the last available error. The wording therefore mislabels diagnostics for all other providers. Use provider-neutral wording such as
Fetch strategies exhausted; surfacing fallback error, or make the message conditional on the provider's policy.
CodexBarLog.logger(LogCategories.provider(provider)).debug(
"Fetch strategies exhausted; surfacing most authoritative error",
Sources/CodexBarCore/Providers/ProviderFetchPlan.swift:403
- The new resolver is bypassed whenever a strategy declines fallback:
AntigravityOAuthFetchStrategy.shouldFallbackreturns false when no offline data exists, andAntigravityOfflineFetchStrategyis terminal. In those cases the pipeline returns the current OAuth/offline error directly, so a later, less-authoritative failure can still mask an earlier app/CLI failure despite this PR's first-authoritative guarantee. ReturnlastAvailableError ?? errorfor the terminal path (while retaining the current error when it is the first one) so all terminal failures use the same fold.
Self.logPerSourceOutcomes(provider: provider, attempts: attempts, surfacedError: error)
return ProviderFetchOutcome(result: .failure(error), attempts: attempts)
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| guard (previous as? AntigravityStatusProbeError) == .notRunning else { | ||
| return previous | ||
| } | ||
| return current |
…r-honesty # Conflicts: # CHANGELOG.md
|
Codex review: needs maintainer review before merge. Reviewed September 18, 2026, 9:13 AM ET / 13:13 UTC (Revision 8). ClawSweeper reviewWhat this changesAdds Antigravity source outcomes to CLI errors, safe debug logs, and diagnostic exports, and corrects misleading account-mismatch guidance. Merge readiness✅ Ready for maintainer review This PR remains useful: the merged terminal-error repair covers only part of the original work. The current patch resolves the prior findings, preserves error precedence, and has no actionable correctness or security findings. Priority: P2 Review scores
Verification
How this fits togetherCodexBar’s provider pipeline tries configured usage sources and records their results. These records feed CLI error messages, debug logs, and shareable diagnostic JSON. flowchart TD
A[Configured usage sources] --> B[Provider fetch pipeline]
B --> C[Ordered source outcomes]
B --> D[Existing error selection]
C --> E[CLI failure summary]
C --> F[Safe debug log]
C --> G[Diagnostic JSON]
D --> E
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Retain the established source-selection policy while exposing truthful, ordered diagnostics with backward-compatible exports. Do we have a high-confidence way to reproduce the issue? Yes, source inspection establishes the misleading account-mismatch guidance and missing diagnostic fields on main; no current-main runtime reproduction was executed. Is this the best way to solve the issue? Yes. The patch extends existing diagnostic paths, preserves the maintainer-confirmed error policy, and tests legacy export compatibility in both directions. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against aca09cf2ff87. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (7 earlier review cycles)
|
clawsweeper
Bot
added
P2
Apply the existing fallback error resolver to terminal failures as well as exhausted fallback chains. Preserve cancellation, original routing errors, and per-attempt diagnostics. Extracted from #3683, whose broader proposal remains open. Co-authored-by: hhh2210 <hzy2210@gmail.com>
|
The terminal-error repair is now on main through #3701, merged as e56bb5b. An unavailable final source now preserves the error selected by the existing provider policy, while cancellation and later substantive failures keep their existing precedence. Each source still records its original diagnostic. Validation: Thanks @hhh2210. This extracts only the terminal resolver repair; the broader first-error/per-source diagnostic proposal in #3683 remains open. |
hhh2210
changed the title
clawsweeper
Bot
added
proof: sufficient
clawsweeper
Bot
added
the
status: 👀 ready for maintainer look
Antigravity failures could hide which source failed, and an account-mismatch message claimed OAuth data was being used before OAuth succeeded. Report ordered source outcomes for auto-mode CLI failures and describe the account restriction accurately. The existing error-selection policy from #3701 remains authoritative.
Add optional strategy IDs and explicit outcomes to diagnostic exports, with legacy Codable compatibility in both directions. Terminal and exhausted pipeline failures produce one debug record using safe error categories; successful and cancelled runs stay quiet. The synthetic CLI fixture pins both its home and Gemini directory inside its own root.
Validation on
b4c6a91b3277e1690dab64aa89839e215f27b0e9:make test: all 1,196 selections across 110 groups passed on the first attempt, with no retries or timeouts (941 seconds).make check: zero violations. Independent P0–P2 review: no actionable findings.Current-head CI passed all checks. The combined five-fix integration tree also passed all 1,203 selections across 111 groups without retries or timeouts, with clean lint and independent review. The changelog entry is under 0.60.6 Unreleased.
Part of #3673; related to #3146 and #3662. Thanks @hhh2210 for the diagnostic fix and regression coverage.