refactor: shorten plugin MCP tool qualified names#1549
Conversation
Drop the redundant literal 'plugin-' prefix from plugin MCP server runtime names, and omit the server-name segment entirely when a plugin declares only one MCP server (the common case). This shortens every plugin-provided MCP tool's qualified name, e.g.: mcp__plugin-ifind-mcp_s__get_financial_statements -> mcp__ifind-mcp__get_financial_statements Breaking change: previously saved per-tool permission approvals for plugin MCP tools will need to be re-granted after upgrading, since the wire-level tool name changes.
🦋 Changeset detectedLatest commit: 1816eb4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d509abe11e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // single-server case). Only append ":serverName" when a plugin declares | ||
| // more than one MCP server — that keeps cross-plugin pairs unambiguous | ||
| // even when either half contains "-" (e.g. "a-b"+"c" vs "a"+"b-c"). | ||
| return serverCount > 1 ? `${pluginId}:${serverName}` : pluginId; |
There was a problem hiding this comment.
Preserve a namespace for plugin MCP servers
When a single-server plugin id matches an existing user/caller MCP server name (for example both are github), this now returns the bare plugin id, and mergePluginMcpConfig() spreads plugin servers after base?.servers, so the plugin silently replaces the user's configured server for that session. The previous plugin-<id>:<server> key avoided collisions with non-plugin MCP servers; this needs either a reserved plugin namespace or explicit collision handling before omitting the server segment.
Useful? React with 👍 / 👎.
commit: |
|
The direction of this PR makes sense, but there are two points to confirm:
Plugin version compatibility should also be considered. Plugins such as Additional research:
|
Related Issue
No related issue. Problem explained below.
Problem
Plugin-provided MCP tool names carry a long, mostly redundant prefix before the actual tool name, e.g.
mcp__plugin-ifind-mcp_s__get_financial_statements. The literalplugin-tag and the server-name segment (often a meaningless single letter like_ssince most plugins declare only one MCP server) are pure overhead. Combined with a long plugin id, this could also push the qualified name past the 64-character cap, triggering a hash-truncated, unreadable name (e.g.mcp__plugin-world_bank_open_data-mcp_s__world_bank_sear_2a768f76).What changed
pluginMcpRuntimeName()inpackages/agent-core/src/plugin/manager.tsnow drops the literalplugin-prefix, and omits the server-name segment entirely when a plugin declares only one MCP server (the common case). It only keepspluginId:serverNamewhen a plugin declares 2+ MCP servers, which is still unambiguous since plugin ids are unique and cannot contain:.packages/agent-core/test/plugin/manager.test.tsandtest/rpc/plugins-rpc.test.tsto match the new runtime-name format, and reworked the disambiguation test to use multi-server fixtures so it still exercises the:boundary.kimi-datasourceplugin'sSKILL.md(which hardcodes the qualified tool names it tells the model to call) and bumped its own version/changelog.This is a breaking change: the wire-level tool name for every installed plugin's MCP tools changes, so any previously saved per-tool permission approval for a plugin tool needs to be re-granted after upgrading. A changeset (
minor) is included describing this.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.