Repository navigation
fix(amazonq): derive MCP tool name mapping from enabled tools - #2904
Draft
dungdong-aws wants to merge 1 commit into
Draft
dungdong-aws wants to merge 1 commit into
dungdong-aws wants to merge 1 commit into
Conversation
toolServer registers MCP tools from getEnabledTools(), while agenticChatController.#getTools built the tool name mapping from getAllTools(). createNamespacedToolName assigns bare names by set occupancy, so a disabled tool could claim a bare name first and push an enabled tool with the same name to its namespaced form. The model was then offered a name the runtime had not registered, and the mapping written by setToolNameMapping resolved the bare name to the disabled tool. #getTools now derives the mapping from getEnabledTools(), the same set toolServer registers from. The test stubs gain getEnabledTools, and a test pins that a disabled tool does not appear in the mapping and does not displace an enabled tool's bare name.
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.
Summary
Two code paths compute MCP tool names independently with
createNamespacedToolName, which assigns bare names by set occupancy, first come first served:toolServer.registerServerToolsbuilds names fromgetEnabledTools(). This is the name a tool is registered under in the runtime.agenticChatController.#getToolsbuilt names fromgetAllTools(), which includes disabled tools. This is the name the model is offered, and#getToolsalso writes the result into the manager viasetToolNameMapping, which is whatgetOriginalToolNamesreads at dispatch.When two servers advertise the same tool name and one of them is disabled, the two sides disagree.
toolServersees only the enabled tool and registers it under the bare name.#getToolssees both, lets the disabled tool claim the bare name first, and pushes the enabled tool to itsserver___toolform. The model is offered a name the runtime has not registered, and the mapping resolves the bare name to a tool the user turned off.#getToolsnow derives the mapping fromgetEnabledTools(), the same settoolServerregisters from.Relationship to #2903
Independent. #2903 reserves built-in tool names so a server-advertised name cannot select built-in dispatch. This change makes the two MCP name-assignment sites agree with each other. Neither depends on the other, they touch different lines of
#getTools, and both callcreateNamespacedToolNamein forms that merge in either order. This was split out of #2903 to keep that change scoped to the built-in collision.Intentional behavior changes
mcpToolSpecNames, which feeds the read-only-mode tool filter, no longer includes disabled tools. Disabled tools are not registered, so they were never present in the list being filtered; this changes nothing observable there.Tests and verification
Adds a
processToolUsescase where two servers advertisesearchand only one is enabled, asserting the written mapping resolvessearchto the enabled server, does not contain the namespaced form for it, and contains no entry for the disabled server. The sharedMcpManagertest stub gainsgetEnabledTools.Prettier passed on both changed files. ESLint reported no errors; the remaining warnings are pre-existing
import/no-nodejs-modules.Verification limits
Compilation and the unit suite were not run locally. The authoring host enforces memory-capped build invocation through a cgroup scope that the sandboxed session could not create, so
tscand Mocha were refused rather than run uncapped. CI is the first execution of the added test.Not verified in a running IDE. The scenario requires two MCP servers sharing a tool name with one disabled.