Skip to content

Follow-ups from PR #3723: OpenCode MCP orphan filter comment, bare-prefix key edge case, docs note scope #3725

Closed

Description

@dyoshikawa

Background

PR #3723 (refactor(mcp): OpenCode MCP tool filter follow-ups from #3698) added comments, docs and tests around how OpenCode tools / permission keys are mapped back to rulesync MCP tool filters. The review found no blocking issues. Three low-severity follow-ups came out of it; two of them concern the second ("orphan") filtering pass on import.

Details

1. The comment calls the second filter pass "redundant", but it is not

src/features/mcp/opencode-mcp.ts (around lines 222-225) says the second filter in orphanMcpToolFiltersToRulesync is redundant for this caller. However, findOpencodeMcpToolKeyOwner requires key.length > prefix.length, so a key that is exactly <sanitized server>_ has no owner and stays in the orphan set.

  • Example: server a_b, key a_b_. The first pass does not claim it. Only the raw startsWith("a_b_") check in orphanMcpToolFiltersToRulesync (src/features/mcp/mcp-transport.ts, around line 132) drops it.
  • Without that check, the key is split on the first _ and imported as a phantom server a with tool b_.

Someone who trusts the comment and removes the OpenCode-side second pass would introduce exactly that regression.

2. Untested edge case: a key equal to a sanitized prefix that differs from the raw name

This existed before #3723. Server a.b sanitizes to prefix a_b_, and a key exactly a_b_:

  • has no owner in the first pass (same length);
  • is not caught by the raw check (a.b_).

It is then imported as an orphan server a with tool b_.

3. The docs note names only "OpenCode"

The new note in docs/reference/file-formats.md (around line 1614) says "OpenCode", but the same adapter also serves OpenCode V1 and MiMo Code. Other notes in that file spell out MiMo Code explicitly.

Solution / Next Steps

  • Reword the comment in opencode-mcp.ts. The second pass is mostly redundant, but it still drops keys that are exactly <server>_. Add a test for the a_b / a_b_ case so the pass cannot be removed silently.
  • Decide how a bare <sanitized server>_ key should be handled (likely: drop it as owned by that server, with no tool). Add a test for the a.b / a_b_ case.
  • Extend the docs note to "OpenCode (and OpenCode V1 / MiMo Code)" and regenerate src/generated/docs-content.ts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentationmaintainer-scrapRough notes for AI implementation. Not for human eyes.refactoring

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions