Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 86 additions & 13 deletions packages/agent/src/agent-loop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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__<server>_<tool>`,
* and proxied bridges add a per-session instance segment
* (`mcp__<server>__<instance>_<tool>`). 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;
}

/**
Expand Down Expand Up @@ -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<string, unknown>;
Expand Down
76 changes: 76 additions & 0 deletions packages/agent/test/agent-loop-tool-not-found-red-team.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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__<server>__<instance>_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);
});
});
Loading