Skip to content

feat(server): closeOnRefusedHandshake option for per-handshake transports#2439

Closed
felixweinberger wants to merge 2 commits into
mainfrom
fweinberger/refused-handshake-and-classifier
Closed

feat(server): closeOnRefusedHandshake option for per-handshake transports#2439
felixweinberger wants to merge 2 commits into
mainfrom
fweinberger/refused-handshake-and-classifier

feat(server): export classifyEntryRequest for hand-wired hybrid routing

f7f843f
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 6, 2026 in 33m 18s

Code review found 2 important issues

Found 3 candidates, confirmed 5. See review comments for details.

Details

Severity Count
🔴 Important 2
🟡 Nit 3
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important packages/server/src/server/streamableHttp.ts:109-130 closeOnRefusedHandshake adds public API + concurrency machinery for a one-line userland check
🔴 Important packages/server/src/server/createMcpHandler.ts:417-441 Documented classifyEntryRequest hybrid pattern strands the session it opens

Annotations

Check failure on line 130 in packages/server/src/server/streamableHttp.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

closeOnRefusedHandshake adds public API + concurrency machinery for a one-line userland check

The new `closeOnRefusedHandshake` option (plus the `_activeRequests` counter and the try/finally + `queueMicrotask` deferral machinery in `handleRequest`) solves a problem that the only wiring it's documented for — the per-handshake session-map recipe, where the consumer owns the single `handleRequest` callsite — can already solve with a two-line userland check: `await transport.handleRequest(req, res, req.body); if (!transport.sessionId) await transport.close();` (in a `finally` to also cover t

Check failure on line 441 in packages/server/src/server/createMcpHandler.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Documented classifyEntryRequest hybrid pattern strands the session it opens

The 'canonical pattern' documented for the new `classifyEntryRequest` export (this JSDoc `@example`, the changeset, the new sentence in `docs/serving/legacy-clients.md`, and the new hybrid-routing test) routes only `outcome.reason === 'initialize'` to the sessionful host — but the classifier never inspects `Mcp-Session-Id`, so every follow-up from the session that host just opened (claim-less `tools/call` POSTs, `notifications/initialized`, the standalone GET SSE stream, DELETE termination, post