Repository navigation
fix: clear active MCP servers after cleanup - #4586
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ba5bdd0d4
ℹ️ 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: fb7978d967
ℹ️ 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".
Summary
MCPServerManager.cleanup_all()removes servers from the internal connected set, but previously leftactive_serversunchanged. With the defaultdrop_failed_servers=True, callers could therefore receive cleaned or disconnected servers fromactive_serverseven though the manager documents that property as the successfully connected subset.This change consistently derives the active list from the canonical connected-server set and refreshes it in a guaranteed cleanup
finallypath. A subsequentreconnect(failed_only=True)cannot reintroduce a cleaned server without reconnecting it, and propagated cleanup cancellation cannot leave disconnected servers advertised as active.Regression coverage
Added focused tests covering:
connect_all();suppress_cancelled_error=False, verifying the cancellation propagates while active and connected state remain consistent.Full repository tests were not run locally in this environment; CI should run the MCP test suite.