fix(audit): detect local-path sub-package MCP source drift (closes #2127)

[danielmeppiel]

apm audit --ci derived current MCP configs only from the root manifest, so changes in a locked local-path sub-package could incorrectly pass against a stale lockfile baseline. This change re-parses lock-bounded local package manifests, merges their MCP declarations after root declarations with the existing first-wins deduplication, and feeds the complete current config set into the existing drift comparison.

How to test

• uv run --extra dev python -m pytest tests/unit/policy/test_ci_checks.py -q
• uv run --extra dev python -m pytest tests/integration/test_transitive_mcp_audit_e2e.py -q
• Run the canonical lint mirror.

Closes #2127

apm-spec-waiver: Correctness fix enforces existing lockfile audit semantics; no new OpenAPM behavior.

[Copilot]

Pull request overview

Fixes a false-negative in apm audit --ci config-consistency by re-deriving current MCP server configs from lock-bounded local-path sub-package manifests (not just the root manifest), so post-install local source drift is compared against the lockfile baseline.

Changes:

• Extend _check_config_consistency() to parse apm.yml files for source: local locked dependencies and merge their MCP declarations (root-first, first-wins dedupe).
• Add a unit regression test covering local-path sub-package MCP config drift vs lockfile baseline.

Show a summary per file

File	Description
src/apm_cli/policy/ci_checks.py	Re-derives current MCP configs by reading lock-bounded local dependency manifests before drift comparison.
tests/unit/policy/test_ci_checks.py	Adds a regression test ensuring local-path sub-package MCP config drift fails config-consistency.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Low

[danielmeppiel]

APM Review Panel: ship_with_followups

Closes a governance gap: audit now detects MCP source drift in local-path sub-packages, with one robustness follow-up pending for corrupt-lockfile resilience.

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

Four active reviewers independently converged on one correctness gap: resolve_local_dep_dir and APMPackage.from_apm_yml can raise inside the new _check_config_consistency path, and the current code does not catch those errors. That would crash audit --ci with a traceback instead of returning a structured CheckResult. The sibling _check_drift path demonstrates the expected pattern. The fix is bounded and in scope: convert resolution and parse failures into an actionable failed check.

The happy-path fix correctly extends audit to catch transitive MCP drift before it reaches production. The DevX reviewer did not produce schema-valid output after its retry cap; its return was excluded, while the same robustness issue was independently confirmed by four valid reviewers.

Aligned with: Governed by policy: audit now detects sub-package MCP source drift that previously went unnoticed. Secure by default: checking transitive MCP declarations reduces unaudited tool-source drift.

Growth signal. This reliability fix reinforces APM's governance narrative without touching conversion surfaces. It is a useful patch-release reliability note.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	1	Guard local resolution and manifest parsing so audit always returns a structured result.
CLI Logging Expert	0	1	1	A corrupt local dependency currently produces a traceback instead of a clean check failure.
DevX UX Expert	0	0	0	Schema retry cap reached; no unique finding accepted.
Supply Chain Security Expert	0	1	1	Drift coverage is stronger; resolution failures still need structured handling.
OSS Growth Hacker	0	0	0	Reliability strengthens the governance narrative.
Test Coverage Expert	0	1	0	Happy-path coverage is strong; the resolution-error regression trap is missing.

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

Top 3 follow-ups

1. [Python Architect] Wrap local dependency resolution and manifest parsing in targeted exception handling that returns a failed CheckResult -- malformed local state must not crash CI audit.
2. [Test Coverage Expert] Add a regression test proving LocalResolutionError becomes a structured config-consistency failure -- a live probe confirmed the exception currently propagates.
3. [CLI Logging Expert] Include the affected dependency and a concrete recovery action in the failure detail -- operators need to know which local package failed and how to repair the graph.

Architecture

classDiagram
    class APMPackage
    class LockFile
    class LockedDependency
    class MCPIntegrator
    class CheckResult
    class ConfigConsistencyCheck
    class LocalResolutionError
    ConfigConsistencyCheck ..> APMPackage : reads manifests
    ConfigConsistencyCheck ..> LockFile : iterates dependencies
    ConfigConsistencyCheck ..> MCPIntegrator : deduplicates configs
    ConfigConsistencyCheck ..> CheckResult : returns
    LockFile *-- LockedDependency : contains
    ConfigConsistencyCheck ..> LocalResolutionError : must convert to failure

Loading

flowchart TD
    A[apm audit --ci] --> B[Config consistency]
    B --> C[Root MCP dependencies]
    C --> D[Resolve locked local packages]
    D --> E[Parse local apm.yml]
    E --> F[Root-first deduplication]
    F --> G[Compare with lock baseline]
    D --> H[Structured failed CheckResult]
    E --> H

Loading

Recommendation

Fold the bounded resolution/parse guard and its regression test into this PR, then re-run the panel. The core drift-detection change is correct on the happy path and already has focused coverage.

---

Full per-persona findings

Python Architect

• [recommended] resolve_local_dep_dir can raise LocalResolutionError at src/apm_cli/policy/ci_checks.py:243.
  The new check must return CheckResult rather than let the exception escape. Mirror the sibling _check_drift failure pattern.
• [nit] APMPackage.from_apm_yml can raise on malformed sub-package manifests at src/apm_cli/policy/ci_checks.py:247.
  Handle parse failures at the same check boundary.

CLI Logging Expert

• [recommended] Unguarded local resolution and parsing can surface a raw traceback at src/apm_cli/policy/ci_checks.py:243.
  Return an actionable config-consistency detail instead.
• [nit] Include a diagnostic breadcrumb identifying the local package that could not be scanned.

DevX UX Expert

No accepted findings. The reviewer returned malformed output after the retry cap; no unique signal was lost because other reviewers independently identified the same failure-mode issue.

Supply Chain Security Expert

• [recommended] A corrupt local dependency graph crashes the audit rather than failing the check at src/apm_cli/policy/ci_checks.py:243.
  Preserve fail-closed behavior through a structured result with remediation.
• [nit] Missing local manifests should produce a useful diagnostic instead of a silent skip.

OSS Growth Hacker

No findings.

Auth Expert -- inactive

The changed policy and unit-test files do not affect authentication, tokens, credentials, hosts, or authorization.

Doc Writer -- inactive

The internal audit correctness fix changes no reader-facing command, flag, schema, or prose contract.

Test Coverage Expert

• [recommended] The resolution-error path lacks a regression trap at tests/unit/policy/test_ci_checks.py.
  Proof (missing at): tests/unit/policy/test_ci_checks.py::TestConfigConsistency::test_local_resolution_error_in_config_consistency_fails_gracefully -- proves: apm audit --ci reports corrupt local dependency graphs without an unhandled exception.

Performance Expert -- inactive

The changed audit path is not an install, update, fetch, resolve, materialize, cache, or transport hot path.

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

[danielmeppiel]

APM Review Panel: ship_now

Audit now detects local-path sub-package MCP source drift with fail-closed checks, mutation-proven regression traps, and fully green CI.

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

All six active reviewers returned zero findings. The architecture review confirms the extension follows established resolver and collect-then-render patterns. The integrity review confirms fail-closed scanning, bounded YAML loading, and root-first deduplication. Test review confirms regression traps for local resolution errors, missing manifests, and parse failures. CLI and DevX review confirm the failure details are actionable. Copilot's one inline finding is resolved in 8d026438.

There is no dissent. This closes a silent audit false-negative while preserving the existing command and manifest contracts.

Aligned with: Governed by policy: config-consistency now enforces local-path MCP source validity. Secure by default: resolution and parse failures fail the audit instead of disappearing. Pragmatic as npm: diagnostics identify the package and a recovery action.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	0	0	Bounded extension follows established patterns; no architecture concerns remain.
CLI Logging Expert	0	0	0	Error details are actionable, guarded, and tested.
DevX UX Expert	0	0	0	Local package failures now produce clear recovery guidance.
Supply Chain Security Expert	0	0	0	Local MCP scanning remains fail-closed with root-first deduplication.
OSS Growth Hacker	0	0	0	Internal reliability improvement is ready to ship.
Test Coverage Expert	0	0	0	Critical branches have mutation-proven regression traps and green CI.

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

Recommendation

Ready for maintainer review. No panel follow-ups remain.

Folded in this run

• (panel) Convert local dependency resolution and manifest parse errors into structured config-consistency failures -- resolved in 8d026438924c6081466eb9257222e38b5b17ed7d.
• (panel) Report missing local package manifests instead of silently skipping the scan -- resolved in 8d026438924c6081466eb9257222e38b5b17ed7d.
• (panel) Add regression traps for resolution, parse, and missing-manifest failure paths -- resolved in 8d026438924c6081466eb9257222e38b5b17ed7d.
• (copilot) Handle LocalResolutionError and local manifest parse failures without crashing audit -- resolved in 8d026438924c6081466eb9257222e38b5b17ed7d.

Copilot signals reviewed

• src/apm_cli/policy/ci_checks.py -- LEGIT: the new local-package scan could raise on corrupt resolution chains or invalid manifests; resolved in 8d026438924c6081466eb9257222e38b5b17ed7d.

Regression-trap evidence (mutation-break gate)

• TestConfigConsistency::test_local_resolution_error_fails_gracefully -- deleted the LocalResolutionError guard; test FAILED as expected; guard restored.
• TestConfigConsistency::test_invalid_local_package_manifest_fails_gracefully -- deleted the parse-error guard; test FAILED as expected; guard restored.
• TestConfigConsistency::test_missing_local_package_manifest_fails_gracefully -- deleted the missing-manifest failure detail; test FAILED as expected; guard restored.

Lint contract

The full CI mirror exited 0 before each push: ruff check, ruff format check, pylint R0801, and auth-signal lint. No diagnostics remained.

CI

All 13 reported checks passed on 9bf52719832ac8dbc16b2bcf36cf85e78e0e94cd, including Lint, both Build/Test shards, Coverage, CodeQL, Spec conformance, and Merge Gate. Final CI run: https://github.com/microsoft/apm/actions/runs/29117129153 (after 1 CI recovery iteration).

Mergeability status

Captured from gh pr view 2132 --repo microsoft/apm --json number,headRefOid,mergeable,mergeStateStatus,statusCheckRollup after the final push.

PR	head SHA	CEO stance	iters	folds	defers	Copilot rounds	CI	mergeable	mergeStateStatus	notes
#2132	9bf5271	ship_now	1	4	0	2	green	MERGEABLE	BLOCKED	pending required review

Convergence

1 outer iteration; 2 Copilot rounds. Final panel stance: ship_now.

Ready for maintainer review.

---

Full per-persona findings

Python Architect

No findings.

CLI Logging Expert

No findings.

DevX UX Expert

No findings.

Supply Chain Security Expert

No findings.

OSS Growth Hacker

No findings.

Auth Expert -- inactive

The policy audit change does not affect authentication, tokens, credentials, hosts, or authorization.

Doc Writer -- inactive

The fix changes no reader-facing command, flag, schema, or prose contract.

Test Coverage Expert

No findings. The targeted audit suite passed 77 tests, and GitHub CI is green.

Performance Expert -- inactive

The audit consistency path is not a package-manager performance hot path.

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