@@ -1530,10 +1537,10 @@ Whenever the user asks you to translate something, translate it to Norwegian Bok
- π’ Connected β running and its tools are available to the AI
- π Connectingβ¦ β starting up or performing the initial handshake
- - π΄ Error / Crashed β failed to start or exited unexpectedly
+ - π΄ Error / Crashed β failed to start or exited unexpectedly; hover the status label for the specific reason
- βͺ Not started β disabled via the toggle
- Toggle a server off/on or delete it entirely with the trash icon. Crashed servers automatically restart up to 3 times with increasing delay (5s, 15s, 30s) before giving up.
+ Toggle a server off/on, edit it with the pencil icon, or delete it entirely with the trash icon. Crashed servers automatically restart up to 3 times with increasing delay (5s, 15s, 30s) before giving up with "Maximum restart attempts reached". A missing command (e.g. npx not found β see above) is treated differently: since no amount of retrying fixes a binary that isn't there, it goes straight to an error with a Get Node.js button instead of cycling through restart attempts first.
Note: Tool names from every external server are prefixed with that server's slug (derived from its Name) so they never collide with Confab's built-in tools or each other.
diff --git a/oAI/Services/ExternalMCPClient.swift b/oAI/Services/ExternalMCPClient.swift
index 1091b77..394e16b 100644
--- a/oAI/Services/ExternalMCPClient.swift
+++ b/oAI/Services/ExternalMCPClient.swift
@@ -71,14 +71,24 @@ final class ExternalMCPClient {
let toolsResult: MCPToolsListResult = try await timedRequest(seconds: 15, method: "tools/list", params: nil)
discoveredTools = toolsResult.tools
} catch {
- // Uniformly route every start() failure (bad config, launch failure, handshake
+ 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
- transport.stop()
stateDelegate?.clientDidChangeState(id: server.id, state: .crashed)
throw error
}
diff --git a/oAI/Services/ExternalMCPManager.swift b/oAI/Services/ExternalMCPManager.swift
index be858a1..7b76fe7 100644
--- a/oAI/Services/ExternalMCPManager.swift
+++ b/oAI/Services/ExternalMCPManager.swift
@@ -76,6 +76,18 @@ final class ExternalMCPManager {
}
}
+ /// Manually retries one server's connection, resetting its restart-attempt counter β for use
+ /// after the user fixes an external condition (e.g. installs a missing command) rather than
+ /// waiting for `.error("Maximum restart attempts reached")`/`.error("Command not found: β¦")`
+ /// to somehow resolve on their own; neither state is retried automatically.
+ func retryClient(id: UUID) {
+ guard let server = serverConfigs[id] else { return }
+ restartAttempts.removeValue(forKey: id)
+ restartTasks[id]?.cancel()
+ restartTasks.removeValue(forKey: id)
+ startClient(for: server)
+ }
+
private func scheduleRestart(for server: ExternalMCPServer, attempt: Int) {
let delays: [Double] = [5, 15, 30]
let delay = delays[min(attempt - 1, delays.count - 1)]
diff --git a/oAI/Services/ExternalMCPModels.swift b/oAI/Services/ExternalMCPModels.swift
index b4fb906..99c6546 100644
--- a/oAI/Services/ExternalMCPModels.swift
+++ b/oAI/Services/ExternalMCPModels.swift
@@ -123,6 +123,16 @@ nonisolated struct ExternalMCPServer: Codable, Identifiable, Sendable {
]
var isSlugReserved: Bool { Self.reservedSlugs.contains(slug) }
+
+ /// Returns a copy with only `isEnabled` flipped. Exists so toggling a server in Settings can't
+ /// silently drop fields β see the fixed bug in `SettingsService.toggleExternalMCPServer`, which
+ /// used to rebuild the struct from a subset of fields and lose `transportKind`/`env`/`url`/
+ /// `bearerToken`/`headers` on every toggle.
+ func withEnabledToggled() -> ExternalMCPServer {
+ var copy = self
+ copy.isEnabled.toggle()
+ return copy
+ }
}
// MARK: - Client State
@@ -154,6 +164,12 @@ enum MCPClientError: LocalizedError {
case handshakeFailed(String)
case writeFailed
case invalidConfiguration(String)
+ /// The stdio server's `command` couldn't be located anywhere on PATH (inherited + the common
+ /// install directories `LoginShellEnvironment` checks). Deliberately distinct from
+ /// `.processLaunchFailed`: this is a permanent, static condition β no amount of
+ /// restart-with-backoff will ever fix a binary that isn't there β so callers route it straight
+ /// to a clear `.error` state instead of the crash/retry loop. See `ExternalMCPClient.start()`.
+ case commandNotFound(String)
var errorDescription: String? {
switch self {
@@ -164,6 +180,7 @@ enum MCPClientError: LocalizedError {
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)"
+ case .commandNotFound(let s): return "Command not found: \(s)"
}
}
}
diff --git a/oAI/Services/MCPTransport.swift b/oAI/Services/MCPTransport.swift
index a051a71..26b69f0 100644
--- a/oAI/Services/MCPTransport.swift
+++ b/oAI/Services/MCPTransport.swift
@@ -39,6 +39,84 @@ enum MCPTransportSupport {
}
}
+// MARK: - Login Shell PATH Resolution
+
+/// GUI apps launched from Finder/Dock/LaunchServices inherit launchd's minimal PATH
+/// (`/usr/bin:/bin:/usr/sbin:/sbin`), not the interactive-shell PATH a Terminal session gets β so
+/// `npx`/`node` installed via nvm, Homebrew, Volta etc. are invisible to a bare `/usr/bin/env `
+/// spawn, even though the same command works fine when the user runs it themselves in Terminal.
+///
+/// History: the obvious fix β spawn the user's real login shell and ask it for `$PATH` β caused two
+/// real hangs in one session and was abandoned. First with `-ilc` (interactive login), which
+/// deadlocked on a full pipe buffer from shell startup noise (fixed by draining concurrently).
+/// After that fix, it hung *again* with non-interactive `-lc`: a live `sample` of the stuck process
+/// showed `NSTask.waitUntilExit()` still parked inside a nested CFRunLoop even though the child
+/// shell process was already gone from the process list β a CFRunLoop/SIGCHLD-notification
+/// reentrancy issue specific to calling it from a Swift-concurrency worker thread inside this app's
+/// XCTest host process. Spawning any subprocess just to read one environment variable is exposed to
+/// this whole class of platform quirk, and a subprocess-based timeout can't protect against a hang
+/// inside the *notification mechanism itself*. Instead: probe the fixed set of directories every
+/// common Node install method actually uses, with plain filesystem checks β no subprocess, no run
+/// loop, nothing that can hang, at the cost of being a known list rather than a fully general answer.
+enum LoginShellEnvironment {
+ /// Directories to add if present, in priority order. Covers Homebrew (Apple Silicon and Intel),
+ /// MacPorts, Volta, and nvm's "default" alias (via `nvmDefaultNodeBinDirectory`).
+ nonisolated static func candidateDirectories(home: String = NSHomeDirectory()) -> [String] {
+ var dirs = [
+ "/opt/homebrew/bin", "/opt/homebrew/sbin",
+ "/usr/local/bin", "/usr/local/sbin",
+ "/opt/local/bin", "/opt/local/sbin",
+ "\(home)/.volta/bin",
+ ]
+ if let nvmDir = nvmDefaultNodeBinDirectory(home: home) {
+ dirs.append(nvmDir)
+ }
+ return dirs
+ }
+
+ /// nvm has no fixed "current" symlink usable in PATH, but it does write its default version as
+ /// plain text to `~/.nvm/alias/default` β reading that file (no subprocess, no shell) lets us
+ /// construct the versioned bin directory nvm's own shell integration would otherwise add.
+ nonisolated static func nvmDefaultNodeBinDirectory(home: String, fileManager: FileManager = .default) -> String? {
+ let aliasPath = "\(home)/.nvm/alias/default"
+ guard let raw = try? String(contentsOfFile: aliasPath, encoding: .utf8) else { return nil }
+ let version = raw.trimmingCharacters(in: .whitespacesAndNewlines)
+ guard !version.isEmpty else { return nil }
+ let versionDir = version.hasPrefix("v") ? version : "v\(version)"
+ return "\(home)/.nvm/versions/node/\(versionDir)/bin"
+ }
+
+ /// Prepends every candidate directory that exists and isn't already on `basePath`. Pure,
+ /// synchronous, and instant β safe to call from anywhere, including MainActor, with no risk of
+ /// blocking.
+ nonisolated static func augmentedPath(
+ basePath: String, home: String = NSHomeDirectory(), fileManager: FileManager = .default
+ ) -> String {
+ let existing = Set(basePath.split(separator: ":").map(String.init))
+ let toAdd = candidateDirectories(home: home).filter { !existing.contains($0) && fileManager.fileExists(atPath: $0) }
+ guard !toAdd.isEmpty else { return basePath }
+ return (toAdd + [basePath]).joined(separator: ":")
+ }
+
+ /// Searches `basePath` (already augmented by the caller, typically) for an executable file
+ /// named `name`. Returns its full path if found. Pure filesystem lookup β no subprocess.
+ nonisolated static func findExecutable(named name: String, in searchPath: String, fileManager: FileManager = .default) -> String? {
+ for dir in searchPath.split(separator: ":") {
+ let candidate = "\(dir)/\(name)"
+ if fileManager.isExecutableFile(atPath: candidate) { return candidate }
+ }
+ return nil
+ }
+
+ /// Homebrew's install prefix, if Homebrew itself is present β used to offer a one-command
+ /// "install Node.js" suggestion when a stdio server's command (typically `npx`) can't be found.
+ nonisolated static func homebrewPrefix(fileManager: FileManager = .default) -> String? {
+ if fileManager.fileExists(atPath: "/opt/homebrew/bin/brew") { return "/opt/homebrew" }
+ if fileManager.fileExists(atPath: "/usr/local/bin/brew") { return "/usr/local" }
+ return nil
+ }
+}
+
// MARK: - Stdio Transport
/// Launches the server as a subprocess and speaks newline-delimited JSON-RPC over its
@@ -63,18 +141,33 @@ final class StdioMCPTransport: MCPTransport {
}
func prepare() async throws {
+ // Inherited environment first, with PATH augmented to include common Homebrew/MacPorts/
+ // Volta/nvm install locations (see LoginShellEnvironment β launchd's default PATH doesn't
+ // include them, so a bare `npx` spawn fails even though it works fine in Terminal), then
+ // layer the user-configured vars over it so they can override.
+ var environment = ProcessInfo.processInfo.environment
+ let basePath = environment["PATH"] ?? "/usr/bin:/bin:/usr/sbin:/sbin"
+ let searchPath = LoginShellEnvironment.augmentedPath(basePath: basePath)
+ environment["PATH"] = searchPath
+ for (key, value) in server.env { environment[key] = value }
+
let proc = Process()
if server.command.hasPrefix("/") {
+ guard FileManager.default.isExecutableFile(atPath: server.command) else {
+ throw MCPClientError.commandNotFound(server.command)
+ }
proc.executableURL = URL(fileURLWithPath: server.command)
proc.arguments = server.args
} else {
+ // Check up front rather than letting `/usr/bin/env ` fail asynchronously after
+ // launch: a missing command is a permanent condition β surfacing it immediately as a
+ // clear error, instead of after 3 rounds of crash/restart backoff, is the whole point.
+ guard LoginShellEnvironment.findExecutable(named: server.command, in: searchPath) != nil else {
+ throw MCPClientError.commandNotFound(server.command)
+ }
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()
diff --git a/oAI/Services/NodeInstallHelper.swift b/oAI/Services/NodeInstallHelper.swift
new file mode 100644
index 0000000..b040311
--- /dev/null
+++ b/oAI/Services/NodeInstallHelper.swift
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: PolyForm-Noncommercial-1.0.0
+// Copyright (C) 2026 Rune Olsen
+
+import Foundation
+
+/// Runs `brew install node` on the user's explicit request (from `NodeInstallHelpSheet`), when a
+/// stdio External MCP server's command couldn't be found and Homebrew is present.
+///
+/// Deliberately does NOT use `Process.waitUntilExit()` β see `LoginShellEnvironment`'s doc comment
+/// in `MCPTransport.swift` for the full story: that call hung for real, twice, in this app's test
+/// process, even with careful draining and multiple timeout layers. `terminationHandler` +
+/// `readabilityHandler` (both callback-driven, no blocking wait) is the pattern this codebase
+/// already uses safely elsewhere (`StdioMCPTransport.prepare()`), so this reuses it instead of
+/// introducing a second instance of the primitive that just caused two real hangs.
+enum NodeInstallHelper {
+ struct InstallResult: Sendable {
+ let success: Bool
+ let output: String
+ }
+
+ static func installNode(homebrewPrefix: String, timeout: TimeInterval = 180) async -> InstallResult {
+ await withCheckedContinuation { continuation in
+ let proc = Process()
+ proc.executableURL = URL(fileURLWithPath: "\(homebrewPrefix)/bin/brew")
+ proc.arguments = ["install", "node"]
+ let outPipe = Pipe()
+ let errPipe = Pipe()
+ proc.standardOutput = outPipe
+ proc.standardError = errPipe
+
+ let ioQueue = DispatchQueue(label: "confab.nodeinstall.io")
+ var collected = Data()
+ let resumeLock = NSLock()
+ var resumed = false
+
+ func finish(_ result: InstallResult) {
+ resumeLock.lock()
+ defer { resumeLock.unlock() }
+ guard !resumed else { return }
+ resumed = true
+ outPipe.fileHandleForReading.readabilityHandler = nil
+ errPipe.fileHandleForReading.readabilityHandler = nil
+ continuation.resume(returning: result)
+ }
+
+ outPipe.fileHandleForReading.readabilityHandler = { handle in
+ let chunk = handle.availableData
+ ioQueue.sync { collected.append(chunk) }
+ }
+ errPipe.fileHandleForReading.readabilityHandler = { handle in
+ let chunk = handle.availableData
+ ioQueue.sync { collected.append(chunk) }
+ }
+
+ proc.terminationHandler = { p in
+ ioQueue.sync {
+ let text = String(data: collected, encoding: .utf8) ?? ""
+ finish(InstallResult(success: p.terminationStatus == 0, output: text))
+ }
+ }
+
+ do {
+ try proc.run()
+ } catch {
+ finish(InstallResult(success: false, output: "Failed to launch brew: \(error.localizedDescription)"))
+ return
+ }
+
+ DispatchQueue.global().asyncAfter(deadline: .now() + timeout) {
+ guard proc.isRunning else { return }
+ proc.terminate()
+ ioQueue.sync {
+ let text = String(data: collected, encoding: .utf8) ?? ""
+ finish(InstallResult(success: false, output: text + "\n[Timed out after \(Int(timeout))s]"))
+ }
+ }
+ }
+ }
+}
diff --git a/oAI/Services/SettingsService.swift b/oAI/Services/SettingsService.swift
index 5c2e4c3..5fced3b 100644
--- a/oAI/Services/SettingsService.swift
+++ b/oAI/Services/SettingsService.swift
@@ -500,11 +500,7 @@ class SettingsService {
}
func toggleExternalMCPServer(id: UUID) {
- externalMCPServers = externalMCPServers.map { s in
- s.id == id ? ExternalMCPServer(id: s.id, name: s.name, command: s.command,
- args: s.args, isEnabled: !s.isEnabled,
- timeout: s.timeout, createdAt: s.createdAt) : s
- }
+ externalMCPServers = externalMCPServers.map { $0.id == id ? $0.withEnabledToggled() : $0 }
}
// MARK: - Favorite Models
diff --git a/oAI/Views/Screens/NodeInstallHelpSheet.swift b/oAI/Views/Screens/NodeInstallHelpSheet.swift
new file mode 100644
index 0000000..0337694
--- /dev/null
+++ b/oAI/Views/Screens/NodeInstallHelpSheet.swift
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: PolyForm-Noncommercial-1.0.0
+// Copyright (C) 2026 Rune Olsen
+
+import SwiftUI
+
+/// Shown when an External MCP server's stdio `command` (typically `npx`) couldn't be found on
+/// PATH β the common cause is Node.js not being installed, or being installed somewhere Confab's
+/// PATH probing doesn't check. Offers a copyable install command, a one-click install via Homebrew
+/// (if present), and a link to the official Node.js installer as a fallback.
+struct NodeInstallHelpSheet: View {
+ let missingCommand: String
+ let onRetry: () -> Void
+ let onDone: () -> Void
+
+ @State private var isInstalling = false
+ @State private var installResult: NodeInstallHelper.InstallResult?
+ @State private var copiedFeedback = false
+
+ private var homebrewPrefix: String? { LoginShellEnvironment.homebrewPrefix() }
+
+ var body: some View {
+ VStack(alignment: .leading, spacing: 20) {
+ HStack(spacing: 12) {
+ Image(systemName: "shippingbox")
+ .font(.title2)
+ .foregroundStyle(.orange)
+ VStack(alignment: .leading, spacing: 2) {
+ Text("Node.js Required")
+ .font(.system(size: 17, weight: .semibold))
+ Text("'\(missingCommand)' couldn't be found on your Mac")
+ .font(.system(size: 13))
+ .foregroundStyle(.secondary)
+ }
+ Spacer()
+ }
+
+ Text("This MCP server is launched with '\(missingCommand)', which comes bundled with Node.js. Confab checked its usual install locations (Homebrew, MacPorts, Volta, nvm) and didn't find it there either.")
+ .font(.system(size: 13))
+ .fixedSize(horizontal: false, vertical: true)
+
+ if let homebrewPrefix {
+ VStack(alignment: .leading, spacing: 8) {
+ Text("Homebrew is installed β this is the easiest way to install Node.js:")
+ .font(.system(size: 13, weight: .medium))
+
+ HStack(spacing: 8) {
+ Text("brew install node")
+ .font(.system(size: 13, design: .monospaced))
+ .textSelection(.enabled)
+ .padding(.horizontal, 10)
+ .padding(.vertical, 6)
+ .background(Color.secondary.opacity(0.1))
+ .clipShape(RoundedRectangle(cornerRadius: 6))
+ Button(copiedFeedback ? "Copied" : "Copy") {
+ NSPasteboard.general.clearContents()
+ NSPasteboard.general.setString("brew install node", forType: .string)
+ copiedFeedback = true
+ }
+ .buttonStyle(.bordered)
+ }
+
+ if isInstalling {
+ HStack(spacing: 8) {
+ ProgressView().controlSize(.small)
+ Text("Installing β this can take a minuteβ¦")
+ .font(.system(size: 12))
+ .foregroundStyle(.secondary)
+ }
+ } else {
+ Button("Install Now") {
+ Task {
+ isInstalling = true
+ installResult = await NodeInstallHelper.installNode(homebrewPrefix: homebrewPrefix)
+ isInstalling = false
+ }
+ }
+ .buttonStyle(.borderedProminent)
+ }
+
+ if let installResult {
+ VStack(alignment: .leading, spacing: 4) {
+ Text(installResult.success ? "β Installed successfully" : "β Install failed")
+ .font(.system(size: 13, weight: .medium))
+ .foregroundStyle(installResult.success ? .green : .red)
+ if !installResult.success && !installResult.output.isEmpty {
+ ScrollView {
+ Text(installResult.output)
+ .font(.system(size: 11, design: .monospaced))
+ .textSelection(.enabled)
+ .frame(maxWidth: .infinity, alignment: .leading)
+ }
+ .frame(maxHeight: 120)
+ .padding(8)
+ .background(Color.secondary.opacity(0.08))
+ .clipShape(RoundedRectangle(cornerRadius: 6))
+ }
+ }
+ }
+ }
+ .padding(12)
+ .background(Color.secondary.opacity(0.05))
+ .clipShape(RoundedRectangle(cornerRadius: 10))
+ } else {
+ VStack(alignment: .leading, spacing: 8) {
+ Text("Homebrew wasn't found either. Install Node.js directly from its official installer:")
+ .font(.system(size: 13))
+ .fixedSize(horizontal: false, vertical: true)
+ Button("Open nodejs.org") {
+ NSWorkspace.shared.open(URL(string: "https://nodejs.org")!)
+ }
+ .buttonStyle(.bordered)
+ }
+ .padding(12)
+ .background(Color.secondary.opacity(0.05))
+ .clipShape(RoundedRectangle(cornerRadius: 10))
+ }
+
+ Text("After installing, click Retry Connection below to try again.")
+ .font(.system(size: 12))
+ .foregroundStyle(.secondary)
+
+ HStack {
+ Button("Open nodejs.org") {
+ NSWorkspace.shared.open(URL(string: "https://nodejs.org")!)
+ }
+ Spacer()
+ Button("Done") { onDone() }
+ Button("Retry Connection") {
+ onRetry()
+ onDone()
+ }
+ .buttonStyle(.borderedProminent)
+ }
+ }
+ .padding(24)
+ .frame(minWidth: 480, minHeight: 340)
+ }
+}
diff --git a/oAI/Views/Screens/SettingsView.swift b/oAI/Views/Screens/SettingsView.swift
index 36fd282..7986248 100644
--- a/oAI/Views/Screens/SettingsView.swift
+++ b/oAI/Views/Screens/SettingsView.swift
@@ -32,6 +32,14 @@ private struct MCPKeyValuePair: Identifiable {
var value: String = ""
}
+/// Carries the data `NodeInstallHelpSheet` needs atomically β see the `.sheet(item:)` note where
+/// this is used.
+private struct NodeInstallHelpContext: Identifiable {
+ let id = UUID()
+ let serverId: UUID
+ let missingCommand: String
+}
+
/// The MCP tab's sidebar-navigated sub-pages. `visibleCases` hides Personal Data/Mail while
/// their respective kill switches (`PersonalDataTools`/`MailTools.isHiddenPendingAppleFix`) are
/// active, mirroring the same gating those sections' own content already enforces.
@@ -134,6 +142,13 @@ struct SettingsView: View {
@State private var newMCPServerBearerToken = ""
@State private var newMCPServerHeaderPairs: [MCPKeyValuePair] = []
@State private var newMCPServerTimeout: Double = 30
+ /// Non-nil while editing an existing server (holds its id/isEnabled/createdAt so saving can
+ /// preserve them); nil means the sheet is in "Add" mode.
+ @State private var editingExternalMCPServer: ExternalMCPServer? = nil
+ /// `.sheet(item:)`, not `.sheet(isPresented:)` + separate state β the atomic-mutation pattern
+ /// this codebase uses for every data-carrying sheet (see the SwiftUI sheet timing gotcha in
+ /// CLAUDE.md).
+ @State private var nodeInstallHelpContext: NodeInstallHelpContext? = nil
private var externalMCPManager = ExternalMCPManager.shared
// Personal Data state (Calendar/Reminders/Contacts/Location/Maps)
@@ -1245,7 +1260,7 @@ It's better to admit "I need more information" or "I cannot do that" than to fak
Text("External MCP Servers")
.font(.system(size: 18, weight: .semibold))
}
- Text("Connect any stdio MCP server (e.g. safaridriver --mcp) to give the AI access to its tools. Tool names are prefixed with the server slug.")
+ Text("Connect any stdio MCP server (e.g. safaridriver --mcp) to give the AI access to its tools. Tool names are prefixed with the server slug. Servers launched via npx require Node.js β if a server shows a 'Get Node.js' button, click it for install help.")
.font(.system(size: 14))
.foregroundStyle(.secondary)
.fixedSize(horizontal: false, vertical: true)
@@ -1284,15 +1299,35 @@ It's better to admit "I need more information" or "I cannot do that" than to fak
.lineLimit(1)
}
Spacer()
+ if let missing = Self.missingCommand(from: externalMCPManager.clientStates[server.id]) {
+ Button {
+ nodeInstallHelpContext = NodeInstallHelpContext(serverId: server.id, missingCommand: missing)
+ } label: {
+ Label("Get Node.js", systemImage: "shippingbox")
+ .font(.system(size: 11, weight: .medium))
+ }
+ .buttonStyle(.bordered)
+ .controlSize(.small)
+ }
Text(mcpStatusLabel(externalMCPManager.clientStates[server.id]))
.font(.system(size: 11))
.foregroundStyle(.secondary)
+ .help(errorTooltip(for: externalMCPManager.clientStates[server.id]) ?? "")
Toggle("", isOn: Binding(
get: { server.isEnabled },
set: { _ in settingsService.toggleExternalMCPServer(id: server.id) }
))
.toggleStyle(.switch)
.labelsHidden()
+ Button {
+ startEditingExternalMCPServer(server)
+ } label: {
+ Image(systemName: "pencil")
+ .foregroundStyle(.secondary)
+ .font(.system(size: 13))
+ }
+ .buttonStyle(.plain)
+ .help("Edit server")
Button {
settingsService.deleteExternalMCPServer(id: server.id)
} label: {
@@ -1312,6 +1347,7 @@ It's better to admit "I need more information" or "I cannot do that" than to fak
}
Button {
+ editingExternalMCPServer = nil
newMCPServerName = ""
newMCPServerTransportKind = .stdio
newMCPServerCommand = ""
@@ -1336,6 +1372,13 @@ It's better to admit "I need more information" or "I cannot do that" than to fak
.sheet(isPresented: $showAddExternalMCPServer) {
addExternalMCPServerSheet
}
+ .sheet(item: $nodeInstallHelpContext) { context in
+ NodeInstallHelpSheet(
+ missingCommand: context.missingCommand,
+ onRetry: { externalMCPManager.retryClient(id: context.serverId) },
+ onDone: { nodeInstallHelpContext = nil }
+ )
+ }
}
// MARK: - CLI Access Section
@@ -1504,7 +1547,7 @@ It's better to admit "I need more information" or "I cannot do that" than to fak
@ViewBuilder
private var addExternalMCPServerSheet: some View {
VStack(alignment: .leading, spacing: 20) {
- Text("Add External MCP Server")
+ Text(editingExternalMCPServer == nil ? "Add External MCP Server" : "Edit External MCP Server")
.font(.system(size: 16, weight: .semibold))
.frame(maxWidth: .infinity, alignment: .center)
@@ -1597,31 +1640,46 @@ It's better to admit "I need more information" or "I cannot do that" than to fak
}
HStack {
- Button("Cancel") { showAddExternalMCPServer = false }
+ Button("Cancel") {
+ editingExternalMCPServer = nil
+ showAddExternalMCPServer = false
+ }
Spacer()
- Button("Add") {
+ Button(editingExternalMCPServer == nil ? "Add" : "Save") {
+ let existing = editingExternalMCPServer
let server: ExternalMCPServer
switch newMCPServerTransportKind {
case .stdio:
server = ExternalMCPServer(
+ id: existing?.id ?? UUID(),
name: newMCPServerName,
transportKind: .stdio,
command: newMCPServerCommand,
args: ExternalMCPServer.parseArguments(newMCPServerArgs),
env: mcpDictionary(from: newMCPServerEnvPairs),
- timeout: newMCPServerTimeout
+ isEnabled: existing?.isEnabled ?? true,
+ timeout: newMCPServerTimeout,
+ createdAt: existing?.createdAt ?? Date()
)
case .http:
server = ExternalMCPServer(
+ id: existing?.id ?? UUID(),
name: newMCPServerName,
transportKind: .http,
url: newMCPServerURL,
bearerToken: newMCPServerBearerToken,
headers: mcpDictionary(from: newMCPServerHeaderPairs),
- timeout: newMCPServerTimeout
+ isEnabled: existing?.isEnabled ?? true,
+ timeout: newMCPServerTimeout,
+ createdAt: existing?.createdAt ?? Date()
)
}
- settingsService.addExternalMCPServer(server)
+ if existing != nil {
+ settingsService.updateExternalMCPServer(server)
+ } else {
+ settingsService.addExternalMCPServer(server)
+ }
+ editingExternalMCPServer = nil
showAddExternalMCPServer = false
}
.buttonStyle(.borderedProminent)
@@ -1645,6 +1703,28 @@ It's better to admit "I need more information" or "I cannot do that" than to fak
Dictionary(uniqueKeysWithValues: pairs.filter { !$0.key.isEmpty }.map { ($0.key, $0.value) })
}
+ private func startEditingExternalMCPServer(_ server: ExternalMCPServer) {
+ editingExternalMCPServer = server
+ newMCPServerName = server.name
+ newMCPServerTransportKind = server.transportKind
+ newMCPServerCommand = server.command
+ newMCPServerArgs = Self.argsToDisplayString(server.args)
+ newMCPServerEnvPairs = server.env.map { MCPKeyValuePair(key: $0.key, value: $0.value) }
+ newMCPServerURL = server.url
+ newMCPServerBearerToken = server.bearerToken
+ newMCPServerHeaderPairs = server.headers.map { MCPKeyValuePair(key: $0.key, value: $0.value) }
+ newMCPServerTimeout = server.timeout
+ showAddExternalMCPServer = true
+ }
+
+ /// Reverses `ExternalMCPServer.parseArguments` for display in the single-line Arguments field β
+ /// quotes any arg containing whitespace so editing and re-saving round-trips correctly.
+ nonisolated static func argsToDisplayString(_ args: [String]) -> String {
+ args.map { arg in
+ arg.contains(where: { $0.isWhitespace }) ? "\"\(arg)\"" : arg
+ }.joined(separator: " ")
+ }
+
@ViewBuilder
private func mcpKeyValueEditor(title: LocalizedStringKey, pairs: Binding<[MCPKeyValuePair]>) -> some View {
VStack(alignment: .leading, spacing: 6) {
@@ -1706,6 +1786,23 @@ It's better to admit "I need more information" or "I cannot do that" than to fak
}
}
+ /// Detects the specific "Command not found" error shape `ExternalMCPClient.start()` produces
+ /// for `MCPClientError.commandNotFound` and pulls out the missing command name, so the row can
+ /// offer targeted help instead of a generic error label. String-matched against
+ /// `MCPClientError.commandNotFound`'s `errorDescription` β both live in this app, so this stays
+ /// in sync by construction; not parsing anything external.
+ nonisolated static func missingCommand(from state: MCPClientState?) -> String? {
+ guard case .error(let message) = state, message.hasPrefix("Command not found: ") else { return nil }
+ return String(message.dropFirst("Command not found: ".count))
+ }
+
+ /// Full error text for a hover tooltip on the status label β `mcpStatusLabel` only shows the
+ /// generic word "Error", this surfaces the actual reason.
+ private func errorTooltip(for state: MCPClientState?) -> String? {
+ guard case .error(let message) = state else { return nil }
+ return message
+ }
+
// MARK: - Appearance Tab
@ViewBuilder
diff --git a/oAITests/ExternalMCPModelsTests.swift b/oAITests/ExternalMCPModelsTests.swift
index 89fb52e..8357e02 100644
--- a/oAITests/ExternalMCPModelsTests.swift
+++ b/oAITests/ExternalMCPModelsTests.swift
@@ -7,6 +7,7 @@
import Testing
import Foundation
+import SwiftUI
@testable import Confab
@Suite("ExternalMCPServer Codable")
@@ -103,3 +104,112 @@ struct ExternalMCPServerCodableTests {
#expect(decoded.args == ["-y", "some-tool"])
}
}
+
+@Suite("ExternalMCPServer.withEnabledToggled")
+struct ExternalMCPServerToggleTests {
+
+ @Test("Flips isEnabled and leaves every other field untouched, including HTTP-only fields")
+ func toggleFlipsOnlyIsEnabledForHTTPServer() {
+ let original = ExternalMCPServer(
+ name: "Obsidian",
+ transportKind: .http,
+ url: "http://127.0.0.1:27123/mcp/",
+ bearerToken: "secret-token",
+ headers: ["X-Custom": "value"],
+ isEnabled: true,
+ timeout: 45
+ )
+ let toggled = original.withEnabledToggled()
+
+ #expect(toggled.isEnabled == false)
+ #expect(toggled.id == original.id)
+ #expect(toggled.transportKind == .http)
+ #expect(toggled.url == original.url)
+ #expect(toggled.bearerToken == original.bearerToken)
+ #expect(toggled.headers == original.headers)
+ #expect(toggled.timeout == original.timeout)
+ #expect(toggled.createdAt == original.createdAt)
+ }
+
+ @Test("Flips isEnabled and leaves env vars untouched for a stdio server")
+ func toggleFlipsOnlyIsEnabledForStdioServer() {
+ let original = ExternalMCPServer(
+ name: "Homepage",
+ command: "npx",
+ args: ["-y", "mcp-remote"],
+ env: ["HOMEPAGE_API_KEY": "abc"],
+ isEnabled: false,
+ timeout: 30
+ )
+ let toggled = original.withEnabledToggled()
+
+ #expect(toggled.isEnabled == true)
+ #expect(toggled.command == "npx")
+ #expect(toggled.args == ["-y", "mcp-remote"])
+ #expect(toggled.env == ["HOMEPAGE_API_KEY": "abc"])
+ }
+
+ @Test("Toggling twice returns to the original isEnabled value")
+ func doubleToggleRoundTrips() {
+ let original = ExternalMCPServer(name: "Safari", command: "safaridriver", args: ["--mcp"])
+ let backAgain = original.withEnabledToggled().withEnabledToggled()
+ #expect(backAgain.isEnabled == original.isEnabled)
+ }
+}
+
+@Suite("SettingsView.argsToDisplayString round-trips through ExternalMCPServer.parseArguments")
+struct ArgsDisplayStringRoundTripTests {
+
+ @Test("Simple space-separated args round-trip unchanged")
+ func simpleArgsRoundTrip() {
+ let args = ["-y", "mcp-remote", "http://localhost:3000/api/mcp"]
+ let displayed = SettingsView.argsToDisplayString(args)
+ #expect(displayed == "-y mcp-remote http://localhost:3000/api/mcp")
+ #expect(ExternalMCPServer.parseArguments(displayed) == args)
+ }
+
+ @Test("An arg containing a space is quoted so re-parsing keeps it as one token")
+ func argWithSpaceIsQuoted() {
+ let args = ["--header", "X-Homepage-Api-Key: some value"]
+ let displayed = SettingsView.argsToDisplayString(args)
+ #expect(displayed == "--header \"X-Homepage-Api-Key: some value\"")
+ #expect(ExternalMCPServer.parseArguments(displayed) == args)
+ }
+
+ @Test("Empty args array displays and re-parses as empty")
+ func emptyArgsRoundTrip() {
+ #expect(SettingsView.argsToDisplayString([]).isEmpty)
+ #expect(ExternalMCPServer.parseArguments(SettingsView.argsToDisplayString([])).isEmpty)
+ }
+}
+
+@Suite("SettingsView.missingCommand")
+struct MissingCommandDetectionTests {
+
+ @Test("Extracts the command name from a commandNotFound-shaped error state")
+ func extractsCommandName() {
+ let state: MCPClientState = .error("Command not found: npx")
+ #expect(SettingsView.missingCommand(from: state) == "npx")
+ }
+
+ @Test("Returns nil for an unrelated error message")
+ func nilForUnrelatedError() {
+ let state: MCPClientState = .error("Maximum restart attempts reached")
+ #expect(SettingsView.missingCommand(from: state) == nil)
+ }
+
+ @Test("Returns nil for non-error states")
+ func nilForNonErrorStates() {
+ #expect(SettingsView.missingCommand(from: .ready) == nil)
+ #expect(SettingsView.missingCommand(from: .crashed) == nil)
+ #expect(SettingsView.missingCommand(from: .connecting) == nil)
+ #expect(SettingsView.missingCommand(from: nil) == nil)
+ }
+
+ @Test("Matches MCPClientError.commandNotFound's real errorDescription text exactly")
+ func matchesRealErrorDescription() {
+ let error = MCPClientError.commandNotFound("npx")
+ let state: MCPClientState = .error(error.errorDescription ?? "")
+ #expect(SettingsView.missingCommand(from: state) == "npx")
+ }
+}
diff --git a/oAITests/MCPTransportTests.swift b/oAITests/MCPTransportTests.swift
index 2a50f93..701dfe0 100644
--- a/oAITests/MCPTransportTests.swift
+++ b/oAITests/MCPTransportTests.swift
@@ -129,3 +129,160 @@ struct HTTPMCPTransportParseTests {
}
}
}
+
+@Suite("LoginShellEnvironment (deterministic, no-subprocess PATH probing)")
+struct LoginShellEnvironmentTests {
+
+ @Test("candidateDirectories includes the fixed Homebrew/MacPorts/Volta locations")
+ func candidateDirectoriesIncludesFixedLocations() {
+ let dirs = LoginShellEnvironment.candidateDirectories(home: "/Users/testuser")
+ #expect(dirs.contains("/opt/homebrew/bin"))
+ #expect(dirs.contains("/usr/local/bin"))
+ #expect(dirs.contains("/opt/local/bin"))
+ #expect(dirs.contains("/Users/testuser/.volta/bin"))
+ }
+
+ @Test("nvmDefaultNodeBinDirectory returns nil when the alias file doesn't exist")
+ func nvmReturnsNilWithoutAliasFile() {
+ #expect(LoginShellEnvironment.nvmDefaultNodeBinDirectory(home: "/nonexistent-\(UUID().uuidString)") == nil)
+ }
+
+ @Test("candidateDirectories appends the nvm default dir when present")
+ func candidateDirectoriesIncludesNvmWhenPresent() throws {
+ let tmpHome = NSTemporaryDirectory() + "confab-test-nvm-\(UUID().uuidString)"
+ let aliasDir = tmpHome + "/.nvm/alias"
+ try FileManager.default.createDirectory(atPath: aliasDir, withIntermediateDirectories: true)
+ try "v20.11.0\n".write(toFile: aliasDir + "/default", atomically: true, encoding: .utf8)
+ defer { try? FileManager.default.removeItem(atPath: tmpHome) }
+
+ let nvmDir = LoginShellEnvironment.nvmDefaultNodeBinDirectory(home: tmpHome)
+ #expect(nvmDir == "\(tmpHome)/.nvm/versions/node/v20.11.0/bin")
+ #expect(LoginShellEnvironment.candidateDirectories(home: tmpHome).contains(nvmDir ?? ""))
+ }
+
+ @Test("nvmDefaultNodeBinDirectory adds a 'v' prefix if the alias file lacks one")
+ func nvmAddsVPrefixIfMissing() throws {
+ let tmpHome = NSTemporaryDirectory() + "confab-test-nvm-\(UUID().uuidString)"
+ let aliasDir = tmpHome + "/.nvm/alias"
+ try FileManager.default.createDirectory(atPath: aliasDir, withIntermediateDirectories: true)
+ try "20.11.0".write(toFile: aliasDir + "/default", atomically: true, encoding: .utf8)
+ defer { try? FileManager.default.removeItem(atPath: tmpHome) }
+
+ #expect(LoginShellEnvironment.nvmDefaultNodeBinDirectory(home: tmpHome) == "\(tmpHome)/.nvm/versions/node/v20.11.0/bin")
+ }
+
+ @Test("augmentedPath prepends only directories that actually exist")
+ func augmentedPathOnlyAddsExistingDirs() {
+ // /usr/bin always exists on macOS; a random UUID-named dir never will.
+ let result = LoginShellEnvironment.augmentedPath(
+ basePath: "/usr/bin:/bin", home: "/nonexistent-\(UUID().uuidString)"
+ )
+ // None of the fixed candidates exist under a bogus home + this sandboxed test environment
+ // is unlikely to have /opt/homebrew, /usr/local, or /opt/local β but if it does (real dev
+ // machine running the suite), that's fine too: just confirm the base path is preserved.
+ #expect(result.hasSuffix("/usr/bin:/bin"))
+ }
+
+ @Test("augmentedPath doesn't duplicate a directory already present in basePath")
+ func augmentedPathAvoidsDuplicates() {
+ let result = LoginShellEnvironment.augmentedPath(basePath: "/opt/homebrew/bin:/usr/bin:/bin")
+ let components = result.split(separator: ":").map(String.init)
+ #expect(components.filter { $0 == "/opt/homebrew/bin" }.count == 1)
+ }
+
+ @Test("findExecutable finds a real executable on a real search path")
+ func findExecutableFindsRealBinary() {
+ // /bin/ls exists and is executable on every macOS install.
+ #expect(LoginShellEnvironment.findExecutable(named: "ls", in: "/usr/bin:/bin") == "/bin/ls")
+ }
+
+ @Test("findExecutable returns nil for a name that doesn't exist anywhere on the path")
+ func findExecutableReturnsNilForMissingBinary() {
+ #expect(LoginShellEnvironment.findExecutable(named: "definitely-not-a-real-binary-\(UUID().uuidString)", in: "/usr/bin:/bin") == nil)
+ }
+
+ @Test("findExecutable checks directories in order and returns the first match")
+ func findExecutableRespectsOrder() throws {
+ let tmpDir1 = NSTemporaryDirectory() + "confab-test-bin1-\(UUID().uuidString)"
+ let tmpDir2 = NSTemporaryDirectory() + "confab-test-bin2-\(UUID().uuidString)"
+ try FileManager.default.createDirectory(atPath: tmpDir1, withIntermediateDirectories: true)
+ try FileManager.default.createDirectory(atPath: tmpDir2, withIntermediateDirectories: true)
+ defer {
+ try? FileManager.default.removeItem(atPath: tmpDir1)
+ try? FileManager.default.removeItem(atPath: tmpDir2)
+ }
+ let toolName = "confab-test-tool-\(UUID().uuidString)"
+ for dir in [tmpDir1, tmpDir2] {
+ let path = "\(dir)/\(toolName)"
+ FileManager.default.createFile(atPath: path, contents: Data("#!/bin/sh\n".utf8))
+ try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: path)
+ }
+ #expect(LoginShellEnvironment.findExecutable(named: toolName, in: "\(tmpDir1):\(tmpDir2)") == "\(tmpDir1)/\(toolName)")
+ }
+
+ @Test("homebrewPrefix returns nil when neither known brew binary exists")
+ func homebrewPrefixNilWhenAbsent() {
+ // Can't easily fake a FileManager that reports both real paths as absent without a full
+ // protocol seam here, but we can confirm the function doesn't crash and returns a sensible
+ // type; the presence/absence branches are exercised implicitly by whichever machine runs
+ // this (either is a valid, non-crashing outcome).
+ let result = LoginShellEnvironment.homebrewPrefix()
+ #expect(result == nil || result == "/opt/homebrew" || result == "/usr/local")
+ }
+}
+
+@Suite("StdioMCPTransport.prepare() fails fast on a missing command")
+@MainActor
+struct StdioMCPTransportPrepareTests {
+
+ @Test("Throws commandNotFound immediately for a command that doesn't exist anywhere on PATH β never spawns a process")
+ func throwsForMissingRelativeCommand() async {
+ let server = ExternalMCPServer(
+ name: "Bogus",
+ command: "definitely-not-a-real-command-\(UUID().uuidString)",
+ args: []
+ )
+ let transport = StdioMCPTransport(server: server)
+ do {
+ try await transport.prepare()
+ Issue.record("Expected prepare() to throw")
+ } catch let error as MCPClientError {
+ switch error {
+ case .commandNotFound(let cmd): #expect(cmd == server.command)
+ default: Issue.record("Expected .commandNotFound, got \(error)")
+ }
+ } catch {
+ Issue.record("Expected MCPClientError, got \(error)")
+ }
+ }
+
+ @Test("Throws commandNotFound for an absolute path that doesn't exist")
+ func throwsForMissingAbsoluteCommand() async {
+ let server = ExternalMCPServer(
+ name: "Bogus",
+ command: "/nonexistent/\(UUID().uuidString)/binary",
+ args: []
+ )
+ let transport = StdioMCPTransport(server: server)
+ do {
+ try await transport.prepare()
+ Issue.record("Expected prepare() to throw")
+ } catch let error as MCPClientError {
+ switch error {
+ case .commandNotFound(let cmd): #expect(cmd == server.command)
+ default: Issue.record("Expected .commandNotFound, got \(error)")
+ }
+ } catch {
+ Issue.record("Expected MCPClientError, got \(error)")
+ }
+ }
+
+ @Test("Succeeds in resolving a real, always-present absolute command")
+ func doesNotThrowForRealAbsoluteCommand() async throws {
+ // /bin/echo exists on every macOS install and exits immediately β safe to actually launch.
+ let server = ExternalMCPServer(name: "Echo", command: "/bin/echo", args: ["hi"])
+ let transport = StdioMCPTransport(server: server)
+ try await transport.prepare()
+ transport.stop()
+ }
+}