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

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

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.