Skip to content

fix(mcp): reclaim discovery-only connections - #53046

Closed
mlimarenko wants to merge 1 commit into
anomalyco:devfrom
mlimarenko:mcp-discovery-retention
Closed

mlimarenko wants to merge 1 commit into
anomalyco:devfrom
mlimarenko:mcp-discovery-retention

Conversation

@mlimarenko

@mlimarenko mlimarenko commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Issue for this PR

Fixes #51003 for discovery-only MCP connections.

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

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?

  • 114 focused Core MCP tests passed, including pooled-holder release, cached metadata, reacquisition and disposal.
  • 23 attached-client/instructions/command/plugin-command tests, 3 schema tests, 8 app MCP-machine tests, 15 GUI MCP-machine tests and 4 rendered TUI dialog tests passed.
  • A concurrent multi-file Core run hit one stdio-pagination timeout; focused per-file runs passed. The cause is not established, and the full suite is deferred to CI.
  • Affected package typechecks, changed lint and generated-client verification passed; native pre-push lint/typechecks passed.
  • Full upstream CI is not established on this refreshed tip while the PR is closed.

Screenshots / recordings

Desktop and constrained-viewport MCP controls were exercised by the native Playwright regression tests.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Visual

<svg xmlns="http://www.w3.org/2000/svg" width="240" height="72" viewBox="0 0 240 72">
  <style>@media(prefers-reduced-motion:no-preference){.state{animation:show 2s infinite}}@keyframes show{0%,100%{opacity:.5}50%{opacity:1}}</style>
  <rect x="8" y="12" width="224" height="48" rx="8" fill="none" stroke="currentColor"/>
  <text class="state" x="120" y="41" text-anchor="middle" fill="currentColor" font-family="sans-serif" font-size="14">Discover → idle</text>
</svg>

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

@mlimarenko

Copy link
Copy Markdown
Contributor Author

The PR body already contains Fixes #51003: #51003.

The needs:issue label appears to be a false positive for the required v2 target. I checked GraphQL: baseRefName = v2, closingIssuesReferences.totalCount = 0. The default-branch pr-standards workflow only checks that count; the v2 workflow already has the PR-body fallback for this case.

Could that existing fallback be brought to dev, or the incorrect label removed? The PR remains targeted at v2 per CONTRIBUTING.

@mlimarenko

Copy link
Copy Markdown
Contributor Author

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.

@mlimarenko
mlimarenko force-pushed the mcp-discovery-retention branch from 72c048d to b449a13 Compare October 4, 2026 02:15
@mlimarenko

Copy link
Copy Markdown
Contributor Author

Rebased onto v2 at f72a103a4b94fb9a467294877bf6243dd99c6bdc; current head is b449a13fde05fa232e21b915788559235b51b655. git range-diff confirms the patches are unchanged.

118 affected Core tests, 3 schema tests and 2 app MCP toggle tests passed. Native pre-push bun run check also passed (36 typecheck tasks).

The upstream test, check and nix-eval runs on this head are action_required, rather than failed tests. Could a maintainer approve the fork workflow runs? Test run.

@mlimarenko

Copy link
Copy Markdown
Contributor Author

Rebased onto current v2 at 8eb2ebd96cb2e25f9607f753e892a533628b8d46. New head: d54720e36b97ad66b369d96eedfd753970e0414f.

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 bun run check also passed all 36 tasks. GitHub reports this head as conflict-free.

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.

@mlimarenko
mlimarenko marked this pull request as ready for review October 6, 2026 09:31
@mlimarenko
mlimarenko force-pushed the mcp-discovery-retention branch from 8791bf2 to 324e30a Compare October 8, 2026 01:05
@mlimarenko

Copy link
Copy Markdown
Contributor Author

Rebased onto v2 at 216c31174. Current head: 324e30ab8ca64c10014b7bdbd79aaaf8c59dd69f.

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 bun run check passed on this head. Full upstream CI remains subject to maintainer approval for fork workflow runs; it is not claimed green. Could a maintainer approve the test, check, and nix-eval workflows?

@mlimarenko

mlimarenko commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor Author

Rebased onto current v2 at 7d179e83f42a9bb6377b6f34d8d4fcbf04707621 (package version 2.0.26). New head: 55b671dda329a87966443cedac1a20baa2720f73.

Both patch commits are unchanged according to git range-diff. Targeted tests passed: 128 Core, 3 schema, and 2 app MCP synchronization tests. The native pre-push bun run check passed all 36 tasks and the lease-protected push completed successfully. Existing lint warnings remain.

The description still references Fixes #51003; the previous linked-issue bot warning is not a missing reference. Full upstream CI is not claimed green; fork workflows may require maintainer approval.

Base clarification: release commit 9b4ec5714d481559990db0a816d5dec19541a814 is a sibling of the current development line, not its ancestor; these PRs target and are rebased onto upstream/v2 as requested.

@thdxr
thdxr changed the base branch from v2 to dev October 10, 2026 19:30
@mlimarenko

mlimarenko commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Rebased onto v2 at 4617210822bdbb31e5749751afbfb4584ccedac8; head: e7c9baa47591e58441ceaf8a5e8bdd0e8bf84a57.

git range-diff shows unchanged runtime patches; the generated client was regenerated to preserve upstream's IntegrationID type. Passed 128 affected Core tests, 3 Schema tests, 2 app MCP-sync tests, and native pre-push bun run check (36 typecheck tasks; lint warnings remain). The PR's current dev target is unchanged.

Upstream CI is not green: check, test, and nix-eval on this exact head are action_required and need maintainer approval. PR standards passed.

@mlimarenko

Copy link
Copy Markdown
Contributor Author

