From f1e838e64491602224189228639a70b3982ed4c5 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 15 Oct 2026 19:24:31 -0700 Subject: [PATCH 1/5] fix(agent): fold gpt tool names for workers Spawn preflight treated apply_patch as an unknown engine, so a gpt worker could not require the wire name its allowlist already covers with write, edit, and delete. Worker advertise lists now use the same family projection as the parent. --- src/agent/prompt-sizes.test.ts | 55 ++++++-- src/agent/prompt-sizes.ts | 14 +- src/agent/tool-aliases.ts | 9 +- src/agent/tool-search.test.ts | 40 ++++++ src/agent/tool-search.ts | 24 +++- src/subagent/agent-fleet.ts | 5 + src/subagent/capability-preflight.test.ts | 67 ++++++++++ src/subagent/capability-preflight.ts | 156 +++++++++++++++------- src/subagent/run.ts | 3 +- 9 files changed, 307 insertions(+), 66 deletions(-) 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..4033726e3 100644 --- a/src/agent/tool-aliases.ts +++ b/src/agent/tool-aliases.ts @@ -42,12 +42,15 @@ 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([ +/** Registry engines the gpt profile folds into apply_patch, in advertise order. */ +export const GPT_PATCH_ENGINES = [ "write_file", "edit_file", "delete_file", -]); +] as const; + +/** Tools the gpt profile folds into the single apply_patch envelope tool. */ +const PATCH_FOLDED_ENGINES: ReadonlySet = new Set(GPT_PATCH_ENGINES); /** gpt models are trained on apply_patch, not write/edit/delete. Other * profiles and lists without a file-mutation tool pass through unchanged. */ diff --git a/src/agent/tool-search.test.ts b/src/agent/tool-search.test.ts index c15447df7..d52b02b00 100644 --- a/src/agent/tool-search.test.ts +++ b/src/agent/tool-search.test.ts @@ -1010,6 +1010,46 @@ 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("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..50fdc695f 100644 --- a/src/agent/tool-search.ts +++ b/src/agent/tool-search.ts @@ -13,6 +13,8 @@ import type { SessionMode } from "../config/session-mode.js"; import { sessionModeEnablesSubAgents } from "../config/session-mode.js"; import { advertisedToolName, + foldFileToolNames, + GPT_PATCH_ENGINES, 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 !== "gpt") return out; + return foldFileToolNames( + out.map((name) => advertisedToolName(name, "gpt")), + "gpt", + ); } // Built-in file/search/web tools advertised with full schema alongside the @@ -169,11 +177,19 @@ function isAlreadyAdvertised( advertisedNames: readonly string[], ): boolean { const engine = canonicalToolName(defName); - const wire = advertisedToolName(engine); - return ( + if ( advertisedNames.includes(defName) || advertisedNames.includes(engine) || - advertisedNames.includes(wire) + advertisedNames.includes(advertisedToolName(engine)) || + advertisedNames.includes(advertisedToolName(engine, "gpt")) + ) { + return true; + } + // apply_patch stands in for the file engines; a gpt prefix lists the wire + // name only, so search must not offer write/edit/delete beside it. + return ( + advertisedNames.includes("apply_patch") && + (GPT_PATCH_ENGINES as readonly string[]).includes(engine) ); } 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..b25065f37 100644 --- a/src/subagent/capability-preflight.test.ts +++ b/src/subagent/capability-preflight.test.ts @@ -10,6 +10,7 @@ import { type CapabilityUnavailable, } from "./capability-preflight.js"; import type { CapabilityFilter } from "../agent/profile-types.js"; +import { DOCS_TOOLS, READ_TOOLS } from "../agent/directors/tool-sets.js"; const ALLOW_SHELL: CapabilityFilter = { mode: "allow", @@ -198,6 +199,72 @@ 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 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"] }); + }); }); describe("formatCapabilityUnavailable", () => { diff --git a/src/subagent/capability-preflight.ts b/src/subagent/capability-preflight.ts index 3686befc6..af42b126e 100644 --- a/src/subagent/capability-preflight.ts +++ b/src/subagent/capability-preflight.ts @@ -5,8 +5,10 @@ * 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 - * misses reject. + * same requirement. `apply_patch` is not an engine id: on the gpt profile + * it maps onto the file-mutation engines the director already allows, and + * with no resolved profile it stays unknown. Fail-closed: unknown names + * and allowlist/denylist misses reject. * * `stale_snapshot` is never emitted here — it is the mount-time echo in * run.ts (a stamped tool missing from the live mount). It lives in this @@ -14,6 +16,7 @@ */ import { canonicalToolName } from "../agent/canonical-tool-name.js"; +import { GPT_PATCH_ENGINES, type ToolProfile } from "../agent/tool-aliases.js"; import { DIRECTOR_REGISTRY, packageToCapabilities, @@ -49,6 +52,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. `apply_patch` is the gpt + * wire name for the file-mutation engines, not a catalog id. Omit when no + * family is resolved — then apply_patch stays unknown. + */ + toolProfile?: ToolProfile | undefined; } export interface CapabilityUnavailable { @@ -218,6 +227,50 @@ export function leafTierAlternatives(): readonly string[] { .slice(0, 3); } +function patchEnginesCovered( + filter: CapabilityFilter | undefined, +): readonly string[] { + if (filter === undefined) return GPT_PATCH_ENGINES; + const named = new Set(filter.tools.map((name) => canonicalToolName(name))); + if (filter.mode === "allow") { + return GPT_PATCH_ENGINES.filter((engine) => named.has(engine)); + } + return GPT_PATCH_ENGINES.filter((engine) => !named.has(engine)); +} + +/** + * `apply_patch` is the gpt wire name for the file engines already on the + * director. Other profiles, and a missing profile, leave it unknown. + */ +function expandApplyPatch( + trimmed: string, + engine: string, + profile: ToolProfile | undefined, + filter: CapabilityFilter | undefined, +): string[] | { unavailable: CapabilityUnavailable } { + if (engine !== "apply_patch") return [engine]; + if (profile !== "gpt") { + const suggestion = nearestToolName(trimmed); + return { + unavailable: { + code: "unknown_tool", + tool: trimmed, + ...(suggestion !== undefined ? { suggestion } : {}), + }, + }; + } + const covered = patchEnginesCovered(filter); + if (covered.length > 0) return [...covered]; + const denied = filter?.mode === "exclude"; + return { + unavailable: { + code: denied ? "permission_static" : "missing_tool", + tool: "apply_patch", + alternatives: rerouteAlternatives("write_file"), + }, + }; +} + export function preflightCapabilities( input: PreflightCapabilitiesInput, ): CapabilityPreflightResult { @@ -244,64 +297,75 @@ 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 expanded = expandApplyPatch( + trimmed, + engine, + input.toolProfile, + resolvedFilter, + ); + if (!Array.isArray(expanded)) { + return { ok: false, unavailable: expanded.unavailable }; } - if (!postFilter.has(engine) && !isLiveMcp) { - if (allow !== undefined && !allow.has(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 }, + }; + } + if (!postFilter.has(resolved) && !isLiveMcp) { + if (allow !== undefined && !allow.has(resolved)) { + return { + ok: false, + unavailable: { + code: "missing_tool", + tool: resolved, + alternatives: rerouteAlternatives(resolved), + }, + }; + } + if (deny !== undefined && deny.has(resolved)) { + return { + ok: false, + unavailable: { + code: "permission_static", + tool: resolved, + alternatives: rerouteAlternatives(resolved), + }, + }; + } + } else if (isLiveMcp && deny !== undefined && deny.has(resolved)) { + // 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), + 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 (!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), ); From e0abe4a0ce4fe9634ef641c144a2dacbec9aa9f7 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 15 Oct 2026 19:30:11 -0700 Subject: [PATCH 2/5] fix(agent): project worker wires from the profile table requires_tools and worker advertisement resolve wire names through the profile projection, so a later family uses the same path as gpt. apply_patch, shell, and update_plan are rows in that table. --- src/agent/tool-aliases.ts | 102 +++++++++++++++++++--- src/agent/tool-search.test.ts | 20 +++++ src/agent/tool-search.ts | 22 ++--- src/subagent/capability-preflight.test.ts | 56 +++++++++++- src/subagent/capability-preflight.ts | 72 +++++++-------- 5 files changed, 208 insertions(+), 64 deletions(-) diff --git a/src/agent/tool-aliases.ts b/src/agent/tool-aliases.ts index 4033726e3..98c259867 100644 --- a/src/agent/tool-aliases.ts +++ b/src/agent/tool-aliases.ts @@ -42,29 +42,101 @@ const ENGINE_TO_WIRE_BY_PROFILE: Record< gpt: { ...DEFAULT_ENGINE_TO_WIRE, ...GPT_OVERRIDES }, }; -/** Registry engines the gpt profile folds into apply_patch, in advertise order. */ -export const GPT_PATCH_ENGINES = [ - "write_file", - "edit_file", - "delete_file", -] as const; +/** + * 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()); +} -/** Tools the gpt profile folds into the single apply_patch envelope tool. */ -const PATCH_FOLDED_ENGINES: ReadonlySet = new Set(GPT_PATCH_ENGINES); +/** 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 fold = PROFILE_FOLDS[profile]; + const wire = + fold !== undefined && fold.engines.includes(engine) + ? fold.wire + : advertisedToolName(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], ); } @@ -72,10 +144,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 { diff --git a/src/agent/tool-search.test.ts b/src/agent/tool-search.test.ts index d52b02b00..242471a74 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, @@ -1050,6 +1051,25 @@ describe("advertisedToolNamesForWorker", () => { expect(build).not.toContain("write_file"); }); + 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 50fdc695f..c9486db94 100644 --- a/src/agent/tool-search.ts +++ b/src/agent/tool-search.ts @@ -12,9 +12,9 @@ import { import type { SessionMode } from "../config/session-mode.js"; import { sessionModeEnablesSubAgents } from "../config/session-mode.js"; import { + advertisedNamesForEngine, advertisedToolName, foldFileToolNames, - GPT_PATCH_ENGINES, projectToolDefinition, type ToolProfile, } from "./tool-aliases.js"; @@ -139,10 +139,10 @@ export function advertisedToolNamesForWorker(opts: { seen.add(name); out.push(name); } - if (opts.profile !== "gpt") return out; + if (opts.profile === undefined || opts.profile === "default") return out; return foldFileToolNames( - out.map((name) => advertisedToolName(name, "gpt")), - "gpt", + out.map((name) => advertisedToolName(name, opts.profile)), + opts.profile, ); } @@ -177,19 +177,11 @@ function isAlreadyAdvertised( advertisedNames: readonly string[], ): boolean { const engine = canonicalToolName(defName); - if ( - advertisedNames.includes(defName) || - advertisedNames.includes(engine) || - advertisedNames.includes(advertisedToolName(engine)) || - advertisedNames.includes(advertisedToolName(engine, "gpt")) - ) { + if (advertisedNames.includes(defName) || advertisedNames.includes(engine)) { return true; } - // apply_patch stands in for the file engines; a gpt prefix lists the wire - // name only, so search must not offer write/edit/delete beside it. - return ( - advertisedNames.includes("apply_patch") && - (GPT_PATCH_ENGINES as readonly string[]).includes(engine) + return advertisedNamesForEngine(engine).some((name) => + advertisedNames.includes(name), ); } diff --git a/src/subagent/capability-preflight.test.ts b/src/subagent/capability-preflight.test.ts index b25065f37..69876c7c0 100644 --- a/src/subagent/capability-preflight.test.ts +++ b/src/subagent/capability-preflight.test.ts @@ -10,7 +10,12 @@ import { type CapabilityUnavailable, } from "./capability-preflight.js"; import type { CapabilityFilter } from "../agent/profile-types.js"; -import { DOCS_TOOLS, READ_TOOLS } from "../agent/directors/tool-sets.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", @@ -255,6 +260,55 @@ describe("preflightCapabilities", () => { 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"], diff --git a/src/subagent/capability-preflight.ts b/src/subagent/capability-preflight.ts index af42b126e..01074a1aa 100644 --- a/src/subagent/capability-preflight.ts +++ b/src/subagent/capability-preflight.ts @@ -5,10 +5,11 @@ * 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. `apply_patch` is not an engine id: on the gpt profile - * it maps onto the file-mutation engines the director already allows, and - * with no resolved profile it stays unknown. Fail-closed: unknown names - * and allowlist/denylist misses reject. + * same requirement. A wire name the worker's resolved profile advertises + * maps onto the engine ids that profile projects it to, when the director + * already allows one of those engines. A name that profile does not + * advertise, and that is not a catalog engine, stays unknown. Fail-closed: + * unknown names and allowlist/denylist misses reject. * * `stale_snapshot` is never emitted here — it is the mount-time echo in * run.ts (a stamped tool missing from the live mount). It lives in this @@ -16,7 +17,10 @@ */ import { canonicalToolName } from "../agent/canonical-tool-name.js"; -import { GPT_PATCH_ENGINES, type ToolProfile } from "../agent/tool-aliases.js"; +import { + enginesForAdvertisedWire, + type ToolProfile, +} from "../agent/tool-aliases.js"; import { DIRECTOR_REGISTRY, packageToCapabilities, @@ -227,46 +231,45 @@ export function leafTierAlternatives(): readonly string[] { .slice(0, 3); } -function patchEnginesCovered( +function enginesCoveredByDirector( + engines: readonly string[], filter: CapabilityFilter | undefined, + postFilter: ReadonlySet, ): readonly string[] { - if (filter === undefined) return GPT_PATCH_ENGINES; + if (filter === undefined) return engines; const named = new Set(filter.tools.map((name) => canonicalToolName(name))); if (filter.mode === "allow") { - return GPT_PATCH_ENGINES.filter((engine) => named.has(engine)); + return engines.filter( + (engine) => named.has(engine) || postFilter.has(engine), + ); } - return GPT_PATCH_ENGINES.filter((engine) => !named.has(engine)); + return engines.filter((engine) => !named.has(engine)); } /** - * `apply_patch` is the gpt wire name for the file engines already on the - * director. Other profiles, and a missing profile, leave it unknown. + * Profile wire names become the engines that profile advertises. Undefined + * means this name is not a wire for the resolved profile — the caller falls + * through to catalog engines. No resolved profile does not guess. */ -function expandApplyPatch( +function expandAdvertisedWire( trimmed: string, - engine: string, profile: ToolProfile | undefined, filter: CapabilityFilter | undefined, -): string[] | { unavailable: CapabilityUnavailable } { - if (engine !== "apply_patch") return [engine]; - if (profile !== "gpt") { - const suggestion = nearestToolName(trimmed); - return { - unavailable: { - code: "unknown_tool", - tool: trimmed, - ...(suggestion !== undefined ? { suggestion } : {}), - }, - }; - } - const covered = patchEnginesCovered(filter); + postFilter: ReadonlySet, +): string[] | { unavailable: CapabilityUnavailable } | undefined { + if (profile === undefined) return undefined; + const mapped = enginesForAdvertisedWire(trimmed, profile); + if (mapped === undefined) return undefined; + const covered = enginesCoveredByDirector(mapped, filter, postFilter); if (covered.length > 0) return [...covered]; - const denied = filter?.mode === "exclude"; + const hint = mapped[0]; return { unavailable: { - code: denied ? "permission_static" : "missing_tool", - tool: "apply_patch", - alternatives: rerouteAlternatives("write_file"), + code: filter?.mode === "exclude" ? "permission_static" : "missing_tool", + tool: trimmed, + ...(hint !== undefined + ? { alternatives: rerouteAlternatives(hint) } + : {}), }, }; } @@ -297,15 +300,16 @@ export function preflightCapabilities( return { ok: false, unavailable: { code: "unknown_tool", tool: raw } }; } const engine = canonicalToolName(trimmed); - const expanded = expandApplyPatch( + const wireExpanded = expandAdvertisedWire( trimmed, - engine, input.toolProfile, resolvedFilter, + postFilter, ); - if (!Array.isArray(expanded)) { - return { ok: false, unavailable: expanded.unavailable }; + if (wireExpanded !== undefined && !Array.isArray(wireExpanded)) { + return { ok: false, unavailable: wireExpanded.unavailable }; } + 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 From acc4bed91ac99204b6e3dd377048b40518446ac8 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 15 Oct 2026 19:36:47 -0700 Subject: [PATCH 3/5] fix(agent): reject wires the worker profile does not advertise requires_tools accepts a wire name only when that profile projects it onto an allowlisted engine. shell and update_plan stay gpt-only; default keeps bash for run_shell. --- .../agent-fleet-requires-tools.test.ts | 34 ++++++++++++-- src/subagent/capability-preflight.test.ts | 47 ++++++++++++++++++- src/subagent/capability-preflight.ts | 47 ++++++++++++++----- 3 files changed, 111 insertions(+), 17 deletions(-) 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/capability-preflight.test.ts b/src/subagent/capability-preflight.test.ts index 69876c7c0..5ccf65f6d 100644 --- a/src/subagent/capability-preflight.test.ts +++ b/src/subagent/capability-preflight.test.ts @@ -67,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"] }); }); @@ -319,6 +319,51 @@ describe("preflightCapabilities", () => { }); 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"); + }); }); describe("formatCapabilityUnavailable", () => { diff --git a/src/subagent/capability-preflight.ts b/src/subagent/capability-preflight.ts index 01074a1aa..88b0be84f 100644 --- a/src/subagent/capability-preflight.ts +++ b/src/subagent/capability-preflight.ts @@ -3,13 +3,14 @@ * * 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. A wire name the worker's resolved profile advertises - * maps onto the engine ids that profile projects it to, when the director - * already allows one of those engines. A name that profile does not - * advertise, and that is not a catalog engine, stays unknown. Fail-closed: - * unknown names and allowlist/denylist misses reject. + * 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 * run.ts (a stamped tool missing from the live mount). It lives in this @@ -19,6 +20,7 @@ import { canonicalToolName } from "../agent/canonical-tool-name.js"; import { enginesForAdvertisedWire, + toolProfiles, type ToolProfile, } from "../agent/tool-aliases.js"; import { @@ -57,9 +59,9 @@ export interface PreflightCapabilitiesInput { /** Worker label for messages (director id or profile id). */ agentLabel: string; /** - * Tool profile of the model this spawn will run. `apply_patch` is the gpt - * wire name for the file-mutation engines, not a catalog id. Omit when no - * family is resolved — then apply_patch stays unknown. + * 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; } @@ -248,8 +250,9 @@ function enginesCoveredByDirector( /** * Profile wire names become the engines that profile advertises. Undefined - * means this name is not a wire for the resolved profile — the caller falls - * through to catalog engines. No resolved profile does not guess. + * 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, @@ -259,7 +262,25 @@ function expandAdvertisedWire( ): string[] | { unavailable: CapabilityUnavailable } | undefined { if (profile === undefined) return undefined; const mapped = enginesForAdvertisedWire(trimmed, profile); - if (mapped === undefined) return undefined; + 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]; From 8be41a1c2bcdc8aa2954cdf07cf3c2f9a8bd8389 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 15 Oct 2026 19:57:33 -0700 Subject: [PATCH 4/5] fix(agent): treat gpt shell as advertised The worker prefix lists shell, but the call gate only matched the default bash wire, so a shell call promoted run_shell. An exclude of manage_tasks now denies both update_plan and the engine id; shell and run_shell follow the same rule. --- src/agent/tool-aliases.ts | 26 +++++++---- src/agent/tool-search.test.ts | 56 +++++++++++++++++++++++ src/session/assemble-runtime.ts | 10 ++-- src/subagent/capability-preflight.test.ts | 34 ++++++++++++++ src/subagent/capability-preflight.ts | 40 ++++++++-------- 5 files changed, 131 insertions(+), 35 deletions(-) diff --git a/src/agent/tool-aliases.ts b/src/agent/tool-aliases.ts index 98c259867..444b9cbae 100644 --- a/src/agent/tool-aliases.ts +++ b/src/agent/tool-aliases.ts @@ -106,16 +106,19 @@ export function enginesForAdvertisedWire( 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 fold = PROFILE_FOLDS[profile]; - const wire = - fold !== undefined && fold.engines.includes(engine) - ? fold.wire - : advertisedToolName(engine, profile); + const wire = profileWireName(engine, profile); if (seen.has(wire)) continue; seen.add(wire); names.push(wire); @@ -205,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 242471a74..496563d0f 100644 --- a/src/agent/tool-search.test.ts +++ b/src/agent/tool-search.test.ts @@ -1051,6 +1051,62 @@ describe("advertisedToolNamesForWorker", () => { 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()) { 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/capability-preflight.test.ts b/src/subagent/capability-preflight.test.ts index 5ccf65f6d..1ece3cc3a 100644 --- a/src/subagent/capability-preflight.test.ts +++ b/src/subagent/capability-preflight.test.ts @@ -364,6 +364,40 @@ describe("preflightCapabilities", () => { 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 88b0be84f..ea1b6a3cf 100644 --- a/src/subagent/capability-preflight.ts +++ b/src/subagent/capability-preflight.ts @@ -245,6 +245,9 @@ function enginesCoveredByDirector( (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)); } @@ -353,6 +356,21 @@ export function preflightCapabilities( 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: resolved, + alternatives: rerouteAlternatives(resolved), + }, + }; + } if (!postFilter.has(resolved) && !isLiveMcp) { if (allow !== undefined && !allow.has(resolved)) { return { @@ -364,28 +382,6 @@ export function preflightCapabilities( }, }; } - if (deny !== undefined && deny.has(resolved)) { - return { - ok: false, - unavailable: { - code: "permission_static", - tool: resolved, - alternatives: rerouteAlternatives(resolved), - }, - }; - } - } else if (isLiveMcp && deny !== undefined && deny.has(resolved)) { - // 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: resolved, - alternatives: rerouteAlternatives(resolved), - }, - }; } if (!seen.has(resolved)) { seen.add(resolved); From fb9f15fbfd44e7774da4b0b87d73df668a8c71ef Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 15 Oct 2026 20:01:21 -0700 Subject: [PATCH 5/5] chore(dead-exports): raise scanned-file floor to the live count --- scripts/dead-export-guard.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 }