Skip to content

fix: reap the backend session (and stdio child) when a web tab closes or reloads - #2853

Merged
cliffhall merged 2 commits into
v2/mainfrom
v2/fix/2824-remote-session-grace-reap
Oct 29, 2026
Merged

cliffhall merged 2 commits into
v2/mainfrom
v2/fix/2824-remote-session-grace-reap

Conversation

@cliffhall

Copy link
Copy Markdown
Member

Closes #2824

Problem

Closing or reloading a connected web tab dropped /api/mcp/events and sent the backend nothing else. clearEventConsumer() only deleted the session if its transport was already dead, so the RemoteSession and its spawned stdio child lived until the Inspector exited. RemoteSession.eventQueue also had no cap.

Fix

Backend (core/mcp/remote/node/)

  • A detach grace timer (RemoteSession.armDetachGrace, default DEFAULT_DETACH_GRACE_MS = 60s, overridable with the new RemoteServerOptions.sessionDetachGraceMs). It starts after /api/mcp/connect returns, which covers a browser that leaves before opening the stream, and again whenever the events stream drops. If a consumer attaches, the timer is cancelled. If it expires, the session is removed from the map, its pending request waits are rejected and its transport is closed, which ends the stdio child. The timer is unref'd.
  • Reattach is preserved. The only reattach the current design has is the OAuth full-page redirect (resumeAfterOAuth → attachToSession). Plain reloads start fresh. 60s rather than the issue's 30s gives an interactive OAuth login time to come back. A reattach that misses the window still recovers through the existing fresh-connect fallback.
  • A replaced events stream now detaches only its own consumer (detachEventConsumer). Before, a late abort from the old stream could clear the new live consumer, and with this change it would also have started reaping a session that is in use.
  • The event queue is capped at MAX_QUEUED_EVENTS = 1000. Once full, the oldest event is dropped, so the newest (including a terminal transport_error) are kept. The next consumer first gets a stdio_log saying how many events were dropped.

