refactor(mcp): rewrite server, bridge, and protocol layer#964
Merged
refactor(mcp): rewrite server, bridge, and protocol layer#964
Conversation
Wire layer (TablePro/Core/MCP/Wire/) — pure value types with no transport awareness: tagged JsonRpcMessage enum (4 cases), JsonRpcId supporting null explicitly, JsonRpcCodec, strict-CRLF HttpRequestParser, SseEncoder/Decoder, typed JsonRpcDecodingError. Replaces ad-hoc message types and tolerant HTTP parser. Session layer (Session/, Auth/, RateLimit/) — actor-based with eviction broadcast: MCPSessionStore actor with multicast events stream, idle eviction under MCPClock abstraction, MCPBearerTokenAuthenticator with SHA-256 token fingerprint, rate limiter rekeyed on (clientAddress, principalFingerprint) to fix the localhost auth-DoS issue. Idle timeout raised from 5 to 15 min. Old types renamed with Legacy prefix to coexist during the rewrite — they will be deleted in phase 6 once all callers are migrated.
Three new transports under Core/MCP/Transport/, all built on the new
JsonRpcMessage type. Bridge transports (StdioMessageTransport, Streamable
HttpClientTransport) conform to MCPMessageTransport — symmetric inbound
stream + send. The stdio transport reads via FileHandle.bytes (no more
availableData polling) and writes only valid JSON-RPC to stdout. The
streaming HTTP client uses URLSession.bytes for incremental SSE parsing
and synthesizes JSON-RPC error envelopes from non-2xx HTTP responses so
no malformed line ever reaches the host's stdin — fixing the bridge bug
at the architecture level.
The in-app HTTP server transport (MCPHttpServerTransport) is exchange-
based: each inbound POST yields an MCPInboundExchange whose responder
handles wire-level details. Every error path now produces a JSON-RPC
envelope; the previous bare {"error":...} body shape is impossible to
emit by construction. CORS now includes Last-Event-ID. Remote access
without TLS is rejected at configuration time, not at runtime.
MCPProtocolError carries both JSON-RPC code and HTTP status; static
factories (sessionNotFound, unauthenticated, etc.) encode the spec-
correct mappings in one place.
ProtocolDispatcher actor routes inbound JSON-RPC by method, validates session state and required scopes, registers cancellation tokens, builds request contexts, invokes handlers, and translates typed errors back into JSON-RPC envelopes. Catches MCPProtocolError specifically and falls back to internalError for everything else. Per-method handlers split out as small structs: Initialize, Ping, ToolsList, ToolsCall, ResourcesList/Read/TemplatesList, plus stubs for prompts/logging/ completion. Tools live in their own registry — 19 implementations under Protocol/Tools/ each conforming to MCPToolImplementation. JsonValueLegacy Bridge converts at the MCPConnectionBridge boundary so the legacy JSONValue type can stay on the existing data layer until phase 6. Streaming progress works via MCPProgressEmitter — handlers emit progress events that the transport routes as notifications/progress to the session SSE stream. ExecuteQueryTool uses this at four checkpoints. Cancellation flows through MCPCancellationToken, looked up by (requestId, sessionId) in MCPInflightRegistry; notifications/cancelled triggers token cancel.
The tablepro-mcp binary is now a thin composition root over the new transport types. BridgeMain wires up MCPStdioMessageTransport (host side) and MCPStreamableHttpClientTransport (upstream side); BridgeProxy forwards JsonRpcMessage objects between them with a TaskGroup so both directions run concurrently. Errors land in os_log via MCPCompositeBridgeLogger and never leak to stdout — the host-facing transport guarantees only valid JSON-RPC bytes reach Claude Desktop's stdin. Handshake module now uses Duration/ContinuousClock instead of TimeInterval math, and properly distinguishes 'file missing' from 'process not running' when deciding whether to relaunch. mcp-server target's pbxproj exception list expanded to include the Wire/ and Transport/ files the bridge depends on; main.swift renamed to BridgeMain.swift since @main on a struct is incompatible with main.swift filename. Old MCPBridgeProxy.swift deleted.
MCPServerManager is now a thin composition root: it builds an MCPHttpServerTransport, an MCPProtocolDispatcher with all method handlers registered, an MCPSessionStore, MCPBearerTokenAuthenticator, and MCPRateLimiter, then wires the transport's exchanges stream into the dispatcher. Public API (start/stop/restart/lazyStart/tokenStore/state) preserved so AppDelegate, AppSettingsManager, MCPSection, and the launch intent router don't need changes beyond a SessionSnapshot type rename. Legacy code deleted: MCPServer, MCPRouter, MCPRouteHandler, MCPHTTPParser, MCPMessageTypes (JSONRPCRequest/Response/etc), MCPSession, MCPSessionPhase, MCPRateLimiter, MCPToolHandler (+Integrations), MCPResourceHandler, Routes/MCPProtocolHandler, Routes/IntegrationsExchange Handler, JsonValueLegacyBridge. About 7000 lines removed. MCPConnectionBridge migrated from legacy JSONValue to the new JsonValue type. All tools, resource handlers, and helpers updated accordingly. MCPError extracted into its own file (still used by the connection bridge, auth policy, and pairing service). Pre-existing test target compile errors fixed in passing: typos in TableRowsMutationTests, missing 'try' in EvictionTests/RowOperationsDispatch Tests/SortCacheInvalidationTests, obsolete sidebarLoadingState references removed, DeeplinkParser migration in ConnectionSharingTests, MCPTokenStore Tests updated for ConnectionAccess, plus deletion of obsolete legacy MCP test files (MCPRouterTests, MCPAuthGuardTests, etc.) that tested code we just deleted. CHANGELOG entry under Unreleased explains the user-facing impact. xcodebuild build clean for both TablePro and mcp-server schemes; swiftlint strict zero violations on Core/MCP/.
Conform to LocalizedError so error.localizedDescription returns the actual reason (e.g., 'Remote access requires TLS to be enabled') instead of the opaque NSError-bridged 'MCPHttpServerError error 0'. Also log the configuration shape on start so failures show bindAddress, port, and tls status.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2108fb4909
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+195
to
+196
| if method == "initialize" { | ||
| return try? await sessionStore.create() |
There was a problem hiding this comment.
initialize sessions are already created in MCPHttpServerTransport.handlePostMcp before dispatch, but resolveOrCreateSession creates a second session whenever method == "initialize". This leaks an uninitialized orphan session per initialize call and effectively halves usable capacity (maxSessions is 16), which can trigger premature capacityExceeded under normal reconnect churn.
Useful? React with 👍 / 👎.
| let sink = TransportResponderSink(transport: self, context: context) | ||
| let responder = MCPExchangeResponder(sink: sink, requestId: nil) | ||
|
|
||
| let placeholderRequest = JsonRpcRequest(id: .null, method: "$/sse-stream", params: nil) |
There was a problem hiding this comment.
GET /mcp is converted into a synthetic request with method "$/sse-stream", but no handler is registered for that method in the dispatcher handler set. In the current flow this yields methodNotFound and closes the connection, so server-initiated SSE channels cannot be established and session-targeted notifications (including progress) are dropped.
Useful? React with 👍 / 👎.
| if await state.serverInitiatedStreamOpen { | ||
| return | ||
| } | ||
| await state.markServerInitiatedStreamOpen(true) |
There was a problem hiding this comment.
openSseStream() sets serverInitiatedStreamOpen to true before launching the GET stream task, but runServerInitiatedStream() never resets it on completion/error. After any disconnect, subsequent openSseStream() calls return early and never attempt reconnection, leaving the client permanently without its SSE channel until transport restart.
Useful? React with 👍 / 👎.
NWListener(using:on:) with parameters.requiredLocalEndpoint already set produces EINVAL because the port and the endpoint conflict. The endpoint in NWParameters carries both host and port; passing 'on: port' tries to override the port at the listener constructor and the framework rejects it with errno 22. Use NWListener(using:) and let the configured endpoint in parameters drive the bind.
…trings Auto-extracted localization keys from the new Phase 4 tool descriptions land in Localizable.xcstrings. .gitignore extended to cover Xcode coverage output (.profraw) and the Claude Code per-session worktree directory so they don't show as untracked clutter on other branches.
The 'Setup for Claude Desktop / Cursor' helpers were emitting JSON with the 'url' transport key, which Claude Desktop rejects entirely (it only speaks stdio) and which makes Cursor reach the HTTP server directly with no bridge. Both should point at the bundled tablepro-mcp binary so the stdio bridge handles the handshake and lazy-launch. The Claude Code command line is updated to the same form: 'claude mcp add tablepro -- <bridge-path>' instead of '--transport http <url>'. Path is computed from Bundle.main so it works for non-/Applications installs (Setapp, DerivedData, etc.).
HttpConnectionContext.send was fire-and-forget — it queued the response bytes via NWConnection.send but returned immediately. handleReceive called context.cancel() right after, racing the flush, so URLSession got 'connection lost' instead of the response body. Make send await the .contentProcessed callback (via withCheckedContinuation) and propagate the await up through writeJsonResponse, writeOptions204, writeNoContent, writeAccepted, writeSseStreamHeaders, writeSseFrame, writeRaw. Cancel now safely runs only after the network framework confirms the data has been handed off.
…auth ToolsCallHandler now records every tool invocation (success/denied/error) with the principal's token label and the tool's connection_id argument (when present). ResourcesReadHandler does the same for resources/read. ToolQueryExecutor adds an MCPAuditLogger.logQueryExecuted entry alongside the existing QueryHistoryStorage write so SQL execution shows up in the Activity Log. MCPBearerTokenAuthenticator records auth success/failure/ rate-limit events with the client IP. Drops the 'legacy' label from the domain error mapper in ResourcesReadHandler — MCPError is the data layer's domain error type, not legacy code.
…verlap The legacy MCPError enum had two kinds of cases mixed together: domain errors thrown by the data layer (notConnected, forbidden, timeout, etc.) and JSON-RPC-shaped errors (parseError, invalidRequest, methodNotFound, internalError, invalidParams) that overlapped with the protocol layer's MCPProtocolError. The unused JSON-RPC cases are deleted; the still-used ones are renamed to domain-flavored names (.invalidParams -> .invalid Argument, .internalError -> .dataSourceError) so the data layer no longer borrows protocol-layer terminology. mapDomainError in ResourcesReadHandler now translates the full set of domain errors into appropriate MCPProtocolError values (notFound -> resourceNotFound + 404, timeout -> requestTimeout, etc.) without a default case, so future additions to MCPDataLayerError will fail the compile and force an explicit decision about the protocol mapping.
LaunchIntentRouter, PairingApprovalSheet, and MCPPairingServiceTests also catch the data-layer error type; updated to the new name.
…HANGELOG MCPTokenStore now detects the old allowedConnectionIds field via a byte scan and re-saves immediately after decode, so stale on-disk format disappears after one launch instead of lingering forever. The decoder side already handled both shapes — this just forces the encode pass. CHANGELOG split the rewrite into Added (streaming progress as a real new capability), Fixed (5 user-facing bugs that are gone), and Changed (idle timeout + the 'this is a rewrite, but transparent to you' note).
Old MCP wasn't shipped to users yet, so there's no on-disk format to migrate. Removes the dual-decode branch and the byte-scan migration writeback. Tokens persisted before this change with the old field name won't decode — that's fine, they were never released.
MCPStdioMessageTransport and MCPStreamableHttpClientTransport are now actors. The previous outer-class-with-NSLock-around-internal-actor pattern carried @unchecked Sendable; converting to actors gets compiler- verified Sendable for free. Internal mutable state (readerTask, isClosed, sessionId, pendingRequests, etc.) is actor-isolated. The 'inbound' stream stays nonisolated public let so existing callers like BridgeProxy keep working without await on the property access. The 'var capturedContinuation: ...!' implicitly-unwrapped continuation capture is replaced by a small StreamContinuationBox value type that holds the continuation behind a lock. Sendable without @unchecked. MCPBridgeLogger.StderrWriter is also now an actor; the sync log() API is preserved by spawning a fire-and-forget Task — log ordering is best- effort, which is fine for stderr. Adds 15 new tool test files (DescribeTable, Disconnect, ExportData, FocusQueryTab, GetConnectionStatus, GetTableDdl, ListConnections, ListDatabases, ListRecentTabs, ListSchemas, ListTables, OpenConnection Window, OpenTableTab, SearchQueryHistory, SwitchSchema). Each follows the ConnectToolTests pattern: metadata + missing-arg + malformed-arg. Fixes a switch-vs-struct typo in two tests.
Spins up real MCPHttpServerTransport + MCPStreamableHttpClientTransport +
MCPStdioMessageTransport in-process and exercises the full request/response
path through a TestBridgeProxy that replicates production BridgeProxy
logic (production class lives in the mcp-server target only, not visible
to TableProTests via pbxproj membership exceptions).
Four scenarios: happy-path init+tools/list, idle session eviction returns
-32001 envelope, server emitting non-spec body is wrapped into a JSON-RPC
error envelope by the client transport, malformed request returns a
JSON-RPC error envelope (not the legacy plain {error:...} shape).
Catches the class of wiring bugs that produced the 7 fix commits after
the initial PR open.
Phase 6 deleted Routes/IntegrationsExchangeHandler.swift on the assumption
that the bridge runs in-process with the app, so an HTTP route for
pairing-code exchange was redundant. That assumption is wrong: the
Raycast extension is a separate Node.js process that POSTs to
/v1/integrations/exchange to redeem a pairing code for a bearer token,
and any other future external integration would do the same.
Restore the route inline in MCPHttpServerTransport.dispatch — POST to
/v1/integrations/exchange parses {code, code_verifier}, calls MCPPairing
Service.shared.exchange on MainActor, returns {token} on 200 or a plain
{error: ...} JSON body on 4xx/5xx. Adds writePlainJsonResponse +
writePlainJsonError helpers on HttpConnectionContext for the non-MCP
shape this endpoint uses.
mcp-tools.mdx — fix scope mismatches across tools (disconnect, switch_*,
confirm_destructive_operation, navigation tools), document the streaming
progress flow via _meta.progressToken, replace the HTTP-status error
table with the JsonRpcErrorCode + HTTP status pairs that match
JsonRpcErrorCode.swift / MCPProtocolError.swift, fix execute_query and
export_data shapes to match what the actual handlers return, fix
describe_table's database/schema defaulting note, add the spec-compliant
404/-32001 'Session not found' paragraph.
mcp-clients.mdx — drop the spurious '--transport stdio' flag from the
Claude Code command (stdio is the default). Add 404 and 429 entries to
the verification troubleshooting list. Document the WWW-Authenticate
header on 401.
mcp-resources.mdx — new Discovery section for resources/list and
resources/templates/list (latter was undocumented). Add the {contents:
[{uri,mimeType,text}]} envelope shape that resources/read returns.
tokens.mdx — rewrite TokenPermissions -> Set<MCPScope> mapping to match
MCPBearerTokenAuthenticator.mcpScopes. Replace the per-IP escalating
lockout description with the new flat policy from MCPRateLimitPolicy
(5 fails / 60s window / 5min lockout / (client_address, principal_
fingerprint) keyed).
url-scheme.mdx — remove the 'token=' query parameter described against
DeeplinkParser.parseQuery, which doesn't read it. Add the 51,200-char
SQL cap and a pointer to MCP execute_query for headless work.
versioning.mdx — note protocolVersion: 2025-03-26 from InitializeHandler.
features/mcp.mdx — replace 5-minute escalating lockout with the new
policy, matching tokens.mdx.
Adds info/warning logs at handleNewConnection and at every branch of handleIntegrationsExchange so we can see whether requests are arriving, whether bodies parse, and whether the handler hits the success or failure path. No behavior change.
The hig-audit notes were committed by accident during the doc audit pass. Untrack from index — keep on disk locally — so they don't ride along on future clones.
…ecting The earlier 'strict negotiation' rejected any protocolVersion not in our supported set with -32600 invalid request. Per the MCP transport spec, the server SHOULD respond with the highest version it supports when the client requests a newer one. Claude Code now uses 2025-11-25 (latest); our supportedProtocolVersions is just 2025-03-26, so its initialize was rejected and the bridge probe surfaced as 'Failed to connect'. Make negotiate() always return a valid version, downgrading unknown values to supportedProtocolVersion. Drop the unused 'unsupported version' guard.
Server now advertises and negotiates 2025-11-25 (the latest spec) as the preferred protocol version, with 2025-06-18 and 2025-03-26 as backward-compatible fallbacks. Capabilities advertise only what we genuinely implement — completions endpoint added (was already a stub handler), elicitation deliberately omitted because the server doesn't initiate sampling. 2025-11-25 features wired: - Structured tool output (structuredContent alongside content[]): list_*, describe_table, get_table_ddl, get_connection_status, list_recent_tabs, search_query_history, execute_query, and confirm_destructive_operation now return their structured data twice, once as text for older clients and once as structuredContent for clients that prefer the typed shape. - Tool annotations (readOnlyHint, destructiveHint, idempotentHint, openWorldHint, title) emitted by tools/list. Read tools are marked readOnly+idempotent; execute_query and export_data are openWorld; confirm_destructive_operation is destructive; switch_*, connect, disconnect, and the open_/focus_ UI tools opt out of safety hints. - ServerInfo now includes a 'title' field per the new spec. Tests updated: InitializeHandler accepts all three versions and downgrades unknown ones to 2025-11-25; ToolsListHandler emits the annotation map; ToolsCallHandler passes structuredContent through.
…te, widen token fingerprint
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Full rewrite of TablePro's MCP layer, plus a pre-ship cleanup pass. 195 files changed, +15,600 / -7,040 across 50 commits.
The original bug: a stale
Mcp-Session-Idafter the idle timeout broke Claude Desktop's stdio parser. That cannot happen now. Every non-2xx response is a JSON-RPC envelope. The stdio bridge writes only validatedJsonRpcMessagebytes to stdout.Architecture
Spec compliance
The server accepts three protocol versions:
2025-03-26,2025-06-18,2025-11-25. The server echoes the version the client asks for. Unknown versions get2025-11-25back, and the client decides whether to use it.Capabilities advertised match what the server actually does. No
elicitation, nosubscribe, nolistChanged: true.2025-11-25features:tools/callreturns bothcontent[](for older clients) andstructuredContent(typed object).readOnlyHint,idempotentHint,destructiveHint,openWorldHintper tool.serverInfo.title.Error handling is now spec-correct.
{"jsonrpc":"2.0","error":{"code":-32001,"message":"Session not found"}}.WWW-Authenticate.Retry-Afterwith the actual lockout duration.-32009unauthenticated,-32007forbidden, etc.Streaming
MCPProgressEmitteris now wired through the SSE channel that the client opens via GET/mcp.executeQueryemits progress at four stages. Clients that pass_meta.progressTokenget them.The HTTP client transport uses
URLSession.bytes(for:)for incremental SSE parsing instead of buffering. The stdio transport reads viaFileHandle.bytesinstead ofavailableData, which exited mid-session on transient stdin pauses.Concurrency
The dispatcher spawns a child Task per inbound exchange. Concurrent tool calls actually parallelize.
The transport-allocated session is reused on
initializeinstead of creating a second one. Token revocation cancels in-flight requests throughMCPInflightRegistry.HttpWriternow uses a chained Task pipeline for true serial outbound writes.Bridge
tablepro-mcpis a 50-line composition root.MCPStdioMessageTransporthost-side,MCPStreamableHttpClientTransportupstream,BridgeProxyforwards messages with aTaskGroup. Errors land in os_log and stderr viaMCPCompositeBridgeLogger. The host-facing transport never writes anything that is not a valid JSON-RPC message.What got deleted (about 7,000 lines)
MCPServer,MCPRouter,MCPRouteHandler,MCPHTTPParser,MCPMessageTypes,MCPSession,MCPSessionPhase,MCPRateLimiter,MCPToolHandler(+Integrations),MCPResourceHandler,Routes/MCPProtocolHandler,Routes/IntegrationsExchangeHandler,JsonValueLegacyBridge.MCPErrorwas split intoMCPDataLayerError(pure domain).The Phase 6 deletion accidentally removed
/v1/integrations/exchange. Restored after Raycast's pairing flow needed it.Tests
Unit tests across wire codec, transport, session, auth, rate limit, dispatcher, handlers. Integration tests spin up real
MCPHttpServerTransport+MCPStreamableHttpClientTransport+BridgeProxyand exercise four scenarios end to end.AppDelegate.applicationDidFinishLaunchingskips the full app boot whenXCTestConfigurationFilePathis set, so test runs no longer leak orphan TablePro processes with the welcome window.What stayed compatible
MCPServerManager.shared.{start,stop,restart,lazyStart,tokenStore,state}is unchanged.AppDelegate,AppSettingsManager,MCPSection, andLaunchIntentRouterdid not need changes.~/Library/Application Support/TablePro/mcp-handshake.jsonis unchanged. Existing bridges keep working.structuredContent.Test plan
xcodebuild -scheme TablePro buildcleanxcodebuild -scheme mcp-server buildcleanswiftlint lint --strict TablePro/Core/MCP TablePro/CLIcleanclaude mcp add tablepro -- /Applications/TablePro.app/Contents/MacOS/tablepro-mcpthenclaude mcp listshows ConnectedinitializewithprotocolVersion: 2025-11-25returns2025-11-25execute_queryfrom Raycast with progress in the toastexecute_query, expect the call to abortRetry-AfterKnown follow-ups
MCPBridgeIntegrationTests.testIdleSessionEvictionReturnsSessionNotFoundErrorflakes when run with neighbours. Pre-existing, fixture isolation.mcp-servertarget's pbxproj membership exception list is hand-maintained. Extracting Wire and Transport into a Swift package would remove that.