Skip to content

agentHost: allow uninstalling MCP servers mirrored from the upstream registry - #347197

Open
Vritant Bhardwaj (vritant24) wants to merge 1 commit into
mainfrom
agents/fix-vscode-issue-346415-73de51cf
Open

Vritant Bhardwaj (vritant24) wants to merge 1 commit into
mainfrom
agents/fix-vscode-issue-346415-73de51cf

Conversation

@vritant24

Copy link
Copy Markdown
Member

Fixes #346415

Problem

Uninstalling an MCP server that GitHub's MCP service mirrors from the upstream MCP registry (for example Figma MCP Server, urn:air:registry.modelcontextprotocol.io:com.figma.mcp:mcp) failed with "The customization removal did not complete."

When VS Code confirms an owned MCP uninstall, prepareUninstall required the confirmation's provenance.authority to equal the host taken from the registry ID. For mirrored servers these differ: the registry host is registry.modelcontextprotocol.io, but the server is served by api.mcp.github.com. The check failed, the confirmation handler answered cancel, and assertApplied reported a generic error with no cause.

Changes

  • Uninstall confirmation: removed the registry-host comparison and the now-unused isMatchingInstallationAuthority. The authority now only has to be a non-empty string. McpUninstallPlan and McpInstallationSummary don't include provenance, so there's no reliable value to compare it against. The confirmation is still tied to the exact installation by its installation ID, identity, config key, user scope, media type, expiry, owned-secret count and restore/shared-auth flags. Install is unchanged; it already compares against the provenance in the runtime's plan.
  • Diagnostics: when an install or removal ends with an unexpected outcome, the error now includes the SDK outcome, e.g. "The customization removal did not complete. (cancelled)". Sensitive and typed-reason failures are unchanged.
  • Tests: the MCP install/uninstall test now uses a registry ID whose host differs from the serving authority (the real-world shape), and checks that an empty authority is still cancelled. The skill-removal test expects the more specific messages.

A longer-term option is for the runtime to return provenance in the uninstall plan, so VS Code can compare it exactly the way install does. The separate configuration-changed refusal when the entry is missing from mcp-config.json is runtime bug github/copilot-agent-runtime#34949.

Validation

  • .\scripts\test.bat --run src/vs/platform/agentHost/test/node/copilotCustomizationInstallations.test.ts: passed (46 tests)
  • pnpm run typecheck-client: passed
  • pnpm run eslint on the two changed files: passed

How to test

  1. In Customizations → Browse, install Figma MCP Server.
  2. Go to Customizations → MCP Servers → Figma MCP Server → Uninstall, and approve the review.
  3. The server is removed. Before this change, you'd see "Could not uninstall Figma MCP Server. The customization removal did not complete."

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

…registry

The MCP uninstall confirmation required the provenance authority to equal the registry ID host. Servers mirrored from registry.modelcontextprotocol.io are served by api.mcp.github.com, so the check failed, the runtime cancelled, and uninstall reported a generic error. The uninstall plan carries no provenance to compare against, so only require a non-empty authority. Unfinished install/removal errors now include the SDK outcome.

Fixes #346415

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The remaining review finding flags duplicated outcome-detail formatting that could reuse resultError to avoid maintaining diagnostics in two places.

1 open issue

Review effort: Max

Comment on lines +1639 to +1642
const detail = describeSdkOutcome(result);
throw new Error(detail
? localize('copilot.customizationInstallation.resultErrorDetail', "{0} ({1})", message, detail)
: message);

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Uninstalling a registry MCP server from Customizations fails with "The customization removal did not complete"

2 participants