Skip to content

fix: allow global direct MCP installs (closes #2548) - #2734

Open
Daniel Meppiel (danielmeppiel) wants to merge 8 commits into
mainfrom
fix/2548-global-mcp-install
Open

fix: allow global direct MCP installs (closes #2548)#2734
Daniel Meppiel (danielmeppiel) wants to merge 8 commits into
mainfrom
fix/2548-global-mcp-install

Conversation

@danielmeppiel

@danielmeppiel Daniel Meppiel (danielmeppiel) commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

fix(mcp): honor global scope for direct MCP installs

TL;DR

apm install -g --mcp NAME now uses the user-scope manifest, lockfile, and runtime configuration paths instead of being rejected before target resolution. The same install-scope decision now drives both the direct MCP branch and the standard install pipeline, while runtime capability filtering still skips workspace-only targets.

Note

Closes #2548.

Problem (WHY)

Approach (WHAT)

# Fix
1 Compute InstallScope once in install() from --global.
2 Pass that scope through the direct MCP handler to manifest and integration consumers.
3 Remove global-scope policy from the conflict validator; runtime adapters retain capability filtering.
4 Add behavioral and static mutation guards, then synchronize user-facing docs.

Implementation (HOW)

  • src/apm_cli/commands/install.py - computes one install scope before direct MCP dispatch and forwards it to user-manifest and integrator paths.
  • src/apm_cli/install/mcp/conflicts.py - removes the obsolete blanket rejection and no longer accepts scope as conflict input.
  • tests/unit/test_install_command.py - adds the [BUG] apm install -g --mcp is rejected outright: "MCP servers are project-scoped" #2548 regression trap proving that global direct MCP installs mutate only the user manifest and pass InstallScope.USER downstream.
  • tests/integration/test_config_surface_lifecycle_contract.py - exercises the branch CLI end to end and verifies the Claude user config write.
  • tests/unit/install/test_mcp_conflicts.py, tests/integration/test_wave2_adapters_coverage.py, and tests/unit/test_global_mcp_scope.py - align conflict coverage and preserve global-capable versus workspace-only runtime filtering.
  • .apm/architecture/owners/install-deployment.json, scripts/architecture_linter/checks/install_request_and_source.py, and architecture tests - register and enforce the single install-scope owner in the current sharded linter.
  • CLI/consumer docs, the apm-usage command resource, and openapm-v0.1.md - document user-manifest routing, zero/mixed target behavior, registry safety, and req-tg-014.

Diagrams

Legend: The dashed nodes are the scope-routing path changed by this PR; adapter capability filtering remains downstream.

flowchart LR
    subgraph Select[Scope selection]
        G[--global] --> S[InstallScope.USER]
    end
    subgraph Direct[Direct MCP install]
        S --> H[_handle_mcp_install]
        H --> M[user APM manifest]
        H --> I[MCPIntegrator]
    end
    subgraph Filter[Runtime capability filtering]
        I --> C[global-capable runtime]
        I --> W[workspace-only runtime]
        C --> U[user config write]
        W --> K[skip]
    end
    classDef new stroke-dasharray: 5 5;
    class S,H,M,I new;
Loading

Trade-offs

  • Thread the enum, not the boolean. The handler receives canonical InstallScope; it does not re-derive scope from global_.
  • Keep capability checks downstream. The blanket conflict was removed, but adapter-owned supports_user_scope filtering remains unchanged rather than duplicating a runtime allowlist at CLI ingress.
  • Add a dual guardrail. The extra static check is justified because this repairs split authority; a behavioral test alone would not prevent another hard-coded project scope.

Benefits

  1. Both documented apm install -g --mcp transport forms now pass CLI validation.
  2. Direct global MCP entries are persisted under ~/.apm/ and deployed to user runtime config, not the current project.
  3. Workspace-only targets remain filtered at user scope, with their existing warning and no runtime write.

Validation

Targeted suites:

1341 affected tests and 13 subtests passed

Real CLI lifecycle regression:

179 spec-conformance tests passed; 2 skipped

Canonical lint and boundary gates:

All checks passed!
1679 files already formatted
Your code has been rated at 10.00/10
[+] auth-signal lint clean
[+] architecture boundary lint clean

Mutation checks:

Behavior mutation: regression test failed when direct MCP scope was hard-coded to PROJECT.
Static mutation: the sharded architecture rule reported the removed user-scope forwarding guard.

Scenario Evidence

# Scenario (user promise) Principle(s) Test(s) proving it Type
1 apm install -g --mcp NAME writes the user manifest and a global-capable runtime config Multi-harness support, DevX (pragmatic as npm) tests/unit/test_install_command.py::TestInstallMcpFlag::test_global_mcp_uses_user_manifest_scope (regression-trap for #2548)
tests/integration/test_config_surface_lifecycle_contract.py::test_global_direct_mcp_uses_user_manifest_and_runtime_config
unit + e2e
2 Mixed, excluded, disabled-experimental, and zero-supported global target sets resolve before user-state mutation Multi-harness support, DevX (pragmatic as npm) tests/unit/test_install_command.py::TestInstallMcpFlag::test_global_mcp_rejects_workspace_only_target_before_manifest_write
tests/unit/test_install_command.py::TestInstallMcpFlag::test_global_mcp_filters_mixed_target_set_before_dispatch
tests/integration/test_config_surface_lifecycle_contract.py::test_global_direct_mcp_filters_mixed_and_rejects_zero_supported_targets
unit + e2e
3 A persisted MCP registry drives the next direct install without silently enabling plaintext HTTP Vendor-neutral, Secure by default, DevX tests/integration/test_config_surface_lifecycle_contract.py::test_configured_mcp_registry_drives_global_direct_install
tests/integration/test_config_surface_lifecycle_contract.py::test_ambient_registry_source_is_pinned_for_replay
tests/unit/install/test_mcp_registry_module.py::TestRegistryEnvOverride::test_http_url_preserves_separate_opt_in_when_disabled
e2e + unit
4 Ambient or query-string credentials do not reach the manifest or terminal output Secure by default tests/unit/test_install_command.py::TestInstallMcpFlag::test_registry_env_url_pins_source_without_http_opt_in
tests/unit/test_install_command.py::TestInstallMcpFlag::test_invalid_configured_registry_url_fails_before_dispatch
tests/unit/install/test_mcp_registry_module.py::TestValidateRegistryUrl::test_query_and_fragment_rejected
tests/integration/test_config_surface_lifecycle_contract.py::test_ambient_registry_credentials_never_reach_manifest_or_output
unit + e2e
5 A global direct MCP dry-run creates no user manifest, lockfile, or runtime config DevX tests/unit/test_install_command.py::TestInstallMcpFlag::test_global_mcp_dry_run_creates_no_user_state
tests/integration/test_config_surface_lifecycle_contract.py::test_global_direct_mcp_dry_run_creates_no_user_state
unit + e2e

How to test

  • Create ~/.apm/apm.yml with targets: [claude].
  • Run apm install -g --mcp probe --no-policy -- echo ready and expect exit code 0.
  • Confirm probe appears in ~/.apm/apm.yml and ~/.claude.json, not the project apm.yml.
  • Run the targeted regression tests above and expect all to pass.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

Review tier: Lite
Findings: 1 Medium severity

New issues introduced by this change (1)
Severity Finding
Medium severity src/​apm_cli/​commands/​install.py — In _handle_mcp_install, you compute resolved_registry_url using the full precedence chain (flag…
What changed in this PR

This PR fixes the apm install -g --mcp NAME direct-install path so it honors user (global) scope instead of being rejected and/or writing to project-scoped state, aligning the direct MCP branch with the standard install pipeline’s single scope decision.

Changes:

  • Compute InstallScope once in install() (from --global) and thread it through the direct --mcp handler.
  • Remove the blanket --global rejection from validate_mcp_conflicts.
  • Add regression + architecture guardrails (unit/integration tests + boundary lint), and update docs/usage references to describe user-scope behavior and workspace-only skips.
File Description
src/​apm_cli/​commands/​install.py Computes scope once and routes direct --mcp installs through scope-aware manifest/apm-dir paths.
src/​apm_cli/​install/​mcp/​conflicts.py Removes the obsolete “--global not supported” conflict from the MCP conflict matrix.
tests/​unit/​test_install_command.py Adds a unit regression test asserting -g --mcp mutates only the user manifest and threads scope to the integrator.
tests/​unit/​install/​test_mcp_conflicts.py Removes E2 coverage tied to the old global-scope rejection.
tests/​integration/​test_wave2_adapters_coverage.py Updates integration coverage to match the revised MCP conflict matrix API/behavior.
tests/​integration/​test_config_surface_lifecycle_contract.py Adds an e2e scenario verifying global direct MCP installs update ~/.apm/apm.yml and user runtime config (Claude).
tests/​integration/​test_architecture_authorities.py Adds an AST-based guard ensuring the direct MCP path consumes the install command’s scope decision.
scripts/​lint-architecture-boundaries.sh Adds a static boundary lint enforcing single-owner scope routing for direct MCP installs.
docs/​src/​content/​docs/​reference/​cli/​install.md Updates --global docs to describe -g --mcp user-manifest routing and workspace-only skip behavior.
docs/​src/​content/​docs/​consumer/​install-mcp-servers.md Clarifies that direct global installs read/update ~/.apm/apm.yml (no project fallback).
packages/​apm-guide/​.apm/​skills/​apm-usage/​commands.md Updates CLI usage guidance to reflect user-scope MCP behavior for -g --mcp.
.github/​instructions/​architecture.instructions.md Registers “Install command scope selection” as a canonical owner entry.
.apm/​instructions/​architecture.instructions.md Mirrors the same architecture canonical-owner addition for in-repo APM instructions.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/apm_cli/commands/install.py Outdated
apm_dir=mcp_apm_dir,
scope=mcp_scope,
scope=scope,
registry_url=validated_registry_url,
@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: needs_rework

PR #2734 restores one-command global MCP installs, but this HEAD still breaks targetless user-scope discovery and weakens registry URL security safeguards.

cc Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review.

The restored global direct-install workflow is valuable, but target resolution still consults the invoking project when the user manifest has no target. Python Architecture, DevX UX, and Test Coverage independently reproduced this gap, and no e2e regression trap covers it.

Supply Chain Security and Auth also converge on one in-scope regression: forwarding ambient registry URLs can implicitly permit HTTP and can carry credentials into persisted configuration or logs. Security should win for defaults; explicit CLI HTTP intent can remain ergonomic. The separate policy-root concern needs focused evidence before action.

Aligned with: Portable by manifest: user state now routes consistently, but targetless resolution must honor the same scope. Secure by default: ambient registry URLs must not silently enable HTTP or expose credentials. Multi-harness, multi-host: global runtime discovery must ignore project-only signals. Pragmatic as npm: the one-command flow should remain predictable.

Growth signal. Restoring apm install -g --mcp NAME removes setup friction and deserves a concise changelog entry.

Panel summary

Persona B R N Takeaway
Python Architect 1 0 0 Fix user-scope target resolution; the default global MCP path still scans the project.
CLI Logging Expert 0 1 0 Missing user-manifest recovery guidance points to a project-only command.
DevX UX Expert 1 1 0 Targetless discovery and unsupported-only selections violate the documented flow.
Supply Chain Security Expert 2 1 0 Preserve HTTPS defaults and prevent registry credential exposure.
OSS Growth Hacker 0 1 0 Give the restored workflow a changelog discovery path.
Auth Expert 2 0 0 Ambient registry URLs can bypass HTTP safeguards and expose credentials.
Doc Writer 0 1 1 Docs are accurate; add Hermes parity to two runtime lists.
Test Coverage Expert 1 1 0 Add targetless and persisted-registry e2e coverage.

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

Top 5 follow-ups

  1. [Test Coverage Expert] (blocking-severity) Thread user scope through target resolution and add a targetless global MCP e2e regression test -- the current path inherits project signals.
  2. [Supply Chain Security Expert] (blocking-severity) Preserve registry source provenance, require explicit HTTP opt-in, and prevent ambient URL credentials from persistence or logs -- this consolidates the Security and Auth findings.
  3. [OSS Growth Hacker] Add a concise Unreleased changelog entry for restored global direct MCP installs -- this is mandatory release hygiene for user-visible behavior.
  4. [DevX UX Expert] Preflight unsupported global target sets before mutating the user manifest -- a failed install should not leave partial global state.
  5. [CLI Logging Expert] Provide accurate recovery guidance when the user manifest is missing -- apm init creates project state, not ~/.apm/apm.yml.

Architecture

classDiagram
    direction LR
    class InstallCommand {
      <<ApplicationAdapter>>
      +install(global_, mcp_name, target)
      +_handle_mcp_install(scope, target)
    }
    class InstallScope {
      <<Enum>>
      PROJECT
      USER
    }
    class ScopePaths {
      <<CanonicalOwner>>
      +get_manifest_path(scope) Path
      +get_apm_dir(scope) Path
    }
    class TargetResolver {
      <<CanonicalOwner>>
      +resolve_manifest_target_decision(root, manifest_path, explicit_target) EffectiveTargetDecision
    }
    class MCPCommand {
      <<Orchestrator>>
      +run_mcp_install(scope, target_decision)
    }
    class MCPIntegrator {
      <<Facade>>
      +install(scope, user_scope, target_decision)
    }
    class ScopeBoundaryGuard {
      <<StaticBoundaryCheck>>
      +check_install_scope_selection(provider)
    }
    InstallCommand ..> InstallScope : computes once
    InstallCommand ..> ScopePaths : resolves paths
    InstallCommand ..> TargetResolver : resolves targets
    InstallCommand ..> MCPCommand : dispatches
    MCPCommand ..> MCPIntegrator : delegates
    ScopeBoundaryGuard ..> InstallCommand : verifies
    note for TargetResolver "Gap: user-scope intent is not forwarded"
    class InstallCommand:::touched
    class ScopeBoundaryGuard:::touched
    classDef touched fill:#fff3b0,stroke:#d47600
Loading
flowchart TD
    A["apm install -g --mcp NAME"] --> B["install(): scope = InstallScope.USER"]
    B --> C["validate_mcp_conflicts()"]
    C --> D["_handle_mcp_install(scope=USER)"]
    D --> E["[I/O] get_manifest_path(USER): ~/.apm/apm.yml"]
    E --> F{"resolve_manifest_target_decision(): target source"}
    F -->|explicit or manifest| G["EffectiveTargetDecision"]
    F -->|none| H["[I/O] current gap: resolve_targets(Path.cwd())"]
    H --> I["Project markers or exit 2"]
    G --> J["[FS] run_mcp_install(): user manifest and runtime config"]
    I --> J
    J --> K["[LOCK] update ~/.apm/apm.lock.yaml"]
Loading

Recommendation

Address targetless scope resolution and ambient registry security, add durable regression coverage, and include the changelog line before reassessing. Keep policy-root behavior as a lower-priority investigation unless focused evidence proves it incorrect.


Full per-persona findings

Python Architect

  • [blocking] Global MCP target resolution still uses project-scope auto-detection at src/apm_cli/commands/install.py:766.
    Forward user_scope through the canonical resolver and extend the e2e and static guard.

CLI Logging Expert

  • [recommended] Missing user-manifest recovery guidance recommends apm init, which creates project state.

DevX UX Expert

  • [blocking] Targetless global MCP installs inherit project detection.
  • [recommended] Unsupported-only global target sets can mutate the user manifest before reporting failure.

Supply Chain Security Expert

  • [blocking] Ambient registry configuration can implicitly opt into plaintext HTTP.
  • [blocking] Environment URL credentials can reach manifests and verbose logs.
  • [recommended] Policy discovery root parity needs a focused test before changing semantics.

OSS Growth Hacker

  • [recommended] Add the restored command to the Unreleased changelog.

Auth Expert

  • [blocking] Ambient HTTP registry URLs bypass the separate opt-in.
  • [blocking] Ambient registry credentials can be persisted and printed.

Doc Writer

  • [recommended] Add Hermes-when-enabled parity to two global runtime lists.
  • [nit] The necessary documentation additions slightly increase word count.

Test Coverage Expert

  • [blocking] Add targetless global MCP e2e coverage with a conflicting project signal.
  • [recommended] Persisted registry routing currently has only mocked unit coverage.

Performance Expert -- inactive

Straight-line routing, linter, tests, and docs do not touch a performance surface.

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

@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Spec Guardian: next_brief

Scope: editorial-patch; diff = +49/-11 lines across 1 spec artifact. Shocked-meter avg: 7.75/10.

The average clears the ship-meter floor, but the package-manager panel found one round-2 mixed-target replay issue, so the binding B-item rule selects next_brief. The orchestrator folded that issue immediately after the panel: req-tg-014 now forbids persisting or remapping unsupported members, bootstrap uses the supported runtime set, and the regression test proves a second targetless replay stays on that same set.

The two-round editorial budget is exhausted. The mechanical linter and conformance suite now pass; the remaining action is maintainer verification of the post-panel fold rather than another automated panel round.

Convergence

Panel Stance Shocked New B New R New N
Swagger Editor ship_with_followups 8/10 0 2 1
OCI Editor ship_with_followups 8/10 0 1 0
Package Manager Editor needs_next_brief 6/10 1 1 0
TAG Architect ship 9/10 0 0 1

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

Convergent themes

  • T1 -- Environment-independent target bootstrap tests (supporting: sw-rec-r2-2, oci-rec-r2-1)
  • T2 -- Exact all semantics and correct target anchor (supporting: sw-nit-r2-1, pkg-rec-r2-1, tag-nit-r2-1)

Fold now (5 items)

  1. [F1 / standalone] req-tg-014 -- Retain the explicit MUST that missing declared capability means unsupported.
    Success criterion: req-tg-014 contains the explicit MUST.
  2. [F2 / T2] Section 4.2.1 -- Retain explicit all expansion and the #421-target link.
    Success criterion: no #421-targets reference remains.
  3. [F3 / standalone] req-tg-014 mixed persistence -- Retain supported, replay-equivalent target identifiers and forbid unsupported persistence or remapping.
    Success criterion: a second targetless replay selects the same supported set.
  4. [F4 / standalone] target bootstrap -- Retain bootstrap construction from supported_runtimes.
    Success criterion: bootstrap does not persist original unsupported aliases.
  5. [F5 / T1] bootstrap tests -- Retain deterministic discovery and mixed replay assertions.
    Success criterion: targetless and mixed-set focused tests pass.

Linter notes (1 scope note)

  • [11] Non-spec Python files are part of this PR; the general apm-review-panel ran in parallel. Checks 1-10 pass, including ASCII, forbidden tokens, schemas, fixtures, 120 unique anchors, count reconciliation, links, fixture citations, and changelog coverage. Mermaid check skipped because the spec contains zero Mermaid blocks.

Linter handoff: req-tg-014 uses MUST; #421-target resolves; mixed persistence is replay-equivalent; bootstrap uses supported_runtimes; the conformance suite reports 178 passed and 2 skipped; normative count remains 120 (115 MUST, 5 SHOULD).


Full round-2 panel findings

Swagger Editor -- shocked 8/10, confidence high

  • [recommended] Make the missing-capability rule explicitly normative. Folded.
  • [recommended] Remove the environment-dependent targetless unit assertion. Folded.
  • [nit] Correct the Section 4.2.1 fragment. Folded.

OCI Editor -- shocked 8/10, confidence high

  • Round-1 pre-mutation issue closed.
  • [recommended] Make the targetless unit deterministic. Folded.

Package Manager Editor -- shocked 6/10, confidence high

  • [blocking] Persisting the original mixed selection could change runtime selection on replay. Folded after panel into spec, implementation, and test.
  • [recommended] Define all behavior directly and fix its link. Folded.

TAG Architect -- shocked 9/10, confidence high

  • Both round-1 scope and capability-contract issues closed.
  • [nit] Correct the Section 4.2.1 fragment. Folded.

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

@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

APM Review Panel: ship_with_followups

PR #2734 closes the global direct-MCP correctness and security gaps; only two scope-crossing architecture refactors remain to track.

cc Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review.

At 6d04d3600bb1b3ef120b9e70d40e109b2c88e6d7, concrete security, auth, DevX, documentation, and coverage findings are folded. Exact HTTP opt-in, credential-safe endpoint validation, ambient registry provenance, pre-write target selection, dry-run behavior, and user-root discovery are all regression-tested.

The Python Architect's remaining concerns are valid debt but require dedicated cross-module migrations rather than expanding this bug fix. Current GitHub CI is green.

Aligned with: Secure by default: endpoint credentials are rejected or redacted and plaintext HTTP requires an exact opt-in. Portable by manifest: validated ambient registry identity persists for deterministic replay. Pragmatic as npm: global direct installs now have predictable target, dry-run, and recovery behavior.

Panel summary

Persona B R N Takeaway
Python Architect 2 0 0 Two cross-cutting owner consolidations should move to dedicated architecture work.
CLI Logging Expert 0 0 0 Output is actionable, redacted, scope-aware, and dry-run aware.
DevX UX Expert 0 1 0 Persisted HTTP recovery guidance was folded.
Supply Chain Security Expert 2 0 0 Strict HTTP parsing and ambient provenance findings were folded.
OSS Growth Hacker 0 0 0 Changelog, docs, and runnable evidence form a complete adoption story.
Auth Expert 1 0 0 Embedded client userinfo rejection was folded.
Doc Writer 0 2 0 HTTP opt-in and precise no-mutation wording were folded.
Test Coverage Expert 0 1 0 The excluded-target recovery assertion was folded.

B = blocking-severity findings, R = recommended, N = nits.
Counts show signals raised during the terminal pass; the sections below record their disposition.

Top 2 follow-ups

  1. [Python Architect] Introduce one immutable MCP target plan shared by command and integrator -- this removes duplicate planning but requires a cross-module API migration.
  2. [Python Architect] Centralize registry parsing and safe display in a RegistryEndpoint value object -- this requires registry-wide migration across config, install, client, and command surfaces.

Recommendation

Ship the focused fix with green CI, then track the canonical MCP target plan and RegistryEndpoint consolidation as dedicated architecture work.

Folded in this run

  • (Copilot) Apply persisted registry configuration during direct MCP integration -- resolved in 8ab1773351.
  • (panel) Port the scope owner guard to the sharded architecture linter -- resolved in 1f8da5be26.
  • (panel) Thread user scope through target resolution and discover from the user deploy root -- resolved in 2dd2a5d752.
  • (panel) Reject zero-supported, excluded, and disabled experimental targets before writes -- resolved in 5e6ae8e628.
  • (panel) Filter mixed target sets and persist replay-equivalent supported targets -- resolved in 5bc5cdc745.
  • (panel) Keep global direct MCP dry-runs write-free -- resolved in 2dd2a5d752.
  • (panel) Require exact ambient/config HTTP opt-in and pin validated registry provenance -- resolved in 6d04d3600b.
  • (panel) Reject and redact userinfo, query, fragment, and malformed-port values end to end -- resolved in 6d04d3600b.
  • (panel) Add real-binary target, registry, redaction, replay, exclusion, and dry-run coverage -- resolved in 6d04d3600b.
  • (panel) Add req-tg-014, generated conformance disclosure, changelog, CLI docs, and apm-guide parity -- resolved in 64be60be4b.

Copilot signals reviewed

  • src/apm_cli/commands/install.py:844 -- LEGIT: persisted apm config registry selection was diagnosed but not applied during integration (resolved in 8ab1773351).

Deferred (out-of-scope follow-ups)

  • (panel) Introduce one immutable MCP target plan -- scope boundary: PR scope is one global direct-MCP bug; this requires a cross-module planner API migration across every MCP install caller.
  • (panel) Centralize registry parsing and safe display -- scope boundary: PR scope is direct global MCP behavior; this requires registry-wide migration across config, install, client, and command surfaces.

Regression-trap evidence (mutation-break gate)

  • Targetless user discovery and the static scope owner failed when user_scope=is_user_scope(scope) was removed; restored.
  • Mixed, excluded, disabled-experimental, and zero-supported target tests failed when their pre-write guards were removed; restored.
  • Configured and ambient registry tests failed when provenance or exact HTTP opt-in was removed; restored.
  • Dry-run state tests failed when the preview return was bypassed; restored.
  • Userinfo, query, fragment, and malformed-port tests failed when rejection/redaction guards were removed; restored.
  • Generated req-tg-014 disclosure test failed when the disclosure was removed; restored.

Lint contract

The full local CI mirror passed at the exact final head: ruff check and format silent, YAML/file-length/relative-path guards clean, pylint R0801 10.00/10, auth-signal and architecture boundary lints clean.

CI

All 18 checks are conclusive on 6d04d3600bb1b3ef120b9e70d40e109b2c88e6d7: 17 SUCCESS and 1 SKIPPED. CI run: https://github.com/microsoft/apm/actions/runs/33481602427. Spec conformance: https://github.com/microsoft/apm/actions/runs/33481602377. One CI recovery iteration repaired the initial Mode B spec-citation failure.

Mergeability status

PR head SHA CEO stance iters folds defers Copilot rounds CI mergeable mergeStateStatus notes
#2734 6d04d36 ship_with_followups 4 12 2 2 green MERGEABLE BLOCKED pending required review

Convergence

4 outer iterations; 2 Copilot rounds. Final panel stance: ship_with_followups. All concrete in-scope follow-ups were folded; two scope-crossing refactors are recorded above.

Ready for maintainer review.


Full terminal disposition
  • Security/auth findings: folded and verified by unit plus real-binary tests.
  • DevX/docs findings: folded into CLI help, Starlight reference/consumer pages, config/MCP reference, and the shipped command resource.
  • Test evidence: 1341 affected tests and 13 subtests passed; 179 spec tests passed with 2 skipped; latest GitHub CI is green.
  • Architecture consolidation findings: deferred with explicit scope boundaries.

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

@danielmeppiel

Copy link
Copy Markdown
Collaborator Author

Shepherd-driver terminal advisory: converged at reviewed head

PR #2734 completed the Phase 4 convergence loop at 6d04d3600bb1b3ef120b9e70d40e109b2c88e6d7. All in-scope findings were folded, exact-head CI is green, and the terminal panel stance is ship_with_followups.

Folded in this run

  • (Copilot) Apply persisted registry configuration during direct MCP integration - resolved in 8ab1773351.
  • (panel) Port the scope owner guard to the sharded architecture linter - resolved in 1f8da5be26.
  • (panel) Thread user scope through target resolution and discover from the user deploy root - resolved in 2dd2a5d752.
  • (panel) Reject zero-supported, excluded, and disabled experimental targets before writes - resolved in 5e6ae8e628.
  • (panel) Filter mixed target sets and persist replay-equivalent supported targets - resolved in 5bc5cdc745.
  • (panel) Keep global direct MCP dry-runs write-free - resolved in 2dd2a5d752.
  • (panel) Require exact ambient/config HTTP opt-in and pin validated registry provenance - resolved in 6d04d3600b.
  • (panel) Reject and redact userinfo, query, fragment, and malformed-port values end to end - resolved in 6d04d3600b.
  • (panel) Add real-binary target, registry, redaction, replay, exclusion, and dry-run coverage - resolved in 6d04d3600b.
  • (panel) Add req-tg-014, generated conformance disclosure, changelog, CLI docs, and apm-guide parity - resolved in 64be60be4b.

Copilot signal reviewed

  • src/apm_cli/commands/install.py:844 - LEGIT: persisted apm config registry selection was diagnosed but not applied during integration; resolved in 8ab1773351.

Deferred (out-of-scope follow-ups)

  • (panel) Introduce one immutable MCP target plan - scope boundary: this focused global direct-MCP fix would become a cross-module planner API migration across every MCP install caller.
  • (panel) Centralize registry parsing and safe display - scope boundary: this direct global MCP fix would become a registry-wide migration across config, install, client, and command surfaces.

Canonical-owner and functional evidence

  • Evidence contract: architecture evidence v2, classification new-owner, dual guardrail required and satisfied.
  • Exact owner report: base 18e0da74d195cdd03f56b3ac04c88e0d2e2dbf65, head 6d04d3600bb1b3ef120b9e70d40e109b2c88e6d7, owner-table SHA-256 164f0fff895f8b1a735d13dbb09fcdae50fee629ba9def3798874e9c95e360ec.
  • Touched decisions: Effective install target selection; Install command scope selection; Install invocation option defaults; MCP target-selection precedence.
  • Exact-head functional tests: test_global_direct_mcp_uses_user_manifest_and_runtime_config and test_manifest_and_explicit_target_precedence_for_mcp_and_lsp - 2 passed in 3.32s.
  • Architecture mutation test: test_owner_rule_catches_its_guard_mutation[install-deployment-install-scope-selection] - 1 passed in 4.13s.
  • Static boundary lint: bash scripts/lint-architecture-boundaries.sh exited 0 with silent output.
  • Mutation-break: removing user_scope=is_user_scope(scope) broke targetless user discovery and triggered the scope-owner guard; the guard was restored.
  • owner_touch_gate.py verify semantically verified all four touched decisions against the exact base/head pair.

Lint and CI

The full exact-head lint mirror passed: ruff check and format silent; YAML, file-length, and relative-path guards clean; pylint R0801 10.00/10; auth-signal and architecture boundary lints clean.

All 18 checks are conclusive on the reviewed head: 17 SUCCESS and 1 SKIPPED. CI run: https://github.com/microsoft/apm/actions/runs/33481602427.

Mergeability snapshot

PR head SHA panel stance iters folds defers Copilot rounds CI mergeable mergeStateStatus notes
#2734 6d04d36 ship_with_followups 4 10 2 2 green CONFLICTING DIRTY post-wave main drift; Phase 5 conflict resolution handles this separately

The reviewed head remains the completed Phase 4 result. Main advanced after that terminal wave, so the new conflict is not an unresolved Phase 4 finding and will be handled by the composed Phase 5 mergeability gate. No code was modified or pushed while posting this advisory.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Pass the fully resolved registry URL into direct MCP integration so the persisted apm config value matches the diagnostic and behavior. Addresses Copilot inline on src/apm_cli/commands/install.py:844.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Thread user scope through target resolution, bootstrap the user manifest, reject workspace-only selections before writes, and preserve registry HTTP and credential boundaries. Adds exact lifecycle, architecture, and mutation evidence for panel follow-ups.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Validate and filter user-scope runtime capability before manifest bootstrap, preserve replay-equivalent mixed selections, and codify the contract as OpenAPM req-tg-014. Addresses review-panel and spec-guardian follow-ups.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reject opaque registry query credentials, redact ambient URL components, publish the req-tg-014 conformance disclosure, and align global-target help and docs. Addresses final review-panel follow-ups.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep direct MCP dry-runs write-free, discover runtimes from the user deploy root, and fail closed when rendering malformed or credential-bearing ambient registry URLs. Adds real-binary target and redaction coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply exclusions and experimental gates before user state, pin validated ambient registry identity for replay, require explicit ambient HTTP opt-in, and align dry-run guidance. Adds real-binary regression evidence for each path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Require exact plaintext-registry opt-in, reject embedded client credentials, validate persisted ports, and align CLI recovery guidance with the tested manifest and runtime boundaries.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] apm install -g --mcp is rejected outright: "MCP servers are project-scoped"

2 participants