Fix Edit/Save not applying to an already-connected External MCP server
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).
This commit is contained in:
@@ -52,8 +52,18 @@ final class ExternalMCPManager {
|
|||||||
restartAttempts.removeValue(forKey: id)
|
restartAttempts.removeValue(forKey: id)
|
||||||
removeCachedSchemas(for: id)
|
removeCachedSchemas(for: id)
|
||||||
}
|
}
|
||||||
for server in servers where server.isEnabled && clients[server.id] == nil {
|
for server in servers where server.isEnabled {
|
||||||
|
if clients[server.id] == nil {
|
||||||
startClient(for: server)
|
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.
|
/// to somehow resolve on their own; neither state is retried automatically.
|
||||||
func retryClient(id: UUID) {
|
func retryClient(id: UUID) {
|
||||||
guard let server = serverConfigs[id] else { return }
|
guard let server = serverConfigs[id] else { return }
|
||||||
restartAttempts.removeValue(forKey: id)
|
restartFresh(server)
|
||||||
restartTasks[id]?.cancel()
|
}
|
||||||
restartTasks.removeValue(forKey: id)
|
|
||||||
|
/// 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)
|
startClient(for: server)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -14,7 +14,7 @@ nonisolated enum MCPTransportKind: String, Codable, Sendable, CaseIterable {
|
|||||||
case http
|
case http
|
||||||
}
|
}
|
||||||
|
|
||||||
nonisolated struct ExternalMCPServer: Codable, Identifiable, Sendable {
|
nonisolated struct ExternalMCPServer: Codable, Identifiable, Sendable, Equatable {
|
||||||
var id: UUID
|
var id: UUID
|
||||||
var name: String
|
var name: String
|
||||||
var transportKind: MCPTransportKind
|
var transportKind: MCPTransportKind
|
||||||
|
|||||||
@@ -213,3 +213,25 @@ struct MissingCommandDetectionTests {
|
|||||||
#expect(SettingsView.missingCommand(from: state) == "npx")
|
#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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user