feat(cli): filter MCP server spawning by -t/--toolsets flag + fix orphan subprocess leak

[SelfParody]

Problem

Every hermes -z invocation spawns ALL configured MCP servers as fresh subprocesses, regardless of the -t/--toolsets flag. With 4 MCP servers configured (Notion, Trello, Fireflies, Slack), this adds ~60 seconds of cold-start overhead per invocation (verified via stack-sampling: agent blocks in os_waitpid → __wait4 for the entire gap). The -t flag currently only filters which tools the LLM sees in the prompt — it does not prevent unneeded MCP servers from spawning.

Additionally, MCP subprocesses orphan to PPID=1 on hermes -z exit because the existing shutdown_mcp_servers() and _kill_orphaned_mcp_children() helpers are not called from the oneshot CLI exit path. Observed: 184 orphaned MCP processes accumulating 6.5 GB RAM in a single development session.

Impact

Test	Before	After	Δ
hermes -z "Say hello" -t web	65.5s	6.6s	−90%
hermes -z "Say hello" -t slack	67.1s	5.5s	−92%
hermes -z "Say hello" (no -t)	76.7s	76.7s	no change (backwards-compatible)
PPID=1 orphans across 3× -z runs	+12	+0	leak fixed
SIGINT mid-flight cleanup	leaks	0 orphans	now reaped

What this PR does

Patch B — MCP spawn filtering (tools/mcp_tool.py + hermes_cli/main.py)

• Adds allowed_mcp_names: Optional[List[str]] = None to discover_mcp_tools().
• When -t/--toolsets is set, only MCP servers whose names appear in the toolset list are spawned. Built-in toolset names (e.g. web, memory) in the list are silently ignored — they don't need MCP spawning anyway.
• Without -t, all configured MCPs spawn as before. No regression.
• Skipped servers logged at debug level.

Patch C — Orphan cleanup (hermes_cli/main.py)

