From d3213c5ff9740974afe29b366697c74739e976d0 Mon Sep 17 00:00:00 2001 From: "apollo-the-bot[bot]" Date: Tue, 12 May 2026 19:20:25 +0000 Subject: [PATCH] fix(macos): require explicit Provider selection in connection editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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//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. --- .../Features/Settings/ProvidersSheet.swift | 71 +++++++++++++++---- 1 file changed, 56 insertions(+), 15 deletions(-) diff --git a/clients/macos/vellum-assistant/Features/Settings/ProvidersSheet.swift b/clients/macos/vellum-assistant/Features/Settings/ProvidersSheet.swift index 4c5f0523701..e713f83d589 100644 --- a/clients/macos/vellum-assistant/Features/Settings/ProvidersSheet.swift +++ b/clients/macos/vellum-assistant/Features/Settings/ProvidersSheet.swift @@ -480,9 +480,18 @@ struct ProvidersSheet: View { private var editorProviderField: some View { VStack(alignment: .leading, spacing: VSpacing.xs) { - Text("Provider") - .font(VFont.labelDefault) - .foregroundStyle(VColor.contentSecondary) + HStack(spacing: VSpacing.xs) { + Text("Provider") + .font(VFont.labelDefault) + .foregroundStyle(VColor.contentSecondary) + // Warning badge — only renders in create mode while the + // dropdown is still empty. Edit mode pins the value to the + // connection's saved provider (the dropdown is disabled in + // that path), so `isProviderMissing` can never trip there. + if isProviderMissing { + VBadge(label: "Pick a provider", tone: .warning, emphasis: .subtle) + } + } VDropdown( placeholder: "Select a provider\u{2026}", selection: $editorDraft.provider, @@ -730,7 +739,7 @@ struct ProvidersSheet: View { saveAsNewFromManagedEdit() } } - VButton(label: editorPrimaryLabel, style: .primary) { + VButton(label: editorPrimaryLabel, style: .primary, isDisabled: !canSave) { Task { await commitEditor() } } } @@ -749,6 +758,29 @@ struct ProvidersSheet: View { } } + // MARK: - Editor Validation + + /// True when the user hasn't picked a provider yet. Provider is required + /// for a new connection — the dropdown starts empty in `beginCreate` so + /// the user has to make an explicit choice rather than silently + /// inheriting the first catalog entry (Anthropic), which previously led + /// to OpenRouter / Fireworks keys being saved against the wrong provider + /// when users pasted-and-saved without scanning the dropdown. Mirrors + /// `InferenceProfileEditor.isProviderMissing`. + private var isProviderMissing: Bool { + editorDraft.provider.isEmpty + } + + /// Combined gate for the primary footer button. Today provider is the + /// only piece we lift out of `commitEditor` so the user gets immediate + /// feedback (greyed-out Save + warning badge) instead of an error after + /// a click; name/credential validation still runs server-side in + /// `commitEditor` because those errors need context the dropdown can't + /// convey (e.g. duplicate-name conflicts only the daemon knows about). + private var canSave: Bool { + !isProviderMissing + } + // MARK: - Conflict Sheet private func conflictSheet(_ info: ConflictInfo) -> some View { @@ -792,17 +824,15 @@ struct ProvidersSheet: View { private func beginCreate() { actionError = nil isKeyDirty = false - let provider = store.providerCatalog.first?.id ?? "" - editorDraft = ConnectionDraft( - provider: provider - ) - if !provider.isEmpty { - if provider == "ollama" { - editorDraft.authType = "none" - } else { - editorDraft.credential = "credential/\(provider)/api_key" - } - } + // 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 + // hit Save without noticing the dropdown was still on Anthropic, + // persisting the row as `provider=anthropic` with the OpenRouter key + // in the credential slot. Forcing an explicit selection eliminates + // that whole class of mismatched-provider bug. Mirrors the + // `isProviderMissing` guard in InferenceProfileEditor. + editorDraft = ConnectionDraft() editorState = .create maskedCredentialValue = nil Task { await loadAvailableCredentials() } @@ -980,6 +1010,17 @@ struct ProvidersSheet: View { actionError = "Name is required." return } + // Belt-and-suspenders for the Save button's `isDisabled: !canSave` + // gate: if a future caller exposes a path that bypasses the disabled + // button (programmatic Enter-key submit, future keyboard shortcuts, + // accidentally re-enabled state), surface the same error inline + // instead of POSTing a row with `provider=""` to the daemon. The + // daemon's zod schema would reject it, but the user-facing message + // belongs here. + guard !draft.provider.isEmpty else { + actionError = "Select a provider." + return + } var credentialRef = draft.credential.trimmingCharacters(in: .whitespacesAndNewlines)