Skip to content

Add watcher preopen controls and harden server shutdown#120

Open
isham703 wants to merge 1 commit into
isaacphi:mainfrom
isham703:codex/swift-mcp-memory-fix
Open

Add watcher preopen controls and harden server shutdown#120
isham703 wants to merge 1 commit into
isaacphi:mainfrom
isham703:codex/swift-mcp-memory-fix

Conversation

@isham703
Copy link
Copy Markdown

@isham703 isham703 commented Mar 2, 2026

Summary

This PR addresses Swift MCP memory/process buildup observed in large workspaces by adding opt-in runtime controls and hardening shutdown behavior, while preserving existing defaults.

Changes

  • Add runtime flags/env controls (default-preserving):
    • --watcher-preopen-on-register (default true)
    • --watcher-preopen-max-files (default 0 = unlimited)
    • --idle-timeout (default 0s = disabled)
  • Add watcher config fields:
    • PreopenOnRegistration bool
    • PreopenMaxFiles int
  • Gate registration-time preopen scans behind PreopenOnRegistration.
  • Enforce deterministic preopen cap via PreopenMaxFiles (counts successful opens only).
  • Add idle watchdog and activity tracking using MCP BeforeAny hook.
  • Make server cleanup idempotent (sync.Once) and ensure cleanup always runs when stdio loop exits.
  • Make Client.Close() idempotent in LSP client to avoid repeated wait/close races.
  • Update README with new flags/env and Codex-specific example.

Why

In practice, duplicated MCP stacks plus registration-triggered preopen scans can create high memory pressure and process accumulation. This change gives clients an opt-out/cap for preopen and adds reliable idle/process cleanup.

Compatibility

Defaults preserve current behavior globally, so existing clients are unaffected unless they opt in.

Tests

  • Added/updated unit coverage for:
    • preopen disabled => no registration scan opens
    • preopen enabled => matching files open
    • preopen max cap => bounded opens
    • repeated close calls => idempotent/no-op-safe
  • Ran package tests (non-integration) successfully.

Files

  • main.go
  • internal/watcher/interfaces.go
  • internal/watcher/watcher.go
  • internal/lsp/client.go
  • internal/lsp/client_close_test.go
  • internal/watcher/testing/mock_client.go
  • internal/watcher/testing/watcher_test.go
  • README.md

STRd6 pushed a commit to STRd6/mcp-language-server that referenced this pull request May 14, 2026
When the LSP registers workspace/didChangeWatchedFiles patterns, the
old behavior spawned a goroutine that walked the entire workspace and
sent didOpen for every matching file. The TODO said this was for
typescript-language-server, but empirically TS does not register
watchers at all (neither do gopls, clangd, civet-lsp). Only
rust-analyzer registers them today, and it indexes via cargo metadata
independently of didOpen state.

For each LSP in our integration-test matrix, all features (definition,
references, hover, rename, document symbols, diagnostics) work with the
scan removed. Tools that need a specific file already call OpenFile on
demand, so the LSP gets a didOpen for the files actually being queried.

Eliminates the symptom behind issue isaacphi#83 ("too many open files") and
the corresponding memory blowup on large rust workspaces, without
needing PR isaacphi#120's runtime flags.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
STRd6 pushed a commit to STRd6/mcp-language-server that referenced this pull request May 14, 2026
cleanup() in main.go can fire from three independent goroutines:
the signal handler (SIGINT/SIGTERM), the parent-death watcher, and
the main goroutine's error-exit path. Today each call independently
drives CloseAllFiles -> Shutdown -> Exit -> Close on the LSP client,
which produces two parallel teardowns when triggers fire close in
time -- duplicate didClose notifications, racing access to
s.lspClient, and occasional shutdown-after-exit log noise.

Wrap the body of cleanup in a package-level sync.Once and lift
lsp.Client.Close into a sync.Once.Do that caches its return value
in c.closeErr. The first caller runs the teardown; subsequent
callers (and the forthcoming idle watchdog) block on the Once
until it completes, then return the cached error without doing
the work twice.

Adapted from upstream PR isaacphi#120 (isham703).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
STRd6 pushed a commit to STRd6/mcp-language-server that referenced this pull request May 14, 2026
Catches the case where the parent editor (Claude Desktop, Cursor,
etc.) goes away without killing its MCP children -- the existing
parentDeath watcher only notices when the parent reparents to
PID 1, not when the parent stays alive but stops talking. With
--idle-timeout=10m the server arms a timer that resets on every
incoming MCP request via mcp-go's BeforeAny hook; if it fires the
server runs the same cleanup path as SIGTERM.

Default 0 keeps existing behavior (no timeout) so CI and
interactive sessions are unaffected. Combined with the sync.Once
cleanup guard, the idle path is just a third trigger of the same
teardown -- not a new shutdown sequence.

Adapted from upstream PR isaacphi#120 (isham703).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant