feat(client): store InitializeResult as initialize_result#2300
feat(client): store InitializeResult as initialize_result#2300
Conversation
915a21c to
3455447
Compare
There was a problem hiding this comment.
LGTM — clean, minimal API change with good test coverage and migration docs.
Extended reasoning...
Overview
This PR replaces ClientSession.get_server_capabilities() (method returning ServerCapabilities | None) and Client.server_capabilities with a single initialize_result property that stores the full InitializeResult. Changes span 6 files: the two client modules (client.py, session.py), their corresponding tests, and the migration guide.
Security risks
None. This is a pure API surface change — no auth, crypto, permissions, or external I/O is affected. The stored InitializeResult was already returned from initialize() and available to callers; this just retains it on the session object.
Level of scrutiny
Low scrutiny warranted. The change is mechanical: store the whole result instead of destructuring one field, expose it as a property, update all call sites. The logic delta is minimal — the only new code path is Client.initialize_result which proxies through session.initialize_result with a None guard (marked pragma: no cover since __aenter__ guarantees initialization).
Other factors
- All old API references (
get_server_capabilities,server_capabilities,_server_capabilities) are fully removed fromsrc/— grep confirms no stale references remain. - Tests cover both
None-before-init and all four fields after init, plus theClient-level property. - Migration docs are clear with before/after examples.
- No CODEOWNERS file exists for these paths.
- The naming aligns with Go SDK and the type name (
InitializeResult), which is well-reasoned.
ClientSession now stores the complete InitializeResult via a server_params property, mirroring ServerSession.client_params. This provides access to server_info, capabilities, instructions, and the negotiated protocol_version through a single, future-proof property. Previously, only capabilities were stored (via _server_capabilities) and users had no access to server_info, instructions, or protocol_version after initialize() returned — they had to manually capture the return value. get_server_capabilities() and Client.server_capabilities have been removed. Github-Issue: #1018
…rties - ClientSession.server_params -> initialize_result (avoids collision with StdioServerParameters idiom; matches Go SDK and FastMCP) - Client: replace server_params proxy with non-nullable server_capabilities, server_info, server_instructions (init is guaranteed inside the context manager, so | None was unreachable) - Client.server_capabilities is preserved from v1 with a better type Github-Issue: #1018
- Remove _initialize_result captured state (tracked same invariant as _session) - Remove server_capabilities/server_info/server_instructions decomposed properties - Client.initialize_result proxies through session, non-nullable - Unreachable None branch marked pragma: no cover (self.session raises first) server_capabilities had no organic usage beyond a did-init-happen probe; every other result type (CallToolResult, etc.) comes back whole. Github-Issue: #1018
Main added 11 client.server_capabilities callsites while this branch was in flight. Each was a 2-line narrowing-check-then-real-check pair; collapses to 1 line since initialize_result is non-nullable. Github-Issue: #1018
eb92227 to
24217ae
Compare
There was a problem hiding this comment.
No bugs found, but this PR removes public API (get_server_capabilities(), server_capabilities) in favor of a new initialize_result property — a breaking change that warrants a maintainer sign-off on the API design.
Extended reasoning...
Overview
This PR modifies ClientSession and Client to store the full InitializeResult instead of only ServerCapabilities. It removes ClientSession.get_server_capabilities() and Client.server_capabilities, replacing them with an initialize_result property. Seven files are touched: the two source files, the migration guide, and four test files.
Security risks
No security risks. The changes are purely about how initialization metadata is stored and exposed — no auth, crypto, or permission logic is affected.
Level of scrutiny
This is a public API breaking change: two previously-available accessors are removed and replaced with a differently-shaped property. While the implementation is clean and well-tested, the API design decision (single initialize_result property vs. decomposed accessors, naming, nullability semantics) is the kind of thing a project maintainer should explicitly approve.
Other factors
Tests are comprehensive — test_initialize_result covers the None-before-init and all-four-fields-after-init cases. All existing test call sites have been updated. The migration guide documents the change clearly. No remaining references to the old API exist in src/. The code is straightforward with no tricky logic.
ClientSessionnow stores the fullInitializeResultvia aninitialize_resultproperty.Clientexposes the same property, non-nullable.Closes #1018. Supersedes #1565 and #2211.
Motivation and Context
Previously,
ClientSessiondestructured onlyresult.capabilitiesand discardedserver_info,instructions, andprotocol_version— users had to manually capture the return value ofinitialize(), whichClient.__aenter__throws away.ClientSession.initialize_result -> InitializeResult | NoneStores the whole
InitializeResult. The name matches the type and the method that populates it (initialize()→initialize_result). Go's SDK calls thisInitializeResult(); FastMCP calls itinitialize_result.Client.initialize_result -> InitializeResultSame property, non-nullable —
__aenter__callsinitialize()before returning, so the result is guaranteed inside the context manager. Proxies throughself.session.initialize_result; no captured state. RaisesRuntimeErroroutside the context (via the existingsessionproperty guard).How Has This Been Tested?
test_initialize_resultassertsNonebefore init and all four fields aftertest_client_is_initializedassertsclient.initialize_result.capabilitiesand.server_info.name./scripts/test— 100% coverage, strict-no-cover cleanpyright— 0 errorsBreaking Changes
ClientSession.get_server_capabilities()removed → usesession.initialize_result.capabilitiesClient.server_capabilitiesremoved → useclient.initialize_result.capabilitiesTypes of changes
Checklist
Additional context
Why not
server_params? Collides withserver_params = StdioServerParameters(...)— the established idiom in README, examples, andClientSessionGroup'sServerParameterstype alias. Also:InitializeResultis aResultsubclass, not aParams— the wire shape is{"result":{...}}, not{"params":{...}}.Why not decompose into
server_capabilities/server_info/server_instructionsonClient? It would have been the only result typeClientcracks open (CallToolResultetc. all come back whole), introduced a 3-of-4 curation (where'sprotocol_version?), and the decomposition had no organic callers — the only pre-PR usage was aserver_capabilities is not Noneprobe.AI Disclaimer