fix(opencode): honor MCP enabled false in opencode.json - #3052
Buyan Khurelbaatar (buyan-kh) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
There are a couple of small but concrete maintainability/convention issues in the newly added code that should be cleaned up before approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
This PR fixes OpenCode MCP config generation so an explicit enabled: false in apm.yml is preserved and written through to opencode.json (instead of being dropped/forced to true). It does so by modeling enabled on MCPDependency, carrying it through the MCP overlay/cache layer, and teaching the OpenCode adapter to apply it.
Changes:
- Model
enabled: bool | NoneonMCPDependencyand validate/round-trip it viafrom_dict()/to_dict(). - Propagate per-dependency enabled state into the MCP server info cache (
_apm_enabled) and have the OpenCode adapter honor it when writingopencode.json. - Add unit + integration tests, plus manifest-schema docs and a changelog entry for the behavior change.
| File | Description |
|---|---|
src/apm_cli/models/dependency/mcp.py |
Adds modeled enabled field, parsing/validation, and serialization. |
src/apm_cli/integration/mcp_integrator.py |
Persists per-dependency enabled into server info cache as _apm_enabled. |
src/apm_cli/adapters/client/opencode.py |
Applies _apm_enabled when rendering OpenCode config and adds _coerce_enabled helper. |
tests/unit/test_mcp_from_dict_unknown_keys.py |
Verifies enabled is modeled (not treated as passthrough extra) and rejects non-bools. |
tests/unit/test_mcp_overlays.py |
Ensures overlays embed/omit _apm_enabled correctly. |
tests/unit/test_opencode_mcp.py |
Adds coverage for per-entry enabled precedence and update_config behavior. |
tests/integration/test_mcp_install_flow.py |
End-to-end install test ensuring enabled is written correctly to opencode.json. |
docs/src/content/docs/reference/manifest-schema.md |
Documents dependencies.mcp[].enabled semantics and passthrough-reserved behavior. |
CHANGELOG.md |
Adds a Fixed entry describing the OpenCode enabled behavior correction. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @staticmethod | ||
| def _coerce_enabled(value, default: bool) -> bool: | ||
| """Return *value* when it is a boolean, otherwise *default*.""" | ||
| if isinstance(value, bool): | ||
| return value | ||
| return default |
| # Harness aliases for modeled fields share the same passthrough boundary. Keeping | ||
| # them beside the manifest vocabulary lets parsing report rejected keys truthfully | ||
| # before every adapter consumes the filtered ``extra`` mapping. | ||
| # ``enabled`` is also the modeled optional boolean below. It stays in this set so | ||
| # an explicit ``extra:`` block cannot override that field. | ||
| _HARNESS_EXTRA_ALIASES = frozenset({"enabled", "environment", "http_headers", "id"}) | ||
| _EXTRA_DENYLIST = _RESERVED_EXTRA_KEYS | _HARNESS_EXTRA_ALIASES |
enabled: false in apm.yml was stripped as a passthrough alias and the OpenCode adapter always wrote true. Model the flag and write it through. Signed-off-by: Buyan Khurelbaatar <buyan-kh@users.noreply.github.com>
9c16e4e to
5d1475a
Compare
|
Buyan Khurelbaatar (@buyan-kh) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|

enabled: false in apm.yml was stripped as a passthrough alias and the OpenCode adapter always wrote true. Model the flag and write it through.
Description
Brief description of changes and motivation.
Issue and approved scope
Issue: # (required; use the canonical issue or a bounded child issue)
Human scope-approval comment: (link, state the trivial-documentation
standing-preapproval case, or indicate private security coordination without
disclosing report details)
Does this PR complete the issue, or what remains?
Type of change
Testing
Spec conformance (OpenAPM v0.1)
If this PR changes behaviour that an OpenAPM v0.1
req-XXXcovers,confirm the three-step ritual in the
development guide:
docs/src/content/docs/specs/openapm-v0.1.mdupdated(new/changed
<a id="req-XXX"></a>anchor + prose + Appendix Crow).
docs/src/content/docs/specs/manifests/openapm-v0.1.requirements.ymlupdated.
@pytest.mark.req("req-XXX")test undertests/spec_conformance/added or extended.CONFORMANCE.{md,json}regenerated viauv run --extra dev python -m tests.spec_conformance.gen_statementand committed.