Repository navigation
fix(mcp): release MCP servers for abandoned directories - #52946
Closed
abhisheksharma611 wants to merge 1 commit into
Closed
abhisheksharma611 wants to merge 1 commit into
abhisheksharma611 wants to merge 1 commit into
Conversation
MCP state is per directory, and building it spawns the configured MCP servers as child processes. Every other per-directory resource lives in `locationServices`, which LayerMap releases after 60 minutes idle, but MCP state is built through `InstanceState`, whose ScopedCache was created with `capacity: Number.POSITIVE_INFINITY` and no expiry, and `InstanceState.invalidate` has no caller. Nothing released those scopes short of an explicit dispose endpoint or process exit. A server session that visits N directories therefore kept N sets of MCP server processes alive for the lifetime of the process. On Windows 11 a single session reached 13 concurrent sets - 65 node, 26 python and 67 cmd processes - and was still growing after four hours. Give `InstanceState.make` an optional capacity (default unchanged) and cap the MCP state, so least-recently-used directories release their scope, which runs the disposal that already terminates their servers.
github-actions
Bot
added
the
needs:compliance
Contributor
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 72 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
Contributor
|
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. |
Contributor
|
The following comment was made by an LLM, it may be inaccurate: |
Open
1 task done
1 task done
1 task done
Author
|
Closing in the comment above. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A long-running
opencode serveaccumulates one complete set of MCP server processes per directory it has ever served, and never gives them back.Measured on Windows 11, OpenCode 2.0.21, one session with 7 configured projects:
node.execmd.exeEvery directory touched — each project, and each chat/session directory — gets its own fleet, because MCP state is built per directory and each build spawns the configured servers.
Root cause
Per-directory state is released in two different places, and MCP is only in one of them:
packages/core/src/location-services.ts:109buildsLocationServiceMapwith{ idleTimeToLive: "60 minutes" }, so watchers, filesystem and the rest of a location's services are released once it goes idle.packages/opencode/src/mcp/index.ts:492throughInstanceState.make, whose cache was created withcapacity: Number.POSITIVE_INFINITYand no expiry (packages/opencode/src/effect/instance-state.ts:31).InstanceState.invalidateexists but has no caller.The only release paths were an explicit
POST /instance/dispose,POST /global/dispose, or process exit. In normal server operation none of those fire, so every scope — and every MCP server process under it — stayed alive for the lifetime of the process.Fix
Give
InstanceState.makean optionalcapacity(default unchanged, so no other caller changes behaviour) and cap the MCP state at 8 directories. Least-recently-used directories then release their scope, which runs the disposal that already terminates their servers.Verification
test/effect/instance-state.test.tsgains a case covering LRU release at capacity. Confirmed red before the fix (0 pass, 1 fail) and green after.bun test test/server/ test/project/ test/mcp/ test/effect/— 512 pass, 25 skip, 0 fail (537 tests, 74 files).tsc --noEmit— no new errors. The 11 remaining errors are pre-existing inpackages/tui/src/component/dialog-move-session.tsx; verified identical on a stashed tree.oxlintcould not be run:tsgolint.exeis blocked by a Windows Application Control policy on this machine (os error 4551). Worth a lint pass in CI.Notes for review
8is a policy choice, not a derived number. It is deliberately generous — more than the number of directories a normal session keeps warm — and exists only to stop unbounded growth. Happy to make it configurable, or to derive it from a limit the project prefers, if that fits the architecture better.descendants()inmcp/index.ts:420, which returns[]on win32 and so skips the process-tree walk during disposal. I measured that this does not leak in practice on Windows: a Bun-spawned orchild_process.spawn-spawned grandchild dies with its parent because the runtime holds the tree in a job object. So I did not change it — flagging in case that assumption is wrong somewhere.