Merge pull request #1027 from corbitsdev/cl-7868-any-mid-session-tool-set-change-costs-a-full-prompt-cache

Hold newly promoted tools off the wire until a cache-safe boundary

Author: TheGreatAxios

src/agent/director.test.ts

@@ -5,7 +5,7 @@ import type {
   ReactorInboundEvent,
   ReactorState,
 } from "@intx/types/runtime";
-import { createChatDirector } from "./director.js";
+import { createChatDirector, toolSetDigest } from "./director.js";
 
 const mockState: ReactorState = { turns: [] } as unknown as ReactorState;
 
@@ -100,6 +100,36 @@ async function runToolOnlyStreak(
   return last;
 }
 
+describe("toolSetDigest", () => {
+  const base = {
+    name: "read_file",
+    description: "read a file",
+    inputSchema: { type: "object" },
+  };
+
+  test("identical sets share a digest", () => {
+    expect(toolSetDigest([{ ...base }])).toBe(toolSetDigest([{ ...base }]));
+  });
+
+  // The digest gates the tool-set-changed log line, and the serialized tools
+  // array is the head of the provider's cached prompt prefix — an
+  // inputSchema-only change reshapes the wire bytes, so it must move the
+  // digest or the cache bust goes unlogged.
+  test("an inputSchema-only change alters the digest", () => {
+    const before = [{ ...base }];
+    const after = [
+      {
+        ...base,
+        inputSchema: {
+          type: "object",
+          properties: { path: { type: "string" } },
+        },
+      },
+    ];
+    expect(toolSetDigest(after)).not.toBe(toolSetDigest(before));
+  });
+});
+
 describe("ChatDirector tool-only loop protection", () => {
   const providerlessPolicy = { providerName: "test-provider" };
 

src/agent/director.ts

@@ -2,6 +2,7 @@ import {
   DefaultDirector,
   type ExtendedInferenceOptions,
 } from "@intx/inference";
+import { createHash } from "node:crypto";
 import { getLogger } from "@intx/log";
 import type {
   ReactorDirector,
@@ -48,6 +49,29 @@ import {
 
 const logger = getLogger([LOG_NAMESPACE_ROOT, "agent", "director"]);
 
+// The serialized `tools` array is the head of the provider's cached prompt
+// prefix, ahead of the system prompt. Measured on OpenCode Go Responses, a warm
+// session holds 99.3% cached and ANY change to that array — a mount, a
+// description edit, or a pure reorder of an unchanged set — drops the next turn
+// to 2-4%. Appending at the end is not cheaper than prepending: 4.5% versus
+// 2.1%, both full misses.
+//
+// `advertisedTools` (src/agent/tool-search.ts) already keeps this array
+// deterministic, so the array should only ever change when a genuine discovery
+// grows it. This digest is here to prove that, because prefix churn is
+// otherwise invisible — it shows up only as a billing and latency spike a turn
+// later. Hashed rather than logged verbatim: MCP tool descriptions are
+// arbitrary-length, server-supplied text and do not belong in the log stream.
+export function toolSetDigest(tools: readonly ToolDefinition[]): string {
+  const shape = tools
+    .map(
+      (t) =>
+        `${t.name}:${t.description ?? ""}:${JSON.stringify(t.inputSchema ?? null)}`,
+    )
+    .join("|");
+  return createHash("sha256").update(shape).digest("hex").slice(0, 12);
+}
+
 function isInternalRecoveryAbort(
   event: Extract<ReactorInboundEvent, { type: "inference.error" }>,
 ): boolean {
@@ -521,7 +545,11 @@ class ChatDirectorImpl extends DefaultDirector {
   }
 
   updateToolDefinitions(toolDefinitions: ToolDefinition[]): void {
+    const before = toolSetDigest(this._toolDefinitions);
+    const after = toolSetDigest(toolDefinitions);
     this._toolDefinitions = toolDefinitions;
+    if (before === after) return;
+    logger.debug`tool-set-changed count=${String(this._toolDefinitions.length)} before=${before} after=${after}`;
   }
 
   getTasks(): Task[] {

src/agent/tool-search.ts

@@ -8,7 +8,8 @@ import { sessionModeEnablesSubAgents } from "../config/session-mode.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 advertised set and the call gate. Shared by the system prompt and
+// onto the call gate at once; the full schema joins the wire set at the next
+// cache-safe boundary. 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
@@ -142,10 +143,11 @@ export const ADVERTISED_TOOL_NAMES: readonly string[] = [
 
 // 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
+// prefix stable) followed by wire-committed tools (MCP or otherwise) in
+// first-commit order. The wire array is byte-stable turn to turn: callers pass
+// only names committed via flushPromotions at a cache-safe boundary, never the
+// live activation list, so a mid-session discovery cannot append here.
+// `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.
 export function advertisedTools(
@@ -209,7 +211,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 advertised until this search promotes them onto the wire. Core tools (read_file, run_shell, 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 matching tools' names, descriptions, and input schemas. Matched tools are promoted and callable on return — invoke them directly, no separate load step.",
+    "Discover callable tools by capability. Most tools — MCP servers, present, and other integrations — are not callable until this search promotes them. Core tools (read_file, run_shell, 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 matching tools' names, descriptions, and input schemas. Matched tools are promoted and callable on return — invoke them directly, no separate load step.",
   inputSchema: {
     type: "object",
     properties: {
@@ -283,9 +285,9 @@ export function createToolIndex(
 export interface ToolSearchDeps {
   search: (query: string) => string[];
   lookup: (name: string) => ToolDefinition | undefined;
-  // Promote matches onto the advertised set and the call gate so the model can
-  // invoke them this turn. The next inference also declares them on the wire
-  // for strict providers.
+  // Promote matches onto the call gate so the model can invoke them this turn
+  // from the result card's schema. Wire declaration follows at the next
+  // cache-safe boundary (compaction fold), never mid-thread.
   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
@@ -374,12 +376,12 @@ 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.
+      // Matches open on the call gate at once; the full schema joins the wire
+      // declarations at the next cache-safe boundary (compaction fold), never
+      // mid-thread, so the provider's cached prefix stays byte-stable. The
+      // tool result below still carries name, description, AND input schema
+      // so the model can shape arguments and call this same turn, before the
+      // promoted definition is declared on the wire.
       deps.promote(names);
       const blocks = names.map((name) =>
         renderToolCard(deps.lookup(name), name),

src/director.test.ts

@@ -1,10 +1,7 @@
 import { describe, test, expect } from "bun:test";
 import { createChatDirector, askOperatorDefinition } from "./agent/director.js";
 import { createAgentToolset } from "./agent/tools.js";
-import {
-  advertisedTools,
-  createActivatedToolTracker,
-} from "./agent/tool-search.js";
+import { createAdvertisedToolset } from "./session/assemble-runtime.js";
 import { createPermissionGate } from "./permission/gate.js";
 import {
   COMPACTOR_KEEP_RECENT_TURNS,
@@ -1145,6 +1142,93 @@ describe("updateToolDefinitions rewrites infer tools", () => {
     expect(JSON.stringify(after)).toBe(JSON.stringify(before));
   });
 
+  // CL-7868 (direction A): the provider cache is a prefix cache keyed on the
+  // tools array, so a tool_search turn that promotes a genuinely new tool must
+  // not reshape the wire set mid-session. The call gate opens (the model
+  // invokes the tool from the search result's schema) while the advertised
+  // array holds steady; the growth event lands at the next cache-safe
+  // boundary (compaction fold), appended after the untouched fixed prefix.
+  test("a tool_search turn promoting a genuinely new tool leaves the wire byte-identical until the fold commits it", async () => {
+    const linearTool = {
+      name: "mcp__linear__list_issues",
+      description: "list issues",
+      inputSchema: { type: "object", properties: {}, required: [] },
+    };
+    const toolset = await createAgentToolset({
+      cwd: process.cwd(),
+      permissionGate: createPermissionGate({
+        approvals: [],
+        interactive: false,
+        skipPermissions: true,
+        reactorGated: false,
+      }),
+      onOperatorGate: async () => ({ kind: "cancel" }),
+    });
+    toolset.dynamicRunner.addTools([
+      { kind: "string", definition: linearTool, handler: async () => "ok" },
+    ]);
+
+    const advertised = createAdvertisedToolset({
+      sessionMode: "orchestrator",
+      toolAvailability: { languageServerAvailable: false },
+      getProvider: () => ({ providerName: "openai", model: "gpt-5" }),
+    });
+    const director = createChatDirector(
+      "base-prompt",
+      advertised.computeAdvertised(toolset.dynamicRunner.currentDefinitions()),
+      { onTasksChange: () => undefined },
+    );
+
+    const before = await firstInferTools(
+      director,
+      makeMessageReceivedEvent("hello"),
+    );
+
+    // tool_search matched a genuinely new tool: the runner opens the call
+    // gate (activation); the per-turn wire recompute deliberately ignores it.
+    expect(advertised.activated.activate(["mcp__linear__list_issues"])).toBe(
+      true,
+    );
+    director.updateToolDefinitions(
+      advertised.computeAdvertised(toolset.dynamicRunner.currentDefinitions()),
+    );
+
+    // Mid-session the wire is byte-identical — the hot prefix never grows —
+    // while the gate is open so the model can invoke the match from the
+    // result card's schema.
+    const after = await firstInferTools(
+      director,
+      makeMessageReceivedEvent("continue"),
+    );
+    expect(JSON.stringify(after)).toBe(JSON.stringify(before));
+    expect(advertised.isAdvertised("mcp__linear__list_issues")).toBe(true);
+
+    // Cache-safe boundary (compaction fold): the pending promotion commits and
+    // the next turn declares it after the untouched fixed prefix.
+    expect(advertised.flushPromotions()).toBe(true);
+    director.updateToolDefinitions(
+      advertised.computeAdvertised(toolset.dynamicRunner.currentDefinitions()),
+    );
+    const folded = await firstInferTools(
+      director,
+      makeMessageReceivedEvent("after fold"),
+    );
+    const foldedNames = (folded as { name: string }[]).map((t) => t.name);
+    expect(foldedNames).toContain("mcp__linear__list_issues");
+    const beforeNames = (before as { name: string }[])
+      .map((t) => t.name)
+      .filter((n) => n !== "submit_output");
+    const foldedPrefix = foldedNames.filter(
+      (n) => n !== "submit_output" && n !== "mcp__linear__list_issues",
+    );
+    expect(foldedPrefix).toEqual(beforeNames);
+    expect(foldedNames.indexOf("mcp__linear__list_issues")).toBe(
+      beforeNames.length,
+    );
+
+    await toolset.dispose();
+  });
+
   // submit_output is always on the wire so a workflow going active never grows
   // the array and busts the provider cache prefix.
   test("submit_output is advertised even with no active workflow", async () => {
@@ -1165,12 +1249,13 @@ describe("updateToolDefinitions rewrites infer tools", () => {
     expect(inferToolNames(inferAction)).toContain("submit_output");
   });
 
-  // 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 () => {
+  // End-to-end: tool_search matches an MCP tool; the runner's gate-only promote
+  // wiring (mirrored here via createAdvertisedToolset + updateToolDefinitions)
+  // keeps it off the wire the next turn — the hot prefix stays byte-identical
+  // while the gate lets the model call it from the result card. The fold
+  // commits the pending promotion with the tool's full definition, appended
+  // after the untouched fixed prefix, and the array then holds steady.
+  test("a tool_search match stays off the wire until the fold, then holds stable", async () => {
     const linearTool = {
       name: "mcp__linear__list_issues",
       description: "list issues",
@@ -1190,10 +1275,14 @@ describe("updateToolDefinitions rewrites infer tools", () => {
       { kind: "string", definition: linearTool, handler: async () => "ok" },
     ]);
 
-    const activated = createActivatedToolTracker();
+    const advertised = createAdvertisedToolset({
+      sessionMode: "orchestrator",
+      toolAvailability: { languageServerAvailable: false },
+      getProvider: () => ({ providerName: "openai", model: "gpt-5" }),
+    });
     const computeAdvertised = (
       all: ReturnType<typeof toolset.dynamicRunner.currentDefinitions>,
-    ) => advertisedTools(all, activated.list());
+    ) => advertised.computeAdvertised(all);
     const director = createChatDirector(
       "base-prompt",
       computeAdvertised(toolset.dynamicRunner.currentDefinitions()),
@@ -1209,17 +1298,34 @@ describe("updateToolDefinitions rewrites infer tools", () => {
     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"]);
+    // Simulate the runner's gate-only promoter plus its boundary wire push:
+    // the gate opens but the wire recompute is byte-identical until the fold.
+    advertised.activated.activate(["mcp__linear__list_issues"]);
     director.updateToolDefinitions(
       computeAdvertised(toolset.dynamicRunner.currentDefinitions()),
     );
 
-    const after = await firstInferTools(
+    const gated = await firstInferTools(
       director,
       makeMessageReceivedEvent("continue"),
     );
+    expect((gated as { name: string }[]).map((t) => t.name)).not.toContain(
+      "mcp__linear__list_issues",
+    );
+    expect(JSON.stringify(gated)).toBe(beforeJson);
+
+    // Simulate the compaction fold: commit the pending promotion, push the
+    // refreshed set, and the tool is declared — appended after the fixed
+    // built-in prefix, which survives untouched ahead of it.
+    advertised.flushPromotions();
+    director.updateToolDefinitions(
+      computeAdvertised(toolset.dynamicRunner.currentDefinitions()),
+    );
+
+    const after = await firstInferTools(
+      director,
+      makeMessageReceivedEvent("after fold"),
+    );
     const afterTools = after as { name: string }[];
     const afterNames = afterTools.map((t) => t.name);
     expect(afterNames).toContain("mcp__linear__list_issues");