From f0375fae548768c650b267ae2c5936e66e4e7d01 Mon Sep 17 00:00:00 2001 From: Credence Date: Mon, 11 May 2026 07:41:05 +0000 Subject: [PATCH 1/2] feat(macos): per-profile provider-connection picker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes audit finding #5 (profile editor cannot bind to a specific provider connection). The daemon schema already accepts `provider_connection` on `ProfileEntry` (see `assistant/src/config/schemas/llm.ts`); this PR plumbs the field through the macOS model + editor. UX: - New "Connection" sub-dropdown renders below Provider when the selected provider has at least one active connection. First option is "Any active connection" (value=""), preserving the daemon's legacy first-active-wins behavior. Subsequent options are the active connections, labelled by `label` (fallback: `name`). - Changing the Provider clears the connection binding so a stale name never survives a provider switch. - A "Not found" warning badge surfaces when the saved binding no longer matches any active connection (most often: a connection was disabled or deleted outside the editor). Wire shape: snake_case `provider_connection` for the JSON key (matches the Zod schema); camelCase `providerConnection` for the Swift property. `preservedJSON` exclusion list updated so the snake_case key doesn't double-write. Connections are fetched once by `InferenceProfilesSheet.task` and re-fetched when the editor closes (in case a connection was added in another surface during the session). Tolerant on transport failure (stale list preserved) — same posture as `SettingsStore.providerKeys`. Tests: - 4 new `InferenceProfileTests` cases cover wire-shape round-trip, empty-string decode, merging-from-fragment, merging-preserves. - 5 new `InferenceProfileEditorTests` cases cover the per-provider filter, empty-state behaviors, the display-name helper, and the full body construction with a populated `connections` list. - `InferenceProfilesSheetTests` updated to inject the mock client. No daemon changes required. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../Features/Settings/InferenceProfile.swift | 22 ++++ .../Settings/InferenceProfileEditor.swift | 82 +++++++++++++ .../Settings/InferenceProfilesSheet.swift | 36 +++++- .../InferenceProfileEditorTests.swift | 111 ++++++++++++++++++ .../Settings/InferenceProfileTests.swift | 73 ++++++++++++ .../InferenceProfilesSheetTests.swift | 10 +- 6 files changed, 331 insertions(+), 3 deletions(-) diff --git a/clients/macos/vellum-assistant/Features/Settings/InferenceProfile.swift b/clients/macos/vellum-assistant/Features/Settings/InferenceProfile.swift index 4ce3d3c2df2..03c1ef107fa 100644 --- a/clients/macos/vellum-assistant/Features/Settings/InferenceProfile.swift +++ b/clients/macos/vellum-assistant/Features/Settings/InferenceProfile.swift @@ -45,6 +45,18 @@ public struct InferenceProfile: Hashable, Identifiable { public var profileDescription: String? public var provider: String? + + /// Name of a `provider_connections` row to bind this profile to. When set, + /// the daemon dispatcher resolves auth from this specific connection + /// instead of falling back to "the first active connection for the + /// provider." Mirrors `ProfileEntry.provider_connection` in + /// `assistant/src/config/schemas/llm.ts`. + /// + /// Wire shape is `provider_connection` (snake_case) because the daemon's + /// Zod schema declares the field with that literal key; see + /// `LLMConfigBase.provider_connection` in `llm.ts`. + public var providerConnection: String? + public var model: String? public var maxTokens: Int? public var effort: String? @@ -96,6 +108,7 @@ public struct InferenceProfile: Hashable, Identifiable { label: String? = nil, profileDescription: String? = nil, provider: String? = nil, + providerConnection: String? = nil, model: String? = nil, maxTokens: Int? = nil, effort: String? = nil, @@ -112,6 +125,7 @@ public struct InferenceProfile: Hashable, Identifiable { self.label = label self.profileDescription = profileDescription self.provider = provider + self.providerConnection = providerConnection self.model = model self.maxTokens = maxTokens self.effort = effort @@ -134,6 +148,7 @@ public struct InferenceProfile: Hashable, Identifiable { label: String? = nil, profileDescription: String? = nil, provider: String? = nil, + providerConnection: String? = nil, model: String? = nil, maxTokens: Int? = nil, effort: String? = nil, @@ -151,6 +166,7 @@ public struct InferenceProfile: Hashable, Identifiable { label: label, profileDescription: profileDescription, provider: provider, + providerConnection: providerConnection, model: model, maxTokens: maxTokens, effort: effort, @@ -176,6 +192,7 @@ public struct InferenceProfile: Hashable, Identifiable { self.label = (json["label"] as? String).flatMap { $0.isEmpty ? nil : $0 } self.profileDescription = (json["description"] as? String).flatMap { $0.isEmpty ? nil : $0 } self.provider = (json["provider"] as? String).flatMap { $0.isEmpty ? nil : $0 } + self.providerConnection = (json["provider_connection"] as? String).flatMap { $0.isEmpty ? nil : $0 } self.model = (json["model"] as? String).flatMap { $0.isEmpty ? nil : $0 } self.maxTokens = Self.intValue(json["maxTokens"]) self.effort = (json["effort"] as? String).flatMap { $0.isEmpty ? nil : $0 } @@ -198,6 +215,7 @@ public struct InferenceProfile: Hashable, Identifiable { var merged = self if let v = fragment.status { merged.status = v } if let v = fragment.provider { merged.provider = v } + if let v = fragment.providerConnection { merged.providerConnection = v } if let v = fragment.model { merged.model = v } if let v = fragment.maxTokens { merged.maxTokens = v } if let v = fragment.effort { merged.effort = v } @@ -225,6 +243,7 @@ public struct InferenceProfile: Hashable, Identifiable { if let label { result["label"] = label } if let profileDescription { result["description"] = profileDescription } if let provider { result["provider"] = provider } + if let providerConnection { result["provider_connection"] = providerConnection } if let model { result["model"] = model } if let maxTokens { result["maxTokens"] = maxTokens } if let effort { result["effort"] = effort } @@ -266,6 +285,7 @@ public struct InferenceProfile: Hashable, Identifiable { "label", "description", "provider", + "provider_connection", "model", "maxTokens", "effort", @@ -311,6 +331,7 @@ public struct InferenceProfile: Hashable, Identifiable { && lhs.label == rhs.label && lhs.profileDescription == rhs.profileDescription && lhs.provider == rhs.provider + && lhs.providerConnection == rhs.providerConnection && lhs.model == rhs.model && lhs.maxTokens == rhs.maxTokens && lhs.effort == rhs.effort @@ -329,6 +350,7 @@ public struct InferenceProfile: Hashable, Identifiable { hasher.combine(label) hasher.combine(profileDescription) hasher.combine(provider) + hasher.combine(providerConnection) hasher.combine(model) hasher.combine(maxTokens) hasher.combine(effort) diff --git a/clients/macos/vellum-assistant/Features/Settings/InferenceProfileEditor.swift b/clients/macos/vellum-assistant/Features/Settings/InferenceProfileEditor.swift index 47061f06b31..2a9046d738e 100644 --- a/clients/macos/vellum-assistant/Features/Settings/InferenceProfileEditor.swift +++ b/clients/macos/vellum-assistant/Features/Settings/InferenceProfileEditor.swift @@ -27,6 +27,14 @@ struct InferenceProfileEditor: View { @Binding var profile: InferenceProfile var isReadOnly: Bool = false var isCreating: Bool = false + /// Provider connections available for the Connection sub-dropdown. The + /// editor reads this list, filters by the currently-selected provider + /// and `.status == .active`, and lets the user route the profile to a + /// specific row. Defaults to empty so test constructions and callers + /// that don't care about connection routing still compile — daemons + /// older than the `provider_connection`-aware profile schema continue + /// to behave as "pick the first active connection for the provider." + var connections: [ProviderConnection] = [] let onSave: () -> Void var onSaveAs: (() -> Void)? let onCancel: () -> Void @@ -126,6 +134,7 @@ struct InferenceProfileEditor: View { descriptionField keyField providerField + connectionField modelField if visibility.maxTokens { maxTokensField @@ -326,6 +335,12 @@ struct InferenceProfileEditor: View { } else { profile.model = nil } + // Reset connection binding too: a stale name almost + // certainly points at a different provider's row, and + // the daemon would reject it at resolve time. Falling + // back to "Any active connection" matches + // the dispatcher's legacy behavior. + profile.providerConnection = nil Self.clampMaxOutputTokensForSelectedModel(&profile) Self.clampContextWindowForSelectedModel(&profile) } @@ -337,6 +352,73 @@ struct InferenceProfileEditor: View { } } + /// Active connections that match the currently-selected provider. Used + /// by `connectionField` to populate its dropdown and to gate visibility: + /// when zero match the dropdown is hidden entirely (the daemon's legacy + /// fallback applies) rather than rendering a placeholder with no + /// actionable options. + var availableConnectionsForProvider: [ProviderConnection] { + guard let provider = profile.provider, !provider.isEmpty else { return [] } + return connections.filter { $0.provider == provider && $0.status == .active } + } + + /// Connection sub-dropdown. Renders only when a provider is selected + /// AND that provider has at least one active connection in + /// ``connections``. The first option preserves the pre-#5 default + /// ("the daemon picks the first active connection") so existing + /// profiles keep working without an explicit migration. + @ViewBuilder + private var connectionField: some View { + let available = availableConnectionsForProvider + if !available.isEmpty, let provider = profile.provider { + labeled( + "Connection", + accessory: { + // Surface the "stale binding" state: the saved name + // doesn't match any active connection for the provider. + // Most commonly this fires when a connection was + // disabled or deleted outside the editor. + if let bound = profile.providerConnection, + !bound.isEmpty, + !available.contains(where: { $0.name == bound }) { + VBadge( + label: "Not found", + tone: .warning, + emphasis: .subtle + ) + } + } + ) { + VDropdown( + placeholder: "Any active connection\u{2026}", + selection: Binding( + get: { profile.providerConnection ?? "" }, + set: { newValue in + profile.providerConnection = newValue.isEmpty ? nil : newValue + } + ), + options: [ + ( + label: "Any active \(store.dynamicProviderDisplayName(provider)) connection", + value: "" + ) + ] + available.map { conn in + (label: Self.connectionDisplayName(conn), value: conn.name) + } + ) + } + } + } + + /// Prefer the human-readable label when present; fall back to the + /// connection's stored `name` (which is the on-disk identifier). Mirrors + /// the convention used by `ProvidersSheet` row rendering so the two + /// surfaces stay visually consistent. + static func connectionDisplayName(_ conn: ProviderConnection) -> String { + if let label = conn.label, !label.isEmpty { return label } + return conn.name + } + private var modelField: some View { let provider = profile.provider ?? "" let models = store.dynamicProviderModels(provider) diff --git a/clients/macos/vellum-assistant/Features/Settings/InferenceProfilesSheet.swift b/clients/macos/vellum-assistant/Features/Settings/InferenceProfilesSheet.swift index 77f717c048f..249b15c39c4 100644 --- a/clients/macos/vellum-assistant/Features/Settings/InferenceProfilesSheet.swift +++ b/clients/macos/vellum-assistant/Features/Settings/InferenceProfilesSheet.swift @@ -26,6 +26,19 @@ struct InferenceProfilesSheet: View { @ObservedObject var store: SettingsStore @Binding var isPresented: Bool + /// Connection client used to populate the editor's per-provider + /// Connection dropdown (audit finding #5). Injected so tests can stub + /// the network. Defaults to the production client; matches the pattern + /// already established by `ProvidersSheet`. + var connectionClient: ProviderConnectionClientProtocol = ProviderConnectionClient() + + /// Cached active+disabled connection list. The editor reads this via + /// the `connections:` parameter and filters down to `.active` matches + /// for the currently-selected provider. Refreshed on appear and after + /// each editor commit so users who add a connection in another surface + /// see it without a manual reload. + @State private var connections: [ProviderConnection] = [] + @State private var editorState: EditorState? /// Local working copy for the active editor session. Bound into @@ -89,17 +102,26 @@ struct InferenceProfilesSheet: View { .sheet(item: $blockedState) { _ in blockedDeleteSheet } - .onChange(of: editorState) { _, newValue in + .onChange(of: editorState) { oldValue, newValue in if newValue == nil { editorDraft = InferenceProfile(name: "") editorOriginalName = nil } + // Re-fetch when the editor transitions from open → closed so a + // freshly-added connection (created in another sheet) shows up + // the next time the editor opens. Also covers the create-mode + // case where the daemon just wrote a new profile that may + // reference a connection added in the same session. + if oldValue != nil && newValue == nil { + Task { await refreshConnections() } + } } .onChange(of: blockedState) { _, newValue in if newValue == nil { replacementSelection = "" } } + .task { await refreshConnections() } .animation(VAnimation.fast, value: editorState != nil) } @@ -301,6 +323,7 @@ struct InferenceProfilesSheet: View { profile: $editorDraft, isReadOnly: isViewMode, isCreating: isCreateMode, + connections: connections, onSave: { Task { await commitEditor() } }, @@ -314,6 +337,17 @@ struct InferenceProfilesSheet: View { ) } + /// Loads provider connections used by the editor's per-provider + /// Connection dropdown. Tolerant: on transport failure the cached list + /// is preserved (stale-but-correct beats blank) — same posture as + /// `SettingsStore.providerKeys`. + private func refreshConnections() async { + guard let fetched = await connectionClient.listProviderConnections(provider: nil) else { + return + } + connections = fetched + } + // MARK: - Blocked Delete Sheet private var blockedDeleteSheet: some View { diff --git a/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileEditorTests.swift b/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileEditorTests.swift index 09fc6556aec..9a41e7b63de 100644 --- a/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileEditorTests.swift +++ b/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileEditorTests.swift @@ -788,4 +788,115 @@ final class InferenceProfileEditorTests: XCTestCase { _ = editor.body XCTAssertEqual(cancelCalls, 0) } + + // MARK: - Connection sub-dropdown (audit finding #5) + + /// Two active openai connections + one disabled + one of a different + /// provider. With provider == "openai" the filter must yield exactly + /// the two active openai rows in input order. + func testAvailableConnectionsForProviderFiltersByProviderAndStatus() { + let connections: [ProviderConnection] = [ + Self.makeConnection(name: "personal-openai", provider: "openai", status: .active, label: "Personal"), + Self.makeConnection(name: "work-openai", provider: "openai", status: .active, label: "Work"), + Self.makeConnection(name: "legacy-openai", provider: "openai", status: .disabled, label: "Legacy"), + Self.makeConnection(name: "anthropic-main", provider: "anthropic", status: .active, label: "Main"), + ] + let editor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile(name: "draft", provider: "openai")), + connections: connections, + onSave: {}, + onCancel: {} + ) + let available = editor.availableConnectionsForProvider + XCTAssertEqual(available.map { $0.name }, ["personal-openai", "work-openai"]) + } + + /// When no provider is selected, no connections are surfaced — the + /// daemon's dispatcher has nothing to bind against until the user + /// picks a provider, so the dropdown stays hidden. + func testAvailableConnectionsForProviderIsEmptyWhenProviderUnset() { + let connections = [ + Self.makeConnection(name: "openai", provider: "openai", status: .active), + ] + let editor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile(name: "draft", provider: nil)), + connections: connections, + onSave: {}, + onCancel: {} + ) + XCTAssertTrue(editor.availableConnectionsForProvider.isEmpty) + } + + /// Empty `connections` (the default — e.g. the daemon predates the + /// connections API) must not crash the filter. + func testAvailableConnectionsForProviderIsEmptyWhenConnectionsEmpty() { + let editor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile(name: "draft", provider: "openai")), + onSave: {}, + onCancel: {} + ) + XCTAssertTrue(editor.availableConnectionsForProvider.isEmpty) + } + + /// Display label prefers the human-readable `label` (e.g. "Personal") + /// over the internal `name`. Falls back to `name` when label is nil OR + /// empty so a daemon that sends `""` for the label doesn't render an + /// invisible row. + func testConnectionDisplayNamePrefersLabel() { + let withLabel = Self.makeConnection(name: "personal-openai", label: "Personal") + XCTAssertEqual(InferenceProfileEditor.connectionDisplayName(withLabel), "Personal") + + let withoutLabel = Self.makeConnection(name: "personal-openai", label: nil) + XCTAssertEqual(InferenceProfileEditor.connectionDisplayName(withoutLabel), "personal-openai") + + let emptyLabel = Self.makeConnection(name: "personal-openai", label: "") + XCTAssertEqual(InferenceProfileEditor.connectionDisplayName(emptyLabel), "personal-openai") + } + + /// The editor must still build when a `connections:` list is passed in + /// alongside other knobs — body construction is the safety net for any + /// SwiftUI type-inference regression we'd otherwise miss until a + /// snapshot build. + func testEditorBodyBuildsWithConnections() { + let connections = [ + Self.makeConnection(name: "personal-openai", provider: "openai", label: "Personal"), + Self.makeConnection(name: "work-openai", provider: "openai", label: "Work"), + ] + let editor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile( + name: "personal", + provider: "openai", + providerConnection: "personal-openai", + model: "gpt-5" + )), + connections: connections, + onSave: {}, + onCancel: {} + ) + XCTAssertNotNil(editor.body) + } + + /// Test helper mirroring `ProvidersSheetTests.makeConnection` so the + /// two surfaces use identical fixture shapes. + private static func makeConnection( + name: String = "my-conn", + provider: String = "openai", + authType: String = "api_key", + status: ConnectionStatus = .active, + label: String? = nil + ) -> ProviderConnection { + ProviderConnection( + name: name, + provider: provider, + auth: ProviderConnectionAuth(type: authType, credential: "sk-test"), + status: status, + label: label, + createdAt: 0, + updatedAt: 0 + ) + } } diff --git a/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileTests.swift b/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileTests.swift index cdfef0a59e1..c885ea96c74 100644 --- a/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileTests.swift +++ b/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileTests.swift @@ -323,4 +323,77 @@ final class InferenceProfileTests: XCTestCase { XCTAssertEqual(merged.status, "disabled") XCTAssertEqual(merged.model, "claude-3.5") } + + // MARK: - provider_connection (audit finding #5) + + /// Profiles that bind to a specific provider connection must round-trip + /// the daemon's snake_case `provider_connection` wire field. The Swift + /// property uses camelCase `providerConnection` but the JSON key matches + /// the Zod schema in `assistant/src/config/schemas/llm.ts`. + func testProviderConnectionRoundTrip() { + let json: [String: Any] = [ + "provider": "openai", + "provider_connection": "personal-openai", + "model": "gpt-5", + ] + let profile = InferenceProfile(name: "personal", json: json) + XCTAssertEqual(profile.provider, "openai") + XCTAssertEqual(profile.providerConnection, "personal-openai") + XCTAssertEqual(profile.model, "gpt-5") + + let reEncoded = profile.toJSON() + XCTAssertEqual(reEncoded["provider_connection"] as? String, "personal-openai") + // Snake_case must not bleed into a camelCase duplicate — the + // preservedJSON exclusion list guards against that. + XCTAssertNil(reEncoded["providerConnection"]) + } + + /// Empty strings should decode as `nil` (rather than getting written + /// back as an empty key) so disabling the binding via the editor's + /// "Any active connection" option doesn't leave a min(1)-violating + /// value on disk that the daemon would reject at Zod parse time. + func testProviderConnectionEmptyStringDecodesAsNil() { + let json: [String: Any] = [ + "provider": "openai", + "provider_connection": "", + ] + let profile = InferenceProfile(name: "draft", json: json) + XCTAssertNil(profile.providerConnection) + + let reEncoded = profile.toJSON() + XCTAssertNil(reEncoded["provider_connection"]) + } + + /// `merging` must propagate a new `providerConnection` (re-bind) and + /// preserve the existing one when the fragment omits it (other-field + /// edits shouldn't clear the binding). + func testMergingProviderConnectionFromFragment() { + let base = InferenceProfile( + name: "personal", + provider: "openai", + providerConnection: "personal-openai", + model: "gpt-5" + ) + let rebind = InferenceProfile(name: "personal", providerConnection: "work-openai") + + let merged = base.merging(rebind) + + XCTAssertEqual(merged.providerConnection, "work-openai") + XCTAssertEqual(merged.provider, "openai") + XCTAssertEqual(merged.model, "gpt-5") + } + + func testMergingPreservesProviderConnectionWhenFragmentOmitsIt() { + let base = InferenceProfile( + name: "personal", + provider: "openai", + providerConnection: "personal-openai" + ) + let modelOnly = InferenceProfile(name: "personal", model: "gpt-5") + + let merged = base.merging(modelOnly) + + XCTAssertEqual(merged.providerConnection, "personal-openai") + XCTAssertEqual(merged.model, "gpt-5") + } } diff --git a/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfilesSheetTests.swift b/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfilesSheetTests.swift index 28fd81ccc14..ff8adb9b831 100644 --- a/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfilesSheetTests.swift +++ b/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfilesSheetTests.swift @@ -55,9 +55,15 @@ final class InferenceProfilesSheetTests: XCTestCase { ]) } - private func makeSheet() -> InferenceProfilesSheet { + private func makeSheet( + connectionClient: ProviderConnectionClientProtocol = MockProviderConnectionClient() + ) -> InferenceProfilesSheet { let isPresented = Binding(get: { true }, set: { _ in }) - return InferenceProfilesSheet(store: store, isPresented: isPresented) + return InferenceProfilesSheet( + store: store, + isPresented: isPresented, + connectionClient: connectionClient + ) } // MARK: - Body construction From b35d2f2e2ccfc3fc796a4dba28130aa3efb14c31 Mon Sep 17 00:00:00 2001 From: credence-the-bot Date: Mon, 11 May 2026 08:22:33 +0000 Subject: [PATCH 2/2] fix(profile-picker): keep visible for stale bindings + clear provider_connection in route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex flagged two P1s on the original PR: 1. **Picker hidden for stale bindings.** When a profile's `providerConnection` referenced a now-disabled or deleted connection AND the workspace had no active connections for that provider, the editor hid the entire Connection field. The user had no way to clear the broken binding from the UI, and the profile remained pinned to an invalid name. Refactor: visibility now gates on (active matches OR non-empty saved binding); when stale, the picker shows a ' (not found)' option so the trigger renders the bound name, and the Not Found badge surfaces the state. Selecting 'Any active …' clears the binding back to the daemon's first-active fallback. 2. **`provider_connection` not in `INFERENCE_PROFILE_UI_KEYS`.** The profile-replace route only clears keys listed in that set, so omitting `provider_connection` from the body (which is what the UI does when the user picks 'Any active') silently retained the old server-side binding. Added `provider_connection` so the route now correctly wipes the stored value when the UI clears it. Tests: - 5 new InferenceProfileEditorTests covering staleProviderConnection truth table (binding missing / disabled / matches / nil / empty) + body-builds-with-stale. - 2 new conversation-query-routes tests covering write + clear paths for `provider_connection` on PUT /v1/config/llm/profiles/:name. 13/13 daemon tests pass; assistant tsc + eslint clean. Addresses Codex P1 #1 + P1 #2 from `@codex review` on `f0375fae54`. --- .../conversation-query-routes.test.ts | 50 ++++++++ .../routes/conversation-query-routes.ts | 1 + .../Settings/InferenceProfileEditor.swift | 61 ++++++--- .../InferenceProfileEditorTests.swift | 120 ++++++++++++++++++ 4 files changed, 211 insertions(+), 21 deletions(-) diff --git a/assistant/src/runtime/routes/__tests__/conversation-query-routes.test.ts b/assistant/src/runtime/routes/__tests__/conversation-query-routes.test.ts index a213903f5a9..6aedb6b3b80 100644 --- a/assistant/src/runtime/routes/__tests__/conversation-query-routes.test.ts +++ b/assistant/src/runtime/routes/__tests__/conversation-query-routes.test.ts @@ -374,4 +374,54 @@ describe("PUT /v1/config/llm/profiles/:name", () => { }); expect(savedProfile.openrouter).toEqual({ only: ["anthropic"] }); }); + + test("writes provider_connection when present in body", () => { + const result = replaceProfileRoute.handler({ + pathParams: { name: "custom" }, + body: { + provider: "openai", + provider_connection: "personal-openai", + model: "gpt-5.5", + }, + }); + + expect(result).toEqual({ ok: true }); + const savedProfile = ( + savedRawConfig?.llm as { + profiles: Record>; + } + ).profiles.custom; + + expect(savedProfile.provider).toBe("openai"); + expect(savedProfile.provider_connection).toBe("personal-openai"); + }); + + test("clears provider_connection when omitted from body (UI-owned key)", () => { + // Seed an existing binding so the test starts from a non-empty state. + ( + rawConfigFixture.llm as { + profiles: { custom: Record }; + } + ).profiles.custom.provider_connection = "stale-openai"; + + const result = replaceProfileRoute.handler({ + pathParams: { name: "custom" }, + body: { + provider: "openai", + model: "gpt-5.5", + // provider_connection deliberately omitted — the UI cleared the + // picker back to "Any active" and the route must wipe the saved + // binding, not silently round-trip it. + }, + }); + + expect(result).toEqual({ ok: true }); + const savedProfile = ( + savedRawConfig?.llm as { + profiles: Record>; + } + ).profiles.custom; + + expect(savedProfile.provider_connection).toBeUndefined(); + }); }); diff --git a/assistant/src/runtime/routes/conversation-query-routes.ts b/assistant/src/runtime/routes/conversation-query-routes.ts index 484bf1a8659..5880879da39 100644 --- a/assistant/src/runtime/routes/conversation-query-routes.ts +++ b/assistant/src/runtime/routes/conversation-query-routes.ts @@ -107,6 +107,7 @@ const RESERVED_PROFILE_NAMES = new Set([ const INFERENCE_PROFILE_UI_KEYS = new Set([ "provider", + "provider_connection", "model", "maxTokens", "effort", diff --git a/clients/macos/vellum-assistant/Features/Settings/InferenceProfileEditor.swift b/clients/macos/vellum-assistant/Features/Settings/InferenceProfileEditor.swift index 2a9046d738e..f719b670b0c 100644 --- a/clients/macos/vellum-assistant/Features/Settings/InferenceProfileEditor.swift +++ b/clients/macos/vellum-assistant/Features/Settings/InferenceProfileEditor.swift @@ -353,24 +353,36 @@ struct InferenceProfileEditor: View { } /// Active connections that match the currently-selected provider. Used - /// by `connectionField` to populate its dropdown and to gate visibility: - /// when zero match the dropdown is hidden entirely (the daemon's legacy - /// fallback applies) rather than rendering a placeholder with no - /// actionable options. + /// by `connectionField` to populate its dropdown. var availableConnectionsForProvider: [ProviderConnection] { guard let provider = profile.provider, !provider.isEmpty else { return [] } return connections.filter { $0.provider == provider && $0.status == .active } } - /// Connection sub-dropdown. Renders only when a provider is selected - /// AND that provider has at least one active connection in - /// ``connections``. The first option preserves the pre-#5 default - /// ("the daemon picks the first active connection") so existing - /// profiles keep working without an explicit migration. + /// The currently-saved binding when it does NOT resolve to any active + /// connection for the selected provider. `nil` when the binding is + /// empty or when it does match. Used to gate the picker's "stale" + /// affordances (extra dropdown option + warning badge). + var staleProviderConnection: String? { + guard let bound = profile.providerConnection, !bound.isEmpty else { return nil } + return availableConnectionsForProvider.contains(where: { $0.name == bound }) + ? nil + : bound + } + + /// Connection sub-dropdown. Renders when a provider is selected AND + /// either at least one active connection matches OR the profile has a + /// non-empty saved binding (so a stale binding can be seen and cleared + /// rather than silently round-tripping on save). The first option + /// preserves the daemon's "first active" fallback so existing profiles + /// keep working without an explicit migration. @ViewBuilder private var connectionField: some View { let available = availableConnectionsForProvider - if !available.isEmpty, let provider = profile.provider { + let stale = staleProviderConnection + if let provider = profile.provider, + !provider.isEmpty, + (!available.isEmpty || stale != nil) { labeled( "Connection", accessory: { @@ -378,9 +390,7 @@ struct InferenceProfileEditor: View { // doesn't match any active connection for the provider. // Most commonly this fires when a connection was // disabled or deleted outside the editor. - if let bound = profile.providerConnection, - !bound.isEmpty, - !available.contains(where: { $0.name == bound }) { + if stale != nil { VBadge( label: "Not found", tone: .warning, @@ -389,6 +399,22 @@ struct InferenceProfileEditor: View { } } ) { + let baseOptions: [(label: String, value: String)] = [ + ( + label: "Any active \(store.dynamicProviderDisplayName(provider)) connection", + value: "" + ) + ] + available.map { conn in + (label: Self.connectionDisplayName(conn), value: conn.name) + } + // When the saved binding is stale, surface it as an explicit + // dropdown option so the trigger renders its name. Selecting + // "Any active …" clears the binding back to the daemon's + // first-active fallback. + let optionsWithStale: [(label: String, value: String)] = + stale.map { name in + baseOptions + [(label: "\(name) (not found)", value: name)] + } ?? baseOptions VDropdown( placeholder: "Any active connection\u{2026}", selection: Binding( @@ -397,14 +423,7 @@ struct InferenceProfileEditor: View { profile.providerConnection = newValue.isEmpty ? nil : newValue } ), - options: [ - ( - label: "Any active \(store.dynamicProviderDisplayName(provider)) connection", - value: "" - ) - ] + available.map { conn in - (label: Self.connectionDisplayName(conn), value: conn.name) - } + options: optionsWithStale ) } } diff --git a/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileEditorTests.swift b/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileEditorTests.swift index 9a41e7b63de..c89a2558754 100644 --- a/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileEditorTests.swift +++ b/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileEditorTests.swift @@ -856,6 +856,126 @@ final class InferenceProfileEditorTests: XCTestCase { XCTAssertEqual(InferenceProfileEditor.connectionDisplayName(emptyLabel), "personal-openai") } + /// Stale binding detection: a saved `providerConnection` that points + /// at a name not present in the active-for-provider set surfaces as + /// `staleProviderConnection`. Gates the "Not found" badge + the extra + /// dropdown option that lets the user clear it. + func testStaleProviderConnectionReturnsNameWhenBindingMissing() { + let connections = [ + Self.makeConnection(name: "personal-openai", provider: "openai", status: .active), + ] + let editor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile( + name: "draft", + provider: "openai", + providerConnection: "ghost-openai" + )), + connections: connections, + onSave: {}, + onCancel: {} + ) + XCTAssertEqual(editor.staleProviderConnection, "ghost-openai") + } + + /// A disabled-status connection with a matching name is NOT in the + /// active-for-provider set, so the binding is "stale" from the + /// editor's POV (the daemon would skip it on dispatch). User can clear + /// or pick a different one. + func testStaleProviderConnectionReturnsNameWhenBindingMatchesDisabled() { + let connections = [ + Self.makeConnection(name: "legacy-openai", provider: "openai", status: .disabled), + ] + let editor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile( + name: "draft", + provider: "openai", + providerConnection: "legacy-openai" + )), + connections: connections, + onSave: {}, + onCancel: {} + ) + XCTAssertEqual(editor.staleProviderConnection, "legacy-openai") + } + + /// Binding resolves cleanly to an active row → `nil`, picker renders + /// in its non-stale shape. + func testStaleProviderConnectionNilWhenBindingMatches() { + let connections = [ + Self.makeConnection(name: "personal-openai", provider: "openai", status: .active), + ] + let editor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile( + name: "draft", + provider: "openai", + providerConnection: "personal-openai" + )), + connections: connections, + onSave: {}, + onCancel: {} + ) + XCTAssertNil(editor.staleProviderConnection) + } + + /// Empty / nil binding → `nil`. Picker renders in its default shape + /// when there are active matches; hides entirely when there are none. + func testStaleProviderConnectionNilWhenBindingEmpty() { + let connections = [ + Self.makeConnection(name: "personal-openai", provider: "openai", status: .active), + ] + let unboundEditor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile( + name: "draft", + provider: "openai", + providerConnection: nil + )), + connections: connections, + onSave: {}, + onCancel: {} + ) + XCTAssertNil(unboundEditor.staleProviderConnection) + + let emptyBindingEditor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile( + name: "draft", + provider: "openai", + providerConnection: "" + )), + connections: connections, + onSave: {}, + onCancel: {} + ) + XCTAssertNil(emptyBindingEditor.staleProviderConnection) + } + + /// Body must still build when the saved binding is stale — the new + /// codepath constructs an extended options list with the "(not found)" + /// entry, and any type-inference regression there would break the + /// SwiftUI compile. Safety net for the picker's stale-state UI. + func testEditorBodyBuildsWithStaleBinding() { + let connections = [ + Self.makeConnection(name: "personal-openai", provider: "openai", status: .active), + ] + let editor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile( + name: "draft", + provider: "openai", + providerConnection: "ghost-openai", + model: "gpt-5" + )), + connections: connections, + onSave: {}, + onCancel: {} + ) + XCTAssertNotNil(editor.body) + } + /// The editor must still build when a `connections:` list is passed in /// alongside other knobs — body construction is the safety net for any /// SwiftUI type-inference regression we'd otherwise miss until a