Repository navigation
perf(sdk): load the SSE transport with the first SSE session - #937
Merged
alexmercerpo merged 1 commit intoOct 21, 2026
Merged
alexmercerpo merged 1 commit into
alexmercerpo merged 1 commit into
Conversation
A stdio server initializes the scope's TransportService, and with it TransportSSEAdapter, which defined RecreateableSSEServerTransport at startup. Its base class reads POST bodies with raw-body and content-type, so a stdio server loaded raw-body, content-type, iconv-lite, http-errors and their dependencies (16 files) that it never uses. TransportSSEAdapter now imports the transport as a type and loads it with loadSseTransport(), which TransportService awaits where it creates the LocalTransporter of an SSE session, as it does for the Streamable HTTP transport since #924.
Contributor
Author
|
@coderabbitai full review |
Contributor
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing: 4
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 33.33% which is insufficient. The required threshold is 65.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: lazy SSE transport loading on the first SSE session. Description check The description is complete and relevant. It explains the motivation, implementation, performance impact, and extensive validation. The optional user-facing change section is omitted appropriately bec… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description is complete and relevant. It explains the motivation, implementation, performance impact, and extensive validation. The optional user-facing change section is omitted appropriately because this is an internal performance change.
-
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Contributor
Performance Test ResultsStatus: ✅ All tests passed Summary
Total: 101 tests across 21 projects 📊 View full report in workflow run Generated at: 2026-10-21T16:00:05.269Z |
alexmercerpo
approved these changes
Oct 21, 2026
Contributor
|
Contributor
Cherry-pick CreatedA cherry-pick PR to Please review and merge if this change should also be in If the cherry-pick is not needed, close the PR. |
This was referenced Oct 21, 2026
alexmercerpo
pushed a commit
that referenced
this pull request
Oct 21, 2026
* perf(sdk): serve in-memory, direct-client and web-standard MCP with the loaded protocol module (#934) createInMemoryServer(), the direct client's notification and elicitation handlers, and runWebStandardMcp() took their classes and schemas from `await import('@frontmcp/protocol')`. In a CommonJS process that import resolves the package's `import` condition, so connect(), create() clients and createFetchHandler() loaded a second, ESM copy of @frontmcp/protocol and, through it, of the MCP SDK, on top of the CommonJS copy the SDK had already required. They now use the statically imported @frontmcp/protocol, as runStdio() does since #923. In an ESM process the import resolves the same ESM module as before. (cherry picked from commit 5b54191) * perf(sdk): load js-yaml, semver and the uipack renderer on first use under require() (#935) A server loaded js-yaml (25 files), semver (23) and the @frontmcp/uipack root at startup, for code most servers never run: SKILL.md and YAML config parsing, the ESM loader's version ranges, and the auth-UI pages. They now come through three subpath imports, #js-yaml, #semver and #uipack-render, whose `require` variant reads each function from its package the first time it is used, like @frontmcp/protocol's #client. The CommonJS build keeps them external and ships both variants; the ESM and browser builds resolve them at build time to the static variant, so those bundles import the packages as before. (cherry picked from commit b7ee831) * perf(sdk): load the SSE transport with the first SSE session (#937) A stdio server initializes the scope's TransportService, and with it TransportSSEAdapter, which defined RecreateableSSEServerTransport at startup. Its base class reads POST bodies with raw-body and content-type, so a stdio server loaded raw-body, content-type, iconv-lite, http-errors and their dependencies (16 files) that it never uses. TransportSSEAdapter now imports the transport as a type and loads it with loadSseTransport(), which TransportService awaits where it creates the LocalTransporter of an SSE session, as it does for the Streamable HTTP transport since #924. (cherry picked from commit f13b8af)
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
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.
Stack 3 of 4, based on #935 (
perf/1.10.x-lazy-optional-deps). Merge in order.What and why
A stdio server serves no HTTP, but its scope still creates the
TransportServiceand with it the transport adapters.TransportSSEAdapterimportedRecreateableSSEServerTransport, whose base class reads POST bodies withraw-bodyandcontent-type, so every stdio server loadedraw-body,content-type,iconv-lite,http-errors,statuses,depd,bytesand five more small packages: 16 files it never uses.TransportSSEAdapternow imports the transport as a type and loads the class withloadSseTransport().TransportServiceawaits it where it creates theLocalTransporterof an SSE session, which it already did asynchronously. That is what #924 did for the Streamable HTTP transport, so a stdio server now loads neither transport, and an HTTP server loads each with its first session of that type. The adapter throws anInternalMcpErrorif it creates a transport before the class is loaded, as the Streamable HTTP adapter does.The HTTP server loads the same modules as before: Express's
body-parserrequiresraw-bodyandcontent-typeanyway. The stdio server loads 264 modules instead of 280.Results
Apple M4 Pro (14 cores, 48 GiB), macOS 15.6.1, Node.js 24.18.1, on AC power, with the machine's normal background load (an IDE and other jobs). The server is the benchmark's FrontMCP implementation (
frontmcp build --target node), run withnodeand no flags, againstnpm packed builds of this branch and its base, installed into scratch copies of the implementation and built withnpm run build, measured in one session in randomized, interleaved rounds with the benchmark's own harness functions. Cold start is the time from spawningnodeto the firsttools/callresponse,initializeincluded: one discarded start per server, then 8 rounds of 4 starts per transport in random order. The 100-tool startup is spawn to the first fulltools/list, 5 rounds of 2 starts, and its RSS is read 1 s after that. Throughput is the benchmark's closed-looptools/callat concurrency 1, 16 and 64 (3 s warmup, 10 s measured, 5 rounds); RSS after startup is read 1 s after the first call, RSS after load after the last level, median of the 5 rounds. Heap used isprocess.memoryUsage().heapUsed700 ms after start (after the first call, no forced GC), median of 10 interleaved starts per transport in a separate run right after. MB is 10^6 bytes. The 1-minute load average was 3.4 to 7.5 during the session (median 4.1), and 3.0 to 5.6 during the heap runs.perf/1.10.x-lazy-optional-depstools/list, p50The stdio cold start was lower in every one of the 8 rounds, by a median of 1.6 ms (0.5 to 14.3 ms). A separate cold-start run of 16 rounds of 4 starts per transport, at a load average of 6.5 to 7.8, gave stdio 176.9 → 173.4 ms at p50 and 184.4 → 178.8 ms at p90, lower in 14 of 16 rounds by a median of 4.2 ms. The HTTP rows, the 100-tool server and throughput don't run this code and are within their run-to-run spread (c=64: 8,683 to 9,005 rps for the base, 8,645 to 9,063 for this PR).
Tests
transport.registry.spec.ts: a new test checks thatTransportServiceloads the SSE transport for SSE sessions only, next to the existing one for the Streamable HTTP transport.transport.sse.adapter.load.spec.ts: creating a transport beforeloadSseTransport()throwsInternalMcpErrorand constructs nothing; after it,createTransportandcreateTransportFromSessionconstruct the transport with the same arguments as before.nx run sdk:test --coverage: 7,142 tests in 559 suites pass.nx run sdk:lint,nx run sdk:typecheckand Prettier on the changed files pass.module.registerHookspreload: the benchmark's stdio server loads 264 modules instead of 280, withoutraw-body,content-type,iconv-lite,http-errorsand their dependencies; its HTTP server loads the same 404 as before.Summary by CodeRabbit