fix: clean stale MCP servers on install/update/uninstall and prevent .claude folder creation

[Sergio Sisternes (sergio-sisternes-epam)]

Summary

Fixes two bugs in the install/update/uninstall lifecycle in complex package scenarios:

1. Stale MCP servers persist after updates or uninstalls — When an upstream package renames or removes an MCP server, the old entry was left behind in .vscode/mcp.json and Copilot CLI config.
2. .claude/ folder created in VS Code-only projects — During uninstall, the re-integration phase called Claude-specific integrators unconditionally, creating .claude/commands/ and .claude/agents/ directories even when the project only targets VS Code.

Root Causes & Fixes

Stale MCP servers (3 sub-issues)

Sub-issue	Root cause	Fix
No tracking	No record of which MCP servers APM previously installed	Added mcp_servers field to lockfile to track managed server names
Parse cache stale on --update	_apm_yml_cache in apm_package.py returned old parsed data after --update overwrote module files	Call clear_apm_yml_cache() after _install_apm_dependencies when update=True
Wrong read ordering	old_mcp_servers was read from lockfile after_install_apm_dependencies regenerated it (losing the mcp_servers field)	Moved the read to before_install_apm_dependencies

.claude folder in VS Code projects

Root cause	Fix
uninstall re-integration called command_integrator.integrate_package_commands(), hook_integrator.integrate_package_hooks_claude(), and agent_integrator.integrate_package_agents_claude() without checking the integration target	Added detect_target / should_integrate_claude guard before the re-integration loop; Claude-specific calls now only run when integrate_claude=True

Changes

• src/apm_cli/cli.py (+162 lines)
  • New helpers: _get_mcp_dep_names(), _remove_stale_mcp_servers(), _update_lockfile_mcp_servers()
  • install(): reads old MCP set before dep install, clears parse cache on update, computes diff, removes stale servers
  • uninstall(): target detection before re-integration, MCP recomputation and stale cleanup
• src/apm_cli/deps/lockfile.py (+4 lines)
  • Added mcp_servers: List[str] field to LockFile dataclass with serialization support

Testing

Full end-to-end test cycle verified:

Step	Result
Fresh install (14 deps, 4 MCP servers)	✅ No .claude/, correct lockfile
Update with MCP rename (github → github-http)	✅ Stale github removed
Update with MCP revert (github-http → github)	✅ Stale github-http removed
Incremental install (xxxxx xxxxx, +17 deps, +3 MCP)	✅ 31 deps, 7 MCP servers
Uninstall (xxxxx)	✅ Back to 14 deps, 4 MCP servers, 3 stale removed
No .claude/ at any step	✅

[Copilot]

Pull request overview

This PR addresses install/update/uninstall lifecycle bugs by tracking APM-managed MCP servers in apm.lock, removing stale MCP server entries after dependency changes, and preventing Claude-specific re-integration from creating .claude/ artifacts in VS Code-only projects.

Changes:

• Extend apm.lock schema with mcp_servers and wire it into YAML serialization/deserialization.
• Update install to capture prior MCP state, clear apm.yml parse cache on --update, compute stale MCP server diffs, remove stale entries, and persist the new set.
• Update uninstall re-integration to guard Claude-specific integration via target detection, and add best-effort stale MCP cleanup after uninstall.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
src/apm_cli/deps/lockfile.py	Adds mcp_servers tracking to the lockfile format to support stale MCP cleanup.
src/apm_cli/cli.py	Implements MCP stale removal + lockfile updates during install/uninstall, and guards Claude re-integration based on detected target.

---

You can also share your feedback on Copilot code review. Take the survey.

[Sergio Sisternes (sergio-sisternes-epam)]

Additional fix: lockfile wipe when installing individual packages (fbfd627)

During end-to-end testing I discovered a pre-existing bug: running apm install <pkg> (with a specific package argument) would wipe the entire apm.lock — all other dependencies and mcp_servers entries were lost.

Root cause:_install_apm_dependencies() filters deps_to_install down to only the requested package(s) when only_packages is set. Then LockFile.from_installed_packages() regenerates the lockfile from that single result, discarding everything else.

Fix: When only_packages is set, the new code reads the existing lockfile and merges new/updated entries into it via LockFile.read() + add_dependency(), preserving all pre-existing dependencies and mcp_servers.

Validated: Ran apm install --trust-transitive-mcp .../general/xxxxxx on a project with 14 dependencies + 7 MCP servers — all entries preserved after install.

