fix(macos): require explicit Provider selection in connection editor#30425
Closed
vellum-apollo-bot[bot] wants to merge 1 commit into
Closed
fix(macos): require explicit Provider selection in connection editor#30425vellum-apollo-bot[bot] wants to merge 1 commit into
vellum-apollo-bot[bot] wants to merge 1 commit into
Conversation
Marina QA on 0.8.1 Desktop Cloud-hosted: an OpenRouter connection was labeled with the Anthropic tag. The reporter theorized that the system auto-detected provider from the "sk-" key prefix — there is no such heuristic anywhere in the codebase. The actual root cause sits in the macOS provider-connection editor. `ProvidersSheet.beginCreate()` pre-filled `editorDraft.provider = providerCatalog.first?.id`. Anthropic is the first entry in PROVIDER_CATALOG, so opening "New Connection" silently selected Anthropic. If a user pasted an OpenRouter key (or any provider other than Anthropic) into the API Key field and hit Save without scanning the Provider dropdown, the row persisted as `provider=anthropic` with the OpenRouter key stored at `credential/anthropic/api_key`. Downstream, the connection rendered the Anthropic display tag (correct given the bad row) and the Profiles picker filtered it out when the user wanted to bind an OpenRouter profile to it (since `c.provider !== "openrouter"`). Fix — mirrors the `isProviderMissing` pattern that #30313 introduced on the Inference Profile editor: 1. `beginCreate()` starts with an empty `ConnectionDraft`. No pre-filled provider, no pre-filled credential ref. The dropdown placeholder ("Select a provider…") makes the unfilled state visible. Provider's `onChange` already populates the credential ref (`credential/<provider>/api_key`) and the auth-type override (Ollama → none) when the user picks a value, so removing the pre-fill doesn't drop functionality. 2. New `isProviderMissing` + `canSave` computed properties expose the validation state to the view layer. `isProviderMissing` only trips in create mode — `beginEdit` pre-fills from `conn.provider`, and `saveAsNewFromManagedEdit` carries the managed row's provider into the clone, so neither path produces an empty draft. 3. Provider field renders a "Pick a provider" warning badge inline with the label while `isProviderMissing` is true. Same affordance pattern as the "Pick a model" badge in `InferenceProfileEditor.modelField`. 4. Primary footer button gets `isDisabled: !canSave` so the user can't fire Save until the dropdown is satisfied. The button stays enabled for legitimate edit paths (`beginEdit`, `saveAsNewFromManagedEdit`) because both seed a non-empty provider. 5. `commitEditor()` gains a belt-and-suspenders guard that surfaces "Select a provider." inline if a future code path bypasses the disabled button (keyboard submit, programmatic Save, etc.). The daemon's zod schema would reject `provider=""` either way, but this keeps the error message in the editor where the user is. Edit mode is unaffected: `editorProviderField` is only rendered for `.create` state and the dropdown is hidden in `.edit` / `.managedEdit`, so existing connections render their saved provider tag as before. The Save-as-New flow seeds `editorDraft.provider` from the source managed row before flipping to `.create`, so `canSave` returns true on first frame. Cherry-pick candidate for release/v0.8.1.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| } | ||
| // Provider intentionally left empty — the old pre-fill to | ||
| // `providerCatalog.first?.id` quietly defaulted to Anthropic and led | ||
| // users (e.g. Marina QA on 0.8.1) to paste an OpenRouter key and |
Contributor
There was a problem hiding this comment.
🔴 Comment includes real person's name "Marina QA", violating AGENTS.md generic-examples rule
The newly added comment at line 829 references Marina QA on 0.8.1, identifying a specific person. AGENTS.md's Generic Examples section explicitly prohibits this: "Never include personal user data — real names, emails, phone numbers, account IDs, or other identifying details of specific people — anywhere in the codebase. This covers code, tests, fixtures, documentation, comments, commit messages, and AGENTS.md files. Always use generic placeholders." The comment should use a generic placeholder like "a QA tester" instead of naming a real person.
Suggested change
| // users (e.g. Marina QA on 0.8.1) to paste an OpenRouter key and | |
| // users (e.g. a QA tester on 0.8.1) to paste an OpenRouter key and |
Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
Contributor
Author
|
Reopened. Tried to flip the canonical PR back to #30423 per Vargas, but GitHub rejected the reopen — once a force-push leaves a PR's recorded |
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Stops the macOS provider-connection editor from silently pre-selecting Anthropic when the user opens "New Connection." Users now have to make an explicit Provider pick before Save activates.
Why — Marina QA, 0.8.1 Desktop Cloud-hosted
Marina reported that an OpenRouter connection she created was labeled with the Anthropic tag. Her theory: the system was auto-detecting provider from the
sk-key prefix. There is no such heuristic anywhere in the codebase.Actual root cause sits in
ProvidersSheet.beginCreate():providerCatalog.firstis Anthropic (first entry inPROVIDER_CATALOG). So opening the editor silently seeded the dropdown to Anthropic. If a user pasted an OpenRouter / Fireworks / etc. key and hit Save without scanning the dropdown, the connection persisted asprovider=anthropicwith the foreign key stored atcredential/anthropic/api_key. Downstream:c.provider !== "openrouter")How
Same shape as the
isProviderMissingpattern on the Inference Profile editor:beginCreate()starts with an emptyConnectionDraft. Dropdown placeholder ("Select a provider…") makes the unfilled state visible. The dropdown'sonChangealready populates the credential ref + auth-type override on pick, so removing the pre-fill doesn't drop functionality.isProviderMissing+canSavecomputed properties expose the validation state.InferenceProfileEditor.modelField.isDisabled: !canSave. Save can't fire until the dropdown is satisfied.commitEditor()belt-and-suspenders guard surfaces "Select a provider." inline if a future code path bypasses the disabled button (programmatic submit, keyboard shortcut). Daemon zod would rejectprovider=""anyway; this keeps the error message in the editor.Edit-mode safety
editorProviderFieldonly renders for.createstate. The dropdown is hidden in.edit/.managedEdit. BothbeginEditandsaveAsNewFromManagedEditseededitorDraft.providerfrom the source row before flipping state, socanSavereturns true on first frame for those paths. Existing connections render their saved provider tag unchanged.Test
Cherry-pick
cherry-pick-to-releaselabel set forrelease/v0.8.1.Related: companion bug #2 from same QA pass landed as #30413 (Provider picker on Language Model card).