Skip to content

Commit da1847c

Browse files
Merge pull request #1452 from corbitsdev/fix/worker-family-tool-names
fix(agent): fold gpt tool names for workers
2 parents 8fabd1c + fb9f15f commit da1847c

12 files changed

Lines changed: 666 additions & 91 deletions

‎scripts/dead-export-guard.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
{
22
"tsconfig": "tsconfig.json",
3-
"minScannedFiles": 1240
3+
"minScannedFiles": 1366
44
}

‎src/agent/prompt-sizes.test.ts‎

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -204,15 +204,33 @@ describe("director prompt size budget", () => {
204204
expect(new Set(names).size, `${directorId} [${family}]`).toBe(
205205
names.length,
206206
);
207-
// Hidden Codex aliases (apply_patch/shell/update_plan) are not advertised,
208-
// as must list_dir, which no subagent mount installs.
209-
for (const phantom of [
210-
"list_dir",
211-
"apply_patch",
212-
"shell",
213-
"update_plan",
214-
]) {
215-
expect(names, `${directorId} [${family}]`).not.toContain(phantom);
207+
// list_dir is not installed on a subagent mount. apply_patch, shell,
208+
// and update_plan are the gpt wire names — other families keep the
209+
// default wires.
210+
expect(names, `${directorId} [${family}]`).not.toContain("list_dir");
211+
if (family === "gpt") {
212+
const defaults = canonicalToolNamesForDirector(
213+
DIRECTOR_REGISTRY[directorId],
214+
"default",
215+
);
216+
expect(names.includes("apply_patch"), directorId).toBe(
217+
defaults.includes("write"),
218+
);
219+
expect(names.includes("shell"), directorId).toBe(
220+
defaults.includes("bash"),
221+
);
222+
expect(names.includes("update_plan"), directorId).toBe(
223+
defaults.includes("todowrite"),
224+
);
225+
expect(names, directorId).not.toContain("write");
226+
expect(names, directorId).not.toContain("edit");
227+
expect(names, directorId).not.toContain("delete");
228+
expect(names, directorId).not.toContain("bash");
229+
expect(names, directorId).not.toContain("todowrite");
230+
} else {
231+
for (const phantom of ["apply_patch", "shell", "update_plan"]) {
232+
expect(names, `${directorId} [${family}]`).not.toContain(phantom);
233+
}
216234
}
217235
if (DIRECTOR_REGISTRY[directorId].spawn.maySpawn) {
218236
for (const verb of ["spawn_agent", "send_input"]) {
@@ -225,6 +243,25 @@ describe("director prompt size budget", () => {
225243
}
226244
}
227245
});
246+
247+
test("gpt shakespeare lists apply_patch, not write/edit/delete", () => {
248+
const names = canonicalToolNamesForDirector(
249+
DIRECTOR_REGISTRY.shakespeare,
250+
"gpt",
251+
);
252+
expect(names).toContain("apply_patch");
253+
expect(names).not.toContain("write");
254+
expect(names).not.toContain("edit");
255+
expect(names).not.toContain("delete");
256+
expect(names).not.toContain("write_file");
257+
expect(names).not.toContain("shell");
258+
const defaults = canonicalToolNamesForDirector(
259+
DIRECTOR_REGISTRY.shakespeare,
260+
"default",
261+
);
262+
expect(defaults).not.toContain("apply_patch");
263+
expect(defaults).toContain("write");
264+
});
228265
});
229266

230267
describe("dispatch grok prefix (infer envelope vs trimmed director)", () => {

‎src/agent/prompt-sizes.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@ import {
1414
import { buildSubAgentSystemPrompt } from "./prompts.js";
1515
import { resolveModelFamilyPolicy } from "./model-family-policy.js";
1616
import { shouldApplyGrokAntiThrash } from "../subagent/provider-family.js";
17-
import { advertisedToolName } from "./tool-aliases.js";
17+
import {
18+
advertisedToolName,
19+
foldFileToolNames,
20+
toolProfileForFamily,
21+
} from "./tool-aliases.js";
1822
import { canonicalToolName } from "./canonical-tool-name.js";
1923
import { manageTasksDefinition } from "./tasks.js";
2024
import { DELETE_FILE_DEFINITION } from "../plugins/delete-file-plugin.js";
@@ -86,7 +90,7 @@ function preFilterMountNames(): readonly string[] {
8690
*/
8791
export function canonicalToolNamesForDirector(
8892
pkg: DirectorPackage,
89-
_family: PromptSizeFamily,
93+
family: PromptSizeFamily,
9094
): readonly string[] {
9195
const filtered = [...preFilterMountNames()];
9296
const capabilities = packageToCapabilities(pkg);
@@ -130,7 +134,11 @@ export function canonicalToolNamesForDirector(
130134
`canonicalToolNamesForDirector(${pkg.id}): "${dupe}" mounted twice — the assembly drifted from src/subagent/run.ts`,
131135
);
132136
}
133-
return names.map((name) => advertisedToolName(name));
137+
const profile = toolProfileForFamily(family);
138+
return foldFileToolNames(
139+
names.map((name) => advertisedToolName(name, profile)),
140+
profile,
141+
);
134142
}
135143

136144
/** Assemble one director prompt exactly as run.ts does. */

‎src/agent/tool-aliases.ts‎

Lines changed: 102 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,37 +42,117 @@ const ENGINE_TO_WIRE_BY_PROFILE: Record<
4242
gpt: { ...DEFAULT_ENGINE_TO_WIRE, ...GPT_OVERRIDES },
4343
};
4444

45-
/** Tools the gpt profile folds into the single apply_patch envelope tool. */
46-
const PATCH_FOLDED_ENGINES: ReadonlySet<string> = new Set([
47-
"write_file",
48-
"edit_file",
49-
"delete_file",
50-
]);
45+
/**
46+
* Engines one advertised wire replaces. A profile without an entry keeps
47+
* the 1:1 wire table. Adding a family is a new ToolProfile plus a row here
48+
* and in ENGINE_TO_WIRE_BY_PROFILE — not a new director package.
49+
*/
50+
const PROFILE_FOLDS: Record<
51+
ToolProfile,
52+
{ wire: string; engines: readonly string[] } | undefined
53+
> = {
54+
default: undefined,
55+
gpt: {
56+
wire: "apply_patch",
57+
engines: ["write_file", "edit_file", "delete_file"],
58+
},
59+
};
60+
61+
function buildAdvertisedWireEngines(
62+
profile: ToolProfile,
63+
): ReadonlyMap<string, readonly string[]> {
64+
const fold = PROFILE_FOLDS[profile];
65+
const folded = new Set(fold?.engines ?? []);
66+
const map = new Map<string, string[]>();
67+
for (const [engine, wire] of Object.entries(
68+
ENGINE_TO_WIRE_BY_PROFILE[profile],
69+
)) {
70+
if (folded.has(engine)) continue;
71+
const existing = map.get(wire);
72+
if (existing === undefined) map.set(wire, [engine]);
73+
else existing.push(engine);
74+
}
75+
if (fold !== undefined) map.set(fold.wire, [...fold.engines]);
76+
return map;
77+
}
78+
79+
const ADVERTISED_WIRE_ENGINES = Object.fromEntries(
80+
(Object.keys(ENGINE_TO_WIRE_BY_PROFILE) as ToolProfile[]).map((profile) => [
81+
profile,
82+
buildAdvertisedWireEngines(profile),
83+
]),
84+
) as Record<ToolProfile, ReadonlyMap<string, readonly string[]>>;
85+
86+
export function toolProfiles(): readonly ToolProfile[] {
87+
return Object.keys(ENGINE_TO_WIRE_BY_PROFILE) as ToolProfile[];
88+
}
89+
90+
/** Wire name → engine ids this profile advertises. Folded wires map to every engine they replace. */
91+
export function advertisedWireEngines(
92+
profile: ToolProfile,
93+
): ReadonlyMap<string, readonly string[]> {
94+
return ADVERTISED_WIRE_ENGINES[profile];
95+
}
96+
97+
/**
98+
* Engines `wire` stands for on `profile`. Undefined when this profile does
99+
* not advertise that name (a later family follows the same table).
100+
*/
101+
export function enginesForAdvertisedWire(
102+
wire: string,
103+
profile: ToolProfile,
104+
): readonly string[] | undefined {
105+
const table = ADVERTISED_WIRE_ENGINES[profile];
106+
return table.get(wire) ?? table.get(wire.toLowerCase());
107+
}
108+
109+
/** Wire this profile advertises for `engine`, including a fold that replaces it. */
110+
function profileWireName(engine: string, profile: ToolProfile): string {
111+
const fold = PROFILE_FOLDS[profile];
112+
if (fold !== undefined && fold.engines.includes(engine)) return fold.wire;
113+
return advertisedToolName(engine, profile);
114+
}
115+
116+
/** Every wire any profile advertises for `engine`, including fold wires. */
117+
export function advertisedNamesForEngine(engine: string): readonly string[] {
118+
const names: string[] = [];
119+
const seen = new Set<string>();
120+
for (const profile of toolProfiles()) {
121+
const wire = profileWireName(engine, profile);
122+
if (seen.has(wire)) continue;
123+
seen.add(wire);
124+
names.push(wire);
125+
}
126+
return names;
127+
}
51128

52129
/** gpt models are trained on apply_patch, not write/edit/delete. Other
53-
* profiles and lists without a file-mutation tool pass through unchanged. */
130+
* profiles and lists without a folded tool pass through unchanged. */
54131
export function foldFileToolNames(
55132
names: readonly string[],
56133
profile: ToolProfile,
57134
): readonly string[] {
58-
if (profile !== "gpt") return names;
135+
const fold = PROFILE_FOLDS[profile];
136+
if (fold === undefined) return names;
59137
const folded = (name: string): boolean =>
60-
PATCH_FOLDED_ENGINES.has(engineToolName(name));
138+
fold.engines.includes(engineToolName(name));
61139
if (!names.some(folded)) return names;
62140
const first = names.findIndex(folded);
63141
return names.flatMap((name, i) =>
64-
i === first ? ["apply_patch"] : folded(name) ? [] : [name],
142+
i === first ? [fold.wire] : folded(name) ? [] : [name],
65143
);
66144
}
67145

68146
export function foldFileToolDefinitions(
69147
defs: readonly ToolDefinition[],
70148
profile: ToolProfile,
71149
): ToolDefinition[] {
72-
if (profile !== "gpt" || !defs.some((d) => d.name === "apply_patch")) {
150+
const fold = PROFILE_FOLDS[profile];
151+
if (fold === undefined || !defs.some((d) => d.name === fold.wire)) {
73152
return [...defs];
74153
}
75-
return defs.filter((d) => !PATCH_FOLDED_ENGINES.has(engineToolName(d.name)));
154+
const folded = new Set(fold.engines);
155+
return defs.filter((d) => !folded.has(engineToolName(d.name)));
76156
}
77157

78158
export function toolProfileForFamily(family: ModelFamily): ToolProfile {
@@ -128,16 +208,23 @@ export function advertisedToolName(
128208
}
129209

130210
/** True when `name` (wire, engine, or hidden alias) is covered by an
131-
* advertised or activated listing (which may store wire or engine ids). */
211+
* advertised or activated listing (which may store wire or engine ids).
212+
* A call is advertised when this profile's wire is on the list, and also
213+
* when the default wire is — primary prefixes still store `bash` while a
214+
* gpt worker prefix stores `shell`. */
132215
export function nameMatchesAdvertisedListing(
133216
name: string,
134217
isListed: (candidate: string) => boolean,
218+
profile: ToolProfile = "default",
135219
): boolean {
136220
if (isListed(name)) return true;
137221
const engine = engineToolName(name);
138222
if (engine !== name && isListed(engine)) return true;
139-
const wire = advertisedToolName(engine);
140-
return wire !== name && isListed(wire);
223+
const wires =
224+
profile === "default"
225+
? [profileWireName(engine, profile)]
226+
: [profileWireName(engine, profile), profileWireName(engine, "default")];
227+
return wires.some((wire) => wire !== name && isListed(wire));
141228
}
142229

143230
// update_plan dispatches through translateUpdatePlanArgs, so advertising

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

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
ORCHESTRATOR_TOOLS,
3030
READ_TOOLS,
3131
} from "./directors/tool-sets.js";
32+
import { advertisedWireEngines, toolProfiles } from "./tool-aliases.js";
3233

3334
const FULL_AVAILABILITY: ToolAvailability = {
3435
languageServerAvailable: true,
@@ -1010,6 +1011,121 @@ describe("advertisedToolNamesForWorker", () => {
10101011
expect(docs).toContain("tool_search");
10111012
});
10121013

1014+
test("gpt docs surface folds file tools into apply_patch", () => {
1015+
const docs = advertisedToolNamesForWorker({
1016+
allow: DOCS_TOOLS,
1017+
profile: "gpt",
1018+
});
1019+
expect(docs).toContain("apply_patch");
1020+
expect(docs).not.toContain("write_file");
1021+
expect(docs).not.toContain("edit_file");
1022+
expect(docs).not.toContain("delete_file");
1023+
expect(docs).not.toContain("write");
1024+
expect(docs).not.toContain("edit");
1025+
expect(docs).not.toContain("delete");
1026+
expect(docs).not.toContain("shell");
1027+
expect(docs).not.toContain("run_shell");
1028+
});
1029+
1030+
test("default docs surface does not advertise apply_patch", () => {
1031+
const docs = advertisedToolNamesForWorker({
1032+
allow: DOCS_TOOLS,
1033+
profile: "default",
1034+
});
1035+
expect(docs).not.toContain("apply_patch");
1036+
expect(docs).toContain("write_file");
1037+
expect(docs).toContain("edit_file");
1038+
expect(docs).toContain("delete_file");
1039+
});
1040+
1041+
test("gpt build surface renames shell and folds file tools", () => {
1042+
const build = advertisedToolNamesForWorker({
1043+
allow: BUILD_TOOLS,
1044+
profile: "gpt",
1045+
});
1046+
expect(build).toContain("apply_patch");
1047+
expect(build).toContain("shell");
1048+
expect(build).not.toContain("bash");
1049+
expect(build).not.toContain("run_shell");
1050+
expect(build).not.toContain("write");
1051+
expect(build).not.toContain("write_file");
1052+
});
1053+
1054+
test("gpt shell is advertised, so a shell call does not promote run_shell", async () => {
1055+
const prefix = advertisedToolNamesForWorker({
1056+
allow: BUILD_TOOLS,
1057+
profile: "gpt",
1058+
});
1059+
const runner = createDynamicToolRunner([
1060+
{
1061+
kind: "string",
1062+
definition: {
1063+
name: "run_shell",
1064+
description: "run a shell command",
1065+
inputSchema: { type: "object", properties: {}, required: [] },
1066+
},
1067+
handler: async () => "ran:shell",
1068+
},
1069+
{
1070+
kind: "string",
1071+
definition: {
1072+
name: "apply_patch",
1073+
description: "apply a patch",
1074+
inputSchema: { type: "object", properties: {}, required: [] },
1075+
},
1076+
handler: async () => "ran:patch",
1077+
},
1078+
]);
1079+
const advertised = createAdvertisedToolset({
1080+
sessionMode: "orchestrator",
1081+
toolAvailability: { languageServerAvailable: false },
1082+
getProvider: () => ({ providerName: "openai", model: "gpt-5-codex" }),
1083+
builtInPrefix: prefix,
1084+
});
1085+
expect(advertised.isAdvertised("shell")).toBe(true);
1086+
expect(advertised.isAdvertised("run_shell")).toBe(true);
1087+
expect(advertised.isAdvertised("apply_patch")).toBe(true);
1088+
const promoted: string[] = [];
1089+
runner.setCallGate((name) => advertised.isAdvertised(name));
1090+
runner.setOnUndeclaredCall((name) => {
1091+
promoted.push(name);
1092+
advertised.activated.activate([name]);
1093+
});
1094+
const signal = new AbortController().signal;
1095+
const shell = await runner.run(
1096+
{ id: "shell", name: "shell", arguments: { command: "true" } },
1097+
signal,
1098+
);
1099+
const patch = await runner.run(
1100+
{ id: "patch", name: "apply_patch", arguments: {} },
1101+
signal,
1102+
);
1103+
expect(shell.content).toBe("ran:shell");
1104+
expect(shell.isError).toBeUndefined();
1105+
expect(patch.content).toBe("ran:patch");
1106+
expect(patch.isError).toBeUndefined();
1107+
expect(promoted).toEqual([]);
1108+
});
1109+
1110+
test("every non-default profile projects allowlisted engines through its wire table", () => {
1111+
const allow = BUILD_TOOLS as readonly string[];
1112+
for (const profile of toolProfiles()) {
1113+
const names = advertisedToolNamesForWorker({
1114+
allow: BUILD_TOOLS,
1115+
profile,
1116+
});
1117+
if (profile === "default") {
1118+
expect(names).toContain("write_file");
1119+
expect(names).not.toContain("apply_patch");
1120+
continue;
1121+
}
1122+
for (const [wire, engines] of advertisedWireEngines(profile)) {
1123+
if (!engines.some((engine) => allow.includes(engine))) continue;
1124+
expect(names, `${profile} ${wire}`).toContain(wire);
1125+
}
1126+
}
1127+
});
1128+
10131129
test("nested orchestrator allowlist adds fleet verbs but not search_agents", () => {
10141130
const orch = advertisedToolNamesForWorker({ allow: ORCHESTRATOR_TOOLS });
10151131
expect(orch).toContain("spawn_agent");

0 commit comments

Comments
 (0)