Repository navigation
fix(mcp): destroy stdin on idle timeout and arm it where hosts respawn - #7525
Merged
Merged
Conversation
…rs exit The idle timer fired but nothing tore the server down. `isClosed` is only consulted after a message arrives (`for await` ... `if (isClosed) break`), and an idle stdio server is idle precisely because no message arrives, so the read loop parked forever on an open pipe. This only shows up when the parent stays alive while abandoning the pipe: the parent watchdog needs the parent to die, and the stdin `close`/`end` paths need the parent to release the write end. A host that keeps the fd open and simply forgets the child hits none of them, and the child lives forever. Destroy the input in the idle callback, mirroring what the parent-exit callback already does six lines below. The resulting ERR_STREAM_PREMATURE_CLOSE is already swallowed by the existing catch because `isClosed` is set. Adds the first two tests that exercise the idle path: one in-process, and one that spawns a real child whose parent holds stdin open, because a settled read loop still leaves a live process if anything keeps the event loop alive. Every pre-existing test sets `idleTimeoutMs: 0`, which is why the gap survived. Note this only takes effect where an idle timeout is enabled. codegraph's two servers take the 10-minute default; lsp-core, lsp-daemon and git-bash-mcp all pass `idleTimeoutMs: 0` and so still have no teardown for a live parent that abandons the pipe.
…setup The idle-timeout test's handler declared a `message` parameter, which `McpRequestHandler` types as `unknown`, so `message.id` failed typecheck on all three CI runners. That handler is never invoked in the test: its premise is a live parent that abandons stdin without writing, so no message ever arrives. Dropping the parameter matches the ten other handler call sites in the file and clears the error without a cast. Also extract `buildServerScript` and `spawnServerChild` so the watchdog and idle child processes share one spawn setup, addressing the review comment about the two helpers drifting. Each caller still passes its own config lines verbatim, so what the child actually does still reads at the call site. Verified: `tsgo --noEmit` clean for the package and repo-wide, `bun test` 22 pass / 0 fail. A temporary probe confirmed the refactored generator still produces a child that answers a real JSON-RPC request with no stderr, because the idle test asserts on process exit and would pass just as happily against a child that crashed on a malformed script. Co-Authored-By: Claude <[email protected]>
The senpi host auto-reconnects a stdio MCP server that exits: transport close marks the connection degraded and the reconnect scheduler respawns the child with backoff (senpi connection.ts + reconnect.ts), so an idle exit costs one backoff-delayed respawn instead of a broken session. Arm the idle timeout at the mcp-stdio-core default (10 min, matching the senpi-hosted memory MCP) with an OMO_AST_GREP_IDLE_TIMEOUT_MS override and an explicit option seam. Values <= 0 still disable the timer. Now that #6548 makes the idle callback destroy stdin, an idle ast-grep child exits instead of parking forever on a pipe held by a live parent.
opencode marks a closed stdio MCP server failed until a user reconnects via /mcp, and codex only auto-reconnects its own codex_apps server, so arming the idle timer on those hosts would trade a leak for broken live sessions. The comments pin that analysis at each idleTimeoutMs: 0 call site so it is not 'finished' later without respawn evidence.
github-actions
Bot
added
ast-grep-mcp
The committed extensions bundles embed inputs touched by this branch: mcp-stdio-core's server.ts (idle destroy from #6548) reaches omo.js and omo-memory-mcp.js transitively, and ast-grep-mcp's mcp.ts feeds the staged runtime that build-extension.mjs --check rebuilds. Regenerated via bun run build:senpi-plugin under the CI-pinned bun 1.4.0; verified with node packages/omo-senpi/plugin/scripts/build-extension.mjs --check (exit 0).
github-actions
Bot
added
omo-senpi
code-yeongyu
force-pushed
the
fix/mem-mcp-idle-teardown
branch
3 times, most recently
from
August 31, 2026 03:46
28e8173 to
38154b9
Compare
…x host The stub called runJsonRpcStdioServer without idleTimeoutMs, inheriting the 10-minute default. That default was inert before #6548; with the idle callback now destroying stdin, the stub would silently die after 10 idle minutes on codex — a host that never respawns an exited stdio MCP server (codex-rs auto-reconnects only its own codex_apps server) — turning later codegraph tool calls into dead-connection errors instead of the skip reason. Pin idleTimeoutMs: 0 with the no-respawn rationale, add a lifecycleLog seam plus a test pinning idle_timeout_ms: 0, and pin the same contract in mcp-stdio-core: an explicitly zero idle timeout creates no timer at all. Regenerate the component's committed dist via the plugin-level build (bun run --cwd packages/omo-codex/plugin build), which also syncs the previously stale #6548 idle-destroy delta into it. The log option uses a conditional spread to stay assignable under the component's exactOptionalPropertyTypes tsconfig.
code-yeongyu
force-pushed
the
fix/mem-mcp-idle-teardown
branch
from
August 31, 2026 03:47
38154b9 to
7c3cb83
Compare
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.
Advances #6547. Supersedes #6548 — the two
mcp-stdio-corecommits are @lifrary's work cherry-picked verbatim (authorship preserved); this PR rebases them onto dev (resolving theonPoll/watchdog conflict inserver.test.tsby keeping dev's per-poll harness inside the refactored child-spawn helpers) and then closes the remaining teardown gaps only where host respawn is evidenced.Summary
mcp-stdio-core(from fix(mcp-stdio-core): destroy stdin on idle timeout so abandoned servers exit #6548, by @lifrary): the idle callback now callsconfig.input.destroy(). Previously the timer only setisClosedand firedonIdleTimeout, but the read loop observesisClosedonly after a message arrives — an idle server is idle precisely because none does. A live parent that abandons the pipe kept the child parked onfor awaitforever. With the destroy, the idle timeout becomes a real exit path (same shape as the parent watchdog).ast-grep-mcp: arms the idle timeout on the senpi-hosted stdio server — the one consumer whose host provably respawns. Default 10 minutes (matches themcp-stdio-coredefault already shipped for the senpi-hosted memory MCP; an idle exit costs one backoff-delayed respawn, not a broken session). Configurable viaOMO_AST_GREP_IDLE_TIMEOUT_MS(mirrorsOMO_AST_GREP_PROJECT_CWDand the git-bash*_TIMEOUT_MSenv convention) with an explicit option seam;<= 0disables.idleTimeoutMs: 0call sites are not "finished" later without respawn evidence.158d5dcaf): the codegraph unavailable stub inherited the now-live 10-min default and would die mid-session on codex; pinned toidleTimeoutMs: 0with a regression test, and the component's committeddist/regenerated via the plugin-level build (it had also gone stale for fix(mcp-stdio-core): destroy stdin on idle timeout so abandoned servers exit #6548's destroy line — nothing in CI verifies it, which is how that slipped).Per-site verdict table
Respawn evidence gathered from host sources (upstream trees fetched via GitHub API, 2026-08-31):
mcp-stdio-core/src/server.tsidle callbacklsp-daemon/src/proxy.ts:93sst/opencodemcp/index.tswatch():client.oncloseremoves the client and setsstatus: failed "Connection closed"; reconnect only via user-drivenMCP.add/MCP.connect(/mcpcommand)idleTimeoutMs: 0; comment addedlsp-core/src/mcp.ts:77(lsp-tools-mcp CLI)idleTimeoutMs: 0; comment addedast-grep-mcp/src/mcp.ts:284registerMcpServer,lifecycle: "lazy")mcp/connection.ts:transport.onclose->markDegraded;reconnect.tsconfigureMcpReconnectrespawns viarenew()->connect()with backoff 500ms-8s (breaker 5/30s). Sibling senpi-hosted memory MCP already ships the 10-min defaultgit-bash-mcp/src/mcp.ts:76openai/codexcodex-mcp/src/rmcp_client.rs: reconnect-with-backoff only forCODEX_APPS_MCP_SERVER_NAME; user-configured stdio servers get no respawnidleTimeoutMs: 0; comment addedomo-codex/.../mcp-unavailable.ts(unavailable stub, inherited the 10-min default)idleTimeoutMs: 0— post-#6548 the default became a real stdin destroy, which would silently kill this stub after 10 idle minutes on a no-respawn host; regression test pinsidle_timeout_ms: 0, component dist regeneratedomo-codex/.../mcp-bridge.ts:84-88(watchdog only, no idle timer)runJsonRpcStdioServercaller — it drivescreateParentWatchdog+ manual forward loops, so there is no idle timeout to configure; verified by sweep, no edit neededomo-senpi/.../memory-server.ts(inherits the 10-min default)Why not enable idle everywhere
An unproven idle-kill that breaks live sessions is worse than the leak. On opencode, a closed stdio MCP server is
faileduntil the user manually reconnects via/mcp; on codex, only OpenAI's owncodex_appsserver auto-reconnects. Killing an idlelspproxy orgit_bashchild under those hosts removes tools from live sessions permanently.Residual (why this advances rather than fixes #6547)
The OpenCode-path leak — an opencode host that abandons the
lspproxy's stdin while staying alive — is still open, by design of this PR: closing it requires an opencode-side respawn or per-session stdin close, which is out of this repo's control. Also unchanged:onIdleTimeoutremains un-awaited and the idle clock arms on message arrival rather than handler completion (pre-existing shape).Verification
TDD, all runs on a remote CI clone (
bun /tmp/omo-mac-test.mjs), never locally:331 pass / 4 fail— whilemcp-stdio-core(including fix(mcp-stdio-core): destroy stdin on idle timeout so abandoned servers exit #6548's two idle tests) is green in the same run335 pass / 0 fail(ast-grep + mcp-stdio-core)mcp-stdio-core,ast-grep-mcp,lsp-daemon,lsp-core,git-bash-mcp): 507 pass / 0 fail (2 pre-existing Windows-gated skips)idle_timeout_ms: 600000vs expected0at test-only commit, GREEN1 pass / 0 failafter fix; component suite delta parent→tip is exactly +1 passing test (47/31/78 → 48/31/79, the 31 reproduce identically at parent and are clone-env artifacts; CI's codex-compatibility runs them green); componenttsc --noEmitclean; mcp-stdio-core + ast-grep after the new pin: 336 pass / 0 failtsgo --noEmitclean on all five touched packagesFull evidence:
.omo/evidence/20260831-mem-mcp-idle/REPORT.md(RED/GREEN logs + verdict table).