Repository navigation
Conversation
A stdio MCP server configured as `npx -y <server>` runs as a child of the launcher the stdio transport spawns, and `StdioClientTransport.close()` only signals its own direct child. Every teardown path except the instance finalizer therefore left the real server, and anything it spawned, running. Move the descendant sweep the finalizer already did into one helper and use it wherever a client is released: disconnect, the replaced client on reconnect, and the failed connect attempt.
|
The following comment was made by an LLM, it may be inaccurate: Found a potentially related PR: #46312 - This PR appears to be addressing the same issue: terminating MCP server process trees. Since PR #46312 already exists, it's worth checking whether it was closed/merged and if this PR (#50509) is re-opening the issue or providing a different solution. |
A stdio MCP server configured as `npx -y <server>` runs as a child of the launcher the stdio transport spawns, and `StdioClientTransport.close()` only signals its own direct child. Every teardown path except the instance finalizer therefore left the real server, and anything it spawned, running. Move the descendant sweep the finalizer already did into one helper and use it wherever a client is released: disconnect, the replaced client on reconnect, and the failed connect attempt. (cherry picked from anomalyco#50509)
|
Heads-up: Sorry for leaving it in the queue this long. If the underlying bug still reproduces on |
|
This branch has been upgraded to V2. We're no longer taking PRs for V1. If you think this is still relevant, please port it over and open it for V2. — from 𝕺𝖕𝖊𝖓𝕮𝖔𝖉𝖊 |
Issue for this PR
Closes #50363
Type of change
What does this PR do?
A local MCP server configured as
npx -y <server>runs as a child of the launcher the stdio transport spawns, andStdioClientTransport.close()signals only its own direct child.mcp/index.tsalready swept that tree in the instance finalizer, butdisconnect(), a reconnect that replaces a client, and a failed connect only calledclose(), so the real server and anything below it kept running.This moves the sweep into one
terminateTree()helper and uses it on every release. The sweep has to run beforeclose():pgrep -Pcan only find the children while the launcher is still alive, because once it exits they are re-parented.Not covered, and not fixable in-process: a server whose parent is killed outright (SIGKILL, crash) is re-parented to PID 1, so nothing here can signal it afterwards.
How did you verify your code works?
Two tests in
packages/opencode/test/mcp/lifecycle.test.ts, using the existing pid-file fixture extended so the fixture spawns a child process (the "real server") and publishes its pid:local stdio disconnect terminates the whole process treelocal stdio timeout terminates the whole process treeBoth fail with the fix reverted — the leaked child process was still running after the test finished — and pass with it.
The full package suite reports 3583 pass / 4 fail on this machine; all 4 are unrelated to this change (three
test/server/httpapi-*tests time out on a clean checkout here too, andsession.compaction.process > stops quickly when aborted during retry backoffpasses in isolation).Screenshots / recordings
Not a UI change.
Checklist