Skip to content

[v1.x] fix: allow registering after connect() when the capability was declared - #2958

Merged
felixweinberger merged 4 commits into
v1.xfrom
fix/v1-register-after-connect
Oct 5, 2026
Merged

felixweinberger merged 4 commits into
v1.xfrom
fix/v1-register-after-connect

Conversation

@claude

@claude claude Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Requested by Felix Weinberger · Slack thread

Before: A server that declares tools, resources or prompts in the McpServer options and registers its first item after connect() gets Cannot register capabilities after connecting to transport, because the handlers (and with them the capability) were only installed on the first registration. This is the problem reported in #893.

After: A capability declared in the constructor options gets its handlers in the constructor, as on main (#2269), so registering after connect() works. Two visible side effects, both matching v2: a declared capability with nothing registered answers its list request with an empty list instead of Method not found (the server-side behaviour #181 asks for), and a capability declared as {} is advertised with listChanged: true. Registering after connect() without having declared the capability still throws.

How it was verified: Three new tests in test/server/mcp.test.ts. On v1.x as it is, the first two fail with exactly Cannot register capabilities after connecting to transport and MCP error -32601: Method not found, and the third passes; with the change all three pass. Unit suite 1858 of 1858, tsc --noEmit 0 errors, Prettier clean, ESLint clean on src/server/mcp.ts.

🤖 Generated with Claude Code

https://claude.ai/code/session_012VRbFCp41otcScXE1YY3es


Generated by Claude Code

McpServer installed the tools, resources and prompts handlers on the first registration, and installing them registers the capability, which throws once a transport is connected. So a server that declared a capability in its options and registered its first tool after connect() got 'Cannot register capabilities after connecting to transport'.

A capability declared in the constructor options now gets its handlers in the constructor, as on main (#2269). Registering after connect() then works, and a declared capability with nothing registered answers its list request with an empty list instead of 'Method not found'. A capability declared as {} is advertised with listChanged: true. Registering after connect() without having declared the capability still throws.
@claude
claude Bot requested a review from a team as a code owner October 5, 2026 12:50
@changeset-bot

changeset-bot Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 20cb3c5

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@modelcontextprotocol/sdk@2958

commit: 20cb3c5

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012VRbFCp41otcScXE1YY3es

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

  • 🟡 src/server/mcp.ts — Servers that declare prompts (or resources) but not completions get a ghost prompt or template after a failed late registration: registerPrompt throws, yet clients see the item in prompts/list and get Method not found on completion/complete. src/server/mcp.ts:923 stores the prompt before src/server/mcp.ts:932 calls setCompletionRequestHandler, which throws after connect() via registerCapabilities. The handlers installed by src/server/mcp.ts:143-145 now serve that leftover entry; on the base branch nothing was installed, so the entry stayed invisible. Fix: store the prompt/template only after the completion handler is installed, or fail before mutating _registeredPrompts/_registeredResourceTemplates, so a thrown registration leaves no served item.

    Why this was flagged

    A server built with capabilities: { prompts: {} } (no completions) calls registerPrompt after connect() with an argsSchema containing a completable field, which is the late-registration case this PR enables. _createRegisteredPrompt writes this._registeredPrompts[name] at src/server/mcp.ts:923, then src/server/mcp.ts:932 calls setCompletionRequestHandler, whose registerCapabilities at src/server/index.ts:201-202 throws Cannot register capabilities after connecting to transport. The caller sees a failure, but the constructor already installed the prompts/list and prompts/get handlers at src/server/mcp.ts:143-145, so clients now list and can get that prompt while completion/complete answers Method not found; a retry with the same name throws Prompt ... is already registered at src/server/mcp.ts:1216-1218. On the base branch, with no prompt registered before connect, the prompt handlers were never installed, so the leftover entry was never served.

    Verification: src/server/mcp.ts:923 this._registeredPrompts[name] = registeredPrompt; runs before line 932 this.setCompletionRequestHandler(), which at line 477 calls this.server.registerCapabilities({ completions: {} }); src/server/index.ts:201-202 throws Cannot register capabilities after connecting to transport once a transport is attached. Nothing removes the stored entry on that throw.

  • 🟡 src/server/mcp.ts — Clients of servers that declare a capability and register their first item right after connect() now receive notifications/tools/list_changed before the initialize handshake has finished. src/server/mcp.ts:987-988 installs nothing new (handlers already exist from src/server/mcp.ts:137-145) and immediately calls sendToolListChanged, which src/server/mcp.ts:1266-1268 sends as soon as a transport is attached, with no check that the client has sent notifications/initialized. On the base branch this population got a throw instead of a pre-initialization notification. Fix: defer list_changed notifications until initialization completes (queue them and flush on oninitialized), for tools, resources and prompts alike.

    Why this was flagged

    Trigger: new McpServer(info, { capabilities: { tools: {} } }), await connect(stdioTransport), then registerTool in the same tick or before the client's initialize round-trip completes, the exact pattern this PR advertises as now working. src/server/mcp.ts:985-988 stores the tool and calls sendToolListChanged; src/server/mcp.ts:1267 only checks this.server.transport !== undefined, and src/server/index.ts:643-644 plus src/shared/protocol.ts:1336-1443 write the notification straight to the transport. Nothing in Server tracks whether notifications/initialized has arrived, so a stdio client reads notifications/tools/list_changed on stdout before the initialize result. On the base branch this population never reached src/server/mcp.ts:988: the first registration threw at setToolRequestHandlers, so no pre-initialization notification was emitted. Strict clients that reject messages before initialization fail the session; permissive ones re-list tools before they can. Remedy: queue list_changed notifications until oninitialized fires and flush them then.

    Verification: Trigger: registerTool right after await connect(stdioTransport), before the client's initialize/initialized round-trip completes (on base the same call threw at registerCapabilities). src/server/mcp.ts:987-988 runs this.sendToolListChanged(); src/server/mcp.ts:1266-1269 gates only on isConnected(); src/shared/protocol.ts:1336-1341 never checks whether notifications/initialized has arrived.

  • 🟣 src/server/mcp.ts — Operators of servers that declare a capability and register items after connect() can now get an unhandled promise rejection, which terminates a Node process, when the transport's send rejects. src/server/mcp.ts:1268 calls this.server.sendToolListChanged() without await or catch; src/shared/protocol.ts:1443 awaits this._transport.send on the non-debounced path, so a rejected send (an eventStore.storeEvent failure at src/server/webStandardStreamableHttp.ts:1143, or a stdout error) surfaces as an unhandled rejection. On the base branch the declared-capability late registration threw synchronously before reaching this line. Fix: attach a catch that routes the error to server.onerror at all three send*ListChanged sites (src/server/mcp.ts:1259, 1268, 1277).

    Why this was flagged

    Trigger: a server declared with capabilities: { tools: {} } connected over Streamable HTTP with an eventStore, registering a tool after connect() (the path this PR enables at src/server/mcp.ts:137-145). src/server/mcp.ts:987-988 runs sendToolListChanged; src/server/mcp.ts:1267 sees a transport and calls src/server/index.ts:643-644, which returns the promise from Protocol.notification. With no debouncedNotificationMethods configured, src/shared/protocol.ts:1443 does await this._transport.send(...); in src/server/webStandardStreamableHttp.ts:1141-1143 storeEvent is awaited before the no-stream early return at line 1146-1149, so a failing event store rejects the send. The promise is dropped at src/server/mcp.ts:1268, so Node raises an unhandled rejection and, by default, exits. On the base branch this population threw synchronously at setToolRequestHandlers and never reached the floating promise. Remedy: .catch(error => this.server.onerror?.(error)) at src/server/mcp.ts:1259, 1268 and 1277.

    Verification: pre-existing. The defective code is untouched by this PR and the base already fails the same way by the same route. src/server/mcp.ts:1268 this.server.sendToolListChanged(); (no await, no .catch) -> src/shared/protocol.ts:1443 on the non-debounced path. A rejected send therefore becomes an unhandled promise rejection, which by default terminates a Node process.

Comment thread src/server/mcp.ts
Comment thread .changeset/register-after-connect.md Outdated
Comment thread test/server/mcp.test.ts
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012VRbFCp41otcScXE1YY3es

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — the changeset rewording in the latest commit now matches what the constructor does (any declared tools/resources/prompts capability is advertised with listChanged: true), and the code is unchanged from the version already reviewed. The two remaining inline notes (declared completions left on the lazy path, e2e coverage) are optional and do not block this fix.

Extended reasoning...

The diff adds nine lines to the McpServer constructor in src/server/mcp.ts that call the existing idempotent setToolRequestHandlers/setResourceRequestHandlers/setPromptRequestHandlers helpers when the matching capability is declared, plus three end-to-end InMemoryTransport tests and a patch changeset. It touches no auth, crypto, or input-parsing surface; the behavioural change (empty lists and listChanged: true for declared-but-empty capabilities) is documented in the changeset and mirrors main. The only change since the prior review is a one-line changeset wording fix that addresses the nit I raised about it, and the remaining open inline notes were marked optional. A human reviewer has already approved, the change is small and self-contained on the v1.x maintenance line, and the hunt ran dry, so approval is warranted.

Still open from earlier reviews (2):

  • Unresolved: 2 minor or pre-existing.

@felixweinberger
felixweinberger merged commit fa56d81 into v1.x Oct 5, 2026
12 checks passed
@felixweinberger
felixweinberger deleted the fix/v1-register-after-connect branch October 5, 2026 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants