fix(mcp): honour per-dep registry URL during install (#1393)

[sergio-sisternes-epam]

Description

When an MCP dependency in apm.yml specifies a per-dep registry: URL, APM now uses that URL for server resolution instead of the global default. Previously, the field was stored but ignored with a "not yet applied" warning.

Fixes

Fixes #1393

Type of change

• Bug fix (non-breaking change which fixes an issue)

Changes

1. Extracted _install_registry_group() helper in mcp_integrator_install.py — encapsulates the validate-fetch-overlay-install loop for a group of registry deps
2. Grouped registry deps by dep.registry URL — deps with per-dep registry URLs get their own MCPServerOperations(registry_url=url) instance
3. Removed the "not yet applied" warning for registry field in _apply_overlay()
4. Updated existing test — test_registry_str_overlay_emits_warning now verifies no warning is emitted

Testing

• 6 new tests in test_mcp_per_dep_registry.py (grouping, routing, mixed deps)
• 132 existing MCP tests pass
• Lint clean

[Copilot]

Pull request overview

This PR fixes MCP installation so that a dependency’s per-entry registry: URL in apm.yml is actually used during install-time server resolution, instead of being stored but ignored.

Changes:

• Group MCP registry-resolved dependencies by dep.registry and instantiate MCPServerOperations(registry_url=...) per unique registry URL.
• Extract _install_registry_group() to encapsulate the validate -> fetch -> overlay -> install loop for one registry group.
• Remove the “registry not yet applied” overlay warning and update/add unit tests to validate the new routing behavior.

Reviewed changes

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

File	Description
tests/unit/integration/test_mcp_per_dep_registry.py	Adds coverage for per-dependency registry grouping and routing behavior (but currently includes non-ASCII characters in docstrings/comments).
tests/unit/integration/test_mcp_integrator.py	Updates overlay warning expectations so registry: <url> no longer emits a warning.
src/apm_cli/integration/mcp_integrator.py	Removes the warning previously emitted for dep.registry overlay strings.
src/apm_cli/integration/mcp_integrator_install.py	Implements per-dependency registry URL routing by grouping deps and running installs per registry group.

[danielmeppiel]

APM Review Panel: ship_with_followups

Community-contributed bug fix enables per-dep MCP registry routing, unlocking mixed public+private server topologies for enterprise adopters (closes #1393).

cc @sergio-sisternes-epam @danielmeppiel -- a fresh advisory pass is ready for your review.

This PR delivers on a manifest-level promise that was documented but silently broken: the per-dependency registry: <url> field on MCP deps now actually routes install traffic to the specified registry. The fix is architecturally clean -- extracting a _install_registry_group helper, grouping deps by registry URL, and constructing a correctly-parameterized MCPServerOperations per group. The python-architect confirms the refactor is sound (13-param signature is tolerable; Any typing is a follow-up nit). Auth-expert and supply-chain-security-expert converge on safety: no auth headers leak to arbitrary URLs, and the existing HTTPS-only scheme validation applies.

Panel signals converge strongly on two gaps that should ship alongside or immediately after: (1) a CHANGELOG entry -- three panelists independently flag this (devx-ux, oss-growth, doc-writer), making it the highest-confidence consensus finding; (2) a docs example in install-mcp-servers.md and apm-guide dependencies.md showing the per-dep URL form (oss-growth + doc-writer). The cli-logging and devx-ux experts converge on per-group output disambiguation -- users with 2+ registries currently see duplicated heartbeat lines with no registry context. This is a UX polish item worth a fast follow-up issue but does not block ship.

The test-coverage-expert notes unit regression traps exist and are well-structured, but no integration-tier test proves the full HTTP routing path. The evidence.outcome: missing on a portability-by-manifest surface elevates this above a typical nit -- it belongs in the follow-up queue but does not block given the unit coverage is sound and the PR is a bug fix restoring already-documented behavior.

Aligned with: Portable by manifest -- per-dep registry: <url> is a manifest-level portability contract; teams can check in mixed-topology MCP deps and have them resolve identically across machines. Pragmatic as npm -- mirrors npm scoped-registry overrides; familiar mental model for JS-ecosystem adopters. OSS community-driven -- first-time community contributor fixing a real user-reported bug; clean PR, good test coverage. Secure by default -- auth-expert confirms no token leakage on arbitrary registry URLs; HTTPS-only scheme validation applies. Governed by policy -- gap identified: policy engine checks MCP server names but lacks an allowed_registries constraint (follow-up for enterprise lockdown).

Growth signal. PR #1443 is the first concrete proof-point for "APM works with private/enterprise MCP registries." The story angle for the next release is "Mix public and private MCP servers in one manifest" -- targeting platform engineering teams evaluating APM for internal AI tooling. The CHANGELOG entry should frame this as an enterprise capability unlock, not just a bug fix. A 3-line apm.yml example in install-mcp-servers.md makes the feature discoverable to evaluators scanning docs.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	2	Clean helper extraction with correct grouping logic; 13-param signature is tolerable at this scope but operations: Any loses type safety unnecessarily.
CLI Logging Expert	0	2	1	Heartbeat repeats per-group without naming the registry; users with 2+ registries see duplicated lines with no way to tell them apart.
DevX UX Expert	0	1	2	Per-dep registry install promise delivered; error path leaks misleading guidance when URL is invalid; search hint points at wrong registry.
Supply Chain Security Expert	0	1	1	Per-dep registry URL passes through existing scheme validation (https-only default); no token leakage. Recommend adding allowed_registries policy gate as hardening follow-up.
OSS Growth Hacker	0	2	1	Strong community-contributor signal; per-dep registry routing unlocks a compelling enterprise story angle worth a release beat and a docs example.
Auth Expert	0	0	1	Per-dep registry routing is auth-neutral: SimpleRegistryClient uses bare requests.Session with no Authorization headers, so no token leakage on arbitrary URLs.
Doc Writer	0	2	1	No stale "ignored" claim in user docs (field was already documented), but CHANGELOG entry and an apm-guide example are missing for a now-honoured field.
Test Coverage Expert	0	1	2	Unit regression-trap for per-dep registry routing exists but no integration-with-fixtures test proves the end-to-end install path honours registry: URL.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 6 follow-ups

1. [Doc Writer] Add CHANGELOG [Unreleased] Fixed entry framing per-dep registry routing as enterprise capability unlock (closes [BUG] apm install ignores dependencies.mcp[].registry #1393). -- Three panelists converge (devx-ux, oss-growth, doc-writer). Silent release-comms drift otherwise; evaluators scan CHANGELOG for enterprise signals.
2. [Doc Writer] Add per-dep registry: <url> example to apm-guide/dependencies.md and consumer/install-mcp-servers.md. -- Feature is invisible without docs example. Two panelists converge (oss-growth, doc-writer). Agent-facing apm-guide is authoritative for LLM consumers.
3. [DevX UX Expert] Wrap MCPServerOperations(registry_url=...) in try/except ValueError with actionable error pointing at the apm.yml dep, not MCP_REGISTRY_URL env var. -- Invalid per-dep URL currently surfaces misleading guidance or raw traceback. One try/except delivers correct error ergonomics.
4. [CLI Logging Expert] Disambiguate heartbeat and success lines per registry group (verbose-mode registry annotation). -- Users with 2+ registries see duplicated output with no way to tell groups apart. Progressive-disclosure fix: verbose annotation only.
5. [Test Coverage Expert] Add integration-tier test proving apm install routes deps to per-dep registry URL (mock only HTTP layer). -- Unit regression trap exists but integration tier is missing on a portability-by-manifest surface. Elevates above typical nit per evidence weighting.
6. [Supply Chain Security Expert] Add optional allowed_registries policy field to McpPolicy for enterprise registry lockdown. -- No urgency (apm.yml trust boundary already allows command: arbitrary code) but completes the governed-by-policy story for enterprise adopters.

Architecture

classDiagram
    direction LR
    class MCPIntegrator {
      <<BaseIntegrator>>
      +install(mcp_deps, ...)
      +_apply_overlay(cache, dep)
      +_detect_mcp_config_drift(deps, configs)
      +_install_for_runtime(rt, deps, ...)
    }
    class run_mcp_install {
      <<ModuleFunction>>
      +run_mcp_install(mcp_deps, ...) int
    }
    class _install_registry_group {
      <<ExtractedHelper>>
      +_install_registry_group(operations, ...) int
    }
    class MCPServerOperations {
      <<Service>>
      +__init__(registry_url)
      +validate_servers_exist(names)
      +batch_fetch_server_info(servers)
    }
    class SimpleRegistryClient {
      <<Adapter>>
      +__init__(registry_url)
    }
    class MCPDependency {
      <<ValueObject>>
      +name str
      +registry str or None or False
      +is_self_defined bool
    }
    run_mcp_install ..> _install_registry_group : delegates per group
    run_mcp_install ..> MCPServerOperations : constructs per registry URL
    _install_registry_group ..> MCPServerOperations : calls validate/fetch/install
    _install_registry_group ..> MCPIntegrator : calls static helpers
    MCPServerOperations *-- SimpleRegistryClient : owns
    run_mcp_install ..> MCPDependency : reads registry field
    class _install_registry_group:::touched
    class run_mcp_install:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    A["apm install"] --> B["MCPIntegrator.install delegates"]
    B --> C["run_mcp_install"]
    C --> D{"Split: self_defined vs registry deps"}
    D -->|self_defined| E["_install_self_defined_servers"]
    D -->|registry| F["Group deps by dep.registry URL"]
    F --> G{"For each group_registry_url"}
    G --> H["MCPServerOperations(registry_url=group_url)"]
    H --> I["_install_registry_group helper"]
    I --> J["operations.validate_servers_exist"]
    J --> K{"invalid?"}
    K -->|yes| L["raise RuntimeError"]
    K -->|no| M["check + drift detection"]
    M --> N["batch_fetch_server_info"]
    N --> O["apply_overlay per dep"]
    O --> P["install_for_runtime per rt"]
    P --> Q["configured_count += 1"]
    G -->|next group| H

Loading

Recommendation

Merge after adding the CHANGELOG entry (highest-confidence panel consensus, can land in same PR). The docs example and ValueError handling are strong fast-follows worth filing as issues immediately post-merge. The contributor did clean work -- warm merge comment naming the enterprise unlock will reinforce the contributor funnel.

---

Full per-persona findings

Python Architect

• [recommended] operations: Any discards static type info when MCPServerOperations is importable under TYPE_CHECKING at src/apm_cli/integration/mcp_integrator_install.py:37
  Type hints on all public APIs; MCPServerOperations is a concrete class. Using Any hides the contract from static analysis.
  Suggested: Add to TYPE_CHECKING block: from apm_cli.registry.operations import MCPServerOperations; change Any to MCPServerOperations. Same for project_root / console / logger.
• [nit] 13-parameter helper could use a grouped context dataclass in a follow-up
  Pragmatic follow-up: bundle project_root / user_scope / verbose / console / logger / target_runtimes / stored_mcp_configs into a frozen MCPInstallContext dataclass.
• [nit] group_dep_names list is redundant with group_dep_map.keys() for object deps at src/apm_cli/integration/mcp_integrator_install.py:250
  Defensible for backward-compat with plain-string deps that lack .name, but worth a one-line comment.

CLI Logging Expert

• [recommended] Heartbeat emitted per-group without registry context -- confusing for multi-registry installs at src/apm_cli/integration/mcp_integrator_install.py:66
mcp_lookup_heartbeat fires inside _install_registry_group; user with 2 registries sees two identical lines. Newspaper test fails. Verbose-mode annotation 'Looking up 3 MCP servers in https://...' would satisfy progressive disclosure.
  Suggested: Pass group_registry_url into helper; when not None, append to heartbeat / verbose_detail.
• [recommended] Removed "registry not yet applied" warning replaced with silent acceptance -- missing positive info line for custom registry at src/apm_cli/integration/mcp_integrator.py
  When user explicitly configured a non-default registry URL, should see confirmation it took effect (helps debugging auth/network issues). verbose_detail satisfies progressive disclosure.
  Suggested: Before calling helper: if group_registry_url and verbose: logger.verbose_detail(f'Using custom registry: {group_registry_url}')
• [nit] "All registry MCP servers already configured" can repeat per-group without disambiguation at src/apm_cli/integration/mcp_integrator_install.py:111
  Non-rich path emits the success line per group; with two groups up-to-date, user sees same line twice.

DevX UX Expert

• [recommended] ValueError from invalid per-dep registry URL surfaces misleading "Check MCP_REGISTRY_URL" guidance or raw traceback at src/apm_cli/integration/mcp_integrator_install.py:254
SimpleRegistryClient.__init__ raises ValueError with "Check MCP_REGISTRY_URL if set" text, but for a per-dep registry: field the env var is irrelevant. try/except only catches ImportError, so ValueError propagates unhandled.
  Suggested: Wrap MCPServerOperations(registry_url=group_registry_url) in try/except ValueError that emits actionable error pointing at apm.yml dep.
• [nit] Recovery hint "apm mcp search " after per-dep registry failure points at default registry, not the failing one at src/apm_cli/integration/mcp_integrator_install.py:348
  Search always hits default; misleading for private-registry users. Out of scope for this bugfix but worth follow-up issue.
• [nit] Missing CHANGELOG entry for user-visible behavior change at CHANGELOG.md
  PR removes a user-visible warning and enables a previously-promised feature.

Supply Chain Security Expert

• [recommended] No policy gate restricts which registry URLs MCP deps may target at src/apm_cli/policy/policy_checks.py
  Policy engine checks MCP server names (check 7) but no allowed_registries equivalent. apm.yml already a trust boundary (command: allows arbitrary code), so strictly less dangerous than existing surfaces -- but worth a follow-up enterprise lockdown hook.
  Suggested: Add optional allowed_registries: [...] field to McpPolicy; when set, reject deps whose registry: value does not prefix-match. Unset = backward-compat allow-all.
• [nit] Consider logging when a per-dep registry override is active
  Diagnostic log line improves auditability in CI.

OSS Growth Hacker

• [recommended] Add a CHANGELOG entry that frames the fix as an enterprise capability unlock, not just a bug fix at CHANGELOG.md
  Enterprise framing converts plumbing-fix into adoption signal for evaluators scanning the changelog.
  Suggested: Under Unreleased Fixed: 'MCP per-dep registry routing: registry: https://... on individual MCP deps is now honoured at install time, enabling mixed public+private MCP server topologies. Thanks @sergio-sisternes-epam. (fix(mcp): honour per-dep registry URL during install (#1393) #1443, closes [BUG] apm install ignores dependencies.mcp[].registry #1393)'
• [recommended] Consumer docs (install-mcp-servers.md) only show registry:false -- add a string-URL example to surface the new capability at docs/src/content/docs/consumer/install-mcp-servers.md
  Enterprise adopters land on install-mcp-servers.md first; without an example the feature is invisible.
  Suggested: Add a section "Using a private MCP registry" with a 3-line apm.yml snippet showing registry: 'https://mcp.corp.example.com' alongside a public-registry dep.
• [nit] Warm contributor reception in merge comment -- name the enterprise unlock
  Improves contributor funnel conversion.

Auth Expert

• [nit] Add explicit # SECURITY: no auth headers -- registry calls are anonymous by design comment in SimpleRegistryClient at src/apm_cli/registry/client.py:150
  Class has no doc/assertion guaranteeing "this session MUST remain unauthenticated". A one-line SECURITY comment would make the invariant grep-discoverable to prevent future contributors from naively adding session.headers['Authorization'].

Doc Writer

• [recommended] Add an [Unreleased] Fixed entry for per-dep registry: <url> on MCP deps (closes [BUG] apm install ignores dependencies.mcp[].registry #1393) at CHANGELOG.md
  Keep a Changelog + project pattern; PR removes a user-visible warning and makes a documented field functional -- exactly the shape users grep CHANGELOG for. Silent release-comms drift otherwise.
  Suggested: Under [Unreleased] Fixed: apm install --mcp honours the per-dependency registry: <url> override on MCP deps. (fix(mcp): honour per-dep registry URL during install (#1393) #1443, closes [BUG] apm install ignores dependencies.mcp[].registry #1393)
• [recommended] packages/apm-guide/.apm/skills/apm-usage/dependencies.md MCP examples omit the per-dep registry: <url> form that this PR makes functional at packages/apm-guide/.apm/skills/apm-usage/dependencies.md:188
  apm-guide is agent-facing source of truth for dependency formats; per repo convention MCP-dep-format changes must update it. Currently strictly behind the manifest-schema doc.
  Suggested: Add a 3-line example: registry: https://mcp.internal.example.com on a named MCP dep.
• [nit] docs/src/content/docs/producer/author-primitives/mcp-as-primitive.md could mirror the same per-dep URL example for parity at docs/src/content/docs/producer/author-primitives/mcp-as-primitive.md:68
  Field table already documents URL form; examples block omits it.

Test Coverage Expert

• [recommended] No integration-tier test proves apm install routes deps to per-dep registry URL end-to-end at tests/integration/test_mcp_install_flow.py
  New unit tests mock MCPServerOperations at the class boundary -- they prove grouping logic but not real HTTP routing. Tier floor for install-pipeline is integration-with-fixtures; unit alone does not certify user promise.
  Suggested: Add integration test calling run_mcp_install with MCPDependency(registry='https://custom.example.com') and assert MCPServerOperations constructor received that URL -- mock only HTTP layer beneath.
  Proof (missing at): tests/integration/test_mcp_install_flow.py -- proves: When user sets registry: https://custom.example.com on dep in apm.yml, apm install fetches that dep from the custom registry, not default. [devx, portability-by-manifest]
• [nit] Unit regression trap exists for per-dep registry routing (kwarg assertion)
  test_single_custom_registry_creates_one_instance asserts ops_cls.call_args == call(registry_url=url) -- valid regression trap at unit tier.
  Proof (unknown): tests/unit/integration/test_mcp_per_dep_registry.py::test_single_custom_registry_creates_one_instance -- proves: If someone reverts per-dep registry routing, this test fails. [devx]
  assert ops_cls.call_args == call(registry_url=url)
• [nit] Self-defined (registry=False) exclusion test only proves non-pollution, not correct installation
  Mocks _check_self_defined_servers_needing_installation to return [] -- self-defined install path is unchanged by this PR and tested elsewhere; noting for completeness.
  Proof (unknown): tests/unit/integration/test_mcp_per_dep_registry.py::test_false_registry_deps_are_excluded_from_registry_groups -- proves: Self-defined deps do not pollute registry groups. [devx]

Performance Expert -- inactive

PR touches integration/mcp_integrator_install.py only -- outside the perf-hot-path file set. The per-registry-group serial loop adds O(N) sequential HTTP round-trips where N = distinct registry URLs (typically 1-2); parallelizing groups would shave ~100-300ms per extra registry but is not blocking given common-case N=1.

_{This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.}

[danielmeppiel]

Panel follow-ups folded into the PR

Maintainer asked for the high-consensus panel recommendations to land in-PR. All green on 113ba88 -- summary below.

Folded (this PR)

• CHANGELOG [Unreleased] Fixed entry -- frames per-dep registry routing as an enterprise capability unlock. Three-way panel consensus (doc-writer / oss-growth / devx-ux).
• Docs example: "Using a private MCP registry" in docs/src/content/docs/consumer/install-mcp-servers.md, with parity examples in docs/src/content/docs/producer/author-primitives/mcp-as-primitive.md and packages/apm-guide/.apm/skills/apm-usage/dependencies.md (agent-facing source-of-truth for the MCP dep schema).
• Actionable error on invalid per-dep registry URL -- MCPServerOperations(registry_url=group_registry_url) is now wrapped in try/except ValueError that points the user at the apm.yml dep (and strips the misleading Check MCP_REGISTRY_URL if set env-var hint when the URL came from a per-dep field). Devx-ux recommendation.
• Integration test at tests/integration/test_mcp_install_flow.py::TestPerDepRegistryEndToEnd -- drives run_mcp_install with a real MCPServerOperations + real SimpleRegistryClient, mocking only the HTTP layer (SimpleRegistryClient.find_server_by_reference), and asserts the captured self.registry_url equals the per-dep URL. Mutation-break gate verified locally by forcing registry_url=None. Test-coverage-expert recommendation.
• Pre-existing test drift fixed: test_registry_overlay_warns_when_string in two integration files and test_custom_registry_overlay_emits_warning in tests/unit/test_mcp_overlays.py still asserted the removed "not yet applied" warning -- now inverted to match the documented new behaviour, mirroring test_registry_str_overlay_no_longer_emits_warning.

Deferred to follow-up issues

• CLI logging: per-group heartbeat / success-line registry annotation (cli-logging + devx-ux). Touches multiple logging paths; worth a dedicated PR.
• Supply-chain: optional allowed_registries field on McpPolicy for enterprise registry lockdown (supply-chain-security). Architectural addition, not on the critical path.
• Python-architect nits: operations: Any -> typed MCPServerOperations; 13-param helper -> MCPInstallContext dataclass. Both explicitly tagged as nits / follow-ups by the panel.
• Auth-expert nit: SECURITY comment in SimpleRegistryClient. Trivial, but reads cleaner as a doc-only follow-up.

Validation

• Lint: uv run --extra dev ruff check src/ tests/ && uv run --extra dev ruff format --check src/ tests/ -- silent.
• Tests: 349 passed across the focused MCP suite locally; CI shard 1+2, coverage combine, lint, gate, NOTICE drift, CodeQL all green on 113ba88.

Co-authored-by: @sergio-sisternes-epam for the original fix.