fix(agent-core): disambiguate MCP tools with colliding names across s…#665
fix(agent-core): disambiguate MCP tools with colliding names across s…#665LifeJiggy wants to merge 6 commits into
Conversation
…ervers Instead of dropping the second server's tool when two MCP servers expose tools with the same name, append a numeric suffix (__2, __3) so both tools remain accessible. This fixes MoonshotAI#575. Enhancements: - Track collision count across registrations for incrementing suffixes - Updated collision error message to mention disambiguation - Both original and disambiguated tools are callable independently Tests: - Cross-server collision disambiguation with suffix assignment - Multiple collisions get incrementing suffixes - Both tools callable independently via loopTools - Collision results contain disambiguation details
🦋 Changeset detectedLatest commit: c159bf2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
…angeset - Use executeTool helper instead of direct resolveExecution().execute() to handle Promise<ToolExecution> return type - Add changeset for agent-core and kimi-code patch bumps
…QUALIFIED_LENGTH When a disambiguated name (base + __N suffix) would exceed the 64-char limit enforced by LLM providers, truncate the base name to fit. This prevents API errors from providers that reject tool names over 64 chars. Also added test verifying the length constraint is respected.
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e1c2379f5
ℹ️ 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".
…sambiguation When truncating a disambiguated name to fit MAX_QUALIFIED_LENGTH, the resulting name might already be registered by another tool. Now probes by incrementing the suffix count until a unique name is found. Fixes P2 from Codex review: two long colliding names could truncate to the same prefix+suffix, causing the latter to silently overwrite the former.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfc73e2a22
ℹ️ 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".
When a server re-registers after a reconnect, reset the collision count for its base tool names so the disambiguated suffix numbers stay stable. This prevents saved approvals, pending tool references, and model context from becoming stale after normal reconnect flows. Also fixes typecheck error by rewriting the probing test to use only public API (no direct access to protected mcpTools map).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71bbcbbf51
ℹ️ 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".
…ambiguation When a server has same-server tool name duplicates AND the base name collides with another server, the duplicate check must use the base name (before suffix), not the disambiguated name. Otherwise the second same-server duplicate slips through with a different suffix instead of being reported as a same-server collision.
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Instead of dropping the second server's tool when two MCP servers expose tools with the same name, append a numeric suffix (__2, __3) so both tools remain accessible. This fixes #575.
Enhancements:
Tests:
Related Issue
Resolve #575
Problem
When multiple MCP servers expose tools with the same name (e.g., both have read_node), the second server's tool was silently dropped. Only one server's tool was accessible, making it impossible to use overlapping MCP servers simultaneously.
What changed
Instead of dropping colliding tools, the ToolManager now disambiguates them by appending a numeric suffix (__2, __3) to the qualified name. Both tools remain registered and callable.
Checklist