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,7 +374,16 @@ struct ProvidersSheet: View {
}
.onChange(of: editorDraft.provider) { _, newProvider in
if case .create = editorState, !newProvider.isEmpty {
editorDraft.credential = "credential/\(newProvider)/api_key"
if newProvider == "ollama" {
editorDraft.authType = "none"
editorDraft.credential = ""
} else {
if editorDraft.authType == "none" ||
(editorDraft.authType == "platform" && !store.isManagedCapable(newProvider)) {
editorDraft.authType = "api_key"
}
editorDraft.credential = "credential/\(newProvider)/api_key"
}
maskedCredentialValue = nil
Task { await loadAvailableCredentials() }
}
Expand Down Expand Up @@ -403,7 +412,8 @@ struct ProvidersSheet: View {
}
Spacer(minLength: 0)
VButton(
label: "Cancel",
label: "Close",
iconOnly: VIcon.x.rawValue,
style: .ghost,
tintColor: VColor.contentTertiary
) {
Expand Down Expand Up @@ -481,6 +491,33 @@ struct ProvidersSheet: View {
}
}

private var authTypeOptions: [(label: String, value: String)] {
let provider = editorDraft.provider
if provider == "ollama" {
return [(label: "None (no credentials)", value: "none")]
}
var options: [(label: String, value: String)] = [
(label: "API Key", value: "api_key"),
]
if store.isManagedCapable(provider) {
options.append((label: "Platform (managed by Vellum)", value: "platform"))
}
// Preserve the current auth type in edit mode so existing connections
// display their saved value even if the type is no longer offered for
// new connections (e.g. a non-ollama connection with "none" auth).
let current = editorDraft.authType
if !current.isEmpty && !options.contains(where: { $0.value == current }) {
let label: String = switch current {
case "none": "None (no credentials)"
case "platform": "Platform (managed by Vellum)"
case "api_key": "API Key"
default: current
}
options.append((label: label, value: current))
}
return options
}
Comment on lines +494 to +519
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Auth type dropdown loses current selection for existing non-ollama connections with auth type "none"

The new authTypeOptions computed property only includes "none" for the "ollama" provider. Previously, all providers offered "None (no credentials)" in the dropdown. If a user previously created a non-ollama connection with auth type "none" (which was allowed by the old UI), opening that connection in the editor via beginEdit (ProvidersSheet.swift:797) sets editorDraft.authType = conn.auth.type (i.e., "none"), but the dropdown options won't contain "none". The VDropdown's selectedLabel (VDropdown.swift:110-111) returns nil because no option matches, causing the dropdown to display the placeholder "Select auth type…" instead of the current value. This is confusing: the user may think no auth type is configured and inadvertently switch to "api_key" (which then demands an API key they may not have). While saving without touching the dropdown preserves the correct "none" auth type, the misleading display risks accidental misconfiguration.

Prompt for agents
The authTypeOptions computed property (ProvidersSheet.swift:493-505) restricts the 'None' auth option to only the 'ollama' provider. However, existing non-ollama connections may already have auth type 'none' (since the old UI allowed it for all providers). When editing such a connection, the VDropdown selection won't match any option, showing a confusing placeholder instead of the current value.

Possible fixes:
1. When in edit mode (not create), include the connection's current auth type in the options even if it wouldn't normally be available for that provider. This ensures the dropdown displays the current value.
2. Add a migration or fixup that converts existing non-ollama 'none' auth connections to a valid auth type.
3. Add a 'None' option to the options list when editorDraft.authType is already 'none' regardless of provider, e.g.:
   if editorDraft.authType == "none" { options.append((label: "None (no credentials)", value: "none")) }

The same concern applies to 'platform' auth type on non-managed-capable providers, though that scenario is less likely to occur in practice.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch — fixed in 94459f6. When editing an existing connection, the dropdown now includes the current auth type even if it wouldn't normally be offered for that provider. This prevents the confusing placeholder display for legacy non-ollama connections with "none" auth (and similarly for "platform" on non-managed providers). Applied the same fix to the web PR as well.


private var editorAuthTypeField: some View {
VStack(alignment: .leading, spacing: VSpacing.xs) {
Text("Auth Type")
Expand All @@ -489,11 +526,7 @@ struct ProvidersSheet: View {
VDropdown(
placeholder: "Select auth type\u{2026}",
selection: $editorDraft.authType,
options: [
(label: "API Key", value: "api_key"),
(label: "Platform (managed by Vellum)", value: "platform"),
(label: "None (no credentials)", value: "none"),
]
options: authTypeOptions
)
}
}
Expand Down Expand Up @@ -764,7 +797,11 @@ struct ProvidersSheet: View {
provider: provider
)
if !provider.isEmpty {
editorDraft.credential = "credential/\(provider)/api_key"
if provider == "ollama" {
editorDraft.authType = "none"
} else {
editorDraft.credential = "credential/\(provider)/api_key"
}
}
editorState = .create
maskedCredentialValue = nil
Expand Down Expand Up @@ -849,13 +886,13 @@ struct ProvidersSheet: View {
existingNames: Set(connections.map { $0.name })
)
isKeyDirty = true
// Default to api_key auth — the whole reason to clone a managed
// connection is to use your own key, so `platform` isn't the
// right default for the fork. Seed the credential reference to
// the provider's default slot so an immediate save (with no
// Advanced expansion) lands the key under `credential/<provider>/api_key`.
editorDraft.authType = "api_key"
editorDraft.credential = "credential/\(provider)/api_key"
if provider == "ollama" {
editorDraft.authType = "none"
editorDraft.credential = ""
} else {
editorDraft.authType = "api_key"
editorDraft.credential = "credential/\(provider)/api_key"
}
editorDraft.apiKeyValue = ""
// New connection starts active by convention; user can toggle off
// before saving if they want it disabled.
Expand Down
9 changes: 5 additions & 4 deletions clients/shared/DesignSystem/Core/Inputs/VDropdown.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public struct VDropdown<T: Hashable>: View {
@Binding public var selection: T
public var emptyValue: T?
public var maxWidth: CGFloat = .infinity
public var menuWidth: CGFloat = 180
public var menuWidth: CGFloat?
public var menuMaxHeight: CGFloat?
public var icon: VIcon?
public var optionIcon: ((T) -> VIcon?)?
Expand All @@ -60,7 +60,7 @@ public struct VDropdown<T: Hashable>: View {
selection: Binding<T>,
emptyValue: T? = nil,
maxWidth: CGFloat = .infinity,
menuWidth: CGFloat = 180,
menuWidth: CGFloat? = nil,
menuMaxHeight: CGFloat? = nil,
icon: VIcon? = nil,
optionIcon: ((T) -> VIcon?)? = nil,
Expand All @@ -87,7 +87,7 @@ public struct VDropdown<T: Hashable>: View {
options: [(label: String, value: T)],
emptyValue: T? = nil,
maxWidth: CGFloat = .infinity,
menuWidth: CGFloat = 180,
menuWidth: CGFloat? = nil,
menuMaxHeight: CGFloat? = nil,
icon: VIcon? = nil,
optionIcon: ((T) -> VIcon?)? = nil,
Expand Down Expand Up @@ -196,8 +196,9 @@ public struct VDropdown<T: Hashable>: View {
// window behind it (e.g. the Share Feedback modal over the main app
// window) — attaching to the wrong parent shoves the modal behind via
// `addChildWindow`.
let effectiveMenuWidth = menuWidth ?? triggerFrame.width
activePanel = VMenuPanel.show(at: screenPoint, sourceWindow: window, sourceAppearance: appearance, excludeRect: triggerScreenRect) {
VMenu(width: menuWidth, maxHeight: menuMaxHeight) {
VMenu(width: effectiveMenuWidth, maxHeight: menuMaxHeight) {
ForEach(optionList) { option in
VMenuItem(
icon: option.icon?.rawValue ?? optionIcon?(option.value)?.rawValue,
Expand Down