Skip to content

Commit dbaf3e9

Browse files
committed
fix(agent): memoize toolSetDigest per tool-set identity
toolSetDigest re-hashed the full tool array on every call. Cache the digest in a WeakMap keyed by 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) recomputes. A WeakMap also avoids retaining arrays the director has dropped. Output is byte-identical for the same tool set. Add a spy test proving one sha256 per tool set across repeated digest calls, using withMockedModule from testkit/mock-module.ts.
1 parent 40e19ce commit dbaf3e9

2 files changed

Lines changed: 66 additions & 1 deletion

File tree

‎src/agent/director.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,26 @@ const logger = getLogger([LOG_NAMESPACE_ROOT, "agent", "director"]);
5959
// (mount, description edit, reorder) drops the next turn's cache hit. The
6060
// digest stays deterministic — it only moves when discovery grows the set.
6161
// Hashed: MCP descriptions are arbitrary server text, not log material.
62+
//
63+
// Memoized per array identity: the director owns stable tool arrays and swaps
64+
// them wholesale (constructor, updateToolDefinitions) without in-place
65+
// mutation, so the same reference always means the same content and a new
66+
// reference (tools changed) misses the WeakMap and recomputes. A WeakMap also
67+
// avoids retaining arrays the director has dropped.
68+
const toolSetDigestCache = new WeakMap<readonly ToolDefinition[], string>();
69+
6270
export function toolSetDigest(tools: readonly ToolDefinition[]): string {
71+
const cached = toolSetDigestCache.get(tools);
72+
if (cached !== undefined) return cached;
6373
const shape = tools
6474
.map(
6575
(t) =>
6676
`${t.name}:${t.description ?? ""}:${JSON.stringify(t.inputSchema ?? null)}`,
6777
)
6878
.join("|");
69-
return createHash("sha256").update(shape).digest("hex").slice(0, 12);
79+
const digest = createHash("sha256").update(shape).digest("hex").slice(0, 12);
80+
toolSetDigestCache.set(tools, digest);
81+
return digest;
7082
}
7183

7284
function isInternalRecoveryAbort(

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { withMockedModule } from "../../testkit/mock-module.js";
3+
4+
// Spy on createHash to prove toolSetDigest memoizes: repeated digest calls
5+
// with the same tool array must run the sha256 exactly once, and a new array
6+
// (tools changed) must run it again. The mock delegates to the real
7+
// implementation (captured before mocking) so behavior is unchanged;
8+
// hashCalls only measures the delta around each assertion.
9+
let hashCalls = 0;
10+
await withMockedModule<typeof import("node:crypto")>(
11+
import.meta.resolve("node:crypto"),
12+
(real) => ({
13+
...real,
14+
createHash: (algorithm: string) => {
15+
hashCalls += 1;
16+
return real.createHash(algorithm);
17+
},
18+
}),
19+
);
20+
21+
// Dynamic import so director.ts resolves node:crypto through the mock above.
22+
const { toolSetDigest } = await import("./director.js");
23+
24+
const base = {
25+
name: "read_file",
26+
description: "read a file",
27+
inputSchema: { type: "object" },
28+
};
29+
30+
describe("toolSetDigest memoization", () => {
31+
test("hashes once per tool set across repeated digest calls", () => {
32+
const tools = [{ ...base }];
33+
hashCalls = 0;
34+
const first = toolSetDigest(tools);
35+
const second = toolSetDigest(tools);
36+
const third = toolSetDigest(tools);
37+
expect(second).toBe(first);
38+
expect(third).toBe(first);
39+
expect(hashCalls).toBe(1);
40+
});
41+
42+
test("recomputes when a new tool set arrives", () => {
43+
const tools = [{ ...base }];
44+
const changed = [{ ...base, description: "changed" }];
45+
hashCalls = 0;
46+
const digest = toolSetDigest(tools);
47+
expect(hashCalls).toBe(1);
48+
const changedDigest = toolSetDigest(changed);
49+
expect(hashCalls).toBe(2);
50+
expect(changedDigest).not.toBe(toolSetDigest(tools));
51+
expect(digest).toBe(toolSetDigest(tools));
52+
});
53+
});

0 commit comments

Comments
 (0)