[diyaavirmani]

Description

Closes #1853.

The tool registry is a plain name-keyed map, and MCP tools were registered after the built-ins. So an MCP tool with a built-in's name silently replaced it: handler, schema and approval policy. The filesystem server's write_file and list_directory, and the git server's git_commit, are common examples. git_commit then ran without a prompt in auto-accept. When the server later went unhealthy or disconnected, its tools were removed by name, which deleted the built-in for the rest of the session.

As agreed in the issue, built-ins always win.

Change (source/tools/tool-manager.ts):

• ToolManager tracks the names the MCP client registered.
• An MCP tool whose name is already taken by a built-in, custom or skill tool is skipped, the same way a colliding custom tool is. It's listed on that server's init result (MCPInitResult.skippedTools).
• The unhealthy path, the recovery to connected, and disconnectMCP only add or remove names that MCP itself registered. A built-in can no longer be replaced on recovery or removed on disconnect.
• The MCP lookups also only apply to those names. That covers getMCPToolInfo, used by /tools and the confirmation prompt, and plan mode's read-only filter. So a skipped tool can't relabel the built-in as an MCP tool, and its missing readOnlyHint can't hide the built-in in plan mode. I found the plan-mode case while working on this: an unannotated MCP list_directory was hiding the built-in list_directory in plan mode.

Warning: describeSkippedMCPTools() builds one message, and the TUI (a WarningMessage), --plain (writeStatus) and the daemon (console.error) all print it at startup:

MCP server "fixture" offers tools that a built-in or custom tool already uses, so they were skipped: write_file

Docs: a short "Tool Name Collisions" section in docs/configuration/mcp-configuration.md.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update

Changeset

• Added a changeset (pnpm changeset) describing this change for the changelog

Testing

Automated Tests

• New features include passing tests in .spec.ts/tsx files
• All existing tests pass (pnpm test:all completes successfully): see below; the only failures also happen without this change
• Tests cover both success and error scenarios

New tests in source/tools/tool-manager.spec.ts. They use a fake client through the existing createMCPClient() seam:

• an MCP tool named like a built-in is skipped, the built-in's entry is untouched, skippedTools lists it, and the server's other tool still registers
• skippedTools is unset when nothing collides
• an MCP tool named like a skill tool is skipped
• an unhealthy server removes only its own tools, not a built-in it collided with (the maintainer's case)
• a server recovering to connected doesn't replace the built-in, and its own tool comes back (the maintainer's extra case)
• disconnectMCP removes only MCP tools (the maintainer's extra case)
• git_commit still needs approval in auto-accept with a server offering git_commit
• plan mode keeps a built-in that an unannotated MCP tool is named like
• describeSkippedMCPTools formatting

With only tool-manager.ts reverted (plus a stub for the new formatter), 7 of these fail. The other two, the "nothing collides" case and the formatter, are expected to pass there.

Two existing tests created their "MCP" tools with registerSkillTool and set the private mcpClient field, so as far as ToolManager is concerned those tools were never registered by MCP. They now connect a fake client through initializeMCP, with the same assertions:

• "plan mode hides MCP tools unless the server annotates them read-only" (tool-manager.spec.ts)
• "custom tools survive an MCP disconnect" (custom-tools/end-to-end.spec.ts)

Format, lint, types and knip pass. On macOS the full AVA suite passed 9,711 tests (20 skipped). The only 2 failures are the gh-pr-worktree tests that fail the same way on upstream main (/var vs /private/var tmpdir paths on macOS).

Manual Testing

• Tested with Ollama
• Tested with OpenRouter
• Tested with OpenAI-compatible API
• Tested MCP integration (if applicable)

End to end through the real CLI, on macOS with Node 22.23.3:

• --plain run --json, auto-accept,
• a scripted local OpenAI-compatible model,
• a real stdio MCP server built on @modelcontextprotocol/sdk that logs each call and never writes to disk.

These are the cases from the issue. The source of truth is notes.txt on disk, the MCP server's own call log, and the CLI's JSON report.

Case	v1.31.0 (npm, latest release)	upstream main 9a244b89	v1.32.0 release PR (257b95e2)	this branch	v1.32.0 release PR + this fix
no MCP, write_file (control)	written	written	written	written	written
MCP offers write_file	MCP handles it, not written	not written	not written	warning, built-in writes notes.txt, MCP never called	same as this branch
MCP offers echo_tool + crash (control)	list_directory works	works	works	works	works
MCP offers list_directory + crash	Not connected	Unknown tool: list_directory, no longer offered	same as main	warning, built-in list_directory works and stays offered	same as this branch
no MCP, git_commit (control)	approval required (exit 2)	same	same	same	same
MCP offers git_commit	runs with no prompt	no prompt	no prompt	approval required (exit 2)	same as this branch

In the interactive TUI, I started in a project whose server offers write_file and echo_tool, accepted the trust prompt, and ran /tools:

• upstream main: no warning, and /tools lists write_file (fixture).
• This branch: the warning shows, /tools lists echo_tool (fixture), and write_file is listed with the built-ins.

The fix cherry-picks cleanly onto the v1.32.0 release PR, and the branch merges cleanly with current main.

Checklist

• If this was for an open issue, I was assigned to it
• Code follows project style guidelines
• Self-review completed
• Documentation updated (if needed)
• No breaking changes (or clearly documented)
• Appropriate logging added using structured logging (startup warnings go through the existing per-surface output: WarningMessage, writeStatus and console.error, the same as MCP connection failures)

[github-actions]

nc-review: comments — 1 nit

@diyaavirmani — a few things worth a look, none blocking.

Closes #1853 cleanly. ToolManager now tracks names registered by MCP and treats any name collision with a built-in/custom/skill tool as an MCP skip (parallel to the existing custom-tool collision handling). Unhealthy/recovery/disconnect paths now mutate only MCP-owned names; the MCP lookups (getMCPToolInfo, plan-mode getMcpReadOnly) are gated on the same set, so an unannotated MCP list_directory can no longer hide the read-only built-in. A unified startup warning is wired into the TUI (WarningMessage), --plain (writeStatus), and the daemon (console.error). Tests cover all three collision surfaces and the unhealthy/reconnect/disconnect paths; two legacy tests were migrated off the mcpClient field-seam to the existing createMCPClient seam. Ready to merge.

⚪ nit · tests · source/tools/tool-manager.spec.ts:409

The recovery test covers echo_tool coming back and list_directory (a built-in shadowed by an unannotated MCP name) staying registered, which is the right shape. One edge it doesn't exercise: the recovered path's registerMCPEntries should call mcpClient.getToolEntries(change.serverName). If a future change drops the forServerName filter, every server's tools would land on every health-flip, and this test would still pass. A second server with ephemeral_tool would catch that and is cheap to add, but the omission is not load-bearing.

---

_{🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional}

_{Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.}

[github-actions]

Hi @diyaavirmani, thanks for this PR! It looks like a maintainer has left feedback
or review activity and there are still some outstanding items to wrap up.

Whenever you get a chance, could you take a look at the open comments?
If anything is unclear or you'd like a hand, just reply here and we'll help you get it across the line.