Rebased onto current dev at 34d8ded0c58dd66808c1d60e081e8c66da12a896, rather than the old v2 alias. Published head: db50e194565e05826ebd0307ad6def0bb1dd8fe3.

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 lint:changed and the native pre-push gate passed (36/36 typecheck tasks).

The exact-head test, check, and nix-eval workflows are action_required, not green; maintainer approval is still needed. PR standards passed.

@kris-cloudaccess

Copy link
Copy Markdown

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 (codebase-memory-mcp, donsetch, serena, sqz), fresh service per build, one opencode run session to trigger a location's MCP, then a 90 s idle window. MCP children counted under the service PID.

Step This PR (v0.0.0-pr-53046-202610160748) Official 2.0.30
Service start 0 MCP processes (lazy) 0
After one session 4 processes (one set for that location) 4
After 90 s idle 0 processes — released 4 — retained indefinitely

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 donsetch had grown to 401 MB after a day of use). Details in #52965. With this PR the idle connections are reclaimed and the footprint returns to zero after use.

Built with bun run script/build.ts --single --skip-web-ui at ff37439; both services ran on their own ports in isolated data dirs.

One unrelated observation: opencode mcp list in an isolated env printed "No MCP servers configured" even though the same config (4 servers) loaded fine for sessions — possibly because the command only reads project-scope config. Mentioning it in case that's unexpected.

@kris-cloudaccess

Copy link
Copy Markdown

Small correction to my validation above: the "No MCP servers configured" I saw for opencode mcp list was my isolated environment's mistake (I wrote the config one directory level too high for XDG_CONFIG_HOME), not a behavior of this PR.

With the config in the right place, mcp list correctly shows all servers with the new lazy status — ○ pending until first use — while the official 2.0.30 shows ✓ connected. That's the intended behavior change and it looks right.

@mlimarenko
mlimarenko force-pushed the mcp-discovery-retention branch from ff37439 to 982736a Compare October 16, 2026 14:29
@mlimarenko
mlimarenko force-pushed the mcp-discovery-retention branch from 982736a to fa2b995 Compare October 17, 2026 07:34
@kris-cloudaccess

Copy link
Copy Markdown

Second-platform corroboration (macOS), same head fa2b995:

Setup: built with Bun 1.4.3 (bun run script/build.ts --single --skip-web-ui), run as a PATH shadow on macOS 26 (Intel); OPENCODE_DISABLE_CHANNEL_DB=1 keeps the main opencode.db (sessions restore; no channel DB).

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 failed states for remote servers.

After (this PR):

  • All six MCP servers sit idle/pending (lazy); a tool call starts only the server needed (verified with codebase-memory on demand).
  • After ~80 s of inactivity: 0 MCP server processes remain (official keeps a 4-process set after 90 s idle in the same setup).
  • Footprint for service + one TUI ≈ 780 MB total (was ≈1.4–1.9 GB in the leak era).
  • Model requests, quota, and account switching are unaffected; sessions and tools behave normally.

Two small notes (not blockers):

  1. On macOS the pre-swap service daemon did not exit with its last client — an explicit kill -TERM was needed when replacing the running service binary.
  2. One transient re-init after an overnight sleep/wake produced DNS ENOTFOUND failures for two remote MCP servers (cleared by a restart) — connect retry/backoff on init failure would round this out.

Happy to re-test on subsequent heads.

@mlimarenko

mlimarenko commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor Author

@kris-cloudaccess Thanks for testing on both Linux and macOS, and for following up on mcp list. The config-path correction clears that up; no CLI change is needed for that observation.

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 ENOTFOUND failure is a fix claimed by this PR; I haven't reproduced those macOS cases.

@kris-cloudaccess

Copy link
Copy Markdown

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 codebase-memory check was a separate on-demand start step, and I did not re-count processes after it; I shouldn't have implied the two were one sequence.

I can confirm the retention rule from the current Linux run (same head fa2b995 merged onto v2.1.0, our daily driver): after real MCP tool calls (serena memory read/write), opencode mcp list shows serena connected — its process still alive 75+ minutes after start and ~40 minutes after the last tool call — while codebase-memory-mcp, context7, donsetch, gh_grep, and sqz are all idle with no processes. So tool-used connections are retained; only discovery-only ones are reclaimed, as you described.

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.

@mlimarenko

Copy link
Copy Markdown
Contributor Author

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 idle. Both match the intended behavior.

No need to repeat the macOS matrix for this question. Thanks for testing both platforms and checking the retained case too.

@mlimarenko
mlimarenko force-pushed the mcp-discovery-retention branch 2 times, most recently from 8f8bc3c to cf89e3c Compare October 21, 2026 08:04
@mlimarenko

Copy link
Copy Markdown
Contributor Author

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.

@mlimarenko
mlimarenko force-pushed the mcp-discovery-retention branch from 16c3eb1 to 3678a22 Compare November 2, 2026 08:24
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

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:

  • The PR was created more than 1 month ago
  • The PR had fewer than 2 positive reactions
  • Positive reactions are counted as thumbs-up, heart, celebration, or rocket reactions on the PR

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.

@mlimarenko

Copy link
Copy Markdown
Contributor Author

This PR is still active. Could a maintainer reopen it after the automated age/reaction cleanup?

The refreshed branch tip is 694f1ffaf034d67318dedabb9d2e3c57f8b280ec, based on 893e8dc43f05e872d4cac603486ce8a7181170fa. The adaptation preserves upstream's new per-server startup timeout and pooled-connection owner. A discovery-only Location releases its own pool lease without closing another holder's connection, keeps its discovered metadata, and can rejoin transparently. Connections used for real work remain retained; attached-client calls do not activate unrelated configured servers.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mcp: global stdio servers spawn once per loaded directory and exhaust memory

3 participants