From 3ff2db698c2633023934d8477a9e995f78fa011e Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Tue, 2 Jun 2026 15:54:39 +0900 Subject: fix(perm): decouple perm_user_agent from perm_summon for spawning user agents Granting only the user-agent (top-level) permission without the subagent-summon permission left the agent unable to summon user agents: the whole summon tool was gated behind perm_summon, so perm_user_agent alone produced no summon tool. Register summon when EITHER perm_summon OR perm_user_agent is granted. createSummonTool now takes an independent subagentEnabled flag (mirrors perm_summon) alongside userAgentEnabled (mirrors perm_user_agent): - subagent-only -> ordinary subagents, no top_level - user-agent-only -> spawns ONLY top-level user agents (top_level forced, background/top_level params dropped, user-agent catalog only) - both -> unchanged full behavior retrieve stays bundled with perm_summon (user agents are fire-and-forget). Adds core summon tests (user-agent-only mode + legacy-default regression) and an agent-manager summon/user_agent permission-split suite. --- packages/api/src/agent-manager.ts | 36 ++++--- packages/api/tests/agent-manager.test.ts | 61 ++++++++++++ packages/core/src/tools/summon.ts | 163 ++++++++++++++++++++++--------- packages/core/tests/tools/summon.test.ts | 108 ++++++++++++++++++++ 4 files changed, 308 insertions(+), 60 deletions(-) diff --git a/packages/api/src/agent-manager.ts b/packages/api/src/agent-manager.ts index 85dd160..9499ce5 100644 --- a/packages/api/src/agent-manager.ts +++ b/packages/api/src/agent-manager.ts @@ -575,7 +575,13 @@ export class AgentManager { }); } toolEntries.push({ name: "todo", tool: createTaskListTool(tabAgent.taskList) }); - if (permSummon) { + // The `summon` tool is registered when EITHER the subagent + // permission (`perm_summon`) OR the user-agent permission + // (`perm_user_agent`) is granted — the two are independent. + // `perm_summon` enables ordinary subagent spawning; granting + // only `perm_user_agent` exposes summon in user-agent-only mode + // (spawns top-level user agents exclusively). + if (permSummon || permUserAgent) { // Capture parent's allowed tool names for child permission enforcement const parentAllowedTools = new Set(toolEntries.map((e) => e.name)); const allAgentDefs = loadAgents(workingDirectory); @@ -609,19 +615,25 @@ export class AgentManager { availableUserAgents, agentDirPaths, permUserAgent, + permSummon, ), }); - toolEntries.push({ - name: "retrieve", - tool: createRetrieveTool({ - getResult: (id) => - tabAgent.shellStore.has(id) - ? tabAgent.shellStore.getResult(id) - : tabAgent.transcriptStore.has(id) - ? tabAgent.transcriptStore.getResult(id) - : this.getChildResult(id), - }), - }); + // `retrieve` collects subagent results. User agents are + // fire-and-forget, so it is bundled with the subagent + // permission only — a user-agent-only grant doesn't get it. + if (permSummon) { + toolEntries.push({ + name: "retrieve", + tool: createRetrieveTool({ + getResult: (id) => + tabAgent.shellStore.has(id) + ? tabAgent.shellStore.getResult(id) + : tabAgent.transcriptStore.has(id) + ? tabAgent.transcriptStore.getResult(id) + : this.getChildResult(id), + }), + }); + } } if (permSendToTab || permReadTab) { const tabCommAllowed = new Set(); diff --git a/packages/api/tests/agent-manager.test.ts b/packages/api/tests/agent-manager.test.ts index 014022a..f3ea207 100644 --- a/packages/api/tests/agent-manager.test.ts +++ b/packages/api/tests/agent-manager.test.ts @@ -319,6 +319,22 @@ vi.mock("@dispatch/core", () => ({ execute: async () => "mock", }; }, + // Summon parent-path dependencies. The real implementations load agent + // definitions from disk; tests only need the summon/retrieve tool entries + // to appear, so these return empty projections. + loadAgents() { + return []; + }, + toAvailableSubagents() { + return []; + }, + toAvailableUserAgents() { + return []; + }, + getAgentDirPaths() { + return []; + }, + GLOBAL_AGENTS_DIR: "/tmp/global-agents", createTab() {}, getTab(id: string) { return fakeTabs.get(id) ?? null; @@ -1441,6 +1457,51 @@ describe("AgentManager", () => { }); }); + describe("summon / user_agent permission split", () => { + // Drives the real parent-path tool construction in + // getOrCreateAgentForTab by toggling perm_summon and perm_user_agent + // independently, then inspecting which tools the constructed Agent + // received. The summon tool must be registered when EITHER permission + // is granted; `retrieve` rides with the subagent permission only + // (user agents are fire-and-forget). + async function toolsForPerms(tabId: string, perms: Record): Promise { + for (const [k, v] of Object.entries(perms)) setFakeSetting(k, v); + const manager = new AgentManager(); + await manager.processMessage(tabId, "go"); + return constructedAgents.at(-1)?.toolNames ?? []; + } + + it("grants summon + retrieve when only perm_summon is allowed", async () => { + const tools = await toolsForPerms("tab-summon-only", { perm_summon: "allow" }); + expect(tools).toContain("summon"); + expect(tools).toContain("retrieve"); + }); + + it("grants summon WITHOUT retrieve when only perm_user_agent is allowed", async () => { + // Regression: granting only the user-agent permission used to leave + // the agent unable to summon user agents because the whole summon + // tool was gated behind perm_summon. + const tools = await toolsForPerms("tab-user-agent-only", { perm_user_agent: "allow" }); + expect(tools).toContain("summon"); + expect(tools).not.toContain("retrieve"); + }); + + it("grants summon + retrieve when both permissions are allowed", async () => { + const tools = await toolsForPerms("tab-summon-both", { + perm_summon: "allow", + perm_user_agent: "allow", + }); + expect(tools).toContain("summon"); + expect(tools).toContain("retrieve"); + }); + + it("grants neither summon nor retrieve when both permissions are off", async () => { + const tools = await toolsForPerms("tab-summon-neither", {}); + expect(tools).not.toContain("summon"); + expect(tools).not.toContain("retrieve"); + }); + }); + // ─── Usage side-channel persistence ────────────────────────────── // // `usage` AgentEvents (one per LLM round-trip) are persisted as invisible diff --git a/packages/core/src/tools/summon.ts b/packages/core/src/tools/summon.ts index 4820e89..cfee8b8 100644 --- a/packages/core/src/tools/summon.ts +++ b/packages/core/src/tools/summon.ts @@ -60,10 +60,13 @@ function renderAgentGroup(label: string, agents: AvailableAgent[]): string[] { * the disk locations where they live, injected into the summon tool's * description. * - * When `userAgentEnabled` is false only subagents are shown (under the - * generic "Available agents" heading). When it is true, subagents and - * user agents are listed as two labelled groups so the LLM understands - * which slugs require `top_level=true`. + * `subagentEnabled` and `userAgentEnabled` independently control which + * groups are shown — they mirror the `perm_summon` and `perm_user_agent` + * permissions respectively: + * - subagents only → generic "Available agents" heading; + * - user agents only → a single user-agent group (top_level is implied); + * - both → two labelled groups so the LLM understands which slugs + * require `top_level=true`. * * Returns a compact "no agents defined" notice when nothing is visible. */ @@ -72,6 +75,7 @@ function buildAgentsCatalog( userAgents: AvailableAgent[], agentDirs: string[], userAgentEnabled: boolean, + subagentEnabled: boolean, ): string { const lines: string[] = []; lines.push(""); @@ -80,8 +84,9 @@ function buildAgentsCatalog( lines.push(` - ${d}`); } + const visibleSubagents = subagentEnabled ? subagents : []; const visibleUserAgents = userAgentEnabled ? userAgents : []; - if (subagents.length === 0 && visibleUserAgents.length === 0) { + if (visibleSubagents.length === 0 && visibleUserAgents.length === 0) { lines.push(""); lines.push("No agent definitions are currently defined."); return lines.join("\n"); @@ -93,12 +98,26 @@ function buildAgentsCatalog( lines.push("and working directory; the 'tools' parameter is ignored."); lines.push(""); + // User-agent-only mode: list just the user agents. top_level is implied + // (it is the only thing this grant can spawn), so the heading omits it. + if (!subagentEnabled && userAgentEnabled) { + lines.push( + ...renderAgentGroup( + "User agents (spawned as independent top-level tabs):", + visibleUserAgents, + ), + ); + return lines.join("\n"); + } + + // Subagent-only mode: single generic heading. if (!userAgentEnabled) { - lines.push(...renderAgentGroup("Available agents:", subagents)); + lines.push(...renderAgentGroup("Available agents:", visibleSubagents)); return lines.join("\n"); } - const subagentLines = renderAgentGroup("Subagents (spawned as child tabs):", subagents); + // Both enabled: two labelled groups. + const subagentLines = renderAgentGroup("Subagents (spawned as child tabs):", visibleSubagents); const userAgentLines = renderAgentGroup( "User agents (spawned as independent top-level tabs, requires top_level=true):", visibleUserAgents, @@ -122,9 +141,14 @@ function buildAgentsCatalog( * its description; this is information-only — the runtime resolves * slugs through `loadAgent` independently. * - * `userAgentEnabled` controls whether the `top_level` parameter and the - * user-agent catalog are surfaced to the LLM. It mirrors the - * `perm_user_agent` permission. + * `userAgentEnabled` mirrors the `perm_user_agent` permission and + * `subagentEnabled` mirrors the `perm_summon` permission. They are + * independent: the tool is registered whenever at least one is granted. + * - subagentEnabled only → spawn ordinary subagents (no `top_level`); + * - userAgentEnabled only → spawn ONLY top-level user agents + * (`top_level` is forced on, the `background` knob is dropped, and + * the catalog lists user agents only); + * - both → full behavior (subagents plus `top_level` user agents). */ export function createSummonTool( _defaultWorkingDirectory: string, @@ -133,39 +157,29 @@ export function createSummonTool( availableUserAgents: AvailableAgent[] = [], agentDirs: string[] = [], userAgentEnabled = false, + subagentEnabled = true, ): ToolDefinition { + // When only the user-agent permission is granted the tool spawns user + // agents exclusively: `top_level` is implied (and forced), subagent + // mechanics (background, retrieve, parallel work) are irrelevant. + const userAgentOnly = userAgentEnabled && !subagentEnabled; + const catalog = buildAgentsCatalog( availableSubagents, availableUserAgents, agentDirs, userAgentEnabled, + subagentEnabled, ); const subagentSlugs = availableSubagents.map((a) => a.slug); const userAgentSlugs = availableUserAgents.map((a) => a.slug); - const allSlugs = userAgentEnabled ? [...subagentSlugs, ...userAgentSlugs] : subagentSlugs; + const allSlugs = userAgentOnly + ? userAgentSlugs + : userAgentEnabled + ? [...subagentSlugs, ...userAgentSlugs] + : subagentSlugs; - const description = [ - "Spawn a new child agent to work on a task independently.", - "", - "By default, blocks until the child agent finishes and returns the result directly.", - "Set background=true to return immediately with an agent_id instead — use retrieve to collect the result later.", - "", - "The child agent runs in its own tab visible to the user. Use the 'retrieve' tool with the returned agent_id to get the result when needed.", - "", - "Pattern for parallel work:", - " 1. Call summon multiple times with background=true to start several agents", - " 2. Do your own work or wait", - " 3. Call retrieve for each agent_id to collect results", - ...(userAgentEnabled - ? [ - "", - "Set top_level=true to spawn an independent user agent — a first-class", - "top-level tab with no parent. User agents are fire-and-forget: you get", - "an agent_id back but cannot retrieve their result. top_level requires an", - "'agent' definition listed under 'User agents' below.", - ] - : []), - "", + const toolNamesList = [ "The 'tools' parameter controls what the child can do. Available tool names:", " - read_file: Read file contents", " - read_file_slice: Read a character-range slice of a single line", @@ -179,11 +193,50 @@ export function createSummonTool( " - youtube_transcribe: Fetch YouTube video transcripts", " - send_to_tab: Send a message to another tab/agent by its ID", " - read_tab: Read another tab/agent's latest response by its ID", - "", - "The 'agent' parameter is required — every spawned agent must use a definition.", - "Tools default to the agent definition's tools, intersected with your own tools (you can't grant capabilities you don't have).", - catalog, - ].join("\n"); + ]; + + const description = userAgentOnly + ? [ + "Spawn an independent top-level user agent to work on a task.", + "", + "User agents are first-class top-level tabs with no parent. They are", + "fire-and-forget: you get an agent_id back but cannot retrieve their result.", + "The user agent runs in its own tab visible to the user.", + "", + ...toolNamesList, + "", + "The 'agent' parameter is required — every spawned agent must use a definition.", + "Tools default to the agent definition's tools, intersected with your own tools (you can't grant capabilities you don't have).", + catalog, + ].join("\n") + : [ + "Spawn a new child agent to work on a task independently.", + "", + "By default, blocks until the child agent finishes and returns the result directly.", + "Set background=true to return immediately with an agent_id instead — use retrieve to collect the result later.", + "", + "The child agent runs in its own tab visible to the user. Use the 'retrieve' tool with the returned agent_id to get the result when needed.", + "", + "Pattern for parallel work:", + " 1. Call summon multiple times with background=true to start several agents", + " 2. Do your own work or wait", + " 3. Call retrieve for each agent_id to collect results", + ...(userAgentEnabled + ? [ + "", + "Set top_level=true to spawn an independent user agent — a first-class", + "top-level tab with no parent. User agents are fire-and-forget: you get", + "an agent_id back but cannot retrieve their result. top_level requires an", + "'agent' definition listed under 'User agents' below.", + ] + : []), + "", + ...toolNamesList, + "", + "The 'agent' parameter is required — every spawned agent must use a definition.", + "Tools default to the agent definition's tools, intersected with your own tools (you can't grant capabilities you don't have).", + catalog, + ].join("\n"); const parametersShape = { task: z @@ -205,7 +258,10 @@ export function createSummonTool( .filter(Boolean) .join(" "), ), - ...(userAgentEnabled + // `top_level` is only an explicit choice when BOTH subagents and user + // agents are available. In user-agent-only mode it is implied (forced + // on), so the knob is omitted entirely. + ...(userAgentEnabled && !userAgentOnly ? { top_level: z .boolean() @@ -248,12 +304,18 @@ export function createSummonTool( .describe( "Absolute path for the child to work in. Defaults to the agent definition's cwd (or the spawning agent's directory).", ), - background: z - .boolean() - .optional() - .describe( - "If true, returns immediately with an agent_id for later retrieval. If false (default), blocks until the child agent finishes and returns the result directly. Ignored when top_level is true.", - ), + // `background` is meaningless for fire-and-forget user agents, so the + // knob is omitted in user-agent-only mode. + ...(userAgentOnly + ? {} + : { + background: z + .boolean() + .optional() + .describe( + "If true, returns immediately with an agent_id for later retrieval. If false (default), blocks until the child agent finishes and returns the result directly. Ignored when top_level is true.", + ), + }), }; return { @@ -266,9 +328,14 @@ export function createSummonTool( const tools = args.tools as string[] | undefined; const workingDirectory = args.working_directory as string | undefined; const background = (args.background as boolean | undefined) ?? false; - const topLevel = userAgentEnabled - ? ((args.top_level as boolean | undefined) ?? false) - : false; + // User-agent-only mode always spawns top-level user agents. When both + // capabilities are present the caller chooses via `top_level`. When + // only subagents are available, top-level spawning is unavailable. + const topLevel = userAgentOnly + ? true + : userAgentEnabled + ? ((args.top_level as boolean | undefined) ?? false) + : false; try { const agentId = await callbacks.spawn({ diff --git a/packages/core/tests/tools/summon.test.ts b/packages/core/tests/tools/summon.test.ts index f59f345..4885a94 100644 --- a/packages/core/tests/tools/summon.test.ts +++ b/packages/core/tests/tools/summon.test.ts @@ -239,3 +239,111 @@ describe("createSummonTool — execute() argument forwarding", () => { expect(getResult).toHaveBeenCalled(); }); }); + +describe("createSummonTool — user-agent-only mode (perm_user_agent without perm_summon)", () => { + // userAgentEnabled=true, subagentEnabled=false → the tool spawns ONLY + // top-level user agents. `top_level` is implied (and forced), the + // subagent/parallel-work prose is dropped, and only the user-agent + // catalog group is shown. + const subagents: AvailableAgent[] = [ + { + slug: "programmer", + name: "Programmer", + description: "Codes things", + path: "/agents/programmer.toml", + }, + ]; + const userAgents: AvailableAgent[] = [ + { + slug: "default", + name: "Default", + description: "Default agent", + path: "/agents/default.toml", + }, + ]; + + function userAgentOnlyTool( + spawn = vi.fn(async () => "ua-1"), + getResult = vi.fn(async () => ({ status: "done" as const, result: "nope" })), + ) { + return { + spawn, + getResult, + tool: createSummonTool( + "/tmp/work", + { spawn, getResult }, + subagents, + userAgents, + ["/agents"], + true, // userAgentEnabled + false, // subagentEnabled + ), + }; + } + + it("describes spawning user agents and omits subagent/parallel-work prose", () => { + const { tool } = userAgentOnlyTool(); + expect(tool.description).toContain("Spawn an independent top-level user agent"); + expect(tool.description).toContain("fire-and-forget"); + expect(tool.description).not.toContain("Pattern for parallel work"); + expect(tool.description).not.toContain("Set background=true"); + }); + + it("lists only the user-agent catalog group, not subagents", () => { + const { tool } = userAgentOnlyTool(); + expect(tool.description).toContain("User agents (spawned as independent top-level tabs):"); + expect(tool.description).toContain("default"); + // Subagents must not be advertised in user-agent-only mode. + expect(tool.description).not.toContain("Subagents (spawned as child tabs):"); + expect(tool.description).not.toContain("- programmer: Programmer"); + }); + + it("only lists user-agent slugs in the 'agent' parameter description", () => { + const { tool } = userAgentOnlyTool(); + const agentParam = (tool.parameters as unknown as { shape: { agent: { description: string } } }) + .shape.agent; + expect(agentParam.description).toContain("default"); + expect(agentParam.description).not.toContain("programmer"); + }); + + it("omits the top_level parameter (it is implied)", () => { + const { tool } = userAgentOnlyTool(); + const shape = (tool.parameters as unknown as { shape: Record }).shape; + expect("top_level" in shape).toBe(false); + }); + + it("omits the background parameter (user agents are fire-and-forget)", () => { + const { tool } = userAgentOnlyTool(); + const shape = (tool.parameters as unknown as { shape: Record }).shape; + expect("background" in shape).toBe(false); + }); + + it("forces topLevel=true on spawn even when top_level is not passed", async () => { + const spawn = vi.fn(async () => "ua-99"); + const getResult = vi.fn(async () => ({ status: "done" as const, result: "nope" })); + const { tool } = userAgentOnlyTool(spawn, getResult); + const out = await tool.execute({ task: "do stuff", agent: "default" }); + expect(out).toContain("User agent spawned successfully"); + expect(out).toContain("ua-99"); + expect(out).toContain("fire-and-forget"); + // Never blocks on a result for fire-and-forget user agents. + expect(getResult).not.toHaveBeenCalled(); + const callArg = spawn.mock.calls[0]?.[0]; + expect(callArg).toMatchObject({ topLevel: true, agentSlug: "default" }); + }); +}); + +describe("createSummonTool — subagentEnabled defaults preserve legacy behavior", () => { + it("defaults subagentEnabled=true so omitting it keeps subagent spawning", async () => { + const spawn = vi.fn(async () => "tab-1"); + const getResult = vi.fn(async () => ({ status: "done" as const, result: "child" })); + // No userAgentEnabled/subagentEnabled args → legacy subagent-only mode. + const tool = createSummonTool("/tmp/work", { spawn, getResult }, [], []); + const out = await tool.execute({ task: "x", agent: "programmer" }); + // Foreground subagent summon blocks and returns the child result. + expect(out).toBe("agent_id: tab-1\n\nchild"); + expect(getResult).toHaveBeenCalled(); + const callArg = spawn.mock.calls[0]?.[0]; + expect(callArg).not.toHaveProperty("topLevel"); + }); +}); -- cgit v1.2.3