fix(cli): accept an MCP entry the client annotated, instead of refusing it - #1640
Merged
Conversation
…ng it OpenCode writes "enabled": true beside the "command" and "type" we write. Our ownership check required the entry's key set to match EXACTLY (config_json_like.c: member_count != found_count), so a three-key entry with two recognised keys was classified as FOREIGN - and we refused to touch an entry we had written ourselves. install then failed with: error: agent_config agent=OpenCode op=mcp_install path=.../opencode.json Confirmed on two independent configs: Linux (#1630) and Windows (#1582). In gotspatel's file EVERY MCP server carries the key - mssql, forgetful, chrome-devtools and ours - so this is OpenCode's normal shape, not an unusual hand-edit. Anyone who has ever toggled a server on or off in the UI was hit. Two of my own hypotheses were wrong before the reporters' files settled it: it is not JSONC comment parsing, and it is not the .jsonc targeting that #1575 fixed. #1575 fixed WHICH file we open; this happens after, on what we find inside. The distinction now reported is MATCH_WITH_EXTRAS, and the caller treats it as ALREADY SATISFIED - success, without touching the file. That is deliberate and it is the safe half of the fix: cbm_json_like_upsert_entry REPLACES an entry wholesale, so writing our canonical shape over an annotated entry would silently delete the client's keys. A refusal the user can see beats a deletion they cannot. Doing nothing is also correct on the merits: the entry already names this binary with the right type, which is the entire content of the install. Merging our fields into an annotated entry while preserving the rest is the fuller fix and stays tracked in #1630. This makes the common case work without risking anyone's configuration tonight. Ownership is NOT loosened otherwise: an entry whose command points at a different binary is still foreign and still refused, byte-identically, and that direction is pinned by its own test. Tests use the reporters' actual entry shape. cli suite: 272 passed. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This was referenced Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(cli): accept an MCP entry the client annotated, instead of refusing it
OpenCode writes "enabled": true beside the "command" and "type" we write. Our
ownership check required the entry's key set to match EXACTLY
(config_json_like.c: member_count != found_count), so a three-key entry with two
recognised keys was classified as FOREIGN - and we refused to touch an entry we
had written ourselves. install then failed with:
error: agent_config agent=OpenCode op=mcp_install path=.../opencode.json
Confirmed on two independent configs: Linux (#1630) and Windows (#1582). In
gotspatel's file EVERY MCP server carries the key - mssql, forgetful,
chrome-devtools and ours - so this is OpenCode's normal shape, not an unusual
hand-edit. Anyone who has ever toggled a server on or off in the UI was hit.
Two of my own hypotheses were wrong before the reporters' files settled it: it is
not JSONC comment parsing, and it is not the .jsonc targeting that #1575 fixed.
#1575 fixed WHICH file we open; this happens after, on what we find inside.
The distinction now reported is MATCH_WITH_EXTRAS, and the caller treats it as
ALREADY SATISFIED - success, without touching the file. That is deliberate and it
is the safe half of the fix: cbm_json_like_upsert_entry REPLACES an entry
wholesale, so writing our canonical shape over an annotated entry would silently
delete the client's keys. A refusal the user can see beats a deletion they
cannot. Doing nothing is also correct on the merits: the entry already names this
binary with the right type, which is the entire content of the install.
Merging our fields into an annotated entry while preserving the rest is the
fuller fix and stays tracked in #1630. This makes the common case work without
risking anyone's configuration tonight.
Ownership is NOT loosened otherwise: an entry whose command points at a different
binary is still foreign and still refused, byte-identically, and that direction
is pinned by its own test.
Tests use the reporters' actual entry shape. cli suite: 272 passed.