diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 94fed4e16..12c4d45e3 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -205,21 +205,22 @@ The **`task`** tool **spawns a sub-agent** on a separate inference source (tier/ When profiles exist (local `.agents/agents/` and/or enabled **`kind: "agent"`** plugins, including **data-only** markdown plugins with no `index.ts`), the chat model also receives **`search_agents`** — a lexical index over profile id, description, and role text so the model can discover ids before calling `task(agent=...)`. Results include each match's full loaded system prompt / body so the parent can inspect plugin or Claude marketplace agents without `read_file` on paths outside the session cwd (path-escape blocks those roots by design; writes remain blocked). `task` and `search_agents` are core tools on the primary session. -Profiles with `orchestrator: true` may themselves call `task` (one hop only): nested dispatch installs `task` + `search_agents` with `allowOrchestrator: false` so the tree bottoms out. Unknown `agent` ids fail closed. +Profiles with `orchestrator: true` may themselves call `task` (one hop only): nested dispatch installs `task` with `allowOrchestrator: false` so the tree bottoms out. `search_agents` is Tier-1 Skywalker only — nested orchestrators keep task/spawn but do not discover the fleet. Unknown `agent` ids fail closed. #### Fleet authority tiers (`src/subagent/authority.ts`) (CL-6941) Every director package carries a required `tier: SubagentTier` field (`src/agent/directors/types.ts`) — data on the package, never a prompt instruction: -| Tier | Who | Fleet surface | -| ------------------------- | ----------------------------------------------- | ---------------------------------------------------------------------------------------------------------------- | -| 1 — `orchestrator` | skywalker (primary) | Full fleet control over the whole tree. | -| 2 — `nested-orchestrator` | greybeard, or any package with `spawn.maySpawn` | Same fleet surface, scoped to its own subtree: may manage only its own descendants, never a sibling or ancestor. | -| 3 — `leaf` | every other director | No fleet verbs at all. | +| Tier | Who | Fleet surface | +| ------------------------- | ----------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 — `orchestrator` | skywalker (primary) | Full fleet control over the whole tree. | +| 2 — `nested-orchestrator` | greybeard, or any package with `spawn.maySpawn` | Fleet surface without discovery: may manage only its own descendants via task/spawn, never siblings or ancestors; no `search_agents` / `list_agents`. | +| 3 — `leaf` | every other director | No fleet verbs at all. | Enforcement is runtime code at the existing tool-mount point, not prompt wording — this is the fix for four prior mechanisms (`writePaths`, `report.requiredSections`, a `--config` comment, the thrash matcher) that were documented-as-enforced while enforcing nothing: -- **Mount-time gate — live today, and fails closed.** `task-tool.ts` resolves the caller's tier at dispatch time — a closed director's `DirectorPackage.tier` — and forwards it as `RunSubAgentParams.orchestratorTier`. `runSubAgent` (`src/subagent/run.ts`) then calls `assertTierMayMountFleetVerb(tier, toolName)` (`src/subagent/authority.ts`) before installing `task` / `search_agents`, treating a **missing** `orchestratorTier` as `"leaf"` — deny, not skip. This is the case that matters most: a project-local or plugin `AgentProfile` with `orchestrator: true` is outside the closed director set and is **not** trusted with fleet verbs just because `orchestrator: true` is set — there is no profile-level opt-in today, so the mount always throws `FleetAuthorityError` for a profile-sourced orchestrator (CL-6942/CL-6944 can add one when a real caller needs it). `FLEET_VERBS` in `authority.ts` also names the not-yet-implemented verbs (`spawn_agent`, `wait_agents`, `list_agents`, `send_input`, `interrupt_agent`, `close_agent`, `resume_agent`, `read_agent_trace`, `followup_task`) so their future mount sites inherit the same gate. +- **Mount-time gate — live today, and fails closed.** `task-tool.ts` resolves the caller's tier at dispatch time — a closed director's `DirectorPackage.tier` — and forwards it as `RunSubAgentParams.orchestratorTier`. `runSubAgent` (`src/subagent/run.ts`) then calls `assertTierMayMountFleetVerb(tier, toolName)` (`src/subagent/authority.ts`) before installing fleet verbs, treating a **missing** `orchestratorTier` as `"leaf"` — deny, not skip. Discovery verbs (`search_agents`, and `list_agents` when mounted) additionally require Tier 1 (`tier === "orchestrator"`) — nested orchestrators keep `task`/spawn but do not mount discovery (CL-7051). This is the case that matters most: a project-local or plugin `AgentProfile` with `orchestrator: true` is outside the closed director set and is **not** trusted with fleet verbs just because `orchestrator: true` is set — there is no profile-level opt-in today, so the mount always throws `FleetAuthorityError` for a profile-sourced orchestrator (CL-6942/CL-6944 can add one when a real caller needs it). `FLEET_VERBS` in `authority.ts` also names the not-yet-implemented verbs (`spawn_agent`, `wait_agents`, `list_agents`, `send_input`, `interrupt_agent`, `close_agent`, `resume_agent`, `read_agent_trace`, `followup_task`) so their future mount sites inherit the same gate; `DISCOVERY_VERBS` further restricts `search_agents` / `list_agents` to Tier 1. + - **Subtree authority — a seam, not yet wired.** `assertCanTargetAgent(actor, targetId, nodes)` (`src/subagent/authority.ts`) implements the "root owns its tree; a child manages only its own descendants" rule (Tier 1 may target anyone, Tier 2 may target only its own descendants over the same `{id, parentSessionId}` shape `SubAgentSessionStore` already tracks, Tier 3 always fails closed) — but **it has no production call site yet**. No verb today lets one live agent address another (`task` only spawns), so this rule is exercised only by `authority.test.ts` and is not enforced at runtime in this PR. It exists so CL-6942 (split spawn from wait) and CL-6944 (`send_input` steering) — the first verbs that make an agent addressable by another — can call it from day one instead of each inventing its own check. Treat it as unenforced until one of those wires a call site. - `task()` is unaffected and remains the only spawn verb until the new verbs land beside it (deprecated-not-deleted per the CL-6940 epic). Its argument schema and wire contract are unchanged; the tier check only gates which packages may have it mounted at all. diff --git a/src/agent/directors/greybeard/package.test.ts b/src/agent/directors/greybeard/package.test.ts index f8b620d8b..f15510a1d 100644 --- a/src/agent/directors/greybeard/package.test.ts +++ b/src/agent/directors/greybeard/package.test.ts @@ -44,10 +44,10 @@ describe("greybeardPackage", () => { expect(greybeardPackage.systemPrompt).toMatch(/never spawn a parallel diagnostic fleet/i); }); - test("tools.allow is orchestrator surface without product writes", () => { + test("tools.allow is orchestrator surface without product writes or discovery", () => { const allow = greybeardPackage.tools?.allow ?? []; expect(allow).toContain("task"); - expect(allow).toContain("search_agents"); + expect(allow).not.toContain("search_agents"); expect(allow).not.toContain("write_file"); expect(allow).not.toContain("edit_file"); expect(allow).not.toContain("delete_file"); diff --git a/src/agent/directors/tool-sets.test.ts b/src/agent/directors/tool-sets.test.ts index 4deae1f3f..0542ce863 100644 --- a/src/agent/directors/tool-sets.test.ts +++ b/src/agent/directors/tool-sets.test.ts @@ -52,6 +52,11 @@ describe("SKYWALKER_TOOLS / ORCHESTRATOR_TOOLS", () => { expect(SKYWALKER_TOOLS).toContain("task"); expect(ORCHESTRATOR_TOOLS).toContain("task"); }); + + test("search_agents is Skywalker-only; nested orchestrator surface omits discovery", () => { + expect(SKYWALKER_TOOLS).toContain("search_agents"); + expect(ORCHESTRATOR_TOOLS as readonly string[]).not.toContain("search_agents"); + }); }); describe("BUILD_TOOLS", () => { diff --git a/src/agent/directors/tool-sets.ts b/src/agent/directors/tool-sets.ts index 9bf82dfbe..6c15bf843 100644 --- a/src/agent/directors/tool-sets.ts +++ b/src/agent/directors/tool-sets.ts @@ -57,12 +57,17 @@ export const REVIEW_TOOLS = [...READ_TOOLS] as const; /** Mechanical intern: shell-first, minimal surface. */ export const INTERN_TOOLS = ["run_shell", "read_file", "list_dir"] as const; -/** Nested orchestrator surface (greybeard / package filter): dispatch only. */ -export const ORCHESTRATOR_TOOLS = [...READ_TOOLS, "search_agents", "task"] as const; +/** + * Nested orchestrator surface (greybeard / package filter): dispatch only. + * search_agents is Tier-1 Skywalker only — nested directors keep task/spawn + * but must not discover the fleet (CL-7051). + */ +export const ORCHESTRATOR_TOOLS = [...READ_TOOLS, "task"] as const; -/** Skywalker primary: orchestrator surface plus product writes for DIY tiny work. */ +/** Skywalker primary: orchestrator surface plus discovery + product writes for DIY tiny work. */ export const SKYWALKER_TOOLS = [ ...ORCHESTRATOR_TOOLS, + "search_agents", "write_file", "edit_file", "delete_file", diff --git a/src/agent/prompts.ts b/src/agent/prompts.ts index f2e8e2053..a345d01b2 100644 --- a/src/agent/prompts.ts +++ b/src/agent/prompts.ts @@ -343,8 +343,9 @@ export function buildSubAgentAppendix(opts: { orchestrator?: boolean } = {}): st // rule only. const recursionRule = opts.orchestrator === true - ? '- You are an orchestrator: you MAY call `task` to spawn other sub-agents (e.g. task(agent="greybeard", prompt="...")). This is an explicit exception to the no-recursion rule that applies to workers — use it to delegate specialist work, then synthesize their reports into your own. Prefer search_agents before naming a specialist. `task` spawns an agent; it is not a checklist item (use manage_tasks for your own checklist).' + ? '- You are an orchestrator: you MAY call `task` to spawn other sub-agents (e.g. task(agent="explore", prompt="...")). This is an explicit exception to the no-recursion rule that applies to workers — use it to delegate specialist work, then synthesize their reports into your own. Stay on your spawn allowlist. `task` spawns an agent; it is not a checklist item (use manage_tasks for your own checklist).' : `- Only the primary ${PRODUCT_NAME} session (or an orchestrator profile) may call \`task\` to spawn sub-agents. You are a worker: return a concrete report to the caller instead of spawning further agents. Use manage_tasks for your own work checklist if the job is multi-step.`; + return [ `## ${PRODUCT_NAME} notes`, "", diff --git a/src/agent/tool-search.test.ts b/src/agent/tool-search.test.ts index 3368ed6c3..e3ac2307d 100644 --- a/src/agent/tool-search.test.ts +++ b/src/agent/tool-search.test.ts @@ -83,6 +83,16 @@ describe("createToolIndex", () => { ); }); + test("search_agents stays a primary CORE advertisement (Skywalker), not a leaf surface", () => { + // Primary session mode always advertises discovery; leaf/nested directors + // never receive CORE — their envelopes omit search_agents, and runSubAgent + // mounts it only for Tier 1 (see tool-sets.test.ts / authority.test.ts). + expect(CORE_TOOL_NAMES).toContain("search_agents"); + expect(advertisedToolNamesForSessionMode("orchestrator", FULL_AVAILABILITY)).toContain( + "search_agents", + ); + }); + test("manage_tasks is advertised regardless of availability", () => { expect(coreToolNamesForSessionMode("orchestrator", NO_AVAILABILITY)).toContain("manage_tasks"); }); diff --git a/src/subagent/authority.test.ts b/src/subagent/authority.test.ts index f38ca8df9..7356dc364 100644 --- a/src/subagent/authority.test.ts +++ b/src/subagent/authority.test.ts @@ -3,7 +3,9 @@ import { assertCanTargetAgent, assertTierMayMountFleetVerb, FleetAuthorityError, + isDiscoveryVerb, isFleetVerb, + shouldMountSearchAgents, } from "./authority.js"; describe("assertTierMayMountFleetVerb", () => { @@ -25,14 +27,39 @@ describe("assertTierMayMountFleetVerb", () => { expect(() => assertTierMayMountFleetVerb("leaf", "read_file")).not.toThrow(); }); - test("Tier 1 and Tier 2 may mount fleet verbs", () => { + test("Tier 1 and Tier 2 may mount non-discovery fleet verbs", () => { expect(() => assertTierMayMountFleetVerb("orchestrator", "task")).not.toThrow(); expect(() => assertTierMayMountFleetVerb("nested-orchestrator", "task")).not.toThrow(); }); - test("isFleetVerb matches the same set used for the gate", () => { + test("discovery verbs are Tier 1 only (CL-7051)", () => { + expect(() => assertTierMayMountFleetVerb("orchestrator", "search_agents")).not.toThrow(); + expect(() => assertTierMayMountFleetVerb("orchestrator", "list_agents")).not.toThrow(); + expect(() => assertTierMayMountFleetVerb("nested-orchestrator", "search_agents")).toThrow( + FleetAuthorityError, + ); + expect(() => assertTierMayMountFleetVerb("nested-orchestrator", "list_agents")).toThrow( + FleetAuthorityError, + ); + expect(() => assertTierMayMountFleetVerb("leaf", "search_agents")).toThrow(FleetAuthorityError); + expect(() => assertTierMayMountFleetVerb("leaf", "list_agents")).toThrow(FleetAuthorityError); + }); + + test("isFleetVerb / isDiscoveryVerb match the same sets used for the gate", () => { expect(isFleetVerb("task")).toBe(true); expect(isFleetVerb("write_file")).toBe(false); + expect(isDiscoveryVerb("search_agents")).toBe(true); + expect(isDiscoveryVerb("list_agents")).toBe(true); + expect(isDiscoveryVerb("task")).toBe(false); + }); +}); + +describe("shouldMountSearchAgents", () => { + test("mounts only for Tier 1 when profiles are available", () => { + expect(shouldMountSearchAgents("orchestrator", true)).toBe(true); + expect(shouldMountSearchAgents("orchestrator", false)).toBe(false); + expect(shouldMountSearchAgents("nested-orchestrator", true)).toBe(false); + expect(shouldMountSearchAgents("leaf", true)).toBe(false); }); }); diff --git a/src/subagent/authority.ts b/src/subagent/authority.ts index 42e8fc465..250239455 100644 --- a/src/subagent/authority.ts +++ b/src/subagent/authority.ts @@ -8,7 +8,9 @@ * (today: task, search_agents, read_agent_trace; the spawn_agent/ * wait_agents/list_agents/send_input/interrupt_agent/close_agent/ * resume_agent/followup_task verbs land in later child issues against - * this same gate). + * this same gate). Discovery verbs (search_agents, list_agents) are + * further restricted to Tier 1 (primary Skywalker) only — nested + * orchestrators keep task/spawn but must not discover the fleet. * - assertCanTargetAgent: a Tier 2 nested orchestrator may act only on its * own descendants, never a sibling or anything above it in the tree. * Tier 1 (the primary orchestrator) may target anyone. Callers pass the @@ -41,10 +43,21 @@ export const FLEET_VERBS = new Set([ "followup_task", ]); +/** + * Fleet discovery verbs — Tier 1 (primary Skywalker) only. Nested + * orchestrators may keep task/spawn but must not index or list the fleet. + * list_agents is named here so a future mount site inherits the gate. + */ +export const DISCOVERY_VERBS = new Set(["search_agents", "list_agents"]); + export function isFleetVerb(toolName: string): boolean { return FLEET_VERBS.has(toolName); } +export function isDiscoveryVerb(toolName: string): boolean { + return DISCOVERY_VERBS.has(toolName); +} + export class FleetAuthorityError extends Error { constructor(message: string) { super(message); @@ -53,11 +66,21 @@ export class FleetAuthorityError extends Error { } /** - * Guard at the tool-mount point: throws if a Tier 3 leaf is about to receive - * a fleet verb. Call this where tools are assembled (run.ts), not from a - * prompt instruction — a leaf must never even hold the tool. + * Guard at the tool-mount point: throws if the caller's tier may not receive + * this fleet verb. Call this where tools are assembled (run.ts), not from a + * prompt instruction — a denied tier must never even hold the tool. + * + * Rules: + * - Discovery verbs (search_agents, list_agents): Tier 1 only + * - Other fleet verbs: Tier 1 and Tier 2; Tier 3 leaf denied */ export function assertTierMayMountFleetVerb(tier: SubagentTier, toolName: string): void { + if (isDiscoveryVerb(toolName) && tier !== "orchestrator") { + throw new FleetAuthorityError( + `Only Tier 1 (primary Skywalker) can mount discovery verb "${toolName}". ` + + `Nested orchestrators keep task/spawn but do not discover the fleet.`, + ); + } if (tier === "leaf" && isFleetVerb(toolName)) { throw new FleetAuthorityError( `Tier 3 leaf directors cannot mount fleet verb "${toolName}". ` + @@ -66,6 +89,15 @@ export function assertTierMayMountFleetVerb(tier: SubagentTier, toolName: string } } +/** + * Whether runSubAgent should install createSearchAgentsTool for this dispatch. + * Profiles must exist (otherwise the tool has nothing to search); tier must be + * Tier 1. Nested orchestrators and leaves never mount discovery. + */ +export function shouldMountSearchAgents(tier: SubagentTier, profilesAvailable: boolean): boolean { + return tier === "orchestrator" && profilesAvailable; +} + /** Minimal shape of a live fleet member — matches SubAgentSessionStore records. */ export interface FleetNode { readonly id: string; diff --git a/src/subagent/run-authority.test.ts b/src/subagent/run-authority.test.ts index 3dfbd3e9b..7113ab184 100644 --- a/src/subagent/run-authority.test.ts +++ b/src/subagent/run-authority.test.ts @@ -80,4 +80,24 @@ describe("runSubAgent fleet-verb mount gate (CL-6941, fails closed)", () => { expect(String((err as Error).message)).toContain("nestedDispatch"); } }); + + test("nested-orchestrator passes the non-discovery gate even when profiles would exist (CL-7051)", async () => { + // Before CL-7051 the blanket assert loop included search_agents, so a + // Tier-1-only discovery verb would have denied nested-orchestrator here + // once discovery became Tier-1-only. nestedDispatch is still omitted so + // we stop at the next check — proving search_agents is no longer in the + // blanket assert for Tier 2. + const cwd = await tmpCwd(); + try { + await runSubAgent({ + ...baseParams(cwd, join(cwd, ".ctx")), + orchestrator: true, + orchestratorTier: "nested-orchestrator", + }); + throw new Error("expected runSubAgent to reject (missing nestedDispatch)"); + } catch (err) { + expect(err).not.toBeInstanceOf(FleetAuthorityError); + expect(String((err as Error).message)).toContain("nestedDispatch"); + } + }); }); diff --git a/src/subagent/run.ts b/src/subagent/run.ts index 9feb03660..51273a6d4 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -91,7 +91,8 @@ import { type ForcedStopReason, } from "./stop-policy.js"; import { SubAgentDirector } from "./nudge-director.js"; -import { assertTierMayMountFleetVerb } from "./authority.js"; +import { assertTierMayMountFleetVerb, shouldMountSearchAgents } from "./authority.js"; + import { createReadAgentTraceTool } from "./trace-tool.js"; import { createSubmitResultState, @@ -466,18 +467,21 @@ export async function runSubAgent(params: RunSubAgentParams): Promise { const profiles = nd.profiles; diff --git a/src/subagent/types.ts b/src/subagent/types.ts index adfdb8c6c..342cdfe53 100644 --- a/src/subagent/types.ts +++ b/src/subagent/types.ts @@ -122,13 +122,15 @@ export type RunSubAgentParams = { * Fleet authority tier for this dispatch, resolved by the caller * (task-tool.ts) from either the closed DirectorPackage.tier or an explicit * AgentProfile.tier opt-in. Required whenever orchestrator is true: - * runSubAgent fails closed (denies task/search_agents) when orchestrator is + * runSubAgent fails closed (denies fleet verbs) when orchestrator is * true and this is undefined or "leaf" — an unrecognized or unresolved tier - * must never mount a fleet verb. See src/subagent/authority.ts. + * must never mount a fleet verb. Discovery verbs (search_agents) additionally + * require tier === "orchestrator" (Tier 1 Skywalker only). See + * src/subagent/authority.ts. */ orchestratorTier?: SubagentTier; - // Present only when orchestrator is true. Installs task + search_agents so - // the orchestrator can actually dispatch workers. + // Present only when orchestrator is true. Installs task (and search_agents + // only for Tier 1) so the orchestrator can actually dispatch workers. nestedDispatch?: NestedDispatchDeps; /** * Optional wall-clock budget for this worker's whole run (ms). Opt-in only — @@ -136,6 +138,7 @@ export type RunSubAgentParams = { * operator cancel alone. */ deadlineMs?: number; + /** * Resolved director tier, independent of `orchestratorTier` (which * is only ever set when `orchestrator` is true). Set by task-tool.ts from