fix(agent,tools): name what was rejected, and stop guessing what a stale tool call meant

[probepark]

Scoped replacement for the tool-diagnostics portion of the closed #4021. Two commits, 8 files, one theme: when the harness already knows what the caller meant, it should act on that instead of burning a turn.

Both defects were observed repeatedly today across live agent sessions.

The defects

1. A tool call resolved to exactly one active tool was rejected instead of dispatched (832b2e12f, #4020)

Tool mcp__jzi2uzmxd57z__mr6er53iidr3_todo_write not found. It is active as `todo_write` — call that name instead.
Tool mcp__jzi2uzmxd57z__gtjwrxwd7nfm_search not found. It is active as `search` — call that name instead.

The message names the correct tool with certainty, then throws anyway. findToolCallNameAliases already does the resolution — parseToolCallName strips the bridge namespace and compares the base name, matching a namespaced call only against its own server or an unnamespaced tool — so when it returns exactly one alias the target is unambiguous.

It recurs constantly because proxied bridges mint a per-session instance segment (mcp__<server>__<instance>_<tool>), so a name replayed from earlier context differs from the live registry only in that segment. New session, new instance id, same stale names. The source comment already documents this; the code just refused to act on it.

Exactly one match is now dispatched, with the rename recorded. Argument validation, hooks, permissions and telemetry all run against the resolved tool. Zero or multiple candidates keep today's error, still naming every candidate — guessing between two tools would route the model at one it did not ask for.

This is the unfixed half of #3917, which was closed by improving the error text while the call kept being rejected.

2. todo_write rejections did not name the offending key (bdc2b241b)

Validation failed for tool "todo_write": raw arguments rejected before coercion;
todo_write operation entries accept only op, list, task, phase, items, and text keys

The message lists what is allowed but never which key was wrong, so the caller cannot tell what to remove and a retry reproduces the failure verbatim. Two sessions burned turns on it, both by passing note — op: "note" is valid and its body field is text, an easy and repeatable confusion.

hasUnknownKeys returned a boolean and discarded the offending keys, so nothing downstream could name them. It now returns them, the rejection carries them as structured detail, and the thrown message appends them to the existing guidance:

{"code":"todo-write-unknown-op-entry-key",
 "detail":{"rejectedKeys":["note"],
           "hint":"note is an op, not a key: use { op: \"note\", text: \"...\" }"}}

A correction is suggested only where the mapping is exact — an unrecognised key is named with no guess attached. No fuzzy or edit-distance matching: a confident wrong suggestion is worse than naming the key and stopping. The allowed key set is unchanged; the schema was right, only the diagnostic was wrong. Codes that carry no detail (the ask-* family) keep their exact current message.

Verification

Load-bearing proof — restoring packages/agent/src, packages/ai/src and packages/coding-agent/src from origin/dev and re-running:

with fix:     68 pass / 0 fail
without fix:  68 pass / 13 fail

agent-loop red-team + alias-dispatch + tool-argument-coercion
  + todo-write + descriptors + ask                              208 pass / 0 fail
bun --cwd=packages/agent run check                              exit 0
bun --cwd=packages/ai run check                                 exit 0

Existing red-team assertions were extended, never weakened, and the Tool <name> not found base wording stays byte-for-byte stable on the still-throwing paths.

Relationship to #4021

#4021 bundled twelve unrelated defects into 53 files and was closed with the instruction to open fresh, scoped PRs. This is the fourth, after #4031 (ACP turn lifecycle), #4033 (chat daemon reconnect) and #4035 (broker/host lifecycle). One remains: gc/disk retention.

Closes #4020

[probepark]

Self-review: REQUEST_CHANGES on my own PR. A red-team pass found that the auto-dispatch mis-routes real tool calls, and I reproduced it independently. This is the exact failure mode I said would be a blocker when I wrote the PR — and it is present.

Blocker: parseToolCallName over-strips, so unrelated tools become the "single unambiguous candidate"

The regex /^mcp__([^_]+)(?:__[^_]+)?_/ treats the second segment as an optional instance id and strips it. For a two-segment MCP name the second segment is part of the TOOL name, not an instance. Verified at HEAD:

mcp__brave__web_search       -> { server: "brave",     base: "search" }
mcp__jbcontext__code_search  -> { server: "jbcontext", base: "search" }
mcp__github__close_issue     -> { server: "github",    base: "issue" }
mcp__github__create_issue    -> { server: "github",    base: "issue" }

web_search → search. code_search → search. close_issue and create_issue → the same base issue, on the same server, so the cross-server guard never fires.

Before this PR those calls produced an error. Now they are executed against the wrong tool. Reproduced on this branch (4/4 fail here, all correctly error on origin/dev):

• active [search, read], call mcp__brave__web_search → runs the local regex-grep search. The model asked a web-search server and got a repo grep. searchRuns expected 0, actual 1.
• active [search, read, bash], call mcp__jbcontext__code_search → runs the local search. Both are real tool names in this repo's own AGENTS.md.
• active [mcp__github__create_issue], call mcp__github__close_issue → dispatched to create_issue. Only a schema mismatch stops a write to the wrong endpoint; that is luck, not a guard.

A fourth finding also reproduces: a single tool reachable under both name and customWireName is double-counted as two candidates, so a legitimate rename refuses to dispatch — the opposite failure.

Why this is not fixable by tightening the guard alone

The premise — "the namespace-stripped base name identifies the tool" — is false whenever a tool name itself contains _, which is the common case. Any dispatch built on that parse can silently route a model at a tool it did not ask for, and a write-capable MCP tool makes that unsafe rather than merely wrong.

The correct resolution is an exact-identity match: same server, same tool name, differing only in the bridge instance segment — provable, not inferred from splitting on underscores. If that cannot be established from the registry, the call must keep erroring with the existing message.

Not merging until the parse is identity-based and all four reproductions pass as regressions. The todo_write half of this PR (naming the rejected key) is unaffected by this finding and I am keeping it.

The other split PRs are under review; I will post each outcome, including the ones I reject.

[probepark]

Blocker fixed in ca8991b89: resolution is identity-based — a stale name matches only when byte-identical to a registry-known name for that tool except the bridge instance segment, candidates dedupe by the tool they denote, and anything unprovable keeps the unchanged error. All four reproductions pass as regressions (they error/dispatch correctly now); 2 pass/7 fail without src; packages/agent 711 pass / 0 fail.

[probepark]

Scope changed, deliberately. The tool auto-dispatch half of this PR is removed; what ships is the diagnostic work.

Nine review rounds each produced a new lookalike, because the premise was false: a wire name cannot prove identity. The instance segment a bridge mints and a tool name that merely contains an underscore are indistinguishable, so mcp__brave__web_search resolved to a local search and close_issue to create_issue on the same server. Every tightening of the parse found another collision. Mis-dispatching a write-capable MCP tool is not a risk worth a convenience, so unresolvable names keep the existing not-found error and are pointed at tool discovery — which resolves them from the registry instead of from string shape.

Restoring dispatch later needs the registry to expose the instance segment, not a better parser. That is a separate change.

agent-loop-tool-call-alias-dispatch.test.ts is deleted because its entire subject is gone; the eight cases in the identity suite assert the replacement contract, including every lookalike the reviews surfaced.

What ships: todo_write rejections now name the offending key, with an exact correction only where the mapping is unambiguous, and stale tool names get an honest error plus a discovery pointer.

Local codex-pro gate: the final round's mis-dispatch claim was rejected with counter-evidence — the dispatch path (parseToolCallName, callNameDenotesTool) is gone, all three reviewer reproductions pass, and the cited mcp__brave__current_search case is covered by a passing test. 780 pass / 0 fail across packages/agent + todo-write + coercion; 36 pass / 7 fail with src stashed; check exit 0.