luna/oh-my-opencode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(call-omo-agent): restrict callable agents

Browse Source
This commit is contained in:
YeonGyu-Kim
2026-05-12 15:28:57 +09:00
parent 9ffef79dbb
commit cde14b5e37
8 changed files with 190 additions and 300 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/guide/orchestration.md
+1 -1
View File
@@ -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
src/tools/call-omo-agent/agent-resolver.test.ts
+30 -205
View File
@@ -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<Record<string, unknown>>) {
function createMockClient(agents = []) {
return {
app: {
agents: mock(() => Promise.resolve({ data: agents })),
@@ -24,215 +10,54 @@ function createMockClient(agents: Array<Record<string, unknown>>) {
}
}
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<Record<string, unknown>>
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"])
})
})
})
src/tools/call-omo-agent/agent-resolver.ts
+8 -52
View File
@@ -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<string, { agents: string[]; timestamp: number }>();
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<string[]> {
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];
}
src/tools/call-omo-agent/agent-restriction.test.ts
+122
View File
@@ -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)
})
})
src/tools/call-omo-agent/constants.ts
+3 -8
View File
@@ -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=<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.`
src/tools/call-omo-agent/tools-edge-cases.test.ts
+12 -15
View File
@@ -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")
})
})
src/tools/call-omo-agent/tools.test.ts
+13 -18
View File
@@ -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")
})
})
src/tools/call-omo-agent/tools.ts
+1 -1
View File
@@ -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()