Adds a per-server `trust: full|untrusted` config key
(mcp_servers.<name>.trust). On an untrusted server, every write-capable
tool call — any tool whose discovery-time annotations do not carry
readOnlyHint=True — routes through the existing approval surface
(tools.approval.request_elicitation_consent, same lazy-import +
surface-routing pattern the MCP elicitation handler uses) before the RPC
fires. Denied/cancelled/errored approvals fail closed: the RPC never
runs, including the lazy first-use server spawn.
Design points:
- Classification happens at CALL TIME from metadata captured at
DISCOVERY (_record_tool_trust_metadata in _register_server_tools and
the lazy cache-registration path). No toolset/schema mutation, so the
toolset stays byte-stable and prompt caching is preserved.
- readOnlyHint is a server-supplied HINT: on an untrusted server a lying
server can at most skip approval for tools it claims read-only — it
can never widen access. Trust tiering itself is operator config.
- Missing/malformed annotations => write-capable (fail closed).
- Unrecognized trust values => untrusted (fail closed); missing key =>
full (backward compatible, documented in mcp-config-reference).
- The schema cache now persists readOnlyHint so lazy-registered servers
gate identically on next startup without spawning.
Tests: tests/tools/test_mcp_trust_gating.py (11 tests, TDD red->green):
approval invoked + accept proceeds, deny/cancel blocks RPC, readOnlyHint
=true skips gate, trusted/unconfigured servers skip gate, explicit
readOnlyHint=false gated, approval exception fails closed, trust
normalization, discovery-time capture (SDK objects and cached dicts).
Ported from: cloudflare-os classifyTool() (Apache-2.0), corroborated by
Claude Cowork (idea-level).
MCP servers generated from Rust/TypeScript union types commonly emit
closed value sets as const unions:
{"anyOf": [{"const": "red"}, {"const": "green"}, {"const": "blue"}]}
Strict tool-calling backends reject or mishandle these; the equivalent
property-level enum form is universally supported. Add
collapse_const_unions() to tools/schema_sanitizer.py and wire it into
the _normalize_mcp_input_schema discovery pipeline after the nullable
strip.
Rules:
- Collapse only when EVERY non-null branch is a pure const of the same
primitive type (bool never merges with integer).
- Mixed unions, non-uniform const types, and mismatched declared types
pass through untouched.
- A single {"type": "null"} branch is tolerated: consts -> enum,
null -> nullable: true hint (matches strip_nullable_unions, which
leaves null+multi-const unions alone by its one-non-null-branch rule).
- Outer title/description/default/examples carried onto the replacement.
- Deterministic, branch-order-preserving, non-mutating — applied at
discovery only, so schemas stay byte-stable per conversation.
Ported from: block/goose tool_schema_normalize.rs (Apache-2.0)
Two small config-gated features:
1. Kanban orphaned-card reconciliation (kanban.reconcile_orphans, default
true, config.yaml): a running card with broken claim bookkeeping
(claim_lock or claim_expires NULL — crash mid-claim, manual SQL, DB
restore) is invisible to all existing recovery paths
(release_stale_claims requires claim_expires NOT NULL,
detect_crashed_workers requires host-local lock + pid,
detect_stale_running is config-disabled by default) and shows Running
forever. New reconcile_orphaned_running() pass in kanban_db.py runs
each dispatch_once tick: requeues orphans to ready with an explanatory
comment, closes any leaked run, emits a 'reconciled' event, and defers
when the recorded PID is still alive on this host (never requeue
beside a live worker). Surfaced via DispatchResult.reconciled_orphans.
2. Per-server MCP identity header (mcp_servers.<name>.identity_header,
config.yaml): optional {name, value_from: static|profile, value}
mapping; the header is attached to that server's HTTP/SSE transport
requests. 'static' sends the config value; 'profile' resolves the
active Hermes profile name once at connect time (no per-call
mutation). Explicit per-server headers of the same name (any casing)
win. Invalid blocks warn-and-ignore; stdio servers warn-and-ignore.
Tests: tests/gateway/test_kanban_reconcile_orphans.py (9),
tests/tools/test_mcp_identity_header.py (13), all written first (RED)
then implemented (GREEN). No new HERMES_* env vars.
Inspired by: openai/symphony tracker reconciliation (Apache-2.0) +
Poke per-user MCP identity (idea-level).
Five review findings folded:
- schema cache writes via utils.atomic_json_write (fsync; was bare
tmp+replace), file moved to cache/mcp_schema_cache.json with 0o600
(sibling precedent: registry discovery cache)
- phantom-tool reconciliation: after a lazy server's first-use connect,
cached tools the live server no longer offers are deregistered (were
permanent registry ghosts burning circuit-breaker strikes on every
'Unknown tool' round-trip); stale fingerprint logged
- cache-load path now runs _scan_mcp_description like the eager path
(cache file is user-writable JSON; defense-in-depth)
- write-through skips the disk rewrite when the entry is unchanged
(a flapping stdio server was rewriting byte-identical JSON per
revival)
- _lazy_server_fingerprints no longer write-only dead state (consumed
by the reconciliation logging)
444 mcp tests green (440 pre-fold + 4 new guards); phantom-dereg and
write-skip mutation-checked.
Wires the fingerprint-keyed schema cache (previous commit, @Vansh5632's
design from #56832) into the startup path, re-derived onto main's
current connect machinery:
- register_mcp_servers: servers with mcp_servers.<name>.lazy=true whose
config fingerprint matches a valid cache entry register tools from
cache WITHOUT spawning; miss/stale falls back to eager connect.
- First tool use routes through _ensure_lazy_server_connected, which
composes with the connect cooldown (#50394) and _server_connecting
dedup rather than duplicating the connect path.
- resource/prompt utility handlers (list_resources/get_prompt) also
connect-on-first-use — closes the gap flagged in the original
sweeper review.
- Write-through: a live connect refreshes the cache entry.
Config gate is per-server, default OFF, matching the
idle_timeout_seconds key pattern. 24 lazy/cache tests + 440 mcp-wide
green; mutation-checked (cache-read disabled -> registration test
fails; connect bypassed -> 3 first-use tests fail).
Three fixes for concurrent MCP server spawn races in register_mcp_servers()
and discover_mcp_tools():
1. register_mcp_servers: add k not in _server_connecting guard to the
new_servers filter. Without this, a concurrent second call sees the
same servers as 'new' and spawns duplicate stdio subprocesses.
2. discover_mcp_tools: same _server_connecting guard in the
new_server_names filter. This entry point is called from CLI, TUI,
gateway, and cron — any two racing would double-spawn.
3. Stale _server_connecting cleanup on TimeoutError/InterruptedError.
When _run_on_mcp_loop times out or is interrupted, _discover_all's
gather may not have finished, leaving entries stranded in
_server_connecting that block future reconnection attempts. The
cleanup clears only entries added by this call (not external ones),
logs a warning, and records connect errors.
Salvage of #58879 by @nanami7777777 (superset of #58867 by @liuhao1024).
Adapted to current main which has evolved significantly since July 5.
Closes#58862Closes#58867Closes#58879
Inspired by Claude Code v2.1.219: MCP config string values with hidden
leading/trailing whitespace (pasted tokens with trailing newlines, URLs
with leading spaces) now trigger a startup warning naming the server and
the dotted key path, instead of failing later as an opaque auth/connect
error.
Advisory only: values are never mutated, secrets are never logged (only
key paths), and warnings dedupe to once per process per (server, path).
Checked after ${VAR} interpolation so whitespace inside referenced env
vars is caught too.
Follow-up to the salvaged #62026 ownership fix, folding in #72054's
CancelledError rule by @adurham: start() already cancels/reaps its own
run task when the caller's connect timeout cancels start() itself, so
_connect_server() must propagate cancellation without awaiting a
redundant shutdown() inside a cancelled context. Non-cancellation
failures on the unclaimed (standalone probe) path still reap the parked
task, now with the reap failure logged instead of raising over the real
error.
Also maps mrz@mrzlab630.pw for the attribution check.
Co-authored-by: Adam Durham <amdnative@gmail.com>
_stop_mcp_loop() stopped and closed the background loop without reaping
the tasks still on it. A task left suspended is resumed later by the GC,
whose finalizer drives its cleanup against the now-closed loop:
Exception ignored in: <coroutine object MCPServerTask.run ...>
File "tools/mcp_tool.py", line 2947, in run
parked = await self._wait_for_reconnect_or_shutdown(
File "tools/mcp_tool.py", line 2161, in _wait_for_reconnect_or_shutdown
t.cancel()
RuntimeError: Event loop is closed
shutdown_mcp_servers() only reaps servers held in _servers, so a server
that parked after exhausting its initial-connect budget — never inserted
there, because start() raises _error before the caller registers it — has
no owner to signal it and stays suspended until the loop is gone.
Drain the loop the way asyncio.run() does: cancel the remaining tasks and
gather them while the loop is still open, so each runs its own finally.
Cancel alone is not enough — Task.cancel() only schedules the throw.
This resolves the reported traceback, but not the ownership bug that
strands the task in the first place; that needs a follow-up. Deliberately
not using "Fixes" so #60197 stays open for it.
Addresses #60197
Addresses #66113
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The bounded lock wait was 10 x 0.2s = 2s, but the concurrent-discovery
scenario this lock exists for (#62771: hermes serve + gateway spawning
every MCP server twice) reports 40-60s discovery rounds. A 2s budget
guarantees the loser times out and runs unguarded exactly when the guard
matters. Widen to 240 x 0.5s = 120s; fail-soft unguarded fallback and
test overrides unchanged.
The include/exclude filter matched exact names only — glob-style entries
('*_radar_*') silently matched nothing, so a Cloudflare flat-mode config
meant to trim 3,320 tools to ~1,900 actually registered 3,319. Unmatched
patterns produced no warning.
- matches_name_filter(): exact membership first (O(1) for literal lists),
then fnmatch.fnmatchcase for entries containing * ? [ — same pattern
semantics as approvals.deny. Entries without metacharacters stay
strictly literal ('docs' never matches 'docs_search').
- _should_register() uses it for both include and exclude (symmetric)
- hermes mcp tools picker (mcp_config.py) pre-selection uses the same
matcher so the UI agrees with runtime registration
E2E against the live Cloudflare capture with the real exclude list:
3,320 -> 1,905 surviving (1,415 excluded); radar/DLP gone,
purge_cache/dns_records kept. 220/220 mcp_tool tests (4 new).
On Windows, pipe I/O can deliver non-UTF-8 bytes at chunk boundaries,
causing `UnicodeDecodeError` when the MCP SDK's `TextReceiveStream`
uses `errors="strict"`. Set `encoding_error_handler="replace"` on
`StdioServerParameters` so undecodable bytes become U+FFFD instead
of crashing.
Ports #63234 forward onto current main per teknium1's review.
gateway/session.py hard-coded the stale-API disclaimer for every Slack
session regardless of whether Slack tools were actually loaded. This
contradicted the system prompt when MCP or native slack tools were
present, causing the agent to refuse Slack API actions it could
actually perform (issue #6536).
Per review, the original predicate only checked the native 'slack'
toolset, missing Slack MCP servers (registered under mcp-<server> in
tools/mcp_tool.py) entirely. _slack_tools_loaded() now checks two
independent paths:
1. Native 'slack' toolset + SLACK_BOT_TOKEN (as before, but now calls
_get_platform_tools() with include_default_mcp_servers=True instead
of False, so a default-enabled MCP server also counts).
2. A connected MCP server that has ACTUALLY registered tools into the
live registry (new tools.mcp_tool.get_registered_mcp_server_names()),
whose name suggests Slack. This is session-scoped in the sense that
matters here: MCP servers connect once per gateway process (not
per-session), so checking the live per-server tool-registration map
is the correct availability-filtered signal -- unlike the earlier
get_all_tool_names() approach this replaces, which conflated ALL
built-in tool names process-wide, this only inspects the small,
purpose-built MCP server-name map.
Added a real regression test that registers a tool via the actual
tools.mcp_tool._track_mcp_tool_server() tracking function (not a mock
of the capability check) to verify a genuine Slack MCP server is
detected, plus a negative case for an unrelated MCP server.
5/5 Slack-specific tests pass; 126/126 in the full
tests/gateway/test_session.py file.
Removes Homebrew and PyPI wheel/sdist as Hermes distribution paths while
preserving the supported source, Docker, and Nix workflows.
Changes:
- Removes the Homebrew formula, PyPI publish workflow, sdist manifest
(MANIFEST.in), and wheel/sdist release-attachment logic from scripts/release.py.
- Keeps setuptools metadata and entry points required by editable installs
and Docker/Nix builds, but adds a setup.py guard that rejects wheel/sdist
builds outside a sealed Nix derivation (HERMES_NIX_BUILD=1).
- Removes pip/Homebrew install detection, PyPI update checks, the pip
self-update path, the deprecation-banner state, the postinstall subcommand,
wheel data-directory fallbacks in agent/i18n.py and hermes_constants.py,
and the ACP Registry manifest/version-lockstep release logic.
- Adds /nix/store/ path detection so `nix run` / `nix profile install`
installs (which don't set HERMES_MANAGED) are correctly identified as
"nix" rather than falling through to "git"/"unknown".
- Retired install-method values ("pip", "homebrew") in existing
.install_method stamps (both code-scoped and home-scoped) are ignored by
the allowlist reader and fall through to "unknown" instead of resurrecting
a retired enum value.
- Updates Nix packaging to ship bare runtime data (locales, optional-mcps)
through store symlinks and wrapper env vars instead of wheel data-files.
- Removes the ACP Registry manifest/icon and their version-lockstep tests.
- Deletes or rewrites packaging, pip-update, Homebrew, and ACP Registry
tests; adds parametrized coverage for the packaging build guard covering
BOTH sdist and wheel paths (the guards live in separate cmdclass entries
— a passing sdist test proves nothing about the wheel path).
- Updates installation/platform documentation and related user-facing copy.
- Adjusts the supply-chain scan so deleted install-hook files do not trigger
a finding, while additions or modifications still require the existing
ci-reviewed label gate.
Supported installation paths (unchanged):
- git installer (install.sh)
- Docker
- Nix/NixOS
- editable development installs (uv sync, uv pip install -e ., pip install -e .)
Surgical reapply of PR #37523 onto current main (the original branch
predates the SecretSource registry refactor). _build_safe_env() now
forwards env vars tagged in env_loader._SECRET_SOURCES — widened from
Bitwarden-only to any registered secret source (Bitwarden, 1Password,
plugin backends), since the provenance map is source-agnostic.
Explicit server env: config still wins; untagged secrets stay filtered.
Fixes#37499.
Builds on diffen77's #66547 (cherry-picked as the previous commits).
Extend _is_session_expired_error's iterative traversal to follow
__cause__/__context__ in addition to ExceptionGroup .exceptions — SDK
wrappers often raise a generic RuntimeError *from* the message-less
ClosedResourceError, leaving the transport signal reachable only via
the chain. The identity-visited set guards chain cycles (handlers
re-raising previously seen exceptions), and a bounded node budget
(_EXC_TRAVERSAL_MAX_NODES) caps pathological acyclic graphs.
Adds regression tests: cause/context chain detection, interruption
precedence through chains, cyclic cause/context termination, and
budget-bounded termination.
Builds on trevorgordon981's #50589 (cherry-picked as the previous commit).
The #50394 cooldown reset only ran inside the async _shutdown coroutine,
which is skipped on the empty-_servers fast path — the most common state
when a server failed to connect (failed servers are never recorded in
_servers). It was also skipped when the MCP loop wasn't running.
Clear _server_connect_retry_after/_server_connect_failures on the fast
path and in a final unconditional sweep so a full shutdown/restart always
re-attempts every configured server immediately. Adds regression tests
for both paths.
Log-storm hygiene for the reconnect machinery (#65673, #66092):
- State transitions carry exactly one WARNING each:
connected → degraded (keepalive failure), degraded → parked
(budget exhausted / rapid-drop park / permanent error),
parked → connected (revival proven healthy, via
_mark_session_proven).
- Per-attempt retry logs (initial-connect attempts, connection-lost
reconnect attempts, per-cycle rebuild notices, park-wake probes)
demoted to DEBUG.
- Backoff sleeps get +/-20% uniform jitter (_jittered) so a herd of
servers that lost the same backend doesn't retry — and log — in
lockstep.
- Add module-level _unwrap_exception_group (ported from
hermes_cli/mcp_config.py, adapted): handles nested groups, prefers
non-cancellation leaves over the CancelledErrors anyio sprays across
sibling tasks, and re-raises KeyboardInterrupt/SystemExit leaves.
- Add _classify_mcp_failure(exc) -> 'permanent'|'transient'.
Permanent: auth 401/403, NonMcpEndpointError, InvalidMcpUrlError,
FileNotFoundError/ENOENT on the stdio command. Transient: everything
else (network/EOF/ClosedResource/TaskGroup drops). Permanent failures
park immediately without burning the retry ladder — every retry
against a missing binary or a revoked credential hits the same wall.
- Every log site in run() and the keepalive failure log now logs the
unwrapped root cause as 'TypeName: message', so a dead stdio pipe
says 'BrokenPipeError' instead of the opaque
'unhandled errors in a TaskGroup (1 sub-exception)' (and empty
str(exc) dead-pipe errors are no longer blank).
Harden the immediate-reconnect path from #66271:
- _reconnect_or_reraise_group re-raises when the transport TaskGroup
carries a KeyboardInterrupt / SystemExit leaf — fatal signals must
propagate to the interpreter, never be converted into a reconnect.
- Rapid-drop budget: a completed handshake alone no longer clears
_reconnect_retries/backoff on clean transport return. A session is
UNPROVEN until it demonstrates real health — survived >=1 full
keepalive interval (keepalive success path) or served >=1 successful
tool call (_mark_session_proven). Unproven clean returns are charged
against _MAX_RECONNECT_RETRIES, so a flapping transport that
handshakes fine and drops moments later still reaches the park
instead of respawning forever (#62212: 6212 spawns in 63h).
Proven sessions keep the #57604 behaviour: budget clears, transient
blips over a long-lived session never accumulate toward parking.
Streamable-HTTP / SSE MCP transports run their stream pump inside an anyio
TaskGroup. A transient stream drop (idle timeout, brief backend blip,
server-side TCP close) surfaces as a BaseExceptionGroup escaping the
transport context manager. It reached run()'s error path, which applied
exponential backoff (1s..16s) and eventually parked the server for 300s and
deregistered its tools — turning a sub-second glitch into a multi-minute
tool outage even though the POST path was still healthy.
_run_http now wraps all three transport branches (SSE, new + deprecated
Streamable-HTTP) and routes a BaseExceptionGroup through a new
_reconnect_or_reraise_group() helper that returns 'reconnect' (immediate
rebuild, no backoff/park/deregister). It re-raises instead of masking when
shutdown is in progress, when the group carries a real CancelledError
(cancellation must propagate, cf. #9930), or when no live session was
established this attempt (a connect/handshake failure that should fall
through to run()'s backoff rather than hot-loop).
Fixes#66092
Co-authored-by: 雨哥 <hanyu1212@users.noreply.github.com>
Use the direct POSIX parent relationship instead of process creation time and pid_exists checks. Remove the dead create-time argument chain while preserving process-group cleanup and signal forwarding.\n\nRefs #62505
Per the MCP spec the cursor is an opaque string; anything else
(including MagicMock auto-attributes in tests) means no more pages.
Fixes test_mcp_tool_session_expired mock-session runaway.
Port from anomalyco/opencode#35439/#35500: preserve full MCP catalogs
across paginated tools/list responses.
The MCP spec allows servers to paginate tools/list, resources/list, and
prompts/list via an opaque nextCursor token. The Python SDK's
ClientSession.list_* methods fetch exactly one page per call, and hermes
never passed the cursor back — on a paginated server every tool,
resource, and prompt past page 1 was silently invisible to the agent.
Adds _paginate_full_list() (cursor-draining helper with a 50-page
runaway cap and spec-correct opaque-string cursor validation) and
applies it at all three discovery sites: _discover_tools(), the
tools/list_changed refresh handler, and the list_resources/list_prompts
utility handlers. The keepalive probe intentionally keeps its
single-page call (liveness only).
E2E: real stdio MCP server serving 3 pages (2/1/1 tools) — old code
discovered 2 tools, new code discovers all 4.
Follow-ups on top of #64061's salvage:
- ResourceLink markers now point at mcp__<server>__read_resource (the
actual registered tool name via mcp_prefixed_tool_name) instead of a
nonexistent <server>_read_resource the agent could hallucinate-call.
- The isError path now surfaces EmbeddedResource .resource.text blocks
instead of dropping them, so error payloads carried in resources no
longer collapse to a bare 'MCP tool returned an error'. (Same-class
fix flagged in #64061 and independently addressed in #63576 by
@alauer.)
- 3 new error-path tests + updated ResourceLink wire-name assertion.
MCP tool results with non-image binary resources (PDFs, archives, office
docs) were silently dropped: the success path only handled TextContent and
ImageContent, so a PDF-returning MCP tool appeared to return metadata only.
- EmbeddedResource blob contents are decoded (50MB cap), materialized into
the Hermes document cache via cache_document_from_bytes (sanitized
filename, traversal-safe), and surfaced as a local-path marker the agent
can read with file/terminal tools.
- EmbeddedResource text contents are inlined directly.
- ResourceLink blocks preserve the URI and point the agent at the server's
read_resource tool; no arbitrary network fetch outside the MCP session.
- AudioContent blocks are cached via cache_audio_from_bytes as MEDIA: tags.
- read_resource blob contents are materialized the same way instead of
returning '[binary data, N bytes]'.
- Unsupported blocks are logged instead of silently discarded.
- Existing ImageContent MEDIA: behavior unchanged.
Reported by an enterprise customer; reproduced against an HTTP MCP server
returning application/pdf resources.
Two fixes on top of the salvaged parent-death watchdog:
- Apply the watchdog wrap AFTER the OSV malware preflight so the check
inspects the real npx/uvx package instead of the python wrapper
(the wrap previously made the preflight a silent no-op for every
stdio server).
- The real server runs in its own process group under the watchdog, so
the graceful-shutdown killpg no longer reached it; the watchdog now
forwards SIGTERM/SIGINT to the child's group, keeping wedged servers
killable on clean shutdown.
A stdio MCP server (e.g. `npx -y mcp-remote <url>`) is spawned as a direct
child of the Hermes process. Existing teardown (MCPServerTask.shutdown() /
_kill_orphaned_mcp_children()) reaps it correctly on a clean exit, but a
kill -9 / crash / force-quit of the Hermes process skips that path entirely
-- the child (and its own descendants, e.g. mcp-remote's spawned node
process) is orphaned and keeps running. Repeated ungraceful restarts pile up
N orphaned processes racing to hold the same upstream SSE session, producing
errors like 'Invalid request parameters' on legitimate reconnects.
macOS/Linux have no portable equivalent of prctl(PR_SET_PDEATHSIG) at the
Python subprocess level, so this adds a thin supervisor
(tools/mcp_stdio_watchdog.py) that:
- execs the real command as its own child in its own process group
- passes stdin/stdout/stderr through untouched (MCP stdio protocol
talks directly over those streams)
- polls the original spawning PID with the same orphan-detection
algorithm already proven in tui_gateway/slash_worker.py (ppid
comparison + psutil creation-time guard against PID reuse)
- SIGTERM-then-SIGKILL's the child's process group the moment the
original parent is gone
Wired into _run_stdio via a new _wrap_command_with_watchdog() helper,
POSIX-only (matches the existing killpg-based cleanup's platform scope),
fails open (any error resolving pid/create-time falls back to the
unwrapped command) so this can never be the reason a working MCP server
stops starting.
Verified: reproduced the exact orphan scenario standalone (fake parent
process spawns watchdog + fake long-running MCP child, kill -9 the fake
parent, confirm the watchdog reaps the child within its poll window with
zero leaked processes). Updated test_mcp_tool_issue_948.py's resolved-path
assertion to check the watchdog-wrapped command instead of the raw
resolved binary. Full test_mcp_tool.py + test_mcp_stability.py +
test_mcp_tool_issue_948.py suite: 232 passed. Full -k mcp sweep across the
whole test tree: 1003 passed, 2 skipped, 0 failed.
Sibling sites of the same bug class as the salvaged stdio fix:
- SSE, streamable-HTTP (new + deprecated API) initialize() calls are now
bounded by the same connect_timeout, so an endpoint that accepts the
connection but never answers the handshake cannot park the run() task
forever.
- start() now cancels its ensure_future'd run() task when the caller's
connect timeout cancels start() itself — the orphaned-task leak was
the root mechanism behind #59349, and this closes the class for any
future pre-ready hang.
A stdio MCP server that never completes `initialize` (e.g. emits a
non-JSON-RPC frame and then blocks on stdin) leaks a child process plus its
stdio pipes/pidfd on every discovery-retry cycle — unbounded, until the
gateway hits EMFILE and every new open()/spawn fails (#59349).
Root cause (confirmed by instrumenting the live repro, and different from the
issue's own hypothesis): the spawned child IS captured in `new_pids`, so the
report's "new_pids empty at finally" guess is not it. The real cause is that
`session.initialize()` hangs forever on the garbage stream. `connect_timeout`
only bounds the caller's `.result()` wait on the foreground thread — it does
NOT cancel the `_run_stdio` coroutine on the background MCP loop. So the
coroutine is stuck at `await session.initialize()` permanently, its cleanup
`finally` never runs, the child is never reaped, and it stays invisible to the
orphan-reaper (whose `_orphan_stdio_pids` set never gets populated).
Fix: wrap `session.initialize()` in `asyncio.wait_for(..., connect_timeout)`
so a stalled handshake fails instead of hanging. The TimeoutError unwinds
through the SDK context managers (closing the child's stdin -> EOF -> exit)
and lets the existing `finally` reap any straggler. Cross-platform — no
signals/pgid/proc.
Scope: stdio only. The HTTP path has the same `await session.initialize()`
shape but spawns no subprocess (so it can't cause this leak) and already has
httpx transport timeouts.
Verified: the reporter's repro goes from unbounded growth to draining to zero;
added a hermetic regression test (fake transport whose `initialize()` hangs,
asserts the connect is bounded by connect_timeout) that fails on the pre-fix
code and passes on the fix; 566 existing MCP tests pass; ruff clean.
Repro confirmed on macOS (pipe FDs); the Linux-specific pidfd growth in the
report should be equivalent — the reporter offered to validate on Linux.
Closes#59349
Merge the two cherry-picked reap call sites into one unscoped sweep at
the top of _run_stdio (the unscoped sweep is a superset of the
per-server one), and run it via asyncio.to_thread so the 2s
SIGTERM->SIGKILL escalation cannot stall the shared MCP event loop.
GodsBoy
Teknium
kshitij
Yuanang Yang
Stepan Zadolia
Seppe Gadeyne
shady
Atakan
liuhao1024
flyingdoubleg
ygd58
ethernet
SeoYeonKim
Börje
trevorgordon981
yungchentang
hanyu1212
Jupiter363
Bruce-anle
Ben Barclay
Victor Kyriazakos
harjoth
Sage
rainbowgits
yoma