Skip to content

fix: clean up sessions on close and handle DELETE /mcp - #73

Merged
nickytonline merged 1 commit into
mainfrom
fix/session-transport-cleanup
Jun 30, 2026
Merged

fix: clean up sessions on close and handle DELETE /mcp#73
nickytonline merged 1 commit into
mainfrom
fix/session-transport-cleanup

Conversation

@nickytonline

@nickytonline nickytonline commented Jun 30, 2026

Copy link
Copy Markdown
Owner

Sessions were stored in the transports map on initialization but never
removed, causing a memory leak in long-running servers as every client
connection (including those that later disconnect or crash) kept its
StreamableHTTPServerTransport pinned in memory forever.

Wire transport.onclose inside onsessioninitialized so the session is
removed from the map when the transport closes. The sessionId is
captured by closure, avoiding any race between ID assignment and
cleanup registration.

Also add app.delete('/mcp', mcpHandler) to implement the DELETE method
defined by the Streamable HTTP transport spec for explicit session
termination. The SDK's handleRequest handles DELETE semantics and fires
onclose, which removes the session via the fix above. A guard returns
400 for DELETE requests missing a session ID.

Generated with Devin

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

Summary by CodeRabbit

  • New Features
    • Added support for ending MCP sessions with HTTP DELETE requests.
    • Requests without a session ID now return a clear 400 error.
  • Bug Fixes
    • Improved session cleanup when connections close, helping prevent stale sessions from lingering.

Sessions were stored in the transports map on initialization but never
removed, causing a memory leak in long-running servers as every client
connection (including those that later disconnect or crash) kept its
StreamableHTTPServerTransport pinned in memory forever.

Wire transport.onclose inside onsessioninitialized so the session is
removed from the map when the transport closes. The sessionId is
captured by closure, avoiding any race between ID assignment and
cleanup registration.

Also add app.delete('/mcp', mcpHandler) to implement the DELETE method
defined by the Streamable HTTP transport spec for explicit session
termination. The SDK's handleRequest handles DELETE semantics and fires
onclose, which removes the session via the fix above. A guard returns
400 for DELETE requests missing a session ID.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 30, 2026 19:51
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 95c91550-7669-4d38-809d-1857d8386ea2

📥 Commits

Reviewing files that changed from the base of the PR and between 513fcf1 and c6b26fc.

📒 Files selected for processing (1)
  • src/index.ts

📝 Walkthrough

Walkthrough

This change adds support for terminating MCP sessions via HTTP DELETE requests in the Express server. A transport.onclose handler removes sessions from the transports map on closure. DELETE requests missing an mcp-session-id header now return HTTP 400. A new DELETE /mcp route is registered.

Changes

DELETE Session Termination

Layer / File(s) Summary
Transport cleanup on close
src/index.ts
Adds a transport.onclose callback that removes the session's entry from the transports map and logs the closure.
DELETE request validation and routing
src/index.ts
Adds 400 error handling for DELETE requests missing mcp-session-id and registers a new DELETE /mcp route using mcpHandler.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ExpressApp
  participant mcpHandler
  participant TransportsMap
  Client->>ExpressApp: DELETE /mcp
  ExpressApp->>mcpHandler: route request
  mcpHandler->>mcpHandler: check mcp-session-id header
  alt missing session id
    mcpHandler->>Client: 400 error response
  else session id present
    mcpHandler->>TransportsMap: lookup transport
    TransportsMap->>mcpHandler: transport.onclose triggers
    mcpHandler->>TransportsMap: delete session entry
  end
Loading

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

A knock at the door, a session must go,
I tidy the map with a soft little "no" —
DELETE comes a-hopping, with id in tow,
Or else it gets bounced with a 400 throw.
🐰✂️ snip goes the transport, clean as can be!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: session cleanup on close and DELETE /mcp support.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/session-transport-cleanup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes a long-running memory leak in the MCP HTTP transport session registry by ensuring session transports are removed when a session closes, and adds support for explicit session termination via DELETE /mcp as defined by the Streamable HTTP transport spec.

Changes:

  • Register a transport.onclose handler during session initialization to remove the session from the in-memory transports map.
  • Add validation for malformed DELETE /mcp requests missing an mcp-session-id header.
  • Wire up app.delete("/mcp", mcpHandler) so the Express endpoint accepts DELETE requests.

