diff --git a/clients/macos/vellum-assistant/Features/Settings/InferenceProfileEditor.swift b/clients/macos/vellum-assistant/Features/Settings/InferenceProfileEditor.swift index 43183b4aaf..308fb03ab2 100644 --- a/clients/macos/vellum-assistant/Features/Settings/InferenceProfileEditor.swift +++ b/clients/macos/vellum-assistant/Features/Settings/InferenceProfileEditor.swift @@ -30,11 +30,21 @@ struct InferenceProfileEditor: View { /// 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 + /// specific row. Defaults to nil 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] = [] + /// + /// `nil` vs `[]` is meaningful: + /// - `nil` → the parent has not yet fetched `listProviderConnections` + /// (pre-load window between `.task` firing and the daemon response). + /// The provider picker falls back to the full catalog so the trigger + /// isn't empty during that gap. + /// - `[]` → the daemon returned zero connections. A fresh workspace + /// with nothing configured. The provider picker filters to empty, + /// the empty-state hint fires, and the user is steered to Providers + /// instead of being allowed to pick a non-dispatchable provider. + var connections: [ProviderConnection]? = nil let onSave: () -> Void var onSaveAs: (() -> Void)? let onCancel: () -> Void @@ -317,8 +327,39 @@ struct InferenceProfileEditor: View { .joined(separator: "-") } + /// Provider IDs visible in the picker. Filtered to providers that + /// have at least one ACTIVE connection — picking a provider with + /// zero active connections binds the profile to a route the daemon + /// can't dispatch through, leaving the user stuck. The currently- + /// bound `provider` is always kept in the list so editing/viewing a + /// stale profile (whose connection was disabled after the binding + /// was saved) still renders a sensible trigger. + /// + /// Pre-load fallback: when `connections` is `nil` (the sheet's + /// `.task` hasn't completed its first `listProviderConnections` + /// fetch yet, or an older daemon that doesn't surface the connection + /// list), return the full catalog so the user doesn't see an empty + /// picker on first open. An EMPTY-but-loaded `connections == []` is + /// distinct: the daemon confirmed zero connections, so the filter + /// runs and yields empty — the empty-state hint fires and steers + /// the user to Providers instead of letting them save a profile + /// bound to a non-dispatchable provider. + /// + /// Mirrors web's `visibleProviders` + `providerOptionsSource` in + /// `web/src/app/(app)/assistant/settings/ai/profile-editor-modal.tsx` + /// (PR #6509). The web sibling has the same nil-vs-empty trap and + /// is being addressed in a follow-up. var availableProviderIds: [String] { - store.dynamicProviderIds + guard let connections else { return store.dynamicProviderIds } + + var activeProviderSet = Set() + for connection in connections where connection.status == .active { + activeProviderSet.insert(connection.provider) + } + if let bound = profile.provider, !bound.isEmpty { + activeProviderSet.insert(bound) + } + return store.dynamicProviderIds.filter { activeProviderSet.contains($0) } } private var providerField: some View { @@ -369,14 +410,21 @@ struct InferenceProfileEditor: View { (label: store.dynamicProviderDisplayName(provider), value: provider) } ) + if availableProviderIds.isEmpty && !isReadOnly { + Text("No active provider connections. Open Providers to add or enable one.") + .font(VFont.bodySmallDefault) + .foregroundStyle(VColor.contentTertiary) + } } } /// Active connections that match the currently-selected provider. Used - /// by `connectionField` to populate its dropdown. + /// by `connectionField` to populate its dropdown. During pre-load + /// (`connections == nil`) there's nothing to pick — the connection + /// sub-dropdown stays hidden until the fetch completes. var availableConnectionsForProvider: [ProviderConnection] { guard let provider = profile.provider, !provider.isEmpty else { return [] } - return connections.filter { $0.provider == provider && $0.status == .active } + return (connections ?? []).filter { $0.provider == provider && $0.status == .active } } /// The currently-saved binding when it does NOT resolve to any active diff --git a/clients/macos/vellum-assistant/Features/Settings/InferenceProfilesSheet.swift b/clients/macos/vellum-assistant/Features/Settings/InferenceProfilesSheet.swift index 3187aa4773..7d3b9ff4d2 100644 --- a/clients/macos/vellum-assistant/Features/Settings/InferenceProfilesSheet.swift +++ b/clients/macos/vellum-assistant/Features/Settings/InferenceProfilesSheet.swift @@ -37,7 +37,13 @@ struct InferenceProfilesSheet: View { /// 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] = [] + /// + /// `nil` until the first `listProviderConnections` response lands — + /// the editor uses this to distinguish "still loading" from "loaded, + /// daemon returned zero connections." Without the distinction, a + /// fresh workspace would see the full provider catalog and could + /// bind a profile to a non-dispatchable provider. + @State private var connections: [ProviderConnection]? = nil @State private var editorState: EditorState? diff --git a/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileEditorTests.swift b/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileEditorTests.swift index c89a255875..028c72bcd4 100644 --- a/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileEditorTests.swift +++ b/clients/macos/vellum-assistantTests/Features/Settings/InferenceProfileEditorTests.swift @@ -1000,6 +1000,112 @@ final class InferenceProfileEditorTests: XCTestCase { XCTAssertNotNil(editor.body) } + // MARK: - Provider picker filter (iter3 QA issue #1, parity with web PR #6509) + + /// Only providers with at least one ACTIVE connection are surfaced in + /// the picker. A provider whose only connection is disabled (openai + /// below) must not appear, because binding a profile to it would + /// route through a credential the daemon will skip on dispatch. + func testAvailableProviderIdsHidesProvidersWithoutActiveConnection() { + let connections: [ProviderConnection] = [ + Self.makeConnection(name: "active-anthropic", provider: "anthropic", status: .active), + Self.makeConnection(name: "disabled-openai", provider: "openai", status: .disabled), + ] + let editor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile(name: "draft")), + connections: connections, + onSave: {}, + onCancel: {} + ) + XCTAssertEqual(editor.availableProviderIds, ["anthropic"]) + } + + /// Stale binding recovery: the editor is opened on a profile whose + /// `provider` value no longer has any active connection. The bound + /// provider must still appear in the picker so the user can see it + /// (and pick a different one) instead of finding an empty trigger. + func testAvailableProviderIdsKeepsCurrentBoundProvider() { + let connections: [ProviderConnection] = [ + Self.makeConnection(name: "active-anthropic", provider: "anthropic", status: .active), + Self.makeConnection(name: "disabled-openai", provider: "openai", status: .disabled), + ] + let editor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile( + name: "draft", + provider: "openai", + model: "gpt-5" + )), + connections: connections, + onSave: {}, + onCancel: {} + ) + // Order follows store.dynamicProviderIds catalog order, which + // happens to put anthropic before openai in the test fixture. + XCTAssertEqual(editor.availableProviderIds, ["anthropic", "openai"]) + } + + /// Pre-load fallback: when `connections` is nil (the parent sheet's + /// `.task` hasn't completed its first `listProviderConnections` + /// fetch yet, or the daemon predates the connections API), the + /// picker shows the full catalog so the user isn't faced with an + /// empty trigger during the network round-trip. Once connections + /// load — even to `[]` — the active-only filter kicks in. + /// + /// This is the half of the "nil vs []" distinction. The other half + /// is `testAvailableProviderIdsIsEmptyWhenConnectionsLoadedButEmpty` + /// below. + func testAvailableProviderIdsFallsBackToFullCatalogWhenConnectionsAreNil() { + let editor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile(name: "draft")), + onSave: {}, + onCancel: {} + ) + // Default `connections` is nil — the pre-load state. + XCTAssertEqual(editor.availableProviderIds, store.dynamicProviderIds) + XCTAssertFalse(editor.availableProviderIds.isEmpty) + } + + /// Loaded-but-empty: the daemon confirmed zero connections (fresh + /// workspace). This MUST NOT fall back to the full catalog — that + /// would let the user save a profile bound to a non-dispatchable + /// provider, which is the exact trap this PR is closing. The picker + /// renders empty; the empty-state hint elsewhere in the editor + /// steers the user to the Providers surface. + /// + /// Codex P1 (PR #30330): the original `guard !connections.isEmpty` + /// fallback conflated this case with pre-load and re-introduced the + /// QA trap for fresh workspaces. The fix is the nil/empty split. + func testAvailableProviderIdsIsEmptyWhenConnectionsLoadedButEmpty() { + let editor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile(name: "draft")), + connections: [], + onSave: {}, + onCancel: {} + ) + XCTAssertTrue(editor.availableProviderIds.isEmpty) + } + + /// All-disabled connections + no bound provider → the filter yields + /// empty. The picker still renders (the empty-state hint below it + /// drives the user to Providers), but no provider rows appear. + func testAvailableProviderIdsIsEmptyWhenOnlyDisabledConnectionsAndNoBoundProvider() { + let connections = [ + Self.makeConnection(name: "disabled-openai", provider: "openai", status: .disabled), + ] + let editor = InferenceProfileEditor( + store: store, + profile: .constant(InferenceProfile(name: "draft")), + connections: connections, + onSave: {}, + onCancel: {} + ) + XCTAssertTrue(editor.availableProviderIds.isEmpty) + } + /// Test helper mirroring `ProvidersSheetTests.makeConnection` so the /// two surfaces use identical fixture shapes. private static func makeConnection(