Repository navigation
fix(core): close a Location's MCP servers when it shuts down - #51730
kernel-oops wants to merge 1 commit into
Conversation
Location shutdown closed permissions, forms and RPC but not MCP. After an invalidation or reload, running steps can keep the old Location graph (and so its scope) alive long after shutdown, so its stdio MCP servers outlived it and accumulated. MCP now has a shutdown that stops every server and refuses later starts (such as event-driven reconnects), and Location shutdown calls it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RG8UhiogA1Ho2eZRx8tn2y
|
The following comment was made by an LLM, it may be inaccurate: |
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
Issue for this PR
Fixes #50758 for the reload case: old stdio MCP processes surviving a config or Location reload and overlapping with the new ones. Processes orphaned by a full service restart are a separate path and are not changed here.
Related to #51003, which this does not fix: global stdio MCP servers are still started once per Location.
Type of change
What does this PR do?
LocationLifecycleshutdown closes permissions, forms and RPC, but not MCP. The MCP connections only close when the Location's layer scope is finalised, and after an invalidation or reload that can happen much later: running steps keep the old Location graph borrowed until they reach a hand-off point, so the old graph's stdio MCP servers stay alive alongside the new graph's. On a busy server with frequent config or plugin reloads, those old processes pile up.This adds
Mcp.Interface.shutdown, which stops every server in the Location, marks it disabled and refuses any later start (a credential switch or other event could otherwise reconnect it), and calls it from Location shutdown right after RPC is closed. Nothing changes for a Location that is simply released: its scope still closes the connections as before; shutdown just no longer waits for that.In-flight MCP calls on the old graph fail once it is shut down. That matches how its permissions and forms already behave at shutdown.
How did you verify your code works?
test/mcp.test.ts(POSIX only, like the existing descendant-process test): connects a real stdio fixture server tagged with a unique argument, callsshutdown, and checks the process is gone, the server reportsdisabledwith no tools, and a laterconnectdoes not respawn it. The respawn check fails without the new start guard.bun test test/mcp.test.ts test/mcp-instructions.test.ts test/mcp-oauth.test.ts test/location-activity.test.ts test/session-remove.test.ts(107 pass) andbun run typecheckinpackages/core.location-lifecycle.tsis one call; I did not add a full Location-graph test for it. Happy to add one if you point me at the preferred fixture.Screenshots / recordings
Not a UI change.
Checklist
🤖 Generated with Claude Code
https://claude.ai/code/session_01RG8UhiogA1Ho2eZRx8tn2y