Repository navigation
fix: improve connected provider detection and activation - #60591
dimassfeb-09 wants to merge 1 commit into
Conversation
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes. The TUI half works. The core activation rule breaks providers that are set up with only an inline API key, and it fails an existing core test.
I ran the TUI from source on the base and on this PR and opened the model picker with a provider defined only in config. On the base it counts as not connected. With this PR it counts as connected and its models are grouped under the provider (screenshots below). In core, I checked an existing provider with a scratch test using the existing test fixture. A settings.timeout-only override no longer makes it available (the intended fix). A settings.apiKey-only override no longer makes it available either (a regression). packages/core test/config/provider.test.ts gives 39 pass, 1 fail with this PR; all 40 pass on the base. The failing test is "passes configured stream usage compatibility to the runtime model". test/util/connected-provider.test.ts passes.
Public API: unchanged.
Concerns:
- The activation rule in
overlayProvideris too narrow (inline comment). #59440 is open for the same core problem and keeps inline-credential providers enabled. Please drop the core change here, or match that rule, so this PR is only the TUI fix. - No test for the new behaviour. The description mentions
test/util/connected-provider.test.ts, but that file isn't changed. A small case for the providers-with-models path would help, and the failing core test needs updating or a reason it should change.
| provider.activation = "enabled" | ||
| // Only providers that actually bring their own endpoint or models become available without credentials. | ||
| // Setting-only overrides (e.g. a timeout) keep the catalog provider's normal connection requirement. | ||
| if ( |
There was a problem hiding this comment.
This rule drops cases that worked before. A catalog provider configured with only an inline settings.apiKey (e.g. { "openai": { "settings": { "apiKey": "sk-..." } } }) now keeps activation: "auto". It has no connection, so it is no longer available, even though the key would let its requests authenticate. I confirmed this with the core test fixture: available on the base, not with this PR. The same happens to a provider configured with only compatibility or headers, which fails the existing test "passes configured stream usage compatibility to the runtime model". It also affects a new provider that copies a catalog provider through canonical without listing models.
On the other hand, a baseURL-only override of a catalog provider now becomes enabled without a key. The model resolver then sends requests with no auth.
#59440 handles this with a different rule: enable only new providers, integration documents, or entries with a non-empty inline apiKey/authToken/accessToken. I'd drop this hunk and leave it to that PR, or use the same rule.
| return (integration.connections?.length ?? 0) > 0 | ||
| } | ||
|
|
||
| export function hasConnectedProvider( |
There was a problem hiding this comment.
Every caller passes (integrations, providers, models), so the object-input form and the Array.isArray branch aren't needed. They also replace the IntegrationInfo typing with loose *Like types. A plain (integrations, providers, models) signature using the client types would be simpler and keep type checking.
Issue for this PR
Closes #60590
Type of change
What does this PR do?
Fixes onboarding/connected-provider state in the TUI. Previously a provider config override that only changed settings (e.g. a timeout) could mark the provider as enabled, and connected detection only looked at integration connections. Now activation is enabled only when the override brings its own endpoint, models, or base URL, and connected detection also counts providers with models. Also made use-connected and sidebar onboarding location-aware.
How did you verify your code works?
Ran
bun test test/util/connected-provider.test.tsin packages/tui.Visual
Screenshots / recordings
Checklist
EOF
Added by the opencode bot (AI-generated).
Fixes #60587
Related: #49309