From 0b263d0f1e726dcd258fa3fc91558354659ddce7 Mon Sep 17 00:00:00 2001 From: RamsesJM <159502489+RamsesJM@users.noreply.github.com> Date: Wed, 28 Oct 2026 21:04:45 -0400 Subject: [PATCH 1/3] feat(mcp): write native OpenCode 2 entries under mcp.servers OpenCode 1 and 2 share opencode.json locations, and OpenCode 1 cannot read the v2 mcp.servers layout. Select the layout from the target file: when it declares an mcp.servers map, write native v2 entries there; otherwise keep the v1 layout, which OpenCode 2 also reads. In the v2 layout, boolean enabled is written as disabled and other values are rejected without touching the file. Extra fields such as codemode pass through unchanged. A v1 entry of the same name is removed only when it is unchanged from what APM wrote, and stale cleanup also prunes mcp.servers. Refs #3112 --- CHANGELOG.md | 4 + .../docs/consumer/install-mcp-servers.md | 43 +++- .../content/docs/reference/manifest-schema.md | 2 +- src/apm_cli/adapters/client/opencode.py | 119 ++++++++- src/apm_cli/integration/mcp_integrator.py | 17 +- .../integration/test_opencode_v2_lifecycle.py | 109 ++++++++ tests/unit/test_opencode_mcp_v2.py | 243 ++++++++++++++++++ 7 files changed, 526 insertions(+), 11 deletions(-) create mode 100644 tests/integration/test_opencode_v2_lifecycle.py create mode 100644 tests/unit/test_opencode_mcp_v2.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 82960dc779..3e84a43b5b 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] +### Added + +- OpenCode targets write native OpenCode 2 MCP entries under `mcp.servers` when `opencode.json` already declares that map, translating `enabled` to `disabled` and keeping `codemode` and other extra fields; files without it keep the OpenCode 1 layout. -- by @RamsesJM (#3112) + ## [0.35.0] - 2026-10-22 ### Added diff --git a/docs/src/content/docs/consumer/install-mcp-servers.md b/docs/src/content/docs/consumer/install-mcp-servers.md index 074a4ba70b..fb64b8ce3b 100644 --- a/docs/src/content/docs/consumer/install-mcp-servers.md +++ b/docs/src/content/docs/consumer/install-mcp-servers.md @@ -166,7 +166,7 @@ unresolved required entries fail closed. | Gemini CLI | `.gemini/settings.json` (project, only if `.gemini/` exists) or `~/.gemini/settings.json` (`-g`) | both | JSON `mcpServers` | | Antigravity CLI | `.agents/mcp_config.json` (project, only if `.agents/` exists) or `~/.gemini/config/mcp_config.json` (`-g`) | both | JSON `mcpServers` | | Hermes Agent | `$HERMES_HOME/config.yaml` (unset/blank: `~/.hermes/config.yaml`; explicit `--target hermes` only) | home-scoped | YAML `mcp_servers` | -| OpenCode | `opencode.json` (project, only if `.opencode/` exists) or `~/.config/opencode/opencode.json` (`-g`) | both | JSON `mcp` | +| OpenCode | `opencode.json` (project, only if `.opencode/` exists) or `~/.config/opencode/opencode.json` (`-g`) | both | JSON `mcp` (v1) or `mcp.servers` (v2) | | Windsurf | `~/.codeium/windsurf/mcp_config.json` | global | JSON `mcpServers` | | Kiro IDE | `.kiro/settings/mcp.json` (project, only if `.kiro/` exists) or `~/.kiro/settings/mcp.json` (`-g`) | both | JSON `mcpServers` | | JetBrains Copilot | `%LOCALAPPDATA%\github-copilot\intellij\mcp.json` (Windows) or `$XDG_CONFIG_HOME/github-copilot/intellij/mcp.json` (macOS/Linux; defaults to `~/.config/github-copilot/intellij/mcp.json`) | global | JSON `servers` | @@ -182,7 +182,8 @@ value unchanged into that server's `opencode.json` entry. If omitted, APM keeps the existing `true` default. APM does not validate or coerce explicit values; OpenCode interprets them. Other targets ignore this OpenCode-only field. Reinstall applies changes to `enabled`, including its JSON type; removing -the field restores `true`. +the field restores `true`. For the OpenCode 2 layout, see +[OpenCode v2 configuration](#opencode-v2-configuration). Global Cursor and OpenCode installs preserve your personal servers, including ones that share a name with a package-declared server. Reinstalling and @@ -197,6 +198,44 @@ it, even on a no-op reinstall. Fix the reported config and rerun the original command with `--global` and the same `--target` selection. A home-directory alias itself remains supported. +### OpenCode v2 configuration + +OpenCode 1 and OpenCode 2 read the same `opencode.json` files, but only +OpenCode 2 understands its native layout, which nests servers under +`mcp.servers`. APM therefore chooses the layout from the target file +itself: + +- If the file declares an `mcp.servers` object, APM writes each server + there in the OpenCode 2 format. +- Otherwise APM keeps writing the OpenCode 1 layout, which OpenCode 2 also + reads. + +To opt in, add an empty map to the target file and reinstall: + +```json +{ + "mcp": { + "servers": {} + } +} +``` + +In the OpenCode 2 layout, `enabled: false` is written as `disabled: true`, +and `enabled: true` or an omitted value writes no field, the OpenCode 2 +default. Other values have no OpenCode 2 equivalent, so APM warns and +leaves the file unchanged. To expose a server's tools directly instead of +through Code Mode, set `codemode: false` in the dependency's `extra` block; +APM writes extra fields verbatim, so use the OpenCode 2 spelling for any +other native setting, such as snake_case `oauth` fields. + +APM does not convert configuration it did not write. When it switches to +`mcp.servers`, it removes an OpenCode 1 entry of the same name only if the +entry is unchanged from what APM wrote; an edited entry stays in place with +a warning, and OpenCode 2 uses the `mcp.servers` entry. Your own servers, +`mcp.timeout` and other settings are preserved, and pruning removes only +entries APM wrote. Remove `mcp.servers` before running OpenCode 1 against +the same file, since OpenCode 1 cannot read it. + ## Resolve a connection name conflict For self-defined Claude and Codex connections, a normal package update or root diff --git a/docs/src/content/docs/reference/manifest-schema.md b/docs/src/content/docs/reference/manifest-schema.md index f87244d751..6c85e44ac8 100644 --- a/docs/src/content/docs/reference/manifest-schema.md +++ b/docs/src/content/docs/reference/manifest-schema.md @@ -681,7 +681,7 @@ A plain registry reference: `io.github.github/github-mcp-server`. | `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`. | -| `enabled` | any JSON value | OPTIONAL | OpenCode only | Passed unchanged to OpenCode's `opencode.json` entry. When omitted, APM keeps its existing `true` default. APM does not validate the value; OpenCode interprets it. Other targets ignore this field. | +| `enabled` | any JSON value | OPTIONAL | OpenCode only | Passed unchanged to OpenCode's `opencode.json` entry. When omitted, APM keeps its existing `true` default. APM does not validate the value; OpenCode interprets it. In the OpenCode 2 `mcp.servers` layout, a boolean is written as `disabled` and any other value is rejected; see [OpenCode v2 configuration](../../consumer/install-mcp-servers/#opencode-v2-configuration). Other targets ignore this field. | Any additional keys not listed above are preserved as **extra passthrough fields** and round-tripped verbatim into the generated target manifests. This allows harness-specific configuration (e.g. Claude Code's `oauth` block for remote-MCP OAuth client config) to be declared in `apm.yml` and appear in the generated config without modification. A warning is emitted at parse time naming each non-standard key. diff --git a/src/apm_cli/adapters/client/opencode.py b/src/apm_cli/adapters/client/opencode.py index 59973735a2..408f5df401 100644 --- a/src/apm_cli/adapters/client/opencode.py +++ b/src/apm_cli/adapters/client/opencode.py @@ -23,6 +23,12 @@ - Command format: single array ``command`` (not ``command`` + ``args``) - Env key: ``environment`` (not ``env``) +OpenCode v2 nests servers under ``mcp.servers`` and replaces ``enabled`` +with its inverse, ``disabled``. Both versions read the same files, but +v1 cannot read the v2 layout, so APM writes v2 entries only when the +target file already declares an ``mcp.servers`` map. Otherwise it keeps +the v1 layout above, which v2 also accepts. + At project scope APM only writes to ``opencode.json`` when the ``.opencode/`` directory already exists -- OpenCode support is opt-in. """ @@ -34,11 +40,16 @@ from typing import Any from ...models.dependency.mcp import _EXTRA_DENYLIST, opencode_enabled_value +from ...utils.console import _rich_warning from .copilot import CopilotClientAdapter _ENV_REF_TEMPLATE = "{env:%s}" _ENV_REF_RE = re.compile(re.escape(_ENV_REF_TEMPLATE) % r"([A-Za-z_][A-Za-z0-9_]*)") +_V2_SERVERS_KEY = "servers" +# Settings that share the ``mcp`` object with v1 server entries in v2 files. +_V2_MCP_SETTINGS = frozenset({_V2_SERVERS_KEY, "timeout"}) + class OpenCodeClientAdapter(CopilotClientAdapter): """OpenCode MCP client adapter. @@ -127,12 +138,71 @@ def update_config(self, config_updates, enabled: Any = True): if "mcp" not in current_config: current_config["mcp"] = {} + if self.uses_native_v2(current_config): + return self._update_v2_config(config_path, current_config, config_updates, enabled) + for name, copilot_entry in config_updates.items(): current_config["mcp"][name] = self._to_opencode_format(copilot_entry, enabled=enabled) self._write_json_config(config_path, current_config) return True + def _update_v2_config( + self, + config_path: Path, + current_config: dict, + config_updates: dict, + enabled: Any, + ) -> bool: + """Write *config_updates* under ``mcp.servers`` in a v2 config. + + A v1-layout entry with the same name is removed only when it is + exactly what APM would have written there, so user edits survive. + Nothing is written if any entry cannot be expressed in v2. + """ + mcp = current_config["mcp"] + servers = mcp[_V2_SERVERS_KEY] + try: + rendered = { + name: self._to_opencode_v2_format(entry, enabled=enabled) + for name, entry in config_updates.items() + } + except ValueError as exc: + _rich_warning(f"{exc} -- skipping {config_path}", symbol="warning") + return False + + for name, entry in rendered.items(): + legacy = mcp.get(name) if name not in _V2_MCP_SETTINGS else None + if legacy is not None: + if legacy == self._to_opencode_format(config_updates[name], enabled=enabled): + del mcp[name] + else: + _rich_warning( + f"Kept the edited v1 entry 'mcp.{name}' in {config_path}; " + f"OpenCode v2 uses 'mcp.servers.{name}' instead", + symbol="warning", + ) + servers[name] = entry + + self._write_json_config(config_path, current_config) + return True + + @staticmethod + def uses_native_v2(config: Any) -> bool: + """Return whether *config* declares the OpenCode v2 server layout. + + A v1 server that happens to be named ``servers`` carries a string + ``type`` and is not treated as a v2 server map. + """ + mcp = config.get("mcp") if isinstance(config, dict) else None + servers = mcp.get(_V2_SERVERS_KEY) if isinstance(mcp, dict) else None + return isinstance(servers, dict) and not isinstance(servers.get("type"), str) + + @classmethod + def native_v2_servers(cls, config: dict) -> dict | None: + """Return the mutable ``mcp.servers`` map of a v2 config, else ``None``.""" + return config["mcp"][_V2_SERVERS_KEY] if cls.uses_native_v2(config) else None + def validate_config_for_install(self) -> bool: """Refuse unsafe user configs even when no server write would be needed.""" return ( @@ -141,7 +211,22 @@ def validate_config_for_install(self) -> bool: ) def get_current_config(self): - """Read the current ``opencode.json`` contents.""" + """Read the current ``opencode.json`` contents. + + For a v2 config, ``mcp`` lists the effective servers: v1-layout + entries overlaid by ``mcp.servers``, matching OpenCode's own + precedence when both declare a name. + """ + config = self._read_config_file() + if not self.uses_native_v2(config): + return config + mcp = config["mcp"] + effective = {name: entry for name, entry in mcp.items() if name not in _V2_MCP_SETTINGS} + effective.update(mcp[_V2_SERVERS_KEY]) + return {**config, "mcp": effective} + + def _read_config_file(self) -> dict: + """Return the parsed config file, or ``{}`` when it is absent or unreadable.""" config_path = self.get_config_path() if not os.path.exists(config_path): return {} @@ -152,11 +237,12 @@ def get_current_config(self): return {} def render_server_config(self, server_info: dict) -> dict: - """Render the ``mcp`` entry APM writes, for exact baseline comparisons.""" - return self._to_opencode_format( - super().render_server_config(server_info), - enabled=opencode_enabled_value(server_info), - ) + """Render the entry APM writes, for exact baseline comparisons.""" + copilot_entry = super().render_server_config(server_info) + enabled = opencode_enabled_value(server_info) + if self.uses_native_v2(self._read_config_file()): + return self._to_opencode_v2_format(copilot_entry, enabled=enabled) + return self._to_opencode_format(copilot_entry, enabled=enabled) def configure_mcp_server( self, @@ -233,3 +319,24 @@ def _to_opencode_format(copilot_entry: dict, enabled: Any = True) -> dict: entry[key] = value return entry + + @classmethod + def _to_opencode_v2_format(cls, copilot_entry: dict, enabled: Any = True) -> dict: + """Convert a Copilot-format server config to the OpenCode v2 layout. + + Identical to the v1 entry except that ``enabled`` becomes + ``disabled: true`` when false and is omitted when true, the v2 + default. Passthrough fields such as ``codemode`` are kept as is. + + Raises: + ValueError: *enabled* is not a boolean, so it has no v2 equivalent. + """ + if not isinstance(enabled, bool): + raise ValueError( + f"OpenCode v2 needs a boolean 'enabled' value, got {json.dumps(enabled)}" + ) + entry = cls._to_opencode_format(copilot_entry, enabled=enabled) + del entry["enabled"] + if not enabled: + entry["disabled"] = True + return entry diff --git a/src/apm_cli/integration/mcp_integrator.py b/src/apm_cli/integration/mcp_integrator.py index 805e5452f5..cdbdf6d937 100644 --- a/src/apm_cli/integration/mcp_integrator.py +++ b/src/apm_cli/integration/mcp_integrator.py @@ -19,7 +19,7 @@ import re import shutil import warnings -from collections.abc import MutableMapping +from collections.abc import Callable, MutableMapping from pathlib import Path from typing import TYPE_CHECKING @@ -131,6 +131,7 @@ def _clean_json_mcp_config( trailing_newline: bool = False, use_rich: bool = False, fail_on_write_error: bool = False, + nested_servers: Callable[[dict], dict | None] | None = None, ) -> int: """Remove stale entries from a JSON-based MCP config file. @@ -143,6 +144,9 @@ def _clean_json_mcp_config( trailing_newline: When True, append a trailing newline after JSON serialisation. use_rich: When True, emit removal notices via ``_rich_success``; otherwise use ``logger.progress``. + nested_servers: Returns a second server map inside the parsed config, + or ``None``. Stale names are removed from it as well, for layouts + such as OpenCode v2 that keep servers below the top-level key. Returns: Number of entries removed. @@ -164,9 +168,17 @@ def _clean_json_mcp_config( servers = config.get(servers_key, {}) if not isinstance(servers, dict): raise ValueError(f"{label} {servers_key} must be a mapping") - removed = [n for n in stale_names if n in servers] + nested = nested_servers(config) if nested_servers else None + removed = [ + n for n in stale_names if n in servers and (nested is None or servers[n] is not nested) + ] for name in removed: del servers[name] + if nested: + nested_removed = [n for n in stale_names if n in nested] + for name in nested_removed: + del nested[name] + removed = sorted(builtins.set(removed) | builtins.set(nested_removed)) if removed: text = json.dumps(config, indent=2) if trailing_newline: @@ -871,6 +883,7 @@ def remove_stale( "OpenCode MCP config", servers_key="mcp", fail_on_write_error=fail_on_write_error, + nested_servers=OpenCodeClientAdapter.native_v2_servers, ) if "windsurf" in target_runtimes: diff --git a/tests/integration/test_opencode_v2_lifecycle.py b/tests/integration/test_opencode_v2_lifecycle.py new file mode 100644 index 0000000000..82fd4c270c --- /dev/null +++ b/tests/integration/test_opencode_v2_lifecycle.py @@ -0,0 +1,109 @@ +"""Real CLI lifecycle coverage for the OpenCode v2 MCP layout.""" + +from __future__ import annotations + +import json +import os +from pathlib import Path + +import pytest + +from apm_cli.deps.lockfile import LockFile +from apm_cli.utils.yaml_io import dump_yaml +from tests.utils.apm_lifecycle_runner import ApmLifecycleRunner +from tests.utils.isolated_apm_environment import IsolatedApmEnvironment + +pytestmark = [ + pytest.mark.integration, + pytest.mark.e2e, + pytest.mark.lifecycle_smoke, + pytest.mark.requires_apm_binary, + pytest.mark.requires_e2e_mode, +] + +PERSONAL_SERVER = {"type": "local", "command": ["personal-server"]} +USER_TIMEOUTS = {"startup": 5000} + + +def test_opencode_v2_install_reinstall_and_prune(tmp_path: Path, apm_binary_path: Path) -> None: + """A v2 config receives native entries and keeps user-owned settings.""" + isolated = IsolatedApmEnvironment.create(tmp_path / "scenario", base_env=dict(os.environ)) + project = isolated.work_root / "project" + project.mkdir() + (project / ".opencode").mkdir() + config_path = project / "opencode.json" + config_path.write_text( + json.dumps( + { + "theme": "user-theme", + "mcp": {"timeout": USER_TIMEOUTS, "servers": {"personal": PERSONAL_SERVER}}, + } + ), + encoding="utf-8", + ) + local = { + "name": "local", + "registry": False, + "transport": "stdio", + "command": "echo", + "enabled": False, + } + remote = { + "name": "remote", + "registry": False, + "transport": "http", + "url": "https://example.com/mcp", + "extra": {"codemode": False}, + } + manifest = { + "name": "opencode-v2-lifecycle", + "version": "1.0.0", + "targets": ["opencode"], + "dependencies": {"apm": [], "mcp": [local, remote]}, + } + dump_yaml(manifest, project / "apm.yml") + LockFile().write(project / "apm.lock.yaml") + runner = ApmLifecycleRunner((str(apm_binary_path),)) + environment = isolated.subprocess_env() + + def install(scenario_id: str): + return runner.run( + ("install", "--no-policy"), cwd=project, env=environment, scenario_id=scenario_id + ) + + result = install("opencode-v2-install") + assert result.returncode == 0, result.stdout + result.stderr + rendered = json.loads(config_path.read_text(encoding="utf-8")) + mcp = rendered["mcp"] + assert rendered["theme"] == "user-theme" + assert mcp["timeout"] == USER_TIMEOUTS + assert set(mcp) == {"timeout", "servers"} + servers = mcp["servers"] + assert servers["personal"] == PERSONAL_SERVER + assert servers["local"]["type"] == "local" + assert servers["local"]["command"][0] == "echo" + assert servers["local"]["disabled"] is True + assert servers["remote"]["type"] == "remote" + assert servers["remote"]["url"] == "https://example.com/mcp" + assert servers["remote"]["codemode"] is False + assert "disabled" not in servers["remote"] + for entry in servers.values(): + assert "enabled" not in entry + + installed = config_path.read_bytes() + result = install("opencode-v2-reinstall") + assert result.returncode == 0, result.stdout + result.stderr + assert config_path.read_bytes() == installed + + manifest["dependencies"]["mcp"] = [local] + dump_yaml(manifest, project / "apm.yml") + result = install("opencode-v2-prune") + assert result.returncode == 0, result.stdout + result.stderr + servers = json.loads(config_path.read_text(encoding="utf-8"))["mcp"]["servers"] + assert set(servers) == {"personal", "local"} + + pruned = config_path.read_bytes() + local["enabled"] = "false" + dump_yaml(manifest, project / "apm.yml") + install("opencode-v2-non-boolean-enabled") + assert config_path.read_bytes() == pruned diff --git a/tests/unit/test_opencode_mcp_v2.py b/tests/unit/test_opencode_mcp_v2.py new file mode 100644 index 0000000000..a93b46efab --- /dev/null +++ b/tests/unit/test_opencode_mcp_v2.py @@ -0,0 +1,243 @@ +"""OpenCode v2 MCP configuration: format selection, rendering and cleanup.""" + +import json +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +import pytest + +from apm_cli.adapters.client.opencode import OpenCodeClientAdapter + +pytestmark = pytest.mark.component + +LOCAL_ENTRY = {"command": "npx", "args": ["-y", "pkg"], "env": {"KEY": "{env:KEY}"}} +REMOTE_ENTRY = {"url": "https://example.com/mcp", "headers": {"X-Api": "{env:API}"}} + + +class TestUsesNativeV2(unittest.TestCase): + def test_servers_map_selects_v2(self): + for config in ({"mcp": {"servers": {}}}, {"mcp": {"servers": {"a": {}}, "b": {}}}): + with self.subTest(config=config): + self.assertTrue(OpenCodeClientAdapter.uses_native_v2(config)) + + def test_v1_or_missing_layout_keeps_v1(self): + for config in ( + {}, + {"mcp": {}}, + {"mcp": {"local": {"type": "local", "command": ["x"]}}}, + {"mcp": {"servers": []}}, + {"mcp": []}, + ): + with self.subTest(config=config): + self.assertFalse(OpenCodeClientAdapter.uses_native_v2(config)) + + def test_v1_server_named_servers_is_not_a_v2_map(self): + config = {"mcp": {"servers": {"type": "remote", "url": "https://example.com"}}} + self.assertFalse(OpenCodeClientAdapter.uses_native_v2(config)) + + +class TestToOpencodeV2Format(unittest.TestCase): + def test_local_entry(self): + result = OpenCodeClientAdapter._to_opencode_v2_format(LOCAL_ENTRY) + self.assertEqual( + result, + {"type": "local", "command": ["npx", "-y", "pkg"], "environment": {"KEY": "{env:KEY}"}}, + ) + + def test_remote_entry(self): + result = OpenCodeClientAdapter._to_opencode_v2_format(REMOTE_ENTRY) + self.assertEqual( + result, + {"type": "remote", "url": "https://example.com/mcp", "headers": {"X-Api": "{env:API}"}}, + ) + + def test_enabled_maps_to_disabled(self): + disabled = OpenCodeClientAdapter._to_opencode_v2_format(LOCAL_ENTRY, enabled=False) + enabled = OpenCodeClientAdapter._to_opencode_v2_format(LOCAL_ENTRY, enabled=True) + self.assertIs(disabled["disabled"], True) + self.assertNotIn("disabled", enabled) + self.assertNotIn("enabled", disabled) + self.assertNotIn("enabled", enabled) + + def test_non_boolean_enabled_is_rejected(self): + for value in (None, "false", 0): + with self.subTest(value=value), self.assertRaises(ValueError): + OpenCodeClientAdapter._to_opencode_v2_format(LOCAL_ENTRY, enabled=value) + + def test_codemode_passthrough_is_preserved(self): + result = OpenCodeClientAdapter._to_opencode_v2_format({**REMOTE_ENTRY, "codemode": False}) + self.assertIs(result["codemode"], False) + + +class _ProjectFixture(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.root = Path(self.tmp.name) + (self.root / ".opencode").mkdir() + self.config_path = self.root / "opencode.json" + cwd = patch("os.getcwd", return_value=self.tmp.name) + cwd.start() + self.addCleanup(cwd.stop) + self.addCleanup(self.tmp.cleanup) + self.adapter = OpenCodeClientAdapter() + + def write_config(self, config: dict) -> None: + self.config_path.write_text(json.dumps(config), encoding="utf-8") + + def read_config(self) -> dict: + return json.loads(self.config_path.read_text(encoding="utf-8")) + + +class TestUpdateConfigV2(_ProjectFixture): + def test_writes_under_servers_and_preserves_unrelated_settings(self): + self.write_config( + { + "theme": "dark", + "mcp": { + "timeout": {"startup": 5000}, + "servers": {"personal": {"type": "local", "command": ["me"]}}, + }, + } + ) + + self.assertTrue(self.adapter.update_config({"pkg": LOCAL_ENTRY})) + + config = self.read_config() + self.assertEqual(config["theme"], "dark") + self.assertEqual(config["mcp"]["timeout"], {"startup": 5000}) + self.assertEqual(config["mcp"]["servers"]["personal"], {"type": "local", "command": ["me"]}) + self.assertEqual(config["mcp"]["servers"]["pkg"]["command"], ["npx", "-y", "pkg"]) + self.assertNotIn("pkg", config["mcp"]) + + def test_disabled_server(self): + self.write_config({"mcp": {"servers": {}}}) + + self.adapter.update_config({"pkg": REMOTE_ENTRY}, enabled=False) + + self.assertIs(self.read_config()["mcp"]["servers"]["pkg"]["disabled"], True) + + def test_non_boolean_enabled_leaves_file_unchanged(self): + original = {"mcp": {"servers": {}}} + self.write_config(original) + + self.assertFalse(self.adapter.update_config({"pkg": LOCAL_ENTRY}, enabled="yes")) + + self.assertEqual(self.read_config(), original) + + def test_moves_matching_legacy_entry(self): + legacy = OpenCodeClientAdapter._to_opencode_format(LOCAL_ENTRY) + self.write_config({"mcp": {"pkg": legacy, "servers": {}}}) + + self.adapter.update_config({"pkg": LOCAL_ENTRY}) + + config = self.read_config() + self.assertNotIn("pkg", {k for k in config["mcp"] if k != "servers"}) + self.assertIn("pkg", config["mcp"]["servers"]) + + def test_keeps_edited_legacy_entry(self): + edited = {**OpenCodeClientAdapter._to_opencode_format(LOCAL_ENTRY), "cwd": "tools"} + self.write_config({"mcp": {"pkg": edited, "servers": {}}}) + + self.adapter.update_config({"pkg": LOCAL_ENTRY}) + + config = self.read_config() + self.assertEqual(config["mcp"]["pkg"], edited) + self.assertIn("pkg", config["mcp"]["servers"]) + + def test_v1_file_keeps_v1_layout(self): + self.write_config({"mcp": {"personal": {"type": "local", "command": ["me"]}}}) + + self.adapter.update_config({"pkg": LOCAL_ENTRY}) + + config = self.read_config() + self.assertNotIn("servers", config["mcp"]) + self.assertIs(config["mcp"]["pkg"]["enabled"], True) + + +class TestReadAndRenderV2(_ProjectFixture): + def test_current_config_lists_effective_servers(self): + self.write_config( + { + "mcp": { + "timeout": {"startup": 5000}, + "legacy": {"type": "local", "command": ["old"]}, + "both": {"type": "local", "command": ["v1"]}, + "servers": {"both": {"type": "local", "command": ["v2"]}}, + } + } + ) + + servers = self.adapter.get_current_config()["mcp"] + + self.assertEqual(set(servers), {"legacy", "both"}) + self.assertEqual(servers["both"]["command"], ["v2"]) + + def test_render_matches_written_v2_entry(self): + self.write_config({"mcp": {"servers": {}}}) + server_info = { + "name": "pkg", + "_raw_stdio": {"command": "npx", "args": ["-y", "pkg"], "env": {}}, + } + + rendered = self.adapter.render_server_config(server_info) + + self.assertNotIn("enabled", rendered) + self.assertEqual(rendered["type"], "local") + + +class TestRemoveStaleV2(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.root = Path(self.tmp.name) + (self.root / ".opencode").mkdir() + self.config_path = self.root / "opencode.json" + + def remove_stale(self, names: set[str]) -> dict: + from apm_cli.integration.mcp_integrator import MCPIntegrator + + MCPIntegrator.remove_stale(names, runtime="opencode", project_root=self.root) + return json.loads(self.config_path.read_text(encoding="utf-8")) + + def test_removes_from_servers_and_legacy_layout(self): + self.config_path.write_text( + json.dumps( + { + "mcp": { + "timeout": {"startup": 5000}, + "stale": {"type": "local"}, + "servers": {"stale": {"type": "local"}, "keep": {"type": "local"}}, + } + } + ), + encoding="utf-8", + ) + + mcp = self.remove_stale({"stale"})["mcp"] + + self.assertEqual(mcp["timeout"], {"startup": 5000}) + self.assertNotIn("stale", mcp) + self.assertEqual(set(mcp["servers"]), {"keep"}) + + def test_stale_name_never_removes_the_servers_map(self): + self.config_path.write_text( + json.dumps({"mcp": {"servers": {"servers": {"type": "local"}}}}), encoding="utf-8" + ) + + mcp = self.remove_stale({"servers"})["mcp"] + + self.assertEqual(mcp, {"servers": {}}) + + def test_v1_server_named_servers_is_not_descended(self): + entry = {"type": "remote", "url": "https://example.com", "stale": "value"} + self.config_path.write_text(json.dumps({"mcp": {"servers": entry}}), encoding="utf-8") + + mcp = self.remove_stale({"stale"})["mcp"] + + self.assertEqual(mcp["servers"], entry) + + +if __name__ == "__main__": + unittest.main() From b74d54a69eed4929608a778969df2b6d9ca80256 Mon Sep 17 00:00:00 2001 From: RamsesJM <159502489+RamsesJM@users.noreply.github.com> Date: Wed, 28 Oct 2026 21:34:31 -0400 Subject: [PATCH 2/3] docs(changelog): reference #3414 for OpenCode 2 MCP support --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e84a43b5b..ed3b976599 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- OpenCode targets write native OpenCode 2 MCP entries under `mcp.servers` when `opencode.json` already declares that map, translating `enabled` to `disabled` and keeping `codemode` and other extra fields; files without it keep the OpenCode 1 layout. -- by @RamsesJM (#3112) +- OpenCode targets write native OpenCode 2 MCP entries under `mcp.servers` when `opencode.json` already declares that map, translating `enabled` to `disabled` and keeping `codemode` and other extra fields; files without it keep the OpenCode 1 layout. -- by @RamsesJM (#3414) ## [0.35.0] - 2026-10-22 From 6117faf2162b6f8c0406a48c7a12bb64a7e20a44 Mon Sep 17 00:00:00 2001 From: RamsesJM <159502489+RamsesJM@users.noreply.github.com> Date: Wed, 28 Oct 2026 21:47:28 -0400 Subject: [PATCH 3/3] fix(mcp): tighten OpenCode 2 ownership and migration rules - Keep mcp.timeout and other v2 settings out of top-level pruning. - Prune a v1 entry only when mcp.servers does not declare the same name, so an edited entry kept during migration survives removal. - Compare enabled by JSON type before retiring a v1 entry. - Report only mcp.servers entries in a v2 config, so adding an empty map migrates existing servers on the next install. - Derive disabled from enabled even when an extra field supplies it. --- .../docs/consumer/install-mcp-servers.md | 3 +- src/apm_cli/adapters/client/opencode.py | 29 +++++--- src/apm_cli/integration/mcp_integrator.py | 22 +++++- .../integration/test_opencode_v2_lifecycle.py | 45 +++++++++++ tests/unit/test_opencode_mcp_v2.py | 74 +++++++++++++++++-- 5 files changed, 148 insertions(+), 25 deletions(-) diff --git a/docs/src/content/docs/consumer/install-mcp-servers.md b/docs/src/content/docs/consumer/install-mcp-servers.md index fb64b8ce3b..29ecc5fded 100644 --- a/docs/src/content/docs/consumer/install-mcp-servers.md +++ b/docs/src/content/docs/consumer/install-mcp-servers.md @@ -223,7 +223,8 @@ To opt in, add an empty map to the target file and reinstall: In the OpenCode 2 layout, `enabled: false` is written as `disabled: true`, and `enabled: true` or an omitted value writes no field, the OpenCode 2 default. Other values have no OpenCode 2 equivalent, so APM warns and -leaves the file unchanged. To expose a server's tools directly instead of +leaves the file unchanged. A `disabled` value in the `extra` block is +ignored, so `enabled` alone decides. To expose a server's tools directly instead of through Code Mode, set `codemode: false` in the dependency's `extra` block; APM writes extra fields verbatim, so use the OpenCode 2 spelling for any other native setting, such as snake_case `oauth` fields. diff --git a/src/apm_cli/adapters/client/opencode.py b/src/apm_cli/adapters/client/opencode.py index 408f5df401..03298f8f26 100644 --- a/src/apm_cli/adapters/client/opencode.py +++ b/src/apm_cli/adapters/client/opencode.py @@ -39,7 +39,11 @@ from pathlib import Path from typing import Any -from ...models.dependency.mcp import _EXTRA_DENYLIST, opencode_enabled_value +from ...models.dependency.mcp import ( + _EXTRA_DENYLIST, + opencode_enabled_matches, + opencode_enabled_value, +) from ...utils.console import _rich_warning from .copilot import CopilotClientAdapter @@ -48,7 +52,7 @@ _V2_SERVERS_KEY = "servers" # Settings that share the ``mcp`` object with v1 server entries in v2 files. -_V2_MCP_SETTINGS = frozenset({_V2_SERVERS_KEY, "timeout"}) +V2_MCP_SETTINGS = frozenset({_V2_SERVERS_KEY, "timeout"}) class OpenCodeClientAdapter(CopilotClientAdapter): @@ -172,9 +176,10 @@ def _update_v2_config( return False for name, entry in rendered.items(): - legacy = mcp.get(name) if name not in _V2_MCP_SETTINGS else None + legacy = mcp.get(name) if name not in V2_MCP_SETTINGS else None if legacy is not None: - if legacy == self._to_opencode_format(config_updates[name], enabled=enabled): + written = self._to_opencode_format(config_updates[name], enabled=enabled) + if legacy == written and opencode_enabled_matches(legacy, written): del mcp[name] else: _rich_warning( @@ -213,17 +218,14 @@ def validate_config_for_install(self) -> bool: def get_current_config(self): """Read the current ``opencode.json`` contents. - For a v2 config, ``mcp`` lists the effective servers: v1-layout - entries overlaid by ``mcp.servers``, matching OpenCode's own - precedence when both declare a name. + For a v2 config, ``mcp`` lists only the ``mcp.servers`` entries. + A server still in the v1 layout then counts as not installed, so a + reinstall writes its native entry and retires the v1 one. """ config = self._read_config_file() if not self.uses_native_v2(config): return config - mcp = config["mcp"] - effective = {name: entry for name, entry in mcp.items() if name not in _V2_MCP_SETTINGS} - effective.update(mcp[_V2_SERVERS_KEY]) - return {**config, "mcp": effective} + return {**config, "mcp": dict(config["mcp"][_V2_SERVERS_KEY])} def _read_config_file(self) -> dict: """Return the parsed config file, or ``{}`` when it is absent or unreadable.""" @@ -326,7 +328,9 @@ def _to_opencode_v2_format(cls, copilot_entry: dict, enabled: Any = True) -> dic Identical to the v1 entry except that ``enabled`` becomes ``disabled: true`` when false and is omitted when true, the v2 - default. Passthrough fields such as ``codemode`` are kept as is. + default. ``disabled`` always follows ``enabled``, even when an + extra field supplies it; other passthrough fields such as + ``codemode`` are kept as is. Raises: ValueError: *enabled* is not a boolean, so it has no v2 equivalent. @@ -337,6 +341,7 @@ def _to_opencode_v2_format(cls, copilot_entry: dict, enabled: Any = True) -> dic ) entry = cls._to_opencode_format(copilot_entry, enabled=enabled) del entry["enabled"] + entry.pop("disabled", None) if not enabled: entry["disabled"] = True return entry diff --git a/src/apm_cli/integration/mcp_integrator.py b/src/apm_cli/integration/mcp_integrator.py index cdbdf6d937..3735632aa3 100644 --- a/src/apm_cli/integration/mcp_integrator.py +++ b/src/apm_cli/integration/mcp_integrator.py @@ -132,6 +132,7 @@ def _clean_json_mcp_config( use_rich: bool = False, fail_on_write_error: bool = False, nested_servers: Callable[[dict], dict | None] | None = None, + nested_settings: frozenset[str] = frozenset(), ) -> int: """Remove stale entries from a JSON-based MCP config file. @@ -147,6 +148,10 @@ def _clean_json_mcp_config( nested_servers: Returns a second server map inside the parsed config, or ``None``. Stale names are removed from it as well, for layouts such as OpenCode v2 that keep servers below the top-level key. + A top-level entry is then removed only when the nested map does + not declare the same name, since that entry is not APM's. + nested_settings: Top-level keys that hold settings rather than + servers whenever ``nested_servers`` returns a map. Returns: Number of entries removed. @@ -169,9 +174,14 @@ def _clean_json_mcp_config( if not isinstance(servers, dict): raise ValueError(f"{label} {servers_key} must be a mapping") nested = nested_servers(config) if nested_servers else None - removed = [ - n for n in stale_names if n in servers and (nested is None or servers[n] is not nested) - ] + if nested is None: + removed = [n for n in stale_names if n in servers] + else: + removed = [ + n + for n in stale_names + if n in servers and n not in nested and n not in nested_settings + ] for name in removed: del servers[name] if nested: @@ -874,7 +884,10 @@ def remove_stale( # Project scope is opt-in; user scope uses ~/.config/opencode/. if "opencode" in target_runtimes: if clean_user_scope or (project_root_path / ".opencode").is_dir(): - from apm_cli.adapters.client.opencode import OpenCodeClientAdapter + from apm_cli.adapters.client.opencode import ( + V2_MCP_SETTINGS, + OpenCodeClientAdapter, + ) _clean_json_mcp_config( OpenCodeClientAdapter.config_path_for(project_root_path, clean_user_scope), @@ -884,6 +897,7 @@ def remove_stale( servers_key="mcp", fail_on_write_error=fail_on_write_error, nested_servers=OpenCodeClientAdapter.native_v2_servers, + nested_settings=V2_MCP_SETTINGS, ) if "windsurf" in target_runtimes: diff --git a/tests/integration/test_opencode_v2_lifecycle.py b/tests/integration/test_opencode_v2_lifecycle.py index 82fd4c270c..5d0dde4f64 100644 --- a/tests/integration/test_opencode_v2_lifecycle.py +++ b/tests/integration/test_opencode_v2_lifecycle.py @@ -107,3 +107,48 @@ def install(scenario_id: str): dump_yaml(manifest, project / "apm.yml") install("opencode-v2-non-boolean-enabled") assert config_path.read_bytes() == pruned + + +def test_opencode_v1_install_moves_to_v2_after_opt_in( + tmp_path: Path, apm_binary_path: Path +) -> None: + """Adding an empty mcp.servers map makes the next install write native entries.""" + isolated = IsolatedApmEnvironment.create(tmp_path / "scenario", base_env=dict(os.environ)) + project = isolated.work_root / "project" + project.mkdir() + (project / ".opencode").mkdir() + config_path = project / "opencode.json" + config_path.write_text(json.dumps({"mcp": {"personal": PERSONAL_SERVER}}), encoding="utf-8") + manifest = { + "name": "opencode-v2-migration", + "version": "1.0.0", + "targets": ["opencode"], + "dependencies": { + "apm": [], + "mcp": [{"name": "local", "registry": False, "transport": "stdio", "command": "echo"}], + }, + } + dump_yaml(manifest, project / "apm.yml") + LockFile().write(project / "apm.lock.yaml") + runner = ApmLifecycleRunner((str(apm_binary_path),)) + environment = isolated.subprocess_env() + + result = runner.run( + ("install", "--no-policy"), cwd=project, env=environment, scenario_id="opencode-v1-install" + ) + assert result.returncode == 0, result.stdout + result.stderr + v1_config = json.loads(config_path.read_text(encoding="utf-8")) + assert v1_config["mcp"]["local"]["enabled"] is True + assert "servers" not in v1_config["mcp"] + + v1_config["mcp"]["servers"] = {} + config_path.write_text(json.dumps(v1_config), encoding="utf-8") + result = runner.run( + ("install", "--no-policy"), cwd=project, env=environment, scenario_id="opencode-v2-opt-in" + ) + assert result.returncode == 0, result.stdout + result.stderr + mcp = json.loads(config_path.read_text(encoding="utf-8"))["mcp"] + assert set(mcp) == {"personal", "servers"} + assert mcp["personal"] == PERSONAL_SERVER + assert set(mcp["servers"]) == {"local"} + assert "enabled" not in mcp["servers"]["local"] diff --git a/tests/unit/test_opencode_mcp_v2.py b/tests/unit/test_opencode_mcp_v2.py index a93b46efab..10fa7ca0b7 100644 --- a/tests/unit/test_opencode_mcp_v2.py +++ b/tests/unit/test_opencode_mcp_v2.py @@ -70,6 +70,15 @@ def test_codemode_passthrough_is_preserved(self): result = OpenCodeClientAdapter._to_opencode_v2_format({**REMOTE_ENTRY, "codemode": False}) self.assertIs(result["codemode"], False) + def test_modeled_enabled_overrides_disabled_passthrough(self): + for passthrough in (True, False, "yes"): + with self.subTest(passthrough=passthrough): + entry = {**LOCAL_ENTRY, "disabled": passthrough} + enabled = OpenCodeClientAdapter._to_opencode_v2_format(entry, enabled=True) + disabled = OpenCodeClientAdapter._to_opencode_v2_format(entry, enabled=False) + self.assertNotIn("disabled", enabled) + self.assertIs(disabled["disabled"], True) + class _ProjectFixture(unittest.TestCase): def setUp(self): @@ -146,6 +155,15 @@ def test_keeps_edited_legacy_entry(self): self.assertEqual(config["mcp"]["pkg"], edited) self.assertIn("pkg", config["mcp"]["servers"]) + def test_keeps_legacy_entry_whose_enabled_type_changed(self): + edited = {**OpenCodeClientAdapter._to_opencode_format(LOCAL_ENTRY), "enabled": 1} + self.write_config({"mcp": {"pkg": edited, "servers": {}}}) + + self.adapter.update_config({"pkg": LOCAL_ENTRY}) + + legacy = self.read_config()["mcp"]["pkg"] + self.assertIs(type(legacy["enabled"]), int) + def test_v1_file_keeps_v1_layout(self): self.write_config({"mcp": {"personal": {"type": "local", "command": ["me"]}}}) @@ -157,22 +175,32 @@ def test_v1_file_keeps_v1_layout(self): class TestReadAndRenderV2(_ProjectFixture): - def test_current_config_lists_effective_servers(self): + def test_current_config_lists_only_native_servers(self): self.write_config( { "mcp": { "timeout": {"startup": 5000}, "legacy": {"type": "local", "command": ["old"]}, - "both": {"type": "local", "command": ["v1"]}, - "servers": {"both": {"type": "local", "command": ["v2"]}}, + "servers": {"native": {"type": "local", "command": ["new"]}}, } } ) servers = self.adapter.get_current_config()["mcp"] - self.assertEqual(set(servers), {"legacy", "both"}) - self.assertEqual(servers["both"]["command"], ["v2"]) + self.assertEqual(servers, {"native": {"type": "local", "command": ["new"]}}) + + def test_legacy_only_server_is_reported_as_needing_installation(self): + from apm_cli.integration.mcp_integrator import MCPIntegrator + + legacy = OpenCodeClientAdapter._to_opencode_format(LOCAL_ENTRY) + self.write_config({"mcp": {"pkg": legacy, "servers": {}}}) + + pending = MCPIntegrator._check_self_defined_servers_needing_installation( + ["pkg"], ["opencode"], project_root=self.root + ) + + self.assertEqual(pending, ["pkg"]) def test_render_matches_written_v2_entry(self): self.write_config({"mcp": {"servers": {}}}) @@ -201,13 +229,12 @@ def remove_stale(self, names: set[str]) -> dict: MCPIntegrator.remove_stale(names, runtime="opencode", project_root=self.root) return json.loads(self.config_path.read_text(encoding="utf-8")) - def test_removes_from_servers_and_legacy_layout(self): + def test_removes_native_entry_and_preserves_settings(self): self.config_path.write_text( json.dumps( { "mcp": { "timeout": {"startup": 5000}, - "stale": {"type": "local"}, "servers": {"stale": {"type": "local"}, "keep": {"type": "local"}}, } } @@ -218,9 +245,40 @@ def test_removes_from_servers_and_legacy_layout(self): mcp = self.remove_stale({"stale"})["mcp"] self.assertEqual(mcp["timeout"], {"startup": 5000}) - self.assertNotIn("stale", mcp) self.assertEqual(set(mcp["servers"]), {"keep"}) + def test_stale_name_never_removes_v2_settings(self): + self.config_path.write_text( + json.dumps( + {"mcp": {"timeout": {"startup": 5000}, "servers": {"timeout": {"type": "local"}}}} + ), + encoding="utf-8", + ) + + mcp = self.remove_stale({"timeout"})["mcp"] + + self.assertEqual(mcp, {"timeout": {"startup": 5000}, "servers": {}}) + + def test_kept_legacy_entry_survives_pruning_of_native_entry(self): + edited = {"type": "local", "command": ["edited"]} + self.config_path.write_text( + json.dumps({"mcp": {"pkg": edited, "servers": {"pkg": {"type": "local"}}}}), + encoding="utf-8", + ) + + mcp = self.remove_stale({"pkg"})["mcp"] + + self.assertEqual(mcp, {"pkg": edited, "servers": {}}) + + def test_legacy_only_entry_is_pruned_in_v2_file(self): + self.config_path.write_text( + json.dumps({"mcp": {"pkg": {"type": "local"}, "servers": {}}}), encoding="utf-8" + ) + + mcp = self.remove_stale({"pkg"})["mcp"] + + self.assertEqual(mcp, {"servers": {}}) + def test_stale_name_never_removes_the_servers_map(self): self.config_path.write_text( json.dumps({"mcp": {"servers": {"servers": {"type": "local"}}}}), encoding="utf-8"