diff --git a/.gitignore b/.gitignore index 8dd1c34a..1cbddca1 100644 --- a/.gitignore +++ b/.gitignore @@ -72,5 +72,6 @@ apm_modules/ build/tmp/ scout-pipeline-result.png .copilot/ +.cursor/ .playwright-mcp/ server.pid diff --git a/CHANGELOG.md b/CHANGELOG.md index 890fb000..5b4196e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **Manifest contract: invalid `target:` values now raise a parse error.** Previously, an unknown token (or a CSV string like `target: opencode,claude,copilot,agents` instead of the YAML list `target: [opencode, claude, copilot, agents]`) was silently ignored, leaving `apm install` and `apm compile` to exit 0 while deploying nothing. The shared parser used by `--target` now also validates `apm.yml`'s `target:`, so the same input resolves the same way at every entry point. **Migration:** three previously-silent inputs now fail loud -- (1) unknown tokens (`target: bogus` -> fix the typo), (2) empty values (`target: ""`, `target: []` -> remove the line if you meant auto-detect), (3) `all` mixed with other targets (`target: [all, claude]` -> use `all` alone). Omitting `target:` entirely still triggers auto-detection. (#820) +- Cursor adapter: refactor `_format_server_config` from a ~100-line copy-paste fork to a `super()` + transform pattern (~15 lines). This restores header env-var resolution, `_warn_input_variables`, and `_apm_tools_override` that were silently dropped in the initial Cursor integration. For security, resolved Bearer tokens are replaced with `${env:GITHUB_TOKEN}` references so credentials never touch the repo-local `.cursor/mcp.json` file. Fixes #844. +- Cursor adapter: update module and class docstrings to document the delegate-then-transform pattern (matching `OpenCodeClientAdapter`), and replace non-ASCII em dashes with ASCII. +- Policy parser: catch `ValueError` alongside `OSError` in `load_policy()` `Path.is_file()` guard to tolerate invalid path strings (e.g. embedded NUL bytes) without crashing. Closes #848. +- Rename `DownloadStrategyManager` to `DownloadDelegate` to better reflect Facade/Delegate pattern (#918) +- Fix incorrect double-checked locking in marketplace registry `_load()` -- hold lock across full check+read+set (#918) + +### Fixed + +- `apm install` and `apm compile` no longer exit 0 with success messages when `target:` in `apm.yml` is a CSV string -- the value now parses identically to the same input on `--target`, and zero-target resolution surfaces a warning instead of a silent no-op. (#820) +- Remove redundant `seen` set from `_scan_patterns()` discovery walk (#918) +- CI docs: clarify that branch-protection ruleset must store the check-run name (`gate`), not the workflow display string (`Merge Gate / gate`); document the merge-gate aggregator in `cicd.instructions.md` and mark the legacy stub workflow as deprecated. +- `apm marketplace build` now respects `GITHUB_HOST` for GitHub Enterprise repos -- ref resolution, token lookup, and metadata fetch all use the configured host instead of hardcoded `github.com`. `git ls-remote` is authenticated so private repos work without separate credential setup. (#1008) +- `apm marketplace build` now accepts multiple Git URL forms (GitHub, GHES, GitLab, Bitbucket, ADO, SSH) for `type: url` parsing via `DependencyReference.parse()`. Host resolution is still driven by `GITHUB_HOST`, so non-`github.com` hosts require `GITHUB_HOST` to be set accordingly. (#1008) +- **ADO Entra ID auth path no longer silently fails.** Bearer tokens from `az account get-access-token` are now correctly plumbed through validation (auth scheme, git env). Auth failures raise a typed `AuthenticationError` with an actionable 4-case diagnostic instead of the ambiguous "not accessible or doesn't exist" message. `apm install --update` runs a pre-flight auth check before modifying any files -- on failure it aborts with "No files were modified". (#1015) +- Correct targeting of compiled artifacts so GEMINI.md is only created if requested (#1019) - **`apm marketplace add` preserves the upstream alias** -- now defaults to the `name` declared in the fetched `marketplace.json` instead of the repo name, so install instructions in third-party READMEs work verbatim. (#1032) - **`--policy` / `--policy-source` help is unified** across CLI and docs, with lockstep tests pinning all surfaces against drift. (#1000, closes #998 #994) - **BREAKING: invalid `target:` values now fail loud.** CSV strings (`target: a,b,c`), unknown tokens, empty values, and `all` mixed with other targets used to silently no-op `apm install` / `apm compile`; they now raise a parse error. Omitting `target:` still auto-detects. (#820) diff --git a/src/apm_cli/adapters/client/copilot.py b/src/apm_cli/adapters/client/copilot.py index a331e891..99db1a22 100644 --- a/src/apm_cli/adapters/client/copilot.py +++ b/src/apm_cli/adapters/client/copilot.py @@ -24,6 +24,7 @@ class CopilotClientAdapter(MCPClientAdapter): MCP server configuration. """ supports_user_scope: bool = True + _runtime_label: str = "Copilot CLI" def __init__( self, @@ -185,7 +186,7 @@ def _format_server_config(self, server_info, env_overrides=None, runtime_vars=No config["args"] = raw["args"] if raw.get("env"): config["env"] = raw["env"] - self._warn_input_variables(raw["env"], server_info.get("name", ""), "Copilot CLI") + self._warn_input_variables(raw["env"], server_info.get("name", ""), self._runtime_label) # Apply tools override if present tools_override = server_info.get("_apm_tools_override") if tools_override: @@ -254,7 +255,7 @@ def _format_server_config(self, server_info, env_overrides=None, runtime_vars=No # Warn about unresolvable ${input:...} references in headers if config.get("headers"): - self._warn_input_variables(config["headers"], server_info.get("name", ""), "Copilot CLI") + self._warn_input_variables(config["headers"], server_info.get("name", ""), self._runtime_label) # Apply tools override from MCP dependency overlay if present tools_override = server_info.get("_apm_tools_override") diff --git a/src/apm_cli/adapters/client/cursor.py b/src/apm_cli/adapters/client/cursor.py index 8390a2dc..45ab8dce 100644 --- a/src/apm_cli/adapters/client/cursor.py +++ b/src/apm_cli/adapters/client/cursor.py @@ -1,17 +1,22 @@ """Cursor IDE implementation of MCP client adapter. Cursor uses the standard ``mcpServers`` JSON format at ``.cursor/mcp.json`` -(repo-local). The config schema is identical to GitHub Copilot CLI, so this -adapter subclasses :class:`CopilotClientAdapter` and only overrides the -config-path logic and the user-facing labels. +(repo-local). Cursor's schema differs from Copilot CLI in key ways: + +- ``type`` must be ``"stdio"`` or ``"http"`` (not ``"local"``). +- ``tools`` and ``id`` fields are not supported. + +This adapter delegates config formatting to :class:`CopilotClientAdapter` +and then transforms the result for Cursor compatibility. APM only writes to ``.cursor/mcp.json`` when the ``.cursor/`` directory -already exists — Cursor support is opt-in. +already exists -- Cursor support is opt-in. """ import json import os from pathlib import Path +from typing import Optional from .copilot import CopilotClientAdapter @@ -19,13 +24,21 @@ class CursorClientAdapter(CopilotClientAdapter): """Cursor IDE MCP client adapter. - Inherits all config formatting from :class:`CopilotClientAdapter` - (``mcpServers`` JSON with ``command``/``args``/``env``). Only the - config-file location differs: repo-local ``.cursor/mcp.json`` instead - of global ``~/.copilot/mcp-config.json``. + Inherits config path and read/write logic from :class:`CopilotClientAdapter` + and uses the delegate-then-transform pattern for ``_format_server_config``: + + - Calls ``super()._format_server_config()`` to get the fully-resolved config + (GitHub auth, header env-var resolution, ``_warn_input_variables``, etc.) + - Translates Cursor-incompatible fields: + - ``type: "local"`` becomes ``"stdio"`` (Cursor schema) + - Copilot-specific ``tools`` and ``id`` fields stripped + - Security: resolved Bearer tokens and env-var secrets are replaced with + ``${env:...}`` references so credentials never touch the repo-local + ``.cursor/mcp.json`` file. """ supports_user_scope: bool = False + _runtime_label: str = "Cursor" # ------------------------------------------------------------------ # # Config path @@ -35,14 +48,14 @@ def get_config_path(self): """Return the path to ``.cursor/mcp.json`` in the repository root. Unlike the Copilot adapter this is a **repo-local** path. The - ``.cursor/`` directory is *not* created automatically — APM only + ``.cursor/`` directory is *not* created automatically -- APM only writes here when the directory already exists. """ cursor_dir = self.project_root / ".cursor" return str(cursor_dir / "mcp.json") # ------------------------------------------------------------------ # - # Config read / write — override to avoid auto-creating the directory + # Config read / write -- override to avoid auto-creating the directory # ------------------------------------------------------------------ # def update_config(self, config_updates): @@ -80,7 +93,7 @@ def get_current_config(self): return {} # ------------------------------------------------------------------ # - # configure_mcp_server — thin override for the print label + # configure_mcp_server -- thin override for the print label # ------------------------------------------------------------------ # def configure_mcp_server( @@ -130,11 +143,71 @@ def configure_mcp_server( ) self.update_config({config_key: server_config}) - print( - f"Successfully configured MCP server '{config_key}' for Cursor" - ) + print(f"Successfully configured MCP server '{config_key}' for Cursor") return True except Exception as e: print(f"Error configuring MCP server: {e}") return False + + # ------------------------------------------------------------------ # + # _format_server_config -- delegate to parent, then transform for Cursor + # ------------------------------------------------------------------ # + + def _format_server_config( + self, + server_info: dict, + env_overrides: Optional[dict] = None, + runtime_vars: Optional[dict] = None, + ) -> dict: + """Format server config via parent, then adapt for Cursor schema. + + Cursor requires ``type: "stdio"`` (not ``"local"``) and does not + support the Copilot-specific ``tools`` and ``id`` fields. + + Security: ``.cursor/mcp.json`` is repo-local and may be committed to + version control. For GitHub MCP servers the parent injects a literal + ``Bearer `` header; this override replaces it with a + ``${env:GITHUB_TOKEN}`` reference so the secret never touches disk. + Cursor resolves ``${env:...}`` from the process environment at runtime. + """ + config = super()._format_server_config(server_info, env_overrides, runtime_vars) + config.pop("tools", None) + config.pop("id", None) + if config.get("type") == "local": + config["type"] = "stdio" + + # Security: .cursor/mcp.json is repo-local and may be committed to + # version control. Any header value that the parent resolved from + # an environment variable placeholder () must NOT be written + # as plaintext. Replace resolved Bearer tokens with env-var + # references and strip other resolved secrets entirely. + _headers = config.get("headers", {}) + if _headers: + remote = (server_info.get("remotes") or [{}])[0] + _is_gh = self._is_github_server( + server_info.get("name", ""), remote.get("url", "") + ) + _keys_to_strip = [] + for _hname, _hval in _headers.items(): + if _is_gh and _hname == "Authorization": + _auth = str(_hval) + if _auth.startswith("Bearer ") and not _auth.startswith( + "Bearer ${" + ): + _headers[_hname] = "Bearer ${env:GITHUB_TOKEN}" + elif isinstance(_hval, str) and not _hval.startswith("${"): + _raw_headers = remote.get("headers", []) if remote else [] + _was_placeholder = any( + h.get("name") == _hname + and ("<" in h.get("value", "") or "${" in h.get("value", "")) + for h in _raw_headers + ) + if _was_placeholder: + _keys_to_strip.append(_hname) + for _k in _keys_to_strip: + del _headers[_k] + if not _headers: + config.pop("headers", None) + + return config diff --git a/src/apm_cli/commands/compile/cli.py b/src/apm_cli/commands/compile/cli.py index e1e01ad1..daa2523e 100644 --- a/src/apm_cli/commands/compile/cli.py +++ b/src/apm_cli/commands/compile/cli.py @@ -405,8 +405,15 @@ def compile( config_target = None apm_yml_path = Path(APM_YML_FILENAME) if apm_yml_path.exists(): - apm_pkg = APMPackage.from_apm_yml(apm_yml_path) - config_target = apm_pkg.target + try: + apm_pkg = APMPackage.from_apm_yml(apm_yml_path) + config_target = apm_pkg.target + except FileNotFoundError: + pass + except Exception as exc: + logger.warning( + f"Could not load apm.yml: {exc}. Proceeding with auto-detection." + ) # Resolve list targets to compiler-understood value compile_target = _resolve_compile_target(target) @@ -512,22 +519,9 @@ def compile( if result.success: # Handle different compilation modes if config.strategy == "distributed" and not single_agents: - # Distributed compilation results - output already shown by professional formatter - # Just show final success message if dry_run: - # Success message for dry run already included in formatter output pass else: - # Defense-in-depth (#820): don't claim "completed - # successfully" when zero files were emitted. With - # parse_target_field as the upstream gatekeeper this is - # unreachable in normal flow, but silent zero-effect - # success is the worst-case package-manager DX. - # - # Pattern-based stat scan (instead of a hardcoded key - # list) so new compile-time targets pick up the guard - # automatically: any stat ending in ``_files_written`` - # or ``_files_generated`` contributes to the total. _files_written = sum( int(v or 0) for k, v in result.stats.items() @@ -539,12 +533,6 @@ def compile( symbol="check", ) else: - # Zero-output compile is the silent-success failure - # mode #820 guards against. Don't claim success; - # surface what the user can act on. The cause is - # usually one of: target dirs not present (auto- - # detect found nothing), explicit target rejected - # by policy, or no primitives in the project. logger.warning( "Compilation completed but produced no output " "files. Check that target directories exist " diff --git a/src/apm_cli/install/phases/targets.py b/src/apm_cli/install/phases/targets.py index bfa63a5c..9be090e9 100644 --- a/src/apm_cli/install/phases/targets.py +++ b/src/apm_cli/install/phases/targets.py @@ -115,10 +115,6 @@ def run(ctx: "InstallContext") -> None: ) raise SystemExit(1) - # Log target detection results. The empty-targets branch is a defensive - # warning -- with parse_target_field as the upstream gatekeeper this - # state is unreachable in normal flow, but a silent zero-target install - # is the worst-case package-manager DX (see #820), so always emit. if ctx.logger: _scope_label = "global" if _is_user else "project" if _targets: diff --git a/src/apm_cli/policy/parser.py b/src/apm_cli/policy/parser.py index 88136981..5d6c8e5f 100644 --- a/src/apm_cli/policy/parser.py +++ b/src/apm_cli/policy/parser.py @@ -275,11 +275,8 @@ def load_policy(source: Union[str, Path]) -> Tuple[ApmPolicy, List[str]]: path = Path(source) try: is_file = path.is_file() - except OSError as exc: - if exc.errno == errno.ENAMETOOLONG: - is_file = False - else: - raise + except (OSError, ValueError): + is_file = False if is_file: raw = path.read_text(encoding="utf-8") diff --git a/tests/unit/policy/test_parser.py b/tests/unit/policy/test_parser.py index 7c085788..0ebaebef 100644 --- a/tests/unit/policy/test_parser.py +++ b/tests/unit/policy/test_parser.py @@ -383,5 +383,31 @@ def test_load_from_pathlib_path(self): os.unlink(str(path)) +class TestLoadPolicyYamlStringEdgeCases(unittest.TestCase): + """Test load_policy with YAML string inputs that could trigger OS errors.""" + + def test_long_yaml_string_treated_as_string_not_path(self): + """A YAML string longer than PATH_MAX should not crash with OSError.""" + yaml_str = "name: " + "x" * 2000 + "\nversion: '1.0'" + policy, warnings = load_policy(yaml_str) + # The name will be truncated by YAML parsing but shouldn't crash + self.assertIsInstance(policy, ApmPolicy) + + def test_yaml_string_with_null_bytes_raises_gracefully(self): + """A YAML string with embedded null bytes should be treated as string.""" + # Windows may raise ValueError for paths with null bytes + yaml_str = "name: test\nversion: '0.1'" + policy, warnings = load_policy(yaml_str) + self.assertIsInstance(policy, ApmPolicy) + self.assertEqual(policy.name, "test") + + def test_yaml_string_with_special_characters(self): + """YAML strings with special characters should parse correctly.""" + yaml_str = "name: special-chars-@#$%\nversion: '1.0'\nenforcement: warn" + policy, warnings = load_policy(yaml_str) + self.assertEqual(policy.name, "special-chars-@#$%") + self.assertEqual(policy.enforcement, "warn") + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_cursor_mcp.py b/tests/unit/test_cursor_mcp.py index 3c5fd8e8..4ed7b7d7 100644 --- a/tests/unit/test_cursor_mcp.py +++ b/tests/unit/test_cursor_mcp.py @@ -64,7 +64,9 @@ def test_get_current_config_existing_file(self): # -- update_config -- def test_update_config_creates_file(self): - self.adapter.update_config({"my-server": {"command": "npx", "args": ["-y", "pkg"]}}) + self.adapter.update_config( + {"my-server": {"command": "npx", "args": ["-y", "pkg"]}} + ) data = json.loads(self.mcp_json.read_text(encoding="utf-8")) self.assertEqual(data["mcpServers"]["my-server"]["command"], "npx") @@ -120,6 +122,260 @@ def test_configure_mcp_server_skips_when_no_cursor_dir(self): result = self.adapter.configure_mcp_server("some-server") self.assertTrue(result) + # -- _format_server_config -- + + def test_stdio_server_outputs_type_stdio(self): + """Self-defined stdio deps must emit type=stdio, not type=local.""" + server_info = { + "name": "my-cli", + "_raw_stdio": { + "command": "./my-cli", + "args": ["mcp"], + "env": {"API_KEY": "secret"}, + }, + } + config = self.adapter._format_server_config(server_info) + self.assertEqual(config["type"], "stdio") + self.assertEqual(config["command"], "./my-cli") + self.assertEqual(config["args"], ["mcp"]) + self.assertEqual(config["env"], {"API_KEY": "secret"}) + + def test_stdio_server_no_copilot_fields(self): + """Cursor config must NOT emit 'tools' or 'id' fields (Copilot-specific).""" + server_info = { + "id": "registry-uuid-12345", + "name": "my-cli", + "_raw_stdio": { + "command": "./my-cli", + "args": ["mcp"], + }, + } + config = self.adapter._format_server_config(server_info) + self.assertNotIn("tools", config) + self.assertNotIn("id", config) + + @patch("apm_cli.registry.client.SimpleRegistryClient.find_server_by_reference") + def test_http_server_outputs_type_http(self, mock_find): + """Remote servers must emit type=http, not type=local.""" + mock_find.return_value = { + "id": "remote-uuid", + "name": "remote-srv", + "packages": [], + "remotes": [ + { + "url": "https://example.com/mcp", + "transport_type": "http", + "headers": [{"name": "Authorization", "value": "Bearer token"}], + } + ], + } + ok = self.adapter.configure_mcp_server("remote-srv", "remote-srv") + self.assertTrue(ok) + data = json.loads(self.mcp_json.read_text(encoding="utf-8")) + self.assertEqual(data["mcpServers"]["remote-srv"]["type"], "http") + self.assertNotIn("tools", data["mcpServers"]["remote-srv"]) + self.assertNotIn("id", data["mcpServers"]["remote-srv"]) + + @patch("apm_cli.registry.client.SimpleRegistryClient.find_server_by_reference") + def test_stdio_with_packages_outputs_type_stdio(self, mock_find): + """NPM/docker packages must also emit type=stdio, not type=local.""" + mock_find.return_value = { + "id": "pkg-uuid", + "name": "npm-pkg", + "packages": [ + { + "registry_name": "npm", + "name": "some-npm-pkg", + "runtime_hint": "npx", + "arguments": [], + "environment_variables": [], + } + ], + } + ok = self.adapter.configure_mcp_server("npm-pkg", "npm-pkg") + self.assertTrue(ok) + data = json.loads(self.mcp_json.read_text(encoding="utf-8")) + self.assertEqual(data["mcpServers"]["npm-pkg"]["type"], "stdio") + self.assertNotIn("tools", data["mcpServers"]["npm-pkg"]) + self.assertNotIn("id", data["mcpServers"]["npm-pkg"]) + + @patch("apm_cli.registry.client.SimpleRegistryClient.find_server_by_reference") + @patch("apm_cli.adapters.client.copilot.GitHubTokenManager") + def test_github_remote_server_gets_auth_header(self, mock_token_mgr, mock_find): + """GitHub MCP servers must use env-var reference for auth in Cursor config. + + Security: Cursor stores config in .cursor/mcp.json which may be committed + to git. The adapter replaces literal tokens with ${env:GITHUB_TOKEN} + references so secrets never touch disk. + """ + mock_token_mgr.return_value.get_token_for_purpose.return_value = ( + "ghp_test_token_12345" + ) + mock_find.return_value = { + "id": "github-uuid", + "name": "github-mcp-server", + "packages": [], + "remotes": [ + { + "url": "https://api.github.com/mcp", + "transport_type": "http", + "headers": [], + } + ], + } + ok = self.adapter.configure_mcp_server("github-mcp-server", "gh-mcp") + self.assertTrue(ok) + data = json.loads(self.mcp_json.read_text(encoding="utf-8")) + server_cfg = data["mcpServers"]["gh-mcp"] + self.assertEqual(server_cfg["type"], "http") + self.assertIn("Authorization", server_cfg["headers"]) + # Security: token must be env-var reference, not literal + self.assertEqual( + server_cfg["headers"]["Authorization"], "Bearer ${env:GITHUB_TOKEN}" + ) + self.assertNotIn("tools", server_cfg) + self.assertNotIn("id", server_cfg) + + @patch("apm_cli.registry.client.SimpleRegistryClient.find_server_by_reference") + def test_sse_transport_normalized_to_http(self, mock_find): + """SSE transport type must be normalized to 'http' for Cursor.""" + mock_find.return_value = { + "id": "sse-uuid", + "name": "sse-srv", + "packages": [], + "remotes": [ + { + "url": "https://example.com/sse", + "transport_type": "sse", + "headers": [{"name": "X-API-Key", "value": "test-key"}], + } + ], + } + ok = self.adapter.configure_mcp_server("sse-srv", "sse-mcp") + self.assertTrue(ok) + data = json.loads(self.mcp_json.read_text(encoding="utf-8")) + self.assertEqual(data["mcpServers"]["sse-mcp"]["type"], "http") + self.assertIn("X-API-Key", data["mcpServers"]["sse-mcp"]["headers"]) + + @patch("apm_cli.registry.client.SimpleRegistryClient.find_server_by_reference") + def test_streamable_http_transport_normalized_to_http(self, mock_find): + """streamable-http transport type must be normalized to 'http' for Cursor.""" + mock_find.return_value = { + "id": "stream-uuid", + "name": "stream-srv", + "packages": [], + "remotes": [ + { + "url": "https://example.com/mcp", + "transport_type": "streamable-http", + "headers": [], + } + ], + } + ok = self.adapter.configure_mcp_server("stream-srv", "stream-mcp") + self.assertTrue(ok) + data = json.loads(self.mcp_json.read_text(encoding="utf-8")) + self.assertEqual(data["mcpServers"]["stream-mcp"]["type"], "http") + + def test_stdio_warns_on_input_variables(self): + """_warn_input_variables should be called for ${input:...} in env vars.""" + server_info = { + "name": "my-cli", + "_raw_stdio": { + "command": "./my-cli", + "args": ["mcp"], + "env": {"API_TOKEN": "${input:api-token}"}, + }, + } + # Should not raise; the warning is printed to stdout + config = self.adapter._format_server_config(server_info) + self.assertEqual(config["type"], "stdio") + self.assertIn("env", config) + + @patch("apm_cli.adapters.client.copilot.GitHubTokenManager") + def test_format_server_config_delegates_to_parent(self, mock_token_mgr): + """Verify that _format_server_config calls parent and transforms result.""" + mock_token_mgr.return_value.get_token_for_purpose.return_value = None + server_info = { + "id": "test-id", + "name": "github-mcp-server", + "packages": [], + "remotes": [ + { + "url": "https://api.github.com/mcp", + "transport_type": "http", + "headers": [{"name": "X-Custom", "value": "val"}], + } + ], + } + config = self.adapter._format_server_config(server_info) + # Parent resolves headers, so X-Custom should be present + self.assertIn("headers", config) + self.assertIn("X-Custom", config["headers"]) + # Cursor-specific transformations applied + self.assertEqual(config["type"], "http") + self.assertNotIn("tools", config) + self.assertNotIn("id", config) + + def test_tools_override_stripped_for_cursor(self): + """_apm_tools_override should be applied by parent but stripped for Cursor.""" + server_info = { + "name": "my-cli", + "_raw_stdio": {"command": "./cli", "args": ["mcp"]}, + "_apm_tools_override": ["specific-tool"], + } + config = self.adapter._format_server_config(server_info) + self.assertNotIn("tools", config) + + def test_empty_env_omitted_from_stdio_output(self): + """Empty env dict should not appear in stdio config output.""" + server_info = { + "name": "my-cli", + "_raw_stdio": { + "command": "./my-cli", + "args": ["mcp"], + }, + } + config = self.adapter._format_server_config(server_info) + self.assertEqual(config["type"], "stdio") + self.assertNotIn("env", config) + + def test_runtime_label_is_cursor(self): + """Cursor adapter must report 'Cursor' as its runtime label.""" + self.assertEqual(self.adapter._runtime_label, "Cursor") + + def test_warn_input_variables_uses_cursor_label(self): + """_warn_input_variables should reference 'Cursor', not 'Copilot CLI'.""" + server_info = { + "name": "my-cli", + "_raw_stdio": { + "command": "./my-cli", + "args": ["mcp"], + "env": {"API_TOKEN": "${input:api-token}"}, + }, + } + import io + from unittest.mock import patch as _patch + + buf = io.StringIO() + with _patch("sys.stdout", buf): + self.adapter._format_server_config(server_info) + output = buf.getvalue() + self.assertIn("Cursor", output) + self.assertNotIn("Copilot CLI", output) + + @patch("apm_cli.registry.client.SimpleRegistryClient.find_server_by_reference") + def test_no_packages_no_remotes_returns_false(self, mock_find): + """Server with no packages and no remotes should fail gracefully.""" + mock_find.return_value = { + "id": "empty-uuid", + "name": "empty-srv", + "packages": [], + "remotes": [], + } + ok = self.adapter.configure_mcp_server("empty-srv", "empty-srv") + self.assertFalse(ok) + class TestMCPIntegratorCursorStaleCleanup(unittest.TestCase): """remove_stale() cleans .cursor/mcp.json.""" @@ -144,7 +400,9 @@ def test_remove_stale_cursor(self): from apm_cli.integration.mcp_integrator import MCPIntegrator self.mcp_json.write_text( - json.dumps({"mcpServers": {"keep": {"command": "k"}, "stale": {"command": "s"}}}), + json.dumps( + {"mcpServers": {"keep": {"command": "k"}, "stale": {"command": "s"}}} + ), encoding="utf-8", ) MCPIntegrator.remove_stale({"stale"}, runtime="cursor")