Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, Record<string, unknown>>;
}
).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<string, unknown> };
}
).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<string, Record<string, unknown>>;
}
).profiles.custom;

expect(savedProfile.provider_connection).toBeUndefined();
});
});
1 change: 1 addition & 0 deletions assistant/src/runtime/routes/conversation-query-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ const RESERVED_PROFILE_NAMES = new Set([

const INFERENCE_PROFILE_UI_KEYS = new Set([
"provider",
"provider_connection",
"model",
"maxTokens",
"effort",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -151,6 +166,7 @@ public struct InferenceProfile: Hashable, Identifiable {
label: label,
profileDescription: profileDescription,
provider: provider,
providerConnection: providerConnection,
model: model,
maxTokens: maxTokens,
effort: effort,
Expand All @@ -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 }
Expand All @@ -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 }
Expand Down Expand Up @@ -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 }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve clear semantics for provider_connection on save

When the user chooses “Any active … connection”, providerConnection becomes nil, and toJSON() omits provider_connection. However, the profile-replace route only clears keys listed in INFERENCE_PROFILE_UI_KEYS (in assistant/src/runtime/routes/conversation-query-routes.ts), and that set does not include provider_connection, so the old server-side binding is retained. The UI then appears cleared while runtime routing still uses the stale connection. Add an explicit clear mechanism for this field (or include it in the server-side UI key clear set).

Useful? React with 👍 / 👎.

if let model { result["model"] = model }
if let maxTokens { result["maxTokens"] = maxTokens }
if let effort { result["effort"] = effort }
Expand Down Expand Up @@ -266,6 +285,7 @@ public struct InferenceProfile: Hashable, Identifiable {
"label",
"description",
"provider",
"provider_connection",
"model",
"maxTokens",
"effort",
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -126,6 +134,7 @@ struct InferenceProfileEditor: View {
descriptionField
keyField
providerField
connectionField
modelField
if visibility.maxTokens {
maxTokensField
Expand Down Expand Up @@ -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 <provider> connection" matches
// the dispatcher's legacy behavior.
profile.providerConnection = nil
Self.clampMaxOutputTokensForSelectedModel(&profile)
Self.clampContextWindowForSelectedModel(&profile)
}
Expand All @@ -337,6 +352,92 @@ struct InferenceProfileEditor: View {
}
}

/// Active connections that match the currently-selected provider. Used
/// 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 }
}

/// 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
let stale = staleProviderConnection
if let provider = profile.provider,
!provider.isEmpty,
(!available.isEmpty || stale != nil) {
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 stale != nil {
VBadge(
label: "Not found",
tone: .warning,
emphasis: .subtle
)
}
}
) {
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(
get: { profile.providerConnection ?? "" },
set: { newValue in
profile.providerConnection = newValue.isEmpty ? nil : newValue
}
),
options: optionsWithStale
)
}
}
}

/// 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -301,6 +323,7 @@ struct InferenceProfilesSheet: View {
profile: $editorDraft,
isReadOnly: isViewMode,
isCreating: isCreateMode,
connections: connections,
onSave: {
Task { await commitEditor() }
},
Expand All @@ -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 {
Expand Down
Loading
Loading