diff --git a/docs/guide/orchestration.md b/docs/guide/orchestration.md index dc50b10bd..e082025f8 100644 --- a/docs/guide/orchestration.md +++ b/docs/guide/orchestration.md @@ -63,7 +63,7 @@ flowchart TB Plan -->|"Read"| Orchestrator Orchestrator -->|"task(category=deep/quick/unspecified-*)"| Junior - Orchestrator -->|"call_omo_agent(subagent_type=oracle)"| Oracle + Orchestrator -->|"task(subagent_type=oracle)"| Oracle Orchestrator -->|"call_omo_agent(subagent_type=explore)"| Explore Orchestrator -->|"call_omo_agent(subagent_type=librarian)"| Librarian Orchestrator -->|"task(category=visual-engineering, load_skills=[frontend-ui-ux])"| Frontend diff --git a/src/tools/call-omo-agent/agent-resolver.test.ts b/src/tools/call-omo-agent/agent-resolver.test.ts index f2e13022b..773cd27aa 100644 --- a/src/tools/call-omo-agent/agent-resolver.test.ts +++ b/src/tools/call-omo-agent/agent-resolver.test.ts @@ -1,22 +1,8 @@ -/** - * Requirement-based tests for resolveCallableAgents(). - * - * These tests are derived from behavioral requirements in the PR description - * and feature spec, NOT from reading the implementation: - * - * R1: ALLOWED_AGENTS always present as baseline - * R2: Dynamic agents from client.app.agents() merged into the result - * R3: Primary-mode agents excluded from callable list - * R4: Falls back to ALLOWED_AGENTS alone when client.app.agents() fails - * R5: All output names are lowercase - * R6: No duplicate agent names in output - * R7: Malformed agent entries (null, missing name, non-string name, whitespace-only) are skipped gracefully - */ const { describe, test, expect, mock, beforeEach } = require("bun:test") const { resolveCallableAgents, clearCallableAgentsCache } = require("./agent-resolver") const { ALLOWED_AGENTS } = require("./constants") -function createMockClient(agents: Array>) { +function createMockClient(agents = []) { return { app: { agents: mock(() => Promise.resolve({ data: agents })), @@ -24,215 +10,54 @@ function createMockClient(agents: Array>) { } } -function createFailingClient(error: Error = new Error("API unavailable")) { - return { - app: { - agents: mock(() => Promise.reject(error)), - }, - } -} - describe("resolveCallableAgents", () => { beforeEach(() => { clearCallableAgentsCache() }) - describe("#given the SDK returns agents successfully", () => { - describe("#when only built-in agents exist", () => { - test("#then every ALLOWED_AGENT appears in the result", async () => { - const builtinAgents = ALLOWED_AGENTS.map((name: string) => ({ - name, - mode: "subagent", - })) - const client = createMockClient(builtinAgents) + describe("#given call_omo_agent is restricted to lookup agents", () => { + test("#then only ALLOWED_AGENTS are returned", async () => { + const client = createMockClient() - const result = await resolveCallableAgents(client) + const result = await resolveCallableAgents(client) - for (const agent of ALLOWED_AGENTS) { - expect(result).toContain(agent) - } - }) + expect(result).toEqual([...ALLOWED_AGENTS]) }) - describe("#when dynamic custom agents are present alongside built-ins", () => { - test("#then custom agents are included in the result", async () => { - const agents = [ - ...ALLOWED_AGENTS.map((name: string) => ({ name, mode: "subagent" })), - { name: "bug-fixer", mode: "subagent" }, - { name: "code-reviewer", mode: "subagent" }, - ] - const client = createMockClient(agents) + test("#then runtime custom agents are ignored and not queried", async () => { + const client = createMockClient([ + { name: "general", mode: "subagent" }, + { name: "bug-fixer", mode: "subagent" }, + ]) - const result = await resolveCallableAgents(client) + const result = await resolveCallableAgents(client) - expect(result).toContain("bug-fixer") - expect(result).toContain("code-reviewer") - }) - - test("#then ALLOWED_AGENTS are still present", async () => { - const agents = [{ name: "custom-agent", mode: "subagent" }] - const client = createMockClient(agents) - - const result = await resolveCallableAgents(client) - - for (const agent of ALLOWED_AGENTS) { - expect(result).toContain(agent) - } - }) + expect(result).toEqual(["explore", "librarian"]) + expect(client.app.agents).not.toHaveBeenCalled() }) - describe("#when an agent has mode=primary", () => { - test("#then it is excluded from the callable list", async () => { - const agents = [ - { name: "sisyphus", mode: "primary" }, - { name: "explore", mode: "subagent" }, - ] - const client = createMockClient(agents) + test("#then non-lookup built-ins are not included", async () => { + const client = createMockClient([ + { name: "oracle", mode: "subagent" }, + { name: "hephaestus", mode: "subagent" }, + { name: "metis", mode: "subagent" }, + ]) - const result = await resolveCallableAgents(client) + const result = await resolveCallableAgents(client) - expect(result).not.toContain("sisyphus") - expect(result).toContain("explore") - }) + expect(result).not.toContain("oracle") + expect(result).not.toContain("hephaestus") + expect(result).not.toContain("metis") }) - describe("#when agent names have mixed case", () => { - test("#then all output names are lowercase", async () => { - const agents = [ - { name: "Bug-Fixer", mode: "subagent" }, - { name: "CODE-REVIEWER", mode: "subagent" }, - ] - const client = createMockClient(agents) + test("#then each call returns a defensive copy", async () => { + const client = createMockClient() - const result = await resolveCallableAgents(client) + const first = await resolveCallableAgents(client) + first.push("general") + const second = await resolveCallableAgents(client) - expect(result).toContain("bug-fixer") - expect(result).toContain("code-reviewer") - for (const name of result) { - expect(name).toBe(name.toLowerCase()) - } - }) - }) - - describe("#when duplicate agent names exist across sources", () => { - test("#then no duplicates appear in the result", async () => { - const agents = [ - { name: "explore", mode: "subagent" }, - { name: "explore", mode: "subagent" }, - { name: "Explore", mode: "subagent" }, - ] - const client = createMockClient(agents) - - const result = await resolveCallableAgents(client) - - const exploreCount = result.filter((n: string) => n === "explore").length - expect(exploreCount).toBe(1) - }) - }) - - describe("#when agent entries are malformed", () => { - test("#then entries with null name are skipped", async () => { - const agents = [ - { name: null, mode: "subagent" }, - { name: "explore", mode: "subagent" }, - ] - const client = createMockClient(agents) - - const result = await resolveCallableAgents(client) - - expect(result).toContain("explore") - expect(result.length).toBeGreaterThanOrEqual(ALLOWED_AGENTS.length) - }) - - test("#then entries with numeric name are skipped", async () => { - const agents = [ - { name: 42, mode: "subagent" }, - { name: "explore", mode: "subagent" }, - ] - const client = createMockClient(agents) - - const result = await resolveCallableAgents(client) - - expect(result).not.toContain("42") - expect(result).toContain("explore") - }) - - test("#then entries with whitespace-only name are skipped", async () => { - const agents = [ - { name: " ", mode: "subagent" }, - { name: "explore", mode: "subagent" }, - ] - const client = createMockClient(agents) - - const result = await resolveCallableAgents(client) - - expect(result).not.toContain("") - expect(result).not.toContain(" ") - expect(result).toContain("explore") - }) - - test("#then entries with missing name property are skipped", async () => { - const agents = [ - { mode: "subagent" }, - { name: "explore", mode: "subagent" }, - ] - const client = createMockClient(agents) - - const result = await resolveCallableAgents(client) - - expect(result).toContain("explore") - expect(result.length).toBeGreaterThanOrEqual(ALLOWED_AGENTS.length) - }) - - test("#then entries that are undefined/null themselves are skipped", async () => { - const agents = [ - null, - undefined, - { name: "explore", mode: "subagent" }, - ] as unknown as Array> - const client = createMockClient(agents) - - const result = await resolveCallableAgents(client) - - expect(result).toContain("explore") - }) - }) - - describe("#when SDK returns an empty list", () => { - test("#then ALLOWED_AGENTS still appear as the baseline", async () => { - const client = createMockClient([]) - - const result = await resolveCallableAgents(client) - - for (const agent of ALLOWED_AGENTS) { - expect(result).toContain(agent) - } - expect(result.length).toBe(ALLOWED_AGENTS.length) - }) - }) - }) - - describe("#given the SDK call fails", () => { - describe("#when client.app.agents() throws an error", () => { - test("#then it falls back to ALLOWED_AGENTS", async () => { - const client = createFailingClient(new Error("Network error")) - - const result = await resolveCallableAgents(client) - - expect(result.length).toBe(ALLOWED_AGENTS.length) - for (const agent of ALLOWED_AGENTS) { - expect(result).toContain(agent) - } - }) - - test("#then custom agents are NOT available in fallback mode", async () => { - const client = createFailingClient() - - const result = await resolveCallableAgents(client) - - expect(result).not.toContain("bug-fixer") - expect(result).not.toContain("custom-agent") - }) + expect(second).toEqual(["explore", "librarian"]) }) }) }) diff --git a/src/tools/call-omo-agent/agent-resolver.ts b/src/tools/call-omo-agent/agent-resolver.ts index 70bc4c32f..350057b72 100644 --- a/src/tools/call-omo-agent/agent-resolver.ts +++ b/src/tools/call-omo-agent/agent-resolver.ts @@ -1,64 +1,20 @@ import type { PluginInput } from "@opencode-ai/plugin"; import { ALLOWED_AGENTS } from "./constants"; -import { normalizeSDKResponse } from "../../shared"; -import { log } from "../../shared/logger"; - -type AgentInfo = { - name: string; - mode?: "subagent" | "primary" | "all"; -}; - -const callableAgentsCache = new Map(); -const CACHE_TTL_MS = 30_000; export function clearCallableAgentsCache(): void { - callableAgentsCache.clear(); + // Kept for existing test setup and external callers; the resolver is now static. } /** - * Resolves the set of callable agent names at execute-time by merging the - * hardcoded `ALLOWED_AGENTS` with any additional agents discovered dynamically - * via `client.app.agents()`. Custom agents loaded from registered agent - * directories appear here alongside built-ins. + * Resolves the set of callable agent names for call_omo_agent. * - * Results are cached per session for 30s to avoid redundant SDK IPC calls. - * - * Falls back to `ALLOWED_AGENTS` alone if the dynamic lookup fails. - * - * @param client - The plugin client with access to the agent registry - * @param sessionId - Optional session ID for cache scoping - * @returns Array of lowercase callable agent names (excludes primary-mode agents) + * This tool is deliberately narrower than delegate-task: it may only launch + * the research lookup agents used by worker-style agents while they continue + * local work. Dynamic agents and other built-ins must go through task(). */ export async function resolveCallableAgents( - client: PluginInput["client"], - sessionId?: string, + _client?: PluginInput["client"], + _sessionId?: string, ): Promise { - const cacheKey = sessionId ?? "__default__"; - const cached = callableAgentsCache.get(cacheKey); - if (cached && Date.now() - cached.timestamp < CACHE_TTL_MS) { - return cached.agents; - } - - try { - const agentsResult = await client.app.agents(); - const agents = normalizeSDKResponse(agentsResult, [] as AgentInfo[], { - preferResponseOnMissingData: true, - }); - - const dynamicAgents = agents - .filter((a) => a && typeof a.name === "string" && a.name.trim().length > 0 && a.mode !== "primary") - .map((a) => a.name.trim().toLowerCase()); - - const merged = new Set([...ALLOWED_AGENTS, ...dynamicAgents]); - const result = [...merged]; - callableAgentsCache.set(cacheKey, { agents: result, timestamp: Date.now() }); - return result; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - log( - "[call_omo_agent] Failed to resolve dynamic agents, falling back to built-in list", - { error: message }, - ); - return [...ALLOWED_AGENTS]; - } + return [...ALLOWED_AGENTS]; } diff --git a/src/tools/call-omo-agent/agent-restriction.test.ts b/src/tools/call-omo-agent/agent-restriction.test.ts new file mode 100644 index 000000000..1ff7b0ca7 --- /dev/null +++ b/src/tools/call-omo-agent/agent-restriction.test.ts @@ -0,0 +1,122 @@ +import { describe, expect, mock, test } from "bun:test" +import { createCallOmoAgent } from "./tools" +import { clearCallableAgentsCache } from "./agent-resolver" + +type AgentEntry = { + name: string + mode: "subagent" | "primary" | "all" +} + +function createPluginInput(agents: AgentEntry[]) { + return { + client: { + app: { + agents: mock(() => Promise.resolve({ data: agents })), + }, + }, + directory: "/test", + } +} + +function createBackgroundManager() { + const launch = mock(() => Promise.resolve({ + id: "task-id", + sessionId: "session-id", + description: "Test task", + agent: "explore", + status: "pending", + })) + + return { + manager: { + launch, + getTask: mock(() => undefined), + reserveSubagentSpawn: mock(() => Promise.resolve({ + spawnContext: { rootSessionID: "root", parentDepth: 0, childDepth: 1 }, + descendantCount: 1, + commit: mock(() => undefined), + rollback: mock(() => undefined), + })), + }, + launch, + } +} + +const toolContext = { + sessionID: "parent-session", + messageID: "message-id", + agent: "sisyphus-junior", + abort: new AbortController().signal, +} + +describe("call_omo_agent restricted agent set", () => { + test("#when runtime exposes general as a subagent #then call_omo_agent rejects it before launch", async () => { + //#given + clearCallableAgentsCache() + const pluginInput = createPluginInput([ + { name: "explore", mode: "subagent" }, + { name: "librarian", mode: "subagent" }, + { name: "general", mode: "subagent" }, + ]) + const { manager, launch } = createBackgroundManager() + const toolDefinition = createCallOmoAgent(pluginInput, manager) + + //#when + const result = await toolDefinition.execute( + { description: "Test", prompt: "Do work", subagent_type: "general", run_in_background: true }, + toolContext, + ) + + //#then + expect(result).toContain("Invalid agent type") + expect(result).toContain("Only explore, librarian are allowed") + expect(launch).not.toHaveBeenCalled() + }) + + test("#when caller requests oracle #then call_omo_agent rejects it because only research lookup agents are callable", async () => { + //#given + clearCallableAgentsCache() + const pluginInput = createPluginInput([ + { name: "explore", mode: "subagent" }, + { name: "librarian", mode: "subagent" }, + { name: "oracle", mode: "subagent" }, + ]) + const { manager, launch } = createBackgroundManager() + const toolDefinition = createCallOmoAgent(pluginInput, manager) + + //#when + const result = await toolDefinition.execute( + { description: "Test", prompt: "Review this", subagent_type: "oracle", run_in_background: true }, + toolContext, + ) + + //#then + expect(result).toContain("Invalid agent type") + expect(result).toContain("Only explore, librarian are allowed") + expect(launch).not.toHaveBeenCalled() + }) + + test("#when caller requests explore or librarian #then call_omo_agent still launches them", async () => { + //#given + clearCallableAgentsCache() + const pluginInput = createPluginInput([ + { name: "explore", mode: "subagent" }, + { name: "librarian", mode: "subagent" }, + ]) + const { manager, launch } = createBackgroundManager() + const toolDefinition = createCallOmoAgent(pluginInput, manager) + + //#when + await toolDefinition.execute( + { description: "Explore", prompt: "Read code", subagent_type: "explore", run_in_background: true }, + toolContext, + ) + await toolDefinition.execute( + { description: "Research", prompt: "Find docs", subagent_type: "librarian", run_in_background: true }, + toolContext, + ) + + //#then + expect(launch).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/tools/call-omo-agent/constants.ts b/src/tools/call-omo-agent/constants.ts index 823313f2f..e09c7574a 100644 --- a/src/tools/call-omo-agent/constants.ts +++ b/src/tools/call-omo-agent/constants.ts @@ -1,18 +1,13 @@ export const ALLOWED_AGENTS = [ "explore", "librarian", - "oracle", - "hephaestus", - "metis", - "momus", - "multimodal-looker", ] as const -export const CALL_OMO_AGENT_DESCRIPTION = `Spawn explore/librarian agent or custom agents. run_in_background REQUIRED (true=async with task_id, false=sync). +export const CALL_OMO_AGENT_DESCRIPTION = `Spawn explore/librarian agent. run_in_background REQUIRED (true=async with task_id, false=sync). -Built-in agents: +Allowed agents: {agents} -Custom agents registered via user or project agent directories are also supported. +Other built-in agents, custom agents, and task categories are intentionally not supported by this tool. Pass \`session_id=\` to continue previous agent with full context. Nested subagent depth is tracked automatically and blocked past the configured limit. Prompts MUST be in English. Use \`background_output\` for async results.` diff --git a/src/tools/call-omo-agent/tools-edge-cases.test.ts b/src/tools/call-omo-agent/tools-edge-cases.test.ts index e075389cf..b9d1082b6 100644 --- a/src/tools/call-omo-agent/tools-edge-cases.test.ts +++ b/src/tools/call-omo-agent/tools-edge-cases.test.ts @@ -1,10 +1,10 @@ /** * Requirement-based integration tests for createCallOmoAgent edge cases - * introduced by the dev rebase and dynamic agent resolution feature. + * around restricted agent validation and execution cleanup. * * R1: Spawn reservation is rolled back when execution fails after reservation - * R2: Agent names with leading/trailing whitespace are trimmed before matching - * R3: An agent present in both ALLOWED_AGENTS and dynamic list is callable (no conflict) + * R2: Dynamic runtime agents do not expand the call_omo_agent allowlist + * R3: An agent present in both ALLOWED_AGENTS and runtime results is callable * R4: session_id continuation rejects in background mode when session already exists */ const { describe, test, expect, mock, beforeEach } = require("bun:test") @@ -27,11 +27,6 @@ function createMockCtx(agents: Array<{ name: string; mode?: string }> = []): Plu const DEFAULT_AGENTS = [ { name: "explore", mode: "subagent" }, { name: "librarian", mode: "subagent" }, - { name: "oracle", mode: "subagent" }, - { name: "hephaestus", mode: "subagent" }, - { name: "metis", mode: "subagent" }, - { name: "momus", mode: "subagent" }, - { name: "multimodal-looker", mode: "subagent" }, ] const reserveCommitMock = mock(() => 1) @@ -91,8 +86,8 @@ describe("createCallOmoAgent edge cases", () => { }) }) - describe("#given agent names with extra whitespace from SDK", () => { - test("#then whitespace-padded names are trimmed and matched correctly", async () => { + describe("#given a non-allowed agent appears in runtime agent results", () => { + test("#then the runtime agent is still rejected", async () => { const agents = [ ...DEFAULT_AGENTS, { name: " bug-fixer ", mode: "subagent" }, @@ -123,11 +118,12 @@ describe("createCallOmoAgent edge cases", () => { toolCtx, ) - expect(result).not.toContain("Invalid agent type") + expect(result).toContain("Invalid agent type") + expect(result).toContain("Only explore, librarian are allowed") }) }) - describe("#given an agent exists in both ALLOWED_AGENTS and dynamic results", () => { + describe("#given an agent exists in both ALLOWED_AGENTS and runtime results", () => { test("#then the agent is callable without conflict", async () => { const agents = [ ...DEFAULT_AGENTS, @@ -163,8 +159,8 @@ describe("createCallOmoAgent edge cases", () => { }) }) - describe("#given a disabled custom agent from dynamic resolution", () => { - test("#then disabled_agents check takes precedence over dynamic availability", async () => { + describe("#given a disabled custom agent appears in runtime results", () => { + test("#then restricted agent validation takes precedence over dynamic availability", async () => { const agents = [ ...DEFAULT_AGENTS, { name: "bug-fixer", mode: "subagent" }, @@ -189,7 +185,8 @@ describe("createCallOmoAgent edge cases", () => { toolCtx, ) - expect(result).toContain("disabled via disabled_agents") + expect(result).toContain("Invalid agent type") + expect(result).not.toContain("disabled via disabled_agents") }) }) diff --git a/src/tools/call-omo-agent/tools.test.ts b/src/tools/call-omo-agent/tools.test.ts index bad411339..49d968225 100644 --- a/src/tools/call-omo-agent/tools.test.ts +++ b/src/tools/call-omo-agent/tools.test.ts @@ -35,11 +35,6 @@ function createFailingMockCtx(error: Error = new Error("API unavailable")): Plug const DEFAULT_AGENTS = [ { name: "explore", mode: "subagent" }, { name: "librarian", mode: "subagent" }, - { name: "oracle", mode: "subagent" }, - { name: "hephaestus", mode: "subagent" }, - { name: "metis", mode: "subagent" }, - { name: "momus", mode: "subagent" }, - { name: "multimodal-looker", mode: "subagent" }, ] const assertCanSpawnMock = mock(() => Promise.resolve(undefined)) @@ -135,7 +130,7 @@ describe("createCallOmoAgent", () => { }) }) - describe("dynamic custom agent resolution", () => { + describe("restricted agent validation", () => { test("should reject missing subagent_type without throwing", async () => { const mockCtx = createMockCtx(DEFAULT_AGENTS) const toolDef = createCallOmoAgent(mockCtx, mockBackgroundManager, []) @@ -149,22 +144,22 @@ describe("createCallOmoAgent", () => { expect(result).toContain("subagent_type is required") }) - test("should accept a custom agent returned by client.app.agents()", async () => { - const agents = [...DEFAULT_AGENTS, { name: "bug-fixer", mode: "subagent" }] + test("should reject general even when returned by client.app.agents()", async () => { + const agents = [...DEFAULT_AGENTS, { name: "general", mode: "subagent" }] const mockCtx = createMockCtx(agents) const toolDef = createCallOmoAgent(mockCtx, mockBackgroundManager, []) const executeFunc = toolDef.execute as Function const result = await executeFunc( - { description: "Test", prompt: "Fix bug", subagent_type: "bug-fixer", run_in_background: true }, + { description: "Test", prompt: "Fix bug", subagent_type: "general", run_in_background: true }, toolCtx ) - expect(result).not.toContain("Invalid agent type") - expect(result).not.toContain("not found") + expect(result).toContain("Invalid agent type") + expect(result).toContain("Only explore, librarian are allowed") }) - test("should reject a custom agent NOT returned by client.app.agents()", async () => { + test("should reject unknown non-allowed agents", async () => { const mockCtx = createMockCtx(DEFAULT_AGENTS) const toolDef = createCallOmoAgent(mockCtx, mockBackgroundManager, []) const executeFunc = toolDef.execute as Function @@ -177,14 +172,13 @@ describe("createCallOmoAgent", () => { expect(result).toContain("Invalid agent type") }) - test("should perform case-insensitive matching for custom agents", async () => { - const agents = [...DEFAULT_AGENTS, { name: "Bug-Fixer", mode: "subagent" }] - const mockCtx = createMockCtx(agents) + test("should perform case-insensitive matching for allowed agents", async () => { + const mockCtx = createMockCtx(DEFAULT_AGENTS) const toolDef = createCallOmoAgent(mockCtx, mockBackgroundManager, []) const executeFunc = toolDef.execute as Function const result = await executeFunc( - { description: "Test", prompt: "Fix bug", subagent_type: "bug-fixer", run_in_background: true }, + { description: "Test", prompt: "Explore", subagent_type: "EXPLORE", run_in_background: true }, toolCtx ) @@ -234,7 +228,7 @@ describe("createCallOmoAgent", () => { expect(result).toContain("Invalid agent type") }) - test("should still apply disabled_agents check to dynamically resolved custom agents", async () => { + test("should reject non-allowed agents before disabled_agents can make them appear callable", async () => { const agents = [...DEFAULT_AGENTS, { name: "bug-fixer", mode: "subagent" }] const mockCtx = createMockCtx(agents) const toolDef = createCallOmoAgent(mockCtx, mockBackgroundManager, ["bug-fixer"]) @@ -245,7 +239,8 @@ describe("createCallOmoAgent", () => { toolCtx ) - expect(result).toContain("disabled via disabled_agents") + expect(result).toContain("Invalid agent type") + expect(result).not.toContain("disabled via disabled_agents") }) }) diff --git a/src/tools/call-omo-agent/tools.ts b/src/tools/call-omo-agent/tools.ts index 6e315cd86..2d8f4f405 100644 --- a/src/tools/call-omo-agent/tools.ts +++ b/src/tools/call-omo-agent/tools.ts @@ -122,7 +122,7 @@ export function createCallOmoAgent( subagent_type: tool.schema .string() .describe( - "The agent to invoke. Supports built-in agents and any custom agents registered at runtime.", + "The agent to invoke. Only explore and librarian are allowed.", ), run_in_background: tool.schema .boolean()