Skip to content

fix(mcp): Revert changes causing startup issues and remove refresh notifications #6878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Aug 9, 2025
Merged

fix(mcp): Revert changes causing startup issues and remove refresh notifications #6878

merged 4 commits into from
Aug 9, 2025

Conversation

@hannesrudolph
Copy link
Collaborator

@hannesrudolph hannesrudolph commented Aug 9, 2025 •
edited
Loading

Description

This PR reverts the optimization that caused MCP servers to fail on startup and permanently removes the MCP refresh info notifications from the extension logic. The UI already provides a green indicator for MCP status, so duplicative VS Code popups are unnecessary.

Problem

PR #6779 introduced an optimization to avoid unnecessary MCP server refreshes on settings save, which led to MCP servers failing on startup.

Solution

  1. Revert the problematic change from PR fix: prevent unnecessary MCP server refresh on settings save (#6772) #6779 to restore stable MCP startup.

  2. Remove all VS Code information notifications related to MCP refresh in the backend:

    • "already refreshing"
    • "refreshing all"
    • "active global/project servers"
    • "all refreshed"
      The UI status indicator remains the single source of truth for user feedback.

  3. Restore test coverage for the webview message handler MCP toggle path to prevent regressions.

Changes Made

Rationale

  • Reduces user-facing noise and avoids redundant UX (green UI indicator suffices)
  • Simplifies backend logic, eliminating temporary guards/comments

Testing

  • Unit tests:
    • cd src && npx vitest run core/webview/tests/webviewMessageHandler.spec.ts
    • cd src && npx vitest run services/mcp
  • CI:
    • All checks passing (Analyze, CodeQL, compile, unit tests, integration tests, translations, knip)

Checklist

  • MCP servers start successfully
  • UI indicator remains the only user-facing refresh signal
  • Tests updated and passing
  • No user-facing regressions

@hannesrudolph
Revert "fix: prevent unnecessary MCP server refresh on settings save (#…
eceda36 
…6772) (#6779)"

This reverts commit 8d05bc1.
@hannesrudolph
fix(mcp): Revert changes causing startup issues and temporarily disab…
949c90f 
…le notifications

- Reverted PR #6779 which prevented unnecessary MCP server refreshes but caused startup failures
- Temporarily disabled MCP notification popups as a stopgap solution
- Added TODO comments explaining the temporary nature of disabled notifications
- This allows MCP servers to function properly while a more robust solution is developed
Copilot AI review requested due to automatic review settings August 9, 2025 17:27
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Aug 9, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Aug 9, 2025
roomote[bot]
roomote bot reviewed Aug 9, 2025
View reviewed changes
Copy link
Contributor

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR! The revert approach is a pragmatic solution to fix the MCP startup issues. I've reviewed the changes and have some suggestions for improvement.

Copilot AI reviewed Aug 9, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@hannesrudolph hannesrudolph changed the title fix(mcp): Revert changes causing startup issues and temporarily disable notifications fix(mcp): Revert changes causing startup issues and disable notifications Aug 9, 2025
@hannesrudolph hannesrudolph changed the title fix(mcp): Revert changes causing startup issues and disable notifications fix(mcp): Revert changes causing startup issues and remove refresh notifications Aug 9, 2025
@hannesrudolph hannesrudolph moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Aug 9, 2025
@mrubens mrubens merged commit 75b861c into main Aug 9, 2025
10 checks passed
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 9, 2025
@mrubens mrubens deleted the fix/mcp-startup-issues-revert branch August 9, 2025 18:28
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Aug 9, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Aug 9, 2025
@roomote roomote bot mentioned this pull request Aug 9, 2025
Closed
@daniel-lxs daniel-lxs mentioned this pull request Aug 12, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@roomote roomote[bot] roomote[bot] left review comments

@mrubens mrubens mrubens approved these changes

@cte cte Awaiting requested review from cte cte is a code owner

@jr jr Awaiting requested review from jr jr is a code owner

Assignees

No one assigned

Labels

bug Something isn't working Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Roo Code Roadmap
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hannesrudolph @mrubens