diff --git a/packages/agent/src/agent-loop.ts b/packages/agent/src/agent-loop.ts index 9a76d2571b..cb42639054 100644 --- a/packages/agent/src/agent-loop.ts +++ b/packages/agent/src/agent-loop.ts @@ -2486,14 +2486,79 @@ function emitAbortedAssistantMessage( } /** - * Match a tool against the model-visible call name. Tools emitted via OpenAI's - * custom-tool path (e.g. `apply_patch` on GPT-5) arrive under their wire-level - * name, which may differ from the harness-internal `name`, so dispatch and any - * "is this tool callable" check must consider both. Internal `name` takes - * precedence when a caller needs a single match. + * Model-visible call names of a tool. Tools emitted via OpenAI's custom-tool + * path (e.g. `apply_patch` on GPT-5) arrive under their wire-level name, which + * may differ from the harness-internal `name`, so dispatch and any "is this + * tool callable" check must consider both. */ -function toolMatchesCallName(tool: { name: string; customWireName?: string }, callName: string): boolean { - return tool.name === callName || (tool.customWireName !== undefined && tool.customWireName === callName); +function toolCallNames(tool: { name: string; customWireName?: string }): string[] { + return tool.customWireName === undefined || tool.customWireName === tool.name + ? [tool.name] + : [tool.name, tool.customWireName]; +} + +/** + * Wire name of the tool-discovery tool. Sessions that hide discoverable + * built-ins expose it under this name, or under a bridge alias of it. + */ +const TOOL_DISCOVERY_NAME = "search_tool_bm25"; + +/** + * Split an MCP bridge namespace off a call name so it can be compared against + * the tool it actually denotes. Bridges expose tools as `mcp___`, + * and proxied bridges add a per-session instance segment + * (`mcp_____`). A name the model replayed from earlier + * context therefore differs from the live registry only in that segment. + */ +function parseToolCallName(name: string): { server?: string; base: string } { + const namespace = /^mcp__([^_]+)(?:__[^_]+)?_/.exec(name); + if (!namespace) return { base: name }; + return { server: namespace[1], base: name.slice(namespace[0].length) }; +} + +/** + * Call names of active tools that denote the same tool as an unresolved call + * name. Two servers can expose the same tool name, so a namespaced call is only + * matched against its own server or against an unnamespaced tool. + */ +function findToolCallNameAliases( + callName: string, + tools: ReadonlyArray<{ name: string; customWireName?: string }> | undefined, + limit = 3, +): string[] { + const target = parseToolCallName(callName); + if (target.base.length === 0) return []; + const aliases: string[] = []; + for (const tool of tools ?? []) { + for (const candidate of toolCallNames(tool)) { + if (candidate === callName) continue; + const parsed = parseToolCallName(candidate); + if (parsed.base !== target.base) continue; + if (parsed.server !== undefined && target.server !== undefined && parsed.server !== target.server) continue; + if (aliases.includes(candidate)) continue; + aliases.push(candidate); + if (aliases.length === limit) return aliases; + } + } + return aliases; +} + +/** + * Resolve how tool discovery is actually callable in this session. Assuming the + * bare `search_tool_bm25` literal both drops the hint when the discovery tool is + * bridged and, worse, would name a second non-callable tool if emitted anyway. + */ +function findToolDiscoveryCallName( + tools: ReadonlyArray<{ name: string; customWireName?: string }> | undefined, +): string | undefined { + let bridged: string | undefined; + for (const tool of tools ?? []) { + for (const candidate of toolCallNames(tool)) { + if (candidate === TOOL_DISCOVERY_NAME) return candidate; + if (bridged === undefined && parseToolCallName(candidate).base === TOOL_DISCOVERY_NAME) bridged = candidate; + } + } + return bridged; } /** @@ -2697,12 +2762,20 @@ async function executeToolCalls( // base wording stays byte-for-byte stable for downstream consumers; // the period and hint are appended only when discovery is callable. const base = `Tool ${toolCall.name} not found`; - const hasToolDiscovery = tools?.some(t => toolMatchesCallName(t, "search_tool_bm25")) ?? false; - throw new Error( - hasToolDiscovery - ? `${base}. If you are unsure whether this tool exists or how to use it, call \`search_tool_bm25\` to discover and activate the matching tool, then retry.` - : base, - ); + const hints: string[] = []; + const aliases = findToolCallNameAliases(toolCall.name, tools); + if (aliases.length > 0) { + hints.push( + `It is active as ${aliases.map(name => `\`${name}\``).join(" or ")} — call that name instead.`, + ); + } + const discoveryCallName = findToolDiscoveryCallName(tools); + if (discoveryCallName !== undefined) { + hints.push( + `If you are unsure whether this tool exists or how to use it, call \`${discoveryCallName}\` to discover and activate the matching tool, then retry.`, + ); + } + throw new Error(hints.length > 0 ? `${base}. ${hints.join(" ")}` : base); } let effectiveArgs: Record; diff --git a/packages/agent/test/agent-loop-tool-not-found-red-team.test.ts b/packages/agent/test/agent-loop-tool-not-found-red-team.test.ts index 4b589f175c..6d6dd57729 100644 --- a/packages/agent/test/agent-loop-tool-not-found-red-team.test.ts +++ b/packages/agent/test/agent-loop-tool-not-found-red-team.test.ts @@ -134,4 +134,80 @@ describe("agentLoop: tool-not-found discovery hint red team", () => { expect(toolResults).toHaveLength(1); expect(toolResults[0].text).toContain(`Tool ${toolName} not found`); }); + + // Issue #3917, captured sessions 019fd580/019fd583/019fd595: the model called + // `mcp_____search` while plain `search` was active, five + // times across three sessions, and the bare not-found named no way back. + it("names the active tool when the call carries an MCP bridge namespace", async () => { + const toolName = "mcp__jzi2uzmxd57z__wbg7pcrl46bd_search"; + const toolResults = await collectToolResults([makeTool("search"), makeTool("read")], toolName); + + expect(toolResults).toHaveLength(1); + expectBaseNotFound(toolResults[0], toolName); + expect(toolResults[0].text).toContain("It is active as `search`"); + expect(toolResults[0].text).not.toContain("`read`"); + }); + + // Bridges mint the instance segment per session, so a name replayed from + // earlier context differs from the live registry only in that segment. + it("names the live alias when only the bridge instance segment went stale", async () => { + const toolName = "mcp__jzi2uzmxd57z__jgspauo3hmi5_subagent"; + const toolResults = await collectToolResults([makeTool("mcp__jzi2uzmxd57z__gbbgnmhc3qkt_subagent")], toolName); + + expect(toolResults).toHaveLength(1); + expectBaseNotFound(toolResults[0], toolName); + expect(toolResults[0].text).toContain("It is active as `mcp__jzi2uzmxd57z__gbbgnmhc3qkt_subagent`"); + }); + + it("resolves an alias reachable only through customWireName", async () => { + const toolName = "mcp__srv__stale_apply_patch"; + const toolResults = await collectToolResults([makeTool("edit", { customWireName: "apply_patch" })], toolName); + + expect(toolResults).toHaveLength(1); + expectBaseNotFound(toolResults[0], toolName); + expect(toolResults[0].text).toContain("It is active as `apply_patch`"); + }); + + it("does not invent an alias when no active tool shares the base name", async () => { + const toolName = "mcp__srv__abc_write"; + const toolResults = await collectToolResults([makeTool("read"), makeTool("search")], toolName); + + expect(toolResults).toHaveLength(1); + expectBaseNotFound(toolResults[0], toolName); + expect(toolResults[0].text).not.toContain("It is active as"); + }); + + // Two servers can expose the same tool name, and routing the model at the + // wrong server's tool is worse than the dead end. + it("does not cross servers when suggesting an alias", async () => { + const toolName = "mcp__alpha__abc_search"; + const toolResults = await collectToolResults([makeTool("mcp__beta__abc_search")], toolName); + + expect(toolResults).toHaveLength(1); + expectBaseNotFound(toolResults[0], toolName); + expect(toolResults[0].text).not.toContain("It is active as"); + }); + + // Emitting the bare `search_tool_bm25` literal here would name a second + // non-callable tool, so the hint has to carry the bridged call name. + it("points at the bridged discovery call name instead of the bare literal", async () => { + const toolName = "remembered_discoverable_tool"; + const toolResults = await collectToolResults([makeTool("mcp__srv__abc_search_tool_bm25")], toolName); + + expect(toolResults).toHaveLength(1); + expectBaseNotFound(toolResults[0], toolName); + expect(toolResults[0].text).toContain("call `mcp__srv__abc_search_tool_bm25` to discover"); + expect(toolResults[0].text).not.toContain("call `search_tool_bm25` to discover"); + }); + + it("prefers the unbridged discovery name when both are callable", async () => { + const toolName = "remembered_discoverable_tool"; + const toolResults = await collectToolResults( + [makeTool("mcp__srv__abc_search_tool_bm25"), makeTool("search_tool_bm25")], + toolName, + ); + + expect(toolResults).toHaveLength(1); + expect(toolResults[0].text).toContain(DISCOVERY_HINT); + }); });