From c24411c766b4cc8d52d57980e23435261469463d Mon Sep 17 00:00:00 2001 From: xuyuanhao Date: Sun, 26 Apr 2026 14:43:01 +0800 Subject: [PATCH 1/7] fix(mcp): translate bare ${VAR} env-var refs in self-defined server headers Self-defined MCP servers using bare ${VARNAME} or ${env:VARNAME} in headers had those references written verbatim into .vscode/mcp.json and ~/.copilot/mcp-config.json, so the literal placeholder string was sent as the header value at runtime. VS Code mcp.json natively resolves ${env:VAR} and ${input:VAR} but not bare ${VAR}, so the VSCode adapter now translates ${VAR} -> ${env:VAR} before writing. Copilot mcp-config.json has no native interpolation, so its existing _resolve_env_variable method (previously matching only legacy syntax) now also recognizes ${VAR} and ${env:VAR}, with single-pass substitution preserving the original "resolve exactly once" semantics. Gemini inherits the same fix via CopilotClientAdapter. Fixes #944 --- src/apm_cli/adapters/client/base.py | 7 ++ src/apm_cli/adapters/client/copilot.py | 57 ++++++----- src/apm_cli/adapters/client/vscode.py | 39 ++++++- tests/unit/test_copilot_adapter.py | 124 ++++++++++++++++++++++ tests/unit/test_vscode_adapter.py | 136 +++++++++++++++++++++++++ 5 files changed, 336 insertions(+), 27 deletions(-) diff --git a/src/apm_cli/adapters/client/base.py b/src/apm_cli/adapters/client/base.py index 62864af0a..b0698f599 100644 --- a/src/apm_cli/adapters/client/base.py +++ b/src/apm_cli/adapters/client/base.py @@ -7,6 +7,13 @@ _INPUT_VAR_RE = re.compile(r"\$\{input:([^}]+)\}") +# Matches ${VAR} and ${env:VAR}, capturing VAR. Intentionally does NOT match +# ${input:VAR} (the optional ``env:`` group cannot also satisfy ``input:``), +# nor GitHub Actions ``${{ ... }}`` templates (the second ``{`` fails the +# identifier class). This keeps env-var handling fully disjoint from input +# variable handling, so existing _INPUT_VAR_RE call sites are unaffected. +_ENV_VAR_RE = re.compile(r"\$\{(?:env:)?([A-Za-z_][A-Za-z0-9_]*)\}") + class MCPClientAdapter(ABC): """Base adapter for MCP clients.""" diff --git a/src/apm_cli/adapters/client/copilot.py b/src/apm_cli/adapters/client/copilot.py index f0db8d084..af046f84f 100644 --- a/src/apm_cli/adapters/client/copilot.py +++ b/src/apm_cli/adapters/client/copilot.py @@ -7,6 +7,7 @@ import json import os +import re from pathlib import Path from ...core.docker_args import DockerArgsProcessor @@ -14,7 +15,19 @@ from ...registry.client import SimpleRegistryClient from ...registry.integration import RegistryIntegration from ...utils.github_host import is_github_hostname -from .base import MCPClientAdapter +from .base import MCPClientAdapter, _ENV_VAR_RE + +# Combined env-var placeholder regex covering all three syntaxes Copilot accepts: +# legacy APM (group 1, uppercase only) +# ${VARNAME} POSIX shell (group 2) +# ${env:VARNAME} VS Code-flavored (group 2) +# A single-pass substitution preserves the original ```` semantics: +# resolved values are NOT re-scanned, so a token whose literal text contains +# ``${...}`` does not get recursively expanded. Module-level compile avoids +# per-call cost. ``${input:...}`` is intentionally not matched here. +_COPILOT_ENV_RE = re.compile( + r'<([A-Z_][A-Z0-9_]*)>|' + _ENV_VAR_RE.pattern +) class CopilotClientAdapter(MCPClientAdapter): @@ -461,8 +474,6 @@ def _resolve_env_variable(self, name, value, env_overrides=None): Returns: str: Resolved environment variable value. """ - import os - import re import sys from rich.prompt import Prompt @@ -480,28 +491,26 @@ def _resolve_env_variable(self, name, value, env_overrides=None): if not is_interactive: skip_prompting = True - # Check if value contains environment variable reference - env_pattern = r"<([A-Z_][A-Z0-9_]*)>" - matches = re.findall(env_pattern, value) - - if matches: - for env_name in matches: - # First check overrides, then environment - env_value = env_overrides.get(env_name) or os.getenv(env_name) - if not env_value and not skip_prompting: - # Only prompt if not in managed mode - prompt_text = f"Enter value for {env_name}" - env_value = Prompt.ask( - prompt_text, - password=True # noqa: SIM210 - if "token" in env_name.lower() or "key" in env_name.lower() - else False, - ) - - if env_value: - value = value.replace(f"<{env_name}>", env_value) + # Three accepted placeholder syntaxes (see _COPILOT_ENV_RE at module + # top), all resolved against env_overrides -> os.environ -> optional + # interactive prompt. Single-pass substitution preserves the legacy + # ```` semantics: resolved values are not re-scanned for further + # placeholder expansion. + def _replace(match): + # Group 1 = legacy ; group 2 = ${VAR} / ${env:VAR}. + env_name = match.group(1) or match.group(2) + env_value = env_overrides.get(env_name) or os.getenv(env_name) + if not env_value and not skip_prompting: + prompt_text = f"Enter value for {env_name}" + env_value = Prompt.ask( + prompt_text, + password=True # noqa: SIM210 + if "token" in env_name.lower() or "key" in env_name.lower() + else False, + ) + return env_value if env_value else match.group(0) - return value + return _COPILOT_ENV_RE.sub(_replace, value) def _inject_env_vars_into_docker_args(self, docker_args, env_vars): """Inject environment variables into Docker arguments following registry template. diff --git a/src/apm_cli/adapters/client/vscode.py b/src/apm_cli/adapters/client/vscode.py index cff27a6fc..2e138e3f9 100644 --- a/src/apm_cli/adapters/client/vscode.py +++ b/src/apm_cli/adapters/client/vscode.py @@ -11,7 +11,7 @@ from ...registry.client import SimpleRegistryClient from ...registry.integration import RegistryIntegration -from .base import _INPUT_VAR_RE, MCPClientAdapter +from .base import _ENV_VAR_RE, _INPUT_VAR_RE, MCPClientAdapter class VSCodeClientAdapter(MCPClientAdapter): @@ -242,9 +242,13 @@ def _format_server_config(self, server_info): "args": raw["args"], } if raw.get("env"): - server_config["env"] = raw["env"] + # Translate bare ${VAR} -> ${env:VAR} so VS Code's runtime env + # interpolation resolves them at server-start. ${input:...} + # references are preserved for input-variable extraction below. + env_translated = self._translate_env_vars_for_vscode(raw["env"]) + server_config["env"] = env_translated input_vars.extend( - self._extract_input_variables(raw["env"], server_info.get("name", "")) + self._extract_input_variables(env_translated, server_info.get("name", "")) ) return server_config, input_vars @@ -361,6 +365,10 @@ def _format_server_config(self, server_info): headers = { h["name"]: h["value"] for h in headers if "name" in h and "value" in h } + # Translate bare ${VAR} -> ${env:VAR} so VS Code resolves + # them from the host environment at runtime, instead of + # sending the literal placeholder as the header value. + headers = self._translate_env_vars_for_vscode(headers) server_config = { "type": transport, "url": remote["url"].strip(), @@ -389,6 +397,31 @@ def _format_server_config(self, server_info): return server_config, input_vars + @staticmethod + def _translate_env_vars_for_vscode(mapping): + """Normalize ``${VAR}`` and ``${env:VAR}`` references to ``${env:VAR}``. + + VS Code's mcp.json natively resolves ``${env:VAR}`` from the host + environment at server-start time. Bare ``${VAR}`` is *not* part of the + mcp.json grammar, so VS Code would otherwise pass the literal text + through (silently breaking auth headers, env vars, etc.). + + This translation is purely textual and idempotent: + - ``${VAR}`` -> ``${env:VAR}`` + - ``${env:VAR}`` -> ``${env:VAR}`` (no change) + - ``${input:X}`` -> ``${input:X}`` (no change; handled separately) + - non-string values pass through + + A new dict is returned so callers may continue to use the original + for input-variable extraction without ordering concerns. + """ + if not mapping: + return mapping + return { + k: (_ENV_VAR_RE.sub(r"${env:\1}", v) if isinstance(v, str) else v) + for k, v in mapping.items() + } + def _extract_input_variables(self, mapping, server_name): """Scan dict values for ${input:...} references and return input variable definitions. diff --git a/tests/unit/test_copilot_adapter.py b/tests/unit/test_copilot_adapter.py index 75bb816e2..979ef5aaf 100644 --- a/tests/unit/test_copilot_adapter.py +++ b/tests/unit/test_copilot_adapter.py @@ -149,6 +149,130 @@ def test_remote_skips_entries_without_url(self): self.assertEqual(config["url"], "https://good.example.com/sse") +class TestCopilotEnvVarResolutionInHeaders(unittest.TestCase): + """Issue #944: ``${VAR}`` and ``${env:VAR}`` in headers are install-time resolved. + + Copilot CLI's mcp-config.json has no runtime env interpolation, so APM bakes + the actual value in. The legacy ```` syntax already worked; these tests + cover the new ``${VAR}`` and ``${env:VAR}`` syntaxes added for #944. Together + with the existing ```` path, the three syntaxes share the same + env_overrides -> os.environ -> prompt resolution flow. + """ + + def _adapter(self): + with patch("apm_cli.adapters.client.copilot.SimpleRegistryClient"), patch( + "apm_cli.adapters.client.copilot.RegistryIntegration" + ): + return CopilotClientAdapter() + + def test_resolves_bare_dollar_brace_var(self): + adapter = self._adapter() + with patch.dict(os.environ, {"MY_TOKEN": "secret-xyz"}, clear=False): + result = adapter._resolve_env_variable( + "Authorization", "Bearer ${MY_TOKEN}", env_overrides=None + ) + self.assertEqual(result, "Bearer secret-xyz") + + def test_resolves_env_prefixed_var(self): + """``${env:VAR}`` (VS Code-flavored) also resolves to the host env value.""" + adapter = self._adapter() + with patch.dict(os.environ, {"MY_TOKEN": "secret-xyz"}, clear=False): + result = adapter._resolve_env_variable( + "Authorization", "Bearer ${env:MY_TOKEN}", env_overrides=None + ) + self.assertEqual(result, "Bearer secret-xyz") + + def test_legacy_angle_bracket_still_works(self): + """Regression: ```` legacy syntax must keep functioning.""" + adapter = self._adapter() + with patch.dict(os.environ, {"MY_TOKEN": "secret-xyz"}, clear=False): + result = adapter._resolve_env_variable( + "Authorization", "Bearer ", env_overrides=None + ) + self.assertEqual(result, "Bearer secret-xyz") + + def test_env_overrides_take_precedence(self): + """``env_overrides`` wins over ``os.environ``, identical to legacy behavior.""" + adapter = self._adapter() + with patch.dict(os.environ, {"MY_TOKEN": "from-env"}, clear=False): + result = adapter._resolve_env_variable( + "Authorization", + "Bearer ${MY_TOKEN}", + env_overrides={"MY_TOKEN": "from-overrides"}, + ) + self.assertEqual(result, "Bearer from-overrides") + + def test_unresolvable_passes_through(self): + """Unset vars survive verbatim in non-interactive (env_overrides supplied) mode.""" + adapter = self._adapter() + # Make sure target var is not in env + with patch.dict(os.environ, {}, clear=True): + result = adapter._resolve_env_variable( + "Authorization", + "Bearer ${MISSING_VAR}", + env_overrides={"OTHER": "x"}, # presence forces non-interactive path + ) + self.assertEqual(result, "Bearer ${MISSING_VAR}") + + def test_input_syntax_is_not_resolved(self): + """``${input:...}`` must NOT be resolved here -- it's runtime-prompted by VS Code.""" + adapter = self._adapter() + with patch.dict(os.environ, {"input": "should-not-match"}, clear=False): + result = adapter._resolve_env_variable( + "Authorization", + "Bearer ${input:my-token}", + env_overrides={"OTHER": "x"}, + ) + self.assertEqual(result, "Bearer ${input:my-token}") + + def test_github_actions_template_is_not_touched(self): + """``${{ secrets.X }}`` (GHA template) must pass through unchanged.""" + adapter = self._adapter() + result = adapter._resolve_env_variable( + "Authorization", + "Bearer ${{ secrets.GITHUB_TOKEN }}", + env_overrides={"OTHER": "x"}, + ) + self.assertEqual(result, "Bearer ${{ secrets.GITHUB_TOKEN }}") + + def test_resolved_value_is_not_recursively_expanded(self): + """Regression guard: a resolved value containing placeholder-like text + must NOT be re-scanned for further substitution. + + Mirrors the original ````-only semantics where each placeholder is + resolved exactly once. Important for tokens/values that legitimately + contain ``${...}`` literal text (e.g. regex patterns, templated strings). + """ + adapter = self._adapter() + with patch.dict( + os.environ, + {"OUTER": "literal-${INNER}", "INNER": "should-not-appear"}, + clear=False, + ): + # Test all three placeholder syntaxes for symmetry. + for syntax in ("", "${OUTER}", "${env:OUTER}"): + with self.subTest(syntax=syntax): + result = adapter._resolve_env_variable( + "Authorization", syntax, env_overrides={"OTHER": "x"} + ) + self.assertEqual(result, "literal-${INNER}") + + def test_mixed_syntaxes_in_one_value(self): + """A header may mix legacy and new placeholders; all should resolve.""" + adapter = self._adapter() + with patch.dict( + os.environ, + {"OLD": "old-val", "NEW": "new-val", "ENV_PREFIXED": "env-val"}, + clear=False, + ): + result = adapter._resolve_env_variable( + "X-Mixed", + "old= new=${NEW} env=${env:ENV_PREFIXED}", + env_overrides=None, + ) + self.assertEqual(result, "old=old-val new=new-val env=env-val") + + class TestCopilotSelectRemoteWithUrl(unittest.TestCase): """Direct unit tests for the ``_select_remote_with_url`` helper.""" diff --git a/tests/unit/test_vscode_adapter.py b/tests/unit/test_vscode_adapter.py index c7e494868..9b719fd26 100644 --- a/tests/unit/test_vscode_adapter.py +++ b/tests/unit/test_vscode_adapter.py @@ -436,6 +436,142 @@ def test_format_server_config_remote_default_http_preserves_headers(self, mock_g self.assertTrue(len(inputs) > 0) self.assertEqual(inputs[0]["id"], "auth-token") + @patch("apm_cli.adapters.client.vscode.VSCodeClientAdapter.get_config_path") + def test_format_server_config_translates_bare_env_var_in_headers(self, mock_get_path): + """Bare ${VAR} in remote headers must be translated to ${env:VAR}. + + Issue #944: VS Code's mcp.json grammar only resolves ``${env:VAR}`` and + ``${input:VAR}``. Without translation a bare ``${MY_TOKEN}`` is sent as + the literal string ``Bearer ${MY_TOKEN}`` to the MCP server, silently + breaking auth. + """ + mock_get_path.return_value = self.temp_path + adapter = VSCodeClientAdapter() + + server_info = { + "name": "bare-env-srv", + "remotes": [ + { + "transport_type": "http", + "url": "https://example.com/mcp", + "headers": [ + {"name": "Authorization", "value": "Bearer ${MY_SECRET_TOKEN}"}, + ], + } + ], + } + config, inputs = adapter._format_server_config(server_info) + + self.assertEqual( + config["headers"]["Authorization"], + "Bearer ${env:MY_SECRET_TOKEN}", + ) + # Translation must not fabricate input variables + self.assertEqual(inputs, []) + + @patch("apm_cli.adapters.client.vscode.VSCodeClientAdapter.get_config_path") + def test_format_server_config_preserves_env_and_input_syntax(self, mock_get_path): + """Existing ``${env:...}`` and ``${input:...}`` references must round-trip.""" + mock_get_path.return_value = self.temp_path + adapter = VSCodeClientAdapter() + + server_info = { + "name": "mixed-srv", + "remotes": [ + { + "transport_type": "http", + "url": "https://example.com/mcp", + "headers": [ + {"name": "X-Mixed", "value": "raw=${RAW} env=${env:E} input=${input:i}"}, + ], + } + ], + } + config, inputs = adapter._format_server_config(server_info) + + # Only the bare ${RAW} should change; ${env:E} and ${input:i} pass through. + self.assertEqual( + config["headers"]["X-Mixed"], + "raw=${env:RAW} env=${env:E} input=${input:i}", + ) + # ${input:i} is still extracted as an input variable. + ids = [v["id"] for v in inputs] + self.assertIn("i", ids) + + @patch("apm_cli.adapters.client.vscode.VSCodeClientAdapter.get_config_path") + def test_format_server_config_translates_bare_env_var_in_stdio_env(self, mock_get_path): + """Self-defined stdio env values get the same ${VAR} -> ${env:VAR} fix.""" + mock_get_path.return_value = self.temp_path + adapter = VSCodeClientAdapter() + + server_info = { + "name": "stdio-env-srv", + "_raw_stdio": { + "command": "python", + "args": ["-m", "my_server"], + "env": {"API_KEY": "${MY_KEY}"}, + }, + } + config, inputs = adapter._format_server_config(server_info) + + self.assertEqual(config["env"]["API_KEY"], "${env:MY_KEY}") + self.assertEqual(inputs, []) + + +class TestTranslateEnvVarsForVscode(unittest.TestCase): + """Direct unit tests for the ``_translate_env_vars_for_vscode`` helper. + + Mirrors the dedicated-class style of ``TestExtractInputVariables`` and + ``TestWarnInputVariables``, isolating helper behavior from full-adapter + integration tests above. + """ + + def test_translates_bare_dollar_brace(self): + out = VSCodeClientAdapter._translate_env_vars_for_vscode( + {"H": "Bearer ${MY_TOKEN}"} + ) + self.assertEqual(out["H"], "Bearer ${env:MY_TOKEN}") + + def test_preserves_existing_env_prefix(self): + out = VSCodeClientAdapter._translate_env_vars_for_vscode( + {"H": "Bearer ${env:MY_TOKEN}"} + ) + self.assertEqual(out["H"], "Bearer ${env:MY_TOKEN}") + + def test_preserves_input_variables(self): + out = VSCodeClientAdapter._translate_env_vars_for_vscode( + {"H": "Bearer ${input:my-token}"} + ) + self.assertEqual(out["H"], "Bearer ${input:my-token}") + + def test_idempotent(self): + """Re-running translation on already-translated values is a no-op.""" + once = VSCodeClientAdapter._translate_env_vars_for_vscode( + {"H": "raw=${RAW} env=${env:E} input=${input:i}"} + ) + twice = VSCodeClientAdapter._translate_env_vars_for_vscode(once) + self.assertEqual(once, twice) + + def test_does_not_match_github_actions_template(self): + """``${{ secrets.X }}`` (GHA template) must not be touched.""" + out = VSCodeClientAdapter._translate_env_vars_for_vscode( + {"X": "value=${{ secrets.GITHUB_TOKEN }}"} + ) + self.assertEqual(out["X"], "value=${{ secrets.GITHUB_TOKEN }}") + + def test_empty_mapping(self): + self.assertEqual(VSCodeClientAdapter._translate_env_vars_for_vscode({}), {}) + + def test_none_mapping(self): + self.assertIsNone(VSCodeClientAdapter._translate_env_vars_for_vscode(None)) + + def test_non_string_values_pass_through(self): + """Non-string values (int, bool, None) must not raise.""" + out = VSCodeClientAdapter._translate_env_vars_for_vscode( + {"n": 42, "b": True, "x": None} + ) + self.assertEqual(out, {"n": 42, "b": True, "x": None}) + class TestVSCodeSelectBestPackage(unittest.TestCase): """Test cases for _select_best_package logic.""" From c6b27a1436214947622e1effe6c713c6149910d3 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Thu, 30 Apr 2026 11:35:02 +0200 Subject: [PATCH 2/7] panel: doc-writer adds env-var placeholder docs for / Documents the new bare ${VAR} and ${env:VAR} placeholder syntaxes that this PR enables for self-defined MCP server headers and env values. Adds: - docs/src/content/docs/reference/manifest-schema.md: section 4.2.4 rewritten to cover all three placeholder syntaxes (${VAR}, ${env:VAR}, ${input:}) plus the legacy form, with a per-target resolution matrix (VS Code vs Copilot CLI vs Codex). Field rows for `env` and `headers` updated to reference the new syntaxes. - packages/apm-guide/.apm/skills/apm-usage/dependencies.md: example block annotated with the three placeholder syntaxes per the doc-sync rule for primitive formats. No source changes. Unit suite (6790 passed) and MCP integration tests remain green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../content/docs/reference/manifest-schema.md | 32 +++++++++++-------- .../.apm/skills/apm-usage/dependencies.md | 6 ++++ 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/docs/src/content/docs/reference/manifest-schema.md b/docs/src/content/docs/reference/manifest-schema.md index d43f3914b..a6c75d221 100644 --- a/docs/src/content/docs/reference/manifest-schema.md +++ b/docs/src/content/docs/reference/manifest-schema.md @@ -359,12 +359,12 @@ A plain registry reference: `io.github.github/github-mcp-server` |---|---|---|---|---| | `name` | `string` | REQUIRED | Non-empty | Server identifier (registry name or custom name). | | `transport` | `enum` | Conditional | `stdio` · `sse` · `http` · `streamable-http` | Transport protocol. REQUIRED when `registry: false`. Values are MCP transport names, not URL schemes: remote variants connect over HTTPS. | -| `env` | `map` | OPTIONAL | | Environment variable overrides. Values may contain `${input:}` references (VS Code only — see §4.2.4). | +| `env` | `map` | OPTIONAL | | Environment variable overrides. Values may contain `${VAR}`, `${env:VAR}`, or `${input:}` references — see §4.2.4. | | `args` | `dict` or `list` | OPTIONAL | | Dict for overlay variable overrides (registry), list for positional args (self-defined). | | `version` | `string` | OPTIONAL | | Pin to a specific server version. | | `registry` | `bool` or `string` | OPTIONAL | Default: `true` (public registry) | `false` = self-defined (private) server. String = custom registry URL. | | `package` | `enum` | OPTIONAL | `npm` · `pypi` · `oci` | Package manager type hint. | -| `headers` | `map` | OPTIONAL | | Custom HTTP headers for remote endpoints. Values may contain `${input:}` references (VS Code only — see §4.2.4). | +| `headers` | `map` | OPTIONAL | | Custom HTTP headers for remote endpoints. Values may contain `${VAR}`, `${env:VAR}`, or `${input:}` references — see §4.2.4. | | `tools` | `list` | OPTIONAL | Default: `["*"]` | Restrict which tools are exposed. | | `url` | `string` | Conditional | | Endpoint URL. REQUIRED when `registry: false` and `transport` is `http`, `sse`, or `streamable-http`. | | `command` | `string` | Conditional | Single binary path; no embedded whitespace unless `args` is also present | Binary path. REQUIRED when `registry: false` and `transport` is `stdio`. | @@ -400,13 +400,24 @@ dependencies: API_KEY: ${{ secrets.KEY }} ``` -#### 4.2.4. `${input:...}` Variables +#### 4.2.4. Variable References in `headers` and `env` -Values in `headers` and `env` may contain VS Code input variable references using the syntax `${input:}`. At runtime, VS Code prompts the user for each referenced input before starting the server. +Values in `headers` and `env` may contain three placeholder syntaxes. APM resolves them per-target so secrets stay out of generated config files where possible. -- **Registry-backed servers** — APM auto-generates input prompts from registry metadata. +| Syntax | Source | VS Code | Copilot CLI / Codex | +|---|---|---|---| +| `${VAR}` | host environment | Translated to `${env:VAR}` (resolved at server-start by VS Code) | Resolved at install time from env (or interactive prompt) | +| `${env:VAR}` | host environment | Native — passed through verbatim | Resolved at install time from env (or interactive prompt) | +| `${input:}` | user prompt | Native — VS Code prompts at runtime | Not supported — use `${VAR}` or `${env:VAR}` instead | +| `` (legacy) | host environment | Not recognized | Resolved at install time (kept for back-compat) | + +- **VS Code** has native `${env:VAR}` and `${input:VAR}` interpolation, so APM emits placeholders rather than baking secrets into `mcp.json`. Bare `${VAR}` is normalized to `${env:VAR}` for you. +- **Copilot CLI** and **Codex** have no runtime interpolation, so APM resolves `${VAR}`, `${env:VAR}`, and the legacy `` at install time using `os.environ` (or an interactive prompt when missing). Resolved values are not re-scanned, so a value containing literal `${...}` text is preserved. +- **Registry-backed servers** — APM auto-generates input prompts from registry metadata for `${input:...}`. - **Self-defined servers** — APM detects `${input:...}` patterns in `apm.yml` and generates matching input definitions automatically. +GitHub Actions templates (`${{ ... }}`) are intentionally left untouched. + ```yaml dependencies: mcp: @@ -415,16 +426,11 @@ dependencies: transport: http url: https://my-server.example.com/mcp/ headers: - Authorization: "Bearer ${input:my-server-token}" - X-Project: "${input:my-server-project}" + Authorization: "Bearer ${MY_SECRET_TOKEN}" # bare env-var + X-Tenant: "${env:TENANT_ID}" # env-prefixed + X-Project: "${input:my-server-project}" # VS Code input prompt ``` -| Runtime | `${input:...}` support | -|---------|----------------------| -| VS Code | Yes — prompts user at runtime | -| Copilot CLI | No — use environment variables | -| Codex | No — use environment variables | - --- ## 5. devDependencies diff --git a/packages/apm-guide/.apm/skills/apm-usage/dependencies.md b/packages/apm-guide/.apm/skills/apm-usage/dependencies.md index 503297591..5e4e6b7a6 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/dependencies.md +++ b/packages/apm-guide/.apm/skills/apm-usage/dependencies.md @@ -155,6 +155,12 @@ dependencies: package: npm # npm|pypi|oci headers: X-Custom: "value" + # Env-var placeholders in headers/env values: + # ${VAR} or ${env:VAR} -> resolved from host env (Copilot/Codex bake + # in at install; VS Code resolves at runtime) + # ${input:} -> VS Code prompts user at runtime + # -> legacy Copilot syntax (still supported) + Authorization: "Bearer ${MY_TOKEN}" tools: ["repos", "issues"] # Self-defined server (not in registry) From eb06151e1ddcd0a830805972b0a04f84e3363b2e Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Thu, 30 Apr 2026 11:48:26 +0200 Subject: [PATCH 3/7] panel: add E2E integration test for env-var header translation Pins the full install pipeline boundary: apm.yml self-defined HTTP MCP server with both bare ${VAR} and explicit ${env:VAR} headers -> apm install --target vscode -> .vscode/mcp.json on disk. Asserts both syntaxes land as ${env:VAR} (the syntax VS Code resolves at server start) and that no host env values leak into the file. Complements unit coverage in tests/unit/test_vscode_adapter.py with one concrete on-disk assertion so the fix can not regress when adapter wiring changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../test_mcp_env_var_headers_e2e.py | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 tests/integration/test_mcp_env_var_headers_e2e.py diff --git a/tests/integration/test_mcp_env_var_headers_e2e.py b/tests/integration/test_mcp_env_var_headers_e2e.py new file mode 100644 index 000000000..98041d072 --- /dev/null +++ b/tests/integration/test_mcp_env_var_headers_e2e.py @@ -0,0 +1,136 @@ +"""End-to-end regression guard for #944 / PR #947: bare ${VAR} env-var +references in self-defined MCP server headers must reach VS Code's mcp.json +as the runtime-resolvable ${env:VAR} placeholder (NOT a literal ${VAR} +that VS Code would treat as opaque text). + +This exercises the full pipeline: + apm.yml -> apm install --target vscode -> .vscode/mcp.json on disk + +The unit tests in tests/unit/test_vscode_adapter.py cover all three syntaxes +in isolation; this test pins the integration boundary so the fix doesn't +regress when adapter wiring changes. +""" + +import json +import os +import shutil +import subprocess +from pathlib import Path + +import pytest +import yaml + + +@pytest.fixture +def apm_command(): + apm_on_path = shutil.which("apm") + if apm_on_path: + return apm_on_path + venv_apm = Path(__file__).parent.parent.parent / ".venv" / "bin" / "apm" + if venv_apm.exists(): + return str(venv_apm) + return "apm" + + +@pytest.fixture +def temp_project(tmp_path): + project_dir = tmp_path / "mcp-env-vars-e2e" + project_dir.mkdir() + # Mark this as a VS Code target via .vscode/ directory presence + (project_dir / ".vscode").mkdir() + return project_dir + + +def _write_apm_yml(project_dir, mcp_servers): + config = { + "name": "mcp-env-vars-e2e", + "version": "1.0.0", + "dependencies": {"apm": [], "mcp": mcp_servers}, + } + (project_dir / "apm.yml").write_text( + yaml.dump(config, default_flow_style=False), encoding="utf-8" + ) + + +class TestMcpEnvVarHeadersVSCode: + """#944 regression: VS Code mcp.json must contain ${env:VAR} placeholders + for both ${VAR} and ${env:VAR} header syntaxes from apm.yml.""" + + def test_self_defined_http_server_translates_both_env_var_syntaxes( + self, temp_project, apm_command + ): + """Both bare ${VAR} and explicit ${env:VAR} in apm.yml headers must + land in mcp.json as ${env:VAR} (the syntax VS Code resolves at + server-start time).""" + _write_apm_yml( + temp_project, + [ + { + "name": "test-http-server", + "registry": False, + "transport": "http", + "url": "https://example.com/mcp", + "headers": { + # Two syntaxes per PR #947's stated VS Code contract + "Authorization": "Bearer ${MY_BEARER_TOKEN}", + "X-Api-Key": "${env:MY_API_KEY}", + }, + } + ], + ) + + env = os.environ.copy() + # Provide values for any prompt-fallback path; install must NOT + # leak these into mcp.json (vscode emits placeholders, not values). + env["MY_BEARER_TOKEN"] = "should-not-appear-in-vscode-json" + env["MY_API_KEY"] = "should-not-appear-in-vscode-json" + env["GIT_TERMINAL_PROMPT"] = "0" + env["APM_NON_INTERACTIVE"] = "1" + + result = subprocess.run( + [apm_command, "install", "--target", "vscode"], + cwd=temp_project, + capture_output=True, + text=True, + timeout=120, + env=env, + ) + + # Surface install output if the flow fails so debugging is fast + assert result.returncode == 0, ( + f"apm install failed (rc={result.returncode}).\n" + f"STDOUT:\n{result.stdout}\nSTDERR:\n{result.stderr}" + ) + + mcp_json = temp_project / ".vscode" / "mcp.json" + assert mcp_json.exists(), ( + f"Expected .vscode/mcp.json to exist after install.\n" + f"STDOUT:\n{result.stdout}\nSTDERR:\n{result.stderr}" + ) + + config = json.loads(mcp_json.read_text(encoding="utf-8")) + servers = config.get("servers") or {} + # Server keys are normalized; pick the only entry + assert len(servers) == 1, f"Expected 1 server in mcp.json, got: {list(servers.keys())}" + server = next(iter(servers.values())) + headers = server.get("headers") or {} + + # ${VAR} syntax MUST be translated to ${env:VAR} + assert headers.get("Authorization") == "Bearer ${env:MY_BEARER_TOKEN}", ( + f"Bare ${{VAR}} syntax must be translated to ${{env:VAR}} for VS Code.\n" + f"Got: {headers!r}" + ) + # ${env:VAR} syntax MUST be preserved + assert headers.get("X-Api-Key") == "${env:MY_API_KEY}", ( + f"${{env:VAR}} syntax must be preserved verbatim.\n" + f"Got: {headers!r}" + ) + + # CRITICAL: literal env values from the host must NOT appear in mcp.json + # (vscode is supposed to resolve placeholders at server-start, not at install) + full_text = mcp_json.read_text(encoding="utf-8") + assert "should-not-appear-in-vscode-json" not in full_text, ( + "VS Code mcp.json leaked the literal env value -- placeholder " + "translation regressed.\n" + f"File contents:\n{full_text}" + ) From 67e55907a48de033e6a4a7b38fbb234cc7ee405b Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Thu, 30 Apr 2026 11:58:48 +0200 Subject: [PATCH 4/7] lint: ruff isort fix on copilot.py imports Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/adapters/client/copilot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apm_cli/adapters/client/copilot.py b/src/apm_cli/adapters/client/copilot.py index af046f84f..acecee6e6 100644 --- a/src/apm_cli/adapters/client/copilot.py +++ b/src/apm_cli/adapters/client/copilot.py @@ -15,7 +15,7 @@ from ...registry.client import SimpleRegistryClient from ...registry.integration import RegistryIntegration from ...utils.github_host import is_github_hostname -from .base import MCPClientAdapter, _ENV_VAR_RE +from .base import _ENV_VAR_RE, MCPClientAdapter # Combined env-var placeholder regex covering all three syntaxes Copilot accepts: # legacy APM (group 1, uppercase only) From b44225601dd9d8ae16de81cd14d2e1a756a30b68 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Thu, 30 Apr 2026 12:02:27 +0200 Subject: [PATCH 5/7] lint: ruff format on new integration test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/adapters/client/copilot.py | 4 +--- .../integration/test_mcp_env_var_headers_e2e.py | 3 +-- tests/unit/test_copilot_adapter.py | 5 +++-- tests/unit/test_vscode_adapter.py | 16 ++++------------ 4 files changed, 9 insertions(+), 19 deletions(-) diff --git a/src/apm_cli/adapters/client/copilot.py b/src/apm_cli/adapters/client/copilot.py index acecee6e6..8ffb04a3c 100644 --- a/src/apm_cli/adapters/client/copilot.py +++ b/src/apm_cli/adapters/client/copilot.py @@ -25,9 +25,7 @@ # resolved values are NOT re-scanned, so a token whose literal text contains # ``${...}`` does not get recursively expanded. Module-level compile avoids # per-call cost. ``${input:...}`` is intentionally not matched here. -_COPILOT_ENV_RE = re.compile( - r'<([A-Z_][A-Z0-9_]*)>|' + _ENV_VAR_RE.pattern -) +_COPILOT_ENV_RE = re.compile(r"<([A-Z_][A-Z0-9_]*)>|" + _ENV_VAR_RE.pattern) class CopilotClientAdapter(MCPClientAdapter): diff --git a/tests/integration/test_mcp_env_var_headers_e2e.py b/tests/integration/test_mcp_env_var_headers_e2e.py index 98041d072..fc4fc6daa 100644 --- a/tests/integration/test_mcp_env_var_headers_e2e.py +++ b/tests/integration/test_mcp_env_var_headers_e2e.py @@ -122,8 +122,7 @@ def test_self_defined_http_server_translates_both_env_var_syntaxes( ) # ${env:VAR} syntax MUST be preserved assert headers.get("X-Api-Key") == "${env:MY_API_KEY}", ( - f"${{env:VAR}} syntax must be preserved verbatim.\n" - f"Got: {headers!r}" + f"${{env:VAR}} syntax must be preserved verbatim.\nGot: {headers!r}" ) # CRITICAL: literal env values from the host must NOT appear in mcp.json diff --git a/tests/unit/test_copilot_adapter.py b/tests/unit/test_copilot_adapter.py index 979ef5aaf..c67e8767d 100644 --- a/tests/unit/test_copilot_adapter.py +++ b/tests/unit/test_copilot_adapter.py @@ -160,8 +160,9 @@ class TestCopilotEnvVarResolutionInHeaders(unittest.TestCase): """ def _adapter(self): - with patch("apm_cli.adapters.client.copilot.SimpleRegistryClient"), patch( - "apm_cli.adapters.client.copilot.RegistryIntegration" + with ( + patch("apm_cli.adapters.client.copilot.SimpleRegistryClient"), + patch("apm_cli.adapters.client.copilot.RegistryIntegration"), ): return CopilotClientAdapter() diff --git a/tests/unit/test_vscode_adapter.py b/tests/unit/test_vscode_adapter.py index 9b719fd26..d3e7439b6 100644 --- a/tests/unit/test_vscode_adapter.py +++ b/tests/unit/test_vscode_adapter.py @@ -527,21 +527,15 @@ class TestTranslateEnvVarsForVscode(unittest.TestCase): """ def test_translates_bare_dollar_brace(self): - out = VSCodeClientAdapter._translate_env_vars_for_vscode( - {"H": "Bearer ${MY_TOKEN}"} - ) + out = VSCodeClientAdapter._translate_env_vars_for_vscode({"H": "Bearer ${MY_TOKEN}"}) self.assertEqual(out["H"], "Bearer ${env:MY_TOKEN}") def test_preserves_existing_env_prefix(self): - out = VSCodeClientAdapter._translate_env_vars_for_vscode( - {"H": "Bearer ${env:MY_TOKEN}"} - ) + out = VSCodeClientAdapter._translate_env_vars_for_vscode({"H": "Bearer ${env:MY_TOKEN}"}) self.assertEqual(out["H"], "Bearer ${env:MY_TOKEN}") def test_preserves_input_variables(self): - out = VSCodeClientAdapter._translate_env_vars_for_vscode( - {"H": "Bearer ${input:my-token}"} - ) + out = VSCodeClientAdapter._translate_env_vars_for_vscode({"H": "Bearer ${input:my-token}"}) self.assertEqual(out["H"], "Bearer ${input:my-token}") def test_idempotent(self): @@ -567,9 +561,7 @@ def test_none_mapping(self): def test_non_string_values_pass_through(self): """Non-string values (int, bool, None) must not raise.""" - out = VSCodeClientAdapter._translate_env_vars_for_vscode( - {"n": 42, "b": True, "x": None} - ) + out = VSCodeClientAdapter._translate_env_vars_for_vscode({"n": 42, "b": True, "x": None}) self.assertEqual(out, {"n": 42, "b": True, "x": None}) From 5e687cf0664f1e6b7c69015a988c4c81414adaad Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Thu, 30 Apr 2026 12:33:42 +0200 Subject: [PATCH 6/7] address Copilot review: doc accuracy + wire integration test into CI - Clarify that Codex resolves only legacy , not ${VAR}/${env:VAR} (manifest-schema.md, dependencies.md) - Wire test_mcp_env_var_headers_e2e.py into scripts/test-integration.sh so CI exercises the regression guard Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/src/content/docs/reference/manifest-schema.md | 3 ++- .../apm-guide/.apm/skills/apm-usage/dependencies.md | 5 +++-- scripts/test-integration.sh | 11 +++++++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/docs/src/content/docs/reference/manifest-schema.md b/docs/src/content/docs/reference/manifest-schema.md index a6c75d221..e52ac6f4b 100644 --- a/docs/src/content/docs/reference/manifest-schema.md +++ b/docs/src/content/docs/reference/manifest-schema.md @@ -412,7 +412,8 @@ Values in `headers` and `env` may contain three placeholder syntaxes. APM resolv | `` (legacy) | host environment | Not recognized | Resolved at install time (kept for back-compat) | - **VS Code** has native `${env:VAR}` and `${input:VAR}` interpolation, so APM emits placeholders rather than baking secrets into `mcp.json`. Bare `${VAR}` is normalized to `${env:VAR}` for you. -- **Copilot CLI** and **Codex** have no runtime interpolation, so APM resolves `${VAR}`, `${env:VAR}`, and the legacy `` at install time using `os.environ` (or an interactive prompt when missing). Resolved values are not re-scanned, so a value containing literal `${...}` text is preserved. +- **Copilot CLI** has no runtime interpolation, so APM resolves `${VAR}`, `${env:VAR}`, and the legacy `` at install time using `os.environ` (or an interactive prompt when missing). Resolved values are not re-scanned, so a value containing literal `${...}` text is preserved. +- **Codex** currently resolves only the legacy `` placeholder at install time; `${VAR}` / `${env:VAR}` are passed through verbatim in the Codex adapter today. - **Registry-backed servers** — APM auto-generates input prompts from registry metadata for `${input:...}`. - **Self-defined servers** — APM detects `${input:...}` patterns in `apm.yml` and generates matching input definitions automatically. diff --git a/packages/apm-guide/.apm/skills/apm-usage/dependencies.md b/packages/apm-guide/.apm/skills/apm-usage/dependencies.md index 5e4e6b7a6..5a0eea5c7 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/dependencies.md +++ b/packages/apm-guide/.apm/skills/apm-usage/dependencies.md @@ -156,8 +156,9 @@ dependencies: headers: X-Custom: "value" # Env-var placeholders in headers/env values: - # ${VAR} or ${env:VAR} -> resolved from host env (Copilot/Codex bake - # in at install; VS Code resolves at runtime) + # ${VAR} or ${env:VAR} -> resolved from host env at install time + # by Copilot (VS Code resolves at runtime; + # Codex passes ${...} through unchanged) # ${input:} -> VS Code prompts user at runtime # -> legacy Copilot syntax (still supported) Authorization: "Bearer ${MY_TOKEN}" diff --git a/scripts/test-integration.sh b/scripts/test-integration.sh index 148f8bd29..dc338f395 100755 --- a/scripts/test-integration.sh +++ b/scripts/test-integration.sh @@ -366,6 +366,17 @@ run_e2e_tests() { log_error "MCP registry tests failed!" exit 1 fi + + # Run MCP env-var headers E2E tests (regression guard for ${VAR} -> ${env:VAR}) + log_info "Running MCP env-var headers E2E tests..." + echo "Command: pytest tests/integration/test_mcp_env_var_headers_e2e.py -v -s --tb=short" + + if pytest tests/integration/test_mcp_env_var_headers_e2e.py -v -s --tb=short; then + log_success "MCP env-var headers tests passed!" + else + log_error "MCP env-var headers tests failed!" + exit 1 + fi # Run APM Dependencies integration tests (NEW - Task 8A) log_info "Running APM Dependencies integration tests with real repositories..." From e97fdf5b0366284dce9cf4d105bc538d877d6b13 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Thu, 30 Apr 2026 12:53:58 +0200 Subject: [PATCH 7/7] ux: legacy warning in VS Code adapter + 'prefer ${VAR}' doc Folds the devx-ux-expert audit recommendation into PR #947 (no follow-up PR sprawl per board policy): 1. Reference doc (manifest-schema.md): explicit recommendation to use ${VAR} or ${env:VAR} in new manifests; labelled legacy (Copilot CLI / Codex only) with note that VS Code would render it as literal text. 2. VS Code adapter: emit a warning at install time when self-defined server headers or env contain legacy placeholders, naming the server, the field (headers/env), and the offending vars. Turns the prior silent failure mode into an observable one. 5 new unit tests cover the warning helper (legacy var detected, multiple unique vars deduped, modern syntax silent, empty/None mapping silent, non-string values silent). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../content/docs/reference/manifest-schema.md | 1 + src/apm_cli/adapters/client/vscode.py | 38 ++++++++++++++++ tests/unit/test_vscode_adapter.py | 45 +++++++++++++++++++ 3 files changed, 84 insertions(+) diff --git a/docs/src/content/docs/reference/manifest-schema.md b/docs/src/content/docs/reference/manifest-schema.md index e52ac6f4b..134bcade8 100644 --- a/docs/src/content/docs/reference/manifest-schema.md +++ b/docs/src/content/docs/reference/manifest-schema.md @@ -414,6 +414,7 @@ Values in `headers` and `env` may contain three placeholder syntaxes. APM resolv - **VS Code** has native `${env:VAR}` and `${input:VAR}` interpolation, so APM emits placeholders rather than baking secrets into `mcp.json`. Bare `${VAR}` is normalized to `${env:VAR}` for you. - **Copilot CLI** has no runtime interpolation, so APM resolves `${VAR}`, `${env:VAR}`, and the legacy `` at install time using `os.environ` (or an interactive prompt when missing). Resolved values are not re-scanned, so a value containing literal `${...}` text is preserved. - **Codex** currently resolves only the legacy `` placeholder at install time; `${VAR}` / `${env:VAR}` are passed through verbatim in the Codex adapter today. +- **Recommended:** Use `${VAR}` or `${env:VAR}` in all new manifests — they work on every target that supports remote MCP servers. `` is legacy and only resolved by Copilot CLI and Codex; in VS Code it would silently render as literal text in the generated config. - **Registry-backed servers** — APM auto-generates input prompts from registry metadata for `${input:...}`. - **Self-defined servers** — APM detects `${input:...}` patterns in `apm.yml` and generates matching input definitions automatically. diff --git a/src/apm_cli/adapters/client/vscode.py b/src/apm_cli/adapters/client/vscode.py index 2e138e3f9..5a6643957 100644 --- a/src/apm_cli/adapters/client/vscode.py +++ b/src/apm_cli/adapters/client/vscode.py @@ -7,12 +7,19 @@ import json import os # noqa: F401 +import re from pathlib import Path from ...registry.client import SimpleRegistryClient from ...registry.integration import RegistryIntegration +from ...utils.console import _rich_warning from .base import _ENV_VAR_RE, _INPUT_VAR_RE, MCPClientAdapter +# Legacy ```` placeholder (Copilot CLI / Codex only). VS Code does not +# resolve angle-bracket placeholders, so emitting them produces literal +# ```` text in headers / env values -- silently breaking auth at runtime. +_LEGACY_ANGLE_VAR_RE = re.compile(r"<([A-Z_][A-Z0-9_]*)>") + class VSCodeClientAdapter(MCPClientAdapter): """VSCode implementation of MCP client adapter. @@ -245,6 +252,9 @@ def _format_server_config(self, server_info): # Translate bare ${VAR} -> ${env:VAR} so VS Code's runtime env # interpolation resolves them at server-start. ${input:...} # references are preserved for input-variable extraction below. + self._warn_on_legacy_angle_vars( + raw["env"], server_info.get("name", "unknown"), "env" + ) env_translated = self._translate_env_vars_for_vscode(raw["env"]) server_config["env"] = env_translated input_vars.extend( @@ -368,6 +378,9 @@ def _format_server_config(self, server_info): # Translate bare ${VAR} -> ${env:VAR} so VS Code resolves # them from the host environment at runtime, instead of # sending the literal placeholder as the header value. + self._warn_on_legacy_angle_vars( + headers, server_info.get("name", "unknown"), "headers" + ) headers = self._translate_env_vars_for_vscode(headers) server_config = { "type": transport, @@ -422,6 +435,31 @@ def _translate_env_vars_for_vscode(mapping): for k, v in mapping.items() } + @staticmethod + def _warn_on_legacy_angle_vars(mapping, server_name, field): + """Emit a warning when legacy ```` placeholders appear in *mapping*. + + VS Code does not resolve ```` placeholders, so they would render + as literal ```` text in the generated mcp.json -- silently + breaking auth headers / env values at server-start. Surface this as + an explicit warning so authors can switch to the cross-harness + ``${VAR}`` / ``${env:VAR}`` syntax (see manifest-schema reference). + """ + if not mapping: + return + offenders = [] + for value in mapping.values(): + if isinstance(value, str): + offenders.extend(_LEGACY_ANGLE_VAR_RE.findall(value)) + if offenders: + unique = sorted(set(offenders)) + _rich_warning( + f"Server '{server_name}' {field} use legacy placeholder(s) " + f"({', '.join('<' + n + '>' for n in unique)}) which VS Code " + f"cannot resolve. Use ${{VAR}} or ${{env:VAR}} instead so the " + f"value resolves at runtime." + ) + def _extract_input_variables(self, mapping, server_name): """Scan dict values for ${input:...} references and return input variable definitions. diff --git a/tests/unit/test_vscode_adapter.py b/tests/unit/test_vscode_adapter.py index d3e7439b6..598d21609 100644 --- a/tests/unit/test_vscode_adapter.py +++ b/tests/unit/test_vscode_adapter.py @@ -1236,5 +1236,50 @@ def test_no_warning_for_empty_mapping(self): mock_print.assert_not_called() +class TestWarnOnLegacyAngleVars(unittest.TestCase): + """VS Code cannot resolve placeholders -- the warning surfaces this.""" + + def test_warning_emitted_for_legacy_var_in_headers(self): + mapping = {"Authorization": "Bearer "} + with patch("apm_cli.adapters.client.vscode._rich_warning") as mock_warn: + VSCodeClientAdapter._warn_on_legacy_angle_vars(mapping, "my-server", "headers") + mock_warn.assert_called_once() + msg = mock_warn.call_args[0][0] + assert "" in msg + assert "my-server" in msg + assert "headers" in msg + assert "${VAR}" in msg or "${env:VAR}" in msg + + def test_warning_lists_multiple_unique_vars(self): + mapping = { + "X-A": "", + "X-B": " and ", # duplicate of A should dedupe + } + with patch("apm_cli.adapters.client.vscode._rich_warning") as mock_warn: + VSCodeClientAdapter._warn_on_legacy_angle_vars(mapping, "s", "headers") + mock_warn.assert_called_once() + msg = mock_warn.call_args[0][0] + assert "" in msg and "" in msg + + def test_no_warning_for_modern_syntax(self): + for value in ("Bearer ${MY_TOKEN}", "Bearer ${env:MY_TOKEN}", "Bearer ${input:tok}"): + with patch("apm_cli.adapters.client.vscode._rich_warning") as mock_warn: + VSCodeClientAdapter._warn_on_legacy_angle_vars({"H": value}, "s", "headers") + mock_warn.assert_not_called() + + def test_no_warning_for_empty_or_none_mapping(self): + with patch("apm_cli.adapters.client.vscode._rich_warning") as mock_warn: + VSCodeClientAdapter._warn_on_legacy_angle_vars({}, "s", "headers") + VSCodeClientAdapter._warn_on_legacy_angle_vars(None, "s", "headers") + mock_warn.assert_not_called() + + def test_no_warning_for_non_string_values(self): + with patch("apm_cli.adapters.client.vscode._rich_warning") as mock_warn: + VSCodeClientAdapter._warn_on_legacy_angle_vars( + {"n": 42, "b": True, "x": None}, "s", "env" + ) + mock_warn.assert_not_called() + + if __name__ == "__main__": unittest.main()