perf(agent): memoize toolSetDigest per director, not per module

The digest is only ever consumed by ChatDirectorImpl.updateToolDefinitions,
so a module-level WeakMap keyed by array identity is heavier than the call
pattern needs. Move the memoization onto the director as a last-digest
field: the incoming set is hashed once when installed and the previous
set's digest is reused from the field on the next update. toolSetDigest
becomes a pure function with identical digest values; the proof test now
spies through updateToolDefinitions to show one hash per installed tool
set per director.

Author: TheGreatAxios

src/agent/director.ts

@@ -60,25 +60,19 @@ const logger = getLogger([LOG_NAMESPACE_ROOT, "agent", "director"]);
 // digest stays deterministic — it only moves when discovery grows the set.
 // Hashed: MCP descriptions are arbitrary server text, not log material.
 //
-// Memoized per array identity: the director owns stable tool arrays and swaps
-// them wholesale (constructor, updateToolDefinitions) without in-place
-// mutation, so the same reference always means the same content and a new
-// reference (tools changed) misses the WeakMap and recomputes. A WeakMap also
-// avoids retaining arrays the director has dropped.
-const toolSetDigestCache = new WeakMap<readonly ToolDefinition[], string>();
-
+// Pure by design: the memoization lives on the director (_toolSetDigest), the
+// only production caller, so the module keeps no per-array state. Callers swap
+// tool arrays wholesale (constructor, updateToolDefinitions) without in-place
+// mutation; mutating an installed array in place would leave the director's
+// cached digest stale.
 export function toolSetDigest(tools: readonly ToolDefinition[]): string {
-  const cached = toolSetDigestCache.get(tools);
-  if (cached !== undefined) return cached;
   const shape = tools
     .map(
       (t) =>
         `${t.name}:${t.description ?? ""}:${JSON.stringify(t.inputSchema ?? null)}`,
     )
     .join("|");
-  const digest = createHash("sha256").update(shape).digest("hex").slice(0, 12);
-  toolSetDigestCache.set(tools, digest);
-  return digest;
+  return createHash("sha256").update(shape).digest("hex").slice(0, 12);
 }
 
 function isInternalRecoveryAbort(
@@ -487,6 +481,13 @@ class ChatDirectorImpl extends DefaultDirector {
   private readonly askOperatorCalls = new Set<string>();
   private readonly _systemPrompt: string;
   private _toolDefinitions: ToolDefinition[];
+  // Digest of the installed _toolDefinitions, stored when the array is set so
+  // updateToolDefinitions can compare an incoming set without re-hashing the
+  // current one. One slot per director — the digest is only consumed by the
+  // next update on this instance, never by shared/static context (all
+  // production call sites are here). Stale only if a caller mutates the
+  // installed array in place; callers swap arrays wholesale instead.
+  private _toolSetDigest: string | undefined;
   private inactivityTimeoutMs: number | undefined;
   private totalTimeoutMs: number | undefined;
   // Host-owned live object, attached via setWorkflowCoordinator — never a
@@ -711,9 +712,13 @@ class ChatDirectorImpl extends DefaultDirector {
   }
 
   updateToolDefinitions(toolDefinitions: ToolDefinition[]): void {
-    const before = toolSetDigest(this._toolDefinitions);
+    // The stored digest covers the current set (installed by the constructor
+    // or a prior update); only the very first update has to hash it, and the
+    // fallback runs once per director.
+    const before = this._toolSetDigest ?? toolSetDigest(this._toolDefinitions);
     const after = toolSetDigest(toolDefinitions);
     this._toolDefinitions = toolDefinitions;
+    this._toolSetDigest = after;
     if (before === after) return;
     logger.debug`tool-set-changed count=${String(this._toolDefinitions.length)} before=${before} after=${after}`;
   }

src/agent/tool-set-digest.test.ts

@@ -1,11 +1,12 @@
 import { describe, expect, test } from "bun:test";
 import { withMockedModule } from "../../testkit/mock-module.js";
 
-// Spy on createHash to prove toolSetDigest memoizes: repeated digest calls
-// with the same tool array must run the sha256 exactly once, and a new array
-// (tools changed) must run it again. The mock delegates to the real
-// implementation (captured before mocking) so behavior is unchanged;
-// hashCalls only measures the delta around each assertion.
+// Spy on createHash to prove toolSetDigest memoizes per director: installing
+// a tool set runs the sha256 exactly once for that set, and comparing a later
+// update against the stored per-director digest must not re-hash the previous
+// set. The mock delegates to the real implementation (captured before mocking)
+// so behavior is unchanged; hashCalls only measures the delta around each
+// assertion.
 let hashCalls = 0;
 await withMockedModule<typeof import("node:crypto")>(
   import.meta.resolve("node:crypto"),
@@ -19,7 +20,7 @@ await withMockedModule<typeof import("node:crypto")>(
 );
 
 // Dynamic import so director.ts resolves node:crypto through the mock above.
-const { toolSetDigest } = await import("./director.js");
+const { createChatDirector } = await import("./director.js");
 
 const base = {
   name: "read_file",
@@ -28,26 +29,32 @@ const base = {
 };
 
 describe("toolSetDigest memoization", () => {
-  test("hashes once per tool set across repeated digest calls", () => {
+  test("hashes each installed tool set once per director", () => {
+    const director = createChatDirector("system", [], {});
     const tools = [{ ...base }];
+    const changed = [{ ...base, description: "changed" }];
+
+    // Warm-up installs `tools`; its digest is stored on the director.
+    director.updateToolDefinitions(tools);
     hashCalls = 0;
-    const first = toolSetDigest(tools);
-    const second = toolSetDigest(tools);
-    const third = toolSetDigest(tools);
-    expect(second).toBe(first);
-    expect(third).toBe(first);
+    // The previous set's digest comes from the per-director field (no re-hash);
+    // only the incoming set is hashed.
+    director.updateToolDefinitions(changed);
     expect(hashCalls).toBe(1);
   });
 
   test("recomputes when a new tool set arrives", () => {
+    const director = createChatDirector("system", [], {});
     const tools = [{ ...base }];
     const changed = [{ ...base, description: "changed" }];
+
+    director.updateToolDefinitions(tools);
+    director.updateToolDefinitions(changed);
     hashCalls = 0;
-    const digest = toolSetDigest(tools);
+    // A fresh reference (even with identical content) misses the stored slot
+    // and re-runs the hash; the digest gates the tool-set-changed log, so
+    // identical content still logs nothing.
+    director.updateToolDefinitions([{ ...base }]);
     expect(hashCalls).toBe(1);
-    const changedDigest = toolSetDigest(changed);
-    expect(hashCalls).toBe(2);
-    expect(changedDigest).not.toBe(toolSetDigest(tools));
-    expect(digest).toBe(toolSetDigest(tools));
   });
 });