Comment thread src/index.ts
Comment on lines +85 to +88
// DELETE without a session ID is malformed
if (req.method === "DELETE" && !sessionId) {
logger.warn("DELETE request without session ID");
res.status(400).json({ error: "Session ID required to terminate a session" });
@greptile-apps

greptile-apps Bot commented Jun 30, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a memory leak where StreamableHTTPServerTransport instances were added to transports on session initialization but never removed, and adds DELETE /mcp support per the Streamable HTTP transport spec.

  • Wires transport.onclose inside onsessioninitialized to delete the session from transports when the transport closes, fixing the unbounded growth of the map.
  • Adds app.delete(\"/mcp\", mcpHandler) and a guard that returns 400 for DELETE requests missing a session ID (previously these would fall through all checks and hang the connection).

Confidence Score: 3/5

The core memory-leak fix is directionally correct but the onclose assignment overwrites the SDK handler set by server.connect, which needs to be chained before merging.

Overwriting transport.onclose means the McpServer instance never clears its _transport reference and never fires its own onclose callbacks, leaving a potential reference cycle between the server and transport objects that survives the session cleanup. The DELETE guard and route registration are both correct and close a real response-hang bug.

src/index.ts — specifically the transport.onclose assignment inside onsessioninitialized and its interaction with server.connect.

Important Files Changed

Filename Overview
src/index.ts Adds session cleanup via transport.onclose and DELETE /mcp route; the onclose assignment overwrites the SDK's internal close handler set by server.connect, breaking McpServer lifecycle tracking and potentially leaving a reference cycle.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant C as MCP Client
    participant E as Express
    participant H as mcpHandler
    participant T as StreamableHTTPServerTransport
    participant S as McpServer

    Note over C,S: Session Initialization (POST /mcp, no session ID)
    C->>E: POST /mcp (init request)
    E->>H: mcpHandler(req, res)
    H->>T: new StreamableHTTPServerTransport
    H->>S: server.connect(transport) sets transport.onclose internally
    H->>T: transport.handleRequest fires onsessioninitialized
    H->>H: "transports[sessionId] = transport"
    H->>T: transport.onclose overwritten SDK handler discarded
    T-->>C: 200 + Mcp-Session-Id header

    Note over C,S: Session Termination (DELETE /mcp)
    C->>E: DELETE /mcp (Mcp-Session-Id: xyz)
    E->>H: mcpHandler(req, res)
    alt no session ID
        H-->>C: 400 Session ID required
    else unknown session
        H-->>C: 404 Session not found
    else valid session
        H->>T: transports[sessionId].handleRequest
        T->>H: onclose fires, delete transports[sessionId]
        T-->>C: 200
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant C as MCP Client
    participant E as Express
    participant H as mcpHandler
    participant T as StreamableHTTPServerTransport
    participant S as McpServer

    Note over C,S: Session Initialization (POST /mcp, no session ID)
    C->>E: POST /mcp (init request)
    E->>H: mcpHandler(req, res)
    H->>T: new StreamableHTTPServerTransport
    H->>S: server.connect(transport) sets transport.onclose internally
    H->>T: transport.handleRequest fires onsessioninitialized
    H->>H: "transports[sessionId] = transport"
    H->>T: transport.onclose overwritten SDK handler discarded
    T-->>C: 200 + Mcp-Session-Id header

    Note over C,S: Session Termination (DELETE /mcp)
    C->>E: DELETE /mcp (Mcp-Session-Id: xyz)
    E->>H: mcpHandler(req, res)
    alt no session ID
        H-->>C: 400 Session ID required
    else unknown session
        H-->>C: 404 Session not found
    else valid session
        H->>T: transports[sessionId].handleRequest
        T->>H: onclose fires, delete transports[sessionId]
        T-->>C: 200
    end
Loading

Reviews (1): Last reviewed commit: "fix: clean up sessions on close and hand..." | Re-trigger Greptile

Comment thread src/index.ts
Comment on lines +53 to +56
transport.onclose = () => {
delete transports[sessionId];
logger.info("MCP session closed", { sessionId });
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 onclose overwrites the SDK's internal close handler

server.connect(transport) (line 62) is called before transport.handleRequest, and the SDK's Protocol.connect wires transport.onclose to clear its own _transport reference and fire any server.onclose callbacks. Because onsessioninitialized fires inside handleRequest — after connect has already set the handler — this assignment silently discards the SDK's hook. As a result, the McpServer instance never learns the transport closed: server._transport stays set, server.onclose never fires, and the server+transport pair can form a reference cycle that keeps both objects alive after the session is removed from transports. Capture the previous handler and chain it to preserve SDK lifecycle semantics.

Suggested change
transport.onclose = () => {
delete transports[sessionId];
logger.info("MCP session closed", { sessionId });
};
const previousOnClose = transport.onclose;
transport.onclose = () => {
delete transports[sessionId];
logger.info("MCP session closed", { sessionId });
previousOnClose?.();
};

Comment thread src/index.ts
sessionIdGenerator: () => randomUUID(),
onsessioninitialized: (sessionId) => {
transports[sessionId] = transport;
transport.onclose = () => {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

The 2026-07-28 spec release candidate (SEP-2567 + SEP-2575) removes the Mcp-Session-Id header and the initialize handshake entirely, but for now we should still fix this in memory session lookup.

Comment thread src/index.ts
}

// DELETE without a session ID is malformed
if (req.method === "DELETE" && !sessionId) {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Adds the missing /delete endpoint.

@nickytonline
nickytonline merged commit 8d8c917 into main Jun 30, 2026
12 checks passed
@nickytonline
nickytonline deleted the fix/session-transport-cleanup branch June 30, 2026 19:55
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 1.0.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants