From 41f0b8f083b5a57d43072c470b9871fae5016465 Mon Sep 17 00:00:00 2001 From: Rune Olsen Date: Wed, 26 Aug 2026 14:12:28 +0200 Subject: [PATCH] Fix Edit/Save not applying to an already-connected External MCP server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ExternalMCPManager.reconfigure() only started brand-new clients — a server ID that already had a client (in ANY state, including .crashed) was silently skipped even when its settings had just changed. Editing a server in Settings and clicking Save persisted correctly but never reached the live connection, which just kept running with its old (often broken) config until the next app launch. Rune hit this directly editing Obsidian's URL/token after the toggle-fields bug corrupted it. Added ExternalMCPServer: Equatable so reconfigure can detect a changed config for a still-enabled server and restart it fresh (extracted the restart-attempt-reset logic already used by retryClient into a shared restartFresh helper). --- oAI/Services/ExternalMCPManager.swift | 27 ++++++++++++++++++++++----- oAI/Services/ExternalMCPModels.swift | 2 +- oAITests/ExternalMCPModelsTests.swift | 22 ++++++++++++++++++++++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/oAI/Services/ExternalMCPManager.swift b/oAI/Services/ExternalMCPManager.swift index 7b76fe7..2c64cd0 100644 --- a/oAI/Services/ExternalMCPManager.swift +++ b/oAI/Services/ExternalMCPManager.swift @@ -52,8 +52,18 @@ final class ExternalMCPManager { restartAttempts.removeValue(forKey: id) removeCachedSchemas(for: id) } - for server in servers where server.isEnabled && clients[server.id] == nil { - startClient(for: server) + for server in servers where server.isEnabled { + if clients[server.id] == nil { + startClient(for: server) + } else if serverConfigs[server.id] != server { + // Same server, different settings — e.g. the user just edited it in Settings. A + // client already existing here (in ANY state, including .crashed/.error) used to + // mean reconfigure did nothing for it at all, so an edited config never reached the + // running connection until the next app launch — this is the fix for that (Rune hit + // it directly: editing Obsidian's URL/token and clicking Save appeared to do + // nothing because the crashed client just kept sitting there with the old config). + restartFresh(server) + } } } @@ -82,9 +92,16 @@ final class ExternalMCPManager { /// 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) + restartFresh(server) + } + + /// Clears any pending restart backoff/attempt count and starts a client from scratch — shared + /// by `retryClient` (manual retry after fixing an external cause) and `reconfigure` (an already- + /// connecting/crashed server whose settings just changed, e.g. via Edit). + private func restartFresh(_ server: ExternalMCPServer) { + restartAttempts.removeValue(forKey: server.id) + restartTasks[server.id]?.cancel() + restartTasks.removeValue(forKey: server.id) startClient(for: server) } diff --git a/oAI/Services/ExternalMCPModels.swift b/oAI/Services/ExternalMCPModels.swift index 99c6546..a265458 100644 --- a/oAI/Services/ExternalMCPModels.swift +++ b/oAI/Services/ExternalMCPModels.swift @@ -14,7 +14,7 @@ nonisolated enum MCPTransportKind: String, Codable, Sendable, CaseIterable { case http } -nonisolated struct ExternalMCPServer: Codable, Identifiable, Sendable { +nonisolated struct ExternalMCPServer: Codable, Identifiable, Sendable, Equatable { var id: UUID var name: String var transportKind: MCPTransportKind diff --git a/oAITests/ExternalMCPModelsTests.swift b/oAITests/ExternalMCPModelsTests.swift index 8357e02..d9f0fb7 100644 --- a/oAITests/ExternalMCPModelsTests.swift +++ b/oAITests/ExternalMCPModelsTests.swift @@ -213,3 +213,25 @@ struct MissingCommandDetectionTests { #expect(SettingsView.missingCommand(from: state) == "npx") } } + +@Suite("ExternalMCPServer.Equatable") +struct ExternalMCPServerEquatableTests { + + @Test("Two servers with identical fields are equal") + func identicalServersAreEqual() { + let id = UUID() + let createdAt = Date() + let a = ExternalMCPServer(id: id, name: "Obsidian", transportKind: .http, url: "http://127.0.0.1:27123/mcp/", bearerToken: "tok", timeout: 30, createdAt: createdAt) + let b = ExternalMCPServer(id: id, name: "Obsidian", transportKind: .http, url: "http://127.0.0.1:27123/mcp/", bearerToken: "tok", timeout: 30, createdAt: createdAt) + #expect(a == b) + } + + @Test("A changed URL/bearerToken makes servers unequal — this is what ExternalMCPManager.reconfigure uses to detect an edit") + func editedFieldsMakeServersUnequal() { + let id = UUID() + let createdAt = Date() + let original = ExternalMCPServer(id: id, name: "Obsidian", transportKind: .http, url: "http://127.0.0.1:27123/mcp/", bearerToken: "old-token", timeout: 30, createdAt: createdAt) + let edited = ExternalMCPServer(id: id, name: "Obsidian", transportKind: .http, url: "http://127.0.0.1:27123/mcp/", bearerToken: "new-token", timeout: 30, createdAt: createdAt) + #expect(original != edited) + } +}