chore: disallow overriding of internal tool implementations

[himanshusinghs]

Proposed changes

This PR follows up on recent conversation regarding tool implementations and the idea that we should not allow library consumers to remove internal tool implementations and if they wish to disable internal tools, they can do so using the session config hook.

Following the outcome of the same conversation, this implements a name collision check to ensure we don't register same named tools more than once.

Additionally, the last commit (9cc5526), moves the name, operationType and category to static properties of ToolClass so that library consumers have a chance of disabling multiple tools conditionally without statically typing the tool names.

I would advice reviewing the PR in two different steps:

1. Review the change that disallow overriding of internal tool implementation and implements the tool name collision check, basically the first two commits - Link here.
2. Review the change that moves the name, operationType and category to static properties of ToolClass to make it easy for library consumers, the last commit - Link here

Checklist

• I have signed the MongoDB CLA

[Copilot]

Pull request overview

This PR refactors the tool registration system to prevent external tools from overriding internal tool implementations. The main change is renaming the tools parameter to additionalTools throughout the codebase, making it clear that custom tools are registered in addition to, rather than instead of, the default tools. Additionally, it implements a name collision detection mechanism to prevent registering tools with duplicate names.

Key Changes:

• Renamed tools parameter to additionalTools across the server, transport, and test files
• Added collision detection in registerTools() to throw errors when duplicate tool names are detected
• Updated test expectations to reflect that custom tools now supplement rather than replace default tools

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/server.ts	Implements core changes: renames tools to additionalTools, separates internal/additional tool implementations, and adds name collision detection logic
src/transports/base.ts	Updates transport runner to use additionalTools parameter name and passes it through to server construction
tests/integration/customTools.test.ts	Updates test to verify custom tools are registered alongside (not replacing) default tools
tests/integration/tools/mongodb/mongodbTool.test.ts	Updates test setup to use additionalTools parameter and adds new test for name collision detection
tests/integration/tools/mongodb/create/createIndex.test.ts	Adjusts assertion to accept both "PENDING" and "BUILDING" statuses for vector search index

[gagik]

Lacking context about the conversation, what is the concern here? If they want to override the default tools, I don't see why they shouldn't be able to.
They might want to replace our tools with their modified versions and it'd be a lot more annoying to deal with the existence of both and go through hacks to disable the rest.

[himanshusinghs]

For context - we discussed that we already have means to replace the internal tools (using disabledTools and adding new tools) and having another method does not make sense.

The usecase of overriding our internal tools is still supported, users just need to disable the internal tool and provide their own implementation but under a different name of their choosing. I don't think disabling tools is a hack, its a feature we have supported all the way so far.

[coveralls]

Pull Request Test Coverage Report for Build 19743479350

Details

• 131 of 136 (96.32%) changed or added relevant lines in 51 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.07%) to 80.54%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/server.ts	24	25	96.0%
src/lib.ts	0	4	0.0%

Totals
Change from base Build 19741078953:	0.07%
Covered Lines:	6698
Relevant Lines:	8237

---

💛 - Coveralls

[gagik]

ConnectClusterTool.toolName? it's better to avoid stutter

[himanshusinghs]

The static property name is unfortunately reserved for Function.name and TS forbids overriding that.

[gagik]

TIL