fix(mcp): show startup progress as each server connects - #5897
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_d0f84519-0fd0-49de-8b93-d0eea008f5e7) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e818c6b8-8f33-4b8e-91f9-f57adb9ba9ae) |
|
Claude finished @Hmbown's task in 2m 35s —— View job Review of #5897 — show MCP startup progress as each server connects
SummaryThe core change is sound. The new test Plugin relocation ( No provider/model-prefix routing code is touched by this PR — the EPIC #2608 invariant doesn't apply here. Minor / non-blocking notes
Nothing else stood out as a correctness, race-condition, or safety issue. I was not able to run |
There was a problem hiding this comment.
🔵 Needs a closer look
It changes concurrency/timing in MCP boot and tool-pool visibility, which is correctness-sensitive and best validated with final human review despite good targeted regression coverage.
Pull request overview
Improves MCP session boot responsiveness by applying connection results as each server finishes handshaking (instead of buffering until the slowest finishes), so the TUI’s “connecting” count and available tool pool reflect real-time progress during startup.
Changes:
- Replaced the “connect and await all” helper with
spawn_pending_connects, returning a bounded-concurrencyJoinSetthat callers canjoin_next()incrementally. - Updated MCP session boot (and pool connect flow) to store each successful connection under a short lock and emit per-server progress updates immediately as each task completes.
- Added an engine integration test that stalls one stdio server during
initializeand asserts the fast server becomes visible (and connecting count drops) before the slow server is released.
File summaries
| File | Description |
|---|---|
| crates/tui/src/mcp.rs | Introduces spawn_pending_connects (returns JoinSet) and adjusts connect flow to consume results incrementally while preserving concurrency limits and error behavior. |
| crates/tui/src/core/engine.rs | Updates MCP session boot to join_next() each connection task, update the shared pool under a short lock, and emit per-completion progress events. |
| crates/tui/src/core/engine/tests.rs | Adds a regression test ensuring mid-boot snapshots show early-ready servers even if another server stalls. |
Review details
- Files reviewed: 8/32 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![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: 4530a19ceb
ℹ️ 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".
| "fast": { "command": "node", "args": [server, "fast", release] }, | ||
| "slow": { "command": "node", "args": [server, "slow", release] } |
There was a problem hiding this comment.
On Rust-only or minimal build hosts where node is not on PATH, both fixture connections fail to spawn, so the progress loop consumes the finished event, waits for its 10-second timeout, and the subsequent finished loop waits another 10 seconds before the test fails. Node is not a required dependency for Codewhale, and the existing Node-backed test in crates/tui/src/mcp/tests.rs explicitly checks availability and skips when absent; add the same guard here or use a self-contained test server so the TUI library suite remains portable.
Useful? React with 👍 / 👎.
| { | ||
| let mut pool = pool_for_task.lock().await; | ||
| match result { | ||
| Ok(connection) => pool.store_ready_connection(name, connection), |
There was a problem hiding this comment.
If the MCP config is edited while a slow startup connection is pending, an already-ready server exposed by this change can be invoked and cause get_or_connect to reload the pool and drop connections under the old config; when the slow task later completes, this unconditional store re-inserts its stale connection and store_ready_connection stamps it with the new catalog generation. Because all_tools projects stored connections and a subsequent get_or_connect returns a ready connection without checking that its server is still configured or enabled, a removed, disabled, or authority-changed server can remain advertised and executable. Reject completions whose captured generation/config authority is stale, or reconcile the config before inserting each result.
AGENTS.md reference: AGENTS.md:L28-L29
Useful? React with 👍 / 👎.
| let _ = progress_tx.send(McpBootUpdate::Progress { | ||
| generation, |
There was a problem hiding this comment.
When one server completes between user turns while at least one slower server is still connecting, this progress path makes the newly ready server's tools appear in the next turn but does not set pending_prefix_change_reason; that reason is set only by the later Finished update. The next request therefore changes the session-pinned tools array as undeclared drift, which trips the C5 debug_assert! in debug builds and records an avoidable KV-cache miss in release builds. Either keep the MCP catalog frozen until boot finishes or stamp a declared prefix change whenever a progress update exposes new tools.
AGENTS.md reference: AGENTS.md:L136-L139
Useful? React with 👍 / 👎.
![codewhale-agent[bot]](https://avatars.githubusercontent.com/u/101357273?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codewhale review
PR replaces bulk MCP connect collection with per-completion JoinSet processing so the engine emits progress and stores ready connections as each server finishes, and relocates the computer-use plugin bundle to crates/tui/plugins. The change is generally well-formed; the main concerns are panic bookkeeping and test coverage for tool availability.
Findings
- [WARNING] Panicked connect tasks leave the real server in the connecting list (
crates/tui/src/core/engine.rs:6497)
When a spawned connect task panics, join_next returns a JoinError and the code synthesizes the name "connection task".remaining.retain(|pending_name| pending_name != &name)does not remove the actual pending server name, so subsequent McpBootUpdate::Progress events can continue to report that server as connecting and connection_errors is keyed under the synthetic name. This was present before, but the new per-completion progress makes the stale UI state directly visible.
Suggestions
crates/tui/src/core/engine/tests.rs— Extend the new mcp_boot_reports_ready_server_before_stalled_server_finishes test to also assert that the fast server's tools are already available in the MCP pool before the slow server is released. The current snapshot assertion proves the progress list is fixed, but not the claimed tool-availability behavior.
Assessment
The core change correctly removes the slowest-server bottleneck and keeps the eight-connection limit intact. I would like the panicked-task bookkeeping fixed and the regression test extended to verify ready tools are admitted before slow servers finish; otherwise the implementation matches the stated goals.
Advisory review by Codewhale (codewhale review --pr 5897 --post, head d4e6c868188d67c31ddef090cee7889783a3ff1d). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| while let Some(joined) = connects.join_next().await { | ||
| let (name, result) = joined | ||
| .unwrap_or_else(|error| ("connection task".to_string(), Err(error.into()))); | ||
| remaining.retain(|pending_name| pending_name != &name); |
There was a problem hiding this comment.
[WARNING] Panicked connect tasks leave the real server in the connecting list
When a spawned connect task panics, join_next returns a JoinError and the code synthesizes the name "connection task". remaining.retain(|pending_name| pending_name != &name) does not remove the actual pending server name, so subsequent McpBootUpdate::Progress events can continue to report that server as connecting and connection_errors is keyed under the synthetic name. This was present before, but the new per-completion progress makes the stale UI state directly visible.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_d3c1fa23-39a5-490f-989b-2dbfdd34d291) |
|
Claude finished @Hmbown's task in 3m 33s —— View job Review of #5897 — show MCP startup progress as each server connects
SummaryNote: The newest commit, 9d05c12 "reject stale boot connections and declare live catalog changes," directly answers every substantive finding raised by the earlier automated reviews on this PR:
I re-read these fixes against their call sites rather than trusting the commit message; the generation/authority checks line up correctly (no intervening Minor / non-blocking
No correctness, race, or panic issues found beyond the above. Provider/model routing (EPIC #2608 invariant) is untouched by this PR — confirmed via |
|
Review follow-up is pushed at 9d05c12. Background completion re-reads configuration, cancels obsolete batches, and refuses to re-stamp stale connection generations. Plugin authority is checked before insertion. Progress application, queue draining, and the in-flight tool snapshot all declare the catalog refresh. Handshake panics retain their server name, and Node fixtures skip cleanly when Node is unavailable. 160 focused tests pass, including ready tool availability before a stalled handshake completes, config removal/disable during that handshake, stale generation rejection, and prefix declarations. Cargo check, clippy with warnings denied, formatting, and diff checks pass. Hosted CI is running on this new head. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2f6d1501-7015-41e6-8320-0278981b398d) |
|
Claude finished @Hmbown's task in 1m 56s —— View job Review of #5897 — show MCP startup progress as each server connects
SummaryDiffed
I also independently re-verified (by reading the actual diffs, not just trusting the prior comments) that
Plugin relocation ( No provider/model-prefix routing code is touched ( Minor / still open, non-blocking
VerdictNothing new to flag. This round's only change ( |
There was a problem hiding this comment.
Codewhale review
PR #5897 replaces the batched MCP connection wait with a JoinSet consumed as tasks complete, so progress events and pool tools are emitted as each server finishes. It also tightens stale-generation handling in store_ready_connection and updates computer-use bundle paths. The core approach is sound and the added tests cover fast-before-slow visibility and stale pool insertion, but two concurrency/bookkeeping risks remain.
Findings
- [WARNING] Pool mutex is held across an await in the incremental boot loop (
crates/tui/src/core/engine.rs:6506)
The engine boot task locks pool_for_task and then awaits pool.reload_if_config_changed() while still holding the guard. This can block mcp_tools() snapshots, TUI progress reads, and other pool users for filesystem/latency, which contradicts the short-lock intent and can stall UI during config reload. - [WARNING] Explicit connect-all records stale config errors as connection failures (
crates/tui/src/mcp.rs:3214)
In McpPool::connect_all_servers, a result rejected by store_ready_connection because the catalog generation changed is passed to note_connect_failure and pushed into the returned errors. That pollutes per-server backoff/auth state for servers that may be valid under the newly reloaded config; stale successes should be ignored or remaining connects aborted, as the engine boot loop does.
Assessment
The implementation is close and the test coverage is good, but the async lock hold should be scoped down and the connect-all stale-result path should avoid recording false failures before merge.
Advisory review by Codewhale (codewhale review --pr 5897 --post, head c92ebf795953318162b77d68d155f132957bbde2). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| // A turn may have reloaded the pool while these handshakes | ||
| // were in flight. Never let their old authority or failures | ||
| // overwrite the newly installed configuration. | ||
| let reload = pool.reload_if_config_changed().await; |
There was a problem hiding this comment.
[WARNING] Pool mutex is held across an await in the incremental boot loop
The engine boot task locks pool_for_task and then awaits pool.reload_if_config_changed() while still holding the guard. This can block mcp_tools() snapshots, TUI progress reads, and other pool users for filesystem/latency, which contradicts the short-lock intent and can stall UI during config reload.
There was a problem hiding this comment.
Checked the callee: reload_if_config_changed is an async wrapper whose body only calls the synchronous reload_from_config_sources(false); it contains no await and performs no handshake/network request. Its future does not park while holding the guard. The mtime/config check is intentional at insertion so a stale handshake cannot restore revoked configuration. The existing reload implementation uses synchronous filesystem reads under the same pool authority; moving those reads off-thread would be a separate reload refactor, not a correction to the claimed suspended-await behavior. The delayed-server test verifies ready tools remain available while another handshake is stalled.
| let result = result | ||
| .and_then(|connection| self.store_ready_connection(name.clone(), connection)); | ||
| if let Err(error) = result { | ||
| self.note_connect_failure(&name, &error); |
There was a problem hiding this comment.
[WARNING] Explicit connect-all records stale config errors as connection failures
In McpPool::connect_all_servers, a result rejected by store_ready_connection because the catalog generation changed is passed to note_connect_failure and pushed into the returned errors. That pollutes per-server backoff/auth state for servers that may be valid under the newly reloaded config; stale successes should be ignored or remaining connects aborted, as the engine boot loop does.
There was a problem hiding this comment.
In connect_all, &mut self is held for the complete batch. The catalog-generation atomic belongs to this pool (it is not a cloned Arc), and its mutation methods require borrowing this same pool; another safe caller cannot mutate it concurrently with that exclusive borrow. The config reload occurs after the batch, and that existing path clears the old pass's errors before retrying. The background engine pass is the path that releases the pool between handshakes, and it now explicitly rejects/cancels obsolete batches before recording success or failure. I do not see a reachable stale-generation/backoff case in the exclusive connect_all path.
c92ebf7 to
25efa6f
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2ffd3c0e-acfc-4d0c-abfb-8e04cfbd4094) |
|
Claude finished @Hmbown's task in 2m 10s —— View job Review of #5897 — show MCP startup progress as each server connects
Summary
Confirmed still correct:
Re-checked the two open codewhale-agent WARNINGs and agree with the author's rebuttals:
Still open, non-blocking (unchanged from prior rounds, already acknowledged by the author):
No provider/model-prefix routing code is touched (checked for I could not run VerdictNo new or unresolved correctness issues found on this head. The substantive P1s from earlier rounds (stale-connection re-insertion, undeclared prefix drift, panic name loss) are genuinely fixed with direct test coverage, and the two most recent WARNINGs don't hold up against the actual (non-yielding reload, exclusive-borrow |
There was a problem hiding this comment.
Codewhale review
The PR replaces batch MCP connection boot with an incremental JoinSet loop so ready servers become visible and usable as soon as their handshake completes. It also hardens store_ready_connection against stale generation and invalid plugin authority, and marks the KV-cache prefix refresh earlier. The added fixtures cover the main boot-ordering regression and stale-generation behavior.
Findings
- [WARNING] MCP pool lock is held across an awaited config reload (
crates/tui/src/core/engine.rs:6505)
In the new per-completion boot loop,pool_for_task.lock().awaitis acquired and thenpool.reload_if_config_changed().awaitis awaited while still holding the guard. A config reload can perform file I/O or other work, so this may block all MCP snapshots and tool reads for the duration of the reload, working against the intended short lock scope. - [INFO] store_ready_connection now depends on callers stamping catalog_generation (
crates/tui/src/mcp.rs:2954)
The function no longer setsconnection.catalog_generationinternally and instead rejects mismatches. The visible production call sites are updated, but the invariant is now enforced only at runtime, so any future or missed caller that passes an unstamped connection will fail with a generation error rather than a compile-time error. - [INFO] Incremental connect_all and JoinError/panic attribution lack focused tests (
crates/tui/src/mcp/tests.rs)
The diff changesconnect_allto consume joins incrementally and routes task panics throughspawn_pending_connects, but no new test appears to exercise the explicit connect-all path, its generation/stale rejection behavior, or the panic-to-error JoinSet attribution added here.
Assessment
The change is broadly correct and directly addresses the startup batching defect with meaningful integration coverage. The main concern is the pool lock being held across an async reload; no blocking correctness issue was identified in the diff.
Advisory review by Codewhale (codewhale review --pr 5897 --post, head 25efa6fe87e1959ea9852f820e4a9a5a8e57101e). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| let mut pool = pool_for_task.lock().await; | ||
| // A turn may have reloaded the pool while these handshakes | ||
| // were in flight. Never let their old authority or failures | ||
| // overwrite the newly installed configuration. |
There was a problem hiding this comment.
[WARNING] MCP pool lock is held across an awaited config reload
In the new per-completion boot loop, pool_for_task.lock().await is acquired and then pool.reload_if_config_changed().await is awaited while still holding the guard. A config reload can perform file I/O or other work, so this may block all MCP snapshots and tool reads for the duration of the reload, working against the intended short lock scope.
| // and the cooldown with it. | ||
| self.connect_backoff.remove(&name); | ||
| if self.needs_auth_servers.remove(&name) { | ||
| self.needs_auth_generation = self.needs_auth_generation.wrapping_add(1); |
There was a problem hiding this comment.
[INFO] store_ready_connection now depends on callers stamping catalog_generation
The function no longer sets connection.catalog_generation internally and instead rejects mismatches. The visible production call sites are updated, but the invariant is now enforced only at runtime, so any future or missed caller that passes an unstamped connection will fail with a generation error rather than a compile-time error.
Addresses #5887. Return the owned concurrent connection tasks to callers and consume each completion immediately. Keep the eight-connection limit, short pool locks, catalog generation checks, required-server errors, and cancellation-on-drop. Validation: 157/157 focused nextest tests passed with repository-standard RUST_MIN_STACK=16777216; includes a real engine event-loop fixture holding one server while the ready server becomes visible. cargo check, Clippy -D warnings, and fmt passed. An earlier broad libtest filter hit the unrelated setup test stack overflow under the default stack; no full workspace gate claimed. Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Revalidate config before each background completion, reject obsolete connection generations, and check plugin authority before insertion. Declare boot catalog changes at progress and tool snapshot boundaries; retain server names when handshakes panic. Validation: 160 focused nextest tests passed; cargo check and clippy -D warnings passed; cargo fmt and git diff --check passed. Node fixture verifies tools are usable before the stalled handshake completes and removed/disabled servers cannot be restored. Runtime root has no npm test/check:web scripts. Signed-off-by: CodeWhale Bot <bot@codewhale.net>
CI correctly rejected the stderr diagnostic under clippy::print_stderr. Replace it with tracing. Validation: cargo clippy --workspace --all-targets --all-features --locked -- -D warnings passed; cargo fmt and diff checks passed. The preceding behavioral test run passed 160/160. Signed-off-by: CodeWhale Bot <bot@codewhale.net>
25efa6f to
d716c0c
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_45ff8562-bf7b-4109-a49e-0fe7c4258c2c) |
|
Claude finished @Hmbown's task in 1m 50s —— View job Review of #5897 — show MCP startup progress as each server connects
Summary
Confirmed correct, with direct test coverage:
No provider/model-prefix routing code is touched by this diff (checked for Still open, non-blocking
VerdictNo new or unresolved correctness issues on this head. The substantive P1s raised earlier in this PR's review history (stale-connection re-insertion, undeclared prefix drift, panic name loss, portable Node-fixture skip) are genuinely fixed with direct test coverage rather than just asserted, and the two most recent WARNINGs (lock-across-await, connect_all stale-error recording) don't hold up against the actual code paths. I was unable to run |
There was a problem hiding this comment.
Codewhale review
PR changes MCP boot to consume connection tasks as they complete, storing ready connections incrementally and emitting progress. The core direction is sound; one edge case leaves the finished boot event reporting aborted connections after a config reload.
Findings
- [WARNING] Finished boot event can report aborted servers as still connecting after config reload (
crates/tui/src/core/engine.rs:6510)
When reload_if_config_changed detects a changed catalog or reload error, the task aborts remaining connects and breaks. The localremainingVec is not cleared, so a subsequent Finished McpSessionBoot may include those aborted server names inconnectingeven though they were cancelled. The new testmcp_boot_does_not_restore_servers_removed_during_handshakedoes not assert final connecting list, so this is not caught. Clearremainingwhen aborting all pending connects. - [INFO] Config reload during boot is only detected when a connection completes (
crates/tui/src/core/engine.rs:6506)
The reload/generation check happens inside thejoin_nextloop, after at least one connection task has completed. If all pending handshakes are stalled, a mid-boot config reload cannot abort the remaining tasks until one handshake finishes or times out. This may delay applying the new config for the full connect timeout. Consider a separate wake/select mechanism if reloads must abort immediately.
Suggestions
-
crates/tui/src/core/engine.rs:6510— Clear the remaining connection list when pending connects are aborted, so the Finished boot event does not report aborted servers as still connecting.connects.abort_all(); connection_errors.clear(); remaining.clear();
Assessment
The incremental connect handling is a good improvement and includes focused regression coverage. The main correctness gap is the stale connecting list after reload/abort; clearing it and adding a test assertion would make the change robust.
Advisory review by Codewhale (codewhale review --pr 5897 --post, head d716c0c8aa0c45b44ed966f6d64e57c9ee5516bd). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| if reload.is_err() | ||
| || pool.current_catalog_generation() != catalog_generation | ||
| { | ||
| connects.abort_all(); |
There was a problem hiding this comment.
[WARNING] Finished boot event can report aborted servers as still connecting after config reload
When reload_if_config_changed detects a changed catalog or reload error, the task aborts remaining connects and breaks. The local remaining Vec is not cleared, so a subsequent Finished McpSessionBoot may include those aborted server names in connecting even though they were cancelled. The new test mcp_boot_does_not_restore_servers_removed_during_handshake does not assert final connecting list, so this is not caught. Clear remaining when aborting all pending connects.
| // A turn may have reloaded the pool while these handshakes | ||
| // were in flight. Never let their old authority or failures | ||
| // overwrite the newly installed configuration. | ||
| let reload = pool.reload_if_config_changed().await; |
There was a problem hiding this comment.
[INFO] Config reload during boot is only detected when a connection completes
The reload/generation check happens inside the join_next loop, after at least one connection task has completed. If all pending handshakes are stalled, a mid-boot config reload cannot abort the remaining tasks until one handshake finishes or times out. This may delay applying the new config for the full connect timeout. Consider a separate wake/select mechanism if reloads must abort immediately.
| connects.abort_all(); | ||
| connection_errors.clear(); |
There was a problem hiding this comment.
Clear the remaining connection list when pending connects are aborted, so the Finished boot event does not report aborted servers as still connecting.
| connects.abort_all(); | |
| connection_errors.clear(); | |
| connects.abort_all(); | |
| connection_errors.clear(); | |
| remaining.clear(); |
Startup buffered every MCP connection result until the slowest batch finished, so the TUI could keep showing “20 connecting” even after individual servers were ready. Ready tools were also unavailable in the pool during that wait.
Consume each owned connection task as it completes, update the engine-owned pool under a short lock, and emit per-server progress immediately. Both startup and explicit connect-all use the same bounded task set. Preserve the eight-connection limit, catalog generations, plugin-authority validation, required-server errors, and cancellation when the task set is dropped.
Fixes #5887. The deterministic fixture proves this startup batching defect; it does not reproduce every server in the founder’s private configuration.
Validation: 157/157 focused MCP and engine nextest tests passed with the documented 16 MiB test stack; cargo check, Clippy with -D warnings, and formatting pass. A real engine event-loop fixture holds one stdio server until a release file is written and verifies that the other server is already visible and the connecting count has dropped before that release. Boot completion, retry ownership, stale-generation handling, catalog and authority tests pass.
An initial broad libtest substring filter also selected an unrelated setup test and aborted on its default-stack overflow; the focused run uses the repository’s documented stack setting. Full workspace tests were not run locally.
Based on the bundle relocation in #5890. This PR is v0.9.13 work and does not retag or republish v0.9.12.
Note
Medium Risk
Changes MCP connection lifecycle, config reload races during concurrent boot, and LLM tool-catalog/prefix pinning timing—important for correctness but covered by focused integration tests.
Overview
MCP session boot no longer waits for the slowest handshake before updating the UI or tool catalog. The engine now consumes
spawn_pending_connectstasks as each finishes, stores ready connections under a short pool lock, and emitsMcpSessionBootprogress with an updated connecting list after every completion instead of buffering until the batch ends.Ready tools are exposed while optional servers are still connecting, and
pending_prefix_change_reasonis set tomcp-session-booton progress, finished boot, and earlymcp_tools()snapshots so the KV prefix refresh is scheduled even when a connection becomes ready before its mailbox event.store_ready_connectionnow returnsResultand rejects connections whose catalog generation no longer matches the pool (with plugin validation on store). Mid-boot config reloads callreload_if_config_changed, abort remaining connect tasks, and avoid letting stale handshakes overwrite the new configuration. Connect tasks are wrapped with panic catching viaFutureExt.New engine and pool tests cover incremental boot visibility, config invalidation during handshake, and prefix-change ordering relative to progress events.
Reviewed by Cursor Bugbot for commit d716c0c. Bugbot is set up for automated code reviews on this repo. Configure here.