[codex] Fix first-run provider autodetection#1921
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 3:09 PM ET / 19:09 UTC. Summary Reproducibility: yes. from source inspection: current main enables Gemini from CLI presence alone and ignores Claude Desktop, matching the root cause described in the PR body. I did not run live app or keychain validation because repository policy prefers focused non-prompting checks for provider/parser behavior. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the focused first-run detection fix after the remaining CI finishes, keeping the policy tests as the stable seam for future provider-detection changes. Do we have a high-confidence way to reproduce the issue? Yes from source inspection: current main enables Gemini from CLI presence alone and ignores Claude Desktop, matching the root cause described in the PR body. I did not run live app or keychain validation because repository policy prefers focused non-prompting checks for provider/parser behavior. Is this the best way to solve the issue? Yes: extracting a small pure policy seam and testing it is the narrowest maintainable fix for this first-run detection rule. The remaining runtime integration is a direct mapping from existing binary, app, credential-file, and Antigravity signals into that policy. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8e9a844dbd36. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
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. How this review workflow works
|
clawsweeper
Bot
added
rating: 🐚 platinum hermit
![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: 6b1d76d7df
ℹ️ 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".
| let claudeDesktopInstalled = NSWorkspace.shared.urlForApplication( | ||
| withBundleIdentifier: "com.anthropic.claudefordesktop") != nil |
There was a problem hiding this comment.
When Claude Desktop is installed but the standalone claude binary is not on PATH/well-known paths and there is no browser Claude session, this signal makes Claude the only enabled provider and skips the Codex fallback. The Claude fetch planner still gets CLI availability from ClaudeCLIResolver.isAvailable/BinaryLocator.claudeWellKnownPaths, which has no Desktop app path, and the web fallback only probes browser cookies, so a desktop-only fresh install can start with an enabled Claude card that has no usable source. Please either resolve Desktop credentials/CLI before counting it as available or keep the fallback in this case.
Useful? React with 👍 / 👎.
1 participant
Summary
Root cause
First-run detection treated any Gemini executable on
PATHas usable, even without OAuth credentials, while Claude detection only considered the Claude Code CLI and ignored the installed desktop app.Validation
DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer swift test --filter ProviderDetectionPolicyTestsDEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer make checkDEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer make test(existingAdaptiveRefreshTimerTestsfailure on currentmain: manual mode observed two startup refresh completions instead of one; reproduced after the shard retry)