Skip to content

fix(server): close stdio transport on stdin close - #2473

Open
harshmathurx wants to merge 5 commits into
modelcontextprotocol:mainfrom
harshmathurx:fix/stdio-server-stdin-close
Open

fix(server): close stdio transport on stdin close#2473
harshmathurx wants to merge 5 commits into
modelcontextprotocol:mainfrom
harshmathurx:fix/stdio-server-stdin-close

Conversation

@harshmathurx

@harshmathurx harshmathurx commented Jul 9, 2026

Copy link
Copy Markdown

Problem

StdioServerTransport did not treat stdin EOF as the server shutdown signal. When an MCP client closes/restarts and drops its end of the stdio pipe, a server with any other live handle can keep running indefinitely instead of closing the transport. That is the process-leak path reported in #2002.

Root cause

The server stdio transport listened for stdin data and error, but not for end or close. Node can therefore signal that stdin is gone without the transport ever invoking its close path. The transport also needed to remove the new lifecycle listeners when closed so repeated close paths do not leave stale listeners behind.

Focused change

  • Close StdioServerTransport when stdin emits close or end.
  • Remove stdin lifecycle listeners during transport close.
  • Add regression coverage for both stdin close paths.
  • Update the hanging integration fixture so an expected late logging rejection during teardown does not mask the transport/process cleanup behavior under Node 22.

Fixes #2002.

@jspahrsummers, tagging you for review because this is a narrow server stdio lifecycle fix for a P1/ready-for-work issue.

Verification

  • npx -y pnpm@10.26.1 --filter @modelcontextprotocol/server test -- packages/server/test/server/stdio.test.ts
  • npx -y pnpm@10.26.1 --filter @modelcontextprotocol/server lint
  • After the Node 22 teardown follow-up: corepack pnpm --dir test/integration test -- processCleanup.test.ts (integration suite passed locally: 18 files / 353 tests)
  • After refreshing from current main: corepack pnpm --filter @modelcontextprotocol/server exec vitest run test/server/stdio.test.ts
  • After refreshing from current main: corepack pnpm --dir test/integration exec vitest run test/processCleanup.test.ts test/server/cloudflareWorkers.test.ts
  • git diff --check

Current CI status

Most checks are green. The remaining failing check is test (20), and the failure does not appear to be in the stdio transport path touched here: the job log showed the targeted stdio/server tests and test/processCleanup.test.ts passing, with the only failure in test/server/cloudflareWorkers.test.ts > Cloudflare Workers compatibility (no nodejs_compat) > should handle MCP requests due to SdkHttpError: Error POSTing to endpoint: Error: Network connection lost. I attempted to rerun that failed upstream job, but GitHub would not allow rerunning it from my account.

Risk / reviewer notes

Low-risk lifecycle fix for stdio servers. It only adds shutdown handling for stdin EOF/close, which matches the stdio transport expectation that servers should exit promptly when standard input is closed. The close path continues to be idempotent and removes listeners during cleanup.

@harshmathurx
harshmathurx requested a review from a team as a code owner July 9, 2026 15:55
@changeset-bot

changeset-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5576d73

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

This PR includes changesets to release 6 packages
Name Type
@modelcontextprotocol/server Patch
@modelcontextprotocol/core Patch
@modelcontextprotocol/client Patch
@modelcontextprotocol/server-legacy Patch
@modelcontextprotocol/codemod Patch
@modelcontextprotocol/core-internal Patch

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 Jul 9, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2473

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2473

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2473

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2473

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2473

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2473

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2473

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2473

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2473

commit: 5576d73

@harshmathurx

Copy link
Copy Markdown
Author

Pushed a small test-fixture follow-up after the Node 22 CI failure.

The failing job crashed because serverThatHangs.ts can receive SIGTERM after the stdio transport has already closed, then its late sendLoggingMessage() rejection escapes the signal handler. The fixture now ignores that expected late logging failure so the test keeps exercising process cleanup instead of crashing during teardown.

Local verification:

  • corepack pnpm --dir test/integration test -- processCleanup.test.ts (ran the integration suite; 18 files / 353 tests passed)
  • git diff --check

@harshmathurx

Copy link
Copy Markdown
Author

@jspahrsummers gentle maintainer ping on this one. I pushed a follow-up for the Node 22 teardown failure and revalidated the targeted integration test locally. Most checks are green; the remaining failing test (20) looked unrelated to the transport fix path when I last checked. If you’d prefer I refresh/rebase the branch against current main before review, happy to do that. Otherwise would appreciate a look when you have a moment.

@harshmathurx

Copy link
Copy Markdown
Author

Updated the PR branch with current main via a normal merge commit so CI can rerun.\n\nThe previously failing Node 20 integration path passed locally before pushing:\n- corepack pnpm --filter @modelcontextprotocol/server exec vitest run test/server/stdio.test.ts\n- corepack pnpm --dir test/integration exec vitest run test/processCleanup.test.ts test/server/cloudflareWorkers.test.ts

@harshmathurx

Copy link
Copy Markdown
Author

I rechecked the current failing Node 20 CI after the branch refresh. The remaining failure still looks unrelated to the stdio transport change here.

Failing check: test (20)

Evidence from the job log:

  • The stdio/server tests touched by this PR passed in that job.
  • test/integration test: ✓ test/processCleanup.test.ts (3 tests) also passed.
  • The only failing test was test/server/cloudflareWorkers.test.ts > Cloudflare Workers compatibility (no nodejs_compat) > should handle MCP requests with SdkHttpError: Error POSTing to endpoint: Error: Network connection lost.
  • The same integration path passed locally before the refresh/push, as noted above.

I tried to rerun the failed job from my side, but GitHub refused the rerun for this upstream workflow. Could a maintainer please rerun the failed Node 20 job when convenient?

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.

Bug: StdioServerTransport doesn't handle stdin close/end — causes zombie process accumulation

1 participant