diff --git a/docs/MCP.md b/docs/MCP.md index 6b7350d8f..fa7a8f65e 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -25,8 +25,12 @@ does not require project trust. Local settings **replace** global MCP entirely when present (they do not merge). Tools from connected servers are not advertised to the model up front; they are -registered for dispatch and surfaced on demand through dynamic tool discovery -(`tool_search`). +registered for free-name dispatch and surfaced on demand through dynamic tool +discovery (`tool_search`). Search returns schema cards into the conversation +history; the model then calls matched tools by exact name. The wire tools array +stays a fixed product prefix for the whole session (file/shell loop, product +loop tools, plus only harness-blocked substitutes: bounded `grep`/`search_files` +and `web_*`) so the provider tools cache stays hot — MCP never joins that prefix. ## Server Kinds diff --git a/src/agent/prompts.ts b/src/agent/prompts.ts index e3f1b2f26..cf0d3c0ec 100644 --- a/src/agent/prompts.ts +++ b/src/agent/prompts.ts @@ -2,6 +2,7 @@ import type { EnvironmentInfo } from "./environment.js"; import type { SkillSummary } from "../extensions/skills.js"; import type { SessionMode } from "../config/session-mode.js"; import { + advertisedToolNamesForSessionMode, coreToolNamesForSessionMode, CORE_TOOL_NAMES, type ToolAvailability, @@ -82,7 +83,7 @@ export function buildHarnessFacts( "- Attached images are native multimodal input; inspect them directly unless file-level forensics are requested.", ...(dynamicTools ? [ - "- Only the core tools below are loaded. Use tool_search to load extra capabilities from plugins or integrations when needed.", + "- Tools listed below are the fixed wire set (always callable). Hundreds more plugin/MCP tools may be registered for free-name dispatch: use tool_search to discover them, then call by exact name — search does not load or promote tools onto the wire.", ...(sessionMode === "orchestrator" ? [ "- Use search_agents before dispatching named specialists or teams (results include full profile bodies; do not read_file plugin paths outside the workspace).", @@ -112,12 +113,13 @@ export function buildGuidelines(opts: { subAgent?: boolean; sessionMode?: Sessio "- No emojis in code or docs unless the user uses them.", "", "Tool choice:", - "- read_file for file contents; grep or search_files to locate code; lsp for symbols, types, references, or call flow before opening large files.", + "- read_file for file contents; grep or search_files to locate code (bounded tools — shell find/rg/grep -r are blocked).", "- edit_file for targeted changes; write_file for new files or full rewrites; delete_file to remove files — never echo, heredoc, sed, or rm in the shell for those jobs.", - "- run_shell for builds, tests, git, and one-off commands — not for shell find, head-position rg, or recursive grep -r (OOM risk), cat, or messaging the user.", + "- run_shell for builds, tests, git, and one-off commands — not for shell find, head-position rg, recursive grep -r, cat, or messaging the user.", + "- web_fetch / web_search for URLs and web queries — never curl/wget or a hand-rolled search.", ...(subAgent ? [] - : ["- tool_search before assuming a plugin or MCP tool exists; use_skill before work covered by a listed skill."]), + : ["- tool_search only for plugin/MCP tools not listed under Tools; use_skill before work covered by a listed skill."]), "", subAgent ? "Proceed vs pause:" : "Ask vs proceed:", ...(subAgent @@ -210,7 +212,7 @@ const TOOL_SUMMARIES: Record = { submit_output: "signal the task is complete — the only way to finish", ask_operator: "pause and ask the user when blocked or genuinely ambiguous", present: "dynamically render aligned/structured output using the layout primitives (stack/row/grid/text etc)", - tool_search: "load more tools by capability when you need them", + tool_search: "discover plugin/MCP tools not listed under Tools (not for file/shell/web/search work already listed)", use_skill: "load a listed skill's full instructions before doing work it covers", }; @@ -307,7 +309,10 @@ export function buildChatSystemPrompt( ): string { const sections = [ baseSection(baseOverride, sessionMode), - buildAvailableTools(coreToolNamesForSessionMode(sessionMode, toolAvailability)), + // List every tool on the wire (core + catalog, including web_fetch/web_search), + // not just CORE. Models that only see core names re-discover catalog tools via + // tool_search and thrash instead of calling what is already declared. + buildAvailableTools(advertisedToolNamesForSessionMode(sessionMode, toolAvailability)), ]; if (skills.length > 0) sections.push(buildSkillsSection(skills)); sections.push(contextSection(env)); diff --git a/src/agent/tool-search.test.ts b/src/agent/tool-search.test.ts index f41d3d1ec..a9645a94c 100644 --- a/src/agent/tool-search.test.ts +++ b/src/agent/tool-search.test.ts @@ -3,7 +3,6 @@ import type { ToolDefinition } from "@intx/types/runtime"; import { createToolIndex, createToolSearchTool, - createActivatedToolTracker, advertisedTools, advertisedToolNamesForSessionMode, coreToolNamesForSessionMode, @@ -21,7 +20,11 @@ const NO_AVAILABILITY: ToolAvailability = { const defs: ToolDefinition[] = [ { name: "read_file", description: "read a file", inputSchema: { type: "object", properties: {}, required: [] } }, - { name: "web_search", description: "search the web for pages", inputSchema: { type: "object", properties: {}, required: [] } }, + { + name: "mcp__exa__web_search_exa", + description: "search the web for any topic and get clean content", + inputSchema: { type: "object", properties: {}, required: [] }, + }, { name: "lsp", description: "resolve symbols, find references", inputSchema: { type: "object", properties: {}, required: [] } }, { name: "mcp__linear__create_issue", @@ -42,7 +45,7 @@ const index = createToolIndex(() => defs); describe("createToolIndex", () => { test("ranks a name-token match above a description-only match", () => { const results = index.search("search"); - expect(results[0]).toBe("web_search"); + expect(results[0]).toBe("mcp__exa__web_search_exa"); }); test("finds an MCP tool by raw substring even when not a whole token", () => { @@ -50,58 +53,66 @@ describe("createToolIndex", () => { }); test("matches by capability words in the description", () => { - expect(index.search("pages")).toContain("web_search"); - }); - - test("never returns lsp — it is a core tool", () => { - expect(CORE_TOOL_NAMES).toContain("lsp"); - expect(index.search("find references")).not.toContain("lsp"); - }); - - test("never returns a core tool (those are always loaded)", () => { - expect(CORE_TOOL_NAMES).toContain("read_file"); - expect(index.search("read a file")).not.toContain("read_file"); + expect(index.search("topic")).toContain("mcp__exa__web_search_exa"); }); - test("orchestrator mode advertises task and search_agents; single mode omits them", () => { - expect(advertisedToolNamesForSessionMode("orchestrator", FULL_AVAILABILITY)).toContain("task"); - expect(advertisedToolNamesForSessionMode("orchestrator", FULL_AVAILABILITY)).toContain("search_agents"); - expect(advertisedToolNamesForSessionMode("single", FULL_AVAILABILITY)).not.toContain("task"); - expect(advertisedToolNamesForSessionMode("single", FULL_AVAILABILITY)).not.toContain("search_agents"); + test("returns empty for a query that matches nothing", () => { + expect(index.search("zzzznonexistent")).toEqual([]); }); - test("manage_tasks is advertised in both session modes regardless of availability", () => { - expect(coreToolNamesForSessionMode("single", NO_AVAILABILITY)).toContain("manage_tasks"); - expect(coreToolNamesForSessionMode("orchestrator", NO_AVAILABILITY)).toContain("manage_tasks"); - }); - - test("present is never in the advertised core set — discovered via tool_search only", () => { - expect(CORE_TOOL_NAMES).not.toContain("present"); - expect(coreToolNamesForSessionMode("orchestrator", FULL_AVAILABILITY)).not.toContain("present"); - }); - - test("lsp is advertised only when a language server was detected at startup", () => { - expect( - coreToolNamesForSessionMode("orchestrator", { languageServerAvailable: true }), - ).toContain("lsp"); - expect( - coreToolNamesForSessionMode("orchestrator", { languageServerAvailable: false }), - ).not.toContain("lsp"); + test("returns empty for a blank query", () => { + expect(index.search(" ")).toEqual([]); }); - test("ask_operator is advertised regardless of session mode or availability", () => { - expect(coreToolNamesForSessionMode("single", NO_AVAILABILITY)).toContain("ask_operator"); - expect(coreToolNamesForSessionMode("orchestrator", NO_AVAILABILITY)).toContain("ask_operator"); + test("excludes already-advertised tools from results", () => { + const withAdvertised = createToolIndex(() => defs, ["mcp__exa__web_search_exa"]); + expect(withAdvertised.search("search")).not.toContain("mcp__exa__web_search_exa"); }); - test("the advertised set is deterministic — repeat calls with the same inputs are identical", () => { - const first = coreToolNamesForSessionMode("orchestrator", NO_AVAILABILITY); - const second = coreToolNamesForSessionMode("orchestrator", NO_AVAILABILITY); - expect(second).toEqual(first); + test("prefers Exa over other MCP web tools when both match", () => { + const withExa: ToolDefinition[] = [ + ...defs, + { + name: "mcp__other__web_search", + description: "search the web for pages", + inputSchema: { type: "object", properties: {}, required: [] }, + }, + ]; + const ranked = createToolIndex(() => withExa).search("search the web"); + expect(ranked[0]).toBe("mcp__exa__web_search_exa"); + expect(ranked.length).toBeLessThanOrEqual(3); + }); + + test("does not return web_fetch or web_search — they are catalog tools on the wire", () => { + expect(CATALOG_TOOL_NAMES).toContain("web_fetch"); + expect(CATALOG_TOOL_NAMES).toContain("web_search"); + const withWeb: ToolDefinition[] = [ + ...defs, + { + name: "web_fetch", + description: "fetch a web page over HTTP", + inputSchema: { type: "object", properties: {}, required: [] }, + }, + { + name: "web_search", + description: "search the web for pages", + inputSchema: { type: "object", properties: {}, required: [] }, + }, + ]; + const ranked = createToolIndex(() => withWeb).search("fetch a web page"); + expect(ranked).not.toContain("web_fetch"); + expect(ranked).not.toContain("web_search"); }); +}); - test("returns nothing for an empty query", () => { - expect(index.search(" ")).toEqual([]); +describe("coreToolNamesForSessionMode", () => { + test("omits multi-agent tools in single mode; lsp is not on the fixed wire", () => { + const single = coreToolNamesForSessionMode("single", FULL_AVAILABILITY); + expect(single).not.toContain("task"); + expect(single).not.toContain("search_agents"); + expect(single).not.toContain("lsp"); + expect(coreToolNamesForSessionMode("orchestrator", FULL_AVAILABILITY)).toContain("task"); + expect(coreToolNamesForSessionMode("orchestrator", NO_AVAILABILITY)).not.toContain("lsp"); }); }); @@ -111,43 +122,38 @@ function call(tool: ReturnType, args: Record { - test("promotes matches and lists them back to the model", async () => { - const promoted: string[] = []; + test("lists matches as free-name dispatch cards (no wire promotion)", async () => { const tool = createToolSearchTool({ search: (q) => index.search(q), lookup: (name) => defs.find((d) => d.name === name), - promote: (names) => promoted.push(...names), }); const out = await call(tool, { query: "search the web" }); - expect(promoted).toContain("web_search"); - expect(out).toContain("web_search"); - expect(out).toContain("search the web"); + expect(out).toContain("mcp__exa__web_search_exa"); + expect(out).toContain("already dispatchable"); + expect(out).toContain("call by exact name"); }); test("surfaces a matched tool's input schema so the model can shape arguments", async () => { const tool = createToolSearchTool({ search: (q) => index.search(q), lookup: (name) => defs.find((d) => d.name === name), - promote: () => undefined, }); const out = await call(tool, { query: "issue tracker" }); expect(out).toContain("mcp__linear__create_issue"); - // Parameter names and the required list must appear — this is the whole - // point: MCP tools are never in the wire tools array up front, so their - // schema reaches the model through the tool_search result this same turn, - // ahead of the promoted definition landing on the next infer call. + // Parameter names and the required list must appear — MCP tools are never in + // the wire tools array, so schema reaches the model through this card only. expect(out).toContain("title"); expect(out).toContain("teamId"); expect(out).toContain("required"); }); test("rejects an empty query", async () => { - const tool = createToolSearchTool({ search: () => [], lookup: () => undefined, promote: () => undefined }); + const tool = createToolSearchTool({ search: () => [], lookup: () => undefined }); expect(await call(tool, { query: " " })).toContain("Error:"); }); test("reports when nothing matches", async () => { - const tool = createToolSearchTool({ search: () => [], lookup: () => undefined, promote: () => undefined }); + const tool = createToolSearchTool({ search: () => [], lookup: () => undefined }); expect(await call(tool, { query: "nonsense" })).toContain("No tools matched"); }); }); @@ -155,15 +161,17 @@ describe("createToolSearchTool", () => { describe("advertisedTools", () => { const registry: ToolDefinition[] = [ { name: "read_file", description: "read", inputSchema: { type: "object", properties: {}, required: [] } }, - { name: "grep", description: "grep", inputSchema: { type: "object", properties: {}, required: [] } }, { name: "write_file", description: "write", inputSchema: { type: "object", properties: {}, required: [] } }, + { name: "grep", description: "grep", inputSchema: { type: "object", properties: {}, required: [] } }, + { name: "list_dir", description: "list", inputSchema: { type: "object", properties: {}, required: [] } }, + { name: "search_files", description: "glob", inputSchema: { type: "object", properties: {}, required: [] } }, + { name: "web_fetch", description: "fetch", inputSchema: { type: "object", properties: {}, required: [] } }, { name: "mcp__linear__create_issue", description: "create", inputSchema: { type: "object", properties: {}, required: [] } }, ]; test("single session mode omits multi-agent tools from the wire prefix", () => { const names = advertisedTools( registry, - [], advertisedToolNamesForSessionMode("single", FULL_AVAILABILITY), ).map((d) => d.name); expect(names).not.toContain("task"); @@ -171,15 +179,22 @@ describe("advertisedTools", () => { expect(names).toContain("read_file"); }); - test("with no activation, advertises only the fixed built-in set, never MCP tools", () => { + test("fixed wire is file/shell loop + blocked-shell substitutes; list_dir/lsp/MCP stay free-name", () => { const names = advertisedTools(registry).map((d) => d.name); expect(names).toContain("read_file"); - expect(names).toContain("grep"); expect(names).toContain("write_file"); + expect(names).toContain("grep"); + expect(names).toContain("search_files"); + expect(names).toContain("web_fetch"); + expect(names).not.toContain("list_dir"); expect(names).not.toContain("mcp__linear__create_issue"); }); - test("with no activation, the array is byte-identical after an MCP tool is registered (cache prefix survives)", () => { + test("catalog is only search + web (banned shell substitutes)", () => { + expect([...CATALOG_TOOL_NAMES]).toEqual(["grep", "search_files", "web_fetch", "web_search"]); + }); + + test("wire array is byte-identical after an MCP tool is registered (cache prefix survives)", () => { const before = JSON.stringify(advertisedTools(registry)); const grown: ToolDefinition[] = [ ...registry, @@ -189,58 +204,25 @@ describe("advertisedTools", () => { expect(after).toBe(before); }); - test("the fixed built-in prefix order never changes, activated or not", () => { + test("fixed built-in prefix order never changes with registry order", () => { const forward = advertisedTools(registry).map((d) => d.name); const reversed = advertisedTools([...registry].reverse()).map((d) => d.name); expect(reversed).toEqual(forward); - - const withActivation = advertisedTools(registry, ["mcp__linear__create_issue"]).map((d) => d.name); - expect(withActivation.slice(0, forward.length)).toEqual(forward); - }); - - test("an activated MCP tool's full definition appears on the wire, appended after the fixed prefix", () => { - const names = advertisedTools(registry, ["mcp__linear__create_issue"]); - const linear = names.find((d) => d.name === "mcp__linear__create_issue"); - expect(linear).toBeDefined(); - expect(linear).toEqual(registry[3]); - // Appended, not interleaved: it lands after every fixed name. - const idx = names.findIndex((d) => d.name === "mcp__linear__create_issue"); - expect(idx).toBe(names.length - 1); - }); - - test("repeated activation of the same tool does not reorder or duplicate it", () => { - const once = advertisedTools(registry, ["mcp__linear__create_issue"]).map((d) => d.name); - const twice = advertisedTools(registry, ["mcp__linear__create_issue", "mcp__linear__create_issue"]).map( - (d) => d.name, - ); - expect(twice).toEqual(once); - expect(twice.filter((n) => n === "mcp__linear__create_issue")).toHaveLength(1); - }); - - test("multiple activations append in first-activation order regardless of registry order", () => { - const multi: ToolDefinition[] = [ - ...registry, - { name: "mcp__acme__do", description: "late", inputSchema: { type: "object", properties: {}, required: [] } }, - ]; - const names = advertisedTools(multi, ["mcp__acme__do", "mcp__linear__create_issue"]).map((d) => d.name); - const tailIdx = names.length - 2; - expect(names.slice(tailIdx)).toEqual(["mcp__acme__do", "mcp__linear__create_issue"]); }); - test("the built-in prefix is byte-identical across repeated turns of the same session", () => { - // Session-start availability is computed once and must never be - // re-evaluated per turn — simulate several turns by calling with the same - // captured prefix and confirm the wire array never drifts. + test("mid-session discovery never grows the wire — free-name only", () => { const prefix = advertisedToolNamesForSessionMode("orchestrator", { languageServerAvailable: true, }); - const turn1 = JSON.stringify(advertisedTools(registry, [], prefix)); - const turn2 = JSON.stringify(advertisedTools(registry, [], prefix)); - const turn3 = JSON.stringify(advertisedTools(registry, ["mcp__linear__create_issue"], prefix)); + const turn1 = JSON.stringify(advertisedTools(registry, prefix)); + const turn2 = JSON.stringify(advertisedTools(registry, prefix)); + const withMcpRegistered: ToolDefinition[] = [ + ...registry, + { name: "mcp__acme__do", description: "late", inputSchema: { type: "object", properties: {}, required: [] } }, + ]; + const turn3 = JSON.stringify(advertisedTools(withMcpRegistered, prefix)); expect(turn2).toBe(turn1); - // Growth from a mid-session discovery only appends — the prefix itself - // (everything before the activated tail) still matches turn 1 exactly. - expect(turn3.startsWith(turn1.slice(0, -1))).toBe(true); + expect(turn3).toBe(turn1); }); test("tool_search never returns an already-advertised built-in", () => { @@ -249,25 +231,3 @@ describe("advertisedTools", () => { } }); }); - -describe("createActivatedToolTracker", () => { - test("activate adds new names and reports a change", () => { - const tracker = createActivatedToolTracker(); - expect(tracker.activate(["mcp__linear__create_issue"])).toBe(true); - expect(tracker.list()).toEqual(["mcp__linear__create_issue"]); - }); - - test("re-activating an already-active name is a no-op — no reorder, no duplicate, no reported change", () => { - const tracker = createActivatedToolTracker(); - tracker.activate(["mcp__acme__do", "mcp__linear__create_issue"]); - expect(tracker.activate(["mcp__linear__create_issue"])).toBe(false); - expect(tracker.list()).toEqual(["mcp__acme__do", "mcp__linear__create_issue"]); - }); - - test("preserves first-activation order across separate calls", () => { - const tracker = createActivatedToolTracker(); - tracker.activate(["b"]); - tracker.activate(["a", "b", "c"]); - expect(tracker.list()).toEqual(["b", "a", "c"]); - }); -}); diff --git a/src/agent/tool-search.ts b/src/agent/tool-search.ts index 89608ac4a..000e5f5fb 100644 --- a/src/agent/tool-search.ts +++ b/src/agent/tool-search.ts @@ -6,53 +6,54 @@ import { type } from "arktype"; import type { SessionMode } from "../config/session-mode.js"; import { sessionModeEnablesSubAgents } from "../config/session-mode.js"; -// Tools whose full schema is always advertised to the model. Everything else is -// registered and dispatchable but discovered on demand via tool_search, keeping -// the per-turn context small. Shared by the system prompt and the advertised-set -// gate so the two never drift. +// Fixed wire tools (full schemas every turn). Everything else is registered for +// free-name dispatch and discovered via tool_search — the wire never grows mid- +// session (Search & Execute / provider tools-array cache). // -// `present` is deliberately absent: most sessions never render a view, and at -// 2,793 chars it is the second-largest schema on the wire. It stays fully -// dispatchable — the model finds it via tool_search when a session actually -// needs it. +// Design (minimal fixed wire + no thrash): +// CORE — file/shell loop (read/write/edit/shell) + product loop (ask/tasks/search/skills) +// + orchestrator spawn tools. Never put "nice to have" here. +// CATALOG — only capabilities whose shell substitutes are *hard-blocked* by the +// harness (bounded code search, SSRF-safe web). If the prompt or authz +// requires a tool, it must sit on the wire — deferred + "use X" is what +// made Grok 4.6 thrash on tool_search. +// +// Behind search (dispatchable by exact name): list_dir, lsp, present, MCP/plugins. export const CORE_TOOL_NAMES: readonly string[] = [ + // File + shell loop "read_file", "edit_file", - "lsp", + "write_file", "run_shell", + // Product loop "ask_operator", "manage_tasks", "tool_search", "use_skill", + // Multi-agent (orchestrator-only filter below) "search_agents", - // Multi-agent dispatch is a first-class loop capability — always advertised so - // the model can call task immediately after search_agents without a tool_search - // round-trip. Catalog-only placement left the model discovering profiles then - // failing on an unloaded task tool. "task", ]; const ORCHESTRATOR_ONLY_TOOL_NAMES: readonly string[] = ["search_agents", "task"]; -// Session-start facts that gate a core tool's advertisement. Each must be -// knowable once, before the first inference call, and must never change for -// the life of the session — the tools array is a provider cache prefix (see -// ADVERTISED_TOOL_NAMES below), so a value that could flip mid-session would -// force a re-prefill worse than the schema bytes it saves. +// Session-start facts that gate advertisement. Each must be knowable once before +// the first inference and stable for the session — the tools array is a provider +// cache prefix, so a flip mid-session re-prefills the whole request. +// +// `languageServerAvailable` is retained for callers that still detect LSP at +// boot; it no longer gates the wire (lsp is free-name / tool_search only). export type ToolAvailability = { - // Whether a language server was resolvable for this project at startup — - // not whether one currently responds. languageServerAvailable: boolean; }; export function coreToolNamesForSessionMode( mode: SessionMode, - availability: ToolAvailability, + _availability: ToolAvailability, ): readonly string[] { const orchestratorEnabled = sessionModeEnablesSubAgents(mode); return CORE_TOOL_NAMES.filter((name) => { if (!orchestratorEnabled && ORCHESTRATOR_ONLY_TOOL_NAMES.includes(name)) return false; - if (name === "lsp") return availability.languageServerAvailable; return true; }); } @@ -64,14 +65,15 @@ export function advertisedToolNamesForSessionMode( return [...coreToolNamesForSessionMode(mode, availability), ...CATALOG_TOOL_NAMES]; } -// Built-in file/search tools advertised alongside the core set. They carry full -// schemas on the wire so the model can call them directly; MCP tools are not -// listed at all — they are discovered blind via tool_search. +// Only tools that replace a *blocked* shell path. Do not grow this with +// convenience tools (list_dir, lsp, present) — those thrash when models +// tool_search for them after the prompt names them, and stay fine as free-name +// when the model actually needs them. export const CATALOG_TOOL_NAMES: readonly string[] = [ - "write_file", - "search_files", "grep", - "list_dir", + "search_files", + "web_fetch", + "web_search", ]; // The maximal set of built-in tools — every gate open — in a deterministic @@ -89,26 +91,18 @@ export const ADVERTISED_TOOL_NAMES: readonly string[] = [ ...CATALOG_TOOL_NAMES, ]; -// Project the live tool registry onto the advertised set: the fixed built-in -// prefix (its order never changes — this is what keeps the provider cache -// prefix stable) followed by session-activated tools (MCP or otherwise) in -// first-activation order. The wire array is byte-stable turn to turn until a -// discovery appends a new name, at which point it grows once and then holds -// steady again. `activated` is expected to already be deduped/ordered (see -// `createActivatedToolTracker`), but names are deduped again here defensively -// so a caller passing raw matches still can't reorder or duplicate an entry. +// Project the live tool registry onto the fixed advertised wire set. Membership +// and order come only from `builtInPrefix` — never from mid-session discovery. +// That keeps the provider tools-array prefix cache stable for the whole session +// (Search & Execute / free-name dispatch: catalog tools are callable by exact +// name through the registry without appearing here). export function advertisedTools( all: readonly ToolDefinition[], - activated: readonly string[] = [], builtInPrefix: readonly string[] = ADVERTISED_TOOL_NAMES, ): ToolDefinition[] { const byName = new Map(all.map((def) => [def.name, def])); const seen = new Set(); - const orderedNames = [ - ...builtInPrefix, - ...activated.filter((name) => !builtInPrefix.includes(name)), - ]; - return orderedNames.flatMap((name) => { + return builtInPrefix.flatMap((name) => { if (seen.has(name)) return []; seen.add(name); const def = byName.get(name); @@ -116,39 +110,14 @@ export function advertisedTools( }); } -// Tracks which non-built-in tool names the session has activated (via -// tool_search matches, or a director-side trigger like the lsp hint), in -// first-activation order. Backed by a Set, so re-activating an already-active -// name is a no-op — it neither reorders nor duplicates the entry. -export type ActivatedToolTracker = { - // Adds any new names and returns whether the set actually changed. - activate(names: readonly string[]): boolean; - list(): string[]; -}; - -export function createActivatedToolTracker(): ActivatedToolTracker { - const activeNames = new Set(); - return { - activate(names: readonly string[]): boolean { - let changed = false; - for (const name of names) { - if (!activeNames.has(name)) { - activeNames.add(name); - changed = true; - } - } - return changed; - }, - list(): string[] { - return [...activeNames]; - }, - }; -} - export const toolSearchDefinition: ToolDefinition = { name: "tool_search", description: - "Discover callable tools by capability. Most tools — file search, web access, and any connected integrations — are dispatchable but not advertised in the tools list. Call this with a short description of what you need (e.g. 'create a file', 'search the web', 'find files', 'issue tracker') to get the matching tools' names, descriptions, and input schemas. The returned tools are already callable — invoke them directly, no separate load step.", + "Discover plugin/MCP tools that are not listed under Tools (issue trackers, etc.). " + + "Returns exact names and compact input schemas. Call matches by exact name — they are " + + "already dispatchable; search does not load or promote them onto the wire. Do not use " + + "tool_search for file/shell/web/search work already listed under Tools, do not re-run " + + "for the same need, and do not narrate a call in prose instead of invoking the tool.", inputSchema: { type: "object", properties: { @@ -167,9 +136,20 @@ function tokenize(text: string): string[] { return text.toLowerCase().match(/[a-z0-9]+/g) ?? []; } +// Prefer Exa over first-party web tools when scores are close, then first-party +// over other MCP integrations, so a web query surfaces the hosted Exa path +// ahead of web_fetch / web_search and a long tail of unrelated mcp__* hits. +function preferenceBoost(name: string): number { + if (name.startsWith("mcp__exa__")) return 0.6; + if (name.startsWith("mcp__")) return 0; + return 0.3; +} + // A dependency-free lexical ranker over each tool's name + description. Exact name // token hits weigh most, then description token hits, then raw-substring matches // (so "linear" finds mcp__linear__* even though it is not a whole token there). +// Default limit is small on purpose: long result cards invite models to re-search +// and thrash instead of calling the top match. export function createToolIndex( getDefs: () => readonly ToolDefinition[], advertisedNames: readonly string[] = ADVERTISED_TOOL_NAMES, @@ -185,19 +165,27 @@ export function createToolIndex( else if ((def.description ?? "").toLowerCase().includes(token)) total += 0.25; } if (def.name.toLowerCase().includes(rawQuery)) total += 1; - return total; + // Preference only breaks ties among real matches — never invents a hit. + if (total <= 0) return 0; + return total + preferenceBoost(def.name); }; return { - search(query: string, limit = 8): string[] { + search(query: string, limit = 3): string[] { const rawQuery = query.toLowerCase().trim(); const queryTokens = tokenize(query); if (queryTokens.length === 0) return []; - return getDefs() + const ranked = getDefs() .filter((def) => !advertisedNames.includes(def.name)) .map((def) => ({ name: def.name, score: score(def, queryTokens, rawQuery) })) .filter((entry) => entry.score > 0) - .sort((a, b) => b.score - a.score) + .sort((a, b) => b.score - a.score); + if (ranked.length === 0) return []; + // Drop weak tail entries more than 2 points below the top score so a strong + // name hit does not drag along near-zero description matches. + const floor = ranked[0]!.score - 2; + return ranked + .filter((entry) => entry.score >= floor) .slice(0, limit) .map((entry) => entry.name); }, @@ -207,18 +195,13 @@ export function createToolIndex( export type ToolSearchDeps = { search: (query: string) => string[]; lookup: (name: string) => ToolDefinition | undefined; - // Make the matched tools' names part of the advertised wire set on the next - // inference. Every registered tool is already dispatchable via `run`, so this - // only affects what the model can see without an intervening tool_search. - promote: (names: string[]) => void; }; const ToolSearchArgs = type({ query: "string" }); // Render one discovered tool as name, description, and pretty-printed input -// schema. The schema is the load-bearing addition: MCP and other unadvertised -// tools never appear in the wire tools array, so this is the model's only view -// of their parameter names, types, and required fields. +// schema. Unadvertised tools never appear in the wire tools array, so this card +// (plus free-name dispatch) is how the model learns parameters and then executes. function renderToolCard(def: ToolDefinition | undefined, name: string): string { if (def === undefined) return `- ${name}`; const header = `- ${def.name}: ${def.description ?? ""}`; @@ -247,15 +230,14 @@ export function createToolSearchTool(deps: ToolSearchDeps): AgentTool { if (names.length === 0) { return `No tools matched "${query}". Try different keywords describing the capability.`; } - // Matches are promoted into the advertised set so the next inference - // declares them on the wire — required for strict providers (e.g. the grok - // Responses API) where a model cannot call a tool that was never declared. - // The tool result below still carries name, description, AND input schema - // so the model can shape arguments this same turn, before the promoted - // definition round-trips through the next infer call. - deps.promote(names); + // Search only. Execute is a free-name tool call against the registry — + // never grow the wire tools array (provider cache prefix must stay fixed). + // Cards carry schema so the model can shape arguments from history alone. const blocks = names.map((name) => renderToolCard(deps.lookup(name), name)); - return `These tools are available — you can call them now:\n\n${blocks.join("\n\n")}`; + return ( + `Matched tools (already dispatchable — call by exact name now; do not re-search):\n\n` + + `${blocks.join("\n\n")}` + ); }, }); } diff --git a/src/agent/tools.ts b/src/agent/tools.ts index 8af17b7c9..63fe60a2f 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -154,14 +154,12 @@ export type MCPConnectCallbacks = { export type AgentToolset = { // The mutable runner the agent dispatches through. Seeded with posix/web/LSP - // tools; MCP tools are added as servers connect. + // tools; MCP tools are added as servers connect. Full registry is free-name + // dispatchable; only the fixed advertised prefix is sent on the wire. dynamicRunner: DynamicToolRunner; // Connect configured MCP servers in the background. Resolves once every server // has either connected or failed; authorization waits are bounded by `signal`. connectMCP: (callbacks: MCPConnectCallbacks, signal?: AbortSignal) => Promise; - // Wire the callback the `tool_search` tool invokes to make matched tools - // advertised. Set by the runner once the director + reload loop exist. - setToolPromoter: (promote: (names: string[]) => void) => void; dispose: () => Promise; }; @@ -325,16 +323,14 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise void } = { promote: () => undefined }; + // tool_search ranks over the live runner (set just below). Matches stay off + // the wire; free-name dispatch runs them from the registry. let runnerRef: DynamicToolRunner | undefined; const toolIndex = createToolIndex(() => runnerRef?.currentDefinitions() ?? [], advertisedBuiltIns); baseTools.push( createToolSearchTool({ search: (query) => toolIndex.search(query), lookup: (name) => runnerRef?.currentDefinitions().find((d) => d.name === name), - promote: (names) => promoter.promote(names), }), ); @@ -400,9 +396,6 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise { - promoter.promote = promote; - }, dispose: async () => { for (const client of connectedClients) { permissionGate.unregisterMcpServer(client.serverName); diff --git a/src/director.test.ts b/src/director.test.ts index ad009043e..01862c90a 100644 --- a/src/director.test.ts +++ b/src/director.test.ts @@ -1,7 +1,7 @@ import { describe, test, expect } from "bun:test"; import { createChatDirector } from "./agent/director.js"; import { createAgentToolset } from "./agent/tools.js"; -import { advertisedTools, createActivatedToolTracker } from "./agent/tool-search.js"; +import { advertisedTools } from "./agent/tool-search.js"; import { createPermissionGate } from "./permission/gate.js"; import { COMPACTOR_KEEP_RECENT_TURNS, compactorNoOpFloor } from "./session/compactor.js"; import type { SessionMetadata, TaskBoundary } from "./session/compactor.js"; @@ -539,12 +539,10 @@ describe("updateToolDefinitions rewrites infer tools", () => { expect(inferToolNames(inferAction)).toContain("advance_workflow"); }); - // End-to-end: tool_search matches an MCP tool, the runner's promote wiring - // (mirrored here via createActivatedToolTracker + updateToolDefinitions) grows - // the wire set once with the tool's full definition, and it then holds steady. - // On a strict provider, a model can only call a tool - // that was actually declared on the wire, so promotion must land here. - test("a tool_search match is on the wire on the next turn, then the array holds stable", async () => { + // tool_search returns cards for matches but does not grow the advertised wire + // set — the provider cache prefix stays fixed for the session. Matches remain + // dispatchable via the registry by exact name without a wire append. + test("a tool_search match stays off the wire; the advertised array holds stable", async () => { const linearTool = { name: "mcp__linear__list_issues", description: "list issues", @@ -559,43 +557,36 @@ describe("updateToolDefinitions rewrites infer tools", () => { { kind: "string", definition: linearTool, handler: async () => "ok" }, ]); - const activated = createActivatedToolTracker(); - const computeAdvertised = (all: ReturnType) => - advertisedTools(all, activated.list()); + // Mirror production: advertised set is built-ins only; tool_search no longer + // activates matches onto the wire. const director = createChatDirector( "base-prompt", - computeAdvertised(toolset.dynamicRunner.currentDefinitions()), + advertisedTools(toolset.dynamicRunner.currentDefinitions()), { onTasksChange: () => {} }, ); - // Before discovery: the MCP tool is registered (dispatchable) but not wired. const before = await firstInferTools(director, makeMessageReceivedEvent("hello")); const beforeNames = (before as Array<{ name: string }>).map((t) => t.name); expect(beforeNames).not.toContain("mcp__linear__list_issues"); const beforeJson = JSON.stringify(before); - // Simulate the runner's promoteTools: tool_search matched this tool, so it - // is activated and the director's tool set is updated for the next infer. - activated.activate(["mcp__linear__list_issues"]); - director.updateToolDefinitions(computeAdvertised(toolset.dynamicRunner.currentDefinitions())); + // After a tool_search-style discovery turn, the wire array must be unchanged. + await director.decide( + makeInferenceDoneEvent([{ id: "ts", name: "tool_search", args: { query: "list issues" } }]), + mockState, + capabilitiesWithInferArgs, + ); + await director.decide(makeToolDoneEvent("ts"), mockState, capabilitiesWithInferArgs); const after = await firstInferTools(director, makeMessageReceivedEvent("continue")); - const afterTools = after as Array<{ name: string }>; - const afterNames = afterTools.map((t) => t.name); - expect(afterNames).toContain("mcp__linear__list_issues"); - // advance_workflow rides along separately (see withCurrentTools), appended - // after computeAdvertised's result every turn — strip it before comparing - // the fixed built-in prefix, which must survive untouched ahead of the - // newly appended MCP tool. - const beforePrefix = beforeNames.filter((n) => n !== "advance_workflow"); - const afterPrefix = afterNames.filter((n) => n !== "advance_workflow" && n !== "mcp__linear__list_issues"); - expect(afterPrefix).toEqual(beforePrefix); - expect(afterNames.indexOf("mcp__linear__list_issues")).toBe(beforePrefix.length); - - // A further turn with no new discovery stays byte-identical to `after`. - const stable = await firstInferTools(director, makeMessageReceivedEvent("keep going")); - expect(JSON.stringify(stable)).toBe(JSON.stringify(after)); - expect(JSON.stringify(after)).not.toBe(beforeJson); + const afterNames = (after as Array<{ name: string }>).map((t) => t.name); + expect(afterNames).not.toContain("mcp__linear__list_issues"); + expect(JSON.stringify(after)).toBe(beforeJson); + + // Registry still has the tool for dispatch even though it is not advertised. + expect( + toolset.dynamicRunner.currentDefinitions().some((d) => d.name === "mcp__linear__list_issues"), + ).toBe(true); await toolset.dispose(); }); diff --git a/src/exec/runner.ts b/src/exec/runner.ts index 8e92447f1..844fcc9e8 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -36,7 +36,6 @@ import { defaultPricingCachePath } from "../cost/pricing-fetcher.js"; import { advertisedToolNamesForSessionMode, advertisedTools, - createActivatedToolTracker, type ToolAvailability, } from "../agent/tool-search.js"; import { detectLanguageServerAvailable } from "../agent/lsp-availability.js"; @@ -408,13 +407,13 @@ export async function runExec(config: Config): Promise { }); const advertisedBuiltInPrefix = advertisedToolNamesForSessionMode(sessionMode, toolAvailability); - const activatedToolNames = createActivatedToolTracker(); - // Advertise then family-gate wire schemas (kimi gets a non-recursive present). + // Fixed wire prefix only (Search & Execute). Full registry is free-name + // dispatchable; tool_search never grows this array. const computeAdvertised = (all: readonly ToolDefinition[]): ToolDefinition[] => - normalizeToolDefinitionsForProvider( - advertisedTools(all, activatedToolNames.list(), advertisedBuiltInPrefix), - { providerName: config.providerName, model: config.model }, - ); + normalizeToolDefinitionsForProvider(advertisedTools(all, advertisedBuiltInPrefix), { + providerName: config.providerName, + model: config.model, + }); const directorHolder: { instance?: ReturnType } = {}; @@ -423,12 +422,6 @@ export async function runExec(config: Config): Promise { configSchema: type({}), factory: (_cfg, _env, agentCtx) => { const d = createChatDirector(agentCtx.systemPrompt, computeAdvertised([...agentCtx.toolDefinitions]), { - onActivateTools: (names) => { - if (!activatedToolNames.activate(names)) return; - directorHolder.instance?.updateToolDefinitions( - computeAdvertised(agentToolset.dynamicRunner.currentDefinitions()), - ); - }, inactivityTimeoutMs: config.inactivityTimeoutMs ?? 750_000, totalTimeoutMs: config.totalTimeoutMs, // Exec mode has no live task panel or task stdout output today (unlike diff --git a/src/permission/classify.ts b/src/permission/classify.ts index efbf1ca4b..fb6c86ac5 100644 --- a/src/permission/classify.ts +++ b/src/permission/classify.ts @@ -13,7 +13,8 @@ import type { RootsProvider } from "./worktree-roots.js"; // Read-only tools never need approval as long as they don't touch a restricted // path; they cannot change the workspace. `lsp` is included here even though -// it is activated dynamically mid-session (see director.ts onActivateTools) — +// it is discovered dynamically mid-session (see tool_search free-name dispatch) — + // hover/definition/reference lookups are as inert as a grep. `manage_tasks` is // included for a related but distinct reason: its handler (src/agent/tools.ts) // has no side effect of its own — the task list is mutated earlier, by the diff --git a/src/prompts.test.ts b/src/prompts.test.ts index 20797b89c..2d3df152a 100644 --- a/src/prompts.test.ts +++ b/src/prompts.test.ts @@ -57,7 +57,8 @@ test("harness facts state only the non-derivable tool and safety rules", () => { expect(facts).not.toMatch(/Use grep, search_files, and list_dir\.$/m); expect(facts).toContain("operator approval"); expect(facts).toContain("tool_search"); - expect(facts).toContain("plugins or integrations"); + expect(facts).toContain("free-name dispatch"); + expect(facts).toContain("fixed wire set"); expect(facts).toContain("slash-command steps"); expect(facts).toContain(".corbits/MEMORY.md"); expect(facts).toContain("Attached images are native multimodal input"); diff --git a/src/tui/dynamic-tool-runner.ts b/src/tui/dynamic-tool-runner.ts index 404fa9db8..f4789def6 100644 --- a/src/tui/dynamic-tool-runner.ts +++ b/src/tui/dynamic-tool-runner.ts @@ -12,8 +12,10 @@ import { stripTerminalControlSequences } from "../util/control-char-strip.js"; // createToolRunner freezes its name map at build time, which cannot accommodate // MCP servers that connect after the TUI has already started. This runner keeps // a mutable map and exposes `addTools` so late-connected servers' tools become -// dispatchable in the running session. `definitions` is a live getter, and the -// director advertises the current set on each inference (see updateToolDefinitions). +// free-name dispatchable in the running session (Search & Execute: the model +// discovers them via tool_search and calls by exact name; the wire tools array +// stays the fixed built-in prefix). `definitions` is a live getter for ranking +// and schema cards; the director only advertises the fixed prefix each turn. // // `watchdogConfig` is read on every run so Settings toggles (timeouts, // waitForApproval) take effect on the next tool call without rebuilding tools. diff --git a/src/tui/runner.ts b/src/tui/runner.ts index def767213..50daaf864 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -125,7 +125,6 @@ import { contextTokensFromUsage } from "../provider/context-window.js"; import { advertisedToolNamesForSessionMode, advertisedTools, - createActivatedToolTracker, type ToolAvailability, } from "../agent/tool-search.js"; import { detectLanguageServerAvailable } from "../agent/lsp-availability.js"; @@ -1279,23 +1278,23 @@ export async function runTUI(initialConfig: Config): Promise { }); workflowControllerHolder.instance = workflowController; - // Dynamic tool discovery: the runner registers every tool (built-in + MCP) for - // dispatch but advertises only the fixed built-in prefix plus whatever the - // session has activated so far (via tool_search matches, promoted below). - // The prefix's membership and order never change, so it alone keeps the - // provider cache prefix stable; activated names append once, in first- - // activation order, and then hold steady until the next discovery. Strict - // providers (grok Responses, codex-responses, OpenAI-style) refuse to call a - // tool that was never declared on the wire, so an MCP tool must be promoted - // here before the model can actually invoke it — merely being dispatchable in - // the runner is not enough on those providers. - const activatedToolNames = createActivatedToolTracker(); - // Advertise then family-gate wire schemas (kimi gets a non-recursive present). + // Search & Execute (fixed wire + free-name dispatch): every tool is registered + // for dispatch (built-in + MCP), but the wire tools array is only the fixed + // built-in prefix. Membership and order never change for the session, so the + // provider tools-array cache prefix stays hot. tool_search returns schema cards + // into history; the model then calls matched tools by exact name. The runner + // dispatches any registered name — nothing is "promoted" onto the wire. + // + // Note: some strict providers refuse tool calls that were never declared on + // the wire. Free-name still works for any provider that emits the call (and + // for deferred built-ins the model invents by name). Growing the wire mid- + // session for MCP was intentionally removed so thrashy models cannot re-prefill + // the tools array every discovery. const computeAdvertised = (all: readonly ToolDefinition[]): ToolDefinition[] => - normalizeToolDefinitionsForProvider( - advertisedTools(all, activatedToolNames.list(), advertisedBuiltInPrefix), - { providerName: config.providerName, model: config.model }, - ); + normalizeToolDefinitionsForProvider(advertisedTools(all, advertisedBuiltInPrefix), { + providerName: config.providerName, + model: config.model, + }); // Reload, interrupt, compaction continuation, and proxy deliver share one queue // so a rebuild never races an in-flight deliver. @@ -1318,7 +1317,6 @@ export async function runTUI(initialConfig: Config): Promise { configSchema: type({}), factory: (_config, _env, agentCtx) => { const d = createChatDirector(agentCtx.systemPrompt, computeAdvertised([...agentCtx.toolDefinitions]), { - onActivateTools: (names) => promoteTools(names), inactivityTimeoutMs: config.inactivityTimeoutMs ?? 750_000, totalTimeoutMs: config.totalTimeoutMs, onTasksChange: (tasks) => emitter.emit("tasks", tasks), @@ -1591,21 +1589,6 @@ export async function runTUI(initialConfig: Config): Promise { }); }; - // tool_search (and contextual triggers, e.g. the lsp hint) promote tools into - // the advertised set. Advertising takes effect on the next infer; a reload is - // scheduled so a newly connected MCP tool also becomes dispatchable after a - // rebuild (built-in tools are already dispatchable, so promoting them alone - // needs no reload, but the reload is a cheap no-op in that case). - const promoteTools = (names: string[]): void => { - if (!activatedToolNames.activate(names)) return; - directorHolder.instance?.updateToolDefinitions( - computeAdvertised(toolset.dynamicRunner.currentDefinitions()), - ); - pendingReload = true; - reloadIfIdle(); - }; - toolset.setToolPromoter(promoteTools); - // The active Codex source, tracked whenever a "codex/" source is // selected so its access token can be refreshed before each send. Seeded from // config when the session starts on a Codex profile (buildAgent sets that @@ -2483,9 +2466,9 @@ export async function runTUI(initialConfig: Config): Promise { void persistRunSnapshot("running"); } }, - // MCP tools register for dispatch but stay unadvertised (blind) until - // tool_search promotes them, so a fresh connection never grows the wire - // set on its own — only a subsequent discovery does. + // MCP tools register for free-name dispatch; the wire set stays the fixed + // built-in prefix (Search & Execute). A connection may still force a + // director refresh so the registry is visible to tool_search ranking. onToolsChanged: (definitions) => directorHolder.instance?.updateToolDefinitions(computeAdvertised(definitions)), },