From 25028e34057c4d0614165322ed904a525744f319 Mon Sep 17 00:00:00 2001 From: Rune Olsen Date: Thu, 13 Aug 2026 16:05:42 +0200 Subject: [PATCH] Add env-var support and native HTTP transport for External MCP Servers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit External MCP Servers previously only spoke stdio (spawn a local command + args). Adds: - env vars for stdio servers (merged into the subprocess environment, not embedded in the args string), with a masked key-value editor - a native Streamable HTTP transport (URL + Bearer token + custom headers), so HTTP-based MCP servers like Obsidian's Local REST API plugin connect directly without needing npx/Node.js as a bridge Introduces an MCPTransport abstraction (stdio/HTTP) so ExternalMCPClient stays transport-agnostic — mirrors how Provider.swift already abstracts AI backends in this codebase. Also fixes a real crash found via live testing against Obsidian: convertInputSchema force-unwrapped a tool parameter's `type`, which isn't required by JSON Schema — Obsidian's plugin was the first real server to send a parameter without one. Live-verified end to end (vault search/read/write/edit) before this commit, per the project's standing rule to hold external-service-dependent changes until they're actually confirmed working, not just compiling and passing tests. --- oAI/Localizable.xcstrings | 340 ++++++++++++++++++ oAI/Services/ExternalMCPClient.swift | 208 +++-------- oAI/Services/ExternalMCPManager.swift | 24 +- oAI/Services/ExternalMCPModels.swift | 59 ++- oAI/Services/MCPTransport.swift | 321 +++++++++++++++++ oAI/Views/Screens/SettingsView.swift | 164 +++++++-- .../ExternalMCPManagerConversionTests.swift | 65 ++++ oAITests/ExternalMCPModelsTests.swift | 105 ++++++ oAITests/MCPTransportTests.swift | 131 +++++++ 9 files changed, 1219 insertions(+), 198 deletions(-) create mode 100644 oAI/Services/MCPTransport.swift create mode 100644 oAITests/ExternalMCPManagerConversionTests.swift create mode 100644 oAITests/ExternalMCPModelsTests.swift create mode 100644 oAITests/MCPTransportTests.swift diff --git a/oAI/Localizable.xcstrings b/oAI/Localizable.xcstrings index 9ca6654..f66cf14 100644 --- a/oAI/Localizable.xcstrings +++ b/oAI/Localizable.xcstrings @@ -1,6 +1,346 @@ { "sourceLanguage" : "en", "strings" : { + "Remote (HTTP)" : { + "localizations" : { + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "Fjern (HTTP)" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Remote (HTTP)" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Distant (HTTP)" + } + }, + "nb" : { + "stringUnit" : { + "state" : "translated", + "value" : "Ekstern (HTTP)" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Fjärr (HTTP)" + } + } + } + }, + "Command: a local program Confab launches itself. Remote (HTTP): an already-running MCP server reachable by URL." : { + "localizations" : { + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "Kommando: et lokalt program Confab selv starter. Fjern (HTTP): en allerede kørende MCP-server, der kan nås via URL." + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Befehl: ein lokales Programm, das Confab selbst startet. Remote (HTTP): ein bereits laufender MCP-Server, der über eine URL erreichbar ist." + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Commande : un programme local que Confab lance lui-même. Distant (HTTP) : un serveur MCP déjà en cours d'exécution, accessible par URL." + } + }, + "nb" : { + "stringUnit" : { + "state" : "translated", + "value" : "Kommando: et lokalt program Confab selv starter. Ekstern (HTTP): en allerede kjørende MCP-server som kan nås via URL." + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Kommando: ett lokalt program som Confab själv startar. Fjärr (HTTP): en redan igångkörd MCP-server som nås via URL." + } + } + } + }, + "Type" : { + "localizations" : { + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "Type" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Typ" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Type" + } + }, + "nb" : { + "stringUnit" : { + "state" : "translated", + "value" : "Type" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Typ" + } + } + } + }, + "Bearer Token" : { + "localizations" : { + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "Bearer-token" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Bearer-Token" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Jeton Bearer" + } + }, + "nb" : { + "stringUnit" : { + "state" : "translated", + "value" : "Bearer-token" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Bearer-token" + } + } + } + }, + "Environment Variables" : { + "localizations" : { + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "Miljøvariabler" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Umgebungsvariablen" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Variables d'environnement" + } + }, + "nb" : { + "stringUnit" : { + "state" : "translated", + "value" : "Miljøvariabler" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Miljövariabler" + } + } + } + }, + "Extra Headers" : { + "localizations" : { + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "Ekstra headers" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Zusätzliche Header" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "En-têtes supplémentaires" + } + }, + "nb" : { + "stringUnit" : { + "state" : "translated", + "value" : "Ekstra headers" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Extra headers" + } + } + } + }, + "Optional" : { + "localizations" : { + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "Valgfri" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Optional" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Facultatif" + } + }, + "nb" : { + "stringUnit" : { + "state" : "translated", + "value" : "Valgfritt" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Valfritt" + } + } + } + }, + "Key" : { + "localizations" : { + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "Nøgle" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Schlüssel" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Clé" + } + }, + "nb" : { + "stringUnit" : { + "state" : "translated", + "value" : "Nøkkel" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Nyckel" + } + } + } + }, + "Value" : { + "localizations" : { + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "Værdi" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Wert" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Valeur" + } + }, + "nb" : { + "stringUnit" : { + "state" : "translated", + "value" : "Verdi" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Värde" + } + } + } + }, + "None configured" : { + "localizations" : { + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "Ingen konfigureret" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Keine konfiguriert" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Aucun configuré" + } + }, + "nb" : { + "stringUnit" : { + "state" : "translated", + "value" : "Ingen konfigurert" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Inga konfigurerade" + } + } + } + }, "By Provider" : { "localizations" : { "da" : { diff --git a/oAI/Services/ExternalMCPClient.swift b/oAI/Services/ExternalMCPClient.swift index 6b85464..1091b77 100644 --- a/oAI/Services/ExternalMCPClient.swift +++ b/oAI/Services/ExternalMCPClient.swift @@ -5,22 +5,18 @@ import Foundation // MARK: - ExternalMCPClient -/// Manages one MCP stdio server process. All state is MainActor-isolated -/// (consistent with SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor project setting). -/// Background I/O runs in Task.detached; state mutations hop back to MainActor. +/// Owns one MCP server connection's lifecycle and JSON-RPC message framing. Delivery (stdio +/// subprocess vs Streamable HTTP) is delegated to a `MCPTransport` — this class only builds +/// envelopes, decodes typed results, and tracks connection state. +/// All state is MainActor-isolated (consistent with SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor +/// project setting). @MainActor final class ExternalMCPClient { let server: ExternalMCPServer weak var stateDelegate: (any ExternalMCPStateDelegate)? - private var process: Process? - private var stdinHandle: FileHandle? - private var readTask: Task? - private var stderrTask: Task? - + private let transport: any MCPTransport private var nextRequestId: Int = 1 - private var pendingCalls: [Int: CheckedContinuation] = [:] - private var lineBuffer = Data() private(set) var state: MCPClientState = .idle private(set) var discoveredTools: [MCPToolDefinition] = [] @@ -28,6 +24,31 @@ final class ExternalMCPClient { init(server: ExternalMCPServer, stateDelegate: (any ExternalMCPStateDelegate)?) { self.server = server self.stateDelegate = stateDelegate + let stdioTransport: StdioMCPTransport? + switch server.transportKind { + case .stdio: + let t = StdioMCPTransport(server: server) + stdioTransport = t + self.transport = t + case .http: + stdioTransport = nil + self.transport = HTTPMCPTransport(server: server) + } + // `self` is only safe to capture once every stored property above has a value — + // wire the crash callback here, after `init` would otherwise be considered complete. + stdioTransport?.onTerminated = { [weak self] in + self?.handleTransportTerminatedUnexpectedly() + } + } + + /// Called by a stdio transport whose subprocess died on its own — as opposed to a + /// deliberate `stop()` call, or a failure already handled inline within `start()`. + /// No HTTP equivalent: a Streamable HTTP connection has no persistent process to crash; + /// its failures surface per-request instead (handled in `start()`/`callTool()` directly). + private func handleTransportTerminatedUnexpectedly() { + guard state != .stopped else { return } + state = .crashed + stateDelegate?.clientDidChangeState(id: server.id, state: .crashed) } // MARK: - Lifecycle @@ -37,54 +58,28 @@ final class ExternalMCPClient { state = .connecting stateDelegate?.clientDidChangeState(id: server.id, state: .connecting) - let proc = Process() - if server.command.hasPrefix("/") { - proc.executableURL = URL(fileURLWithPath: server.command) - proc.arguments = server.args - } else { - proc.executableURL = URL(fileURLWithPath: "/usr/bin/env") - proc.arguments = [server.command] + server.args - } - proc.environment = ProcessInfo.processInfo.environment - - let stdinPipe = Pipe() - let stdoutPipe = Pipe() - let stderrPipe = Pipe() - proc.standardInput = stdinPipe - proc.standardOutput = stdoutPipe - proc.standardError = stderrPipe - - proc.terminationHandler = { [weak self] _ in - Task { @MainActor [weak self] in self?.handleProcessTerminated() } - } - do { - try proc.run() - } catch { - state = .error(error.localizedDescription) - stateDelegate?.clientDidChangeState(id: server.id, state: .error(error.localizedDescription)) - throw MCPClientError.processLaunchFailed(error.localizedDescription) - } + try await transport.prepare() - process = proc - stdinHandle = stdinPipe.fileHandleForWriting - startReadLoop(pipe: stdoutPipe) - startStderrLoop(pipe: stderrPipe) - - do { let _: MCPInitializeResult = try await timedRequest(seconds: 15, method: "initialize", params: [ "protocolVersion": "2024-11-05", "capabilities": [:] as [String: Any], "clientInfo": ["name": "Confab", "version": "1.0"] as [String: Any] ]) - try sendNotification(method: "notifications/initialized") + try await transport.sendNotification(["jsonrpc": "2.0", "method": "notifications/initialized"]) let toolsResult: MCPToolsListResult = try await timedRequest(seconds: 15, method: "tools/list", params: nil) discoveredTools = toolsResult.tools } catch { - state = .error(error.localizedDescription) - stateDelegate?.clientDidChangeState(id: server.id, state: .error(error.localizedDescription)) - proc.terminate() + // Uniformly route every start() failure (bad config, launch failure, handshake + // failure — for either transport) through .crashed, not .error, so + // ExternalMCPManager's restart-with-backoff drives from exactly one place. + // (Previously, stdio relied on the subprocess's termination handler firing + // asynchronously to reach .crashed; that path doesn't exist for HTTP, so failures + // there would otherwise get stuck at .error with no retry.) + state = .crashed + transport.stop() + stateDelegate?.clientDidChangeState(id: server.id, state: .crashed) throw error } @@ -94,14 +89,7 @@ final class ExternalMCPClient { func stop() { state = .stopped - readTask?.cancel() - stderrTask?.cancel() - process?.terminate() - process = nil - stdinHandle = nil - lineBuffer = Data() - for (_, cont) in pendingCalls { cont.resume(throwing: MCPClientError.notConnected) } - pendingCalls.removeAll() + transport.stop() } // MARK: - Tool Execution @@ -128,125 +116,17 @@ final class ExternalMCPClient { } } - // MARK: - I/O Loops (detached from MainActor) - - private func startReadLoop(pipe: Pipe) { - readTask = Task.detached { [weak self] in - let handle = pipe.fileHandleForReading - while true { - let data = handle.availableData - if data.isEmpty { break } - await self?.receiveData(data) - } - } - } - - private func startStderrLoop(pipe: Pipe) { - let name = server.name - stderrTask = Task.detached { - let handle = pipe.fileHandleForReading - var buf = Data() - while true { - let data = handle.availableData - if data.isEmpty { break } - buf.append(data) - while let idx = buf.firstIndex(of: UInt8(ascii: "\n")) { - let line = String(data: buf[buf.startIndex..(seconds: Double, method: String, params: [String: Any]?) async throws -> T { let id = nextRequestId nextRequestId += 1 var message: [String: Any] = ["jsonrpc": "2.0", "method": method, "id": id] if let params { message["params"] = params } - try writeJSON(message) - - // Schedule timeout: cancels the specific pending call by ID - let timeoutId = id - Task { [weak self, timeoutId] in - try? await Task.sleep(nanoseconds: UInt64(seconds * 1_000_000_000)) - self?.cancelPendingCall(id: timeoutId, with: MCPClientError.timeout) - } - - // Await response data, then decode on MainActor - let resultData: Data = try await withCheckedThrowingContinuation { cont in - pendingCalls[id] = cont - } + let resultData = try await transport.sendRequest(message, id: id, timeoutSeconds: seconds) return try JSONDecoder().decode(T.self, from: resultData) } - private func cancelPendingCall(id: Int, with error: Error) { - pendingCalls.removeValue(forKey: id)?.resume(throwing: error) - } - - private func sendNotification(method: String) throws { - try writeJSON(["jsonrpc": "2.0", "method": method]) - } - - private func writeJSON(_ message: [String: Any]) throws { - guard let handle = stdinHandle, process?.isRunning == true else { - throw MCPClientError.writeFailed - } - guard let data = try? JSONSerialization.data(withJSONObject: message), - let line = String(data: data, encoding: .utf8) else { - throw MCPClientError.writeFailed - } - do { - try handle.write(contentsOf: Data((line + "\n").utf8)) - } catch { - throw MCPClientError.writeFailed - } - } - - // MARK: - Process termination - - private func handleProcessTerminated() { - guard state != .stopped else { return } - state = .crashed - for (_, cont) in pendingCalls { cont.resume(throwing: MCPClientError.notConnected) } - pendingCalls.removeAll() - stateDelegate?.clientDidChangeState(id: server.id, state: .crashed) - } - // MARK: - Result conversion private func convertMCPResult(_ result: MCPToolCallResult) -> [String: Any] { diff --git a/oAI/Services/ExternalMCPManager.swift b/oAI/Services/ExternalMCPManager.swift index f05f5b0..be858a1 100644 --- a/oAI/Services/ExternalMCPManager.swift +++ b/oAI/Services/ExternalMCPManager.swift @@ -67,13 +67,10 @@ final class ExternalMCPManager { Task { do { try await client.start() - } catch MCPClientError.processLaunchFailed(let msg) { - // Process never started — termination handler won't fire, so manually trigger crashed - Log.extMcp.error("Failed to launch '\(server.name)': \(msg)") - clientDidChangeState(id: server.id, state: .crashed) } catch { - // Handshake/other failure — proc.terminate() was called in start(), termination - // handler will fire and set .crashed, which drives the restart from one place only. + // start() already notified stateDelegate with .crashed (uniformly, for both + // transports and every failure kind) before throwing — restart-with-backoff is + // already scheduled via clientDidChangeState. Nothing further to do but log. Log.extMcp.warning("'\(server.name)' start failed: \(error.localizedDescription)") } } @@ -126,7 +123,7 @@ final class ExternalMCPManager { private func rebuildCache(for server: ExternalMCPServer, tools: [MCPToolDefinition]) { removeCachedSchemas(for: server.id, slug: server.slug) - let prefixed = tools.compactMap { convertToolDefinition($0, server: server) } + let prefixed = tools.compactMap { Self.convertToolDefinition($0, server: server) } cachedToolSchemas.append(contentsOf: prefixed) Log.extMcp.info("[\(server.name)] cached \(prefixed.count) tools: \(prefixed.map { $0.function.name }.joined(separator: ", "))") } @@ -140,7 +137,7 @@ final class ExternalMCPManager { cachedToolSchemas.removeAll { $0.function.name.hasPrefix("\(slug)_") } } - private func convertToolDefinition(_ def: MCPToolDefinition, server: ExternalMCPServer) -> Tool? { + nonisolated static func convertToolDefinition(_ def: MCPToolDefinition, server: ExternalMCPServer) -> Tool? { Tool( type: "function", function: Tool.Function( @@ -151,13 +148,18 @@ final class ExternalMCPManager { ) } - private func convertInputSchema(_ schema: MCPInputSchema) -> Tool.Function.Parameters { + /// A schema property with no `"type"` at all is valid JSON Schema (e.g. an `enum`-only or + /// composed property) — not every MCP server's tool schemas set it, so this must not assume + /// it's present. (Found via a real crash: Obsidian's Local REST API plugin sends at least one + /// tool parameter with no `type`, which a `prop.type!` force-unwrap here used to crash on.) + nonisolated static func convertInputSchema(_ schema: MCPInputSchema) -> Tool.Function.Parameters { var properties: [String: Tool.Function.Parameters.Property] = [:] for (key, prop) in schema.properties ?? [:] { + let effectiveType = prop.type ?? "string" let normalized: String - switch prop.type ?? "string" { + switch effectiveType { case "integer": normalized = "number" - case "string", "number", "boolean", "array", "object": normalized = prop.type! + case "string", "number", "boolean", "array", "object": normalized = effectiveType default: normalized = "string" } var items: Tool.Function.Parameters.Property.Items? = nil diff --git a/oAI/Services/ExternalMCPModels.swift b/oAI/Services/ExternalMCPModels.swift index 5bcde08..e6e221c 100644 --- a/oAI/Services/ExternalMCPModels.swift +++ b/oAI/Services/ExternalMCPModels.swift @@ -5,26 +5,79 @@ import Foundation // MARK: - Server Configuration +/// Which wire protocol an `ExternalMCPServer` uses. `.stdio` fields are `command`/`args`/`env`; +/// `.http` fields are `url`/`bearerToken`/`headers`. Kept as one flat struct rather than an enum +/// with associated values — simpler `Codable` and simpler settings-JSON storage, at the cost of +/// each server config carrying some always-unused fields for its transport. +enum MCPTransportKind: String, Codable, Sendable, CaseIterable { + case stdio + case http +} + struct ExternalMCPServer: Codable, Identifiable, Sendable { var id: UUID var name: String + var transportKind: MCPTransportKind var command: String var args: [String] + var env: [String: String] + var url: String + var bearerToken: String + var headers: [String: String] var isEnabled: Bool var timeout: TimeInterval var createdAt: Date - init(id: UUID = UUID(), name: String, command: String, args: [String] = [], - isEnabled: Bool = true, timeout: TimeInterval = 30, createdAt: Date = Date()) { + init( + id: UUID = UUID(), + name: String, + transportKind: MCPTransportKind = .stdio, + command: String = "", + args: [String] = [], + env: [String: String] = [:], + url: String = "", + bearerToken: String = "", + headers: [String: String] = [:], + isEnabled: Bool = true, + timeout: TimeInterval = 30, + createdAt: Date = Date() + ) { self.id = id self.name = name + self.transportKind = transportKind self.command = command self.args = args + self.env = env + self.url = url + self.bearerToken = bearerToken + self.headers = headers self.isEnabled = isEnabled self.timeout = timeout self.createdAt = createdAt } + enum CodingKeys: String, CodingKey { + case id, name, transportKind, command, args, env, url, bearerToken, headers, isEnabled, timeout, createdAt + } + + /// Custom decoding so servers saved before `transportKind`/`env`/`url`/`bearerToken`/`headers` + /// existed (plain stdio-only configs) still decode — those keys default rather than fail. + init(from decoder: Decoder) throws { + let c = try decoder.container(keyedBy: CodingKeys.self) + id = try c.decode(UUID.self, forKey: .id) + name = try c.decode(String.self, forKey: .name) + command = try c.decode(String.self, forKey: .command) + args = try c.decode([String].self, forKey: .args) + isEnabled = try c.decode(Bool.self, forKey: .isEnabled) + timeout = try c.decode(TimeInterval.self, forKey: .timeout) + createdAt = try c.decode(Date.self, forKey: .createdAt) + transportKind = try c.decodeIfPresent(MCPTransportKind.self, forKey: .transportKind) ?? .stdio + env = try c.decodeIfPresent([String: String].self, forKey: .env) ?? [:] + url = try c.decodeIfPresent(String.self, forKey: .url) ?? "" + bearerToken = try c.decodeIfPresent(String.self, forKey: .bearerToken) ?? "" + headers = try c.decodeIfPresent([String: String].self, forKey: .headers) ?? [:] + } + var slug: String { Self.makeSlug(from: name) } static func makeSlug(from name: String) -> String { @@ -100,6 +153,7 @@ enum MCPClientError: LocalizedError { case processLaunchFailed(String) case handshakeFailed(String) case writeFailed + case invalidConfiguration(String) var errorDescription: String? { switch self { @@ -109,6 +163,7 @@ enum MCPClientError: LocalizedError { case .processLaunchFailed(let s): return "Failed to launch MCP server: \(s)" case .handshakeFailed(let s): return "MCP handshake failed: \(s)" case .writeFailed: return "Failed to write to MCP server stdin" + case .invalidConfiguration(let s): return "Invalid MCP server configuration: \(s)" } } } diff --git a/oAI/Services/MCPTransport.swift b/oAI/Services/MCPTransport.swift new file mode 100644 index 0000000..a051a71 --- /dev/null +++ b/oAI/Services/MCPTransport.swift @@ -0,0 +1,321 @@ +// SPDX-License-Identifier: PolyForm-Noncommercial-1.0.0 +// Copyright (C) 2026 Rune Olsen + +import Foundation + +// MARK: - Transport abstraction + +/// One MCP server connection's wire protocol. `ExternalMCPClient` builds JSON-RPC envelopes and +/// decodes typed results; the transport only owns *delivery* — how the bytes get to the server +/// and back. A long-lived subprocess pipe for stdio, discrete HTTP requests for Streamable HTTP. +@MainActor +protocol MCPTransport: AnyObject { + /// Prepares the transport for use — launches the subprocess for stdio; a no-op for HTTP, + /// since there's no persistent connection to establish ahead of the first request. + func prepare() async throws + /// Sends a JSON-RPC *request* (`message` includes `"id"`) and returns its `result` payload. + /// Throws `MCPClientError.invalidResponse` if the server returned a JSON-RPC error, or + /// `.timeout` if `timeoutSeconds` elapses first. + func sendRequest(_ message: [String: Any], id: Int, timeoutSeconds: Double) async throws -> Data + /// Sends a JSON-RPC *notification* (no `"id"`, no response expected). + func sendNotification(_ message: [String: Any]) async throws + /// Tears down the transport — terminates the subprocess / drops HTTP session state. + func stop() +} + +enum MCPTransportSupport { + /// Extracts the `result` payload from a decoded top-level JSON-RPC response object, shared + /// by every transport so error/result semantics stay identical regardless of wire protocol. + static func extractResult(from json: [String: Any]) throws -> Data { + if let err = json["error"] as? [String: Any] { + throw MCPClientError.invalidResponse(err["message"] as? String ?? "Unknown error") + } + guard let result = json["result"], + let resultData = try? JSONSerialization.data(withJSONObject: result) + else { + throw MCPClientError.invalidResponse("Missing result field") + } + return resultData + } +} + +// MARK: - Stdio Transport + +/// Launches the server as a subprocess and speaks newline-delimited JSON-RPC over its +/// stdin/stdout, exactly as the original (pre-transport-split) `ExternalMCPClient` did. +@MainActor +final class StdioMCPTransport: MCPTransport { + private let server: ExternalMCPServer + /// Set by the owner after construction (not an init parameter) — the owner (`ExternalMCPClient`) + /// needs to capture `self` weakly to wire this up, which Swift only allows once `self` is + /// fully initialized, i.e. after its own `init` has finished assigning all stored properties. + var onTerminated: (() -> Void)? + + private var process: Process? + private var stdinHandle: FileHandle? + private var readTask: Task? + private var stderrTask: Task? + private var pendingCalls: [Int: CheckedContinuation] = [:] + private var lineBuffer = Data() + + init(server: ExternalMCPServer) { + self.server = server + } + + func prepare() async throws { + let proc = Process() + if server.command.hasPrefix("/") { + proc.executableURL = URL(fileURLWithPath: server.command) + proc.arguments = server.args + } else { + proc.executableURL = URL(fileURLWithPath: "/usr/bin/env") + proc.arguments = [server.command] + server.args + } + // Inherited environment first (so PATH etc. still resolves — e.g. `npx` needs PATH to + // find node), then layer the user-configured vars over it so they can override. + var environment = ProcessInfo.processInfo.environment + for (key, value) in server.env { environment[key] = value } + proc.environment = environment + + let stdinPipe = Pipe() + let stdoutPipe = Pipe() + let stderrPipe = Pipe() + proc.standardInput = stdinPipe + proc.standardOutput = stdoutPipe + proc.standardError = stderrPipe + + proc.terminationHandler = { [weak self] _ in + Task { @MainActor [weak self] in self?.handleProcessTerminated() } + } + + do { + try proc.run() + } catch { + throw MCPClientError.processLaunchFailed(error.localizedDescription) + } + + process = proc + stdinHandle = stdinPipe.fileHandleForWriting + startReadLoop(pipe: stdoutPipe) + startStderrLoop(pipe: stderrPipe) + } + + func sendRequest(_ message: [String: Any], id: Int, timeoutSeconds: Double) async throws -> Data { + try writeJSON(message) + + // Schedule timeout: cancels the specific pending call by ID + let timeoutId = id + Task { [weak self, timeoutId] in + try? await Task.sleep(nanoseconds: UInt64(timeoutSeconds * 1_000_000_000)) + self?.cancelPendingCall(id: timeoutId, with: MCPClientError.timeout) + } + + return try await withCheckedThrowingContinuation { cont in + pendingCalls[id] = cont + } + } + + func sendNotification(_ message: [String: Any]) async throws { + try writeJSON(message) + } + + func stop() { + readTask?.cancel() + stderrTask?.cancel() + process?.terminate() + process = nil + stdinHandle = nil + lineBuffer = Data() + for (_, cont) in pendingCalls { cont.resume(throwing: MCPClientError.notConnected) } + pendingCalls.removeAll() + } + + // MARK: I/O Loops (detached from MainActor) + + private func startReadLoop(pipe: Pipe) { + readTask = Task.detached { [weak self] in + let handle = pipe.fileHandleForReading + while true { + let data = handle.availableData + if data.isEmpty { break } + await self?.receiveData(data) + } + } + } + + private func startStderrLoop(pipe: Pipe) { + let name = server.name + stderrTask = Task.detached { + let handle = pipe.fileHandleForReading + var buf = Data() + while true { + let data = handle.availableData + if data.isEmpty { break } + buf.append(data) + while let idx = buf.firstIndex(of: UInt8(ascii: "\n")) { + let line = String(data: buf[buf.startIndex.. Data { + let request = try buildRequest(for: message, timeoutSeconds: timeoutSeconds) + let (data, response) = try await urlSession.data(for: request) + guard let http = response as? HTTPURLResponse else { + throw MCPClientError.invalidResponse("Non-HTTP response") + } + if let newSessionId = http.value(forHTTPHeaderField: "Mcp-Session-Id") { + sessionId = newSessionId + } + guard (200...299).contains(http.statusCode) else { + throw MCPClientError.invalidResponse("HTTP \(http.statusCode)") + } + let json = try Self.parseResponseBody( + data, contentType: http.value(forHTTPHeaderField: "Content-Type"), expectedId: id + ) + return try MCPTransportSupport.extractResult(from: json) + } + + func sendNotification(_ message: [String: Any]) async throws { + // Notifications carry no "id", so per spec the server responds 202 Accepted, no body. + let request = try buildRequest(for: message, timeoutSeconds: 15) + let (_, response) = try await urlSession.data(for: request) + guard let http = response as? HTTPURLResponse, (200...299).contains(http.statusCode) else { + throw MCPClientError.invalidResponse("Notification not accepted") + } + } + + func stop() { + sessionId = nil + } + + private func buildRequest(for message: [String: Any], timeoutSeconds: Double) throws -> URLRequest { + guard let url = URL(string: server.url) else { + throw MCPClientError.invalidConfiguration("Invalid server URL: \(server.url)") + } + var request = URLRequest(url: url, timeoutInterval: timeoutSeconds) + request.httpMethod = "POST" + request.setValue("application/json", forHTTPHeaderField: "Content-Type") + request.setValue("application/json, text/event-stream", forHTTPHeaderField: "Accept") + request.setValue("2025-06-18", forHTTPHeaderField: "MCP-Protocol-Version") + if !server.bearerToken.isEmpty { + request.setValue("Bearer \(server.bearerToken)", forHTTPHeaderField: "Authorization") + } + for (key, value) in server.headers { + request.setValue(value, forHTTPHeaderField: key) + } + if let sessionId { + request.setValue(sessionId, forHTTPHeaderField: "Mcp-Session-Id") + } + request.httpBody = try JSONSerialization.data(withJSONObject: message) + return request + } + + /// Parses either a direct `application/json` body, or an SSE (`text/event-stream`) body — + /// scanning its `data:` lines for the JSON-RPC message whose `id` matches `expectedId` (the + /// server may send unrelated requests/notifications on the same stream first, per spec). + nonisolated static func parseResponseBody(_ data: Data, contentType: String?, expectedId: Int) throws -> [String: Any] { + if contentType?.contains("text/event-stream") == true { + guard let text = String(data: data, encoding: .utf8) else { + throw MCPClientError.invalidResponse("Non-UTF8 SSE body") + } + for line in text.components(separatedBy: "\n") { + let trimmed = line.trimmingCharacters(in: .whitespaces) + guard trimmed.hasPrefix("data:") else { continue } + let payload = trimmed.dropFirst("data:".count).trimmingCharacters(in: .whitespaces) + guard let payloadData = payload.data(using: .utf8), + let json = try? JSONSerialization.jsonObject(with: payloadData) as? [String: Any], + let id = json["id"] as? Int, id == expectedId + else { continue } + return json + } + throw MCPClientError.invalidResponse("No matching response in SSE stream") + } + guard let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any] else { + throw MCPClientError.invalidResponse("Malformed JSON response body") + } + return json + } +} diff --git a/oAI/Views/Screens/SettingsView.swift b/oAI/Views/Screens/SettingsView.swift index 1c30b9b..a907bba 100644 --- a/oAI/Views/Screens/SettingsView.swift +++ b/oAI/Views/Screens/SettingsView.swift @@ -25,6 +25,13 @@ import SwiftUI import UniformTypeIdentifiers import FoundationModels +/// One editable row in the External MCP Servers "add" sheet's env var / custom header lists. +private struct MCPKeyValuePair: Identifiable { + let id = UUID() + var key: String = "" + var value: String = "" +} + struct SettingsView: View { @Environment(\.dismiss) var dismiss @Bindable private var settingsService = SettingsService.shared @@ -77,8 +84,13 @@ struct SettingsView: View { // External MCP Servers state @State private var showAddExternalMCPServer = false @State private var newMCPServerName = "" + @State private var newMCPServerTransportKind: MCPTransportKind = .stdio @State private var newMCPServerCommand = "" @State private var newMCPServerArgs = "" + @State private var newMCPServerEnvPairs: [MCPKeyValuePair] = [] + @State private var newMCPServerURL = "" + @State private var newMCPServerBearerToken = "" + @State private var newMCPServerHeaderPairs: [MCPKeyValuePair] = [] @State private var newMCPServerTimeout: Double = 30 private var externalMCPManager = ExternalMCPManager.shared @@ -1049,7 +1061,7 @@ It's better to admit "I need more information" or "I cannot do that" than to fak VStack(alignment: .leading, spacing: 2) { Text(server.name) .font(.system(size: 14)) - Text(([server.command] + server.args).joined(separator: " ")) + Text(server.transportKind == .http ? server.url : ([server.command] + server.args).joined(separator: " ")) .font(.system(size: 11, design: .monospaced)) .foregroundStyle(.secondary) .lineLimit(1) @@ -1084,8 +1096,13 @@ It's better to admit "I need more information" or "I cannot do that" than to fak Button { newMCPServerName = "" + newMCPServerTransportKind = .stdio newMCPServerCommand = "" newMCPServerArgs = "" + newMCPServerEnvPairs = [] + newMCPServerURL = "" + newMCPServerBearerToken = "" + newMCPServerHeaderPairs = [] newMCPServerTimeout = 30 showAddExternalMCPServer = true } label: { @@ -1272,19 +1289,47 @@ It's better to admit "I need more information" or "I cannot do that" than to fak .frame(width: 240) } rowDivider() - row("Command") { - TextField("safaridriver", text: $newMCPServerCommand) - .textFieldStyle(.roundedBorder) - .font(.system(size: 13, design: .monospaced)) - .frame(width: 240) + row("Type") { + Picker("", selection: $newMCPServerTransportKind) { + Text("Command").tag(MCPTransportKind.stdio) + Text("Remote (HTTP)").tag(MCPTransportKind.http) + } + .pickerStyle(.segmented) + .labelsHidden() + .frame(width: 240) + .help("Command: a local program Confab launches itself. Remote (HTTP): an already-running MCP server reachable by URL.") } rowDivider() - row("Arguments") { - TextField("--mcp", text: $newMCPServerArgs) - .textFieldStyle(.roundedBorder) - .font(.system(size: 13, design: .monospaced)) - .frame(width: 240) - .help("Space-separated arguments") + switch newMCPServerTransportKind { + case .stdio: + row("Command") { + TextField("safaridriver", text: $newMCPServerCommand) + .textFieldStyle(.roundedBorder) + .font(.system(size: 13, design: .monospaced)) + .frame(width: 240) + } + rowDivider() + row("Arguments") { + TextField("--mcp", text: $newMCPServerArgs) + .textFieldStyle(.roundedBorder) + .font(.system(size: 13, design: .monospaced)) + .frame(width: 240) + .help("Space-separated arguments") + } + case .http: + row("Server URL") { + TextField("http://127.0.0.1:27123/mcp/", text: $newMCPServerURL) + .textFieldStyle(.roundedBorder) + .font(.system(size: 13, design: .monospaced)) + .frame(width: 240) + } + rowDivider() + row("Bearer Token") { + SecureField("Optional", text: $newMCPServerBearerToken) + .textFieldStyle(.roundedBorder) + .font(.system(size: 13, design: .monospaced)) + .frame(width: 240) + } } rowDivider() row("Timeout") { @@ -1299,6 +1344,15 @@ It's better to admit "I need more information" or "I cannot do that" than to fak } } + formSection { + switch newMCPServerTransportKind { + case .stdio: + mcpKeyValueEditor(title: "Environment Variables", pairs: $newMCPServerEnvPairs) + case .http: + mcpKeyValueEditor(title: "Extra Headers", pairs: $newMCPServerHeaderPairs) + } + } + if !newMCPServerName.isEmpty { let slug = ExternalMCPServer.makeSlug(from: newMCPServerName) HStack(spacing: 6) { @@ -1320,25 +1374,93 @@ It's better to admit "I need more information" or "I cannot do that" than to fak Button("Cancel") { showAddExternalMCPServer = false } Spacer() Button("Add") { - let args = ExternalMCPServer.parseArguments(newMCPServerArgs) - let server = ExternalMCPServer( - name: newMCPServerName, - command: newMCPServerCommand, - args: args, - timeout: newMCPServerTimeout - ) + let server: ExternalMCPServer + switch newMCPServerTransportKind { + case .stdio: + server = ExternalMCPServer( + name: newMCPServerName, + transportKind: .stdio, + command: newMCPServerCommand, + args: ExternalMCPServer.parseArguments(newMCPServerArgs), + env: mcpDictionary(from: newMCPServerEnvPairs), + timeout: newMCPServerTimeout + ) + case .http: + server = ExternalMCPServer( + name: newMCPServerName, + transportKind: .http, + url: newMCPServerURL, + bearerToken: newMCPServerBearerToken, + headers: mcpDictionary(from: newMCPServerHeaderPairs), + timeout: newMCPServerTimeout + ) + } settingsService.addExternalMCPServer(server) showAddExternalMCPServer = false } .buttonStyle(.borderedProminent) - .disabled(newMCPServerName.isEmpty || newMCPServerCommand.isEmpty || - ExternalMCPServer.reservedSlugs.contains(ExternalMCPServer.makeSlug(from: newMCPServerName))) + .disabled(isAddExternalMCPServerDisabled) } } .padding(24) .frame(minWidth: 460, minHeight: 320) } + private var isAddExternalMCPServerDisabled: Bool { + if newMCPServerName.isEmpty { return true } + if ExternalMCPServer.reservedSlugs.contains(ExternalMCPServer.makeSlug(from: newMCPServerName)) { return true } + switch newMCPServerTransportKind { + case .stdio: return newMCPServerCommand.isEmpty + case .http: return newMCPServerURL.isEmpty + } + } + + private func mcpDictionary(from pairs: [MCPKeyValuePair]) -> [String: String] { + Dictionary(uniqueKeysWithValues: pairs.filter { !$0.key.isEmpty }.map { ($0.key, $0.value) }) + } + + @ViewBuilder + private func mcpKeyValueEditor(title: LocalizedStringKey, pairs: Binding<[MCPKeyValuePair]>) -> some View { + VStack(alignment: .leading, spacing: 6) { + HStack { + Text(title) + .font(.system(size: 12, weight: .semibold)) + .foregroundStyle(.secondary) + Spacer() + Button { + pairs.wrappedValue.append(MCPKeyValuePair()) + } label: { + Image(systemName: "plus.circle.fill") + } + .buttonStyle(.plain) + } + ForEach(pairs) { $pair in + HStack(spacing: 6) { + TextField("Key", text: $pair.key) + .textFieldStyle(.roundedBorder) + .font(.system(size: 12, design: .monospaced)) + SecureField("Value", text: $pair.value) + .textFieldStyle(.roundedBorder) + .font(.system(size: 12, design: .monospaced)) + Button { + pairs.wrappedValue.removeAll { $0.id == pair.id } + } label: { + Image(systemName: "minus.circle.fill") + .foregroundStyle(.red) + } + .buttonStyle(.plain) + } + } + if pairs.wrappedValue.isEmpty { + Text("None configured") + .font(.system(size: 12)) + .foregroundStyle(.tertiary) + } + } + .padding(.horizontal, 16) + .padding(.vertical, 10) + } + private func mcpStatusColor(_ state: MCPClientState?) -> Color { switch state { case .ready: return .green diff --git a/oAITests/ExternalMCPManagerConversionTests.swift b/oAITests/ExternalMCPManagerConversionTests.swift new file mode 100644 index 0000000..9955f67 --- /dev/null +++ b/oAITests/ExternalMCPManagerConversionTests.swift @@ -0,0 +1,65 @@ +// +// ExternalMCPManagerConversionTests.swift +// ConfabTests +// +// SPDX-License-Identifier: PolyForm-Noncommercial-1.0.0 +// Copyright (C) 2026 Rune Olsen + +import Testing +import Foundation +@testable import Confab + +@Suite("ExternalMCPManager.convertInputSchema") +struct ExternalMCPManagerConversionTests { + + /// Regression test for a real crash: Obsidian's Local REST API plugin's MCP tool schemas + /// include at least one property with no "type" key at all — valid JSON Schema (e.g. an + /// enum-only or composed property) — which a `prop.type!` force-unwrap used to crash on the + /// moment a live HTTP MCP server's tools/list response reached this code. + @Test("A property with no type at all defaults to string instead of crashing") + func propertyWithNoTypeDefaultsToString() { + let prop = MCPPropertySchema(type: nil, description: "no explicit type", enum: nil, items: nil) + let schema = MCPInputSchema(type: "object", properties: ["mode": prop], required: nil) + + let parameters = ExternalMCPManager.convertInputSchema(schema) + + #expect(parameters.properties["mode"]?.type == "string") + #expect(parameters.properties["mode"]?.description == "no explicit type") + } + + @Test("integer is normalized to number") + func integerNormalizesToNumber() { + let prop = MCPPropertySchema(type: "integer", description: nil, enum: nil, items: nil) + let schema = MCPInputSchema(type: "object", properties: ["count": prop], required: nil) + + let parameters = ExternalMCPManager.convertInputSchema(schema) + + #expect(parameters.properties["count"]?.type == "number") + } + + @Test("An unrecognized type string falls back to string") + func unrecognizedTypeFallsBackToString() { + let prop = MCPPropertySchema(type: "something-unusual", description: nil, enum: nil, items: nil) + let schema = MCPInputSchema(type: "object", properties: ["weird": prop], required: nil) + + let parameters = ExternalMCPManager.convertInputSchema(schema) + + #expect(parameters.properties["weird"]?.type == "string") + } + + @Test("Recognized types (string/number/boolean/array/object) pass through unchanged") + func recognizedTypesPassThrough() { + let types = ["string", "number", "boolean", "array", "object"] + var properties: [String: MCPPropertySchema] = [:] + for t in types { + properties[t] = MCPPropertySchema(type: t, description: nil, enum: nil, items: nil) + } + let schema = MCPInputSchema(type: "object", properties: properties, required: nil) + + let parameters = ExternalMCPManager.convertInputSchema(schema) + + for t in types { + #expect(parameters.properties[t]?.type == t) + } + } +} diff --git a/oAITests/ExternalMCPModelsTests.swift b/oAITests/ExternalMCPModelsTests.swift new file mode 100644 index 0000000..89fb52e --- /dev/null +++ b/oAITests/ExternalMCPModelsTests.swift @@ -0,0 +1,105 @@ +// +// ExternalMCPModelsTests.swift +// ConfabTests +// +// SPDX-License-Identifier: PolyForm-Noncommercial-1.0.0 +// Copyright (C) 2026 Rune Olsen + +import Testing +import Foundation +@testable import Confab + +@Suite("ExternalMCPServer Codable") +struct ExternalMCPServerCodableTests { + + private static func makeCoders() -> (JSONEncoder, JSONDecoder) { + let encoder = JSONEncoder() + encoder.dateEncodingStrategy = .iso8601 + let decoder = JSONDecoder() + decoder.dateDecodingStrategy = .iso8601 + return (encoder, decoder) + } + + @Test("Decodes old-shape JSON (saved before env/HTTP fields existed) with safe defaults") + func decodesOldShapeJSON() throws { + // Exactly the 7 keys ExternalMCPServer had before transportKind/env/url/bearerToken/headers + // were added — this is what's actually sitting in existing users' settings DB right now. + let oldJSON = """ + { + "id": "00000000-0000-0000-0000-000000000001", + "name": "Safari", + "command": "safaridriver", + "args": ["--mcp"], + "isEnabled": true, + "timeout": 30, + "createdAt": "2026-01-01T00:00:00Z" + } + """ + let (_, decoder) = Self.makeCoders() + let server = try decoder.decode(ExternalMCPServer.self, from: Data(oldJSON.utf8)) + + #expect(server.name == "Safari") + #expect(server.command == "safaridriver") + #expect(server.args == ["--mcp"]) + #expect(server.transportKind == .stdio) + #expect(server.env.isEmpty) + #expect(server.url.isEmpty) + #expect(server.bearerToken.isEmpty) + #expect(server.headers.isEmpty) + } + + @Test("An array of old-shape servers (the real settings-JSON shape) decodes without dropping any") + func decodesOldShapeArray() throws { + let oldJSON = """ + [ + {"id": "00000000-0000-0000-0000-000000000001", "name": "Safari", "command": "safaridriver", + "args": ["--mcp"], "isEnabled": true, "timeout": 30, "createdAt": "2026-01-01T00:00:00Z"}, + {"id": "00000000-0000-0000-0000-000000000002", "name": "Other", "command": "some-tool", + "args": [], "isEnabled": false, "timeout": 15, "createdAt": "2026-02-01T00:00:00Z"} + ] + """ + let (_, decoder) = Self.makeCoders() + let servers = try decoder.decode([ExternalMCPServer].self, from: Data(oldJSON.utf8)) + #expect(servers.count == 2) + } + + @Test("Round-trips a full HTTP-transport server through encode/decode") + func roundTripsHTTPServer() throws { + let original = ExternalMCPServer( + name: "Obsidian", + transportKind: .http, + url: "http://127.0.0.1:27123/mcp/", + bearerToken: "secret-token", + headers: ["X-Extra": "value"], + timeout: 45 + ) + let (encoder, decoder) = Self.makeCoders() + let data = try encoder.encode(original) + let decoded = try decoder.decode(ExternalMCPServer.self, from: data) + + #expect(decoded.id == original.id) + #expect(decoded.transportKind == .http) + #expect(decoded.url == "http://127.0.0.1:27123/mcp/") + #expect(decoded.bearerToken == "secret-token") + #expect(decoded.headers == ["X-Extra": "value"]) + } + + @Test("Round-trips a stdio-transport server with env vars") + func roundTripsStdioServerWithEnv() throws { + let original = ExternalMCPServer( + name: "Custom", + command: "npx", + args: ["-y", "some-tool"], + env: ["API_KEY": "abc123"], + timeout: 30 + ) + let (encoder, decoder) = Self.makeCoders() + let data = try encoder.encode(original) + let decoded = try decoder.decode(ExternalMCPServer.self, from: data) + + #expect(decoded.transportKind == .stdio) + #expect(decoded.env == ["API_KEY": "abc123"]) + #expect(decoded.command == "npx") + #expect(decoded.args == ["-y", "some-tool"]) + } +} diff --git a/oAITests/MCPTransportTests.swift b/oAITests/MCPTransportTests.swift new file mode 100644 index 0000000..2a50f93 --- /dev/null +++ b/oAITests/MCPTransportTests.swift @@ -0,0 +1,131 @@ +// +// MCPTransportTests.swift +// ConfabTests +// +// SPDX-License-Identifier: PolyForm-Noncommercial-1.0.0 +// Copyright (C) 2026 Rune Olsen + +import Testing +import Foundation +@testable import Confab + +@Suite("MCPTransportSupport.extractResult") +struct MCPTransportSupportTests { + + @Test("Extracts the result payload from a successful JSON-RPC response") + func extractsResult() throws { + let json: [String: Any] = ["jsonrpc": "2.0", "id": 1, "result": ["tools": []]] + let data = try MCPTransportSupport.extractResult(from: json) + let decoded = try JSONSerialization.jsonObject(with: data) as? [String: Any] + #expect(decoded?["tools"] != nil) + } + + @Test("Throws with the server's message when the response is a JSON-RPC error") + func throwsOnJSONRPCError() { + let json: [String: Any] = ["jsonrpc": "2.0", "id": 1, "error": ["code": -32601, "message": "Method not found"]] + do { + _ = try MCPTransportSupport.extractResult(from: json) + Issue.record("Expected extractResult to throw") + } catch let error as MCPClientError { + switch error { + case .invalidResponse(let message): #expect(message == "Method not found") + default: Issue.record("Expected .invalidResponse, got \(error)") + } + } catch { + Issue.record("Expected MCPClientError, got \(error)") + } + } + + @Test("Throws when the response has neither result nor error") + func throwsOnMissingResult() { + let json: [String: Any] = ["jsonrpc": "2.0", "id": 1] + do { + _ = try MCPTransportSupport.extractResult(from: json) + Issue.record("Expected extractResult to throw") + } catch let error as MCPClientError { + switch error { + case .invalidResponse(let message): #expect(message == "Missing result field") + default: Issue.record("Expected .invalidResponse, got \(error)") + } + } catch { + Issue.record("Expected MCPClientError, got \(error)") + } + } +} + +@Suite("HTTPMCPTransport.parseResponseBody") +struct HTTPMCPTransportParseTests { + + @Test("Parses a plain application/json response body directly") + func parsesPlainJSON() throws { + let body = Data(#"{"jsonrpc":"2.0","id":7,"result":{"ok":true}}"#.utf8) + let json = try HTTPMCPTransport.parseResponseBody(body, contentType: "application/json", expectedId: 7) + #expect(json["id"] as? Int == 7) + } + + @Test("Parses a text/event-stream body, finding the data: line matching the expected id") + func parsesSSEMatchingId() throws { + let sse = """ + event: message + data: {"jsonrpc":"2.0","id":7,"result":{"ok":true}} + + """ + let json = try HTTPMCPTransport.parseResponseBody( + Data(sse.utf8), contentType: "text/event-stream", expectedId: 7 + ) + #expect(json["id"] as? Int == 7) + } + + @Test("Skips unrelated server-sent messages before the matching response, per the Streamable HTTP spec") + func skipsUnrelatedMessagesInSSE() throws { + // Spec: "The server MAY send JSON-RPC requests and notifications before sending the + // JSON-RPC response." Simulated here as an unrelated id=99 message before the real id=7 one. + let sse = """ + data: {"jsonrpc":"2.0","id":99,"method":"unrelated/notification"} + + data: {"jsonrpc":"2.0","id":7,"result":{"ok":true}} + + """ + let json = try HTTPMCPTransport.parseResponseBody( + Data(sse.utf8), contentType: "text/event-stream", expectedId: 7 + ) + #expect(json["id"] as? Int == 7) + } + + @Test("Throws when no SSE data: line matches the expected id") + func throwsWhenNoMatchInSSE() { + let sse = """ + data: {"jsonrpc":"2.0","id":99,"result":{}} + + """ + do { + _ = try HTTPMCPTransport.parseResponseBody( + Data(sse.utf8), contentType: "text/event-stream", expectedId: 7 + ) + Issue.record("Expected parseResponseBody to throw") + } catch let error as MCPClientError { + switch error { + case .invalidResponse: break + default: Issue.record("Expected .invalidResponse, got \(error)") + } + } catch { + Issue.record("Expected MCPClientError, got \(error)") + } + } + + @Test("Throws on malformed JSON in a plain application/json body") + func throwsOnMalformedJSON() { + let body = Data("not json".utf8) + do { + _ = try HTTPMCPTransport.parseResponseBody(body, contentType: "application/json", expectedId: 1) + Issue.record("Expected parseResponseBody to throw") + } catch let error as MCPClientError { + switch error { + case .invalidResponse: break + default: Issue.record("Expected .invalidResponse, got \(error)") + } + } catch { + Issue.record("Expected MCPClientError, got \(error)") + } + } +}