[alexb-openrouter]

Summary

Risk: when two client tools in tools share a name, a call is dispatched to whichever one is listed first (findToolByName). That includes calls approved through approveToolCalls: a call approved while only tool A was registered runs on tool B with A's arguments if the resume passes [B, A]. In practice B is usually an unprefixed MCP tool from a lower-trust server whose name collides with a local tool. createMCPTools already rejects duplicates within one server, but nothing checked the combined list passed to callModel.

Fix: the ModelResult constructor (reached from callModel, deferred .resolve() with run, and direct use) now calls assertUniqueClientToolNames(options.tools) and throws:

Duplicate tool name "read_value": every tool passed to callModel must have a unique name. Rename one, or use toolNamePrefix when combining MCP tools with local tools.

Server tools have no client name and are skipped. Because the throw happens before any state load, a paused conversation stays awaiting_approval and can be resumed with a corrected list.

Behavior change: callers who currently pass duplicate names (which the model cannot tell apart anyway, and some providers reject with HTTP 400) now get an error. Changeset is patch.

Tests: tests/unit/duplicate-tool-names.test.ts

• duplicate names throw from both new ModelResult and callModel
• pause on approval with [approved], resume with [other, approved] + approveToolCalls: before the fix other.execute ran with the approved arguments; after the fix it throws and other never runs

pnpm run lint, pnpm run typecheck, and the unit project (110 files, 1267 tests) pass locally.

[devin-ai-integration]

I'll fix CI failures and address comments from users with write access that start with 'Devin'.

• Disable automatic comment, CI, and merge conflict monitoring

[devin-ai-integration]

Verdict: Approve. The diagnosis is right: client tool calls are dispatched by name (findToolByName and the this.options.tools?.find(... t.function.name ...) lookups in model-result.ts), so a duplicate name silently routes a call, including an approved one, to whichever tool is listed first. Checking once in the ModelResult constructor covers every public entry point (callModel constructs ModelResult; executeToolLoop is not exported), and the tool list is not mutated after construction. Throwing before any state load keeps a paused conversation in awaiting_approval, which the resume test confirms.

Out of scope, no change needed here: approvals are still bound to callId + name, not to a tool instance, so resuming with a different tool that reuses the approved name (and no duplicate) still runs the new tool with the approved arguments. If that matters later, approvals could record a tool identity in saved state.

Estimated impact: low. Callers who pass duplicate client tool names now get a synchronous error instead of ambiguous dispatch; there is no other behavior change and the fix is easy to revert.
Risk: implementation low, premise low, impact low (fail-fast validation on a configuration the model cannot disambiguate anyway). CI is green.

1 suggestion, 0 blockers, 0 nits.