• Registers an atexit handler that calls shutdown_mcp_servers() (graceful async close) followed by a 50 ms settle and _kill_orphaned_mcp_children(include_active=True) (force-kill any subprocess that escaped SDK teardown).
• Also installs SIGTERM + SIGINT handlers so kill <pid> and Ctrl+C trigger the same cleanup. SIGKILL still bypasses (kernel-forced), but launchctl unload (SIGTERM) and interactive Ctrl+C are the common production cases.
• All cleanup wrapped in try/except so failed cleanup never crashes the exit path. Signal-handler installation also wrapped (ValueError on non-main thread, OSError on platforms that don't allow these signals — falls back to atexit-only coverage).

Related issues

• Fixes the MCP-subprocess component of Memory leak: Gateway accumulates ~8GB RAM in 1h with 2 active Discord sessions #18438 (gateway memory leak — same root cause manifesting in the gateway path)
• Supersedes the startup-drag portion of fix: MCP token resolution, startup drag, and TUI test-leak resilience #18523 (closed unmerged)
• Extends the toolset-gating pattern from fix(memory): respect toolsets and reconnect openviking #18166 and fix(agent): gate memory tool injection on platform_toolsets containing 'memory' #5788 (memory providers) to MCP servers
• Follow-up to feat(api_server): honor inbound 'model' and 'provider' fields for per-request routing #18549 (feat(api_server): honor inbound model and provider fields) — same author, same general direction (per-request filtering)

Test plan

• time hermes -z "Say hello" -t web drops from 65s+ to <10s on systems with 4 configured MCP servers
• time hermes -z "Say hello" (no -t) unchanged (backwards-compat verified at 76.7s on a 4-MCP system, identical to pre-patch)
• ps -e -o pid,ppid,command | grep "npm exec.*mcp" | awk '$2==1' shows 0 new entries after consecutive hermes -z runs
• Ctrl+C during a hermes -z run; verify no orphan subprocesses remain
• Patch applies cleanly to upstream main (git apply --check)
• CI test suite passes (will verify on PR open)

Code review

Internal code review by qwen3-max-preview flagged two warnings, both addressed in this commit:

1. atexit doesn't catch signals → added SIGTERM + SIGINT handlers.
2. Cleanup-order risk if shutdown hangs → added 50ms settle between graceful shutdown and force-kill, plus added a docstring documenting the trade-off.

Verdict: APPROVE_WITH_CHANGES (now applied).

🤖 Generated with Claude Code

[SelfParody]

Update — v3 force-pushed (force-with-lease)

After fusion code review (Codex + Sonnet + Gemini + GPT-5.5 judge), v2 came back as SHIP_WITH_FIXES with 3 required fixes. All 3 applied + 1 strongly-recommended fix:

Required fixes (all applied)

1. MCP fail-open regression (Codex MAJOR) — v2 moved the tools.mcp_tool import outside the discovery try/except, so an MCP SDK failure aborted the CLI instead of falling back to no-MCP. v3 wraps the import in its own try/except and guards the cleanup machinery + discovery block with if _mcp_imports_ok:.

2. SIGTERM exit code regression (Codex MAJOR) — v2 ended SIGTERM with sys.exit(0), losing the conventional 143 (128+15) signal-derived exit code that supervisors (launchd, systemd, foreman) rely on. v3 restores signal.signal(SIGTERM, SIG_DFL); os.kill(getpid(), SIGTERM) after cleanup. Idempotency guard prevents re-cleanup on the kernel-routed re-raise.

3. Unmatched-MCP warning logic (Codex+Sonnet MAJOR) — v2's warning fired only when the token list was empty, missing the case the comment described. v3 reads mcp_servers config to compute the intended-MCP subset of allowed_mcp, then checks tools.mcp_tool._servers (live registry) to confirm those servers actually spawned. Distinguishes -t web (silent — user intent) from -t bogus_mcp_name (warn — misconfig).

Strongly-recommended fix (applied)

4. _stdio_pids poll-loop robustness (Sonnet MAJOR) — fixed time.monotonic() references to use _time (matching main.py's module-level alias). Added defensive dict(_pid_dict) snapshot to handle mutation-during-iteration. Replaced bare except: pass with logger.debug(..., exc_info=True) so silent failures are diagnosable.

Reviewer false-positive identified by GPT-5.5 judge

Gemini claimed "except Exception catches KeyboardInterrupt" — incorrect; KeyboardInterrupt inherits from BaseException. v3's raise KeyboardInterrupt from the SIGINT handler propagates as expected.

Verified locally (post-rework)

hermes -z "ACK" -t web        : 4.3s wall (was 65s — 93% reduction)
hermes -z "ACK" -t slack      : 5.5s wall (was 67s — 92% reduction)
hermes -z "ACK"  (no -t)      : 75s     (unchanged — backwards-compat)
SIGINT mid-flight             : exit 130 ✓
SIGTERM mid-flight            : exit 143 ✓
-t web (no MCP names)         : no false-positive warning ✓
Orphan accumulation           : 0 new PPID=1 children across 10+ runs

Patch v3 = 282 lines. Applies cleanly to current upstream/main HEAD.

🤖 Generated with Claude Code

[SelfParody]

Update — v5 force-pushed (FINAL after 4 fusion rounds)

After 4 fusion code review rounds (Codex + Sonnet + Gemini reviewers, GPT-5.5 judge), all concerns addressed:

Final fix tracking

v2 required fix	Final status
1. Fail-open import behavior	✅ FIXED
2. SIGTERM exit code 143	✅ FIXED
3. Unmatched -t warning logic	✅ FIXED
4. _stdio_pids robustness	✅ FIXED
5. time/os imports verified	✅ FIXED
6. Poll-loop logger.debug	✅ FIXED
7. Idempotency guard	✅ FIXED
8. SIGTERM uses self-contained _os_local (v4 fusion finding)	✅ FIXED (v5)

Verified locally (final)

hermes -z "ACK" -t web        : 4.8s wall  (was 65s — 93% reduction)
hermes -z "ACK" -t slack      : 5.5s wall  (was 67s — 92% reduction)
hermes -z "ACK"  (no -t)      : 75s      (unchanged — backwards-compat)
SIGINT mid-flight             : exit 130 ✓
SIGTERM mid-flight            : exit 143 ✓
Orphan accumulation           : 0 new PPID=1 across 10+ runs

Notable reviewer false-positive caught by judge

Codex's v4 review claimed the SIGTERM handler would NameError on the bare os.kill call, predicting exit code 1. Ground-truth local test confirmed the exit code is 143 — the bare os resolved via main.py's module-level import os at line 48. Codex's reasoning was based on a misread of scope. Sonnet caught the same line as a "self-containment leak" (NIT severity) and Gemini concurred. The judge correctly identified Codex's NameError claim as a false positive while keeping the cosmetic fix in scope.

v5 applies the cosmetic fix anyway (_os_local.kill(_os_local.getpid(), signum)) for consistency with the rest of the cleanup scope.

Cumulative review cost

Round	Cost
v1 (qwen3-max single)	$0.005
v2 fusion + judge	~$0.50
v3 fusion + judge	~$0.45
v4 fusion + judge	~$0.45
TOTAL	~$1.40

vs estimated cost of merging the v1 broken patch: weeks of "Ctrl+C breaks Hermes CLI" debugging.

🤖 Generated with Claude Code

[teknium1]

Thanks for identifying the unnecessary MCP startup work. The filtering goal remains valid on current main, but this patch needs a structural port rather than a direct salvage.

Problems

• The changed startup hunk no longer exists: current CLI startup routes chat/oneshot through hermes_cli/main.py:12442-12455 and hermes_cli/mcp_startup.py:74-84, where background discovery still calls discover_mcp_tools() without a filter. The PR patch fails to apply at hermes_cli/main.py:10351.
• The cleanup portion is now superseded by the stdio parent-death watchdog at tools/mcp_tool.py:2056-2066 and final active-child reaper at tools/mcp_tool.py:5623-5626 (commit 5089c84dbf852a43ac879217d271ce3b8df9a3b6).
• No regression tests cover filtered background/oneshot discovery; current startup coverage is in tests/hermes_cli/test_mcp_startup.py:48-103.

Suggested changes

• Thread an optional explicit-MCP allowlist through the current mcp_startup helper into discover_mcp_tools(), preserving the unrestricted no--t default.
• Retain the current watchdog/reaper implementation and add focused tests for built-in-only, named-MCP, and omitted--t cases.

Automated hermes-sweeper review.

[kshitijk4poor]

Triage note (perf/P2 sweep, verified against origin/main 30b83ab7b1):

Half landed independently: the orphan-subprocess-leak half is superseded on main — _cleanup_oneshot_runtime() (hermes_cli/main.py ~L166) now calls shutdown_mcp_servers() on the oneshot exit path (~L194). The -t/--toolsets spawn filter is still live: discover_mcp_tools() (tools/mcp_tool.py ~L8053) takes no filter argument, so a -t terminal oneshot still spawns every configured MCP server.

The diff no longer applies to main (git apply --check fails on both hermes_cli/main.py and tools/mcp_tool.py; the oneshot startup block moved substantially — 4 months of drift). Could you rebase to the filter half only (allowed_mcp_names= on discover_mcp_tools + the -t plumbing), dropping the atexit/SIGINT/reap code that main now has? It's complementary to #81310 (TUI-launch dedup in _is_tui_chat_launch) — different surface, no shared hunks.

Benchmark: not reproduced — the diff does not apply to current main, so a same-base before/after (MCP servers spawned for hermes chat -t terminal -q ...) couldn't be measured in the sweep budget. The premise is greppable (discover_mcp_tools spawns unconditionally), so a rebased version can carry a spawn-count table. If there's no response in ~2 weeks a later sweep may close as does-not-apply; the filter is worth landing.

[kshitijk4poor]

Salvaged onto current main via #101634 — your commit is cherry-picked with authorship preserved (@SelfParody). The orphan-subprocess-reap half had already landed on main, so the salvage carries the -t/--toolsets MCP spawn filter, re-wired to the inline/background/deferred discovery paths main has grown since, and moves the filter ahead of the mcp SDK import so -t terminal skips it entirely.

Closing this one in favour of the salvage; thanks for the fix.