GeezerVison

Thank you for contributing this Claude configuration repair. Issue #3252 has an explicit scope-approval record, and this PR appears to address the approved project-scope fix while documenting compatibility findings for other harnesses. The requested reviewers include Daniel Meppiel (@danielmeppiel) and Sergio Sisternes (@sergio-sisternes-epam). This is a recommendation for human review only, not merge approval.

Generated by autopilot-pr-triage-worker. This comment is AI-generated and may contain errors.

Hi Aaryan Dadu (@Aaryan-Dadu), thanks again for this fix. The scope is accepted, and everything is green except one required check: Spec conformance gate, at the step "Mode B detector (silent-extension gate on critical paths)".

The detector flags 57 new substantive lines under OpenAPM critical paths with no spec citation:

• src/apm_cli/install/mcp/ownership.py
• src/apm_cli/integration/mcp_integrator.py
• src/apm_cli/integration/mcp_integrator_install.py

This PR changes observable MCP install behaviour (env references and ownership on reinstall), so please add a spec citation on this branch:

1. A new <a id="req-XXX"></a> anchor with short normative prose in docs/src/content/docs/specs/openapm-v0.1.md, plus the matching Appendix C row.
2. A matching entry in docs/src/content/docs/specs/manifests/openapm-v0.1.requirements.yml.
3. A @pytest.mark.req("req-XXX") test under tests/spec_conformance/ that pins the new behaviour (it can reuse the setup from your existing tests).

Use the next free req- number. You can check alignment locally with uv run --extra dev python -m tests.spec_conformance.orphan_check and bash tests/spec_conformance/mode_b_detector.sh. Please also tick the matching boxes in the "Spec conformance" section of the PR description. CONTRIBUTING.md, under "Mode B (silent extension)", has the details.

Once that check is green, this can go on to review. Thanks!

Generated by autopilot-pr-ready-worker. This comment is AI-generated and may contain errors.

Sergio Sisternes (@sergio-sisternes-epam) I added the spec rule and tests for this. It passes locally, and the checks need approval to run.

APM Review Panel: ship_now

Closes a Tier-1 secret-leakage vector: Claude project .mcp.json now receives ${VAR} references instead of plaintext secrets, with fail-safe exit-on-unprovable ownership and names-only diagnostics.

panel-mode=full; personas=python-architect,test-coverage-expert,doc-writer,supply-chain-security-expert(adhoc: Tier1 secret exposure),cli-logging-expert(adhoc: diagnostic wording),devx-ux-expert(adhoc: install exit on unprovable entries),apm-ceo

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

Six panelists converged. Supply-chain security confirms no expanded value reaches .mcp.json, apm.lock.yaml or console output on the project path, and that migration only replaces an entry APM can prove it wrote (recorded write hash, or exact expansion of the declaration from the current environment). Test coverage confirms each done_when axis of the #3252 scope record is asserted, including installed-CLI runs with synthetic values. Every recommended and nit finding was folded into this branch; one architecture follow-up is deferred because it changes Codex wording outside this Claude-only scope.

Shepherd notes: main had landed #3406 (Codex env_vars) and #3493 (declared self-defined keys) after this PR was opened. The merge keeps one legacy-entry hook (matches_legacy_managed_server_config, which #3406 introduced) and has Claude override it, rather than adding a parallel hook. It also fixes a merge regression where Claude's upgrade notice and conflict guidance looked up slash-named self-defined servers by the truncated key. The spec changelog row became 0.1.57 (0.1.56 was taken on main); the working draft has 137 statements (132 MUST, 5 SHOULD).

Aligned with: secure by default (no secrets in shared config, lock or output; fail-safe on unprovable entries); portable by manifest (references survive across machines); pragmatic as npm (the error says which reference goes in which field).

Panel summary

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

Top 1 follow-ups

