Fix widespread provider detection issues #1151
Conversation
- Replace aggressive defaults with conservative detection - Replace fragile string matching with JSON-based provider detection - Add Copilot detection support (github-copilot/ prefix) - Add comprehensive roundtrip tests for all providers - Fix false positives when no providers configured Fixes: OpenAI, Copilot, and other providers now properly detected when already configured. Installer shows accurate provider state instead of assuming providers exist. Tests: 29 config tests, 197 CLI tests, all passing
|
All contributors have signed the CLA. Thank you! ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
![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: 64a069ed18
ℹ️ 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".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
When users configure models in the categories section without an agents block (which is allowed by the schema), provider detection was returning all false. This broke provider detection for users who only override categories. Changes: - Modified detectProvidersFromOmoConfig to scan both agents and categories - Added detectProvidersFromConfig helper for direct config object testing - Fixed isMax20 detection to scan all models, not just sisyphus - Added tests for categories-only and mixed agents+categories scenarios - Exported getOmoConfig and detectProvidersFromConfig for testing Tests: 31 config tests, 199 CLI tests, all passing
|
@codex review |
The hasAnyOtherClaude check was matching any model containing "claude" substring, which incorrectly matched OpenCode Zen models like opencode/claude-opus-4-5. This caused OpenCode Zen-only configs to be misclassified as having Anthropic subscriptions. Changes: - Restricted Claude detection to anthropic/ prefix only - This excludes opencode/claude-* models from Claude detection - Added test for OpenCode Zen-only config to verify correct behavior Tests: 32 config tests, 200 CLI tests, all passing
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45ef7c36de
ℹ️ 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".
The isMax20 flag should ONLY be true when using Anthropic's claude-opus-4-5 model. Copilot and OpenCode Zen Opus models (github-copilot/claude-opus-4.5, opencode/claude-opus-4-5) should NOT trigger isMax20=true. Reverted change that used findModelContaining() for any provider's Opus models. Restored original logic that checks specifically for anthropic/claude-opus-4-5. Updated test expectations: - Copilot Opus: isMax20=false (not Anthropic) - OpenCode Zen Opus: isMax20=false (not Anthropic) Tests: 34 config tests, 202 CLI tests, all passing
|
I'm going to be honest, I'm not sure this is necessary. Does the installer need to detect existing configurations? |
1 participant
Summary
Problem
The installer was incorrectly detecting AI providers in four scenarios:
1. Aggressive Defaults
The installer was initializing with
hasClaude: true, hasOpenAI: trueby default, causing false positives when no providers were actually configured.2. Categories-Only Configs (CRITICAL FIX)
When users configured models in
categoriessection without anagentsblock (which is allowed by schema), provider detection returned all false. This broke provider detection for users who only override categories.3. Claude Detection False Positives (CRITICAL FIX)
The
hasAnyOtherClaudecheck matched any model containing "claude" substring, which incorrectly matched OpenCode Zen models likeopencode/claude-opus-4-5. This caused OpenCode Zen-only configs to be misclassified as having Anthropic subscriptions andisMax20enabled, leading to incorrect TUI defaults and auth prompts.4. isMax20 Over-Detection (CRITICAL FIX)
Initial fix used
findModelContaining("claude-opus")which matched ANY provider's Opus models (Copilot, OpenCode Zen). This caused false positives -isMax20should ONLY be true for Anthropic'sclaude-opus-4-5model, not for Copilot'sgithub-copilot/claude-opus-4.5or OpenCode Zen'sopencode/claude-opus-4-5.5. Fragile String-Based Detection
Used
configStr.includes('"openai/')which could match in comments or unrelated strings.6. Missing Copilot Detection
hasCopilotalways returnedfalse- no detection logic existed.Solution
false- only reports providers that actually exist in configfindProviderInObjects()helper that traverses config objects and checks model strings withstartsWith()detectProvidersFromOmoConfig()now scans BOTHagentsandcategoriessectionsanthropic/prefix only, excludingopencode/modelsanthropic/claude-opus-4-5- does NOT match Copilot or OpenCode Zen Opus modelsgithub-copilot/prefix in model stringsChanges
src/cli/config-manager.ts:detectProvidersFromOmoConfig()to check both agents and categoriesdetectProvidersFromConfig()helper for direct config object testinganthropic/claude-opus-4-5anthropic/prefix, excludingopencode/modelsfindModelContaining()helper that caused over-detectionsrc/cli/config-manager.test.ts:Testing
Checklist
bun run typecheckpassesbun run buildsucceeds