feat: MCP install target for Claude Code (project + user scope)#655
feat: MCP install target for Claude Code (project + user scope)#655dmartinol wants to merge 17 commits intomicrosoft:mainfrom
Conversation
Signed-off-by: Daniele Martinoli <dmartino@redhat.com>
dmartinol
requested review from
danielmeppiel and
sergio-sisternes-epam
as code owners
There was a problem hiding this comment.
Pull request overview
Adds Claude Code as a supported MCP client for apm install, including project vs user scope configuration, and wires scope through install/uninstall + conflict detection.
Changes:
- Introduces
ClaudeClientAdapterand registers it inClientFactory; adds runtime detection forclaude. - Threads
install_scope/workspace_rootthrough MCP install/check/cleanup paths and adds Claude-specific stale cleanup. - Updates docs and adds unit tests for Claude MCP config merge/normalize and stale cleanup.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_runtime_detection.py | Adds unit test to detect claude in scripts runtime detection. |
| tests/unit/test_claude_mcp.py | New unit tests for Claude adapter config pathing, merge/normalize behavior, and stale cleanup. |
| src/apm_cli/security/audit_report.py | Refactors markdown escaping to avoid f-string backslash expression issues. |
| src/apm_cli/registry/operations.py | Threads install_scope/workspace_root into installed-server checks; adds claude/opencode parsing branches. |
| src/apm_cli/integration/mcp_integrator.py | Adds Claude runtime detection, scope threading, and Claude project/user stale cleanup logic. |
| src/apm_cli/factory.py | Registers claude runtime to ClaudeClientAdapter. |
| src/apm_cli/core/safe_installer.py | Passes install_scope through to adapters for scope-sensitive MCP config. |
| src/apm_cli/core/operations.py | Extends install_package API to accept workspace_root/install_scope. |
| src/apm_cli/core/conflict_detector.py | Treats Claude configs as mcpServers schema for conflict detection. |
| src/apm_cli/commands/uninstall/engine.py | Passes workspace_root into MCP stale cleanup after uninstall. |
| src/apm_cli/commands/uninstall/cli.py | Wires deploy root into uninstall MCP cleanup call. |
| src/apm_cli/commands/install.py | Enables MCP install for --global and passes scope/root through to MCP integrator. |
| src/apm_cli/adapters/client/claude.py | New Claude adapter implementing project .mcp.json + user ~/.claude.json behavior and normalization. |
| docs/src/content/docs/integrations/ide-tool-integration.md | Documents Claude MCP config locations and runtime targeting. |
| if runtime_name == "vscode": | ||
| if _is_vscode_available(): | ||
| ClientFactory.create_client(runtime_name) | ||
| installed_runtimes.append(runtime_name) | ||
| elif runtime_name == "cursor": |
There was a problem hiding this comment.
When running with user/global scope, this runtime detection can still include "vscode" (via _is_vscode_available()). That can cause apm install -g to create/update a workspace-local .vscode/mcp.json in the current working directory (VSCodeClientAdapter auto-creates .vscode/ even on read), which breaks the intended scope separation. Consider filtering out workspace-scoped runtimes (vscode/cursor/opencode/claude-project) when install_scope is InstallScope.USER, unless the user explicitly targets them, or ensure repo-local adapters are rooted at workspace_root rather than CWD.
|
@microsoft-github-policy-service agree |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Daniele Martinoli <dmartino@redhat.com>
Signed-off-by: Daniele Martinoli <dmartino@redhat.com>
| if runtime_name == "vscode": | ||
| if _is_vscode_available(): | ||
| ClientFactory.create_client(runtime_name) | ||
| installed_runtimes.append(runtime_name) | ||
| elif runtime_name == "cursor": |
There was a problem hiding this comment.
When running with install_scope=InstallScope.USER (e.g. apm install -g), this runtime detection still includes workspace-scoped clients like VS Code. Since VSCodeClientAdapter uses os.getcwd() and auto-creates .vscode/ in get_config_path(), a global install can end up creating/modifying .vscode/mcp.json in whatever directory the user ran the command from. Consider filtering target runtimes by scope (USER: only home-scoped runtimes like copilot/codex/claude) or implementing workspace_root support so repo-local adapters read/write under that root instead of CWD.
| stored_mcp_configs=old_mcp_configs, | ||
| diagnostics=apm_diagnostics, | ||
| workspace_root=get_deploy_root(scope), | ||
| install_scope=scope, | ||
| ) |
There was a problem hiding this comment.
workspace_root is documented as the root for repo-local MCP configs, but get_deploy_root(InstallScope.USER) is Path.home(). Passing the home directory here makes runtime detection/checks look for .cursor/ / .opencode/ / .claude/ under $HOME, and does not prevent repo-local adapters from writing under the process CWD. It would be safer to pass Path.cwd() for workspace_root (repo-local), and rely on install_scope for Claude user-vs-project behavior, or otherwise ensure only user-scoped runtimes run in USER installs.
| wr = workspace_root if workspace_root is not None else Path.cwd() | ||
|
|
||
| # Determine which runtimes to clean, mirroring install-time logic. | ||
| all_runtimes = {"vscode", "copilot", "codex", "cursor", "opencode"} | ||
| all_runtimes = {"vscode", "copilot", "codex", "cursor", "opencode", "claude"} | ||
| if runtime: |
There was a problem hiding this comment.
In USER scope, workspace_root will typically be Path.home(), but remove_stale() still considers workspace-scoped runtimes (vscode/cursor/opencode) in all_runtimes. This can cause stale cleanup during apm install -g / user-scope uninstall to touch unexpected paths (and some adapters create directories on read). Consider deriving all_runtimes from install_scope (USER: home-scoped runtimes only) or requiring an explicit --runtime for workspace-scoped cleanup.
sergio-sisternes-epam
left a comment
sergio-sisternes-epam
left a comment
There was a problem hiding this comment.
Great work adding Claude Code as a first-class MCP target — the adapter design (reusing CopilotClientAdapter with normalization) and the opt-in project scope (.claude/ gate) are well thought out.
There are a few issues to address before we can merge:
1. Scope leakage on apm install -g (blocking)
workspace_root is threaded through 3 APIs but explicitly ignored (_ = workspace_root in safe_installer.py, registry/operations.py, and one more). During apm install -g, runtime detection still includes workspace-scoped clients (vscode, cursor, opencode), which could write to CWD-relative config files. This PR widens that surface by enabling MCP install for --global.
Fix: filter out workspace-scoped runtimes when install_scope is InstallScope.USER. Only home-scoped runtimes (copilot, codex, claude-user) should be eligible for global installs. Either implement workspace_root properly or remove the parameter to avoid a misleading API — accepted-but-ignored parameters are a bug magnet.
2. Stale cleanup breaks opt-in contract (blocking)
Claude project install is opt-in (only writes .mcp.json when .claude/ exists), but stale cleanup removes entries from .mcp.json whenever it exists, even if .claude/ is absent. Gate project cleanup on .claude/ existing, same as install, to keep behavior consistent.
3. CLI output convention (non-blocking)
Stale cleanup messages use hardcoded "+" instead of the repo's STATUS_SYMBOLS convention ([+] via _rich_info with symbol="check"). Minor, but worth aligning for consistency.
4. Missing CHANGELOG entry
Please add an entry under ## [Unreleased]:
### Added
- Add Claude Code as MCP install target with project (`.mcp.json`) and user (`~/.claude.json`) scope support (#643)
5. Review Copilot's feedback
Copilot flagged the scope leakage across 5 separate threads with concrete suggestions for each affected location. Please review and address those comments — they map directly to the concerns above and provide useful implementation guidance.
Thanks for the thorough contribution — the feature is valuable, just needs the scope boundaries tightened.
The chrome-devtools MCP is wired up via three artifacts at project root (.mcp.json, justfile recipe, shell.nix env var) because APM's current claude-code runtime adapter lacks MCP support. Once microsoft/apm#655 merges and juspay's fork picks it up, we can fold this into apm.yml's `dependencies.mcp` and let `just ai::apm` regenerate .mcp.json.
Encapsulate what can be encapsulated today. The justfile recipe moves from project-root `justfile` into `agents/ai.just` (sibling to the rest of the APM/agent dev tooling), and `shell.nix` reverts its `KOLU_CHROME_EXECUTABLE` addition — the recipe now resolves Chrome-for-Testing inline by globbing `$PLAYWRIGHT_BROWSERS_PATH/chromium-*/chrome-linux64/chrome`. `.mcp.json` stays at project root (that's where Claude Code reads it from) and its command updates to `just ai::mcp-chrome-devtools` via the existing `mod ai 'agents/ai.just'` import. Full encapsulation — declaring the MCP server inside `agents/apm.yml` and letting `just ai::apm` regenerate `.mcp.json` — is blocked on microsoft/apm#655 (Claude Code MCP adapter) landing in juspay's fork. The follow-up TODO is captured in the recipe's doc comment. Trade-off: the hickey-iteration's elegant Nix-eval-time chrome-path resolution (read browsers.json at eval, fail loud on layout change) is dropped in favor of a runtime shell glob. Failure mode shifts from "fails at `nix develop` eval" to "empty --executable-path at MCP startup → chrome-devtools-mcp errors out loud" — different failure point, still loud, and keeps `shell.nix` free of MCP-specific env vars.
Prototype for #518. Wires the official [chrome-devtools-mcp](https://github.com/ChromeDevTools/chrome-devtools-mcp) server so **Claude (and any other MCP client) can drive a real headless Chrome** against Kolu's own dev server — `evaluate_script`, `list_console_messages`, `take_screenshot`, network inspection, performance traces, all 29 tools. Solves the recurring "I'm guessing at CSS cascade / computed styles" round-trip that inspired the issue. The wiring lives in three places. `.mcp.json` at the repo root is the bit Claude Code actually reads — Claude starts _outside_ the nix devshell, so the `nix develop --command just ai::mcp-chrome-devtools` wrapper bridges into a shell where `npx`/`node` and the Playwright-provided Chrome-for-Testing are on `PATH`. The launcher recipe sits in `agents/ai.just` alongside the rest of the APM/agent dev tooling, and resolves Chrome's binary inline by globbing `$PLAYWRIGHT_BROWSERS_PATH/chromium-*/chrome-linux64/chrome` so there's no shell.nix surface area to maintain. _Reuses Playwright's Chrome-for-Testing 143, which chrome-devtools-mcp officially supports — no new browser dependency, just the Node package fetched by `npx`._ End-to-end smoke verified: JSON-RPC `initialize` returns `chrome_devtools` v0.21.0 with `tools.listChanged` capability advertised. Navigated to `http://localhost:7681`, took an a11y snapshot, evaluated `document.querySelectorAll('[data-terminal-id]')` to probe live sidebar state — all 29 tools work as expected. Warm `nix develop` eval stays at ~0.25s. > **Follow-up encapsulation is gated on [microsoft/apm#655](microsoft/apm#655 Once that PR (Claude Code MCP adapter) merges and lands in juspay's fork, the `chrome-devtools` entry folds into `agents/apm.yml`'s `dependencies.mcp` and `.mcp.json` becomes a generated artifact of `just ai::apm`. The TODO is captured in the `mcp-chrome-devtools` recipe's doc comment. > **Telemetry note.** `chrome-devtools-mcp` sends usage statistics to Google by default, and performance traces send URLs to the Chrome CrUX API. Left default-on for this prototype — if unwanted, add `--no-usage-statistics --no-performance-crux` to the recipe before merge. Refs #518.
- Add WORKSPACE_SCOPED_MCP_RUNTIMES / HOME_SCOPED_MCP_RUNTIMES constants - Reject --global with workspace runtimes (vscode/cursor/opencode) - Filter detected runtimes for apm install -g; adjust VS Code fallback - Mirror filtering in remove_stale when install_scope is USER Addresses PR microsoft#655 review (scope leakage on global install). Made-with: Cursor
Match install opt-in: only edit project .mcp.json when .claude/ exists. PR microsoft#655 Made-with: Cursor
Use _rich_info(..., symbol="check") instead of hardcoded '+' prefix. PR microsoft#655 Made-with: Cursor
PR microsoft#655 / microsoft#643 Made-with: Cursor
Document that repo-local paths use process cwd and USER-scope installs filter workspace runtimes upstream. PR microsoft#655 Made-with: Cursor
- Require .claude/ for project stale cleanup tests; add opt-in skip test - Assert USER-scope remove_stale does not touch .vscode/mcp.json - Assert apm install -g with --runtime vscode raises - Normalize newlines in global-no-manifest install test (Rich wrap) PR microsoft#655 Made-with: Cursor
|
Thanks for the review. Addressed all blocking items:
Global MCP scope: User-scope installs only consider home-scoped runtimes (copilot, codex, claude). Workspace/CWD clients (vscode, cursor, opencode) are excluded from auto-detection and
Claude project cleanup: Stale removal for project
Messages: Stale MCP lines use
CHANGELOG [Unreleased] Added line added as requested.
workspace_root: Documented that adapters still use |
danielmeppiel
left a comment
danielmeppiel
left a comment
There was a problem hiding this comment.
Comparative architectural pass against the existing 5 MCP targets (vscode/copilot/cursor/opencode/codex): the adapter itself is well-formed and follows the Cursor/OpenCode inheritance pattern exactly, the .claude/ opt-in gate mirrors .cursor/, and _normalize_for_claude_code is the same shape as OpenCode's _to_opencode_format. Format normalization correctly lives in the adapter. Good work overall.
Flagging four PR-specific items that should land before merge. The broader integration-layer scaling concerns (per-runtime if/elif chains in remove_stale, _get_installed_server_ids, conflict detection) are pre-existing patterns shared by all 5 existing targets, so they're being filed as a separate architectural issue rather than blocking this PR.
Blocking
1. apm install -g auto-targets Claude without an availability check — src/apm_cli/integration/mcp_integrator.py:955-959, 983-989
In user scope, claude is appended to the runtime list purely because ClientFactory.create_client("claude") succeeds. There's no shutil.which("claude") (or equivalent) check. Existing apm install -g users who don't use Claude Code will suddenly get ~/.claude.json created or modified.
Suggest gating Claude (and ideally any home-scoped runtime) on real binary availability, mirroring how _detect_runtimes already uses command-line presence for the workspace-scoped path.
2. Project-scope Claude install is a silent "success" no-op when .claude/ is absent — src/apm_cli/adapters/client/claude.py:181-185, 227-229 and src/apm_cli/integration/mcp_integrator.py:1233-1242
If .claude/ does not exist, update_config returns and configure_mcp_server returns True. The integrator counts it as configured and the install summary reports success, but no .mcp.json is written. From a user's perspective, apm install --runtime claude says it worked when it didn't.
Two options: (a) return an explicit skip state that the integrator recognizes and excludes from the success count, or (b) emit an actionable info message such as "Skipping Claude project config: no .claude/ directory found. Create .claude/ to opt in, or use apm install -g for user scope." Cursor and OpenCode have the same silent-skip semantic — worth fixing the pattern here once and aligning the others in a follow-up.
3. Stale cleanup fails open when install_scope=None — src/apm_cli/integration/mcp_integrator.py:710-716, 748-776
When install_scope is missing, Claude cleanup is allowed to touch both project .mcp.json AND user ~/.claude.json. With name overlap between scopes, a project-scope operation can delete user-level entries (or vice versa). Suggest failing closed: if scope is unspecified, default to a single scope rather than both, and log clearly which scope was chosen.
4. Non-ASCII em-dash in new source comment — src/apm_cli/integration/mcp_integrator.py (new code near the top of the file)
# target them — see ``install()`` and ``remove_stale()``. uses U+2014 (—). Repo encoding rule requires printable ASCII (U+0020–U+007E). Replace with --. Trivial fix; the file is otherwise ASCII-clean on main.
Non-blocking suggestion (raised for visibility, not gating)
mcp_install_scope is set on the adapter via attribute assignment from 4+ call sites and read with getattr(self, "mcp_install_scope", None). This is the first adapter to carry runtime-mutable state and the first with dual config paths. It works defensively today, but as more adapters need per-invocation configuration the implicit contract will hurt. A future refactor could either thread scope through ClientFactory.create_client(**kwargs) or pass it as an argument to update_config / configure_mcp_server. No need to block on it here.
Tracked separately
The broader integration-layer coupling (per-runtime branches in mcp_integrator.remove_stale, registry/operations._get_installed_server_ids, and core/conflict_detector.get_existing_server_configs that bypass the adapter abstraction and re-implement file I/O) is being filed as a separate refactor issue. This is the second target after VS Code where it's becoming visibly painful, and APM's broader install-target surface is growing faster than MCP coverage of it.
Closes #643
Summary
Adds Claude Code as a first-class MCP client for
apm install/ lockfile-driven MCP lifecycle: registry-backed server entries are merged into Claude’s documented config locations, with project vs user scope aligned toInstallScope(apm installvsapm install --global).Behavior
.mcp.jsonat the repo root (mcpServers), only when.claude/already exists (opt-in, same idea as Cursor +.cursor/).~/.claude.jsontop-levelmcpServerswhen scope is user / global install path.CopilotClientAdapter, then normalizes entries for Claude Code (e.g. drop Copilot-only stdio fields such astype: local, defaulttools, emptyid; keep remotes / HTTP shape per docs).--globalwhen MCP deps exist, so user-scoped Claude (and other home-dir runtimes) can be configured from the manifest.MCPIntegratorpassesinstall_scope(and related wiring) into install/uninstall paths; stale cleanup removes Claude entries from project.mcp.jsonand user~/.claude.jsonwhere applicable.MCPConflictDetectortreats Claude like othermcpServers-shaped clients.ClientFactoryregisters runtimeclaude/Claude.tests/unit/test_claude_mcp.py(adapter, merge/normalize,remove_stale); runtime script detection forclaudeintest_runtime_detection.py.Notes
apmfrom the project root for project MCP files.safe_installeraccepts optionalworkspace_root/install_scopefor API compatibility with the integrator;mcp_install_scopeis set on the adapter for Claude user vs project behavior.audit_report.py: extract pipe-escaping for Markdown tables into a variable to satisfy Python 3.11+ f-string rules (no behavior change).Minor style-only diffs in Python may appear from Black/isort on edited files; repo-wide formatting and CI alignment are out of scope here — see #645.
Testing
uv run pytest tests/unit/test_claude_mcp.py tests/unit/test_runtime_detection.py -xCONTRIBUTING.md