Repository navigation
feat(mcp): write native OpenCode 2 entries under mcp.servers - #3414
RamsesJ.Dev (RamsesJM) wants to merge 3 commits into
Conversation
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 microsoft#3112
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
🟡 Changes recommended
Pruning managed v2 servers can delete user-owned MCP settings and edited legacy entries, risking loss of configuration that should be preserved.
5 open issues
What changed in this PR
Adds native OpenCode 2 MCP configuration support to the existing opencode target for #3112, while retaining the v1 layout unless the configuration explicitly opts in.
Changes:
- Selects native v2 output from an existing
mcp.serversmap and translates booleanenabledvalues. - Extends configuration reads and stale-server cleanup to handle both layouts.
- Adds component and CLI lifecycle coverage, documentation, and a changelog entry.
| File | Description |
|---|---|
tests/unit/test_opencode_mcp_v2.py |
Tests layout selection, rendering, legacy handling, and cleanup. |
tests/integration/test_opencode_v2_lifecycle.py |
Covers CLI install, reinstall, pruning, and non-boolean rejection. |
src/apm_cli/integration/mcp_integrator.py |
Adds nested server-map cleanup for OpenCode. |
src/apm_cli/adapters/client/opencode.py |
Implements v2 selection, rendering, writes, and effective configuration reads. |
docs/src/content/docs/reference/manifest-schema.md |
Documents v2 handling of enabled. |
docs/src/content/docs/consumer/install-mcp-servers.md |
Explains opt-in, field mapping, and migration behavior. |
CHANGELOG.md |
Records native OpenCode 2 MCP support. |
Review effort: Balanced
| removed = [ | ||
| n for n in stale_names if n in servers and (nested is None or servers[n] is not nested) | ||
| ] |
There was a problem hiding this comment.
Fixed in 6117faf. In a v2 config, top-level pruning now skips mcp.servers and mcp.timeout (nested_settings), while a server with either name is still removed from mcp.servers. Covered by test_stale_name_never_removes_v2_settings.
| n for n in stale_names if n in servers and (nested is None or servers[n] is not nested) | ||
| ] |
There was a problem hiding this comment.
Fixed in 6117faf. A v1 entry is pruned only when mcp.servers does not declare the same name. After migration, a remaining same-named v1 entry is by construction one APM kept because it was edited, so it is left alone. Legacy-only entries are still pruned. Covered by test_kept_legacy_entry_survives_pruning_of_native_entry and test_legacy_only_entry_is_pruned_in_v2_file.
| 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): |
There was a problem hiding this comment.
Fixed in 6117faf as suggested: the v1 entry is retired only when it equals the APM rendering and opencode_enabled_matches() holds. Covered by test_keeps_legacy_entry_whose_enabled_type_changed.
| 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} |
There was a problem hiding this comment.
Fixed in 6117faf. In a v2 config, get_current_config() now reports only mcp.servers entries, so a server still in the v1 layout counts as not installed and the next install writes its native entry and retires the matching v1 one. Covered by test_legacy_only_server_is_reported_as_needing_installation and the CLI scenario test_opencode_v1_install_moves_to_v2_after_opt_in.
| entry = cls._to_opencode_format(copilot_entry, enabled=enabled) | ||
| del entry["enabled"] | ||
| if not enabled: | ||
| entry["disabled"] = True |
There was a problem hiding this comment.
Fixed in 6117faf, but locally rather than in _HARNESS_EXTRA_ALIASES: Kiro forwards disabled as a legitimate extension field (KiroClientAdapter._copy_kiro_extensions), so reserving it globally would change another target, which #3112 excludes. The OpenCode v2 conversion now drops any passthrough disabled and derives it solely from enabled. Covered by test_modeled_enabled_overrides_disabled_passthrough.
- 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.
|
@microsoft-github-policy-service agree |
Description
Adds OpenCode 2 MCP support to the existing
opencodetarget. When the targetopencode.jsonalready declares anmcp.serversmap, APM writes native OpenCode 2 entries there; otherwise it keeps the current OpenCode 1 layout. No new target, flag or manifest field is introduced.Why the format is selected from the file
opencode.jsonlocations, and both ship asopencode, so there is no version signal to detect (Migrate from V1).mcp.servers. Emitting v2 by default would break existing v1 users.mcp.serversmap is therefore an explicit, user-owned opt-in that never converts an installation silently, as the scope requires.Behavior in the v2 layout
mcp.servers.<name>enabledomitted ortrueenabled: falsedisabled: trueenabledextra: {codemode: false}codemode: false(existing passthrough)mcp.serversentry.mcp.timeout, user-owned servers and unrelated settings are preserved.mcpandmcp.servers, and never removes theserversmap itself.get_current_config()exposes the effective server set (v1 entries overlaid bymcp.servers), so conflict detection and installed-server checks no longer seeserversortimeoutas server names.Implementation
src/apm_cli/adapters/client/opencode.pyuses_native_v2()selection rule;_to_opencode_v2_format(); v2 write path inupdate_config(); v2-awareget_current_config()andrender_server_config()src/apm_cli/integration/mcp_integrator.py_clean_json_mcp_config()accepts an optionalnested_serversaccessor; OpenCode passesOpenCodeClientAdapter.native_v2_serversdocs/.../consumer/install-mcp-servers.mddocs/.../reference/manifest-schema.mdenabledrow notes the v2 mappingtests/unit/test_opencode_mcp_v2.pytests/integration/test_opencode_v2_lifecycle.pyenabledTrade-offs
"mcp": {"servers": {}}; this is documented.enabledin v2. v1 passes any JSON value through unchanged, but v2 has noenabledfield and inverting a non-boolean would be a guess. v1 behavior is unchanged.mcp.servers.extraare not translated (for example camelCase v1oauthkeys); the docs tell authors to use v2 spelling. Translating them would extend passthrough semantics, which is out of scope.Issue and approved scope
Issue: #3112
Human scope-approval comment: #3112 (comment)
This PR completes the recorded scope: native v2 stdio and HTTP entries with
disabledandcodemode, v1 preserved, and a documented non-destructive selection. It adds noopencode2target, no new trust semantics, and no changes to other targets.Fixes #3112
Type of change
Testing
Full CI on the fork (Lint, both Build & Test shards, both Lifecycle Smoke shards, Spec conformance, Windows Compatibility, Architecture ratchets): RamsesJM#1.
How to test
.opencode/, createopencode.jsoncontaining{"mcp": {"servers": {}}}.enabled: falseand an HTTP server withextra: {codemode: false}inapm.yml, targetingopencode.apm install: both entries appear undermcp.servers, the first withdisabled: true, the second withcodemode: false, and neither withenabled.apm installagain: the file is byte-identical.apm.ymland reinstall: only that entry is removed. Withoutmcp.servers, output is the unchanged v1 layout.Spec conformance (OpenAPM v0.1)