Skip to content

Comments

fix: await MCP server initialization before returning McpHub instance#11518

Merged
hannesrudolph merged 2 commits intomainfrom
fix/mcp-servers-ready-on-first-turn
Feb 18, 2026
Merged

fix: await MCP server initialization before returning McpHub instance#11518
hannesrudolph merged 2 commits intomainfrom
fix/mcp-servers-ready-on-first-turn

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Feb 17, 2026

Summary

  • MCP tools were unavailable on the first task turn when started via IPC because McpHub's constructor fired initializeGlobalMcpServers() and initializeProjectMcpServers() as fire-and-forget async calls. McpServerManager.getInstance() returned a hub with servers still in "connecting" state.
  • Store the combined initialization promise in McpHub and expose waitUntilReady(), then await it in McpServerManager.getInstance() so the hub is only returned after all servers have connected or timed out.
  • Individual server timeouts are preserved — no risk of blocking indefinitely.

Test plan

  • Verified all 47 existing McpHub.spec.ts tests pass
  • Verified no TypeScript errors in roo-cline package
  • Verify MCP tools appear in the system prompt on the very first API call of a new task started via IPC
  • Verify that if an MCP server fails to connect, the task still starts after the server's configured timeout

🤖 Generated with Claude Code

Start a new Roo Code Cloud session on this branch

MCP tools were unavailable on the first task turn when started via IPC
because McpHub's constructor fired initializeGlobalMcpServers() and
initializeProjectMcpServers() without awaiting them. getInstance()
returned a hub with servers still in "connecting" state.

Store the combined initialization promise and expose waitUntilReady(),
then await it in McpServerManager.getInstance() so the hub is only
returned after all servers have connected or timed out.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Feb 17, 2026
@roomote
Copy link
Contributor

roomote bot commented Feb 17, 2026

Rooviewer Clock   See task

All previously flagged issues have been resolved. No new issues found.

  • Race condition in McpServerManager.getInstance(): this.instance is set before waitUntilReady() completes, so concurrent callers bypass the wait and receive a not-yet-ready hub
Previous reviews

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

@roomote
Copy link
Contributor

roomote bot commented Feb 17, 2026

Fixaroo Clock   See task

Fixed the race condition by assigning this.instance only after waitUntilReady() resolves. All local checks passed (lint, type-check, 47 McpHub tests).

View commit | Revert commit

Closes race condition where concurrent callers of getInstance() could
receive a hub that has not finished initialization. The hub is now
created in a local variable and only assigned to this.instance after
waitUntilReady() completes.
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 18, 2026
@hannesrudolph hannesrudolph merged commit bfbfaf6 into main Feb 18, 2026
10 checks passed
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Feb 18, 2026
@hannesrudolph hannesrudolph deleted the fix/mcp-servers-ready-on-first-turn branch February 18, 2026 00:32
@roomote roomote bot mentioned this pull request Feb 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants