Root-caused two real issues Rune hit with Obsidian/Homepage external MCP servers: 1. Toggling a server's enable switch silently wiped transportKind/env/ url/bearerToken/headers back to stdio defaults (only id/name/command/ args/isEnabled/timeout/createdAt were preserved) — almost certainly how Obsidian's config got corrupted into an empty-command stdio entry despite never being edited directly. Fixed via ExternalMCPServer.withEnabledToggled(), which flips only isEnabled. 2. npx (installed via Homebrew) was invisible to Confab because GUI apps only inherit launchd's minimal PATH, not the Terminal PATH. Tried spawning the user's login shell to ask for its real PATH — this caused two real hangs in one session (first an -ilc pipe deadlock, then a waitUntilExit()/CFRunLoop reentrancy issue even after fixing that) and was abandoned entirely in favor of LoginShellEnvironment: deterministic, subprocess-free directory probing (Homebrew, MacPorts, Volta, nvm's alias file) that can't hang by construction. Also added: - Edit capability for existing External MCP servers (previously only Add/Toggle/Delete) — the second thing Rune explicitly asked for, and the way to fix a corrupted entry like Obsidian's without deleting it. - MCPClientError.commandNotFound: a stdio server's command is checked against PATH up front in StdioMCPTransport.prepare() and fails immediately with a clear reason instead of cycling through 3 rounds of crash/restart backoff (5s/15s/30s) for a permanently-missing binary. - A "Get Node.js" button appears when this happens, opening a sheet with a copyable `brew install node`, a one-click install (via NodeInstallHelper, using the terminationHandler/readabilityHandler pattern already proven safe elsewhere in this file — deliberately not waitUntilExit()), or a nodejs.org link if Homebrew isn't present. - ExternalMCPManager.retryClient(id:) to manually retry after fixing the underlying cause. - Help book: new "Servers That Use npx" section, updated Server Status section, updated Settings blurb. 37 new/changed tests covering the toggle fix, PATH probing, the commandNotFound fast-fail path, and missing-command detection — full suite (374 tests) passes clean.
171 lines
7.5 KiB
Swift
171 lines
7.5 KiB
Swift
// SPDX-License-Identifier: PolyForm-Noncommercial-1.0.0
|
|
// Copyright (C) 2026 Rune Olsen
|
|
|
|
import Foundation
|
|
|
|
// MARK: - ExternalMCPClient
|
|
|
|
/// 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 let transport: any MCPTransport
|
|
private var nextRequestId: Int = 1
|
|
|
|
private(set) var state: MCPClientState = .idle
|
|
private(set) var discoveredTools: [MCPToolDefinition] = []
|
|
|
|
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
|
|
|
|
func start() async throws {
|
|
guard state == .idle || state == .stopped || state == .crashed else { return }
|
|
state = .connecting
|
|
stateDelegate?.clientDidChangeState(id: server.id, state: .connecting)
|
|
|
|
do {
|
|
try await transport.prepare()
|
|
|
|
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 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 {
|
|
transport.stop()
|
|
if case MCPClientError.commandNotFound = error {
|
|
// Deliberate exception to the uniform-.crashed rule below: a missing command is a
|
|
// permanent, static condition — no restart-with-backoff will ever fix a binary that
|
|
// isn't there, so skip straight to a clear .error instead of 3 rounds of pointless
|
|
// 5s/15s/30s backoff (which is what used to happen, and is why this exists).
|
|
let newState: MCPClientState = .error(error.localizedDescription)
|
|
state = newState
|
|
stateDelegate?.clientDidChangeState(id: server.id, state: newState)
|
|
throw error
|
|
}
|
|
// Uniformly route every other 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
|
|
stateDelegate?.clientDidChangeState(id: server.id, state: .crashed)
|
|
throw error
|
|
}
|
|
|
|
state = .ready
|
|
stateDelegate?.clientDidBecomeReady(id: server.id, tools: discoveredTools, server: server)
|
|
}
|
|
|
|
func stop() {
|
|
state = .stopped
|
|
transport.stop()
|
|
}
|
|
|
|
// MARK: - Tool Execution
|
|
|
|
func callTool(originalName: String, argumentsJSON: String) async -> [String: Any] {
|
|
guard state == .ready else {
|
|
return ["error": "MCP server '\(server.name)' is not connected"]
|
|
}
|
|
guard let argData = argumentsJSON.data(using: .utf8),
|
|
let argsDict = try? JSONSerialization.jsonObject(with: argData) as? [String: Any] else {
|
|
return ["error": "Invalid arguments JSON for tool \(originalName)"]
|
|
}
|
|
do {
|
|
let result: MCPToolCallResult = try await timedRequest(
|
|
seconds: server.timeout,
|
|
method: "tools/call",
|
|
params: ["name": originalName, "arguments": argsDict]
|
|
)
|
|
return convertMCPResult(result)
|
|
} catch MCPClientError.timeout {
|
|
return ["error": "MCP server '\(server.name)' timed out after \(Int(server.timeout))s"]
|
|
} catch {
|
|
return ["error": "MCP call '\(originalName)' failed: \(error.localizedDescription)"]
|
|
}
|
|
}
|
|
|
|
// MARK: - JSON-RPC
|
|
|
|
private func timedRequest<T: Decodable>(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 }
|
|
let resultData = try await transport.sendRequest(message, id: id, timeoutSeconds: seconds)
|
|
return try JSONDecoder().decode(T.self, from: resultData)
|
|
}
|
|
|
|
// MARK: - Result conversion
|
|
|
|
private func convertMCPResult(_ result: MCPToolCallResult) -> [String: Any] {
|
|
let isError = result.isError ?? false
|
|
var parts: [String] = []
|
|
for content in result.content {
|
|
switch content.type {
|
|
case "text":
|
|
if let text = content.text { parts.append(text) }
|
|
case "image":
|
|
if let base64 = content.data, let imageData = Data(base64Encoded: base64) {
|
|
parts.append("[Image saved to: \(writeTempImage(imageData, mimeType: content.mimeType))]")
|
|
}
|
|
case "resource":
|
|
if let text = content.text { parts.append(text) }
|
|
else if let uri = content.uri { parts.append("[Resource: \(uri)]") }
|
|
default:
|
|
if let text = content.text { parts.append(text) }
|
|
}
|
|
}
|
|
let combined = parts.joined(separator: "\n")
|
|
return isError ? ["error": combined.isEmpty ? "Tool returned an error" : combined] : ["output": combined]
|
|
}
|
|
|
|
private func writeTempImage(_ data: Data, mimeType: String?) -> String {
|
|
let ext = mimeType?.contains("png") == true ? "png" : "jpg"
|
|
let path = "/tmp/oai_mcp_\(Int(Date().timeIntervalSince1970 * 1000)).\(ext)"
|
|
try? data.write(to: URL(fileURLWithPath: path))
|
|
return path
|
|
}
|
|
}
|