Skip to content

Commit 05522d2

Browse files
committed
fix(tools): put tool_search promotions on the next infer wire
Promotion opened the call gate but schemas stayed off the provider tools array until compaction, so MCP names could not be emitted.
1 parent d0a1dcf commit 05522d2

7 files changed

Lines changed: 60 additions & 104 deletions

File tree

‎src/director.test.ts‎

Lines changed: 15 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1242,13 +1242,10 @@ describe("updateToolDefinitions rewrites infer tools", () => {
12421242
expect(JSON.stringify(after)).toBe(JSON.stringify(before));
12431243
});
12441244

1245-
// CL-7868 (direction A): the provider cache is a prefix cache keyed on the
1246-
// tools array, so a tool_search turn that promotes a genuinely new tool must
1247-
// not reshape the wire set mid-session. The call gate opens (the model
1248-
// invokes the tool from the search result's schema) while the advertised
1249-
// array holds steady; the growth event lands at the next cache-safe
1250-
// boundary (compaction fold), appended after the untouched fixed prefix.
1251-
test("a tool_search turn promoting a genuinely new tool leaves the wire byte-identical until the fold commits it", async () => {
1245+
// Promotion must open the call gate and commit the schema onto the next
1246+
// infer tools array. Holding growth until compaction left MCP names
1247+
// dispatchable in the runner but missing from the provider tools list.
1248+
test("a tool_search turn promoting a genuinely new tool commits it onto the next infer wire", async () => {
12521249
const linearTool = {
12531250
name: "mcp__linear__list_issues",
12541251
description: "list issues",
@@ -1289,42 +1286,26 @@ describe("updateToolDefinitions rewrites infer tools", () => {
12891286
expect(advertised.activated.activate(["mcp__linear__list_issues"])).toBe(
12901287
true,
12911288
);
1289+
expect(advertised.flushPromotions()).toBe(true);
12921290
director.updateToolDefinitions(
12931291
advertised.computeAdvertised(toolset.dynamicRunner.currentDefinitions()),
12941292
);
12951293

1296-
// Mid-session the wire is byte-identical — the hot prefix never grows —
1297-
// while the gate is open so the model can invoke the match from the
1298-
// result card's schema.
12991294
const after = await firstInferTools(
13001295
director,
13011296
makeMessageReceivedEvent("continue"),
13021297
);
1303-
expect(JSON.stringify(after)).toBe(JSON.stringify(before));
1298+
expect(JSON.stringify(after)).not.toBe(JSON.stringify(before));
13041299
expect(advertised.isAdvertised("mcp__linear__list_issues")).toBe(true);
1305-
1306-
// Cache-safe boundary (compaction fold): the pending promotion commits and
1307-
// the next turn declares it after the untouched fixed prefix.
1308-
expect(advertised.flushPromotions()).toBe(true);
1309-
director.updateToolDefinitions(
1310-
advertised.computeAdvertised(toolset.dynamicRunner.currentDefinitions()),
1311-
);
1312-
const folded = await firstInferTools(
1313-
director,
1314-
makeMessageReceivedEvent("after fold"),
1315-
);
1316-
const foldedNames = (folded as { name: string }[]).map((t) => t.name);
1317-
expect(foldedNames).toContain("mcp__linear__list_issues");
1300+
const afterNames = (after as { name: string }[]).map((t) => t.name);
1301+
expect(afterNames).toContain("mcp__linear__list_issues");
13181302
const beforeNames = (before as { name: string }[])
13191303
.map((t) => t.name)
13201304
.filter((n) => n !== "submit_output");
1321-
const foldedPrefix = foldedNames.filter(
1305+
const afterPrefix = afterNames.filter(
13221306
(n) => n !== "submit_output" && n !== "mcp__linear__list_issues",
13231307
);
1324-
expect(foldedPrefix).toEqual(beforeNames);
1325-
expect(foldedNames.indexOf("mcp__linear__list_issues")).toBe(
1326-
beforeNames.length,
1327-
);
1308+
expect(afterPrefix).toEqual(beforeNames);
13281309

13291310
await toolset.dispose();
13301311
});
@@ -1347,13 +1328,9 @@ describe("updateToolDefinitions rewrites infer tools", () => {
13471328
expect(inferToolNames(inferAction)).toContain("submit_output");
13481329
});
13491330

1350-
// End-to-end: tool_search matches an MCP tool; the runner's gate-only promote
1351-
// wiring (mirrored here via createAdvertisedToolset + updateToolDefinitions)
1352-
// keeps it off the wire the next turn — the hot prefix stays byte-identical
1353-
// while the gate lets the model call it from the result card. The fold
1354-
// commits the pending promotion with the tool's full definition, appended
1355-
// after the untouched fixed prefix, and the array then holds steady.
1356-
test("a tool_search match stays off the wire until the fold, then holds stable", async () => {
1331+
// tool_search + flush + updateToolDefinitions puts the match on the next
1332+
// infer wire; a further turn with no new discovery stays byte-identical.
1333+
test("a tool_search match is on the next infer wire and then holds stable", async () => {
13571334
const linearTool = {
13581335
name: "mcp__linear__list_issues",
13591336
description: "list issues",
@@ -1399,38 +1376,19 @@ describe("updateToolDefinitions rewrites infer tools", () => {
13991376
// Simulate the runner's gate-only promoter plus its boundary wire push:
14001377
// the gate opens but the wire recompute is byte-identical until the fold.
14011378
advertised.activated.activate(["mcp__linear__list_issues"]);
1402-
director.updateToolDefinitions(
1403-
computeAdvertised(toolset.dynamicRunner.currentDefinitions()),
1404-
);
1405-
1406-
const gated = await firstInferTools(
1407-
director,
1408-
makeMessageReceivedEvent("continue"),
1409-
);
1410-
expect((gated as { name: string }[]).map((t) => t.name)).not.toContain(
1411-
"mcp__linear__list_issues",
1412-
);
1413-
expect(JSON.stringify(gated)).toBe(beforeJson);
1414-
1415-
// Simulate the compaction fold: commit the pending promotion, push the
1416-
// refreshed set, and the tool is declared — appended after the fixed
1417-
// built-in prefix, which survives untouched ahead of it.
14181379
advertised.flushPromotions();
14191380
director.updateToolDefinitions(
14201381
computeAdvertised(toolset.dynamicRunner.currentDefinitions()),
14211382
);
14221383

14231384
const after = await firstInferTools(
14241385
director,
1425-
makeMessageReceivedEvent("after fold"),
1386+
makeMessageReceivedEvent("continue"),
14261387
);
14271388
const afterTools = after as { name: string }[];
14281389
const afterNames = afterTools.map((t) => t.name);
14291390
expect(afterNames).toContain("mcp__linear__list_issues");
1430-
// submit_output rides along separately (see withCurrentTools), appended
1431-
// after computeAdvertised's result every turn — strip it before comparing
1432-
// the fixed built-in prefix, which must survive untouched ahead of the
1433-
// newly appended MCP tool.
1391+
expect(JSON.stringify(after)).not.toBe(beforeJson);
14341392
const beforePrefix = beforeNames.filter((n) => n !== "submit_output");
14351393
const afterPrefix = afterNames.filter(
14361394
(n) => n !== "submit_output" && n !== "mcp__linear__list_issues",
@@ -1440,13 +1398,11 @@ describe("updateToolDefinitions rewrites infer tools", () => {
14401398
beforePrefix.length,
14411399
);
14421400

1443-
// A further turn with no new discovery stays byte-identical to `after`.
14441401
const stable = await firstInferTools(
14451402
director,
14461403
makeMessageReceivedEvent("keep going"),
14471404
);
14481405
expect(JSON.stringify(stable)).toBe(JSON.stringify(after));
1449-
expect(JSON.stringify(after)).not.toBe(beforeJson);
14501406

14511407
await toolset.dispose();
14521408
});

‎src/exec/runner.test.ts‎

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -81,39 +81,28 @@ describe("exec director allowlist", () => {
8181
).toBe(false);
8282
});
8383

84-
test("the promoter gates allow itself and leaves the wire set for the fold", () => {
84+
test("the promoter gates allow itself and commits the match onto the wire", () => {
8585
const overlay = resolveExecDirectorOverlay("explorer");
86-
const { activated, computeAdvertised, flushPromotions } =
87-
createAdvertisedToolset({
88-
sessionMode: "orchestrator",
89-
toolAvailability: { languageServerAvailable: true },
90-
getProvider: () => ({ providerName: "test", model: "test-model" }),
91-
builtInPrefix: overlay.advertisedAllow,
92-
});
86+
const { activated, flushPromotions } = createAdvertisedToolset({
87+
sessionMode: "orchestrator",
88+
toolAvailability: { languageServerAvailable: true },
89+
getProvider: () => ({ providerName: "test", model: "test-model" }),
90+
builtInPrefix: overlay.advertisedAllow,
91+
});
92+
let committed = 0;
9393
const promote = createExecToolPromoter({
9494
activate: (names) => activated.activate(names),
9595
isAllowed: (name) => isExecOverlayToolAllowed(overlay, name),
96-
});
97-
const registry = [
98-
{
99-
name: "read_file",
100-
description: "read a file",
101-
inputSchema: { type: "object", properties: {} },
96+
commitWire: () => {
97+
committed += 1;
98+
flushPromotions();
10299
},
103-
];
104-
const before = JSON.stringify(computeAdvertised(registry));
105-
// A raw activate caller gets no bypass: outside-allow names never open the
106-
// gate, while the allowed match opens it at once.
100+
});
107101
promote([OUTSIDE_ALLOW, "read_file"]);
108102
expect(activated.has(OUTSIDE_ALLOW)).toBe(false);
109103
expect(activated.has("read_file")).toBe(true);
110-
// Gate-only: the wire recompute ignores the fresh activation, so the
111-
// provider's cached prefix holds until the fold commits it.
112-
expect(JSON.stringify(computeAdvertised(registry))).toBe(before);
113-
expect(flushPromotions()).toBe(true);
114-
expect(computeAdvertised(registry).map((d) => d.name)).toContain(
115-
"read_file",
116-
);
104+
expect(committed).toBe(1);
105+
expect(flushPromotions()).toBe(false);
117106
});
118107

119108
test("skywalker overlay leaves every tool allowed", () => {

‎src/exec/runner.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -415,13 +415,11 @@ export function createExecToolPromoter(args: {
415415
activate: (names: readonly string[]) => boolean;
416416
isAllowed: (name: string) => boolean;
417417
persist?: () => void;
418+
commitWire?: () => void;
418419
}): (names: string[]) => void {
419420
return (names) => {
420-
// Gate-only, like the TUI promoteTools: activation lets the model invoke
421-
// the match from the result card's schema at once, while the schema joins
422-
// the wire set at the next cache-safe boundary (compaction fold) so the
423-
// provider's cached prefix never grows mid-thread (CL-7868).
424421
if (!args.activate(names.filter((name) => args.isAllowed(name)))) return;
422+
args.commitWire?.();
425423
args.persist?.();
426424
};
427425
}
@@ -912,6 +910,13 @@ export async function runExec(config: Config): Promise<ExecResult> {
912910
persist: () => {
913911
void persist("running");
914912
},
913+
commitWire: () => {
914+
if (flushPromotions()) {
915+
directorHolder.instance?.updateToolDefinitions(
916+
computeAdvertised(agentToolset.dynamicRunner.currentDefinitions()),
917+
);
918+
}
919+
},
915920
}),
916921
);
917922

‎src/tui/runner/exit.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -406,14 +406,20 @@ export async function createRunLifecycle(
406406
state.reloadIfIdle = reloadIfIdle;
407407

408408
// tool_search (and contextual triggers, e.g. the lsp hint) promote tools by
409-
// opening the call gate: the model invokes the match from the result card's
410-
// schema on the very next turn. The schema itself joins the wire set at the
411-
// next cache-safe boundary (compaction fold), never mid-thread — the
412-
// serialized tools array heads the provider's cached prefix (CL-7868). No
413-
// reload is scheduled: dispatchability comes from the live call gate, not
414-
// the rebuilt agent.
409+
// opening the call gate and committing schemas onto the next infer's wire.
410+
// Holding them off until compaction (CL-7868) left MCP/plugin names callable
411+
// in the runner but missing from the provider tools array, so the model
412+
// could not emit those calls. Cache prefix growth on discovery is the cost
413+
// of making promotion actually work.
415414
const promoteTools = (names: string[]): void => {
416415
if (!services.activatedToolNames.activate(names)) return;
416+
if (services.flushPromotions()) {
417+
services.directorHolder.instance?.updateToolDefinitions(
418+
services.computeAdvertised(
419+
services.toolset.dynamicRunner.currentDefinitions(),
420+
),
421+
);
422+
}
417423
// Activation is model-visible contract — persist it now so a crash or
418424
// restart before the next turn boundary does not strand the transcript's
419425
// "these tools are available" record.

‎src/tui/runner/session.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -718,6 +718,7 @@ export async function assembleTUISession(
718718
workflowHost,
719719
activatedToolNames,
720720
computeAdvertised,
721+
flushPromotions,
721722
buildAgent: chatAgent.buildAgent,
722723
sessionCost,
723724
sessionOps,

‎src/tui/runner/state.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,11 @@ export interface RunnerServices {
151151
typeof import("../../session/assemble-runtime.js").createAdvertisedToolset
152152
>
153153
>["computeAdvertised"];
154+
flushPromotions: Awaited<
155+
ReturnType<
156+
typeof import("../../session/assemble-runtime.js").createAdvertisedToolset
157+
>
158+
>["flushPromotions"];
154159
buildAgent: ReturnType<
155160
typeof import("../../session/assemble-runtime.js").assembleChatAgent
156161
>["buildAgent"];

‎tests/unit/exec/runner.test.ts‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,9 @@ describe("exec tool call gate and promoter", () => {
787787
persist: () => {
788788
persistCount += 1;
789789
},
790+
commitWire: () => {
791+
flushPromotions();
792+
},
790793
});
791794
const search = createToolSearchTool({
792795
search: (query) =>
@@ -821,26 +824,17 @@ describe("exec tool call gate and promoter", () => {
821824
expect(blocked.isError).toBe(true);
822825
expect(blocked.content).toContain("tool_search");
823826

824-
const wireBefore = JSON.stringify(
825-
computeAdvertised(runner.currentDefinitions()),
826-
);
827827
if (search.kind !== "string") throw new Error("expected string tool");
828828
await search.handler({ query: "linear" }, new AbortController().signal);
829829
expect(persistCount()).toBe(1);
830830

831-
// Gate-only promotion: the tool dispatches now, but the wire set holds
832-
// steady until a cache-safe boundary commits it.
833831
const allowed = await dispatch(runner, "mcp__linear__save_issue");
834832
expect(allowed.content).toBe("saved");
835833
expect(allowed.isError).toBeUndefined();
836-
expect(JSON.stringify(computeAdvertised(runner.currentDefinitions()))).toBe(
837-
wireBefore,
838-
);
839-
840-
expect(flushPromotions()).toBe(true);
841834
expect(
842835
computeAdvertised(runner.currentDefinitions()).map((d) => d.name),
843836
).toContain("mcp__linear__save_issue");
837+
expect(flushPromotions()).toBe(false);
844838
});
845839

846840
test("present and plugin names pass the gate after tool_search promote", async () => {

0 commit comments

Comments
 (0)