Skip to content

Commit acc4bed

Browse files
committed
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.
1 parent e0abe4a commit acc4bed

3 files changed

Lines changed: 111 additions & 17 deletions

File tree

‎src/subagent/agent-fleet-requires-tools.test.ts‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,14 @@ const BUILD_LIKE_PROFILE: AgentProfile = {
4949
function makeDeps(
5050
run: (params: RunSubAgentParams) => Promise<RunSubAgentResult>,
5151
capturedTelemetry: TelemetryEvent[],
52+
providerOverride: AgentFleetDeps["provider"] = provider,
5253
): AgentFleetDeps {
5354
const sessions = createSubAgentSessionStore();
5455
return {
5556
permissionGate: testPermissionGate,
5657
cwd: "/tmp",
5758
getWorkdirBase: () => "/tmp/workdir",
58-
provider,
59+
provider: providerOverride,
5960
run,
6061
sessions,
6162
fleetRecords: createFleetMailbox(sessions),
@@ -232,9 +233,36 @@ describe("spawn_agent requires_tools preflight", () => {
232233
expect(seenRequires).toEqual(["manage_tasks"]);
233234
});
234235

235-
test("update_plan alias survives dispatch and stamps manage_tasks", async () => {
236+
test("default family rejects update_plan, a gpt-only wire", async () => {
236237
const telemetry: TelemetryEvent[] = [];
237-
const deps = makeDeps(async () => ({ report: "done" }), telemetry);
238+
let runCalled = false;
239+
const deps = makeDeps(async () => {
240+
runCalled = true;
241+
return { report: "done" };
242+
}, telemetry);
243+
const spawn = createSpawnAgentTool(deps);
244+
245+
const result = await callSpawn(spawn, {
246+
description: "alias plan job",
247+
prompt: "track the work",
248+
agent: "read-only-worker",
249+
requires_tools: ["update_plan"],
250+
});
251+
252+
expect(result.isError).toBe(true);
253+
expect(result.content).toContain('unknown tool "update_plan"');
254+
expect(runCalled).toBe(false);
255+
expect(deps.sessions.list()).toEqual([]);
256+
expect(telemetry).toEqual([]);
257+
});
258+
259+
test("gpt family accepts update_plan and stamps manage_tasks", async () => {
260+
const telemetry: TelemetryEvent[] = [];
261+
const deps = makeDeps(async () => ({ report: "done" }), telemetry, {
262+
...provider,
263+
providerName: "codex",
264+
model: "gpt-5",
265+
});
238266
const spawn = createSpawnAgentTool(deps);
239267

240268
const result = await callSpawn(spawn, {

‎src/subagent/capability-preflight.test.ts‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ describe("preflightCapabilities", () => {
6767
});
6868
});
6969

70-
test("aliases collapse on both sides (shell requirement vs run_shell filter)", () => {
70+
test("unresolved profile still collapses shell onto run_shell", () => {
7171
const result = preflight(["shell"], ALLOW_SHELL);
7272
expect(result).toEqual({ ok: true, canonical: ["run_shell"] });
7373
});
@@ -319,6 +319,51 @@ describe("preflightCapabilities", () => {
319319
});
320320
expect(result).toEqual({ ok: true, canonical: ["write_file"] });
321321
});
322+
323+
test("default profile rejects gpt-only wires even when the engine is allowed", () => {
324+
const resolvedFilter: CapabilityFilter = {
325+
mode: "allow",
326+
tools: [...BUILD_TOOLS, "manage_tasks"],
327+
};
328+
for (const wire of ["apply_patch", "shell", "update_plan"] as const) {
329+
const result = preflightCapabilities({
330+
required: [wire],
331+
resolvedFilter,
332+
knownEngines: DEFAULT_KNOWN_ENGINES,
333+
agentLabel: "coder",
334+
toolProfile: "default",
335+
});
336+
expect(result.ok, wire).toBe(false);
337+
if (result.ok) continue;
338+
expect(result.unavailable.code, wire).toBe("unknown_tool");
339+
expect(result.unavailable.tool, wire).toBe(wire);
340+
}
341+
});
342+
343+
test("default accepts bash, the wire it advertises for run_shell", () => {
344+
const result = preflightCapabilities({
345+
required: ["bash"],
346+
resolvedFilter: ALLOW_SHELL,
347+
knownEngines: DEFAULT_KNOWN_ENGINES,
348+
agentLabel: "coder",
349+
toolProfile: "default",
350+
});
351+
expect(result).toEqual({ ok: true, canonical: ["run_shell"] });
352+
});
353+
354+
test("gpt rejects bash because that profile advertises shell", () => {
355+
const result = preflightCapabilities({
356+
required: ["bash"],
357+
resolvedFilter: ALLOW_SHELL,
358+
knownEngines: DEFAULT_KNOWN_ENGINES,
359+
agentLabel: "coder",
360+
toolProfile: "gpt",
361+
});
362+
expect(result.ok).toBe(false);
363+
if (result.ok) throw new Error("expected rejection");
364+
expect(result.unavailable.code).toBe("unknown_tool");
365+
expect(result.unavailable.tool).toBe("bash");
366+
});
322367
});
323368

324369
describe("formatCapabilityUnavailable", () => {

‎src/subagent/capability-preflight.ts‎

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@
33
*
44
* A `requires_tools` entry is a hard requirement: the tool must mount on
55
* the worker or the dispatch rejects before any session, telemetry, or
6-
* worktree exists. Names are canonical engine ids on both sides (aliases
7-
* collapse via canonicalToolName), so `shell` and `run_shell` are the
8-
* same requirement. A wire name the worker's resolved profile advertises
9-
* maps onto the engine ids that profile projects it to, when the director
10-
* already allows one of those engines. A name that profile does not
11-
* advertise, and that is not a catalog engine, stays unknown. Fail-closed:
12-
* unknown names and allowlist/denylist misses reject.
6+
* worktree exists. Engine ids are canonical on both sides. A wire name
7+
* passes only when the worker's resolved profile advertises it, and then
8+
* only as the engine ids that profile projects it onto — a rename is one
9+
* engine, a fold is every covered engine. A wire some other profile
10+
* advertises is unknown here (`shell` is gpt; default advertises `bash`).
11+
* With no resolved profile, hidden aliases still canonicalize and a fold
12+
* target stays unknown. Fail-closed: unknown names and allowlist/denylist
13+
* misses reject.
1314
*
1415
* `stale_snapshot` is never emitted here — it is the mount-time echo in
1516
* run.ts (a stamped tool missing from the live mount). It lives in this
@@ -19,6 +20,7 @@
1920
import { canonicalToolName } from "../agent/canonical-tool-name.js";
2021
import {
2122
enginesForAdvertisedWire,
23+
toolProfiles,
2224
type ToolProfile,
2325
} from "../agent/tool-aliases.js";
2426
import {
@@ -57,9 +59,9 @@ export interface PreflightCapabilitiesInput {
5759
/** Worker label for messages (director id or profile id). */
5860
agentLabel: string;
5961
/**
60-
* Tool profile of the model this spawn will run. `apply_patch` is the gpt
61-
* wire name for the file-mutation engines, not a catalog id. Omit when no
62-
* family is resolved — then apply_patch stays unknown.
62+
* Tool profile of the model this spawn will run. Wire names resolve through
63+
* that profile's projection. Omit when no family is resolved — hidden
64+
* aliases still canonicalize, and a fold target stays unknown.
6365
*/
6466
toolProfile?: ToolProfile | undefined;
6567
}
@@ -248,8 +250,9 @@ function enginesCoveredByDirector(
248250

249251
/**
250252
* Profile wire names become the engines that profile advertises. Undefined
251-
* means this name is not a wire for the resolved profile — the caller falls
252-
* through to catalog engines. No resolved profile does not guess.
253+
* means this name is not a wire for any profile — the caller falls through
254+
* to catalog engines. A wire only another profile advertises is unknown.
255+
* No resolved profile does not guess.
253256
*/
254257
function expandAdvertisedWire(
255258
trimmed: string,
@@ -259,7 +262,25 @@ function expandAdvertisedWire(
259262
): string[] | { unavailable: CapabilityUnavailable } | undefined {
260263
if (profile === undefined) return undefined;
261264
const mapped = enginesForAdvertisedWire(trimmed, profile);
262-
if (mapped === undefined) return undefined;
265+
if (mapped === undefined) {
266+
const foreign = toolProfiles().some(
267+
(candidate) =>
268+
candidate !== profile &&
269+
enginesForAdvertisedWire(trimmed, candidate) !== undefined,
270+
);
271+
if (!foreign) return undefined;
272+
const suggestion = nearestToolName(trimmed);
273+
const useful =
274+
suggestion !== undefined &&
275+
suggestion.toLowerCase() !== trimmed.toLowerCase();
276+
return {
277+
unavailable: {
278+
code: "unknown_tool",
279+
tool: trimmed,
280+
...(useful ? { suggestion } : {}),
281+
},
282+
};
283+
}
263284
const covered = enginesCoveredByDirector(mapped, filter, postFilter);
264285
if (covered.length > 0) return [...covered];
265286
const hint = mapped[0];

0 commit comments

Comments
 (0)