Conversation
ClientSessionGroup._aggregate_components deleted the session from _session_exit_stacks whenever a server exposed no prompts, resources, or tools. Via connect_with_session (caller-supplied session, never registered) this raised KeyError; via connect_to_server it dropped the freshly-registered exit stack so a later disconnect_from_server could not close the transport. Remove the erroneous cleanup so empty servers are aggregated like any other, and add regression tests for both entry points.
|
This PR has been closed automatically. This repo only keeps pull requests open when they come from a maintainer, or from a contributor a maintainer has assigned to the linked issue, and you aren't currently assigned to #3578. If a maintainer assigns you to #3578, this PR reopens on its own and there's nothing more you need to do here. Assignment is a maintainer call based on capacity; comments that only ask to be assigned don't factor in. What does help is engaging on the issue itself by confirming the repro, explaining why it matters for your use case, or describing the approach you'd take. You're welcome to keep pushing commits here (just avoid force-pushing, since GitHub can't reopen a rewritten branch), but that on its own won't get the PR reviewed or the issue assigned, and realistically most auto-closed PRs stay closed. There's no need to open a new PR either way. CONTRIBUTING.md has the full reasoning, but in short:
Maintainers: reopen, remove |
Fixes #3578
ClientSessionGroup._aggregate_componentsdeleted the session from_session_exit_stackswhenever a server exposed no prompts, resources, or tools. Viaconnect_with_session(caller-supplied session, never registered) this raisedKeyError; viaconnect_to_serverit dropped the freshly-registered exit stack so a laterdisconnect_from_servercould not close the transport.Remove the erroneous cleanup so empty servers are aggregated like any other, and add regression tests for both entry points.
Motivation and Context
A valid MCP server can register zero tools/resources/prompts (e.g. one that only does sampling/roots), and swallowed
list_*failures leave the temp dicts empty too. In those cases the empty-server cleanup block ran and either crashed the connect (KeyErrorviaconnect_with_session) or dropped the session's exit stack so its transport leaked until group teardown (connect_to_server). The block was marked# pragma: no cover, so this path had no test coverage.How Has This Been Tested?
connect_with_sessionwith an empty server: it no longer raises, the session is tracked, and its components stay empty.connect_to_serverwith an empty server: the exit stack is retained and a laterdisconnect_from_servercloses it.AGENTS.md:pytest,ruff format,ruff check,pyright, 100% coverage onsrc/mcp/client/session_group.py, andstrict-no-cover. All green.Breaking Changes
None. Removes an internal cleanup block that only ran on the empty-server edge case; no public signature or documented behavior changes.
Types of changes
Checklist
help wanted, or I'm a maintainer)Additional context
The removed block also let me drop two now-covered
# pragma: no covermarkers on the stack-close lines indisconnect_from_server, since the new test exercises them.