[Sergio Sisternes (sergio-sisternes-epam)]

Fix: builtins.set() collision in _remove_stale_mcp_servers (8abe356)

When implementing review comment 2 (make _remove_stale_mcp_servers runtime-aware), we introduced a bare set(all_runtimes) call at line 2791. In this codebase, set is shadowed by Click's config set subcommand, so the call was dispatching into Click instead of creating a Python set — producing:

Usage: apm [OPTIONS] KEY VALUE
Error: Got unexpected extra argument (copilot)

Every other set() in the file already uses builtins.set() to avoid this collision. Fixed the one we missed.

[Sergio Sisternes (sergio-sisternes-epam)]

@microsoft-github-policy-service agree

[Sergio Sisternes (sergio-sisternes-epam)]

Additional fix: transitive deps excluded by only_packages filter

During thorough integration testing of the install/uninstall cycle, I uncovered an additional bug in the same code path.

Problem: When running apm install <package>, if the target package declares mcp: [] but depends on another package that defines MCP servers, those transitive MCP servers were never collected or configured.

Root cause: The only_packages filter in _install_apm_dependencies() kept only the explicitly named package in deps_to_install, discarding all its transitive dependencies. Since the lockfile is built from installed_packages, those transitive deps were never recorded — and _collect_transitive_mcp_deps() only scans packages present in the lockfile, so their MCP server definitions were silently skipped.

Fix: After building the identity set from user-supplied specs, walk the dependency tree to expand the set with all transitive descendants of the requested packages before applying the filter. (+18 lines in _install_apm_dependencies)

Validation:

• 436 related unit tests pass (install, dependency, transitive, mcp, lockfile)
• Integration test: installing a squad package that depends on an infrastructure package with MCP servers now correctly collects and configures those servers (7 vs 4 previously)
• Uninstall correctly removes the transitive MCP servers when the parent package is removed

Pushed as f644317.

[Sergio Sisternes (sergio-sisternes-epam)]

Test coverage hardening (938a388)

Added edge-case tests to close the gaps that motivated this PR:

Unit tests (tests/unit/test_mcp_lifecycle_e2e.py — 4 new tests):

Scenario	What it validates
TestDeepTransitiveChainMCP	A→B→C→D chain: MCP at depth 4 is collected; MCP at every level is collected
TestDiamondDependencyMCP	A→{B,C}→D diamond: shared leaf MCP deduplicates correctly; branch + leaf MCP all present

Integration tests (tests/integration/test_selective_install_mcp.py — 8 tests total, 3 new):

Scenario	What it validates
TestDeepChainIntegration	CLI apm install acme/pkg-a with 4-level chain records depth-4 MCP in lockfile
TestDiamondDependencyIntegration	CLI diamond install: shared MCP appears once, no duplicates in lockfile
TestMultiPackageSelectiveInstallIntegration	CLI apm install pkg-x pkg-y: both transitive MCP trees merged correctly

These specifically exercise the _collect_descendants / only_identities expansion logic added in the fix — without the fix, all three integration tests fail because transitive deps get filtered out of deps_to_install.

Full suite: 27 MCP lifecycle tests pass (19 unit + 8 integration), 1707 total pass with no regressions.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

---

You can also share your feedback on Copilot code review. Take the survey.

[Daniel Meppiel (danielmeppiel)]

Solid work, Sergio. The bugs are real, the root-cause analysis is thorough, the fixes are correct, and the test coverage is impressive (27 MCP lifecycle tests covering deep chains, diamonds, dedup, stale cleanup, --only=apm preservation, and key normalization).

A few non-blocking observations for future hardening — I've opened #209 to track them:

• Silent except Exception: pass — The best-effort pattern makes sense for cleanup, but logger.debug() in those blocks would help when debugging issues in the field.
• _collect_descendants cycle guard — A visited set would be cheap insurance against stack overflow if a cycle ever slips through the resolver.
• Lockfile double read-modify-write — _update_lockfile_mcp_servers re-reads the lockfile that _install_apm_dependencies just wrote. Ordering is correct today but fragile; worth a comment or passing the object through.
• cli.py size — The three new helpers are good candidates to extract into mcp_lifecycle.py as MCP features grow.

None of these block the merge. CI is green, all review comments addressed. Thanks for the diligent E2E testing and the clear commit-by-commit narrative.