Repository navigation
fix(mcp): reclaim discovery-only connections - #53046
mlimarenko wants to merge 1 commit into
Conversation
|
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. |
|
The following comment was made by an LLM, it may be inaccurate: |
|
The PR body already contains The Could that existing fallback be brought to |
|
Deployed the integrated CLI and Desktop candidate (d780d52a6a4d1e881a7e769a3f7f68cad320d831) locally and observed 32 samples over 607 seconds. The server PID stayed unchanged; all monitored API reads succeeded, with a maximum of 299 ms for message reads. All five sessions active before replacement produced fresh assistant messages and completed steps after native restart recovery. With 76 loaded locations, discovery cleanup reduced the startup tree from 953 processes to 15 within 143 seconds (PSS 22901 -> 2134 MiB; SwapPss 24938 -> 253 MiB). At the end there were 78 loaded locations and 29 active sessions: 120 processes, PSS 6086 MiB, SwapPss 227 MiB. Concurrent builds and browser work contribute to the tree; after the first two minutes, root PSS ranged from 1257 to 2162 MiB. This is production evidence for discovery-resource release, not proof that all used connections or root memory are reclaimed. The full Desktop reproduction of the original intermittent timeout remains unconfirmed; the other integration gaps in the PR body remain explicit. |
72c048d to
b449a13
Compare
|
Rebased onto 118 affected Core tests, 3 schema tests and 2 app MCP toggle tests passed. Native pre-push The upstream |
b449a13 to
d54720e
Compare
|
Rebased onto current Resolved the upstream formatting/rename/import conflicts while retaining the fix's behavior. 119 Core MCP tests, 3 schema tests and 2 app MCP toggle tests passed. The native pre-push The full upstream test/check result is not established on this head yet; only the local checks above are confirmed. The Draft status is unchanged. This fix targets discovery-only MCP connections; it does not claim to eliminate retained Location-graph/root-memory growth. |
8791bf2 to
324e30a
Compare
|
Rebased onto Local targeted checks passed: 128 core tests (MCP lifecycle/OAuth/instructions/import boundaries and commands), 3 schema tests, and 2 MCP UI sync tests. The native pre-push |
324e30a to
55b671d
Compare
|
Rebased onto current Both patch commits are unchanged according to The description still references Base clarification: release commit |
|
Rebased onto
Upstream CI is not green: |
55b671d to
e7c9baa
Compare
e7c9baa to
db50e19
Compare
|
Rebased onto current Resolved the new whole-file GUI lint findings without changing MCP retention behavior. Two MCP-sync tests, six rendered MCP toggle cases, and app typecheck passed. Required The exact-head |
db50e19 to
ff37439
Compare
|
Independent validation on Linux (CachyOS, KDE/Wayland) — the reclaim behavior works and directly fixes the duplicate-set accumulation we were hitting. Setup: isolated XDG dirs (config/data/state), 4 local stdio MCP servers (
Why this matters (from the official build): connections are retained per Location indefinitely; with 3 TUI sessions in 3 project dirs we measured 4 full sets (~730 MB PSS for the MCP processes alone, and the oldest Built with One unrelated observation: |
|
Small correction to my validation above: the "No MCP servers configured" I saw for With the config in the right place, |
ff37439 to
982736a
Compare
982736a to
fa2b995
Compare
|
Second-platform corroboration (macOS), same head Setup: built with Bun 1.4.3 ( Before (official 2.0.32): MCP sets duplicate per session/activity and are not reaped; stack 1.4–1.9 GB with 1–2 TUIs; intermittent After (this PR):
Two small notes (not blockers):
Happy to re-test on subsequent heads. |
|
@kris-cloudaccess Thanks for testing on both Linux and macOS, and for following up on Could you clarify one detail in the macOS run: was the codebase-memory tool call made before the 80-second process count, with the same service and Location still running? This patch deliberately keeps a connection after a tool call so it does not discard browser or terminal state. Only discovery-only connections have the fixed 60-second deadline; catalog polling does not extend it. Your report doesn't say whether those measurements were from the same phase, and I shouldn't have guessed. The Linux 4 → 0 comparison and the macOS memory reduction are useful independent results. Neither the old-daemon replacement note nor the sleep/wake |
|
Thanks — my wording made that look sequential, but it wasn't. To be precise: The 80 s → 0 count was taken in a discovery-only phase: after the shadow was activated and a Location was used normally (a turn that lists tools, with no MCP tool invocation), the servers went from connected to 0 processes after ~80 s idle. The I can confirm the retention rule from the current Linux run (same head If it helps, I can do a phase-separated rerun on macOS (restart → discovery → count at 60/80/120 s; then a codebase-memory tool call → count at 60/80/120 s) and post the numbers. |
|
Thanks, that clears it up. The macOS count covers discovery-only cleanup; the Linux Serena check confirms that a tool-used connection stays alive while the discovery-only ones return to No need to repeat the macOS matrix for this question. Thanks for testing both platforms and checking the retained case too. |
8f8bc3c to
cf89e3c
Compare
|
Fixed a startup race in this PR's unmerged lazy-start changes in e572c6e. If MCP activation happened before the first config registration, activation saw no entries; registration then left enabled servers pending without starting them. Both paths now share the same pending-server startup helper. The regression forces that ordering and checks one spawn, concurrent start idempotence, and idle cleanup. The PR-head MCP tests pass (87 tests), and the native push hook passed lint and typechecks. This is a bug in this branch, not an upstream release regression. |
22650de to
90a138b
Compare
aa6af1e to
16c3eb1
Compare
16c3eb1 to
3678a22
Compare
|
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. |
|
This PR is still active. Could a maintainer reopen it after the automated age/reaction cleanup? The refreshed branch tip is 694f1ffaf034d67318dedabb9d2e3c57f8b280ec, based on The focused MCP suite passes 114 tests, including the new pooled-release/reacquisition/disposal case. Attached-client, instructions, command, schema and MCP UI checks also pass; generated-client verification is clean, and the native pre-push gate passed. Full upstream CI is not established for the refreshed tip while the PR is closed. The scope remains discovery-only resource release, not SIGKILL orphan cleanup or a guarantee that every tool-used connection is reclaimed. The linked issue #51003 was consolidated into #37844, where the bounded reproduction evidence is already recorded. |
Issue for this PR
Fixes #51003 for discovery-only MCP connections.
Type of change
What does this PR do?
Configured MCP servers remain dormant until first use. Discovery-only connections release after the fixed 60-second window while retaining cached tools, prompts, instructions, title and resource catalog; real tool/prompt/resource work retains the connection until disposal. The canonical connection owner distinguishes idle release from disposal. Pooled release drops only the Location's own lease, without closing another holder's transport. Cached idle reads, transparent reacquisition, late registration, attached-only dispatch and upstream's per-server startup timeout are preserved. Consumers expose the idle status. This is not SIGKILL orphan cleanup or reclamation of all tool-used connections.
How did you verify your code works?
Screenshots / recordings
Desktop and constrained-viewport MCP controls were exercised by the native Playwright regression tests.
Checklist
Visual