Skip to content

Commit d677b90

Browse files
committed
fix(tools): keep list_dir off the advertised wire on execute
Promote-on-execute flushed any registered unadvertised name, so a list_dir call put its schema on the wire until fold. Search already hides that builtin in favor of glob; dispatch it without advertising.
1 parent 9f926eb commit d677b90

8 files changed

Lines changed: 136 additions & 11 deletions

File tree

‎src/agent/tool-search.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,7 @@ describe("promote-on-execute", () => {
684684
"ran:never",
685685
"A tool the model never searched for",
686686
),
687+
stringTool("list_dir", "listed", "list a directory's entries"),
687688
];
688689
const runner = createDynamicToolRunner(tools);
689690
const advertised = createAdvertisedToolset({
@@ -773,6 +774,18 @@ describe("promote-on-execute", () => {
773774
expect(promoted).toEqual(["mcp__never__searched"]);
774775
expect(advertisedNames(advertised, runner)).toContain("mcp__never__searched");
775776
});
777+
778+
test("executing list_dir does not join the advertised tail", async () => {
779+
const { advertised, promoted, runner } = wirePromoteOnExecute();
780+
const before = advertisedNames(advertised, runner);
781+
expect(before).not.toContain("list_dir");
782+
const result = await dispatch(runner, "list_dir");
783+
expect(result.content).toBe("listed");
784+
expect(result.isError).toBeUndefined();
785+
expect(promoted).toEqual([]);
786+
expect(advertisedNames(advertised, runner)).toEqual(before);
787+
expect(advertisedNames(advertised, runner)).not.toContain("list_dir");
788+
});
776789
});
777790

