Fix MCP catalog refresh races and transport diagnostics - #1563
Open
niieani wants to merge 3 commits into
Open
Conversation
Share the complete tool-production operation for concurrent refreshes of the same connection, including upstream discovery and catalog persistence. Explicit refreshes promote an in-flight background refresh so authoritative empty catalogs retain explicit semantics. This prevents simultaneous Desktop tool reads from opening overlapping SQLite transactions, which previously produced 'cannot start a transaction within a transaction' and left MCP integrations degraded despite a usable cached catalog. Preserve MCP connector and discovery failure messages as incomplete-catalog reasons so connection health reports the actionable transport failure instead of the generic 'plugin returned an incomplete tool catalog'. Regression coverage exercises concurrent stale reads and verifies MCP discovery failures reach degraded health. Live verification against the active Gateway configuration completed eight concurrent refreshes with eight HTTP 200 responses and no SQLite collision.
Propagate NODE_EXTRA_CA_CERTS, SSL_CERT_FILE, and SSL_CERT_DIR from the installing shell into the supervised daemon environment generated for launchd, systemd, and Windows Task Scheduler. Service managers intentionally start Executor with a minimal environment. On machines whose HTTPS trust depends on an enterprise or intercepting CA bundle, dropping these variables made MCP and other HTTPS integrations fail only in the background service while identical calls from the CLI succeeded. MCP auto transport then obscured the TLS failure behind its final SSE fallback error. Add regression coverage at the shared service-environment boundary to ensure all supported CA path variables survive unit generation without embedding certificate contents or other credentials. Reproduced with an isolated compiled daemon: the Gateway succeeded with the shell CA environment and degraded when both CA variables were removed. After rebuilding and reinstalling the LaunchAgent, the active Gateway cleared its degraded health and eight concurrent refreshes returned eight HTTP 200 responses.
Normalize Effect HTTP transport failures into safe structural categories for TLS verification, DNS resolution, timeouts, refused connections, generic network failures, HTTP failures, and MCP protocol incompatibility. The auto transport previously retried every non-auth failure through legacy SSE. When the first Streamable HTTP attempt failed below the protocol layer, the redundant SSE attempt failed for the same reason and replaced the useful primary context with the confusing final message 'Failed connecting via sse'. Retry SSE only when Streamable HTTP reached the endpoint and received evidence of protocol incompatibility. Preserve both attempt summaries when a legitimate fallback also fails, while keeping raw runtime causes internal and excluding upstream response details from customer-facing health messages. Keep connection-handshake numeric HTTP codes separate from JSON-RPC invocation error codes so protocol errors cannot be misclassified as HTTP status failures. Regression coverage verifies that TLS failures produce actionable CA-trust guidance without retrying SSE or leaking internal details, and that an HTTP 405 performs the fallback and reports both attempts. The MCP SDK suite passes 112 tests with 29 skipped; package and workspace typechecks pass.
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.
Motivation
Remote MCP catalogs can be refreshed concurrently by Desktop and API readers. Each request previously ran both upstream discovery and catalog persistence independently. Overlapping refreshes could therefore open competing SQLite transactions, leave the connection degraded, and preserve only a generic incomplete-catalog error even when a cached catalog remained usable.
Background services also start with a deliberately minimal environment. The installer preserved PATH and Executor settings but dropped standard custom-CA variables. On machines that extend HTTPS trust through those variables, an MCP endpoint worked from an interactive CLI yet failed from the supervised daemon.
Finally, MCP auto transport retried nearly every non-auth Streamable HTTP failure through legacy SSE. TLS, DNS, timeout, and refused-connection failures are below the MCP transport layer, so SSE could not recover. Its secondary failure replaced the useful primary context with a misleading final SSE error.
Observed symptoms and debugging impact
The failure surfaced through several different and misleading errors:
SQLITE_ERROR: cannot start a transaction within a transactionoccurred when concurrent catalog refreshes reached persistence together.Tool sync failing: plugin returned an incomplete tool catalogreplaced the specific MCP discovery failure, making the problem look like an invalid plugin response rather than a failed connection.Failed connecting via ssedescribed only the final fallback attempt. The primary Streamable HTTP request had actually failed TLS verification before MCP transport negotiation began.Together these symptoms pointed in several wrong directions: malformed catalogs, legacy SSE compatibility, endpoint downtime, or bad credentials. The cached catalog could also keep tools available while health remained degraded, further separating the visible warning from runtime behavior. Concurrent Desktop reads made the SQLite failure timing-dependent, while the TLS failure was specific to the supervised process environment.
Diagnosing the complete chain required comparing interactive and supervised execution, preserving the original discovery failure, stressing concurrent refreshes, and reproducing the service with custom CA variables deliberately removed. The fixes below address each root cause rather than masking the degraded state.
Changes
NODE_EXTRA_CA_CERTS,SSL_CERT_FILE, andSSL_CERT_DIRinto launchd, systemd, and Windows supervised environments.Error and security behavior
Customer-facing health messages contain stable, actionable classifications rather than raw SDK errors or upstream response bodies. Internal transport causes remain available for diagnostics but do not cross the connection-error boundary.
A certificate failure now reports:
Verification