diff --git a/src/agent/profiles-snapshot.test.ts b/src/agent/profiles-snapshot.test.ts new file mode 100644 index 000000000..b6d275257 --- /dev/null +++ b/src/agent/profiles-snapshot.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, test } from "bun:test"; +import { mkdtemp, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { + currentProfileSnapshotRevision, + loadAgentProfiles, + loadAgentProfilesWithDiagnostics, +} from "./profiles.js"; + +async function makeAgentsDir(files: Record): Promise { + const dir = await mkdtemp(join(tmpdir(), "cl9476-profiles-")); + for (const [name, content] of Object.entries(files)) { + await writeFile(join(dir, name), content); + } + return dir; +} + +const VALID_PROFILE = JSON.stringify({ + id: "local-reader", + systemPromptRole: "You read files.", + capabilities: { mode: "allow", tools: ["read_file"] }, +}); + +describe("loadAgentProfilesWithDiagnostics", () => { + test("malformed file is reported and never poisons the load", async () => { + const dir = await makeAgentsDir({ + "good.json": VALID_PROFILE, + "broken.json": "{ not valid json", + "notes.txt": "ignored, not a profile extension", + }); + + const { profiles, diagnostics } = + await loadAgentProfilesWithDiagnostics(dir); + + expect(profiles.some((p) => p.id === "local-reader")).toBe(true); + expect(diagnostics.malformed).toHaveLength(1); + expect(diagnostics.malformed[0]?.path).toBe(join(dir, "broken.json")); + expect(diagnostics.malformed[0]?.reason).toBe("invalid JSON"); + expect(typeof diagnostics.revision).toBe("number"); + }); + + test("schema-invalid file is reported with its reason", async () => { + const dir = await makeAgentsDir({ + "bad-shape.json": JSON.stringify({ id: 42 }), + }); + + const { profiles, diagnostics } = + await loadAgentProfilesWithDiagnostics(dir); + + expect(profiles.some((p) => p.id === "local-reader")).toBe(false); + expect(diagnostics.malformed).toHaveLength(1); + expect(diagnostics.malformed[0]?.reason).toBe("schema validation failed"); + }); + + test("loadAgentProfiles delegates and resolves identically", async () => { + const dir = await makeAgentsDir({ + "good.json": VALID_PROFILE, + "broken.json": "{ nope", + }); + + const viaDiagnostics = await loadAgentProfilesWithDiagnostics(dir); + const direct = await loadAgentProfiles(dir); + + expect(direct.map((p) => p.id).sort()).toEqual( + viaDiagnostics.profiles.map((p) => p.id).sort(), + ); + }); + + test("every load bumps the snapshot revision", async () => { + const dir = await makeAgentsDir({ "good.json": VALID_PROFILE }); + const before = currentProfileSnapshotRevision(); + + const first = await loadAgentProfilesWithDiagnostics(dir); + const second = await loadAgentProfilesWithDiagnostics(dir); + + expect(second.diagnostics.revision).toBeGreaterThan( + first.diagnostics.revision, + ); + expect(first.diagnostics.revision).toBeGreaterThan(before); + expect(currentProfileSnapshotRevision()).toBe(second.diagnostics.revision); + }); + + test("missing directory resolves defaults with no malformed entries", async () => { + const { profiles, diagnostics } = await loadAgentProfilesWithDiagnostics( + join(tmpdir(), "cl9476-does-not-exist"), + ); + + expect(profiles.length).toBeGreaterThan(0); + expect(diagnostics.malformed).toEqual([]); + }); +}); diff --git a/src/agent/profiles.ts b/src/agent/profiles.ts index 8f46d76ff..cb2135e1b 100644 --- a/src/agent/profiles.ts +++ b/src/agent/profiles.ts @@ -71,6 +71,31 @@ function isENOENT(err: unknown): boolean { // DIRECTOR_IDS, which are reserved and skipped at load (CL-7015). const registry: AgentProfile[] = [...defaultPlugin.agents]; +// Load diagnostics: additive over loadAgentProfiles. `revision` stamps the +// profile snapshot a dispatch was verified against (agent-fleet records it on +// the session); `malformed` names local files that failed to load and why. +// A malformed file never blocks the load — it is skipped and named here so +// callers can surface it. requires_tools preflight verifies against the +// resolved dispatch capabilities only, never against this list (CL-9476 keeps +// no malformed_profile preflight branch: agent-fleet never plumbed a failing +// source through dispatch, so the branch was dead and has been removed). +export interface MalformedAgentProfile { + path: string; + reason: string; +} + +export interface AgentProfileDiagnostics { + revision: number; + malformed: MalformedAgentProfile[]; +} + +let profileSnapshotRevision = 0; + +/** Revision of the most recent profile snapshot load. */ +export function currentProfileSnapshotRevision(): number { + return profileSnapshotRevision; +} + // Merge a profile into a list: replace a same-id entry or append. Used to layer // profiles by precedence (defaults < plugin < local). function mergeProfileInto(list: AgentProfile[], profile: AgentProfile): void { @@ -95,6 +120,28 @@ export async function loadAgentProfiles( dir: string, extraProfiles: AgentProfile[] = [], ): Promise { + return (await loadAgentProfilesWithDiagnostics(dir, extraProfiles)).profiles; +} + +/** + * loadAgentProfiles plus load diagnostics. Additive: profiles resolve + * exactly as before; unreadable/unparseable/invalid local files are named in + * `diagnostics.malformed` instead of skipped silently. Every call bumps the + * snapshot revision a dispatch stamps on its session record. + */ +export async function loadAgentProfilesWithDiagnostics( + dir: string, + extraProfiles: AgentProfile[] = [], +): Promise<{ profiles: AgentProfile[]; diagnostics: AgentProfileDiagnostics }> { + profileSnapshotRevision += 1; + const revision = profileSnapshotRevision; + const malformed: MalformedAgentProfile[] = []; + const done = ( + profiles: AgentProfile[], + ): { profiles: AgentProfile[]; diagnostics: AgentProfileDiagnostics } => ({ + profiles, + diagnostics: { revision, malformed }, + }); let entries: string[]; try { entries = await readdir(dir); @@ -105,7 +152,7 @@ export async function loadAgentProfiles( if (isReservedDirectorProfile(p)) continue; mergeProfileInto(merged, p); } - return merged; + return done(merged); } throw err; } @@ -121,16 +168,24 @@ export async function loadAgentProfiles( try { raw = await readFile(filePath, "utf8"); } catch { + malformed.push({ path: filePath, reason: "unreadable file" }); continue; } let parsed: unknown; try { parsed = isJSON ? JSON.parse(raw) : Bun.YAML.parse(raw); } catch { + malformed.push({ + path: filePath, + reason: isJSON ? "invalid JSON" : "invalid YAML", + }); continue; } const result = AgentProfileSchema(parsed); - if (result instanceof type.errors) continue; + if (result instanceof type.errors) { + malformed.push({ path: filePath, reason: "schema validation failed" }); + continue; + } const profile = result as AgentProfile; if (isReservedDirectorProfile(profile)) continue; // Resolve systemPromptPath relative to this directory. The file content @@ -158,5 +213,5 @@ export async function loadAgentProfiles( mergeProfileInto(merged, profile); } for (const profile of local) mergeProfileInto(merged, profile); - return merged; + return done(merged); } diff --git a/src/subagent/agent-fleet-requires-tools.test.ts b/src/subagent/agent-fleet-requires-tools.test.ts new file mode 100644 index 000000000..6447ee21a --- /dev/null +++ b/src/subagent/agent-fleet-requires-tools.test.ts @@ -0,0 +1,400 @@ +import { describe, expect, test } from "bun:test"; + +import { + createFleetMailbox, + createSpawnAgentTool, + tierGateRequiresTools, + type AgentFleetDeps, +} from "./agent-fleet.js"; +import { formatCapabilityUnavailable } from "./capability-preflight.js"; +import { unlimitedAdmissionQueue } from "./admission.js"; +import { createPermissionGate } from "../permission/gate.js"; +import { NOOP_TELEMETRY, type TelemetryEvent } from "../telemetry/index.js"; +import type { AgentProfile } from "../agent/profiles.js"; +import { createSubAgentSessionStore } from "./session-store.js"; +import type { RunSubAgentParams, RunSubAgentResult } from "./types.js"; + +const testPermissionGate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: true, + reactorGated: false, +}); + +const provider = { + providerName: "test-provider", + baseURL: "http://localhost", + model: "test-model", +}; + +const READ_ONLY_PROFILE: AgentProfile = { + id: "read-only-worker", + systemPromptRole: "You read files.", + capabilities: { mode: "allow", tools: ["read_file"] }, +}; + +const FULL_MOUNT_PROFILE: AgentProfile = { + id: "full-worker", + systemPromptRole: "You do anything.", +}; + +function makeDeps( + run: (params: RunSubAgentParams) => Promise, + capturedTelemetry: TelemetryEvent[], +): AgentFleetDeps { + const sessions = createSubAgentSessionStore(); + return { + permissionGate: testPermissionGate, + cwd: "/tmp", + getWorkdirBase: () => "/tmp/workdir", + provider, + run, + sessions, + fleetRecords: createFleetMailbox(sessions), + admission: unlimitedAdmissionQueue(), + profiles: [READ_ONLY_PROFILE, FULL_MOUNT_PROFILE], + telemetry: { + ...NOOP_TELEMETRY, + capture: (event: TelemetryEvent) => { + capturedTelemetry.push(event); + }, + }, + }; +} + +async function callSpawn( + tool: ReturnType, + args: Record, +): Promise<{ content: string; isError?: boolean }> { + if (tool.kind !== "full") throw new Error("expected full tool"); + const result = await tool.handler( + { + id: `spawn-${Math.random()}`, + name: "spawn_agent", + arguments: args, + }, + new AbortController().signal, + ); + const content = + typeof result.content === "string" + ? result.content + : JSON.stringify(result.content); + return { + content, + ...(result.isError !== undefined ? { isError: result.isError } : {}), + }; +} + +describe("spawn_agent requires_tools preflight", () => { + test("rejects a tool outside the allowlist with no session, telemetry, or run", async () => { + const telemetry: TelemetryEvent[] = []; + let runCalled = false; + const deps = makeDeps(async () => { + runCalled = true; + return { report: "done" }; + }, telemetry); + const spawn = createSpawnAgentTool(deps); + + const result = await callSpawn(spawn, { + description: "shell job", + prompt: "run a command", + agent: "read-only-worker", + requires_tools: ["run_shell"], + }); + + expect(result.isError).toBe(true); + expect(result.content.startsWith("Error:")).toBe(true); + expect(result.content).toContain("run_shell"); + expect(result.content).toContain("Re-dispatch"); + expect(result.content).not.toContain("continuable"); + expect(runCalled).toBe(false); + expect(deps.sessions.list()).toEqual([]); + expect(telemetry).toEqual([]); + }); + + test("rejects an unknown tool with a did-you-mean hint and no session", async () => { + const telemetry: TelemetryEvent[] = []; + let runCalled = false; + const deps = makeDeps(async () => { + runCalled = true; + return { report: "done" }; + }, telemetry); + const spawn = createSpawnAgentTool(deps); + + const result = await callSpawn(spawn, { + description: "typo job", + prompt: "do it", + agent: "read-only-worker", + requires_tools: ["run_shel"], + }); + + expect(result.isError).toBe(true); + expect(result.content).toContain('Did you mean "run_shell"?'); + expect(runCalled).toBe(false); + expect(deps.sessions.list()).toEqual([]); + expect(telemetry).toEqual([]); + }); + + test("accepts a mounted tool and stamps the session record", async () => { + const telemetry: TelemetryEvent[] = []; + let seenRequires: readonly string[] | undefined; + const deps = makeDeps(async (params) => { + seenRequires = params.requiresTools; + return { report: "done" }; + }, telemetry); + const spawn = createSpawnAgentTool(deps); + + const result = await callSpawn(spawn, { + description: "read job", + prompt: "read a file", + agent: "read-only-worker", + requires_tools: ["read_file"], + }); + + expect(result.isError).not.toBe(true); + const body = JSON.parse(result.content) as { + agent_id: string; + status: string; + }; + const session = deps.sessions.get(body.agent_id); + expect(session?.requiresTools).toEqual(["read_file"]); + expect("snapshotRevision" in (session ?? {})).toBe(false); + expect(seenRequires).toEqual(["read_file"]); + expect(telemetry).toContain("subagent_start"); + }); + + test("aliases collapse at dispatch (shell stamps run_shell)", async () => { + const telemetry: TelemetryEvent[] = []; + const deps = makeDeps(async () => ({ report: "done" }), telemetry); + const spawn = createSpawnAgentTool(deps); + + const result = await callSpawn(spawn, { + description: "alias read job", + prompt: "read a file", + agent: "read-only-worker", + requires_tools: ["read"], + }); + + expect(result.isError).not.toBe(true); + const body = JSON.parse(result.content) as { agent_id: string }; + expect(deps.sessions.get(body.agent_id)?.requiresTools).toEqual([ + "read_file", + ]); + }); + + test("omitted requires_tools leaves the session unstamped", async () => { + const telemetry: TelemetryEvent[] = []; + const deps = makeDeps(async () => ({ report: "done" }), telemetry); + const spawn = createSpawnAgentTool(deps); + + const result = await callSpawn(spawn, { + description: "plain job", + prompt: "read a file", + agent: "read-only-worker", + }); + + expect(result.isError).not.toBe(true); + const body = JSON.parse(result.content) as { agent_id: string }; + const session = deps.sessions.get(body.agent_id); + expect(session?.requiresTools).toBeUndefined(); + expect("snapshotRevision" in (session ?? {})).toBe(false); + }); + + test("manage_tasks survives dispatch under a narrow allowlist and stamps canonically", async () => { + const telemetry: TelemetryEvent[] = []; + let seenRequires: readonly string[] | undefined; + const deps = makeDeps(async (params) => { + seenRequires = params.requiresTools; + return { report: "done" }; + }, telemetry); + const spawn = createSpawnAgentTool(deps); + + const result = await callSpawn(spawn, { + description: "plan job", + prompt: "track the work", + agent: "read-only-worker", + requires_tools: ["manage_tasks"], + }); + + expect(result.isError).not.toBe(true); + const body = JSON.parse(result.content) as { agent_id: string }; + expect(deps.sessions.get(body.agent_id)?.requiresTools).toEqual([ + "manage_tasks", + ]); + expect(seenRequires).toEqual(["manage_tasks"]); + }); + + test("update_plan alias survives dispatch and stamps manage_tasks", async () => { + const telemetry: TelemetryEvent[] = []; + const deps = makeDeps(async () => ({ 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).not.toBe(true); + const body = JSON.parse(result.content) as { agent_id: string }; + expect(deps.sessions.get(body.agent_id)?.requiresTools).toEqual([ + "manage_tasks", + ]); + }); + + test("leaf requires_tools=[spawn_agent] rejects pre-spawn as missing_tool naming the leaf restriction", async () => { + const telemetry: TelemetryEvent[] = []; + let runCalled = false; + const deps = makeDeps(async () => { + runCalled = true; + return { report: "done" }; + }, telemetry); + const spawn = createSpawnAgentTool(deps); + + const result = await callSpawn(spawn, { + description: "fleet job", + prompt: "spawn more workers", + agent: "full-worker", + requires_tools: ["spawn_agent"], + }); + + expect(result.isError).toBe(true); + expect(result.content.startsWith("Error:")).toBe(true); + expect(result.content).toContain("spawn_agent"); + expect(result.content).toContain("Tier 3 leaf"); + expect(result.content).toMatch(/Re-dispatch|drop the requirement/); + expect(result.content).not.toContain("stale_snapshot"); + expect(runCalled).toBe(false); + expect(deps.sessions.list()).toEqual([]); + expect(telemetry).toEqual([]); + }); + + test("leaf requires_tools=[submit_result] passes preflight under a narrow allowlist (leaf reporting channel)", async () => { + const telemetry: TelemetryEvent[] = []; + let runCalled = false; + let seenRequires: readonly string[] | undefined; + let seenTier: unknown; + const deps = makeDeps(async (params) => { + runCalled = true; + seenRequires = params.requiresTools; + seenTier = params.tier; + return { report: "done" }; + }, telemetry); + const spawn = createSpawnAgentTool(deps); + + const result = await callSpawn(spawn, { + description: "leaf report job", + prompt: "read a file and report", + agent: "read-only-worker", + requires_tools: ["submit_result"], + }); + + expect(result.isError).not.toBe(true); + expect(runCalled).toBe(true); + const body = JSON.parse(result.content) as { agent_id: string }; + expect(deps.sessions.get(body.agent_id)?.requiresTools).toEqual([ + "submit_result", + ]); + expect(seenRequires).toEqual(["submit_result"]); + expect(seenTier).toBe("leaf"); + }); + + test("whitespace-only requires_tools rejects fail-closed with no session, telemetry, or run", async () => { + for (const requiresTools of [[" "], ["read_file", " "]]) { + const telemetry: TelemetryEvent[] = []; + let runCalled = false; + const deps = makeDeps(async () => { + runCalled = true; + return { report: "done" }; + }, telemetry); + const spawn = createSpawnAgentTool(deps); + + const result = await callSpawn(spawn, { + description: "blank job", + prompt: "do it", + agent: "read-only-worker", + requires_tools: requiresTools, + }); + + expect(result.isError).toBe(true); + expect(result.content.startsWith("Error:")).toBe(true); + expect(result.content).toContain("non-empty tool names"); + expect(runCalled).toBe(false); + expect(deps.sessions.list()).toEqual([]); + expect(telemetry).toEqual([]); + } + }); +}); + +describe("tierGateRequiresTools", () => { + test("leaf requiring a fleet verb rejects as missing_tool naming the leaf restriction", () => { + const gated = tierGateRequiresTools(["spawn_agent"], "leaf"); + expect(gated?.code).toBe("missing_tool"); + expect(gated?.tool).toBe("spawn_agent"); + expect(gated?.detail).toContain("Tier 3 leaf"); + }); + + test("orchestrator requiring a fleet verb passes", () => { + expect( + tierGateRequiresTools(["spawn_agent"], "orchestrator"), + ).toBeUndefined(); + expect( + tierGateRequiresTools(["spawn_agent"], "nested-orchestrator"), + ).toBeUndefined(); + }); + + test("nested-orchestrator requiring fleet discovery rejects pre-spawn", () => { + const gated = tierGateRequiresTools( + ["search_agents"], + "nested-orchestrator", + ); + expect(gated?.code).toBe("missing_tool"); + expect(gated?.detail).toContain("Tier 2"); + }); + + test("non-leaf requiring the leaf reporting channel rejects pre-spawn", () => { + for (const engine of ["submit_result", "ask_director"]) { + const gated = tierGateRequiresTools([engine], "orchestrator"); + expect(gated?.code).toBe("missing_tool"); + expect(gated?.tool).toBe(engine); + expect(gated?.detail).toContain("Tier 3 leaf workers only"); + } + }); + + test("tier-gated leaf channel names Tier 3 leaves instead of claiming no director mounts it", () => { + const gated = tierGateRequiresTools(["submit_result"], "orchestrator"); + expect(gated?.alternatives ?? []).toEqual([ + "bruckheimer", + "builder", + "counsel", + ]); + const message = formatCapabilityUnavailable( + gated ?? { code: "missing_tool", tool: "submit_result" }, + "test-orchestrator", + ); + expect(message).toContain("Tier 3 leaf workers only"); + expect(message).toContain( + "Re-dispatch to one of (bruckheimer, builder, counsel)", + ); + expect(message).not.toContain("No spawnable director mounts"); + }); + + test("leaf requiring the leaf reporting channel passes", () => { + expect( + tierGateRequiresTools(["submit_result", "ask_director"], "leaf"), + ).toBeUndefined(); + }); + + test("ordinary tools pass on every tier", () => { + for (const tier of [ + "leaf", + "orchestrator", + "nested-orchestrator", + ] as const) { + expect( + tierGateRequiresTools(["read_file", "manage_tasks"], tier), + ).toBeUndefined(); + } + }); +}); diff --git a/src/subagent/agent-fleet.ts b/src/subagent/agent-fleet.ts index 2da7c9b91..9694703a2 100644 --- a/src/subagent/agent-fleet.ts +++ b/src/subagent/agent-fleet.ts @@ -67,6 +67,14 @@ import { type ReasoningEffort, } from "../provider/reasoning-effort.js"; import type { AgentProfile, CapabilityFilter } from "../agent/profiles.js"; +import { + DEFAULT_KNOWN_ENGINES, + formatCapabilityUnavailable, + leafTierAlternatives, + preflightCapabilities, + rerouteAlternatives, + type CapabilityUnavailable, +} from "./capability-preflight.js"; import { isCodexProviderName } from "../config/codex-providers.js"; import { buildDispatchBrief, type TaskIntent } from "./report.js"; import { @@ -101,6 +109,7 @@ import type { DirectorPackage, ModelRole } from "../agent/directors/types.js"; import { SPAWN_AGENT_TOOL_NAME } from "./tool-taxonomy.js"; import { assertCanTargetAgent, + assertTierMayMountFleetVerb, FleetAuthorityError, type FleetNode, type SubagentTier, @@ -543,12 +552,13 @@ const SpawnAgentArgs = type({ "success_criteria?": "string[]", "do_not?": "string[]", "report_focus?": "string", + "requires_tools?": "string[]", }); export const spawnAgentToolDefinition: ToolDefinition = { name: SPAWN_AGENT_TOOL_NAME, description: - "Start a worker agent and return IMMEDIATELY with its agent_id — this never blocks on the worker's completion. Pass agent= a director/profile id returned by search_agents, or intent= (one of explore|implement|review|plan|general). The child starts blank. One focused task per worker. success_criteria is required for implement/review (and their default directors). Fire several spawn_agent calls in one turn to start independent lanes in parallel, then reply and end the turn — workers keep running while you are idle. Reports arrive as mailbox mail where mailbox delivery is mounted; where wait_agents is mounted (exec primary), collect with it instead. Do not poll. Excess fan-out is queued rather than refused.", + "Start a worker agent and return IMMEDIATELY with its agent_id — this never blocks on the worker's completion. Pass agent= a director/profile id returned by search_agents, or intent= (one of explore|implement|review|plan|general). The child starts blank. One focused task per worker. success_criteria is required for implement/review (and their default directors). Fire several spawn_agent calls in one turn to start independent lanes in parallel, then reply and end the turn — workers keep running while you are idle. Reports arrive as mailbox mail where mailbox delivery is mounted; where wait_agents is mounted (exec primary), collect with it instead. Do not poll. Excess fan-out is queued rather than refused. requires_tools declares hard tool requirements verified pre-spawn against the worker's capability mount (fail-closed with a reroute hint); a preflight rejection or a mount-time stale_snapshot failure is non-continuable — re-dispatch deliberately, never auto-retry or spawn a speculative successor.", inputSchema: { type: "object", properties: { @@ -593,6 +603,12 @@ export const spawnAgentToolDefinition: ToolDefinition = { description: "Optional director id (e.g. from search_agents). Alternative to intent=.", }, + requires_tools: { + type: "array", + items: { type: "string" }, + description: + "Optional hard tool requirements (canonical names, e.g. run_shell). Verified pre-spawn against the worker's capability mount — the spawn fails closed with a reroute hint when a tool is missing, and the mount re-checks at run start. Rejections are non-continuable: re-dispatch deliberately, never auto-retry.", + }, }, required: ["description", "prompt"], }, @@ -624,7 +640,8 @@ export const waitAgentsToolDefinition: ToolDefinition = { `(interrupted, cancelled, incomplete-report, and similar). A "failed" entry with "continuable": true is a recoverable transient ` + `provider failure (retryable/timeout/overload) — terminal, not a timeout and not a stall: do not re-wait it, and you may spawn at most ` + `one successor with the same brief. "failed" without the marker (auth, quota, context-overflow, or other errors) is not continuable — ` + - `do not respawn it. awaiting_director is not terminal: re-wait while still pending re-delivers the same question. ` + + `do not respawn it. Capability preflight rejections and mount-time stale_snapshot failures report failed ` + + `without the marker for the same reason — re-dispatch deliberately instead of retrying. awaiting_director is not terminal: re-wait while still pending re-delivers the same question. ` + `Answer with send_input (soft). Do not call this in a tight zero-progress loop: a timeout means the targets are still ` + `queued, running, or awaiting a director answer, not "try again right away" — do other work, reply to the operator, or change the brief. Calling again with the ` + `same targets is a real timed wait, not a spin, but wastes turns if nothing has changed. ` + @@ -709,6 +726,45 @@ function fleetJson(value: unknown): string { return JSON.stringify(value, null, 2); } +/** + * Tier gate for stamped requires_tools (CL-9476). run.ts mounts submit_result + * and ask_director on Tier 3 leaves only, and fleet verbs on orchestrator + * tiers only — a requirement the dispatch tier can never mount rejects here + * as missing_tool (pre-spawn) instead of surviving to the mount-time stale + * echo. Returns the rejection, or undefined when the tier mounts everything + * required. + */ +export function tierGateRequiresTools( + canonical: readonly string[], + tier: SubagentTier, +): CapabilityUnavailable | undefined { + for (const engine of canonical) { + if ( + (engine === "submit_result" || engine === "ask_director") && + tier !== "leaf" + ) { + return { + code: "missing_tool", + tool: engine, + alternatives: leafTierAlternatives(), + detail: `"${engine}" mounts on Tier 3 leaf workers only, never on ${tier} directors`, + }; + } + try { + assertTierMayMountFleetVerb(tier, engine); + } catch (error) { + if (!(error instanceof FleetAuthorityError)) throw error; + return { + code: "missing_tool", + tool: engine, + alternatives: rerouteAlternatives(engine), + detail: error.message.split(". ")[0] ?? error.message, + }; + } + } + return undefined; +} + interface ResolvedAgentDispatch { directorId: string; agentLabel: string; @@ -960,6 +1016,7 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { success_criteria: rawSuccessCriteria, do_not: rawDoNot, report_focus: rawReportFocus, + requires_tools: rawRequiresTools, } = parsed; const description = rawDesc.trim(); const prompt = rawPrompt.trim(); @@ -1065,6 +1122,56 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { ); } + // CL-9476: fail closed before any session, telemetry, or worktree exists. + // A rejection returns here — no auto re-dispatch, no successor, no retry. + // The tier below is the single derivation shared by the tier gate and + // the run mount: run.ts mounts the leaf reporting channel exactly when + // tier is "leaf", so gating on any other value would let a requirement + // pass here and die as a stale snapshot at mount (or vice versa). + const dispatchTier: SubagentTier = resolved.orchestrator + ? (resolved.orchestratorTier ?? resolved.pkg?.tier ?? "orchestrator") + : "leaf"; + let requiresTools: readonly string[] | undefined; + const rawEntries = rawRequiresTools ?? []; + const blankEntry = rawEntries.find((t) => t.trim().length === 0); + if (blankEntry !== undefined) { + return fleetResult( + call.id, + `Error: spawn_agent requires_tools entries must be non-empty tool names — got a blank entry. Correct requires_tools to canonical tool names and re-dispatch.`, + ); + } + const requiresToolsRaw = rawEntries.map((t) => t.trim()); + if (requiresToolsRaw.length > 0) { + const preflight = preflightCapabilities({ + required: requiresToolsRaw, + ...(resolved.capabilities !== undefined + ? { resolvedFilter: resolved.capabilities } + : {}), + knownEngines: DEFAULT_KNOWN_ENGINES, + agentLabel: resolved.agentLabel, + }); + if (!preflight.ok) { + return fleetResult( + call.id, + formatCapabilityUnavailable( + preflight.unavailable, + resolved.agentLabel, + ), + ); + } + requiresTools = preflight.canonical; + // Tier-gated mounts (fleet verbs, the Tier 3 leaf reporting channel) + // reject here as missing_tool when the dispatch tier can never mount + // them — pre-spawn, never surviving to the mount-time stale echo. + const gated = tierGateRequiresTools(preflight.canonical, dispatchTier); + if (gated !== undefined) { + return fleetResult( + call.id, + formatCapabilityUnavailable(gated, resolved.agentLabel), + ); + } + } + const orchestrator = resolved.orchestrator; const nestedSpawnAllowlist = resolved.nestedSpawnAllowlist; const effort = resolveEffortForRole({ @@ -1109,6 +1216,7 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { // governs it from here on. retained: true, provider: provider.providerName, + ...(requiresTools !== undefined ? { requiresTools } : {}), ...(deps.parentSessionId !== undefined ? { parentSessionId: deps.parentSessionId } : {}), @@ -1415,6 +1523,7 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { ...(resolved.capabilities !== undefined ? { capabilities: resolved.capabilities } : {}), + ...(requiresTools !== undefined ? { requiresTools } : {}), ...(allowedSkillNames !== undefined ? { allowedSkillNames } : {}), ...(resolved.pkg?.attachedSkills !== undefined && resolved.pkg.attachedSkills.length > 0 @@ -1439,11 +1548,14 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { ...(deps.deadlineMs !== undefined ? { deadlineMs: deps.deadlineMs } : {}), - ...(resolved.pkg !== undefined - ? { tier: resolved.pkg.tier } - : orchestrator - ? {} - : { tier: "leaf" }), + // Single tier derivation (dispatchTier above): the tier gate and + // the mount agree by construction, so a gated requirement can + // never die as a stale snapshot at mount, or vice versa. An + // orchestrator dispatch without a resolved package carries no + // tier — run.ts fails closed on fleet mounts from there. + ...(resolved.pkg !== undefined || !orchestrator + ? { tier: dispatchTier } + : {}), ...(resolved.pkg?.reportContract?.outputType !== undefined ? { reportType: resolved.pkg.reportContract.outputType } : {}), diff --git a/src/subagent/capability-preflight.test.ts b/src/subagent/capability-preflight.test.ts new file mode 100644 index 000000000..864a51968 --- /dev/null +++ b/src/subagent/capability-preflight.test.ts @@ -0,0 +1,273 @@ +import { describe, expect, test } from "bun:test"; + +import { + checkMountedRequiresTools, + DEFAULT_KNOWN_ENGINES, + formatCapabilityUnavailable, + leafTierAlternatives, + preflightCapabilities, + rerouteAlternatives, + type CapabilityUnavailable, +} from "./capability-preflight.js"; +import type { CapabilityFilter } from "../agent/profile-types.js"; + +const ALLOW_SHELL: CapabilityFilter = { + mode: "allow", + tools: ["run_shell", "read_file"], +}; +const ALLOW_READ_ONLY: CapabilityFilter = { + mode: "allow", + tools: ["read_file"], +}; +const DENY_SHELL: CapabilityFilter = { mode: "exclude", tools: ["run_shell"] }; + +function preflight( + required: readonly string[], + resolvedFilter?: CapabilityFilter, + knownEngines: readonly string[] = DEFAULT_KNOWN_ENGINES, +) { + return preflightCapabilities({ + required, + ...(resolvedFilter !== undefined ? { resolvedFilter } : {}), + knownEngines, + agentLabel: "test-worker", + }); +} + +describe("preflightCapabilities", () => { + test("allow filter mounting the tool passes and returns canonical names", () => { + const result = preflight(["run_shell"], ALLOW_SHELL); + expect(result).toEqual({ ok: true, canonical: ["run_shell"] }); + }); + + test("exclude filter omitting the tool passes", () => { + const result = preflight(["read_file"], DENY_SHELL); + expect(result).toEqual({ ok: true, canonical: ["read_file"] }); + }); + + test("undefined filter is a full mount — every known tool passes", () => { + const result = preflight(["run_shell", "web_fetch", "manage_tasks"]); + expect(result).toEqual({ + ok: true, + canonical: ["run_shell", "web_fetch", "manage_tasks"], + }); + }); + + test("aliases collapse on both sides (shell requirement vs run_shell filter)", () => { + const result = preflight(["shell"], ALLOW_SHELL); + expect(result).toEqual({ ok: true, canonical: ["run_shell"] }); + }); + + test("allowlist omission rejects missing_tool with reroute alternatives", () => { + const result = preflight(["run_shell"], ALLOW_READ_ONLY); + 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("run_shell"); + expect(result.unavailable.alternatives?.length ?? 0).toBeGreaterThan(0); + }); + + test("denylist hit rejects permission_static", () => { + const result = preflight(["run_shell"], DENY_SHELL); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected rejection"); + expect(result.unavailable.code).toBe("permission_static"); + expect(result.unavailable.tool).toBe("run_shell"); + }); + + test("narrowed knownEngines reject missing_binary (test-only seam: production always passes the full catalog)", () => { + const result = preflight(["run_shell"], ALLOW_SHELL, []); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("expected rejection"); + expect(result.unavailable.code).toBe("missing_binary"); + }); + + test("post-filter mounted engines bypass a narrow allowlist", () => { + const result = preflight(["manage_tasks"], ALLOW_READ_ONLY); + expect(result).toEqual({ ok: true, canonical: ["manage_tasks"] }); + }); + + test("unknown tool rejects with a nearest-name suggestion", () => { + const result = preflight(["run_shel"], ALLOW_SHELL); + 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("run_shel"); + expect(result.unavailable.suggestion).toBe("run_shell"); + }); + + test("reroute alternatives collapse aliases (shell and run_shell agree)", () => { + expect(rerouteAlternatives("shell")).toEqual( + rerouteAlternatives("run_shell"), + ); + expect(rerouteAlternatives("run_shell").length).toBeGreaterThan(0); + }); + + test("reroute alternatives sort before the cap of 3 (read_file has 19 mounting directors)", () => { + const alternatives = rerouteAlternatives("read_file"); + expect(alternatives).toEqual(["bruckheimer", "builder", "counsel"]); + expect(alternatives).toEqual([...alternatives].sort()); + }); + + test("leaf reporting channel passes preflight under a narrow allowlist omitting post-filter mounts", () => { + for (const engine of ["submit_result", "ask_director"]) { + expect(preflight([engine], ALLOW_READ_ONLY)).toEqual({ + ok: true, + canonical: [engine], + }); + } + }); + + test("leafTierAlternatives names sorted Tier 3 leaf directors, never skywalker", () => { + const alternatives = leafTierAlternatives(); + expect(alternatives.length).toBeGreaterThan(0); + expect(alternatives.length).toBeLessThanOrEqual(3); + expect(alternatives).toEqual([...alternatives].sort()); + expect(alternatives).not.toContain("skywalker"); + }); +}); + +describe("formatCapabilityUnavailable", () => { + const cases: { + code: CapabilityUnavailable["code"]; + unavailable: CapabilityUnavailable; + }[] = [ + { + code: "missing_tool", + unavailable: { + code: "missing_tool", + tool: "run_shell", + alternatives: ["builder"], + }, + }, + { + code: "permission_static", + unavailable: { + code: "permission_static", + tool: "run_shell", + alternatives: ["builder"], + }, + }, + { + code: "missing_binary", + unavailable: { code: "missing_binary", tool: "run_shell" }, + }, + { + code: "stale_snapshot", + unavailable: { code: "stale_snapshot", tool: "run_shell" }, + }, + { + code: "unknown_tool", + unavailable: { + code: "unknown_tool", + tool: "run_shel", + suggestion: "run_shell", + }, + }, + ]; + + for (const { code, unavailable } of cases) { + test(`${code} message starts with Error, names the tool, and ends in one action sentence`, () => { + const message = formatCapabilityUnavailable(unavailable, "test-worker"); + expect(message.startsWith("Error:")).toBe(true); + expect(message).toContain(unavailable.tool); + expect(message.endsWith(".")).toBe(true); + const sentences = message + .split(/(?<=\.) /) + .map((s) => s.trim()) + .filter((s) => s.length > 0); + // Fact sentence(s) plus exactly one trailing next-action sentence. + expect(sentences.length).toBeGreaterThanOrEqual(2); + const action = sentences[sentences.length - 1] ?? ""; + expect( + /re-dispatch|drop the requirement|correct requires_tools/i.test(action), + ).toBe(true); + }); + } + + test("stale_snapshot message carries setup_error and non-continuable, never a retry affordance", () => { + const message = formatCapabilityUnavailable( + { code: "stale_snapshot", tool: "run_shell" }, + "test-worker", + ); + expect(message).toContain("stale_snapshot"); + expect(message).toContain("setup_error"); + expect(message).toContain("non-continuable"); + expect(message.toLowerCase()).toContain("instead of retrying"); + expect(message.toLowerCase()).not.toContain("auto-retry"); + expect(message).not.toContain("successor"); + expect(message).not.toContain('continuable": true'); + }); + + test("missing_tool message names the reroute target", () => { + const alternatives = rerouteAlternatives("run_shell"); + const message = formatCapabilityUnavailable( + { code: "missing_tool", tool: "run_shell", alternatives }, + "test-worker", + ); + expect(message).toContain(alternatives[0] ?? ""); + }); + + test("missing_tool detail renders a tier fact between fact and action", () => { + const message = formatCapabilityUnavailable( + { + code: "missing_tool", + tool: "spawn_agent", + alternatives: ["skywalker"], + detail: 'Tier 3 leaf directors cannot mount fleet verb "spawn_agent"', + }, + "test-worker", + ); + expect(message.startsWith("Error:")).toBe(true); + expect(message).toContain( + 'Tier 3 leaf directors cannot mount fleet verb "spawn_agent".', + ); + expect(message.indexOf("Tier 3 leaf")).toBeGreaterThan( + message.indexOf("allowlist omits it"), + ); + expect(message.indexOf("Tier 3 leaf")).toBeLessThan( + message.indexOf("Re-dispatch"), + ); + }); + + test("unknown_tool message carries the did-you-mean hint", () => { + const message = formatCapabilityUnavailable( + { code: "unknown_tool", tool: "run_shel", suggestion: "run_shell" }, + "test-worker", + ); + expect(message).toContain('Did you mean "run_shell"?'); + }); + + test("stale_snapshot message names every missing tool, not just the first", () => { + const message = formatCapabilityUnavailable( + { + code: "stale_snapshot", + tool: "run_shell", + tools: ["run_shell", "write_file"], + }, + "test-worker", + ); + expect(message).toContain("stale_snapshot"); + expect(message).toContain("run_shell"); + expect(message).toContain("write_file"); + }); +}); + +describe("checkMountedRequiresTools", () => { + test("stamped tool missing from the live mount reports stale", () => { + expect(checkMountedRequiresTools(["run_shell"], ["read_file"])).toEqual({ + ok: false, + missing: ["run_shell"], + }); + }); + + test("mounted tools pass, collapsing aliases", () => { + expect(checkMountedRequiresTools(["shell"], ["run_shell"])).toEqual({ + ok: true, + }); + }); + + test("empty requirements always pass", () => { + expect(checkMountedRequiresTools([], [])).toEqual({ ok: true }); + }); +}); diff --git a/src/subagent/capability-preflight.ts b/src/subagent/capability-preflight.ts new file mode 100644 index 000000000..23f1c150e --- /dev/null +++ b/src/subagent/capability-preflight.ts @@ -0,0 +1,360 @@ +/** + * Pre-spawn capability preflight for `spawn_agent(requires_tools=...)` (CL-9476). + * + * A `requires_tools` entry is a hard requirement: the named tool must be + * mounted on the worker or the dispatch is rejected before any session, + * telemetry, or worktree exists. Names are canonical engine ids on both sides + * (wire/hidden aliases collapse via canonicalToolName), so `shell` and + * `run_shell` are the same requirement. Fail-closed throughout: unknown names + * and allowlist/denylist misses all reject. + * + * `missing_binary` never fires in production: dispatch always passes the full + * catalog as `knownEngines`, so every catalogued engine verifies. Narrowed + * `knownEngines` sets are a test-only seam for simulating an incomplete + * runtime — the branch exists so tests can prove the fail-closed shape, not + * because production probes binaries per engine (most engines are in-process). + * + * The `stale_snapshot` code is never emitted by preflightCapabilities — it is + * the mount-time echo in run.ts (a tool stamped at dispatch is missing from + * the live mount). It lives in this union so both paths share one formatter. + */ + +import { canonicalToolName } from "../agent/canonical-tool-name.js"; +import { + DIRECTOR_REGISTRY, + packageToCapabilities, +} from "../agent/directors/registry.js"; +import type { CapabilityFilter } from "../agent/profile-types.js"; + +export type CapabilityUnavailableCode = + | "missing_tool" + | "permission_static" + | "missing_binary" + | "stale_snapshot" + | "unknown_tool"; + +export interface PreflightCapabilitiesInput { + /** Raw requires_tools entries (aliases welcome — canonicalized here). */ + required: readonly string[]; + /** Resolved dispatch filter; undefined means full mount (everything passes). */ + resolvedFilter?: CapabilityFilter | undefined; + /** + * Canonical engine ids verifiable in this dispatch. Production always + * passes the full catalog (DEFAULT_KNOWN_ENGINES); narrowed sets are a + * test-only seam for simulating an incomplete runtime. + */ + knownEngines: readonly string[]; + /** Worker label for messages (director id or profile id). */ + agentLabel: string; +} + +export interface CapabilityUnavailable { + code: CapabilityUnavailableCode; + /** Canonical tool id (or the raw entry for unknown). */ + tool: string; + /** Every missing tool, for the multi-missing stale_snapshot echo. */ + tools?: readonly string[]; + /** Nearest known name, for the unknown_tool typo guard. */ + suggestion?: string; + /** Spawnable directors that mount the tool, for reroute hints. */ + alternatives?: readonly string[]; + /** + * Extra fact sentence for missing_tool, naming a mount restriction the + * allowlist framing cannot see — e.g. a tier gate that withholds fleet + * verbs from leaves. Rendered between the fact and the action sentence. + */ + detail?: string; +} + +export type CapabilityPreflightResult = + | { ok: true; canonical: string[] } + | { ok: false; unavailable: CapabilityUnavailable }; + +/** + * Canonical engine ids the fleet knows how to mount: the director tool + * surfaces plus the worker/plumbing verbs mounted outside capability + * filters. Dispatch passes this as `knownEngines`; tests inject narrower + * sets to simulate an absent binary. + */ +export const KNOWN_CAPABILITY_ENGINES: readonly string[] = [ + "read_file", + "write_file", + "edit_file", + "delete_file", + "run_shell", + "grep", + "search_files", + "list_dir", + "lsp", + "shell_collect", + "web_fetch", + "web_search", + "skill_search", + "use_skill", + "manage_tasks", + "spawn_agent", + "list_agents", + "close_agent", + "resume_agent", + "interrupt_agent", + "send_input", + "read_agent_trace", + "search_agents", + "ask_director", + "submit_result", +]; + +/** + * Dispatch-time `knownEngines`: the full catalog. Dispatch always passes this, + * so every catalogued engine verifies and `missing_binary` never fires in + * production. Narrowed `knownEngines` sets are a test-only seam for + * simulating an incomplete runtime — production probes no binaries per engine + * (most engines are in-process). + */ +export const DEFAULT_KNOWN_ENGINES: readonly string[] = + KNOWN_CAPABILITY_ENGINES; + +/** + * Engines mounted outside the capability filter (run.ts appends manage_tasks + * after filtering, and mounts the Tier 3 leaf reporting channel + * submit_result/ask_director whenever tier is "leaf"), so a requires_tools + * entry for them passes preflight even when the dispatch filter is a narrow + * allowlist. The update_plan alias canonicalizes here, so it rides the same + * exemption — and the mount-time echo in run.ts runs after ALL appends, so + * the stamped entry always matches the live mount. Fail-closed: the tier gate + * in agent-fleet.ts still rejects submit_result/ask_director on non-leaf + * tiers after this exemption, so the bypass never mounts them where run.ts + * would not. + */ +const POST_FILTER_MOUNTED_ENGINES: readonly string[] = [ + "manage_tasks", + "submit_result", + "ask_director", +]; + +/** Alias spellings accepted in requires_tools, for typo suggestions. */ +const SUGGESTION_CANDIDATES: readonly string[] = [ + ...KNOWN_CAPABILITY_ENGINES, + "read", + "write", + "edit", + "delete", + "bash", + "glob", + "shell", + "update_plan", +]; + +function levenshtein(a: string, b: string): number { + const prev = Array.from({ length: b.length + 1 }, (_, j) => j); + for (let i = 1; i <= a.length; i++) { + let diag = prev[0] ?? 0; + prev[0] = i; + for (let j = 1; j <= b.length; j++) { + const cost = a[i - 1] === b[j - 1] ? 0 : 1; + const above = (prev[j] ?? 0) + 1; + const left = (prev[j - 1] ?? 0) + 1; + const next = Math.min(above, left, diag + cost); + diag = prev[j] ?? 0; + prev[j] = next; + } + } + return prev[b.length] ?? Number.MAX_SAFE_INTEGER; +} + +function nearestToolName(raw: string): string | undefined { + const lower = raw.toLowerCase(); + let best: string | undefined; + let bestDistance = Number.MAX_SAFE_INTEGER; + for (const candidate of SUGGESTION_CANDIDATES) { + const distance = levenshtein(lower, candidate.toLowerCase()); + if (distance < bestDistance) { + bestDistance = distance; + best = candidate; + } + } + return best !== undefined && bestDistance <= 2 ? best : undefined; +} + +/** + * Spawnable directors (closed set minus primary skywalker) whose mounted + * tool set includes `canonical` — derived from packageToCapabilities over + * DIRECTOR_REGISTRY, so the hint tracks the envelopes. Sorted before the cap + * so the three named are the first alphabetically, not the first in registry + * insertion order. + */ +export function rerouteAlternatives(canonical: string): readonly string[] { + const want = canonicalToolName(canonical); + const out: string[] = []; + for (const pkg of Object.values(DIRECTOR_REGISTRY)) { + if (pkg.id === "skywalker") continue; + const capabilities = packageToCapabilities(pkg); + if (capabilities === undefined) { + out.push(pkg.id); + } else if (capabilities.mode === "allow") { + if (capabilities.tools.some((t) => canonicalToolName(t) === want)) { + out.push(pkg.id); + } + } else if (!capabilities.tools.some((t) => canonicalToolName(t) === want)) { + out.push(pkg.id); + } + } + return out.sort().slice(0, 3); +} + +/** + * Tier-3 leaf directors (closed set, skywalker excluded) for the tier-gate + * hint when requires_tools names the leaf reporting channel on a non-leaf + * tier. submit_result/ask_director mount post-filter, so no envelope mentions + * them and rerouteAlternatives would report none — this names the directors + * that actually mount them instead of the allowlist-miss fallback. + */ +export function leafTierAlternatives(): readonly string[] { + return Object.values(DIRECTOR_REGISTRY) + .filter((pkg) => pkg.id !== "skywalker" && pkg.tier === "leaf") + .map((pkg) => pkg.id) + .sort() + .slice(0, 3); +} + +export function preflightCapabilities( + input: PreflightCapabilitiesInput, +): CapabilityPreflightResult { + const { resolvedFilter, knownEngines } = input; + const known = new Set(knownEngines.map((name) => canonicalToolName(name))); + const allow = + resolvedFilter?.mode === "allow" + ? new Set(resolvedFilter.tools.map((name) => canonicalToolName(name))) + : undefined; + const deny = + resolvedFilter?.mode === "exclude" + ? new Set(resolvedFilter.tools.map((name) => canonicalToolName(name))) + : undefined; + const postFilter = new Set(POST_FILTER_MOUNTED_ENGINES); + const catalog = new Set(KNOWN_CAPABILITY_ENGINES); + const seen = new Set(); + const canonical: string[] = []; + for (const raw of input.required) { + const trimmed = raw.trim(); + if (trimmed.length === 0) { + return { ok: false, unavailable: { code: "unknown_tool", tool: raw } }; + } + const engine = canonicalToolName(trimmed); + if (!catalog.has(engine)) { + const suggestion = nearestToolName(trimmed); + return { + ok: false, + unavailable: { + code: "unknown_tool", + tool: trimmed, + ...(suggestion !== undefined ? { suggestion } : {}), + }, + }; + } + if (!known.has(engine)) { + return { + ok: false, + unavailable: { code: "missing_binary", tool: engine }, + }; + } + if (!postFilter.has(engine)) { + if (allow !== undefined && !allow.has(engine)) { + return { + ok: false, + unavailable: { + code: "missing_tool", + tool: engine, + alternatives: rerouteAlternatives(engine), + }, + }; + } + if (deny !== undefined && deny.has(engine)) { + return { + ok: false, + unavailable: { + code: "permission_static", + tool: engine, + alternatives: rerouteAlternatives(engine), + }, + }; + } + } + if (!seen.has(engine)) { + seen.add(engine); + canonical.push(engine); + } + } + return { ok: true, canonical }; +} + +/** + * Mount-time echo helper (run.ts): the stamped dispatch requirements against + * the live mount. A missing stamped tool means the capability snapshot went + * stale between dispatch and mount — never a dispatch-time outcome. + */ +export function checkMountedRequiresTools( + stampedRequires: readonly string[], + mountedToolNames: readonly string[], +): { ok: true } | { ok: false; missing: string[] } { + const mounted = new Set( + mountedToolNames.map((name) => canonicalToolName(name)), + ); + const missing = stampedRequires.filter( + (name) => !mounted.has(canonicalToolName(name)), + ); + return missing.length === 0 ? { ok: true } : { ok: false, missing }; +} + +/** + * One user message per code. Every message ends in exactly one next-action + * sentence — the caller re-dispatches deliberately; there is no auto + * re-dispatch, successor, or retry path. + */ +export function formatCapabilityUnavailable( + unavailable: CapabilityUnavailable, + agentLabel: string, +): string { + const alternatives = + unavailable.alternatives !== undefined && + unavailable.alternatives.length > 0 + ? ` Re-dispatch to one of (${unavailable.alternatives.join(", ")}) or drop the requirement.` + : ` No spawnable director mounts "${unavailable.tool}" — drop the requirement or add a profile that mounts it.`; + switch (unavailable.code) { + case "missing_tool": { + const detail = + unavailable.detail !== undefined ? ` ${unavailable.detail}.` : ""; + return ( + `Error: worker "${agentLabel}" requires tool "${unavailable.tool}" but its capability allowlist omits it.` + + detail + + alternatives + ); + } + case "permission_static": + return ( + `Error: worker "${agentLabel}" requires tool "${unavailable.tool}" but its capability denylist blocks it.` + + alternatives + ); + case "missing_binary": + return ( + `Error: worker "${agentLabel}" requires tool "${unavailable.tool}" but this dispatch cannot verify its runtime engine (not in the dispatch's known-engine set). ` + + `Re-dispatch without requires_tools=["${unavailable.tool}"] or drop the requirement.` + ); + case "stale_snapshot": { + const missing = unavailable.tools ?? [unavailable.tool]; + return ( + `Error: stale_snapshot setup_error (non-continuable) — worker "${agentLabel}" was dispatched requiring "${missing.join('", "')}" but the live mount no longer provides ${missing.length === 1 ? "it" : "them"}. ` + + `Re-dispatch the worker deliberately with a fresh brief instead of retrying in place.` + ); + } + case "unknown_tool": { + const hint = + unavailable.suggestion !== undefined + ? ` Did you mean "${unavailable.suggestion}"?` + : ""; + return ( + `Error: worker "${agentLabel}" requires unknown tool "${unavailable.tool}".${hint} ` + + `Correct requires_tools to a canonical tool name and re-dispatch.` + ); + } + } +} diff --git a/src/subagent/run-requires-tools.test.ts b/src/subagent/run-requires-tools.test.ts new file mode 100644 index 000000000..d7311da75 --- /dev/null +++ b/src/subagent/run-requires-tools.test.ts @@ -0,0 +1,189 @@ +/** + * runSubAgent mount echo for requires_tools (CL-9476). + * + * Dispatch verifies requires_tools pre-spawn, but the filter or mount may + * shift between dispatch and mount. A stamped tool missing after + * applyCapabilityFilter is a stale snapshot: a setup_error that never + * retries (non-continuable). These tests drive runSubAgent (the real mount + * point) end to end with failing inference — mount decisions run before + * the send, so the echo fires first. + */ + +import { describe, expect, test } from "bun:test"; +import { tmpdir } from "node:os"; +import { mkdtemp } from "node:fs/promises"; +import { join } from "node:path"; + +import { createPermissionGate } from "../permission/gate.js"; +import { runSubAgent } from "./run.js"; +import type { RunSubAgentParams } from "./types.js"; + +const testPermissionGate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: true, + reactorGated: false, +}); + +async function tmpCwd(): Promise { + return mkdtemp(join(tmpdir(), "cl9476-run-requires-tools-")); +} + +function baseParams(cwd: string, baseURL: string): RunSubAgentParams { + return { + cwd, + workdirBase: join(cwd, ".ctx"), + permissionGate: testPermissionGate, + provider: { providerName: "test", baseURL, model: "test-model" }, + description: "requires-tools probe", + prompt: "no-op", + }; +} + +// A local server answering 401 fails the inference send as a +// credential failure, which is never retried — the cycle costs one local +// round trip, and assertions stay timing-independent. +async function withFailingInference( + run: (baseURL: string) => Promise, +): Promise { + const server = Bun.serve({ + port: 0, + fetch: () => + new Response( + JSON.stringify({ error: { message: "requires-tools probe" } }), + { + status: 401, + headers: { "content-type": "application/json" }, + }, + ), + }); + try { + return await run(server.url.origin); + } finally { + server.stop(true); + } +} + +describe("runSubAgent requires_tools mount echo", () => { + test("stamped tool dropped by the filter throws stale_snapshot setup_error", async () => { + const cwd = await tmpCwd(); + const error = await withFailingInference(async (baseURL) => { + try { + await runSubAgent({ + ...baseParams(cwd, baseURL), + capabilities: { mode: "allow", tools: ["read_file"] }, + requiresTools: ["run_shell"], + }); + } catch (err) { + return err; + } + throw new Error("runSubAgent did not throw"); + }); + + expect(error).toBeInstanceOf(Error); + const message = (error as Error).message; + expect(message).toContain("stale_snapshot"); + expect(message).toContain("run_shell"); + expect(message).toContain("setup_error"); + expect(message).toContain("non-continuable"); + expect(message).toContain("Re-dispatch"); + expect(message).not.toContain('continuable": true'); + }, 15_000); + + test("two stamped tools dropped by the filter are both named in the stale_snapshot error", async () => { + const cwd = await tmpCwd(); + const error = await withFailingInference(async (baseURL) => { + try { + await runSubAgent({ + ...baseParams(cwd, baseURL), + capabilities: { mode: "allow", tools: ["read_file"] }, + requiresTools: ["run_shell", "write_file"], + }); + } catch (err) { + return err; + } + throw new Error("runSubAgent did not throw"); + }); + + expect(error).toBeInstanceOf(Error); + const message = (error as Error).message; + expect(message).toContain("stale_snapshot"); + expect(message).toContain("run_shell"); + expect(message).toContain("write_file"); + }, 15_000); + + test("stamped tool present in the mount passes through to inference", async () => { + const cwd = await tmpCwd(); + const error = await withFailingInference(async (baseURL) => { + try { + await runSubAgent({ + ...baseParams(cwd, baseURL), + capabilities: { mode: "allow", tools: ["read_file"] }, + requiresTools: ["read_file"], + }); + } catch (err) { + return err; + } + throw new Error("runSubAgent did not throw"); + }); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).not.toContain("stale_snapshot"); + }, 15_000); + + test("stamped manage_tasks survives the echo under a narrow allowlist", async () => { + const cwd = await tmpCwd(); + const error = await withFailingInference(async (baseURL) => { + try { + await runSubAgent({ + ...baseParams(cwd, baseURL), + capabilities: { mode: "allow", tools: ["read_file"] }, + requiresTools: ["manage_tasks"], + }); + } catch (err) { + return err; + } + throw new Error("runSubAgent did not throw"); + }); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).not.toContain("stale_snapshot"); + }, 15_000); + + test("stamped update_plan alias survives the echo under a narrow allowlist", async () => { + const cwd = await tmpCwd(); + const error = await withFailingInference(async (baseURL) => { + try { + await runSubAgent({ + ...baseParams(cwd, baseURL), + capabilities: { mode: "allow", tools: ["read_file"] }, + requiresTools: ["update_plan"], + }); + } catch (err) { + return err; + } + throw new Error("runSubAgent did not throw"); + }); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).not.toContain("stale_snapshot"); + }, 15_000); + + test("absent requiresTools leaves the mount path unchanged", async () => { + const cwd = await tmpCwd(); + const error = await withFailingInference(async (baseURL) => { + try { + await runSubAgent({ + ...baseParams(cwd, baseURL), + capabilities: { mode: "allow", tools: ["read_file"] }, + }); + } catch (err) { + return err; + } + throw new Error("runSubAgent did not throw"); + }); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).not.toContain("stale_snapshot"); + }, 15_000); +}); diff --git a/src/subagent/run.ts b/src/subagent/run.ts index e35b395b5..b1e3e3bad 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -67,6 +67,10 @@ import { } from "./intervention-log.js"; import { normalizeToolDefinitionsForProvider } from "../agent/tool-schema-normalize.js"; import { canonicalToolName } from "../agent/canonical-tool-name.js"; +import { + checkMountedRequiresTools, + formatCapabilityUnavailable, +} from "./capability-preflight.js"; import { advertisedToolName, projectToolDefinitions, @@ -995,6 +999,32 @@ async function runSubAgentInner( ]; } + // CL-9476 mount echo: dispatch verified requires_tools pre-spawn, but the + // filter or mount may have shifted since — a stamped tool missing here is + // a stale snapshot, a setup_error that never retries (non-continuable). + // Dispatch-side rejection is the normal path. This check runs after ALL + // mounts (manage_tasks, leaf submit_result/ask_director, fleet verbs), so + // a dispatch that passed preflight against the same mount never throws. + if (params.requiresTools !== undefined && params.requiresTools.length > 0) { + const mountedNames = tools.map((tool) => tool.definition.name); + const mountedCheck = checkMountedRequiresTools( + params.requiresTools, + mountedNames, + ); + if (!mountedCheck.ok) { + throw new Error( + formatCapabilityUnavailable( + { + code: "stale_snapshot", + tool: mountedCheck.missing[0] ?? "unknown", + tools: mountedCheck.missing, + }, + params.directorId ?? params.description, + ), + ); + } + } + tools = wrapAgentToolsWithResultTruncation(tools, { getBlobWriter: () => childBlobWriter, getContextDir: () => childContextDir, diff --git a/src/subagent/session-store.ts b/src/subagent/session-store.ts index 0542ec481..346df460d 100644 --- a/src/subagent/session-store.ts +++ b/src/subagent/session-store.ts @@ -150,6 +150,12 @@ export interface SubAgentSession { // Once close_agent runs, this flips back to false and the session is a // normal finished record subject to `maxCompleted` like any other. retained?: boolean; + /** + * Canonical tool names this worker hard-required at dispatch (CL-9476). + * Verified pre-spawn; the mount re-checks them and fails the run as a + * stale snapshot when one went missing in between. + */ + requiresTools?: readonly string[]; } export interface StartSessionInput { @@ -166,6 +172,8 @@ export interface StartSessionInput { retained?: boolean; /** Catalog provider id for followup admission. */ provider?: string; + /** Canonical tool names this worker hard-required at dispatch (CL-9476). */ + requiresTools?: readonly string[]; } export interface SubAgentSessionStoreOptions { @@ -1297,6 +1305,9 @@ export function createSubAgentSessionStore( ? { parentSessionId: input.parentSessionId } : {}), ...(input.provider !== undefined ? { provider: input.provider } : {}), + ...(input.requiresTools !== undefined + ? { requiresTools: [...input.requiresTools] } + : {}), }; sessions.set(id, session); bumpRevision(id); @@ -2317,6 +2328,9 @@ function cloneSession( ? { parentSessionId: session.parentSessionId } : {}), ...(session.provider !== undefined ? { provider: session.provider } : {}), + ...(session.requiresTools !== undefined + ? { requiresTools: [...session.requiresTools] } + : {}), }; } diff --git a/src/subagent/types.ts b/src/subagent/types.ts index bb17fd35a..25335340e 100644 --- a/src/subagent/types.ts +++ b/src/subagent/types.ts @@ -140,6 +140,13 @@ export type RunSubAgentParams = { onProgress?: (info: { description: string; toolName: string }) => void; onRunSettled?: (summary: Readonly) => void; capabilities?: CapabilityFilter; + /** + * Canonical tool names this worker hard-requires (CL-9476). Verified + * pre-spawn by the dispatcher; run.ts re-checks them against the mounted + * tools after the capability filter and fails the run as a stale snapshot + * when one went missing in between. + */ + requiresTools?: readonly string[]; /** * Skill allowlist for the worker's skill_search + use_skill mounts, * resolved by the caller (agent-fleet.ts) as the union of