From 9f926ebed231f52168028ec1ec7680f918fb0421 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 29 Sep 2026 13:29:34 -0700 Subject: [PATCH 1/3] feat(tools): promote discovered tools only on execute Search returns cards only. A call declares that one name, then dispatches. --- src/agent/tool-search.test.ts | 260 +++++++++++++++++++++++++-- src/agent/tool-search.ts | 86 ++++++--- src/agent/tools.ts | 22 ++- src/exec/runner.test.ts | 54 ++++-- src/exec/runner.ts | 14 +- src/session/assemble-runtime.test.ts | 22 ++- src/session/assemble-runtime.ts | 44 +++-- src/tui/dynamic-tool-runner.test.ts | 26 ++- src/tui/dynamic-tool-runner.ts | 61 ++++--- src/tui/mcp-view.test.ts | 2 +- src/tui/runner/exit.ts | 11 +- src/tui/runner/session.ts | 23 +-- src/tui/runner/state.ts | 5 + 13 files changed, 490 insertions(+), 140 deletions(-) diff --git a/src/agent/tool-search.test.ts b/src/agent/tool-search.test.ts index e7a2866ea..95bd9fb0a 100644 --- a/src/agent/tool-search.test.ts +++ b/src/agent/tool-search.test.ts @@ -1,5 +1,8 @@ import { describe, test, expect, jest } from "bun:test"; +import type { AgentTool } from "@intx/agent"; import type { ToolDefinition } from "@intx/types/runtime"; +import { createAdvertisedToolset } from "../session/assemble-runtime.js"; +import { createDynamicToolRunner } from "../tui/dynamic-tool-runner.js"; import { createToolIndex, createToolSearchTool, @@ -12,6 +15,9 @@ import { TOOL_SEARCH_PENDING_WAIT_MS, TOOL_SEARCH_RECONNECT_WAIT_MS, TOOL_SEARCH_DESC_MAX, + TOOL_SEARCH_MAX_RESULTS, + TOOL_SEARCH_LIMIT_MAX, + toolSearchDefinition, type ToolAvailability, } from "./tool-search.js"; @@ -96,6 +102,14 @@ const linearCatalog: ToolDefinition[] = [ const linearIndex = createToolIndex(() => linearCatalog); +const wideCatalog: ToolDefinition[] = Array.from({ length: 12 }, (_, i) => + mcpDef( + `mcp__linear__op_${String(i).padStart(2, "0")}`, + "Linear issue operation", + ), +); +const wideIndex = createToolIndex(() => wideCatalog); + describe("createToolIndex", () => { test("ranks a name-token match above a description-only match", () => { const results = index.search("present"); @@ -272,10 +286,20 @@ describe("createToolIndex", () => { test("caps a broad linear-issue query to a handful of top matches", () => { const ranked = linearIndex.search("linear issue"); expect(ranked.length).toBeGreaterThan(1); - expect(ranked.length).toBeLessThanOrEqual(5); + expect(ranked.length).toBeLessThanOrEqual(TOOL_SEARCH_MAX_RESULTS); expect(ranked.every((name) => name.includes("linear"))).toBe(true); }); + test("default search returns at most 5 equally-scoring matches", () => { + const ranked = wideIndex.search("linear issue"); + expect(ranked).toHaveLength(TOOL_SEARCH_MAX_RESULTS); + }); + + test("an explicit limit overrides the default cap", () => { + expect(wideIndex.search("linear issue", 3)).toHaveLength(3); + expect(wideIndex.search("linear issue", 8)).toHaveLength(8); + }); + test("a specific save query returns one or two tools, not the whole family", () => { const ranked = linearIndex.search("linear issue save"); expect(ranked).toContain("mcp__linear__save_issue"); @@ -292,6 +316,13 @@ function call( return tool.handler(args, new AbortController().signal); } +function listedToolNames(out: string): string[] { + return out + .split("\n") + .filter((line) => line.startsWith("- ")) + .map((line) => line.slice(2).split(":")[0]!.trim()); +} + async function flushMicrotasks(rounds = 100): Promise { for (let i = 0; i < rounds; i++) await Promise.resolve(); } @@ -302,24 +333,117 @@ async function advanceAndFlush(ms: number): Promise { } describe("createToolSearchTool", () => { - test("promotes matches and lists them back to the model", async () => { - const promoted: string[] = []; + test("lists matching cards and does not promote", 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: "render layout" }); - expect(promoted).toContain("present"); expect(out).toContain("present"); expect(out).toContain("layout"); + expect(out).not.toMatch(/promoted/i); + expect( + advertisedTools(defs).map((d) => d.name), + ).not.toContain("present"); + }); + + test("default tool_search returns at most 5 matches and does not promote them", async () => { + const tool = createToolSearchTool({ + search: (q, limit) => wideIndex.search(q, limit), + lookup: (name) => wideCatalog.find((d) => d.name === name), + }); + const before = advertisedTools(wideCatalog).map((d) => d.name); + const out = await call(tool, { query: "linear issue" }); + const listed = listedToolNames(out); + expect(listed).toHaveLength(TOOL_SEARCH_MAX_RESULTS); + expect(listed.length).toBeLessThan(wideCatalog.length); + expect(advertisedTools(wideCatalog).map((d) => d.name)).toEqual(before); + expect(listed.every((name) => !before.includes(name))).toBe(true); + }); + + test("limit 3 overrides the default cap", async () => { + const tool = createToolSearchTool({ + search: (q, limit) => wideIndex.search(q, limit), + lookup: (name) => wideCatalog.find((d) => d.name === name), + }); + const listed = listedToolNames( + await call(tool, { query: "linear issue", limit: 3 }), + ); + expect(listed).toHaveLength(3); + }); + + test("limit above the default returns the wider set", async () => { + const tool = createToolSearchTool({ + search: (q, limit) => wideIndex.search(q, limit), + lookup: (name) => wideCatalog.find((d) => d.name === name), + }); + const listed = listedToolNames( + await call(tool, { query: "linear issue", limit: 8 }), + ); + expect(listed).toHaveLength(8); + }); + + test("limit above the hard cap clamps to 20", async () => { + const hugeCatalog: ToolDefinition[] = Array.from({ length: 25 }, (_, i) => + mcpDef( + `mcp__linear__huge_${String(i).padStart(2, "0")}`, + "Linear issue operation", + ), + ); + const hugeIndex = createToolIndex(() => hugeCatalog); + const tool = createToolSearchTool({ + search: (q, limit) => hugeIndex.search(q, limit), + lookup: (name) => hugeCatalog.find((d) => d.name === name), + }); + const listed = listedToolNames( + await call(tool, { query: "linear issue", limit: 100 }), + ); + expect(listed).toHaveLength(TOOL_SEARCH_LIMIT_MAX); + expect(toolSearchDefinition.inputSchema).toMatchObject({ + properties: { + limit: { + description: expect.stringContaining("hard cap 20"), + }, + }, + }); + }); + + test("missing or invalid limit falls back to 5", async () => { + const makeTool = () => + createToolSearchTool({ + search: (q, limit) => wideIndex.search(q, limit), + lookup: (name) => wideCatalog.find((d) => d.name === name), + }); + for (const args of [ + { query: "linear issue" }, + { query: "linear issue", limit: "nope" }, + { query: "linear issue", limit: 0 }, + { query: "linear issue", limit: -3 }, + { query: "linear issue", limit: Number.NaN }, + ] as Record[]) { + const listed = listedToolNames(await call(makeTool(), args)); + expect(listed).toHaveLength(TOOL_SEARCH_MAX_RESULTS); + } + }); + + test("returned names are a handful, not the whole catalog", async () => { + const tool = createToolSearchTool({ + search: (q, limit) => wideIndex.search(q, limit), + lookup: (name) => wideCatalog.find((d) => d.name === name), + }); + const listed = listedToolNames( + await call(tool, { query: "linear issue", limit: 3 }), + ); + const catalogNames = wideCatalog.map((d) => d.name); + expect(listed).toHaveLength(3); + expect(catalogNames.every((name) => listed.includes(name))).toBe(false); + expect(listed.every((name) => catalogNames.includes(name))).toBe(true); }); test("returns name and capped description without a full input schema", async () => { const tool = createToolSearchTool({ search: (q) => linearIndex.search(q), lookup: (name) => linearCatalog.find((d) => d.name === name), - promote: () => undefined, }); const out = await call(tool, { query: "linear issue" }); expect(out).toContain("mcp__linear__"); @@ -334,14 +458,14 @@ describe("createToolSearchTool", () => { expect(line.length).toBeLessThanOrEqual(200); } expect(out).not.toMatch(/call them now/i); - expect(out).toMatch(/next (turn|infer)/i); + expect(out).not.toMatch(/promoted onto/i); + expect(out).toMatch(/call a listed name/i); }); test("a unique description query hits the long card so the cap is exercised", async () => { const tool = createToolSearchTool({ search: (q) => linearIndex.search(q), lookup: (name) => linearCatalog.find((d) => d.name === name), - promote: () => undefined, }); const ranked = linearIndex.search("end-marker"); expect(ranked).toContain("mcp__linear__verbose_issue"); @@ -362,7 +486,6 @@ describe("createToolSearchTool", () => { const tool = createToolSearchTool({ search: () => [], lookup: () => undefined, - promote: () => undefined, }); expect(await call(tool, { query: " " })).toContain("Error:"); }); @@ -376,7 +499,6 @@ describe("createToolSearchTool", () => { const tool = createToolSearchTool({ search: (query) => createToolIndex(() => live).search(query), lookup: (name) => live.find((def) => def.name === name), - promote: () => undefined, awaitPendingConnections: async (timeoutMs?: number) => { await Promise.race([ connected, @@ -401,7 +523,6 @@ describe("createToolSearchTool", () => { const tool = createToolSearchTool({ search: () => [], lookup: () => undefined, - promote: () => undefined, awaitPendingConnections: () => new Promise(() => { // Never settles: simulates a hung authorization handshake. @@ -421,7 +542,6 @@ describe("createToolSearchTool", () => { const tool = createToolSearchTool({ search: () => [], lookup: () => undefined, - promote: () => undefined, awaitPendingConnections: async () => 2, }); const out = await call(tool, { query: "linear" }); @@ -434,7 +554,6 @@ describe("createToolSearchTool", () => { const tool = createToolSearchTool({ search: () => [], lookup: () => undefined, - promote: () => undefined, awaitPendingConnections: async () => 0, }); const out = await call(tool, { query: "nonsense" }); @@ -450,7 +569,6 @@ describe("createToolSearchTool", () => { const tool = createToolSearchTool({ search: () => [], lookup: () => undefined, - promote: () => undefined, awaitPendingConnections: async () => 0, hasReconnectingMatch: () => { matchChecks += 1; @@ -468,7 +586,6 @@ describe("createToolSearchTool", () => { const live: ToolDefinition[] = []; const timeouts: (number | undefined)[] = []; const matchQueries: string[] = []; - const promoted: string[] = []; let searches = 0; const tool = createToolSearchTool({ search: (query) => { @@ -476,7 +593,6 @@ describe("createToolSearchTool", () => { return createToolIndex(() => live).search(query); }, lookup: (name) => live.find((def) => def.name === name), - promote: (names) => promoted.push(...names), awaitPendingConnections: async (timeoutMs?: number) => { timeouts.push(timeoutMs); await new Promise((resolve) => setTimeout(resolve, timeoutMs ?? 0)); @@ -499,7 +615,6 @@ describe("createToolSearchTool", () => { await advanceAndFlush(TOOL_SEARCH_RECONNECT_WAIT_MS); const out = await pending; expect(out).toContain("mcp__linear__create_issue"); - expect(promoted).toContain("mcp__linear__create_issue"); expect(timeouts).toEqual([ TOOL_SEARCH_PENDING_WAIT_MS, TOOL_SEARCH_RECONNECT_WAIT_MS, @@ -523,7 +638,6 @@ describe("createToolSearchTool", () => { return []; }, lookup: () => undefined, - promote: () => undefined, awaitPendingConnections: async (timeoutMs?: number) => { timeouts.push(timeoutMs); await new Promise((resolve) => setTimeout(resolve, timeoutMs ?? 0)); @@ -551,6 +665,116 @@ describe("createToolSearchTool", () => { }); }); +describe("promote-on-execute", () => { + const stringTool = (name: string, reply: string, description: string): AgentTool => ({ + kind: "string", + definition: { + name, + description, + inputSchema: { type: "object", properties: {}, required: [] }, + }, + handler: async () => reply, + }); + + function wirePromoteOnExecute() { + const tools = [ + ...wideCatalog.map((d) => stringTool(d.name, `ran:${d.name}`, d.description)), + stringTool( + "mcp__never__searched", + "ran:never", + "A tool the model never searched for", + ), + ]; + const runner = createDynamicToolRunner(tools); + const advertised = createAdvertisedToolset({ + sessionMode: "orchestrator", + toolAvailability: { languageServerAvailable: false }, + getProvider: () => ({ providerName: "test", model: "test" }), + }); + const promoted: string[] = []; + runner.setCallGate((name) => advertised.isAdvertised(name), { + isActivated: (name) => advertised.activated.has(name), + }); + runner.setOnUndeclaredCall((name) => { + promoted.push(name); + advertised.activated.activate([name]); + advertised.flushPromotions(); + }); + const search = createToolSearchTool({ + search: (q, limit) => wideIndex.search(q, limit), + lookup: (name) => + runner.currentDefinitions().find((d) => d.name === name), + }); + return { runner, advertised, promoted, search }; + } + + async function dispatch( + runner: ReturnType, + name: string, + ) { + return runner.run( + { id: name, name, arguments: {} }, + new AbortController().signal, + ); + } + + function advertisedNames( + advertised: ReturnType, + runner: ReturnType, + ): string[] { + return advertised + .computeAdvertised(runner.currentDefinitions()) + .map((d) => d.name); + } + + test("a tool_search call does not change the advertised set", async () => { + const { advertised, promoted, search, runner } = wirePromoteOnExecute(); + const before = advertisedNames(advertised, runner); + if (search.kind !== "string") throw new Error("expected string tool"); + const out = await search.handler( + { query: "linear issue", limit: 3 }, + new AbortController().signal, + ); + expect(listedToolNames(out).length).toBe(3); + expect(promoted).toEqual([]); + expect(advertised.activated.list()).toEqual([]); + expect(advertisedNames(advertised, runner)).toEqual(before); + }); + + test("a subsequent call to a searched name promotes only that name", async () => { + const { advertised, promoted, search, runner } = wirePromoteOnExecute(); + if (search.kind !== "string") throw new Error("expected string tool"); + const listed = listedToolNames( + await search.handler( + { query: "linear issue", limit: 3 }, + new AbortController().signal, + ), + ); + expect(listed).toHaveLength(3); + expect(promoted).toEqual([]); + const called = listed[0]!; + const others = listed.slice(1); + const result = await dispatch(runner, called); + expect(result.content).toBe(`ran:${called}`); + expect(result.isError).toBeUndefined(); + expect(promoted).toEqual([called]); + const names = advertisedNames(advertised, runner); + expect(names).toContain(called); + for (const other of others) { + expect(names).not.toContain(other); + } + }); + + test("a never-searched undeclared name still promote-on-execute", async () => { + const { advertised, promoted, runner } = wirePromoteOnExecute(); + const result = await dispatch(runner, "mcp__never__searched"); + expect(result.content).toBe("ran:never"); + expect(result.isError).toBeUndefined(); + expect(promoted).toEqual(["mcp__never__searched"]); + expect(advertisedNames(advertised, runner)).toContain("mcp__never__searched"); + }); +}); + describe("advertisedTools", () => { const registry: ToolDefinition[] = [ { diff --git a/src/agent/tool-search.ts b/src/agent/tool-search.ts index 2125ffd54..8f49233e7 100644 --- a/src/agent/tool-search.ts +++ b/src/agent/tool-search.ts @@ -15,8 +15,9 @@ import { advertisedToolName, projectToolDefinition } from "./tool-aliases.js"; import { canonicalToolName } from "./canonical-tool-name.js"; // Tools whose full schema is always advertised to the model. Everything else is -// registered but discovered on demand via tool_search, which promotes matches -// onto the call gate and the next infer's wire set. Shared by the system prompt +// registered but discovered on demand via tool_search, which returns name + +// description cards only. A found tool's schema joins the wire when the model +// actually calls it (promote-on-execute). Shared by the system prompt // and the advertised-set gate so the two never drift. // // `present` is deliberately absent: most sessions never render a view, and at @@ -202,9 +203,9 @@ 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. +// promote-on-execute of a called name, 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 interface ActivatedToolTracker { // Adds any new names and returns whether the set actually changed. activate(names: readonly string[]): boolean; @@ -243,7 +244,7 @@ export function createActivatedToolTracker(): ActivatedToolTracker { export const toolSearchDefinition: ToolDefinition = { name: "tool_search", description: - "Discover callable tools by capability. Most tools — MCP servers, present, and other integrations — are not on the wire until this search promotes them onto the next inference. Core tools (read, bash, web_fetch, web_search, spawn_agent, …) are already on the wire — do not tool_search for them. wait_agents is mounted on exec-primary runs only, so it is not on the wire elsewhere and this search cannot promote it there. Call this with a short description of what you need (e.g. 'issue tracker', 'render layout', 'granola notes') to get a ranked handful of matching names and short descriptions. Matched tools join the next inference tool list — call them on the next turn, not from this result.", + "Discover callable tools by capability. Most tools — MCP servers, present, and other integrations — are found here as name + description cards; their schemas join the wire when you actually call them. Core tools (read, bash, web_fetch, web_search, spawn_agent, …) are already on the wire — do not tool_search for them. wait_agents is mounted on exec-primary runs only, so it is not on the wire elsewhere and this search cannot surface it there. Call this with a short description of what you need (e.g. 'issue tracker', 'render layout', 'granola notes') to get a ranked handful of matching names and short descriptions (default 5, overridable via limit, max 20). Search does not add schemas to the tool list — call a returned name to use it.", inputSchema: { type: "object", properties: { @@ -251,6 +252,11 @@ export const toolSearchDefinition: ToolDefinition = { type: "string", description: "A short description of the capability you need.", }, + limit: { + type: "number", + description: + "Max matches to return (default 5, hard cap 20). Override when you need a wider set.", + }, }, required: ["query"], }, @@ -263,8 +269,9 @@ export interface ToolIndex { // Rank registered tools against name + description via the shared lexical // scorer. Already-advertised tools are always callable so they never rank; -// the allow list (when set) keeps search from promoting outside it. +// the allow list (when set) keeps search from listing tools outside it. export const TOOL_SEARCH_MAX_RESULTS = 5; +export const TOOL_SEARCH_LIMIT_MAX = 20; export const TOOL_SEARCH_SCORE_RATIO = 0.75; export const TOOL_SEARCH_DESC_MAX = 160; @@ -272,7 +279,7 @@ export function createToolIndex( getDefs: () => readonly ToolDefinition[], advertisedNames: readonly string[] = ADVERTISED_TOOL_NAMES, // Closed allow list (exec director overlays): when set, the index only - // surfaces allowed tools so search cannot promote outside the allow. + // surfaces allowed tools so search cannot list names outside the allow. allow?: readonly string[] | undefined, ): ToolIndex { const score = ( @@ -314,10 +321,8 @@ export function createToolIndex( } export interface ToolSearchDeps { - search: (query: string) => string[]; + search: (query: string, limit?: number) => string[]; lookup: (name: string) => ToolDefinition | undefined; - // Promote matches onto the call gate and the next infer's wire set. - promote: (names: string[]) => void; // Resolves to the remaining in-flight MCP handshake count after waiting up // to `timeoutMs`. The toolset bounds its own wait; the handler re-races // below so even a stuck dependency can never hang the call. Omitted callers @@ -347,7 +352,39 @@ export const TOOL_SEARCH_PENDING_WAIT_MS = 1_000; // out-of-band authorization. export const TOOL_SEARCH_RECONNECT_WAIT_MS = 500; -const ToolSearchArgs = type({ query: "string" }); +const ToolSearchArgs = type({ + query: "string", + "limit?": "number", +}); + +function resolveSearchLimit(value: number | undefined): number { + if (value === undefined || !Number.isFinite(value)) { + return TOOL_SEARCH_MAX_RESULTS; + } + const n = Math.trunc(value); + if (n < 1) return TOOL_SEARCH_MAX_RESULTS; + return Math.min(n, TOOL_SEARCH_LIMIT_MAX); +} + +function parseToolSearchArgs( + rawArgs: Record, +): { error: string } | { query: string; limit: number } { + const parsed = ToolSearchArgs(rawArgs); + const queryRaw = + parsed instanceof type.errors ? rawArgs.query : parsed.query; + if (typeof queryRaw !== "string") { + return { error: "Error: tool_search requires query (string)." }; + } + const query = queryRaw.trim(); + if (query.length === 0) { + return { error: "Error: tool_search requires a non-empty query." }; + } + const limit = + parsed instanceof type.errors + ? TOOL_SEARCH_MAX_RESULTS + : resolveSearchLimit(parsed.limit); + return { query, limit }; +} function capDescription(text: string, max = TOOL_SEARCH_DESC_MAX): string { const oneLine = text.replace(/\s+/g, " ").trim(); @@ -384,14 +421,10 @@ export function createToolSearchTool(deps: ToolSearchDeps): AgentTool { return stringTool({ definition: toolSearchDefinition, handler: async (rawArgs: Record): Promise => { - const parsed = ToolSearchArgs(rawArgs); - if (parsed instanceof type.errors) { - return "Error: tool_search requires query (string)."; - } - const query = parsed.query.trim(); - if (query.length === 0) - return "Error: tool_search requires a non-empty query."; - let names = deps.search(query); + const parsed = parseToolSearchArgs(rawArgs); + if ("error" in parsed) return parsed.error; + const { query, limit } = parsed; + let names = deps.search(query, limit); if (names.length === 0 && deps.awaitPendingConnections !== undefined) { // Tier 1 — miss while connectors start up: wait briefly, then // re-search so late-mounting tools land. The race bounds even a stuck @@ -400,7 +433,7 @@ export function createToolSearchTool(deps: ToolSearchDeps): AgentTool { deps.awaitPendingConnections, deps.pendingWaitMs ?? TOOL_SEARCH_PENDING_WAIT_MS, ); - names = deps.search(query); + names = deps.search(query, limit); if ( names.length === 0 && (stillPending ?? 1) > 0 && @@ -414,7 +447,7 @@ export function createToolSearchTool(deps: ToolSearchDeps): AgentTool { deps.awaitPendingConnections, TOOL_SEARCH_RECONNECT_WAIT_MS, ); - names = deps.search(query); + names = deps.search(query, limit); } if (names.length === 0 && (stillPending ?? 1) > 0) { const detail = @@ -429,14 +462,13 @@ export function createToolSearchTool(deps: ToolSearchDeps): AgentTool { if (names.length === 0) { return `No tools matched "${query}". Try different keywords describing the capability.`; } - // Matches open on the call gate and join the next infer's wire set - // (promoter flushes at activate). The card is names + capped - // descriptions only — full schemas ride the next tools array. - deps.promote(names); + // Cards only — name + capped description. Search does not open the call + // gate or grow the tools array; promote-on-execute declares a name when + // the model actually calls it. const blocks = names.map((name) => renderToolCard(deps.lookup(name), name), ); - return `Promoted onto the next inference tool list. Call them on the next turn:\n\n${blocks.join("\n")}`; + return `Matching tools — call a listed name to use it:\n\n${blocks.join("\n")}`; }, }); } diff --git a/src/agent/tools.ts b/src/agent/tools.ts index 46c308004..cb84630fa 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -291,7 +291,7 @@ export interface AgentToolsetArgs { /** * Closed allow list (exec director overlays). tool_search is mounted only * when the allow includes it, and the search index only surfaces allowed - * tools so search cannot promote outside the allow. Omit for the product + * tools so search cannot list names outside the allow. Omit for the product * default (tool_search mounted, index over the live registry). */ toolSearchAllow?: readonly string[]; @@ -378,8 +378,9 @@ export interface AgentToolset { // Catalog unshadow can change local → global/none without rebuilding the // toolset; connectOne reads this on every late connect. setMcpServersSource: (source: "local" | "global" | "none") => void; - // Wire the callback the `tool_search` tool invokes to make matched tools - // advertised. Set by the runner once the director + reload loop exist. + // Wire the session promoter used for promote-on-execute (a registered + // undeclared call declares that one name, then dispatches). Set by the + // runner once the director + reload loop exist. setToolPromoter: (promote: (names: string[]) => void) => void; // Session-start skill snapshot shared with the prompt listing. skills: SkillSummary[]; @@ -782,11 +783,9 @@ export async function createAgentToolset( }), ]; - // tool_search ranks over the live runner (set just below) and promotes matches - // through a holder the runner wires up once its advertise/reload loop exists. - const promoter: { promote: (names: string[]) => void } = { - promote: () => undefined, - }; + // tool_search ranks over the live runner (set just below). Matches stay + // cards-only; the session promoter (wired once advertise/reload exists) + // declares a name when the model actually calls it. const runnerHolder: { current?: DynamicToolRunner } = {}; const toolIndex = createToolIndex( () => runnerHolder.current?.currentDefinitions() ?? [], @@ -795,19 +794,18 @@ export async function createAgentToolset( ); // Closed exec allow lists omit tool_search itself (leaf posture); when the // allow excludes it the tool is never mounted, so there is nothing to - // search with and nothing the promoter can activate. + // search with. if ( args.toolSearchAllow === undefined || args.toolSearchAllow.includes(toolSearchDefinition.name) ) { baseTools.push( createToolSearchTool({ - search: (query) => toolIndex.search(query), + search: (query, limit) => toolIndex.search(query, limit), lookup: (name) => runnerHolder.current ?.currentDefinitions() .find((d) => d.name === name), - promote: (names) => promoter.promote(names), // Misses wait briefly for in-flight MCP handshakes (bounded, so hung // OAuth cannot hang the call) and re-search before answering. Reads the // connection map live — declared below, populated by the time any @@ -1608,7 +1606,7 @@ export async function createAgentToolset( mcpServersSource = source; }, setToolPromoter: (promote) => { - promoter.promote = promote; + dynamicRunner.setOnUndeclaredCall((name) => promote([name])); }, skills, ...(fleetRecords !== undefined ? { fleetRecords } : {}), diff --git a/src/exec/runner.test.ts b/src/exec/runner.test.ts index 62d37a3cd..6ca803900 100644 --- a/src/exec/runner.test.ts +++ b/src/exec/runner.test.ts @@ -672,12 +672,12 @@ describe("exec tool call gate and promoter", () => { flushPromotions(); }, }); + runner.setOnUndeclaredCall((name) => promote([name])); const search = createToolSearchTool({ search: (query) => createToolIndex(() => runner.currentDefinitions()).search(query), lookup: (name) => runner.currentDefinitions().find((d) => d.name === name), - promote, }); return { runner, @@ -698,41 +698,57 @@ describe("exec tool call gate and promoter", () => { ); } - test("tool_search then MCP dispatch with the gate on", async () => { + test("search does not change the advertised set", async () => { const { runner, search, persistCount, computeAdvertised } = wireExecDiscovery(); - const blocked = await dispatch(runner, "mcp__linear__save_issue"); - expect(blocked.isError).toBe(true); - expect(blocked.content).toContain("tool_search"); + const before = computeAdvertised(runner.currentDefinitions()).map( + (d) => d.name, + ); + expect(before).not.toContain("mcp__linear__save_issue"); if (search.kind !== "string") throw new Error("expected string tool"); await search.handler({ query: "linear" }, new AbortController().signal); - expect(persistCount()).toBe(1); - - const allowed = await dispatch(runner, "mcp__linear__save_issue"); - expect(allowed.content).toBe("saved"); - expect(allowed.isError).toBeUndefined(); + expect(persistCount()).toBe(0); expect( computeAdvertised(runner.currentDefinitions()).map((d) => d.name), - ).toContain("mcp__linear__save_issue"); + ).toEqual(before); }); - test("present and plugin names pass the gate after tool_search promote", async () => { - const { runner, search } = wireExecDiscovery(); - expect((await dispatch(runner, "present")).isError).toBe(true); - expect((await dispatch(runner, "plugin__notes__save")).isError).toBe(true); - + test("a subsequent call to a searched-but-not-yet-declared name promotes only that name", async () => { + const { runner, search, computeAdvertised } = wireExecDiscovery(); if (search.kind !== "string") throw new Error("expected string tool"); + await search.handler({ query: "linear" }, new AbortController().signal); await search.handler( { query: "render layout" }, new AbortController().signal, ); - await search.handler( - { query: "granola notes" }, - new AbortController().signal, + const afterSearch = computeAdvertised(runner.currentDefinitions()).map( + (d) => d.name, + ); + expect(afterSearch).not.toContain("mcp__linear__save_issue"); + expect(afterSearch).not.toContain("present"); + + const allowed = await dispatch(runner, "mcp__linear__save_issue"); + expect(allowed.content).toBe("saved"); + expect(allowed.isError).toBeUndefined(); + const names = computeAdvertised(runner.currentDefinitions()).map( + (d) => d.name, ); + expect(names).toContain("mcp__linear__save_issue"); + expect(names).not.toContain("present"); + expect(names).not.toContain("plugin__notes__save"); + }); + test("present and plugin names promote only the called name", async () => { + const { runner, computeAdvertised } = wireExecDiscovery(); expect((await dispatch(runner, "present")).content).toBe("view"); + expect( + computeAdvertised(runner.currentDefinitions()).map((d) => d.name), + ).toContain("present"); + expect( + computeAdvertised(runner.currentDefinitions()).map((d) => d.name), + ).not.toContain("plugin__notes__save"); + expect((await dispatch(runner, "plugin__notes__save")).content).toBe( "noted", ); diff --git a/src/exec/runner.ts b/src/exec/runner.ts index d4cfd4e13..7fa183b0f 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -880,6 +880,7 @@ export async function runExec(config: Config): Promise { computeAdvertised, isAdvertised, flushPromotions, + pruneIdlePromotions, } = createAdvertisedToolset({ sessionMode, toolAvailability, @@ -937,9 +938,9 @@ export async function runExec(config: Config): Promise { ), telemetry: liveTelemetry, onFolded: () => { - // Fold restarts the cached prefix, so catch promotions still - // pending. Search already flushed names onto the next infer. - if (flushPromotions()) { + // Fold restarts the cached prefix — drop idle execute-promoted + // schemas rather than carrying them forever. + if (pruneIdlePromotions()) { directorHolder.instance?.updateToolDefinitions( computeAdvertised( agentToolset.dynamicRunner.currentDefinitions(), @@ -962,10 +963,9 @@ export async function runExec(config: Config): Promise { evidenceArchiveHolder, }); - // tool_search starts as a no-op promoter; without this, the call gate - // refuses MCP/present/plugin names the result just told the model to - // invoke. Under a closed overlay the promoter only activates allowed - // names, so outside-allow tools can never become advertised or callable. + // Promote-on-execute: an undeclared registered call declares that one + // name, then dispatches. Under a closed overlay the promoter only + // activates allowed names, so outside-allow tools stay refused. const commitPromotedWire = (): void => { if (flushPromotions()) { directorHolder.instance?.updateToolDefinitions( diff --git a/src/session/assemble-runtime.test.ts b/src/session/assemble-runtime.test.ts index 101bc22bb..5b5efa6c7 100644 --- a/src/session/assemble-runtime.test.ts +++ b/src/session/assemble-runtime.test.ts @@ -50,7 +50,7 @@ describe("createAdvertisedToolset", () => { }); // Activation opens the call gate but does not reshape the wire set until - // flushPromotions commits it (promoters flush on search; this unit stays + // flushPromotions commits it (promoters flush on execute; this unit stays // the split). test("activation alone leaves the wire set untouched until flushPromotions commits it", () => { const { activated, computeAdvertised, flushPromotions } = @@ -140,6 +140,26 @@ describe("createAdvertisedToolset", () => { expect(isAdvertised("mcp__acme__do")).toBe(true); }); + test("pruneIdlePromotions drops execute-promoted schemas and keeps the frozen prefix", () => { + const { activated, computeAdvertised, flushPromotions, pruneIdlePromotions } = + createAdvertisedToolset( + wiring({ pinnedTools: ["mcp__linear__save_issue"] }), + ); + const defs = [ + def("read_file"), + def("mcp__linear__save_issue"), + def("mcp__acme__do"), + ]; + activated.activate(["mcp__acme__do"]); + expect(flushPromotions()).toBe(true); + expect(computeAdvertised(defs).map((d) => d.name)).toContain("mcp__acme__do"); + expect(pruneIdlePromotions()).toBe(true); + const names = computeAdvertised(defs).map((d) => d.name); + expect(names).not.toContain("mcp__acme__do"); + expect(names).toContain("mcp__linear__save_issue"); + expect(pruneIdlePromotions()).toBe(false); + }); + test("pinned tools are advertised before any activation and survive clear", () => { const { activated, computeAdvertised } = createAdvertisedToolset( wiring({ pinnedTools: ["mcp__linear__save_issue"] }), diff --git a/src/session/assemble-runtime.ts b/src/session/assemble-runtime.ts index 893e97145..cc615fd6d 100644 --- a/src/session/assemble-runtime.ts +++ b/src/session/assemble-runtime.ts @@ -353,17 +353,19 @@ export interface AdvertisedToolset { activated: ActivatedToolTracker; computeAdvertised: (all: readonly ToolDefinition[]) => ToolDefinition[]; // Whether a name is part of the advertised wire set: built-in prefix, - // project-pinned, or tool_search-activated. The dispatch gate keys off this - // so a registered-but-unadvertised call surfaces as a tool_search error - // instead of silently dispatching. + // project-pinned, or execute-promoted. The dispatch gate keys off this + // so a registered-but-unadvertised call can intercept (declare that one + // name, then dispatch) instead of failing closed. isAdvertised: (name: string) => boolean; // Commit activated-but-unadvertised names onto the wire set, in activation - // order. Returns whether the wire set actually grew. Call on tool_search - // promote so the next infer declares the schema, and at cache-safe - // boundaries (session start/resume, rotation, compaction) for anything - // still pending. Growing the serialized tools array mid-thread re-prefills - // the provider prefix; that is the cost of making a promotion callable. + // order. Returns whether the wire set actually grew. Call on promote-on-execute + // so the next infer declares that one schema, and at cache-safe boundaries + // (session start/resume, rotation) for anything still pending. flushPromotions: () => boolean; + // Fold already broke the provider cache prefix — drop execute-promoted + // schemas that are not in the frozen core/pinned prefix. Discovered names + // remain callable via intercept. Returns whether the wire set shrank. + pruneIdlePromotions: () => boolean; } /** @@ -373,11 +375,10 @@ export interface AdvertisedToolset { * * Activation and advertisement are split on purpose. Activating a name opens * the call gate at once (isAdvertised flips). flushPromotions copies those - * names onto the wire array the next infer sends. tool_search promoters - * flush immediately so a search then save_issue can land on the following - * infer without waiting for compact. Mid-session growth re-prefills the - * provider cache prefix; holding names off the wire until compact left the - * model unable to emit them. + * names onto the wire array the next infer sends. Promote-on-execute flushes + * the one called name so strict providers see it declared; tool_search must + * not flush the match set. Fold is a free cache break — pruneIdlePromotions + * drops the advertised tail back to the frozen prefix. * * `pinnedTools` (local settings) merge into the prefix — advertised from the * first turn and exempt from activation state, so a resume needs no @@ -400,9 +401,10 @@ export function createAdvertisedToolset(args: { const activated = createActivatedToolTracker(); // Wire-committed activations. activate() opens the call gate (see // isAdvertised) at once; names join this snapshot via flushPromotions — - // on tool_search promote for the next infer, and at cache-safe - // boundaries for resume/fold. clear() resets both: a rotated session - // restarts at the prefix (see newSession). + // on promote-on-execute for the next infer, and at cache-safe + // boundaries for resume. clear() resets both: a rotated session + // restarts at the prefix (see newSession). pruneIdlePromotions drops + // the tail at fold. let wireActivated: string[] = []; const wireActivatedSet = new Set(); const advertised: ActivatedToolTracker = { @@ -471,11 +473,21 @@ export function createAdvertisedToolset(args: { } return grew; }; + const pruneIdlePromotions = (): boolean => { + if (wireActivated.length === 0 && activated.list().length === 0) { + return false; + } + activated.clear(); + wireActivated = []; + wireActivatedSet.clear(); + return true; + }; return { activated: advertised, computeAdvertised, isAdvertised, flushPromotions, + pruneIdlePromotions, }; } diff --git a/src/tui/dynamic-tool-runner.test.ts b/src/tui/dynamic-tool-runner.test.ts index a06efe967..24ac73bd1 100644 --- a/src/tui/dynamic-tool-runner.test.ts +++ b/src/tui/dynamic-tool-runner.test.ts @@ -101,7 +101,7 @@ describe("call gate", () => { ); expect(blocked.isError).toBe(true); - // tool_search promotion grows the wire set; the same call then dispatches. + // Promote-on-execute grows the wire set; the same call then dispatches. advertised.add("mcp__acme__do"); const result = await runner.run( { id: "2", name: "mcp__acme__do", arguments: {} }, @@ -110,6 +110,30 @@ describe("call gate", () => { expect(result.content).toBe("blind-result"); expect(result.isError).toBeUndefined(); }); + + test("an undeclared registered call promotes only that name then dispatches", async () => { + const runner = createDynamicToolRunner([ + stringTool("read_file", "core"), + stringTool("mcp__acme__do", "blind-result"), + stringTool("mcp__acme__other", "other"), + ]); + const advertised = new Set(["read_file"]); + const promoted: string[] = []; + runner.setCallGate((name) => advertised.has(name)); + runner.setOnUndeclaredCall((name) => { + promoted.push(name); + advertised.add(name); + }); + + const result = await runner.run( + { id: "1", name: "mcp__acme__do", arguments: {} }, + new AbortController().signal, + ); + expect(result.content).toBe("blind-result"); + expect(result.isError).toBeUndefined(); + expect(promoted).toEqual(["mcp__acme__do"]); + expect(advertised.has("mcp__acme__other")).toBe(false); + }); }); describe("mangled dispatch names", () => { diff --git a/src/tui/dynamic-tool-runner.ts b/src/tui/dynamic-tool-runner.ts index d3b168ea0..53fe988c5 100644 --- a/src/tui/dynamic-tool-runner.ts +++ b/src/tui/dynamic-tool-runner.ts @@ -28,22 +28,32 @@ export type DynamicToolRunner = AgentToolRunner & { removeTools(names: string[]): void; currentDefinitions(): ToolDefinition[]; /** - * When a gate is set, run() refuses a registered tool whose name is not on - * the current wire (built-in prefix + pinned + activated) with an error - * pointing at tool_search, instead of silently dispatching. Without a gate - * every registered tool stays dispatchable — sub-agent runners never set one. + * When a gate is set, a registered tool whose name is not on the current + * wire (built-in prefix + pinned + activated) is interceptable: if + * `setOnUndeclaredCall` was wired, that handler declares the one name + * (promote-on-execute) and run() re-checks the gate, then dispatches. + * If the name is still not callable, run() errors toward tool_search. + * Without a gate every registered tool stays dispatchable — sub-agent + * runners never set one. * - * `options.isActivated` is the promotion side of the gate: a name the model - * was already shown (tool_search activated it) that is absent from the - * registry — its server disconnected or the snapshot rebuilt under it — - * reports "not currently available, server may be reconnecting, retry - * shortly" instead of the bare unknown-tool string. Names never activated - * keep the exact unknown-tool string. + * `options.isActivated` is the promotion side of the gate: a name the + * session already declared that is absent from the registry — its server + * disconnected or the snapshot rebuilt under it — reports "not currently + * available, server may be reconnecting, retry shortly" instead of the + * bare unknown-tool string. Names never activated keep the exact + * unknown-tool string. */ setCallGate( isCallable: (name: string) => boolean, options?: { isActivated?: (name: string) => boolean }, ): void; + /** + * Session promoter used when a known-but-unadvertised call arrives. + * Declare that one name (activate + flush), then dispatch. Search must + * not pre-promote the match set; this is the only path that grows the + * advertised tail mid-session besides director-side triggers. + */ + setOnUndeclaredCall(handler: (name: string) => void): void; }; export function createDynamicToolRunner( @@ -53,6 +63,7 @@ export function createDynamicToolRunner( const byName = new Map(); let callGate: ((name: string) => boolean) | undefined; let isActivated: ((name: string) => boolean) | undefined; + let onUndeclaredCall: ((name: string) => void) | undefined; const addTools = (tools: AgentTool[]): void => { const incoming = new Set(); @@ -88,6 +99,9 @@ export function createDynamicToolRunner( callGate = isCallable; isActivated = options?.isActivated; }, + setOnUndeclaredCall(handler: (name: string) => void): void { + onUndeclaredCall = handler; + }, async run(call: ToolCall, signal: AbortSignal): Promise { const resolved = resolveRegisteredToolName(call.name, (name) => byName.has(name), @@ -137,19 +151,22 @@ export function createDynamicToolRunner( isError: true, }; } - // The model can only intend a name it saw on the wire. A registered tool - // the wire no longer advertises (activation state lost across a rebuild, - // or a name the model emitted unaided) must fail loudly with a route back - // to tool_search — silently dispatching it leaves the transcript claiming - // a call the next infer's wire does not declare. + // A registered tool off the advertised wire: declare that one name + // (strict providers see it on the session tool list before dispatch), + // then run the call. Search does not pre-promote the match set. If the + // promoter cannot admit the name (closed overlay, denied family), the + // gate still fails and the model is pointed at tool_search. if (callGate !== undefined && !callGate(resolved)) { - return { - callId: call.id, - content: - `Error: ${resolved} is not in the currently advertised tool list. ` + - `Call tool_search to activate it, then retry the call.`, - isError: true, - }; + onUndeclaredCall?.(resolved); + if (!callGate(resolved)) { + return { + callId: call.id, + content: + `Error: ${resolved} is not in the currently advertised tool list. ` + + `Call tool_search to find it, then retry the call.`, + isError: true, + }; + } } let dispatchCall: ToolCall; try { diff --git a/src/tui/mcp-view.test.ts b/src/tui/mcp-view.test.ts index 4d6fc2df0..fa80242af 100644 --- a/src/tui/mcp-view.test.ts +++ b/src/tui/mcp-view.test.ts @@ -158,7 +158,7 @@ describe("structured transcript rows", () => { }); const CATALOGUE = [ - "Promoted onto the next inference tool list. Call them on the next turn:", + "Matching tools — call a listed name to use it:", "", ...[ "mcp__linear__list_issues", diff --git a/src/tui/runner/exit.ts b/src/tui/runner/exit.ts index f6bc400f2..a1fd3b22e 100644 --- a/src/tui/runner/exit.ts +++ b/src/tui/runner/exit.ts @@ -438,12 +438,11 @@ export async function createRunLifecycle( }; state.reloadIfIdle = reloadIfIdle; - // tool_search (and contextual triggers, e.g. the lsp hint) promote tools by - // opening the call gate and committing schemas onto the next infer's wire. - // Holding them off until compaction left MCP/plugin names callable in the - // runner but missing from the provider tools array, so the model could not - // emit those calls. Cache prefix growth on discovery is the cost of making - // promotion actually work. + // Promote-on-execute (and contextual triggers, e.g. the lsp hint) open the + // call gate and commit that one name onto the next infer's wire. Search + // returns cards only and does not pre-promote the match set. Cache prefix + // growth on a called name is the cost of making the call valid for strict + // providers. const promoteTools = (names: string[]): void => { activateAndCommitWire(names); }; diff --git a/src/tui/runner/session.ts b/src/tui/runner/session.ts index c96f92066..3b014083d 100644 --- a/src/tui/runner/session.ts +++ b/src/tui/runner/session.ts @@ -475,14 +475,15 @@ export async function assembleTUISession( workflowHostHolder.instance = workflowHost; // Dynamic tool discovery: only the fixed built-in prefix plus - // wire-committed activations reach the wire. MCP tools must be promoted - // here before the model can invoke them; promoters flush schemas onto the - // next infer, and compaction folds catch anything still pending. + // wire-committed activations reach the wire. tool_search returns cards + // only; promote-on-execute declares the one name the model actually calls. + // Fold drops idle execute-promoted schemas (cache already broken). const { activated: activatedToolNames, computeAdvertised, isAdvertised, flushPromotions, + pruneIdlePromotions, } = createAdvertisedToolset({ sessionMode: liveSessionMode, toolAvailability, @@ -497,10 +498,11 @@ export async function assembleTUISession( // turn already declares them. activatedToolNames.activate(start.resumeSeed.activatedTools); flushPromotions(); - // A registered tool the wire never advertised must error toward tool_search - // instead of dispatching blind — the transcript would otherwise claim a call - // the next infer does not declare. submit_output rides every infer via the - // director; hidden posix aliases dispatch when their engine is advertised. + // A registered tool the wire never advertised is intercepted at dispatch: + // setToolPromoter declares that one name, then the runner re-checks the gate. + // Overlay-denied / unknown names still error toward tool_search. + // submit_output rides every infer via the director; hidden posix aliases + // dispatch when their engine is advertised. const unadvertisedCallable = new Set([submitOutputDefinition.name]); toolset.dynamicRunner.setCallGate( (name) => unadvertisedCallable.has(name) || isAdvertised(name), @@ -710,9 +712,9 @@ export async function assembleTUISession( isAborted: () => compactionLifecycle.getSignal().aborted, // Main-session folds only — exec runner and subagents stay silent. onFolded: (info) => { - // Fold restarts the cached prefix, so catch promotions still - // pending. Search already flushed names onto the next infer. - if (flushPromotions()) { + // Fold restarts the cached prefix — drop idle execute-promoted + // schemas rather than carrying them forever. + if (pruneIdlePromotions()) { directorHolder.instance?.updateToolDefinitions( computeAdvertised(toolset.dynamicRunner.currentDefinitions()), ); @@ -787,6 +789,7 @@ export async function assembleTUISession( activatedToolNames, computeAdvertised, flushPromotions, + pruneIdlePromotions, buildAgent: chatAgent.buildAgent, sessionCost, sessionOps, diff --git a/src/tui/runner/state.ts b/src/tui/runner/state.ts index 1db1d4680..fceb9e526 100644 --- a/src/tui/runner/state.ts +++ b/src/tui/runner/state.ts @@ -162,6 +162,11 @@ export interface RunnerServices { typeof import("../../session/assemble-runtime.js").createAdvertisedToolset > >["flushPromotions"]; + pruneIdlePromotions: Awaited< + ReturnType< + typeof import("../../session/assemble-runtime.js").createAdvertisedToolset + > + >["pruneIdlePromotions"]; buildAgent: ReturnType< typeof import("../../session/assemble-runtime.js").assembleChatAgent >["buildAgent"]; From d677b9026ceae0eeb48b3e6e3259d346fac0404f Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 29 Sep 2026 14:35:23 -0700 Subject: [PATCH 2/3] fix(tools): keep list_dir off the advertised wire on execute Promote-on-execute flushed any registered unadvertised name, so a list_dir call put its schema on the wire until fold. Search already hides that builtin in favor of glob; dispatch it without advertising. --- src/agent/tool-search.test.ts | 13 +++++++ src/agent/tool-search.ts | 4 ++- src/exec/runner.test.ts | 54 ++++++++++++++++++++++++++++ src/exec/runner.ts | 7 +++- src/session/assemble-runtime.test.ts | 16 +++++++++ src/session/assemble-runtime.ts | 4 +++ src/tui/dynamic-tool-runner.test.ts | 25 +++++++++++++ src/tui/dynamic-tool-runner.ts | 24 ++++++++----- 8 files changed, 136 insertions(+), 11 deletions(-) diff --git a/src/agent/tool-search.test.ts b/src/agent/tool-search.test.ts index 95bd9fb0a..b5587f795 100644 --- a/src/agent/tool-search.test.ts +++ b/src/agent/tool-search.test.ts @@ -684,6 +684,7 @@ describe("promote-on-execute", () => { "ran:never", "A tool the model never searched for", ), + stringTool("list_dir", "listed", "list a directory's entries"), ]; const runner = createDynamicToolRunner(tools); const advertised = createAdvertisedToolset({ @@ -773,6 +774,18 @@ describe("promote-on-execute", () => { expect(promoted).toEqual(["mcp__never__searched"]); expect(advertisedNames(advertised, runner)).toContain("mcp__never__searched"); }); + + test("executing list_dir does not join the advertised tail", async () => { + const { advertised, promoted, runner } = wirePromoteOnExecute(); + const before = advertisedNames(advertised, runner); + expect(before).not.toContain("list_dir"); + const result = await dispatch(runner, "list_dir"); + expect(result.content).toBe("listed"); + expect(result.isError).toBeUndefined(); + expect(promoted).toEqual([]); + expect(advertisedNames(advertised, runner)).toEqual(before); + expect(advertisedNames(advertised, runner)).not.toContain("list_dir"); + }); }); describe("advertisedTools", () => { diff --git a/src/agent/tool-search.ts b/src/agent/tool-search.ts index 8f49233e7..d9c985393 100644 --- a/src/agent/tool-search.ts +++ b/src/agent/tool-search.ts @@ -162,7 +162,9 @@ function isAlreadyAdvertised( } // Mounted built-ins that stay off the advertised prefix and off tool_search. -const UNADVERTISED_MOUNTED_BUILTINS = new Set(["list_dir"]); +// Promote-on-execute must not flush these onto the wire either — dispatch +// without advertising. glob is the advertised replacement for list_dir. +export const UNADVERTISED_MOUNTED_BUILTINS = new Set(["list_dir"]); // 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 diff --git a/src/exec/runner.test.ts b/src/exec/runner.test.ts index 6ca803900..d68518340 100644 --- a/src/exec/runner.test.ts +++ b/src/exec/runner.test.ts @@ -244,6 +244,42 @@ describe("exec director allowlist", () => { expect(names).not.toContain(OUTSIDE_ALLOW); }); + test("the promoter does not commit list_dir onto the advertised wire", () => { + const { activated, computeAdvertised, flushPromotions } = + createAdvertisedToolset({ + sessionMode: "orchestrator", + toolAvailability: { languageServerAvailable: true }, + getProvider: () => ({ providerName: "test", model: "test-model" }), + }); + let committed = 0; + const promote = createExecToolPromoter({ + activate: (names) => activated.activate(names), + isAllowed: () => true, + commitWire: () => { + if (flushPromotions()) committed += 1; + }, + }); + const registry = [ + { + name: "list_dir", + description: "list a directory", + inputSchema: { type: "object", properties: {} }, + }, + { + name: "mcp__acme__do", + description: "do a thing", + inputSchema: { type: "object", properties: {} }, + }, + ]; + promote(["list_dir", "mcp__acme__do"]); + expect(activated.has("list_dir")).toBe(false); + expect(activated.has("mcp__acme__do")).toBe(true); + expect(committed).toBe(1); + const names = computeAdvertised(registry).map((d) => d.name); + expect(names).not.toContain("list_dir"); + expect(names).toContain("mcp__acme__do"); + }); + test("skywalker overlay leaves every tool allowed", () => { const overlay = resolveExecDirectorOverlay("skywalker"); expect(isExecOverlayToolAllowed(overlay, OUTSIDE_ALLOW)).toBe(true); @@ -652,6 +688,7 @@ describe("exec tool call gate and promoter", () => { "search and render layout primitives for pages", ), stringTool("plugin__notes__save", "noted", "Save granola notes"), + stringTool("list_dir", "listed", "list a directory's entries"), stringTool(submitOutputDefinition.name, "submitted", "submit output"), ]); const { activated, isAdvertised, computeAdvertised, flushPromotions } = @@ -760,4 +797,21 @@ describe("exec tool call gate and promoter", () => { expect(result.content).toBe("submitted"); expect(result.isError).toBeUndefined(); }); + + test("executing list_dir does not add it to computeAdvertised", async () => { + const { runner, computeAdvertised } = wireExecDiscovery(); + const before = computeAdvertised(runner.currentDefinitions()).map( + (d) => d.name, + ); + expect(before).not.toContain("list_dir"); + const result = await dispatch(runner, "list_dir"); + expect(result.content).toBe("listed"); + expect(result.isError).toBeUndefined(); + expect( + computeAdvertised(runner.currentDefinitions()).map((d) => d.name), + ).toEqual(before); + expect( + computeAdvertised(runner.currentDefinitions()).map((d) => d.name), + ).not.toContain("list_dir"); + }); }); diff --git a/src/exec/runner.ts b/src/exec/runner.ts index 7fa183b0f..105855594 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -28,6 +28,7 @@ import { import { type ActivatedToolTracker, type ToolAvailability, + UNADVERTISED_MOUNTED_BUILTINS, } from "../agent/tool-search.js"; import { detectLanguageServerAvailable } from "../agent/lsp-availability.js"; import { @@ -412,7 +413,11 @@ export function createExecToolPromoter(args: { commitWire?: () => void; }): (names: string[]) => void { return (names) => { - if (!args.activate(names.filter((name) => args.isAllowed(name)))) return; + const admitted = names.filter( + (name) => + args.isAllowed(name) && !UNADVERTISED_MOUNTED_BUILTINS.has(name), + ); + if (!args.activate(admitted)) return; args.commitWire?.(); args.persist?.(); }; diff --git a/src/session/assemble-runtime.test.ts b/src/session/assemble-runtime.test.ts index 5b5efa6c7..192bce8ce 100644 --- a/src/session/assemble-runtime.test.ts +++ b/src/session/assemble-runtime.test.ts @@ -140,6 +140,22 @@ describe("createAdvertisedToolset", () => { expect(isAdvertised("mcp__acme__do")).toBe(true); }); + test("flushPromotions does not commit unadvertised mounted builtins onto the wire", () => { + const { activated, computeAdvertised, flushPromotions } = + createAdvertisedToolset(wiring()); + const registry = [def("read_file"), def("list_dir"), def("mcp__acme__do")]; + expect(activated.activate(["list_dir"])).toBe(true); + expect(flushPromotions()).toBe(false); + expect(computeAdvertised(registry).map((d) => d.name)).not.toContain( + "list_dir", + ); + expect(activated.activate(["mcp__acme__do"])).toBe(true); + expect(flushPromotions()).toBe(true); + const names = computeAdvertised(registry).map((d) => d.name); + expect(names).not.toContain("list_dir"); + expect(names).toContain("mcp__acme__do"); + }); + test("pruneIdlePromotions drops execute-promoted schemas and keeps the frozen prefix", () => { const { activated, computeAdvertised, flushPromotions, pruneIdlePromotions } = createAdvertisedToolset( diff --git a/src/session/assemble-runtime.ts b/src/session/assemble-runtime.ts index cc615fd6d..1bf440700 100644 --- a/src/session/assemble-runtime.ts +++ b/src/session/assemble-runtime.ts @@ -46,6 +46,7 @@ import { advertisedTools, advertisedToolNamesForSessionMode, createActivatedToolTracker, + UNADVERTISED_MOUNTED_BUILTINS, type ActivatedToolTracker, type ToolAvailability, } from "../agent/tool-search.js"; @@ -467,6 +468,9 @@ export function createAdvertisedToolset(args: { let grew = false; for (const name of activated.list()) { if (wireActivatedSet.has(name)) continue; + // Search hides these; flushing would put the schema on the wire until + // fold. Dispatch stays available via intercept without advertising. + if (UNADVERTISED_MOUNTED_BUILTINS.has(name)) continue; wireActivatedSet.add(name); wireActivated.push(name); grew = true; diff --git a/src/tui/dynamic-tool-runner.test.ts b/src/tui/dynamic-tool-runner.test.ts index 24ac73bd1..45e2db3d6 100644 --- a/src/tui/dynamic-tool-runner.test.ts +++ b/src/tui/dynamic-tool-runner.test.ts @@ -134,6 +134,31 @@ describe("call gate", () => { expect(promoted).toEqual(["mcp__acme__do"]); expect(advertised.has("mcp__acme__other")).toBe(false); }); + + test("list_dir dispatches under a call gate without promoting", async () => { + const runner = createDynamicToolRunner([ + stringTool("read_file", "core"), + stringTool("list_dir", "listed"), + stringTool("mcp__acme__do", "blind-result"), + ]); + const advertised = new Set(["read_file"]); + const promoted: string[] = []; + runner.setCallGate((name) => advertised.has(name)); + runner.setOnUndeclaredCall((name) => { + promoted.push(name); + advertised.add(name); + }); + + const result = await runner.run( + { id: "1", name: "list_dir", arguments: {} }, + new AbortController().signal, + ); + expect(result.content).toBe("listed"); + expect(result.isError).toBeUndefined(); + expect(promoted).toEqual([]); + expect(advertised.has("list_dir")).toBe(false); + expect(advertised.has("mcp__acme__do")).toBe(false); + }); }); describe("mangled dispatch names", () => { diff --git a/src/tui/dynamic-tool-runner.ts b/src/tui/dynamic-tool-runner.ts index 53fe988c5..fd6adbfb6 100644 --- a/src/tui/dynamic-tool-runner.ts +++ b/src/tui/dynamic-tool-runner.ts @@ -13,6 +13,7 @@ import { import { resolveRegisteredToolName } from "./resolve-registered-tool-name.js"; import { stripTerminalControlSequences } from "../util/control-char-strip.js"; import { prepareDispatchedToolCall } from "../agent/tool-aliases.js"; +import { UNADVERTISED_MOUNTED_BUILTINS } from "../agent/tool-search.js"; // A tool runner whose set of tools can grow after construction. The static // createToolRunner freezes its name map at build time, which cannot accommodate @@ -156,16 +157,21 @@ export function createDynamicToolRunner( // then run the call. Search does not pre-promote the match set. If the // promoter cannot admit the name (closed overlay, denied family), the // gate still fails and the model is pointed at tool_search. + // Unadvertised mounted builtins (list_dir) stay off the wire — search + // hides them in favor of glob — so intercept dispatches without + // flushPromotions rather than advertising until fold. if (callGate !== undefined && !callGate(resolved)) { - onUndeclaredCall?.(resolved); - if (!callGate(resolved)) { - return { - callId: call.id, - content: - `Error: ${resolved} is not in the currently advertised tool list. ` + - `Call tool_search to find it, then retry the call.`, - isError: true, - }; + if (!UNADVERTISED_MOUNTED_BUILTINS.has(resolved)) { + onUndeclaredCall?.(resolved); + if (!callGate(resolved)) { + return { + callId: call.id, + content: + `Error: ${resolved} is not in the currently advertised tool list. ` + + `Call tool_search to find it, then retry the call.`, + isError: true, + }; + } } } let dispatchCall: ToolCall; From 40ef520ce9396f91d03fae910b25f727b9da2ed3 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 29 Sep 2026 16:32:02 -0700 Subject: [PATCH 3/3] fix(tools): advertise late MCP tools on execute in tests Search no longer grows the advertised set. The late-dispatch harness now uses the same call-gate promoter as TUI and exec so schemas land on the next infer. --- e2e/mcp-late-dispatch.test.ts | 35 ++++++++++++++++++++++++---- src/agent/tool-search.test.ts | 26 ++++++++++++++------- src/agent/tool-search.ts | 3 +-- src/auth/store.test.ts | 2 +- src/session/assemble-runtime.test.ts | 16 +++++++++---- 5 files changed, 62 insertions(+), 20 deletions(-) diff --git a/e2e/mcp-late-dispatch.test.ts b/e2e/mcp-late-dispatch.test.ts index dfbfbbbf0..ba2483664 100644 --- a/e2e/mcp-late-dispatch.test.ts +++ b/e2e/mcp-late-dispatch.test.ts @@ -6,8 +6,10 @@ import { createAgentWithLiveToolDispatch } from "../src/agent/live-tool-dispatch import type { MCPClient } from "../src/mcp/client.js"; import { mcpClientTools } from "../src/mcp/plugin.js"; import { createPermissionGate } from "../src/permission/gate.js"; +import { createAdvertisedToolset } from "../src/session/assemble-runtime.js"; import { closeIntegrationSession, + INTEGRATION_SOURCE, openIntegrationSession, runUntilDone, toolDoneEvents, @@ -108,13 +110,35 @@ function toolDoneContents(events: ReactorEmittedEvent[]): string[] { ); } -// Registers dynamic tools as promotable so the next request advertises them. +// Wires production promote-on-execute: search returns cards only; a call to a +// registered-but-unadvertised name declares that one schema, then dispatches. function promoteDynamicTools(session: IntegrationSession): void { - session.toolset.setToolPromoter(() => { + const advertised = createAdvertisedToolset({ + sessionMode: "orchestrator", + toolAvailability: { languageServerAvailable: false }, + getProvider: () => ({ + providerName: INTEGRATION_SOURCE.provider, + model: INTEGRATION_SOURCE.model, + }), + }); + session.toolset.dynamicRunner.setCallGate( + (name) => advertised.isAdvertised(name), + { isActivated: (name) => advertised.activated.has(name) }, + ); + session.toolset.setToolPromoter((names) => { + if (!advertised.activated.activate(names)) return; + if (!advertised.flushPromotions()) return; session.updateToolDefinitions( - session.toolset.dynamicRunner.currentDefinitions(), + advertised.computeAdvertised( + session.toolset.dynamicRunner.currentDefinitions(), + ), ); }); + session.updateToolDefinitions( + advertised.computeAdvertised( + session.toolset.dynamicRunner.currentDefinitions(), + ), + ); } function replyScript( @@ -318,7 +342,10 @@ describe("integration — late MCP dispatch", () => { promotionScript(session, call); const { events } = await runUntilDone(session, "list one linear issue"); - const published = publishedTool(await requestBodies(session)); + const bodies = await requestBodies(session); + expect(bodies.length).toBeGreaterThan(0); + expect(publishedTool(bodies.slice(0, 1))).toBeUndefined(); + const published = publishedTool(bodies); if (schema === undefined) { expect(published?.input_schema.properties).toEqual( diff --git a/src/agent/tool-search.test.ts b/src/agent/tool-search.test.ts index b5587f795..2b48e48bf 100644 --- a/src/agent/tool-search.test.ts +++ b/src/agent/tool-search.test.ts @@ -320,7 +320,10 @@ function listedToolNames(out: string): string[] { return out .split("\n") .filter((line) => line.startsWith("- ")) - .map((line) => line.slice(2).split(":")[0]!.trim()); + .flatMap((line) => { + const name = line.slice(2).split(":")[0]; + return name === undefined ? [] : [name.trim()]; + }); } async function flushMicrotasks(rounds = 100): Promise { @@ -342,9 +345,7 @@ describe("createToolSearchTool", () => { expect(out).toContain("present"); expect(out).toContain("layout"); expect(out).not.toMatch(/promoted/i); - expect( - advertisedTools(defs).map((d) => d.name), - ).not.toContain("present"); + expect(advertisedTools(defs).map((d) => d.name)).not.toContain("present"); }); test("default tool_search returns at most 5 matches and does not promote them", async () => { @@ -666,7 +667,11 @@ describe("createToolSearchTool", () => { }); describe("promote-on-execute", () => { - const stringTool = (name: string, reply: string, description: string): AgentTool => ({ + const stringTool = ( + name: string, + reply: string, + description: string, + ): AgentTool => ({ kind: "string", definition: { name, @@ -678,7 +683,9 @@ describe("promote-on-execute", () => { function wirePromoteOnExecute() { const tools = [ - ...wideCatalog.map((d) => stringTool(d.name, `ran:${d.name}`, d.description)), + ...wideCatalog.map((d) => + stringTool(d.name, `ran:${d.name}`, d.description), + ), stringTool( "mcp__never__searched", "ran:never", @@ -753,7 +760,8 @@ describe("promote-on-execute", () => { ); expect(listed).toHaveLength(3); expect(promoted).toEqual([]); - const called = listed[0]!; + const called = listed[0]; + if (called === undefined) throw new Error("expected a listed tool"); const others = listed.slice(1); const result = await dispatch(runner, called); expect(result.content).toBe(`ran:${called}`); @@ -772,7 +780,9 @@ describe("promote-on-execute", () => { expect(result.content).toBe("ran:never"); expect(result.isError).toBeUndefined(); expect(promoted).toEqual(["mcp__never__searched"]); - expect(advertisedNames(advertised, runner)).toContain("mcp__never__searched"); + expect(advertisedNames(advertised, runner)).toContain( + "mcp__never__searched", + ); }); test("executing list_dir does not join the advertised tail", async () => { diff --git a/src/agent/tool-search.ts b/src/agent/tool-search.ts index d9c985393..960530efe 100644 --- a/src/agent/tool-search.ts +++ b/src/agent/tool-search.ts @@ -372,8 +372,7 @@ function parseToolSearchArgs( rawArgs: Record, ): { error: string } | { query: string; limit: number } { const parsed = ToolSearchArgs(rawArgs); - const queryRaw = - parsed instanceof type.errors ? rawArgs.query : parsed.query; + const queryRaw = parsed instanceof type.errors ? rawArgs.query : parsed.query; if (typeof queryRaw !== "string") { return { error: "Error: tool_search requires query (string)." }; } diff --git a/src/auth/store.test.ts b/src/auth/store.test.ts index a29f23cf6..ae0d7a9ed 100644 --- a/src/auth/store.test.ts +++ b/src/auth/store.test.ts @@ -11,7 +11,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { type } from "arktype"; -import { withMockedModule } from "../../tests/helpers/mock-module.js"; +import { withMockedModule } from "../../testkit/mock-module.js"; import type { BaseTokens } from "./store.js"; let failingUnlinkPath: string | undefined; diff --git a/src/session/assemble-runtime.test.ts b/src/session/assemble-runtime.test.ts index 192bce8ce..e872fbd4a 100644 --- a/src/session/assemble-runtime.test.ts +++ b/src/session/assemble-runtime.test.ts @@ -157,10 +157,14 @@ describe("createAdvertisedToolset", () => { }); test("pruneIdlePromotions drops execute-promoted schemas and keeps the frozen prefix", () => { - const { activated, computeAdvertised, flushPromotions, pruneIdlePromotions } = - createAdvertisedToolset( - wiring({ pinnedTools: ["mcp__linear__save_issue"] }), - ); + const { + activated, + computeAdvertised, + flushPromotions, + pruneIdlePromotions, + } = createAdvertisedToolset( + wiring({ pinnedTools: ["mcp__linear__save_issue"] }), + ); const defs = [ def("read_file"), def("mcp__linear__save_issue"), @@ -168,7 +172,9 @@ describe("createAdvertisedToolset", () => { ]; activated.activate(["mcp__acme__do"]); expect(flushPromotions()).toBe(true); - expect(computeAdvertised(defs).map((d) => d.name)).toContain("mcp__acme__do"); + expect(computeAdvertised(defs).map((d) => d.name)).toContain( + "mcp__acme__do", + ); expect(pruneIdlePromotions()).toBe(true); const names = computeAdvertised(defs).map((d) => d.name); expect(names).not.toContain("mcp__acme__do");