diff --git a/CLI/BonjourDiscovery.swift b/CLI/BonjourDiscovery.swift index 2a32739b..a33b79a6 100644 --- a/CLI/BonjourDiscovery.swift +++ b/CLI/BonjourDiscovery.swift @@ -26,8 +26,9 @@ enum BonjourDiscovery { } /// Starts `browser` on the main queue - /// and returns the first result that satisfies `preferring`, - /// or the first result at all if none does. + /// and returns the first service on this Mac that satisfies `preferring`, + /// or the first service on this Mac if none does. + /// Services on other Macs are ignored (#257). /// /// The browser is cancelled before this function returns or throws, /// whether a result was found, the browser failed, or `timeout` elapsed. @@ -58,7 +59,7 @@ enum BonjourDiscovery { } browser.browseResultsChangedHandler = { results, _ in - guard let selected = results.first(where: isPreferred) ?? results.first else { return } + guard let selected = select(from: results, isLocal: isLocal, preferring: isPreferred) else { return } Task { if await state.checkAndSetResumed() { timeoutTask.cancel() @@ -73,6 +74,28 @@ enum BonjourDiscovery { } } + /// Returns the first local result that satisfies `preferring`, + /// or the first local result if none does. + static func select( + from results: some Collection, + isLocal: (Result) -> Bool, + preferring isPreferred: (Result) -> Bool + ) -> Result? { + let local = results.filter(isLocal) + return local.first(where: isPreferred) ?? local.first + } + + /// Returns whether `result` is a service advertised by this Mac. + /// + /// Bonjour reports a service registered on this Mac on the loopback interface, + /// and a service from another Mac only on network interfaces. + /// iMCP on another Mac accepts connections over its own loopback only (#242), + /// and its advertised port is meaningless here, + /// because the connection goes to this Mac's loopback address (#229). + static func isLocal(_ result: NWBrowser.Result) -> Bool { + result.interfaces.contains { $0.type == .loopback } + } + static func connectionEndpoint( for serviceEndpoint: NWEndpoint, metadata: NWBrowser.Result.Metadata diff --git a/CLI/MCPService.swift b/CLI/MCPService.swift index f4acbb16..1e360d15 100644 --- a/CLI/MCPService.swift +++ b/CLI/MCPService.swift @@ -31,7 +31,7 @@ actor MCPService: Service { using: parameters ) - // Prefer a service advertised as iMCP; fall back to any MCP service. + // Prefer a service advertised as iMCP; fall back to any MCP service on this Mac. // The helper cancels the browser on every exit path, // so a timed-out attempt doesn't leak a DNS-SD connection (#192). let endpoint: NWEndpoint @@ -42,7 +42,13 @@ actor MCPService: Service { preferring: { String(describing: $0.endpoint).contains("iMCP") } ) } catch BonjourDiscovery.Error.timeout { - await log.error("Bonjour service discovery timed out after 30 seconds") + await log.error( + """ + Bonjour service discovery timed out after 30 seconds. \ + Check that iMCP is running, and that imcp-server is turned on in \ + System Settings > Privacy & Security > Local Network. + """ + ) throw MCPError.internalError("Service discovery timeout") } await log.info("Selected endpoint: \(endpoint)") diff --git a/CLI/StdioProxy.swift b/CLI/StdioProxy.swift index 4da36553..e27cb6d8 100644 --- a/CLI/StdioProxy.swift +++ b/CLI/StdioProxy.swift @@ -44,14 +44,8 @@ actor StdioProxy { let connection = NWConnection(to: endpoint, using: parameters) self.connection = connection - connection.start(queue: .main) - - connection.stateUpdateHandler = { [weak self] state in - Task { [weak self] in - await self?.handleConnectionState(state, continuation: nil, connectionState: nil) - } - } - + // Set the handler before starting the connection, + // so the first `.waiting` state isn't missed (#257). try await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in let connectionState = ConnectionState() @@ -64,6 +58,7 @@ actor StdioProxy { ) } } + connection.start(queue: .main) } var sessionError: (any Swift.Error)? @@ -152,6 +147,15 @@ actor StdioProxy { await stop() case .waiting(let error): await log.debug("Connection waiting: \(error)") + // A connection that can't be established, for example because nothing + // listens on the port, waits and retries indefinitely. + // Fail instead, so that the caller reports the error and tries again (#257). + if let continuation = continuation, + await shouldResume(connectionState: connectionState) + { + continuation.resume(throwing: error) + await stop() + } case .preparing: await log.debug("Connection preparing...") case .setup: diff --git a/CLITests/BonjourDiscoveryTests.swift b/CLITests/BonjourDiscoveryTests.swift index 356df8c2..dc80e29c 100644 --- a/CLITests/BonjourDiscoveryTests.swift +++ b/CLITests/BonjourDiscoveryTests.swift @@ -49,6 +49,44 @@ final class BonjourDiscoveryTests: XCTestCase { } } + /// A browse result reduced to what selection looks at. + private struct Candidate: Equatable { + var name: String + var isLocal: Bool + } + + private func select(_ candidates: [Candidate]) -> Candidate? { + BonjourDiscovery.select( + from: candidates, + isLocal: \.isLocal, + preferring: { $0.name.contains("iMCP") } + ) + } + + /// With iMCP on two Macs, both advertise a service named iMCP, + /// and the one registered second is renamed, for example to "iMCP (2)". + /// Only the service on this Mac can accept the connection (#257). + func testSelectsServiceOnThisMacOverAnotherMac() { + let remote = Candidate(name: "iMCP", isLocal: false) + let local = Candidate(name: "iMCP (2)", isLocal: true) + + XCTAssertEqual(select([remote, local]), local) + XCTAssertEqual(select([local, remote]), local) + } + + func testIgnoresServicesOnOtherMacs() { + XCTAssertNil(select([Candidate(name: "iMCP", isLocal: false)])) + XCTAssertNil(select([Candidate(name: "Other", isLocal: false)])) + } + + func testPrefersiMCPAmongServicesOnThisMac() { + let other = Candidate(name: "Other", isLocal: true) + let iMCP = Candidate(name: "iMCP", isLocal: true) + + XCTAssertEqual(select([other, iMCP]), iMCP) + XCTAssertEqual(select([other, Candidate(name: "iMCP", isLocal: false)]), other) + } + /// A browser that times out without finding anything must be cancelled, /// not left running. func testTimedOutBrowserIsCancelled() async throws { diff --git a/README.md b/README.md index 51c8a449..8bca8320 100644 --- a/README.md +++ b/README.md @@ -352,6 +352,17 @@ such as those returned by iMCP tools. ## Debugging +### Allowing local network access + +`imcp-server` finds the iMCP app with Bonjour, +so it needs permission to access the local network. +`imcp-server` has its own entry in +System Settings > Privacy & Security > Local Network, +separate from iMCP and from the app that runs it, such as your terminal. +If that entry is turned off, +`imcp-server` can't find the app +and reports that Bonjour service discovery timed out. + ### Using the MCP Inspector To debug interactions between iMCP and clients,