feat(mcp): expose session_id on MCPServerStreamableHttp#2708
feat(mcp): expose session_id on MCPServerStreamableHttp#2708seratch merged 4 commits intoopenai:mainfrom
Conversation
|
This looks safe to add. Can you resolve the conflicts? |
2253918 to
8d3aa15
Compare
db03700 to
a52f729
Compare
|
Thanks for the review @seratch! Conflicts are resolved — the branch is now rebased cleanly on top of the latest |
seratch
left a comment
There was a problem hiding this comment.
Can you fix the lint error?
Run make format-check
uv run ruff format --check
Would reformat: tests/mcp/test_streamable_http_session_id.py
1 file would be reformatted, 499 files already formatted
make: *** [Makefile:12: format-check] Error 1
Error: Process completed with exit code 2.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f173b5281d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Fixed the lint error! |
Capture the get_session_id callback returned by streamablehttp_client in the base connect() method and expose it as a session_id property on MCPServerStreamableHttp. Closes openai#924
f173b52 to
17861ad
Compare
…ion ID Per seratch review: after __aexit__ or a failed connect, session_id was returning a stale ID because cleanup() only cleared self.session but left self._get_session_id intact. Now reset both in the finally block so session_id reliably returns None once disconnected. Also adds a regression test: test_session_id_is_none_after_cleanup.
|
Fixed! |
|
Can you fix this? |
|
Fixed — shortened the docstring to stay within the 100-char limit. |
Summary
Fixes #924.
MCPServerStreamableHttpusesstreamablehttp_clientwhich returns a 3-tuple(read, write, get_session_id). However, theconnect()method in the base class_MCPServerWithClientSessiondiscarded the third element:This made it impossible to retrieve the server-assigned MCP session ID, which is required to resume a stateful session in stateless worker setups (e.g. serverless functions, load-balanced workers).
Changes
src/agents/mcp/server.py_MCPServerWithClientSession.__init__— initializes_get_session_id: GetSessionIdCallback | None = Noneso the attribute is always present on the instance._MCPServerWithClientSession.connect()— captures the callback instead of discarding it:This is backward-compatible: SSE transport returns a 2-tuple, so
restwill be empty and_get_session_idstaysNone.MCPServerStreamableHttp.session_idproperty — new public property that delegates to the callback:Usage example
Tests
Added
tests/mcp/test_streamable_http_session_id.pywith 6 test cases:test_session_id_is_none_before_connectNoneon a fresh instancetest_session_id_returns_none_when_callback_is_noneNonecallback →Nonesession IDtest_session_id_returns_callback_valuetest_session_id_returns_none_when_callback_returns_noneNonepasses throughtest_session_id_reflects_updated_callback_valuetest_connect_captures_get_session_id_callbackconnect()stores the callback from the transport tupleAll 120 existing MCP tests continue to pass.
Notes
_isolated_client_sessionhelper (used for per-call isolated sessions inMCPServerStreamableHttp) intentionally keeps*_— those ephemeral sessions are not meant to be resumed.__all__or public API exports are needed;session_idis accessed directly on the instance.