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 @@ -987,18 +987,35 @@ struct ProvidersSheet: View {

switch editorState {
case .create:
guard let created = await client.createProviderConnection(
let result = await client.createProviderConnection(
name: name,
provider: draft.provider,
auth: auth,
label: label,
status: status
) else {
)
switch result {
case .created(let created):
connections.append(created)
editorState = nil
case .duplicate:
// 409 — surface the actual reason instead of the generic
// fallback so users immediately see that the key (the
// editor's "Key" field, which becomes the connection's
// unique name) collides with an existing connection.
actionError = "A connection named \"\(name)\" already exists."
return
case .invalid(let message):
// 400 — daemon often includes a useful structured reason
// (e.g. `Invalid provider "x". Valid: ...`). Surface it
// verbatim when present; fall back to a generic invalid
// message otherwise.
actionError = message ?? "Invalid configuration. Check the provider and auth settings."
return
case .error:
actionError = "Couldn't create connection. Please try again."
return
}
connections.append(created)
editorState = nil

case .edit(let originalName), .managedEdit(let originalName):
// Managed-edit and user-edit share the same PATCH path. The auth
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ final class ProvidersSheetTests: XCTestCase {

func testCreateCallsClientWithExpectedArguments() async {
let created = makeConnection(name: "new-conn", provider: "openai", authType: "api_key")
mockClient.createResponse = created
mockClient.createResponse = .created(created)

_ = await mockClient.createProviderConnection(
name: "new-conn",
Expand Down Expand Up @@ -244,7 +244,7 @@ final class ProvidersSheetTests: XCTestCase {
authType: "api_key",
label: "Anthropic"
)
mockClient.createResponse = forked
mockClient.createResponse = .created(forked)

_ = await mockClient.createProviderConnection(
name: "anthropic-personal",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ final class MockProviderConnectionClient: ProviderConnectionClientProtocol {
var createAuthArg: ProviderConnectionAuth?
var createLabelArg: String?
var createStatusArg: ConnectionStatus?
var createResponse: ProviderConnection? = nil
var createResponse: ProviderConnectionCreateResult = .error

func createProviderConnection(name: String, provider: String, auth: ProviderConnectionAuth, label: String?, status: ConnectionStatus?) async -> ProviderConnection? {
func createProviderConnection(name: String, provider: String, auth: ProviderConnectionAuth, label: String?, status: ConnectionStatus?) async -> ProviderConnectionCreateResult {
createCallCount += 1
createNameArg = name
createProviderArg = provider
Expand Down Expand Up @@ -247,28 +247,55 @@ final class ProviderConnectionClientTests: XCTestCase {

func testCreateReturnsConnectionOnSuccess() async {
let conn = makeConnection(name: "new-conn")
mock.createResponse = conn
mock.createResponse = .created(conn)
let auth = ProviderConnectionAuth(type: "api_key", credential: "sk-test")
let result = await mock.createProviderConnection(name: "new-conn", provider: "anthropic", auth: auth, label: nil, status: nil)
XCTAssertEqual(result?.name, "new-conn")
guard case .created(let created) = result else {
XCTFail("Expected .created result, got \(result)")
return
}
XCTAssertEqual(created.name, "new-conn")
XCTAssertEqual(mock.createNameArg, "new-conn")
XCTAssertEqual(mock.createProviderArg, "anthropic")
XCTAssertEqual(mock.createAuthArg?.type, "api_key")
XCTAssertEqual(mock.createAuthArg?.credential, "sk-test")
}

func testCreateReturnsNilOn409NameConflict() async {
mock.createResponse = nil
func testCreateReturnsDuplicateOn409NameConflict() async {
// 409 from the daemon (a connection with this name already exists)
// surfaces as `.duplicate` so callers can show a precise message
// instead of a generic "please try again."
mock.createResponse = .duplicate
let auth = ProviderConnectionAuth(type: "api_key", credential: "sk-test")
let result = await mock.createProviderConnection(name: "existing", provider: "anthropic", auth: auth, label: nil, status: nil)
XCTAssertNil(result, "nil return models 409 name conflict")
guard case .duplicate = result else {
XCTFail("Expected .duplicate result, got \(result)")
return
}
}

func testCreateReturnsNilOn400InvalidAuth() async {
mock.createResponse = nil
func testCreateReturnsInvalidWithMessageOn400() async {
// 400 carries the daemon's structured reason when available so the
// sheet can echo it verbatim (e.g. `Invalid provider "x". Valid: ...`).
mock.createResponse = .invalid(message: "Invalid auth configuration.")
let auth = ProviderConnectionAuth(type: "api_key", credential: "")
let result = await mock.createProviderConnection(name: "conn", provider: "anthropic", auth: auth, label: nil, status: nil)
XCTAssertNil(result, "nil return models 400 invalid auth")
guard case .invalid(let message) = result else {
XCTFail("Expected .invalid result, got \(result)")
return
}
XCTAssertEqual(message, "Invalid auth configuration.")
}

func testCreateReturnsErrorOnUnknownFailure() async {
// Catch-all for network errors, 5xx, decode failures, etc.
mock.createResponse = .error
let auth = ProviderConnectionAuth(type: "api_key", credential: "sk-test")
let result = await mock.createProviderConnection(name: "conn", provider: "anthropic", auth: auth, label: nil, status: nil)
guard case .error = result else {
XCTFail("Expected .error result, got \(result)")
return
}
}

// MARK: - update
Expand Down
53 changes: 48 additions & 5 deletions clients/shared/Network/ProviderConnectionClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,30 @@ public enum ProviderConnectionDeleteResult: Sendable {
case error
}

/// Result of a `POST /v1/inference/provider-connections` request.
///
/// Differentiates the failure modes that warrant a specific user-facing
/// message — duplicate name (409) and invalid request (400) — from the
/// generic catch-all so the editor sheet can surface a precise reason
/// instead of a generic "please try again."
public enum ProviderConnectionCreateResult: Sendable {
case created(ProviderConnection)
/// 409 — a connection with the same name already exists.
case duplicate
/// 400 — invalid request. `message` carries the daemon's human-readable
/// reason (e.g. `Invalid provider "x". Valid: ...`) when present in the
/// `{ error: { message } }` envelope. May be nil if the body can't be
/// parsed; callers should provide a sensible fallback.
case invalid(message: String?)
/// Anything else (network failure, 5xx, decode error).
case error
}

public protocol ProviderConnectionClientProtocol {
func listProviderConnections(provider: String?) async -> [ProviderConnection]?
func getProviderConnection(name: String) async -> ProviderConnection?
/// `label` and `status` are optional extras; pass `nil` to omit from the request body.
func createProviderConnection(name: String, provider: String, auth: ProviderConnectionAuth, label: String?, status: ConnectionStatus?) async -> ProviderConnection?
func createProviderConnection(name: String, provider: String, auth: ProviderConnectionAuth, label: String?, status: ConnectionStatus?) async -> ProviderConnectionCreateResult
/// `status`: nil = omit from body (no change). `label`: nil outer = omit; `.some(nil)` = send null (clear); `.some("v")` = set.
func updateProviderConnection(name: String, auth: ProviderConnectionAuth, status: ConnectionStatus?, label: String??) async -> ProviderConnection?
func deleteProviderConnection(name: String) async -> ProviderConnectionDeleteResult
Expand Down Expand Up @@ -82,7 +101,7 @@ public struct ProviderConnectionClient: ProviderConnectionClientProtocol {
auth: ProviderConnectionAuth,
label: String? = nil,
status: ConnectionStatus? = nil
) async -> ProviderConnection? {
) async -> ProviderConnectionCreateResult {
var body: [String: Any] = [
"name": name,
"provider": provider,
Expand All @@ -95,15 +114,39 @@ public struct ProviderConnectionClient: ProviderConnectionClientProtocol {
path: "inference/provider-connections",
json: body
)
guard response.isSuccess else {
switch response.statusCode {
case 200..<300:
do {
let conn = try JSONDecoder().decode(ProviderConnection.self, from: response.data)
return .created(conn)
} catch {
log.error("createProviderConnection decode: \(error.localizedDescription, privacy: .public)")
return .error
}
case 409:
return .duplicate
case 400:
return .invalid(message: Self.parseErrorMessage(from: response.data))
default:
log.warning("POST /inference/provider-connections failed: \(response.statusCode)")
return nil
return .error
}
return try JSONDecoder().decode(ProviderConnection.self, from: response.data)
} catch {
log.error("createProviderConnection: \(error.localizedDescription, privacy: .public)")
return .error
}
}

/// Extract `error.message` from the standard daemon error envelope
/// `{ error: { code, message, details? } }`. Returns nil when the body
/// can't be parsed or doesn't follow the envelope.
private static func parseErrorMessage(from data: Data) -> String? {
guard let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any],
let error = json["error"] as? [String: Any],
let message = error["message"] as? String else {
return nil
}
return message
}

public func updateProviderConnection(
Expand Down