perf(director): pin tool wire order so discovery order cannot bust the cache

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 a pure reorder of an unchanged set
drops it to 2.1% — the same full miss a real mount costs. Discovery order
is not stable across turns (MCP connect order, plugin load order, map
iteration), so the wire order is now sorted by name.

updateToolDefinitions logs a digest when the set actually changes, so
prefix churn is countable instead of showing up only as a billing spike.

Drops the assertion that a newly discovered tool must land at the end of
the array. Appending was treated as the cache-preserving position; it is
not — append measures 4.5% cached against prepend's 2.1%, both full
misses.

CL-7868

Author: TheGreatAxios

src/agent/director.ts

@@ -48,6 +48,22 @@ 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. Any byte-level change to it — a mount, a
+// description edit, or a pure reorder of an unchanged set — invalidates the
+// whole prefix for that turn. Measured on OpenCode Go Responses: 99.3% cached
+// falling to 2.1% on a reorder alone. Tool discovery order is not stable across
+// turns (MCP connect order, plugin load order, map iteration), so the wire
+// order is pinned by name. See CL-7868.
+function stableToolOrder(tools: readonly ToolDefinition[]): ToolDefinition[] {
+  return [...tools].sort((a, b) => a.name.localeCompare(b.name));
+}
+
+/** Identity of a serialized tool set: name and description, in wire order. */
+function toolSetDigest(tools: readonly ToolDefinition[]): string {
+  return tools.map((t) => `${t.name}:${t.description ?? ""}`).join("|");
+}
+
 function isInternalRecoveryAbort(
   event: Extract<ReactorInboundEvent, { type: "inference.error" }>,
 ): boolean {
@@ -482,9 +498,10 @@ class ChatDirectorImpl extends DefaultDirector {
     toolDefinitions: ToolDefinition[],
     options: ChatDirectorImplOptions,
   ) {
-    super(systemPrompt, toolDefinitions, {});
+    const orderedTools = stableToolOrder(toolDefinitions);
+    super(systemPrompt, orderedTools, {});
     this._systemPrompt = systemPrompt;
-    this._toolDefinitions = toolDefinitions;
+    this._toolDefinitions = orderedTools;
     this.inactivityTimeoutMs = options.inactivityTimeoutMs;
     this.totalTimeoutMs = options.totalTimeoutMs;
     this.taskClassifier = options.taskClassifier;
@@ -494,7 +511,7 @@ class ChatDirectorImpl extends DefaultDirector {
     this.compaction = createCompactionGovernor(
       options.requestContinuation,
       systemPrompt,
-      toolDefinitions,
+      orderedTools,
     );
     this.modelFamilyPolicy =
       options.modelFamilyPolicy ??
@@ -508,7 +525,14 @@ class ChatDirectorImpl extends DefaultDirector {
   }
 
   updateToolDefinitions(toolDefinitions: ToolDefinition[]): void {
-    this._toolDefinitions = toolDefinitions;
+    const ordered = stableToolOrder(toolDefinitions);
+    const before = toolSetDigest(this._toolDefinitions);
+    const after = toolSetDigest(ordered);
+    this._toolDefinitions = ordered;
+    if (before === after) return;
+    // Prefix churn is otherwise silent — it shows up only as a billing and
+    // latency spike on the next turn. Name it so it can be counted.
+    logger.debug`tool-set-changed count=${String(this._toolDefinitions.length)} before=${before} after=${after}`;
   }
 
   getTasks(): Task[] {
@@ -592,7 +616,7 @@ class ChatDirectorImpl extends DefaultDirector {
       (t) => t.name === submitOutputDefinition.name,
     )
       ? this._toolDefinitions
-      : [...this._toolDefinitions, submitOutputDefinition];
+      : stableToolOrder([...this._toolDefinitions, submitOutputDefinition]);
 
     const directive = active
       ? (this.workflowCoordinator?.directive() ?? null)

src/director.test.ts

@@ -1223,18 +1223,22 @@ describe("updateToolDefinitions rewrites infer tools", () => {
     const afterTools = after as { name: string }[];
     const afterNames = afterTools.map((t) => t.name);
     expect(afterNames).toContain("mcp__linear__list_issues");
-    // submit_output 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.
+    // submit_output rides along separately (see withCurrentTools). Strip both
+    // it and the new tool: the rest of the set must be unchanged, and in the
+    // same relative order.
+    //
+    // Where the new tool lands is deliberately NOT asserted. Appending at the
+    // end used to be treated as the cache-preserving position; it is not.
+    // Measured on OpenCode Go Responses, appending a tool drops the hit rate
+    // to 4.5% and prepending to 2.1% — both are a full prefix miss, because
+    // the tools array is hashed as a unit ahead of the system prompt. Wire
+    // order is therefore sorted by name for stability, not positioned for
+    // cache survival. See CL-7868.
     const beforePrefix = beforeNames.filter((n) => n !== "submit_output");
     const afterPrefix = afterNames.filter(
       (n) => n !== "submit_output" && 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(
@@ -1661,3 +1665,55 @@ describe("chatDirector spacer echo", () => {
     expect(exhausted.some((a) => a.type === "reply")).toBe(true);
   });
 });
+
+// The serialized tools array is the head of the provider's cached prompt
+// prefix. A reorder of an unchanged set costs the same full miss as a real
+// mount — measured at 99.3% cached falling to 2.1% on OpenCode Go Responses
+// (CL-7868) — so discovery order must never reach the wire.
+describe("tool wire order", () => {
+  const tool = (name: string) => ({
+    name,
+    description: `The ${name} tool`,
+    inputSchema: { type: "object" as const, properties: {} },
+  });
+
+  async function wireTools(
+    director: ReturnType<typeof createChatDirector>,
+  ): Promise<string[]> {
+    const actions = actionsArray(
+      await director.decide(
+        makeToolErrorEvent("c", "denied by approver: never touch /etc"),
+        mockState,
+        mockCapabilities,
+      ),
+    );
+    const infer = actions.find((a) => a.type === "infer");
+    const options = (infer as { options?: ExtendedInferenceOptions }).options;
+    return (options?.tools ?? []).map((t) => t.name);
+  }
+
+  test("two discovery orders of one tool set produce the same wire order", async () => {
+    const forward = createChatDirector(
+      "sys",
+      [tool("read_file"), tool("grep"), tool("run_shell")],
+      { onTasksChange: () => undefined },
+    );
+    const reverse = createChatDirector(
+      "sys",
+      [tool("run_shell"), tool("grep"), tool("read_file")],
+      { onTasksChange: () => undefined },
+    );
+    expect(await wireTools(forward)).toEqual(await wireTools(reverse));
+  });
+
+  test("a re-mount in a different order does not change the wire order", async () => {
+    const director = createChatDirector(
+      "sys",
+      [tool("read_file"), tool("grep")],
+      { onTasksChange: () => undefined },
+    );
+    const before = await wireTools(director);
+    director.updateToolDefinitions([tool("grep"), tool("read_file")]);
+    expect(await wireTools(director)).toEqual(before);
+  });
+});