diff --git a/CHANGELOG.md b/CHANGELOG.md index 32d7f56ed..1110f2352 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Copilot, Codex, Cursor, Claude, Windsurf, OpenCode, and Gemini adapters handle MCP v0.1 `runtimeArguments`/`packageArguments` with `variables` (no `type` key), matching the VS Code fix from #1444. (#1461, closes #1452, thanks @sergio-sisternes-epam) + ## [0.14.2] - 2026-05-22 ### Added diff --git a/src/apm_cli/adapters/client/_mcp_runtime_args.py b/src/apm_cli/adapters/client/_mcp_runtime_args.py new file mode 100644 index 000000000..b80efb800 --- /dev/null +++ b/src/apm_cli/adapters/client/_mcp_runtime_args.py @@ -0,0 +1,61 @@ +"""Shared helper for MCP v0.1 runtimeArguments.variables handling. + +Extracted to avoid R0801 duplicate-code violations across adapter modules +that each implement a ``_process_arguments`` method but share identical v0.1 +``value_hint`` processing logic (copilot and codex). + +Usage in an adapter's ``_process_arguments`` loop:: + + from ._mcp_runtime_args import process_v01_value_hint_arg + + elif not arg_type and "value_hint" in arg: + value = process_v01_value_hint_arg(arg, runtime_vars) + if value: + processed.append(self._resolve_variable_placeholders( + value, resolved_env, runtime_vars + )) +""" + +from __future__ import annotations + + +def process_v01_value_hint_arg(arg: dict, runtime_vars: dict | None) -> str | None: + """Process a single v0.1-format runtimeArguments entry. + + A v0.1 entry has a ``value_hint`` key but no ``type`` key. An optional + ``variables`` dict maps placeholder names to their metadata. + + The ``is_required`` field on the *arg itself* controls whether the entry + participates in argument building. Entries with ``is_required: False`` + are optional hints that must be skipped -- VS Code's extractor only + includes entries with ``is_required: True``, so including them would + produce extra, unintended CLI args. When ``is_required`` is absent the + entry is treated as required (default True). + + Args: + arg: The argument dict to process. Must contain ``value_hint``. + runtime_vars: Resolved APM template variables (may be ``None`` or + empty). + + Returns: + The processed value string after ``{var_name}`` placeholder + substitution, or ``None`` if the entry should be skipped (optional + hint or empty value). + """ + # Skip optional legacy hints that are not required. + if not arg.get("is_required", True): + return None + + value: str = arg.get("value_hint", "") + if not value: + return None + + if "variables" in arg: + for var_name in arg["variables"]: + if runtime_vars and var_name in runtime_vars: + replacement = runtime_vars[var_name] + else: + replacement = f"${{{var_name}}}" + value = value.replace(f"{{{var_name}}}", replacement) + + return value or None diff --git a/src/apm_cli/adapters/client/codex.py b/src/apm_cli/adapters/client/codex.py index dc3f5293b..cabe500be 100644 --- a/src/apm_cli/adapters/client/codex.py +++ b/src/apm_cli/adapters/client/codex.py @@ -10,6 +10,7 @@ from ...registry.client import SimpleRegistryClient from ...registry.integration import RegistryIntegration from ...utils.console import _rich_success, _rich_warning +from ._mcp_runtime_args import process_v01_value_hint_arg from .base import MCPClientAdapter _log = logging.getLogger(__name__) @@ -422,6 +423,15 @@ def _process_arguments( # pylint: disable=duplicate-code # structural similari str(additional_value), resolved_env, runtime_vars ) processed.append(processed_value) + elif not arg_type and "value_hint" in arg: + # v0.1 registry format: shared helper handles is_required + # guard and {var_name} placeholder substitution. + value = process_v01_value_hint_arg(arg, runtime_vars) + if value: + processed_value = self._resolve_variable_placeholders( + value, resolved_env, runtime_vars + ) + processed.append(processed_value) elif isinstance(arg, str): # Already a string, use as-is but resolve variable placeholders processed_value = self._resolve_variable_placeholders( diff --git a/src/apm_cli/adapters/client/copilot.py b/src/apm_cli/adapters/client/copilot.py index 42a3ccb73..9d683a0c4 100644 --- a/src/apm_cli/adapters/client/copilot.py +++ b/src/apm_cli/adapters/client/copilot.py @@ -18,6 +18,7 @@ from ...registry.integration import RegistryIntegration from ...utils.console import _rich_warning from ...utils.github_host import is_github_hostname +from ._mcp_runtime_args import process_v01_value_hint_arg from .base import ( _ENV_PLACEHOLDER_RE, _ENV_VAR_RE, @@ -982,6 +983,15 @@ def _process_arguments(self, arguments, resolved_env=None, runtime_vars=None): str(value), resolved_env, runtime_vars ) processed.append(processed_value) + elif not arg_type and "value_hint" in arg: + # v0.1 registry format: shared helper handles is_required + # guard and {var_name} placeholder substitution. + value = process_v01_value_hint_arg(arg, runtime_vars) + if value: + processed_value = self._resolve_variable_placeholders( + value, resolved_env, runtime_vars + ) + processed.append(processed_value) elif isinstance(arg, str): # Already a string, use as-is but resolve variable placeholders processed_value = self._resolve_variable_placeholders( diff --git a/tests/unit/adapters/test_v01_variables_process_arguments.py b/tests/unit/adapters/test_v01_variables_process_arguments.py new file mode 100644 index 000000000..19daea189 --- /dev/null +++ b/tests/unit/adapters/test_v01_variables_process_arguments.py @@ -0,0 +1,239 @@ +"""Regression tests for MCP v0.1 runtimeArguments.variables handling in non-vscode adapters. + +Issue #1452: copilot and codex adapters' _process_arguments do not handle +the v0.1 format where a ``variables`` dict is a sibling of ``value_hint`` +(no ``type`` key). This causes Docker mount args with {workspaceFolder} +placeholders to be silently dropped. + +gemini, cursor, and claude inherit from CopilotClientAdapter, so fixing +copilot fixes all three. +""" + +from __future__ import annotations + +import unittest +from pathlib import Path +from unittest.mock import patch + +from apm_cli.adapters.client.codex import CodexClientAdapter +from apm_cli.adapters.client.copilot import CopilotClientAdapter + +# --------------------------------------------------------------------------- +# Shared v0.1 Docker fixture (same shape as real registry data) +# --------------------------------------------------------------------------- + +V01_DOCKER_RUNTIME_ARGS = [ + {"value_hint": "run"}, + {"value_hint": "-i"}, + {"value_hint": "--rm"}, + {"value_hint": "-v"}, + { + "value_hint": "{workspaceFolder}:/workspace", + "variables": { + "workspaceFolder": { + "description": "Workspace folder path", + "is_required": True, + } + }, + }, + {"value_hint": "-w"}, + {"value_hint": "/workspace"}, + {"value_hint": "ghcr.io/example/playwright-mcp:1.2.3"}, +] + + +# --------------------------------------------------------------------------- +# Adapter factories +# --------------------------------------------------------------------------- + + +def _make_copilot(**kwargs) -> CopilotClientAdapter: + with ( + patch("apm_cli.adapters.client.copilot.SimpleRegistryClient"), + patch("apm_cli.adapters.client.copilot.RegistryIntegration"), + ): + return CopilotClientAdapter(**kwargs) + + +def _make_codex(tmp_path: Path | None = None) -> CodexClientAdapter: + with ( + patch("apm_cli.adapters.client.codex.SimpleRegistryClient"), + patch("apm_cli.adapters.client.codex.RegistryIntegration"), + ): + return CodexClientAdapter(project_root=tmp_path) + + +# --------------------------------------------------------------------------- +# Copilot adapter +# --------------------------------------------------------------------------- + + +class TestCopilotProcessArgumentsV01Variables(unittest.TestCase): + """_process_arguments must handle v0.1 value_hint + variables args.""" + + def _adapter(self) -> CopilotClientAdapter: + return _make_copilot() + + def test_v01_plain_value_hint_args_extracted(self): + """Plain value_hint args (no variables, no type) are extracted.""" + adapter = self._adapter() + result = adapter._process_arguments( + [{"value_hint": "run"}, {"value_hint": "--rm"}], + resolved_env={}, + runtime_vars={}, + ) + self.assertEqual(result, ["run", "--rm"]) + + def test_v01_variables_placeholder_resolved_from_runtime_vars(self): + """v0.1 arg with variables dict resolves {workspaceFolder} from runtime_vars.""" + adapter = self._adapter() + result = adapter._process_arguments( + V01_DOCKER_RUNTIME_ARGS, + resolved_env={}, + runtime_vars={"workspaceFolder": "/home/user/project"}, + ) + self.assertIn("/home/user/project:/workspace", result) + + def test_v01_variables_unknown_var_gets_placeholder(self): + """Unknown variable gets a ${varName} placeholder (same as vscode).""" + adapter = self._adapter() + args = [ + { + "value_hint": "{customVar}:/data", + "variables": {"customVar": {"description": "Custom path", "is_required": True}}, + } + ] + result = adapter._process_arguments(args, resolved_env={}, runtime_vars={}) + self.assertEqual(result, ["${customVar}:/data"]) + + def test_v01_full_docker_arg_set_preserved(self): + """All 8 args from the v0.1 Docker fixture are present.""" + adapter = self._adapter() + result = adapter._process_arguments( + V01_DOCKER_RUNTIME_ARGS, + resolved_env={}, + runtime_vars={"workspaceFolder": "/ws"}, + ) + self.assertEqual(len(result), 8) + self.assertEqual(result[0], "run") + self.assertEqual(result[1], "-i") + self.assertEqual(result[2], "--rm") + self.assertEqual(result[3], "-v") + self.assertEqual(result[4], "/ws:/workspace") + self.assertEqual(result[5], "-w") + self.assertEqual(result[6], "/workspace") + self.assertEqual(result[7], "ghcr.io/example/playwright-mcp:1.2.3") + + def test_legacy_optional_hint_skipped(self): + """Legacy entries with is_required: False must not be appended.""" + adapter = self._adapter() + args = [ + {"value_hint": "--optional-flag", "is_required": False}, + {"value_hint": "required-arg"}, + ] + result = adapter._process_arguments(args, resolved_env={}, runtime_vars={}) + self.assertEqual(result, ["required-arg"]) + self.assertNotIn("--optional-flag", result) + + def test_legacy_optional_hint_with_variables_skipped(self): + """Legacy entries with is_required: False and a variables dict are skipped.""" + adapter = self._adapter() + args = [ + { + "value_hint": "{optionalPath}:/data", + "is_required": False, + "variables": { + "optionalPath": {"description": "Optional mount", "is_required": False} + }, + }, + {"value_hint": "required-arg"}, + ] + result = adapter._process_arguments(args, resolved_env={}, runtime_vars={}) + self.assertEqual(result, ["required-arg"]) + + +# --------------------------------------------------------------------------- +# Codex adapter +# --------------------------------------------------------------------------- + + +class TestCodexProcessArgumentsV01Variables(unittest.TestCase): + """_process_arguments must handle v0.1 value_hint + variables args.""" + + def _adapter(self) -> CodexClientAdapter: + return _make_codex() + + def test_v01_plain_value_hint_args_extracted(self): + """Plain value_hint args (no variables, no type) are extracted.""" + adapter = self._adapter() + result = adapter._process_arguments( + [{"value_hint": "run"}, {"value_hint": "--rm"}], + resolved_env={}, + runtime_vars={}, + ) + self.assertEqual(result, ["run", "--rm"]) + + def test_v01_variables_placeholder_resolved_from_runtime_vars(self): + """v0.1 arg with variables dict resolves {workspaceFolder} from runtime_vars.""" + adapter = self._adapter() + result = adapter._process_arguments( + V01_DOCKER_RUNTIME_ARGS, + resolved_env={}, + runtime_vars={"workspaceFolder": "/home/user/project"}, + ) + self.assertIn("/home/user/project:/workspace", result) + + def test_v01_variables_unknown_var_gets_placeholder(self): + """Unknown variable gets a ${varName} placeholder (same as vscode).""" + adapter = self._adapter() + args = [ + { + "value_hint": "{customVar}:/data", + "variables": {"customVar": {"description": "Custom path", "is_required": True}}, + } + ] + result = adapter._process_arguments(args, resolved_env={}, runtime_vars={}) + self.assertEqual(result, ["${customVar}:/data"]) + + def test_v01_full_docker_arg_set_preserved(self): + """All 8 args from the v0.1 Docker fixture are present.""" + adapter = self._adapter() + result = adapter._process_arguments( + V01_DOCKER_RUNTIME_ARGS, + resolved_env={}, + runtime_vars={"workspaceFolder": "/ws"}, + ) + self.assertEqual(len(result), 8) + self.assertEqual(result[0], "run") + self.assertEqual(result[4], "/ws:/workspace") + + def test_legacy_optional_hint_skipped(self): + """Legacy entries with is_required: False must not be appended.""" + adapter = self._adapter() + args = [ + {"value_hint": "--optional-flag", "is_required": False}, + {"value_hint": "required-arg"}, + ] + result = adapter._process_arguments(args, resolved_env={}, runtime_vars={}) + self.assertEqual(result, ["required-arg"]) + self.assertNotIn("--optional-flag", result) + + def test_legacy_optional_hint_with_variables_skipped(self): + """Legacy entries with is_required: False and a variables dict are skipped.""" + adapter = self._adapter() + args = [ + { + "value_hint": "{optionalPath}:/data", + "is_required": False, + "variables": { + "optionalPath": {"description": "Optional mount", "is_required": False} + }, + }, + {"value_hint": "required-arg"}, + ] + result = adapter._process_arguments(args, resolved_env={}, runtime_vars={}) + self.assertEqual(result, ["required-arg"]) + + +if __name__ == "__main__": + unittest.main()