Web client

  • usePageHideRelease (called from App.tsx) sends a best-effort disconnect on pagehide. It skips a back/forward-cache entry (persisted) and an OAuth redirect whose resume snapshot carries this backend session id. The grace timer covers both of those cases.
  • The request is fetch({ keepalive: true }), not navigator.sendBeacon. A beacon cannot set request headers, and every /api/* route authenticates with the x-mcp-remote-auth header. Making a beacon work would mean accepting the API token in a URL or body for this one route, where logs and history keep it. A keepalive fetch survives the unload in the same way and keeps the header.
  • The release depends only on the backend session id, not on the transport's closed flag. In Chromium a reload cancels the events stream before pagehide fires. That marks the transport closed while the backend session is still alive. I found this during the hand smoke test and it is fixed in the second commit.

Tests

  • remote-session.test.ts uses fake timers to cover: the grace expiring, a reattach cancelling it, a re-arm replacing the pending timer, cancelDetachGrace, detachEventConsumer ownership, and the queue cap with its notice (including that a transport_error survives a full queue).
  • session-detach-reap.test.ts (new, integration) runs the real routes against a real stdio child that reports its pid, and asserts that the process is gone. Each wait is on an observable condition, not a sleep. It covers: reaping after the stream drops, reaping a session that never attached, reattaching inside the window, an explicit disconnect followed by a stream drop, and InspectorClient.releaseRemoteSessionOnPageHide.
  • remoteClientTransport.test.ts: the keepalive request and its auth header, a failed request being swallowed, a release still being sent after the stream ended by itself, and nothing being sent after a deliberate close().
  • usePageHideRelease.test.tsx: the decision rules and the listener lifecycle.

Hand smoke

I launched mcp-inspector --web node test-servers/build/test-server-stdio.js with CLIENT_PORT=17300, connected through Playwright, then closed or reloaded the page and counted the launcher's test-server-stdio children:

Action Children before → connected → +1s after
reload (×3) 0 → 1 → 0
close tab (×3) 0 → 1 → 0
close, with no pagehide request reaching the backend 0 → 1 → 1 at +20s → 0 at +65s (grace backstop)

The last row came from Playwright's browser.newPage(). Closing it tears down its whole context, so the keepalive request never left the browser. That made it a useful check of the grace backstop on its own.

npm run local:gate is green.

🤖 Generated with Claude Code

cliffhall and others added 2 commits October 28, 2026 23:47
Closing or reloading a connected web tab dropped /api/mcp/events and sent
nothing else, so the backend RemoteSession and its spawned stdio child lived
until the Inspector exited, queueing every later server message without
bound.

- Backend: arm a detach grace timer (default 60s, `sessionDetachGraceMs`)
  whenever a session has no events consumer — after connect and when its
  stream drops. A reattach (the OAuth redirect's attachToSession) cancels
  it; otherwise the transport is closed and the session dropped. A stream
  that was already replaced no longer clears the live consumer.
- Cap RemoteSession's event queue at 1000, dropping the oldest and telling
  the next consumer how many went.
- Web: on pagehide (not bfcache, not an OAuth redirect that will reattach to
  this session) send a keepalive fetch to /api/mcp/disconnect. Not
  sendBeacon: it cannot carry the x-mcp-remote-auth header every /api route
  requires.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
)

A reload cancels the page's events stream before pagehide fires, which marks
the transport closed while the backend session is still alive. Key the
release on the backend session id alone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall cliffhall added the v2 Issues and PRs for v2 label Oct 29, 2026
@cliffhall
cliffhall requested a balanced review from Copilot October 29, 2026 04:31

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.

🟢 Approval recommended

No active review issues remain for the session cleanup and reattachment changes.

No open issues 🎉

What changed in this PR

Addresses #2824 by releasing abandoned web backend sessions and their spawned stdio processes, while allowing OAuth redirects to reattach within a grace period.

Changes:

  • Adds a configurable detach grace timer, consumer-ownership checks, and a capped event queue with dropped-event notices.
  • Sends authenticated, best-effort keepalive disconnect requests on pagehide, with exceptions for OAuth redirects and back/forward-cache entries.
  • Adds unit and integration coverage for cleanup, reattachment, queue limits, and child-process termination.
File Description
core/​mcp/​remote/​remoteClientTransport.ts Adds authenticated keepalive session release.
core/​mcp/​remote/​node/​server.ts Wires session reaping into connect, stream detach, and disconnect routes.
core/​mcp/​remote/​node/​remote-session.ts Adds grace timers, consumer ownership, and bounded event buffering.
core/​mcp/​inspectorClient.ts Exposes pagehide release through the shared client.
clients/​web/​src/​test/​integration/​mcp/​remote/​session-detach-reap.test.ts Tests session cleanup and real stdio child termination.
clients/​web/​src/​test/​integration/​mcp/​remote/​remote-session.test.ts Tests timers, consumer ownership, and queue limits.
clients/​web/​src/​test/​core/​mcp/​remote/​remoteClientTransport.test.ts Tests keepalive authentication and release lifecycle behavior.
clients/​web/​src/​hooks/​usePageHideRelease.ts Handles pagehide release decisions and listener lifecycle.
clients/​web/​src/​hooks/​usePageHideRelease.test.tsx Tests release exceptions and listener cleanup.
clients/​web/​src/​App.tsx Registers pagehide release for the active client.

Review effort: Balanced

@cliffhall

Copy link
Copy Markdown
Member Author

Copilot review loop closed: round 1 came back clean (no inline comments, no suppressed findings, approval recommended), so per pr-flow 7c there is no further round.

@cliffhall
cliffhall merged commit 08c655b into v2/main Oct 29, 2026
11 checks passed
@cliffhall
cliffhall deleted the v2/fix/2824-remote-session-grace-reap branch October 29, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Issues and PRs for v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Web: closing or reloading a connected tab leaks the backend session and its spawned stdio server process

2 participants