diff --git a/scripts/dead-export-guard.json b/scripts/dead-export-guard.json index cf4803e00..5e8d9b75b 100644 --- a/scripts/dead-export-guard.json +++ b/scripts/dead-export-guard.json @@ -1,4 +1,4 @@ { "tsconfig": "tsconfig.json", - "minScannedFiles": 1240 + "minScannedFiles": 1366 } diff --git a/src/agent/prompt-sizes.test.ts b/src/agent/prompt-sizes.test.ts index 29e677bf4..75c82386f 100644 --- a/src/agent/prompt-sizes.test.ts +++ b/src/agent/prompt-sizes.test.ts @@ -204,15 +204,33 @@ describe("director prompt size budget", () => { expect(new Set(names).size, `${directorId} [${family}]`).toBe( names.length, ); - // Hidden Codex aliases (apply_patch/shell/update_plan) are not advertised, - // as must list_dir, which no subagent mount installs. - for (const phantom of [ - "list_dir", - "apply_patch", - "shell", - "update_plan", - ]) { - expect(names, `${directorId} [${family}]`).not.toContain(phantom); + // list_dir is not installed on a subagent mount. apply_patch, shell, + // and update_plan are the gpt wire names — other families keep the + // default wires. + expect(names, `${directorId} [${family}]`).not.toContain("list_dir"); + if (family === "gpt") { + const defaults = canonicalToolNamesForDirector( + DIRECTOR_REGISTRY[directorId], + "default", + ); + expect(names.includes("apply_patch"), directorId).toBe( + defaults.includes("write"), + ); + expect(names.includes("shell"), directorId).toBe( + defaults.includes("bash"), + ); + expect(names.includes("update_plan"), directorId).toBe( + defaults.includes("todowrite"), + ); + expect(names, directorId).not.toContain("write"); + expect(names, directorId).not.toContain("edit"); + expect(names, directorId).not.toContain("delete"); + expect(names, directorId).not.toContain("bash"); + expect(names, directorId).not.toContain("todowrite"); + } else { + for (const phantom of ["apply_patch", "shell", "update_plan"]) { + expect(names, `${directorId} [${family}]`).not.toContain(phantom); + } } if (DIRECTOR_REGISTRY[directorId].spawn.maySpawn) { for (const verb of ["spawn_agent", "send_input"]) { @@ -225,6 +243,25 @@ describe("director prompt size budget", () => { } } }); + + test("gpt shakespeare lists apply_patch, not write/edit/delete", () => { + const names = canonicalToolNamesForDirector( + DIRECTOR_REGISTRY.shakespeare, + "gpt", + ); + expect(names).toContain("apply_patch"); + expect(names).not.toContain("write"); + expect(names).not.toContain("edit"); + expect(names).not.toContain("delete"); + expect(names).not.toContain("write_file"); + expect(names).not.toContain("shell"); + const defaults = canonicalToolNamesForDirector( + DIRECTOR_REGISTRY.shakespeare, + "default", + ); + expect(defaults).not.toContain("apply_patch"); + expect(defaults).toContain("write"); + }); }); describe("dispatch grok prefix (infer envelope vs trimmed director)", () => { diff --git a/src/agent/prompt-sizes.ts b/src/agent/prompt-sizes.ts index 0e5730ec9..fe505b83a 100644 --- a/src/agent/prompt-sizes.ts +++ b/src/agent/prompt-sizes.ts @@ -14,7 +14,11 @@ import { import { buildSubAgentSystemPrompt } from "./prompts.js"; import { resolveModelFamilyPolicy } from "./model-family-policy.js"; import { shouldApplyGrokAntiThrash } from "../subagent/provider-family.js"; -import { advertisedToolName } from "./tool-aliases.js"; +import { + advertisedToolName, + foldFileToolNames, + toolProfileForFamily, +} from "./tool-aliases.js"; import { canonicalToolName } from "./canonical-tool-name.js"; import { manageTasksDefinition } from "./tasks.js"; import { DELETE_FILE_DEFINITION } from "../plugins/delete-file-plugin.js"; @@ -86,7 +90,7 @@ function preFilterMountNames(): readonly string[] { */ export function canonicalToolNamesForDirector( pkg: DirectorPackage, - _family: PromptSizeFamily, + family: PromptSizeFamily, ): readonly string[] { const filtered = [...preFilterMountNames()]; const capabilities = packageToCapabilities(pkg); @@ -130,7 +134,11 @@ export function canonicalToolNamesForDirector( `canonicalToolNamesForDirector(${pkg.id}): "${dupe}" mounted twice — the assembly drifted from src/subagent/run.ts`, ); } - return names.map((name) => advertisedToolName(name)); + const profile = toolProfileForFamily(family); + return foldFileToolNames( + names.map((name) => advertisedToolName(name, profile)), + profile, + ); } /** Assemble one director prompt exactly as run.ts does. */ diff --git a/src/agent/tool-aliases.ts b/src/agent/tool-aliases.ts index 342134f35..444b9cbae 100644 --- a/src/agent/tool-aliases.ts +++ b/src/agent/tool-aliases.ts @@ -42,26 +42,104 @@ const ENGINE_TO_WIRE_BY_PROFILE: Record< gpt: { ...DEFAULT_ENGINE_TO_WIRE, ...GPT_OVERRIDES }, }; -/** Tools the gpt profile folds into the single apply_patch envelope tool. */ -const PATCH_FOLDED_ENGINES: ReadonlySet = new Set([ - "write_file", - "edit_file", - "delete_file", -]); +/** + * Engines one advertised wire replaces. A profile without an entry keeps + * the 1:1 wire table. Adding a family is a new ToolProfile plus a row here + * and in ENGINE_TO_WIRE_BY_PROFILE — not a new director package. + */ +const PROFILE_FOLDS: Record< + ToolProfile, + { wire: string; engines: readonly string[] } | undefined +> = { + default: undefined, + gpt: { + wire: "apply_patch", + engines: ["write_file", "edit_file", "delete_file"], + }, +}; + +function buildAdvertisedWireEngines( + profile: ToolProfile, +): ReadonlyMap { + const fold = PROFILE_FOLDS[profile]; + const folded = new Set(fold?.engines ?? []); + const map = new Map(); + for (const [engine, wire] of Object.entries( + ENGINE_TO_WIRE_BY_PROFILE[profile], + )) { + if (folded.has(engine)) continue; + const existing = map.get(wire); + if (existing === undefined) map.set(wire, [engine]); + else existing.push(engine); + } + if (fold !== undefined) map.set(fold.wire, [...fold.engines]); + return map; +} + +const ADVERTISED_WIRE_ENGINES = Object.fromEntries( + (Object.keys(ENGINE_TO_WIRE_BY_PROFILE) as ToolProfile[]).map((profile) => [ + profile, + buildAdvertisedWireEngines(profile), + ]), +) as Record>; + +export function toolProfiles(): readonly ToolProfile[] { + return Object.keys(ENGINE_TO_WIRE_BY_PROFILE) as ToolProfile[]; +} + +/** Wire name → engine ids this profile advertises. Folded wires map to every engine they replace. */ +export function advertisedWireEngines( + profile: ToolProfile, +): ReadonlyMap { + return ADVERTISED_WIRE_ENGINES[profile]; +} + +/** + * Engines `wire` stands for on `profile`. Undefined when this profile does + * not advertise that name (a later family follows the same table). + */ +export function enginesForAdvertisedWire( + wire: string, + profile: ToolProfile, +): readonly string[] | undefined { + const table = ADVERTISED_WIRE_ENGINES[profile]; + return table.get(wire) ?? table.get(wire.toLowerCase()); +} + +/** Wire this profile advertises for `engine`, including a fold that replaces it. */ +function profileWireName(engine: string, profile: ToolProfile): string { + const fold = PROFILE_FOLDS[profile]; + if (fold !== undefined && fold.engines.includes(engine)) return fold.wire; + return advertisedToolName(engine, profile); +} + +/** Every wire any profile advertises for `engine`, including fold wires. */ +export function advertisedNamesForEngine(engine: string): readonly string[] { + const names: string[] = []; + const seen = new Set(); + for (const profile of toolProfiles()) { + const wire = profileWireName(engine, profile); + if (seen.has(wire)) continue; + seen.add(wire); + names.push(wire); + } + return names; +} /** gpt models are trained on apply_patch, not write/edit/delete. Other - * profiles and lists without a file-mutation tool pass through unchanged. */ + * profiles and lists without a folded tool pass through unchanged. */ export function foldFileToolNames( names: readonly string[], profile: ToolProfile, ): readonly string[] { - if (profile !== "gpt") return names; + const fold = PROFILE_FOLDS[profile]; + if (fold === undefined) return names; const folded = (name: string): boolean => - PATCH_FOLDED_ENGINES.has(engineToolName(name)); + fold.engines.includes(engineToolName(name)); if (!names.some(folded)) return names; const first = names.findIndex(folded); return names.flatMap((name, i) => - i === first ? ["apply_patch"] : folded(name) ? [] : [name], + i === first ? [fold.wire] : folded(name) ? [] : [name], ); } @@ -69,10 +147,12 @@ export function foldFileToolDefinitions( defs: readonly ToolDefinition[], profile: ToolProfile, ): ToolDefinition[] { - if (profile !== "gpt" || !defs.some((d) => d.name === "apply_patch")) { + const fold = PROFILE_FOLDS[profile]; + if (fold === undefined || !defs.some((d) => d.name === fold.wire)) { return [...defs]; } - return defs.filter((d) => !PATCH_FOLDED_ENGINES.has(engineToolName(d.name))); + const folded = new Set(fold.engines); + return defs.filter((d) => !folded.has(engineToolName(d.name))); } export function toolProfileForFamily(family: ModelFamily): ToolProfile { @@ -128,16 +208,23 @@ export function advertisedToolName( } /** True when `name` (wire, engine, or hidden alias) is covered by an - * advertised or activated listing (which may store wire or engine ids). */ + * advertised or activated listing (which may store wire or engine ids). + * A call is advertised when this profile's wire is on the list, and also + * when the default wire is — primary prefixes still store `bash` while a + * gpt worker prefix stores `shell`. */ export function nameMatchesAdvertisedListing( name: string, isListed: (candidate: string) => boolean, + profile: ToolProfile = "default", ): boolean { if (isListed(name)) return true; const engine = engineToolName(name); if (engine !== name && isListed(engine)) return true; - const wire = advertisedToolName(engine); - return wire !== name && isListed(wire); + const wires = + profile === "default" + ? [profileWireName(engine, profile)] + : [profileWireName(engine, profile), profileWireName(engine, "default")]; + return wires.some((wire) => wire !== name && isListed(wire)); } // update_plan dispatches through translateUpdatePlanArgs, so advertising diff --git a/src/agent/tool-search.test.ts b/src/agent/tool-search.test.ts index c15447df7..496563d0f 100644 --- a/src/agent/tool-search.test.ts +++ b/src/agent/tool-search.test.ts @@ -29,6 +29,7 @@ import { ORCHESTRATOR_TOOLS, READ_TOOLS, } from "./directors/tool-sets.js"; +import { advertisedWireEngines, toolProfiles } from "./tool-aliases.js"; const FULL_AVAILABILITY: ToolAvailability = { languageServerAvailable: true, @@ -1010,6 +1011,121 @@ describe("advertisedToolNamesForWorker", () => { expect(docs).toContain("tool_search"); }); + test("gpt docs surface folds file tools into apply_patch", () => { + const docs = advertisedToolNamesForWorker({ + allow: DOCS_TOOLS, + profile: "gpt", + }); + expect(docs).toContain("apply_patch"); + expect(docs).not.toContain("write_file"); + expect(docs).not.toContain("edit_file"); + expect(docs).not.toContain("delete_file"); + expect(docs).not.toContain("write"); + expect(docs).not.toContain("edit"); + expect(docs).not.toContain("delete"); + expect(docs).not.toContain("shell"); + expect(docs).not.toContain("run_shell"); + }); + + test("default docs surface does not advertise apply_patch", () => { + const docs = advertisedToolNamesForWorker({ + allow: DOCS_TOOLS, + profile: "default", + }); + expect(docs).not.toContain("apply_patch"); + expect(docs).toContain("write_file"); + expect(docs).toContain("edit_file"); + expect(docs).toContain("delete_file"); + }); + + test("gpt build surface renames shell and folds file tools", () => { + const build = advertisedToolNamesForWorker({ + allow: BUILD_TOOLS, + profile: "gpt", + }); + expect(build).toContain("apply_patch"); + expect(build).toContain("shell"); + expect(build).not.toContain("bash"); + expect(build).not.toContain("run_shell"); + expect(build).not.toContain("write"); + expect(build).not.toContain("write_file"); + }); + + test("gpt shell is advertised, so a shell call does not promote run_shell", async () => { + const prefix = advertisedToolNamesForWorker({ + allow: BUILD_TOOLS, + profile: "gpt", + }); + const runner = createDynamicToolRunner([ + { + kind: "string", + definition: { + name: "run_shell", + description: "run a shell command", + inputSchema: { type: "object", properties: {}, required: [] }, + }, + handler: async () => "ran:shell", + }, + { + kind: "string", + definition: { + name: "apply_patch", + description: "apply a patch", + inputSchema: { type: "object", properties: {}, required: [] }, + }, + handler: async () => "ran:patch", + }, + ]); + const advertised = createAdvertisedToolset({ + sessionMode: "orchestrator", + toolAvailability: { languageServerAvailable: false }, + getProvider: () => ({ providerName: "openai", model: "gpt-5-codex" }), + builtInPrefix: prefix, + }); + expect(advertised.isAdvertised("shell")).toBe(true); + expect(advertised.isAdvertised("run_shell")).toBe(true); + expect(advertised.isAdvertised("apply_patch")).toBe(true); + const promoted: string[] = []; + runner.setCallGate((name) => advertised.isAdvertised(name)); + runner.setOnUndeclaredCall((name) => { + promoted.push(name); + advertised.activated.activate([name]); + }); + const signal = new AbortController().signal; + const shell = await runner.run( + { id: "shell", name: "shell", arguments: { command: "true" } }, + signal, + ); + const patch = await runner.run( + { id: "patch", name: "apply_patch", arguments: {} }, + signal, + ); + expect(shell.content).toBe("ran:shell"); + expect(shell.isError).toBeUndefined(); + expect(patch.content).toBe("ran:patch"); + expect(patch.isError).toBeUndefined(); + expect(promoted).toEqual([]); + }); + + test("every non-default profile projects allowlisted engines through its wire table", () => { + const allow = BUILD_TOOLS as readonly string[]; + for (const profile of toolProfiles()) { + const names = advertisedToolNamesForWorker({ + allow: BUILD_TOOLS, + profile, + }); + if (profile === "default") { + expect(names).toContain("write_file"); + expect(names).not.toContain("apply_patch"); + continue; + } + for (const [wire, engines] of advertisedWireEngines(profile)) { + if (!engines.some((engine) => allow.includes(engine))) continue; + expect(names, `${profile} ${wire}`).toContain(wire); + } + } + }); + test("nested orchestrator allowlist adds fleet verbs but not search_agents", () => { const orch = advertisedToolNamesForWorker({ allow: ORCHESTRATOR_TOOLS }); expect(orch).toContain("spawn_agent"); diff --git a/src/agent/tool-search.ts b/src/agent/tool-search.ts index bb430d0f2..c9486db94 100644 --- a/src/agent/tool-search.ts +++ b/src/agent/tool-search.ts @@ -12,7 +12,9 @@ import { import type { SessionMode } from "../config/session-mode.js"; import { sessionModeEnablesSubAgents } from "../config/session-mode.js"; import { + advertisedNamesForEngine, advertisedToolName, + foldFileToolNames, projectToolDefinition, type ToolProfile, } from "./tool-aliases.js"; @@ -117,6 +119,8 @@ export function advertisedToolNamesForWorker(opts: { allow?: readonly string[]; orchestrator?: boolean; languageServerAvailable?: boolean; + /** Family projection. Default keeps registry engine ids. */ + profile?: ToolProfile; }): readonly string[] { const base = opts.allow !== undefined && opts.allow.length > 0 @@ -135,7 +139,11 @@ export function advertisedToolNamesForWorker(opts: { seen.add(name); out.push(name); } - return out; + if (opts.profile === undefined || opts.profile === "default") return out; + return foldFileToolNames( + out.map((name) => advertisedToolName(name, opts.profile)), + opts.profile, + ); } // Built-in file/search/web tools advertised with full schema alongside the @@ -169,11 +177,11 @@ function isAlreadyAdvertised( advertisedNames: readonly string[], ): boolean { const engine = canonicalToolName(defName); - const wire = advertisedToolName(engine); - return ( - advertisedNames.includes(defName) || - advertisedNames.includes(engine) || - advertisedNames.includes(wire) + if (advertisedNames.includes(defName) || advertisedNames.includes(engine)) { + return true; + } + return advertisedNamesForEngine(engine).some((name) => + advertisedNames.includes(name), ); } diff --git a/src/session/assemble-runtime.ts b/src/session/assemble-runtime.ts index 9278259d7..d4e0065ba 100644 --- a/src/session/assemble-runtime.ts +++ b/src/session/assemble-runtime.ts @@ -452,17 +452,17 @@ export function createAdvertisedToolset(args: { ); }; const isAdvertised = (name: string): boolean => { - const denied = deniedFor(args.getProvider()); + const provider = args.getProvider(); + const profile = toolProfileForModel(provider); + const denied = deniedFor(provider); if (denied.includes(name) || denied.includes(canonicalToolName(name))) { return false; } - const effective = foldFileToolNames( - prefix, - toolProfileForModel(args.getProvider()), - ); + const effective = foldFileToolNames(prefix, profile); return nameMatchesAdvertisedListing( name, (n) => effective.includes(n) || activated.has(n), + profile, ); }; const flushPromotions = (): boolean => { diff --git a/src/subagent/agent-fleet-requires-tools.test.ts b/src/subagent/agent-fleet-requires-tools.test.ts index c56f461a0..94363d8d4 100644 --- a/src/subagent/agent-fleet-requires-tools.test.ts +++ b/src/subagent/agent-fleet-requires-tools.test.ts @@ -49,13 +49,14 @@ const BUILD_LIKE_PROFILE: AgentProfile = { function makeDeps( run: (params: RunSubAgentParams) => Promise, capturedTelemetry: TelemetryEvent[], + providerOverride: AgentFleetDeps["provider"] = provider, ): AgentFleetDeps { const sessions = createSubAgentSessionStore(); return { permissionGate: testPermissionGate, cwd: "/tmp", getWorkdirBase: () => "/tmp/workdir", - provider, + provider: providerOverride, run, sessions, fleetRecords: createFleetMailbox(sessions), @@ -232,9 +233,36 @@ describe("spawn_agent requires_tools preflight", () => { expect(seenRequires).toEqual(["manage_tasks"]); }); - test("update_plan alias survives dispatch and stamps manage_tasks", async () => { + test("default family rejects update_plan, a gpt-only wire", async () => { const telemetry: TelemetryEvent[] = []; - const deps = makeDeps(async () => ({ report: "done" }), telemetry); + let runCalled = false; + const deps = makeDeps(async () => { + runCalled = true; + return { report: "done" }; + }, telemetry); + const spawn = createSpawnAgentTool(deps); + + const result = await callSpawn(spawn, { + description: "alias plan job", + prompt: "track the work", + agent: "read-only-worker", + requires_tools: ["update_plan"], + }); + + expect(result.isError).toBe(true); + expect(result.content).toContain('unknown tool "update_plan"'); + expect(runCalled).toBe(false); + expect(deps.sessions.list()).toEqual([]); + expect(telemetry).toEqual([]); + }); + + test("gpt family accepts update_plan and stamps manage_tasks", async () => { + const telemetry: TelemetryEvent[] = []; + const deps = makeDeps(async () => ({ report: "done" }), telemetry, { + ...provider, + providerName: "codex", + model: "gpt-5", + }); const spawn = createSpawnAgentTool(deps); const result = await callSpawn(spawn, { diff --git a/src/subagent/agent-fleet.ts b/src/subagent/agent-fleet.ts index 30e4f59cc..a2f2c1ec0 100644 --- a/src/subagent/agent-fleet.ts +++ b/src/subagent/agent-fleet.ts @@ -44,6 +44,7 @@ import { formatDirectorSystemPrompt, packageAllowedSkillNames, } from "../agent/directors/identity.js"; +import { toolProfileForModel } from "../agent/tool-aliases.js"; import type { Settings } from "../config/settings.js"; import { resolveInferenceWithPolicy } from "../config/settings.js"; import { @@ -1250,6 +1251,10 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { knownEngines: DEFAULT_KNOWN_ENGINES, availableMcpTools, agentLabel: resolved.agentLabel, + toolProfile: toolProfileForModel({ + providerName: provider.providerName, + model: provider.model, + }), }); if (!preflight.ok) { return fleetResult( diff --git a/src/subagent/capability-preflight.test.ts b/src/subagent/capability-preflight.test.ts index d586895b2..1ece3cc3a 100644 --- a/src/subagent/capability-preflight.test.ts +++ b/src/subagent/capability-preflight.test.ts @@ -10,6 +10,12 @@ import { type CapabilityUnavailable, } from "./capability-preflight.js"; import type { CapabilityFilter } from "../agent/profile-types.js"; +import { + BUILD_TOOLS, + DOCS_TOOLS, + READ_TOOLS, +} from "../agent/directors/tool-sets.js"; +import { advertisedWireEngines, toolProfiles } from "../agent/tool-aliases.js"; const ALLOW_SHELL: CapabilityFilter = { mode: "allow", @@ -61,7 +67,7 @@ describe("preflightCapabilities", () => { }); }); - test("aliases collapse on both sides (shell requirement vs run_shell filter)", () => { + test("unresolved profile still collapses shell onto run_shell", () => { const result = preflight(["shell"], ALLOW_SHELL); expect(result).toEqual({ ok: true, canonical: ["run_shell"] }); }); @@ -198,6 +204,200 @@ describe("preflightCapabilities", () => { if (result.ok) throw new Error("expected rejection"); expect(result.unavailable.code).toBe("permission_static"); }); + + test("shakespeare on gpt accepts apply_patch when a file tool is allowed", () => { + const result = preflightCapabilities({ + required: ["apply_patch"], + resolvedFilter: { mode: "allow", tools: [...DOCS_TOOLS] }, + knownEngines: DEFAULT_KNOWN_ENGINES, + agentLabel: "shakespeare", + toolProfile: "gpt", + }); + expect(result).toEqual({ + ok: true, + canonical: ["write_file", "edit_file", "delete_file"], + }); + }); + + test("shakespeare on the default family rejects apply_patch as unknown", () => { + const result = preflightCapabilities({ + required: ["apply_patch"], + resolvedFilter: { mode: "allow", tools: [...DOCS_TOOLS] }, + knownEngines: DEFAULT_KNOWN_ENGINES, + agentLabel: "shakespeare", + toolProfile: "default", + }); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected rejection"); + expect(result.unavailable.code).toBe("unknown_tool"); + expect(result.unavailable.tool).toBe("apply_patch"); + expect( + formatCapabilityUnavailable(result.unavailable, "shakespeare"), + ).toContain('worker "shakespeare" requires unknown tool "apply_patch"'); + }); + + test("unresolved family does not treat apply_patch as known", () => { + const result = preflight(["apply_patch"], { + mode: "allow", + tools: [...DOCS_TOOLS], + }); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected rejection"); + expect(result.unavailable.code).toBe("unknown_tool"); + }); + + test("gpt worker with no file-mutation tool rejects apply_patch", () => { + const result = preflightCapabilities({ + required: ["apply_patch"], + resolvedFilter: { mode: "allow", tools: [...READ_TOOLS] }, + knownEngines: DEFAULT_KNOWN_ENGINES, + agentLabel: "explorer", + toolProfile: "gpt", + }); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected rejection"); + expect(result.unavailable.code).toBe("missing_tool"); + expect(result.unavailable.tool).toBe("apply_patch"); + }); + + test("gpt wire names pass when the worker has the underlying engines", () => { + const resolvedFilter: CapabilityFilter = { + mode: "allow", + tools: [...BUILD_TOOLS], + }; + const cases = [ + { + wire: "apply_patch", + canonical: ["write_file", "edit_file", "delete_file"], + }, + { wire: "shell", canonical: ["run_shell"] }, + { wire: "update_plan", canonical: ["manage_tasks"] }, + ]; + for (const entry of cases) { + expect( + preflightCapabilities({ + required: [entry.wire], + resolvedFilter, + knownEngines: DEFAULT_KNOWN_ENGINES, + agentLabel: "coder", + toolProfile: "gpt", + }), + entry.wire, + ).toEqual({ ok: true, canonical: entry.canonical }); + } + }); + + test("every profile accepts its advertised wires when those engines are allowed", () => { + const known = new Set(DEFAULT_KNOWN_ENGINES); + const tools = [...BUILD_TOOLS, "manage_tasks", "wait_agents"]; + for (const profile of toolProfiles()) { + for (const [wire, mapped] of advertisedWireEngines(profile)) { + if (!mapped.every((engine) => known.has(engine))) continue; + const result = preflightCapabilities({ + required: [wire], + resolvedFilter: { mode: "allow", tools }, + knownEngines: DEFAULT_KNOWN_ENGINES, + agentLabel: "coder", + toolProfile: profile, + }); + expect(result.ok, `${profile} ${wire}`).toBe(true); + if (!result.ok) continue; + for (const engine of result.canonical) { + expect(mapped, `${profile} ${wire}`).toContain(engine); + } + } + } + }); + + test("gpt apply_patch maps onto only the file engines the allowlist has", () => { + const result = preflightCapabilities({ + required: ["apply_patch"], + resolvedFilter: { mode: "allow", tools: ["read_file", "write_file"] }, + knownEngines: DEFAULT_KNOWN_ENGINES, + agentLabel: "shakespeare", + toolProfile: "gpt", + }); + expect(result).toEqual({ ok: true, canonical: ["write_file"] }); + }); + + test("default profile rejects gpt-only wires even when the engine is allowed", () => { + const resolvedFilter: CapabilityFilter = { + mode: "allow", + tools: [...BUILD_TOOLS, "manage_tasks"], + }; + for (const wire of ["apply_patch", "shell", "update_plan"] as const) { + const result = preflightCapabilities({ + required: [wire], + resolvedFilter, + knownEngines: DEFAULT_KNOWN_ENGINES, + agentLabel: "coder", + toolProfile: "default", + }); + expect(result.ok, wire).toBe(false); + if (result.ok) continue; + expect(result.unavailable.code, wire).toBe("unknown_tool"); + expect(result.unavailable.tool, wire).toBe(wire); + } + }); + + test("default accepts bash, the wire it advertises for run_shell", () => { + const result = preflightCapabilities({ + required: ["bash"], + resolvedFilter: ALLOW_SHELL, + knownEngines: DEFAULT_KNOWN_ENGINES, + agentLabel: "coder", + toolProfile: "default", + }); + expect(result).toEqual({ ok: true, canonical: ["run_shell"] }); + }); + + test("gpt rejects bash because that profile advertises shell", () => { + const result = preflightCapabilities({ + required: ["bash"], + resolvedFilter: ALLOW_SHELL, + knownEngines: DEFAULT_KNOWN_ENGINES, + agentLabel: "coder", + toolProfile: "gpt", + }); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected rejection"); + expect(result.unavailable.code).toBe("unknown_tool"); + expect(result.unavailable.tool).toBe("bash"); + }); + + test("gpt exclude denies the wire and the engine together", () => { + const cases = [ + { exclude: "manage_tasks", required: ["update_plan", "manage_tasks"] }, + { exclude: "update_plan", required: ["update_plan", "manage_tasks"] }, + { exclude: "run_shell", required: ["shell", "run_shell"] }, + { exclude: "shell", required: ["shell", "run_shell"] }, + ] as const; + for (const entry of cases) { + for (const required of entry.required) { + const result = preflightCapabilities({ + required: [required], + resolvedFilter: { mode: "exclude", tools: [entry.exclude] }, + knownEngines: DEFAULT_KNOWN_ENGINES, + agentLabel: "coder", + toolProfile: "gpt", + }); + const label = `${entry.exclude} blocks ${required}`; + expect(result.ok, label).toBe(false); + if (result.ok) continue; + expect(result.unavailable.code, label).toBe("permission_static"); + } + } + for (const required of ["manage_tasks", "update_plan"] as const) { + const result = preflightCapabilities({ + required: [required], + resolvedFilter: { mode: "exclude", tools: ["read_file"] }, + knownEngines: DEFAULT_KNOWN_ENGINES, + agentLabel: "coder", + toolProfile: "gpt", + }); + expect(result.ok, required).toBe(true); + } + }); }); describe("formatCapabilityUnavailable", () => { diff --git a/src/subagent/capability-preflight.ts b/src/subagent/capability-preflight.ts index 3686befc6..ea1b6a3cf 100644 --- a/src/subagent/capability-preflight.ts +++ b/src/subagent/capability-preflight.ts @@ -3,9 +3,13 @@ * * A `requires_tools` entry is a hard requirement: the tool must mount on * the worker or the dispatch rejects before any session, telemetry, or - * worktree exists. Names are canonical engine ids on both sides (aliases - * collapse via canonicalToolName), so `shell` and `run_shell` are the - * same requirement. Fail-closed: unknown names and allowlist/denylist + * worktree exists. Engine ids are canonical on both sides. A wire name + * passes only when the worker's resolved profile advertises it, and then + * only as the engine ids that profile projects it onto — a rename is one + * engine, a fold is every covered engine. A wire some other profile + * advertises is unknown here (`shell` is gpt; default advertises `bash`). + * With no resolved profile, hidden aliases still canonicalize and a fold + * target stays unknown. Fail-closed: unknown names and allowlist/denylist * misses reject. * * `stale_snapshot` is never emitted here — it is the mount-time echo in @@ -14,6 +18,11 @@ */ import { canonicalToolName } from "../agent/canonical-tool-name.js"; +import { + enginesForAdvertisedWire, + toolProfiles, + type ToolProfile, +} from "../agent/tool-aliases.js"; import { DIRECTOR_REGISTRY, packageToCapabilities, @@ -49,6 +58,12 @@ export interface PreflightCapabilitiesInput { availableMcpTools?: readonly string[] | undefined; /** Worker label for messages (director id or profile id). */ agentLabel: string; + /** + * Tool profile of the model this spawn will run. Wire names resolve through + * that profile's projection. Omit when no family is resolved — hidden + * aliases still canonicalize, and a fold target stays unknown. + */ + toolProfile?: ToolProfile | undefined; } export interface CapabilityUnavailable { @@ -218,6 +233,71 @@ export function leafTierAlternatives(): readonly string[] { .slice(0, 3); } +function enginesCoveredByDirector( + engines: readonly string[], + filter: CapabilityFilter | undefined, + postFilter: ReadonlySet, +): readonly string[] { + if (filter === undefined) return engines; + const named = new Set(filter.tools.map((name) => canonicalToolName(name))); + if (filter.mode === "allow") { + return engines.filter( + (engine) => named.has(engine) || postFilter.has(engine), + ); + } + // An explicit exclude names the engine. Post-filter mounts (manage_tasks) + // are not exempt here — update_plan and manage_tasks must agree, and the + // engine-id path applies the same denylist. + return engines.filter((engine) => !named.has(engine)); +} + +/** + * Profile wire names become the engines that profile advertises. Undefined + * means this name is not a wire for any profile — the caller falls through + * to catalog engines. A wire only another profile advertises is unknown. + * No resolved profile does not guess. + */ +function expandAdvertisedWire( + trimmed: string, + profile: ToolProfile | undefined, + filter: CapabilityFilter | undefined, + postFilter: ReadonlySet, +): string[] | { unavailable: CapabilityUnavailable } | undefined { + if (profile === undefined) return undefined; + const mapped = enginesForAdvertisedWire(trimmed, profile); + if (mapped === undefined) { + const foreign = toolProfiles().some( + (candidate) => + candidate !== profile && + enginesForAdvertisedWire(trimmed, candidate) !== undefined, + ); + if (!foreign) return undefined; + const suggestion = nearestToolName(trimmed); + const useful = + suggestion !== undefined && + suggestion.toLowerCase() !== trimmed.toLowerCase(); + return { + unavailable: { + code: "unknown_tool", + tool: trimmed, + ...(useful ? { suggestion } : {}), + }, + }; + } + const covered = enginesCoveredByDirector(mapped, filter, postFilter); + if (covered.length > 0) return [...covered]; + const hint = mapped[0]; + return { + unavailable: { + code: filter?.mode === "exclude" ? "permission_static" : "missing_tool", + tool: trimmed, + ...(hint !== undefined + ? { alternatives: rerouteAlternatives(hint) } + : {}), + }, + }; +} + export function preflightCapabilities( input: PreflightCapabilitiesInput, ): CapabilityPreflightResult { @@ -244,64 +324,69 @@ export function preflightCapabilities( return { ok: false, unavailable: { code: "unknown_tool", tool: raw } }; } const engine = canonicalToolName(trimmed); - // A live inherited-MCP tool passes the catalog check — presence proves - // the mount (see availableMcpTools); an `mcp__*` name outside the live - // set still rejects below. - const isLiveMcp = isMcpToolName(engine) && liveMcp.has(engine); - if (!catalog.has(engine) && !isLiveMcp) { - const suggestion = nearestToolName(trimmed); - return { - ok: false, - unavailable: { - code: "unknown_tool", - tool: trimmed, - ...(suggestion !== undefined ? { suggestion } : {}), - }, - }; - } - if (!isLiveMcp && !known.has(engine)) { - return { - ok: false, - unavailable: { code: "missing_binary", tool: engine }, - }; + const wireExpanded = expandAdvertisedWire( + trimmed, + input.toolProfile, + resolvedFilter, + postFilter, + ); + if (wireExpanded !== undefined && !Array.isArray(wireExpanded)) { + return { ok: false, unavailable: wireExpanded.unavailable }; } - if (!postFilter.has(engine) && !isLiveMcp) { - if (allow !== undefined && !allow.has(engine)) { + const expanded = wireExpanded ?? [engine]; + for (const resolved of expanded) { + // A live inherited-MCP tool passes the catalog check — presence proves + // the mount (see availableMcpTools); an `mcp__*` name outside the live + // set still rejects below. + const isLiveMcp = isMcpToolName(resolved) && liveMcp.has(resolved); + if (!catalog.has(resolved) && !isLiveMcp) { + const suggestion = nearestToolName(trimmed); return { ok: false, unavailable: { - code: "missing_tool", - tool: engine, - alternatives: rerouteAlternatives(engine), + code: "unknown_tool", + tool: trimmed, + ...(suggestion !== undefined ? { suggestion } : {}), }, }; } - if (deny !== undefined && deny.has(engine)) { + if (!isLiveMcp && !known.has(resolved)) { + return { + ok: false, + unavailable: { code: "missing_binary", tool: resolved }, + }; + } + // Explicit exclude wins over the post-filter mount exemption and over a + // live MCP mount. Otherwise gpt update_plan (wire path) is + // permission_static while manage_tasks (engine-id path) still passes. + // shell / run_shell share that rule. run.ts strips named tools in + // exclude mode, so an excluded MCP tool must reject here. + if (deny !== undefined && deny.has(resolved)) { return { ok: false, unavailable: { code: "permission_static", - tool: engine, - alternatives: rerouteAlternatives(engine), + tool: resolved, + alternatives: rerouteAlternatives(resolved), }, }; } - } else if (isLiveMcp && deny !== undefined && deny.has(engine)) { - // An explicit exclude naming a live MCP tool still withholds it — - // run.ts strips named tools in exclude mode, so the mount would drop - // it and the requirement must reject here, not as a stale snapshot. - return { - ok: false, - unavailable: { - code: "permission_static", - tool: engine, - alternatives: rerouteAlternatives(engine), - }, - }; - } - if (!seen.has(engine)) { - seen.add(engine); - canonical.push(engine); + if (!postFilter.has(resolved) && !isLiveMcp) { + if (allow !== undefined && !allow.has(resolved)) { + return { + ok: false, + unavailable: { + code: "missing_tool", + tool: resolved, + alternatives: rerouteAlternatives(resolved), + }, + }; + } + } + if (!seen.has(resolved)) { + seen.add(resolved); + canonical.push(resolved); + } } } return { ok: true, canonical }; diff --git a/src/subagent/run.ts b/src/subagent/run.ts index 55057aa4b..3a16cbcc9 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -1165,11 +1165,13 @@ async function runSubAgentInner( } } + const toolProfile = toolProfileForModel(params.provider); const workerPrefix = advertisedToolNamesForWorker({ orchestrator: params.orchestrator === true, ...(params.capabilities?.mode === "allow" ? { allow: params.capabilities.tools } : {}), + profile: toolProfile, }); const advertisedSet = createAdvertisedToolset({ sessionMode: "orchestrator", @@ -1236,7 +1238,6 @@ async function runSubAgentInner( ...(roleBody !== undefined ? [roleBody] : []), ...(attachedSection !== undefined ? [attachedSection] : []), ]; - const toolProfile = toolProfileForModel(params.provider); const advertisedDefs = advertisedSet.computeAdvertised( tools.map((t) => t.definition), );