1. [Python Architect] Open a follow-up issue to converge the Codex expanded_env conflict wording (fix(codex): forward ${VAR} stdio env values with env_vars instead of writing them #3406) with the adapter native_conflict_guidance hook. -- Two runtime-exclusive guidance paths invite wording drift as more harnesses gain reference support.

Recommendation

All panelists signal ship. Every in-scope finding is folded on this head and CI is green. The one deferred item is a cross-harness refactor outside #3252's Claude-only scope.

Folded in this run

• (panel) Conflict guidance names which ${VAR} belongs in each field (names only, never values or defaults) -- resolved in c3f29d3.
• (panel) --force caveat on its own line; Claude upgrade notice says "the project .mcp.json" -- resolved in c3f29d3.
• (panel) Opaque adapter-owned snapshot contract documented for the _begin_env_tracking / _record_env_diagnostics pair -- resolved in c3f29d3.
• (panel) Test that Claude redeclares every mutable Copilot diagnostic bucket; symmetric project-scope assertion -- resolved in c3f29d3.
• (panel) Comment on the deliberate empty-value-as-unset expansion (fail-safe conflict) -- resolved in c3f29d3.
• (shepherd) Upgrade snapshot and conflict guidance use the declared self-defined key after fix(mcp): preserve declared self-defined server names containing slashes #3493 -- resolved in c3f29d3.
• (shepherd) Merge with fix(codex): forward ${VAR} stdio env values with env_vars instead of writing them #3406: one legacy-entry hook; spec row 0.1.57; CONFORMANCE regenerated -- resolved in 64f9f24.
• (panel) MCP variable compatibility page: Codex project and user rows split for fix(codex): forward ${VAR} stdio env values with env_vars instead of writing them #3406 env_vars; known gaps updated -- resolved in 5e68347.
• (panel) Producer page no longer uses Codex project env as the literal-values example -- resolved in 5e68347.
• (panel) Consumer page says install ends with an error naming fields and references -- resolved in 5e68347.
• (panel) CHANGELOG entry trimmed to one line ending (#3377) -- resolved in 5e68347.

Copilot signals reviewed

• review 5637683746 -- NOT-LEGIT: quota notice, not a review finding; the round-2 re-fetch returned zero inline comments.

Deferred (out-of-scope follow-ups)

• (panel) Converge Codex expanded_env conflict wording with the adapter native_conflict_guidance hook -- scope boundary: changes Codex conflict wording owned by [BUG] Codex: stdio MCP env ${VAR} written as plaintext into .codex/config.toml instead of using env_vars #3393/fix(codex): forward ${VAR} stdio env values with env_vars instead of writing them #3406; [BUG] --target claude writes resolved secrets as plaintext into project-scoped .mcp.json (VS Code/Copilot keep ${VAR} placeholders) #3252 is the Claude project repair only; suggested follow-up: a refactor issue for unified per-adapter MCP conflict guidance.

Regression-trap evidence (mutation-break gate)

• tests/unit/test_claude_mcp.py::TestClaudeLegacyEntries::test_conflict_guidance_finds_slash_named_self_defined_entry -- deleted the declared-name lookup in native_conflict_guidance; test FAILED as expected; guard restored.
• tests/unit/test_claude_mcp.py::TestClaudeLegacyEntries::test_upgrade_snapshot_uses_declared_self_defined_key -- deleted server_info from the snapshot key lookup; test FAILED as expected; guard restored.
• tests/unit/test_claude_mcp.py::TestClaudeDiagnosticBucketIsolation::test_every_mutable_copilot_classvar_is_redeclared -- deleted Claude's _security_upgraded_keys redeclaration; test FAILED as expected; guard restored.

Lint contract

uv run --extra dev ruff check and ruff format --check (src/, tests/, architecture linter scripts) both silent; pylint R0801 10.00/10; lint-auth-signals.sh and lint-architecture-boundaries.sh clean. CONFORMANCE.{md,json} regenerated after collecting the full spec suite (byte-identical); orphan_check reports 137 aligned requirements.

CI

CI on 9cf0adc: 24 checks pass, 3 skipped, including Lint, Build & Test shards, Lifecycle Smoke, Windows gates, Spec conformance gate and CodeQL; license/cla passes (after 0 CI fix iteration(s)). A local Cursor e2e failure (test_cursor_registry_env_references_keep_optional_semantics[None-None]) also reproduces on main and is unrelated to this PR.

Mergeability status

Convergence

2 outer iteration(s); 2 Copilot round(s). Final panel stance: ship_now. Ready for maintainer review.

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

Generated by autopilot-pr-merge-worker. This comment is AI-generated and may contain errors.