Fix apm install --global skipping MCP server installation#638
Open
sergio-sisternes-epam wants to merge 5 commits intomicrosoft:mainfrom
Open
Fix apm install --global skipping MCP server installation#638sergio-sisternes-epam wants to merge 5 commits intomicrosoft:mainfrom
apm install --global skipping MCP server installation#638sergio-sisternes-epam wants to merge 5 commits intomicrosoft:mainfrom
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the blanket should_install_mcp=False gate for InstallScope.USER in install.py. Instead, add scope awareness to MCPClientAdapter via a supports_user_scope class attribute that each adapter declares: - CopilotClientAdapter: True (~/.copilot/mcp-config.json) - CodexClientAdapter: True (~/.codex/config.toml) - VSCodeClientAdapter: False (.vscode/mcp.json) - CursorClientAdapter: False (.cursor/mcp.json) - OpenCodeClientAdapter: False (opencode.json) MCPIntegrator.install() and remove_stale() now accept a scope parameter and filter target_runtimes to only global-capable adapters at USER scope. The uninstall path (_cleanup_stale_mcp) also receives scope so global uninstall only cleans global configs. This also unblocks --trust-transitive-mcp at global scope, which was silently ignored when the blanket MCP gate was active. Fixes microsoft#637 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes global-scope MCP installation so apm install --global installs MCP servers for runtimes that have user/global config paths (instead of blanket-skipping MCP at user scope), and threads scope through install/uninstall MCP cleanup.
Changes:
- Add
supports_user_scopeto MCP client adapters and filter MCP install/cleanup targets based onInstallScope. - Remove the user-scope MCP blanket-disable in
apm installand forwardscopeinto MCP install/remove-stale paths. - Add unit tests plus doc/changelog updates describing the new
--globalMCP behavior.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_global_mcp_scope.py | Adds coverage for adapter scope flags, MCPIntegrator filtering, uninstall cleanup behavior, and install command integration. |
| src/apm_cli/integration/mcp_integrator.py | Adds scope to install() / remove_stale() and filters runtimes at user scope using adapter capability. |
| src/apm_cli/commands/install.py | Removes blanket user-scope MCP skip; forwards scope to MCPIntegrator install/remove_stale. |
| src/apm_cli/commands/uninstall/engine.py | Threads scope through stale MCP cleanup to avoid touching workspace configs in user scope. |
| src/apm_cli/commands/uninstall/cli.py | Passes uninstall scope into _cleanup_stale_mcp(...). |
| src/apm_cli/adapters/client/base.py | Introduces supports_user_scope: bool = False default on MCP adapters. |
| src/apm_cli/adapters/client/copilot.py | Marks Copilot adapter as user-scope-capable (supports_user_scope = True). |
| src/apm_cli/adapters/client/codex.py | Marks Codex adapter as user-scope-capable (supports_user_scope = True). |
| src/apm_cli/adapters/client/cursor.py | Explicitly marks Cursor as workspace-only (supports_user_scope = False). |
| src/apm_cli/adapters/client/opencode.py | Explicitly marks OpenCode as workspace-only (supports_user_scope = False). |
| docs/src/content/docs/guides/dependencies.md | Updates docs to reflect partial user-scope MCP support (global-capable runtimes only). |
| CHANGELOG.md | Adds unreleased “Fixed” entries for global-scope MCP install and --trust-transitive-mcp behavior. |
- Wrap ClientFactory.create_client() in try/except ValueError in both install() and remove_stale() scope filtering so unsupported runtimes are silently skipped instead of crashing - Replace brittle source-text assertion in TestInstallCommandMCPScope with a functional test that calls MCPIntegrator.install() with mocked dependencies and verifies scope filtering end-to-end - Consolidate CHANGELOG into single entry referencing PR microsoft#638 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Description
Add scope awareness to MCP server installation so that
apm install --globalwrites MCP server entries to global/user-scoped runtime config paths (e.g.~/.copilot/mcp-config.jsonfor Copilot CLI) instead of unconditionally skipping all MCP installation at user scope.Fixes #637
Type of change
Testing