fix(mcp): resolve self-defined stdio env ${VAR} placeholders at install time (closes #1963)

[danielmeppiel]

Why

Self-defined MCP transports were inconsistent: HTTP headers already resolved ${VAR} placeholders from the install process environment for literal-only harnesses, but self-defined stdio env values were passed back as env_overrides, causing the authored placeholder to shadow the real process environment. That made Claude Code and Codex write ${VAR} verbatim and break authenticated servers, a P6 reliability issue.

What changed

• Stopped passing self-defined stdio env blocks as env_overrides; the adapter now resolves _raw_stdio.env through the same placeholder pipeline used by headers.
• Added a regression test that installs the same self-defined stdio server into Claude Code and Codex and asserts ${MY_TOKEN} resolves while literal env values remain unchanged.
• Updated docs, the APM usage guide, and the changelog entry for the clarified behavior.

How to test

Exact repro:

rm -rf .scratch-1963
mkdir -p .scratch-1963/.claude .scratch-1963/.codex
cat > .scratch-1963/apm.yml <<'YAML'
name: repro-1963
version: 0.0.0
dependencies:
  mcp:
    - name: env-demo
      registry: false
      transport: stdio
      command: npx
      args: ["-y", "example-mcp"]
      env:
        MY_TOKEN: "${MY_TOKEN}"
        LITERAL_VALUE: "literal-value"
    - name: http-demo
      registry: false
      transport: http
      url: https://example.com/mcp
      headers:
        X-Token: "${MY_TOKEN}"
YAML
cd .scratch-1963
MY_TOKEN="repro-secret-not-a-real-token" uv run --project .. apm install --only mcp --target claude
cat .mcp.json
MY_TOKEN="repro-secret-not-a-real-token" uv run --project .. apm install --only mcp --target codex
cat .codex/config.toml

Validation run:

uv run --extra dev pytest tests/unit/install/test_mcp_integrator_install_flow.py::TestRunMcpInstallSelfDefined::test_self_defined_stdio_env_placeholders_resolve_from_process_env -q
uv run --extra dev pytest tests/unit/install/test_mcp_integrator_install_flow.py tests/unit/test_claude_mcp.py tests/unit/test_codex_adapter_phase3.py tests/test_codex_empty_string_and_defaults.py -q
uv run --extra dev ruff check src/ tests/
uv run --extra dev ruff format --check src/ tests/

Mutation-break gate: removed self_defined_env = {} if transport_label == "stdio" else dep.env or {} and restored self_defined_env = dep.env or {}; the new regression test failed for both Claude and Codex with ${MY_TOKEN} written verbatim (failed_without_guard=true).

Closes #1963

[Copilot]

Pull request overview

This PR fixes install-time resolution of ${VAR} placeholders for self-defined stdio MCP server env blocks, ensuring Claude Code and Codex receive literal env values rather than ${VAR} strings caused by env_overrides shadowing os.environ.

Changes:

• Adjusts self-defined MCP install flow so stdio env is resolved via the adapter pipeline (not reused as env_overrides).
• Adds a regression test covering both Claude (.mcp.json) and Codex (.codex/config.toml) output.
• Updates documentation/guide text and adds a changelog entry describing the behavior.

Show a summary per file

File	Description
tests/unit/install/test_mcp_integrator_install_flow.py	Adds regression test asserting ${MY_TOKEN} resolves for self-defined stdio servers in Claude + Codex.
src/apm_cli/integration/mcp_integrator_install.py	Stops passing self-defined stdio dep.env as env_overrides to avoid shadowing os.environ.
packages/apm-guide/.apm/skills/apm-usage/dependencies.md	Updates placeholder behavior notes (currently inconsistent with manifest-schema docs; see comments).
docs/src/content/docs/producer/author-primitives/mcp-as-primitive.md	Updates narrative on placeholder handling; suggested wording tweak to reflect prompting behavior.
CHANGELOG.md	Adds Unreleased entry (needs PR number reference update; see comment).

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 3
• Review effort level: Low

[danielmeppiel]

APM Review Panel: ship_now

Fixes self-defined stdio MCP env placeholder resolution for Claude Code and Codex by eliminating env_overrides shadowing and leveraging existing adapter resolution.

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

This PR resolves a real-world bug where ${VAR} placeholders in stdio MCP server definitions were not resolved when the adapter already handles environment variable expansion natively. The fix is minimal and precise: avoid injecting env_overrides that would shadow the adapter's own resolution logic. The shepherd cycle folded all panel and Copilot findings in commits 9ca597c57 and 122d237c: changelog entry clarified, code comment improved, a non-stdio branch test added, and the changelog now references the PR number.

Panel signals converge cleanly: python-architect finds the change structurally sound, supply-chain-security and auth see no new surface, CLI logging is unaffected, and test coverage now validates both the stdio and non-stdio branches with mutation-break evidence. No specialist raised a remaining in-scope finding after the fold.

Aligned with: portable_by_manifest: Ensures MCP server definitions in apm.yml resolve env placeholders consistently across adapters; multi_harness_multi_host: Fixes adapter-specific resolution for Claude Code and Codex; pragmatic_as_npm: Users expect env vars to just work in server definitions.

Growth signal. This removes a first-run trust issue for Claude Code and Codex MCP authors.

Panel summary

Persona	B	R	N	Takeaway
Python architect	0	0	1	Minimal data-flow fix uses the existing adapter resolution path; comment clarity was folded.
CLI logging expert	0	0	0	No CLI output or logging surface touched.
DevX UX expert	0	0	1	Behavior now matches user expectation for ${VAR} stdio env indirection; docs wording was folded.
Supply chain security expert	0	0	0	The fix improves secret placeholder handling without adding a token leak or new trust boundary.
OSS growth hacker	0	0	0	Closes a sharp MCP authoring reliability gap for Claude Code and Codex users.
Auth expert	0	0	0	Correct auth-adjacent behavior: stdio token placeholders now resolve from process environment.
Doc writer	0	1	0	Docs are aligned after the folded changelog and harness-bucket wording changes.
Test coverage expert	0	0	1	Regression coverage now covers the stdio fix and the non-stdio else branch.

B = highest-signal correctness/security/architecture findings, R = recommended, N = nits. Counts are advisory; the maintainer ships.

Recommendation

All in-scope findings have been folded, tests cover both branches, and mutation-break gates passed. No follow-ups remain. Merge at will after maintainer review.

Folded in this run

• (panel) Clarified changelog wording so Codex remote-header behavior is not conflated with self-defined stdio env resolution -- resolved in 9ca597c57.
• (panel) Named example harness buckets in the MCP authoring guide so users can tell which harnesses preserve placeholders and which require literal env values -- resolved in 9ca597c57.
• (panel) Clarified the stdio env code comment to point at _build_self_defined_info and the adapter pipeline -- resolved in 9ca597c57.
• (panel) Added a non-stdio self-defined env override test to lock the else branch of the new conditional -- resolved in 9ca597c57.
• (copilot) Updated the Unreleased changelog entry to reference PR fix(mcp): resolve self-defined stdio env ${VAR} placeholders at install time (closes #1963) #1966 instead of issue [BUG] Self-defined stdio MCP env ${VAR} placeholders written verbatim instead of resolved at install time (Codex/Claude) #1963 per the changelog convention -- resolved in 122d237c.

Copilot signals reviewed

• review 4600591422 -- LEGIT: stale summary noted the changelog PR-number reference issue; folded by changing (#1963) to (#1966) in 122d237c.

Regression-trap evidence (mutation-break gate)

• tests/unit/install/test_mcp_integrator_install_flow.py::TestRunMcpInstallSelfDefined::test_self_defined_stdio_env_placeholders_resolve_from_process_env -- deleted the stdio env-overrides guard; test FAILED as expected; guard restored.
• tests/unit/install/test_mcp_integrator_install_flow.py::TestRunMcpInstallSelfDefined::test_self_defined_non_stdio_env_remains_env_overrides -- collapsed the non-stdio else branch to {}; test FAILED as expected; guard restored.

Lint contract

uv run --extra dev ruff check src/ tests/ and uv run --extra dev ruff format --check src/ tests/ both passed before each push. Full local lint mirror also passed before the code/test push: pylint R0801 and scripts/lint-auth-signals.sh.

CI

gh pr checks 1966 --repo microsoft/apm --watch observed all checks green on head 122d237c3320c0ba68f2a67c454ea37bf9dac40a (CI run 28447213285; Lint, Build & Test shards, Coverage Combine, APM Self-Check, PR Binary Smoke all passed; CodeQL, Merge Gate, NOTICE Drift Check, Spec conformance, docs build, and license/cla passed or skipped where expected). CI recovery iterations: 0.

Mergeability status

Captured from gh pr view 1966 --json mergeable,mergeStateStatus,statusCheckRollup immediately after the last push of this run.

PR	head SHA	CEO stance	iters	folds	defers	Copilot rounds	CI	mergeable	mergeStateStatus	notes
#1966	122d237	ship_now	1	5	0	2	green	MERGEABLE	BLOCKED	pending required review

Convergence

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

Ready for maintainer review.

[danielmeppiel]

APM Review Panel: ship_now

Fixes silent env-placeholder shadowing for MCP stdio transports so ${VAR} resolves from process env instead of echoing itself as a literal string.

cc @danielmeppiel @sergio-sisternes-epam -- final shepherd pass is ready for review.

The final pass converged: the fix keeps stdio env values in _raw_stdio, avoids env_overrides shadowing os.environ, preserves the non-stdio branch, and keeps secrets out of logs. Follow-ups from Copilot and the panel were folded into the PR. CI is green on the latest head.

Aligned with: portability by manifest, secure by default, multi-harness support.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	0	0	Stdio env routing fix is minimal and correct; adapter pipeline separation preserved.
CLI Logging Expert	0	0	0	No CLI output or logging surface touched; no secret leak introduced.
DevX UX Expert	0	0	0	Placeholder resolution now matches user expectations and docs explain per-target behavior.
Supply Chain Security Expert	0	0	0	Existing adapter pipeline is reused; no new credential surface or secret logging.
OSS Growth Hacker	0	0	0	Fix removes a silent first-run failure for Claude Code and Codex users.
Auth Expert	0	0	0	No AuthResolver bypass or direct token env handling introduced.
Doc Writer	0	0	0	Docs are accurate, concise, and consistent after folds.
Test Coverage Expert	0	0	0	Regression trap covers Claude and Codex; non-stdio branch remains covered.

Folded in this run

• (copilot) Align Codex placeholder guide with manifest schema -- resolved in c9bf77f.
• (copilot) Document optional interactive prompting before unresolved placeholders are preserved -- resolved in 498be31.
• (copilot) Use PR number in the changelog entry -- resolved in 019dc5b.
• (panel) Clarify the source comment so future readers see why env_overrides would shadow os.environ -- resolved in c576efa.
• (panel) Neutralize the docs header comment so it does not claim every harness expands at runtime -- resolved in c576efa.
• (panel) Link the producer docs back to the canonical per-target manifest rules -- resolved in c576efa.
• (panel) Make the changelog symptom searchable and clarify TTY-only prompting wording -- resolved in 498be31.

Copilot signals reviewed

• packages/apm-guide/.apm/skills/apm-usage/dependencies.md:373 -- LEGIT: guide contradicted the manifest-schema Codex placeholder contract (resolved in c9bf77f).
• docs/src/content/docs/producer/author-primitives/mcp-as-primitive.md -- LEGIT: docs omitted the optional interactive prompt step (resolved in 498be31).
• CHANGELOG.md -- LEGIT: changelog needed the PR number rather than issue number (resolved in 019dc5b).

Regression-trap evidence (mutation-break gate)

• tests/unit/install/test_mcp_integrator_install_flow.py::TestRunMcpInstallSelfDefined::test_self_defined_stdio_env_placeholders_resolve_from_process_env -- deleted self_defined_env = {} if transport_label == "stdio" else dep.env or {}; test FAILED as expected for Claude and Codex; guard restored.

Lint contract

uv run --extra dev ruff check src/ tests/ and
uv run --extra dev ruff format --check src/ tests/ both silent. Full local lint mirror also passed with YAML I/O guard, file-length guard, raw relative_to guard, pylint R0801, and scripts/lint-auth-signals.sh.

CI

All GitHub checks passed on c9bf77f: Lint, Build & Test Shard 1/2, PR Binary Smoke, APM Self-Check, Coverage Combine, CodeQL, NOTICE Drift Check, Spec conformance, Merge Gate, Deploy Docs build, and license/cla (deploy skipped as expected) after 0 CI fix iteration(s).

Mergeability status

Captured from gh pr view 1966 --json mergeable,mergeStateStatus,statusCheckRollup immediately after the last push of this run.

PR	head SHA	CEO stance	iters	folds	defers	Copilot rounds	CI	mergeable	mergeStateStatus	notes
#1966	c9bf77f	ship_now	1	7	0	2	green	MERGEABLE	BLOCKED	pending required review

Convergence

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

Ready for maintainer review.

[danielmeppiel]

ship_now

• Added hermetic e2e integration coverage: tests/integration/test_mcp_stdio_env_resolution_e2e.py proves self-defined MCP stdio env ${VAR} placeholders resolve from the process environment.
• Mutation-break: changing stdio self_defined_env back to dep.env or {} makes the test fail with the literal ${APM_STDIO_E2E_TOKEN}; restoring the fix makes it pass.
• Secret value is asserted absent from install stdout/stderr, including verbose output.
• Full integration suite is green: 10098 passed, 226 skipped, 16 deselected, 2 xfailed.