778791
describe("advertisedTools", () => {

‎src/agent/tool-search.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,9 @@ function isAlreadyAdvertised(
162162
}
163163

164164
// Mounted built-ins that stay off the advertised prefix and off tool_search.
165-
const UNADVERTISED_MOUNTED_BUILTINS = new Set(["list_dir"]);
165+
// Promote-on-execute must not flush these onto the wire either — dispatch
166+
// without advertising. glob is the advertised replacement for list_dir.
167+
export const UNADVERTISED_MOUNTED_BUILTINS = new Set(["list_dir"]);
166168

167169
// Project the live tool registry onto the advertised set: the fixed built-in
168170
// prefix (its order never changes — this is what keeps the provider cache

‎src/exec/runner.test.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,42 @@ describe("exec director allowlist", () => {
244244
expect(names).not.toContain(OUTSIDE_ALLOW);
245245
});
246246

247+
test("the promoter does not commit list_dir onto the advertised wire", () => {
248+
const { activated, computeAdvertised, flushPromotions } =
249+
createAdvertisedToolset({
250+
sessionMode: "orchestrator",
251+
toolAvailability: { languageServerAvailable: true },
252+
getProvider: () => ({ providerName: "test", model: "test-model" }),
253+
});
254+
let committed = 0;
255+
const promote = createExecToolPromoter({
256+
activate: (names) => activated.activate(names),
257+
isAllowed: () => true,
258+
commitWire: () => {
259+
if (flushPromotions()) committed += 1;
260+
},
261+
});
262+
const registry = [
263+
{
264+
name: "list_dir",
265+
description: "list a directory",
266+
inputSchema: { type: "object", properties: {} },
267+
},
268+
{
269+
name: "mcp__acme__do",
270+
description: "do a thing",
271+
inputSchema: { type: "object", properties: {} },
272+
},
273+
];
274+
promote(["list_dir", "mcp__acme__do"]);
275+
expect(activated.has("list_dir")).toBe(false);
276+
expect(activated.has("mcp__acme__do")).toBe(true);
277+
expect(committed).toBe(1);
278+
const names = computeAdvertised(registry).map((d) => d.name);
279+
expect(names).not.toContain("list_dir");
280+
expect(names).toContain("mcp__acme__do");
281+
});
282+
247283
test("skywalker overlay leaves every tool allowed", () => {
248284
const overlay = resolveExecDirectorOverlay("skywalker");
249285
expect(isExecOverlayToolAllowed(overlay, OUTSIDE_ALLOW)).toBe(true);
@@ -652,6 +688,7 @@ describe("exec tool call gate and promoter", () => {
652688
"search and render layout primitives for pages",
653689
),
654690
stringTool("plugin__notes__save", "noted", "Save granola notes"),
691+
stringTool("list_dir", "listed", "list a directory's entries"),
655692
stringTool(submitOutputDefinition.name, "submitted", "submit output"),
656693
]);
657694
const { activated, isAdvertised, computeAdvertised, flushPromotions } =
@@ -760,4 +797,21 @@ describe("exec tool call gate and promoter", () => {
760797
expect(result.content).toBe("submitted");
761798
expect(result.isError).toBeUndefined();
762799
});
800+
801+
test("executing list_dir does not add it to computeAdvertised", async () => {
802+
const { runner, computeAdvertised } = wireExecDiscovery();
803+
const before = computeAdvertised(runner.currentDefinitions()).map(
804+
(d) => d.name,
805+
);
806+
expect(before).not.toContain("list_dir");
807+
const result = await dispatch(runner, "list_dir");
808+
expect(result.content).toBe("listed");
809+
expect(result.isError).toBeUndefined();
810+
expect(
811+
computeAdvertised(runner.currentDefinitions()).map((d) => d.name),
812+
).toEqual(before);
813+
expect(
814+
computeAdvertised(runner.currentDefinitions()).map((d) => d.name),
815+
).not.toContain("list_dir");
816+
});
763817
});

‎src/exec/runner.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
import {
2929
type ActivatedToolTracker,
3030
type ToolAvailability,
31+
UNADVERTISED_MOUNTED_BUILTINS,
3132
} from "../agent/tool-search.js";
3233
import { detectLanguageServerAvailable } from "../agent/lsp-availability.js";
3334
import {
@@ -412,7 +413,11 @@ export function createExecToolPromoter(args: {
412413
commitWire?: () => void;
413414
}): (names: string[]) => void {
414415
return (names) => {
415-
if (!args.activate(names.filter((name) => args.isAllowed(name)))) return;
416+
const admitted = names.filter(
417+
(name) =>
418+
args.isAllowed(name) && !UNADVERTISED_MOUNTED_BUILTINS.has(name),
419+
);
420+
if (!args.activate(admitted)) return;
416421
args.commitWire?.();
417422
args.persist?.();
418423
};

‎src/session/assemble-runtime.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,22 @@ describe("createAdvertisedToolset", () => {
140140
expect(isAdvertised("mcp__acme__do")).toBe(true);
141141
});
142142

143+
test("flushPromotions does not commit unadvertised mounted builtins onto the wire", () => {
144+
const { activated, computeAdvertised, flushPromotions } =
145+
createAdvertisedToolset(wiring());
146+
const registry = [def("read_file"), def("list_dir"), def("mcp__acme__do")];
147+
expect(activated.activate(["list_dir"])).toBe(true);
148+
expect(flushPromotions()).toBe(false);
149+
expect(computeAdvertised(registry).map((d) => d.name)).not.toContain(
150+
"list_dir",
151+
);
152+
expect(activated.activate(["mcp__acme__do"])).toBe(true);
153+
expect(flushPromotions()).toBe(true);
154+
const names = computeAdvertised(registry).map((d) => d.name);
155+
expect(names).not.toContain("list_dir");
156+
expect(names).toContain("mcp__acme__do");
157+
});
158+
143159
test("pruneIdlePromotions drops execute-promoted schemas and keeps the frozen prefix", () => {
144160
const { activated, computeAdvertised, flushPromotions, pruneIdlePromotions } =
145161
createAdvertisedToolset(

‎src/session/assemble-runtime.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import {
4646
advertisedTools,
4747
advertisedToolNamesForSessionMode,
4848
createActivatedToolTracker,
49+
UNADVERTISED_MOUNTED_BUILTINS,
4950
type ActivatedToolTracker,
5051
type ToolAvailability,
5152
} from "../agent/tool-search.js";
@@ -467,6 +468,9 @@ export function createAdvertisedToolset(args: {
467468
let grew = false;
468469
for (const name of activated.list()) {
469470
if (wireActivatedSet.has(name)) continue;
471+
// Search hides these; flushing would put the schema on the wire until
472+
// fold. Dispatch stays available via intercept without advertising.
473+
if (UNADVERTISED_MOUNTED_BUILTINS.has(name)) continue;
470474
wireActivatedSet.add(name);
471475
wireActivated.push(name);
472476
grew = true;

‎src/tui/dynamic-tool-runner.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,31 @@ describe("call gate", () => {
134134
expect(promoted).toEqual(["mcp__acme__do"]);
135135
expect(advertised.has("mcp__acme__other")).toBe(false);
136136
});
137+
138+
test("list_dir dispatches under a call gate without promoting", async () => {
139+
const runner = createDynamicToolRunner([
140+
stringTool("read_file", "core"),
141+
stringTool("list_dir", "listed"),
142+
stringTool("mcp__acme__do", "blind-result"),
143+
]);
144+
const advertised = new Set(["read_file"]);
145+
const promoted: string[] = [];
146+
runner.setCallGate((name) => advertised.has(name));
147+
runner.setOnUndeclaredCall((name) => {
148+
promoted.push(name);
149+
advertised.add(name);
150+
});
151+
152+
const result = await runner.run(
153+
{ id: "1", name: "list_dir", arguments: {} },
154+
new AbortController().signal,
155+
);
156+
expect(result.content).toBe("listed");
157+
expect(result.isError).toBeUndefined();
158+
expect(promoted).toEqual([]);
159+
expect(advertised.has("list_dir")).toBe(false);
160+
expect(advertised.has("mcp__acme__do")).toBe(false);
161+
});
137162
});
138163

139164
describe("mangled dispatch names", () => {

‎src/tui/dynamic-tool-runner.ts‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
import { resolveRegisteredToolName } from "./resolve-registered-tool-name.js";
1414
import { stripTerminalControlSequences } from "../util/control-char-strip.js";
1515
import { prepareDispatchedToolCall } from "../agent/tool-aliases.js";
16+
import { UNADVERTISED_MOUNTED_BUILTINS } from "../agent/tool-search.js";
1617

1718
// A tool runner whose set of tools can grow after construction. The static
1819
// createToolRunner freezes its name map at build time, which cannot accommodate
@@ -156,16 +157,21 @@ export function createDynamicToolRunner(
156157
// then run the call. Search does not pre-promote the match set. If the
157158
// promoter cannot admit the name (closed overlay, denied family), the
158159
// gate still fails and the model is pointed at tool_search.
160+
// Unadvertised mounted builtins (list_dir) stay off the wire — search
161+
// hides them in favor of glob — so intercept dispatches without
162+
// flushPromotions rather than advertising until fold.
159163
if (callGate !== undefined && !callGate(resolved)) {
160-
onUndeclaredCall?.(resolved);
161-
if (!callGate(resolved)) {
162-
return {
163-
callId: call.id,
164-
content:
165-
`Error: ${resolved} is not in the currently advertised tool list. ` +
166-
`Call tool_search to find it, then retry the call.`,
167-
isError: true,
168-
};
164+
if (!UNADVERTISED_MOUNTED_BUILTINS.has(resolved)) {
165+
onUndeclaredCall?.(resolved);
166+
if (!callGate(resolved)) {
167+
return {
168+
callId: call.id,
169+
content:
170+
`Error: ${resolved} is not in the currently advertised tool list. ` +
171+
`Call tool_search to find it, then retry the call.`,
172+
isError: true,
173+
};
174+
}
169175
}
170176
}
171177
let dispatchCall: ToolCall;

0 commit comments

Comments
 (0)