Run reviews without unfinished MCP servers#30500
Open
charliemarsh-oai wants to merge 16 commits into
Open
Conversation
273d908 to
98fe05e
Compare
98fe05e to
d63f5c7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Review child sessions currently take the same MCP initialization path as normal sessions. If the parent session's MCP clients are still starting, launching a review can therefore wait on OAuth discovery and client startup even though the review does not require those unfinished servers.
This PR makes MCP availability a launch-time policy for the review child. We snapshot whether the parent MCP connection manager has finished starting every configured client and pass that decision through the child session and turn context as
McpAccess. A review launched after startup settles keeps the normal MCP path, including when an optional server failed; a review launched while any client is pending uses disabled MCP access for its lifetime.In disabled mode, the child skips MCP auth discovery and connection startup, uses an ephemeral empty MCP runtime for each model step, and suppresses Apps, configured and plugin-provided MCP tools, resource tools, MCP skill-dependency installation, and tool suggestions. Plugin policy hooks remain active, and the child never mutates or replaces the parent's shared MCP runtime. The decision does not change underneath an in-flight review, while a later review can use MCP once parent startup has completed.
Stack
This is PR 1 of 2 and provides the safe core behavior after a review has been launched. On its own, it does not make
/reviewavailable while the TUI reports MCP startup; it only guarantees that any client which launches a review at that point will not wait for unfinished MCP servers.#30509 is PR 2 of 2. It changes the TUI's activity state and command gating so
/reviewcan actually launch while MCP startup remains in the background. It depends on this PR so that launching early produces a review without unfinished MCP rather than a review that still waits for it.