fix(mcp): reset stdio session stack state on cleanup#2461
fix(mcp): reset stdio session stack state on cleanup#2461OiPunk wants to merge 5 commits intoopenai:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47e48f8df3
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cbf14ab51
ℹ️ 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".
|
Good catch. You were right that grouped cancellation could still reset the stack. I pushed d7f3e09 to handle this explicitly:
Local verification:
|
|
Follow-up: CI lint asked for formatting only. I pushed 928f113 (no logic changes) to satisfy ruff format check. |
Summary
This PR hardens MCP server teardown by resetting
_MCPServerWithClientSessionstate aftercleanup().Changes:
exit_stackto a freshAsyncExitStackafter every cleanup.server_initialize_resultduring cleanup, alongsidesession.MCPServerStdioinstance works reliably.Why
When cleanup reuses the same exit stack object across connection lifecycles, callbacks/references from prior connection attempts can be retained longer than needed, especially after recoverable cleanup errors. Resetting teardown state makes MCP stdio lifecycle behavior deterministic and reduces risk of lingering subprocess/stream resources.
Testing
uv run ruff check src/agents/mcp/server.py tests/mcp/test_connect_disconnect.pyenv -u ALL_PROXY -u all_proxy -u HTTPS_PROXY -u https_proxy -u HTTP_PROXY -u http_proxy -u NO_PROXY -u no_proxy uv run pytest tests/mcp/test_connect_disconnect.py tests/mcp/test_server_errors.py -qenv -u ALL_PROXY -u all_proxy -u HTTPS_PROXY -u https_proxy -u HTTP_PROXY -u http_proxy -u NO_PROXY -u no_proxy uv run python -m trace --count --coverdir /tmp/openai_agents_618_trace --module pytest tests/mcp/test_connect_disconnect.py -qIssue: #618