Extract MCPIntegrator: move MCP lifecycle logic out of cli.py

[danielmeppiel]

Context

Follow-up from #201 (stale MCP server cleanup). The implementation is correct and well-tested, but after a deeper architectural review, the original 4 hardening items are better addressed as part of a structural extraction that solves the root cause: MCP lifecycle logic is ~800+ lines of bare functions scattered inside cli.py (5,025 lines).

---

Architectural Assessment

The problem

cli.py is a monolith. MCP-related functions alone span ~690 lines (lines 2468–3155), plus orchestration code embedded inside install() and uninstall(). Meanwhile, every other primitive type (prompts, agents, skills, commands, hooks, instructions) has a clean integrator in src/apm_cli/integration/. MCP has all the supporting infrastructure (adapters, registry operations, lockfile tracking) but no orchestration layer — that logic lives as 11 bare _functions in cli.py.

The fix: Extract MCPIntegrator — as a standalone orchestrator, NOT a BaseIntegrator subclass

This is the key design decision. The existing BaseIntegrator pattern is for file-level deployment (copy files, track in deployed_files, handle collisions, resolve links). MCP integration is fundamentally different — it's config-level orchestration (resolve deps, call registry APIs, write runtime configs, track in lockfile). Forcing MCP into BaseIntegrator would mean implementing find_*_files(), copy_*(), sync_integration() methods that don't apply.

MCPIntegrator (NEW — standalone orchestrator)
  ├── uses: MCPServerOperations (registry/operations.py — already exists)
  ├── uses: MCPClientAdapter subclasses (adapters/client/ — already exist)
  ├── uses: LockFile.mcp_servers (deps/lockfile.py — already exists)
  └── owns: lifecycle logic currently scattered in cli.py

What moves into MCPIntegrator

Function in cli.py	Becomes	Responsibility
_collect_transitive_mcp_deps()	MCPIntegrator.collect_transitive()	Dependency resolution
_deduplicate_mcp_deps()	MCPIntegrator.deduplicate()	Dedup logic
_install_mcp_dependencies()	MCPIntegrator.install()	Main orchestrator
_apply_mcp_overlay()	MCPIntegrator._apply_overlay()	Server info mutation
_build_self_defined_server_info()	MCPIntegrator._build_self_defined_info()	Synthetic server factory
_get_mcp_dep_names()	MCPIntegrator.get_server_names()	Name extraction
_remove_stale_mcp_servers()	MCPIntegrator.remove_stale()	Stale cleanup
_update_lockfile_mcp_servers()	MCPIntegrator.update_lockfile()	Lockfile persistence
_detect_runtimes_from_scripts()	MCPIntegrator._detect_runtimes()	Runtime detection
_filter_available_runtimes()	MCPIntegrator._filter_runtimes()	Runtime filtering
_install_for_runtime()	MCPIntegrator._install_for_runtime()	Per-runtime dispatch

11 functions, ~690 lines out of cli.py.

Usage in install() after extraction

mcp = MCPIntegrator()
transitive = mcp.collect_transitive(apm_modules_path, lock_path, trust_transitive_mcp)
all_mcp = mcp.deduplicate(root_mcp_deps, transitive)
mcp_count = mcp.install(all_mcp, runtime, exclude, verbose)

stale = old_mcp_servers - mcp.get_server_names(all_mcp)
if stale:
    mcp.remove_stale(stale, runtime, exclude)
mcp.update_lockfile(mcp.get_server_names(all_mcp))

What NOT to do

• Don't subclass BaseIntegrator — wrong abstraction for config-level orchestration
• Don't extract _install_apm_dependencies yet — it's ~900 lines but has heavier coupling to install()/uninstall() control flow. Larger scope, separate PR
• Don't touch the existing adapters — they're already clean and in the right place. MCPIntegrator uses them

---

Original hardening items (now addressed naturally by the extraction)

All 4 items from the original issue are resolved as part of this work:

1. Silent except Exception: pass → Add logger.debug() when moving functions into MCPIntegrator
2. _collect_descendants cycle guard → Add visited set when moving the transitive expansion
3. _update_lockfile_mcp_servers double read-modify-write → MCPIntegrator.update_lockfile() can accept the lockfile object directly instead of re-reading from disk
4. Extract to dedicated module → This IS the extraction

Impact

Metric	Before	After
cli.py lines	~5,025	~4,335
MCP testability	Must mock CLI context	Direct unit tests on MCPIntegrator
MCP code location	11 bare functions in cli.py	Single class, single file
Behavior change	—	Zero

Priority

Medium — not a bug fix, but a maintainability improvement that unblocks cleaner MCP feature work. Good candidate for a contributor familiar with the integrator pattern.