diff --git a/src/config/schema/agent-definitions.ts b/src/config/schema/agent-definitions.ts index 508bfde0b..328238e2c 100644 --- a/src/config/schema/agent-definitions.ts +++ b/src/config/schema/agent-definitions.ts @@ -1,5 +1,5 @@ import { z } from "zod" -export const AgentDefinitionPathSchema = z.string() +export const AgentDefinitionPathSchema = z.string().min(1) export const AgentDefinitionsConfigSchema = z.array(AgentDefinitionPathSchema).optional() diff --git a/src/features/claude-code-agent-loader/agent-definitions-loader.ts b/src/features/claude-code-agent-loader/agent-definitions-loader.ts index 0736fe347..383f69387 100644 --- a/src/features/claude-code-agent-loader/agent-definitions-loader.ts +++ b/src/features/claude-code-agent-loader/agent-definitions-loader.ts @@ -2,23 +2,11 @@ import { existsSync, readFileSync } from "fs" import { basename, extname } from "path" import { parseFrontmatter } from "../../shared/frontmatter" import { log } from "../../shared/logger" +import { parseToolsConfig } from "../../shared/parse-tools-config" import { parseJsonAgentFile } from "./json-agent-loader" import { mapClaudeModelToOpenCode } from "./claude-model-mapper" import type { AgentScope, AgentFrontmatter, ClaudeCodeAgentConfig, LoadedAgent } from "./types" -function parseToolsConfig(toolsStr?: string): Record | undefined { - if (!toolsStr) return undefined - - const tools = toolsStr.split(",").map((t) => t.trim()).filter(Boolean) - if (tools.length === 0) return undefined - - const result: Record = {} - for (const tool of tools) { - result[tool.toLowerCase()] = true - } - return result -} - export function parseMarkdownAgentFile(filePath: string, scope: AgentScope): LoadedAgent | null { try { if (!existsSync(filePath)) { @@ -67,7 +55,7 @@ export function loadAgentDefinitions( paths: string[], scope: AgentScope ): Record { - const result: Record = {} + const result: Record = Object.create(null) for (const filePath of paths) { if (!existsSync(filePath)) { diff --git a/src/features/claude-code-agent-loader/json-agent-loader.ts b/src/features/claude-code-agent-loader/json-agent-loader.ts index 394d2a25c..fa135fa6f 100644 --- a/src/features/claude-code-agent-loader/json-agent-loader.ts +++ b/src/features/claude-code-agent-loader/json-agent-loader.ts @@ -1,23 +1,9 @@ import { existsSync, readFileSync } from "fs" import { parseJsoncSafe } from "../../shared/jsonc-parser" +import { parseToolsConfig } from "../../shared/parse-tools-config" import { mapClaudeModelToOpenCode } from "./claude-model-mapper" import type { AgentScope, AgentJsonDefinition, ClaudeCodeAgentConfig, LoadedAgent } from "./types" -function parseToolsConfig(tools?: string | string[]): Record | undefined { - if (!tools) return undefined - - const toolsArray = Array.isArray(tools) ? tools : tools.split(",").map((t) => t.trim()) - const filtered = toolsArray.filter((t) => typeof t === "string" && t.length > 0) - - if (filtered.length === 0) return undefined - - const result: Record = {} - for (const tool of filtered) { - result[tool.toLowerCase()] = true - } - return result -} - export function parseJsonAgentFile(filePath: string, scope: AgentScope): LoadedAgent | null { try { if (!existsSync(filePath)) { diff --git a/src/features/claude-code-agent-loader/loader.ts b/src/features/claude-code-agent-loader/loader.ts index 5a1bbf8fd..380f479eb 100644 --- a/src/features/claude-code-agent-loader/loader.ts +++ b/src/features/claude-code-agent-loader/loader.ts @@ -32,7 +32,7 @@ export function loadUserAgents(): Record { const userAgentsDir = join(getClaudeConfigDir(), "agents") const agents = loadAgentsFromDir(userAgentsDir, "user") - const result: Record = {} + const result: Record = Object.create(null) for (const agent of agents) { result[agent.name] = agent.config } @@ -43,7 +43,7 @@ export function loadProjectAgents(directory?: string): Record = {} + const result: Record = Object.create(null) for (const agent of agents) { result[agent.name] = agent.config } @@ -55,7 +55,7 @@ export function loadOpencodeGlobalAgents(): Record = {} + const result: Record = Object.create(null) for (const agent of agents) { result[agent.name] = agent.config } @@ -66,7 +66,7 @@ export function loadOpencodeProjectAgents(directory?: string): Record = {} + const result: Record = Object.create(null) for (const agent of agents) { result[agent.name] = agent.config } diff --git a/src/features/claude-code-agent-loader/opencode-config-agents-reader.test.ts b/src/features/claude-code-agent-loader/opencode-config-agents-reader.test.ts index 52ed83da3..1f7a48deb 100644 --- a/src/features/claude-code-agent-loader/opencode-config-agents-reader.test.ts +++ b/src/features/claude-code-agent-loader/opencode-config-agents-reader.test.ts @@ -1,11 +1,26 @@ -import { describe, expect, it } from "bun:test" +import { describe, expect, it, beforeEach, afterEach } from "bun:test" +import { mock } from "bun:test" import * as fs from "node:fs" import * as os from "node:os" import * as path from "node:path" -import { readOpencodeConfigAgents } from "./opencode-config-agents-reader" +// Mock getOpenCodeConfigDir to prevent global config leakage +let mockGlobalConfigDir: string +mock.module("../../shared/opencode-config-dir", () => ({ + getOpenCodeConfigDir: () => mockGlobalConfigDir, +})) + +const { readOpencodeConfigAgents } = require("./opencode-config-agents-reader") describe("readOpencodeConfigAgents", () => { + beforeEach(() => { + mockGlobalConfigDir = fs.mkdtempSync(path.join(os.tmpdir(), "opencode-mock-global-")) + }) + + afterEach(() => { + fs.rmSync(mockGlobalConfigDir, { recursive: true, force: true }) + }) + it("returns empty record when no opencode.json exists", () => { const nonexistentDir = "/nonexistent/directory/path" const result = readOpencodeConfigAgents(nonexistentDir) diff --git a/src/features/claude-code-agent-loader/opencode-config-agents-reader.ts b/src/features/claude-code-agent-loader/opencode-config-agents-reader.ts index 5b3f41a75..480fdbe68 100644 --- a/src/features/claude-code-agent-loader/opencode-config-agents-reader.ts +++ b/src/features/claude-code-agent-loader/opencode-config-agents-reader.ts @@ -3,6 +3,8 @@ import * as path from "node:path" import { getOpenCodeConfigDir } from "../../shared/opencode-config-dir" import { parseJsoncSafe } from "../../shared/jsonc-parser" +import { parseToolsConfig } from "../../shared/parse-tools-config" +import { resolveAgentDefinitionPaths } from "../../shared/resolve-agent-definition-paths" import { loadAgentDefinitions } from "./agent-definitions-loader" import { mapClaudeModelToOpenCode } from "./claude-model-mapper" import type { ClaudeCodeAgentConfig } from "./types" @@ -25,28 +27,6 @@ function getConfigPaths(directory: string): string[] { return paths } -function parseToolsConfig(toolsValue: unknown): Record | undefined { - if (!toolsValue) return undefined - - let toolsStr: string - if (typeof toolsValue === "string") { - toolsStr = toolsValue - } else if (Array.isArray(toolsValue)) { - toolsStr = toolsValue.filter((t) => typeof t === "string").join(",") - } else { - return undefined - } - - const tools = toolsStr.split(",").map((t) => t.trim()).filter(Boolean) - if (tools.length === 0) return undefined - - const result: Record = {} - for (const tool of tools) { - result[tool.toLowerCase()] = true - } - return result -} - function convertInlineAgent(agentData: unknown): ClaudeCodeAgentConfig | null { if (!agentData || typeof agentData !== "object") { return null @@ -63,9 +43,15 @@ function convertInlineAgent(agentData: unknown): ClaudeCodeAgentConfig | null { ? `${mappedModel.providerID}/${mappedModel.modelID}` : undefined + const VALID_MODES = ["subagent", "primary", "all"] as const + const rawMode = typeof agent.mode === "string" ? agent.mode : undefined + const mode = rawMode && (VALID_MODES as readonly string[]).includes(rawMode) + ? (rawMode as "subagent" | "primary" | "all") + : "subagent" + const config: ClaudeCodeAgentConfig = { description, - mode: (agent.mode as "subagent" | "primary" | "all") || "subagent", + mode, prompt: agent.prompt ? String(agent.prompt) : "", ...(modelString ? { model: modelString } : {}), } @@ -106,9 +92,7 @@ export function readOpencodeConfigAgents(directory: string): Record - path.isAbsolute(p) ? p : path.resolve(configDir, p) - ) + const resolvedPaths = resolveAgentDefinitionPaths(definitionPaths, configDir, directory) const definitionAgents = loadAgentDefinitions(resolvedPaths, "opencode-config") diff --git a/src/features/claude-code-plugin-loader/agent-loader.ts b/src/features/claude-code-plugin-loader/agent-loader.ts index 1aff57e0c..128b9f229 100644 --- a/src/features/claude-code-plugin-loader/agent-loader.ts +++ b/src/features/claude-code-plugin-loader/agent-loader.ts @@ -3,27 +3,11 @@ import { basename, join } from "path" import { parseFrontmatter } from "../../shared/frontmatter" import { isMarkdownFile } from "../../shared/file-utils" import { log } from "../../shared/logger" +import { parseToolsConfig } from "../../shared/parse-tools-config" import type { AgentFrontmatter, ClaudeCodeAgentConfig } from "../claude-code-agent-loader/types" import { mapClaudeModelToOpenCode } from "../claude-code-agent-loader/claude-model-mapper" import type { LoadedPlugin } from "./types" -function parseToolsConfig(toolsStr?: string): Record | undefined { - if (!toolsStr) return undefined - - const tools = toolsStr - .split(",") - .map((tool) => tool.trim()) - .filter(Boolean) - - if (tools.length === 0) return undefined - - const result: Record = {} - for (const tool of tools) { - result[tool.toLowerCase()] = true - } - return result -} - export function loadPluginAgents(plugins: LoadedPlugin[]): Record { const agents: Record = {} diff --git a/src/shared/index.ts b/src/shared/index.ts index cf9792361..826434d42 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -76,3 +76,4 @@ export { SessionCategoryRegistry } from "./session-category-registry" export * from "./plugin-identity" export * from "./log-legacy-plugin-startup-warning" export * from "./task-system-enabled" +export * from "./parse-tools-config" diff --git a/src/shared/parse-tools-config.ts b/src/shared/parse-tools-config.ts new file mode 100644 index 000000000..bc5f50169 --- /dev/null +++ b/src/shared/parse-tools-config.ts @@ -0,0 +1,25 @@ +/** + * Parses a tools configuration value into a boolean record. + * Accepts comma-separated strings, string arrays, or unknown values from config files. + * Returns undefined when input is empty or invalid. + */ +export function parseToolsConfig(toolsValue: unknown): Record | undefined { + if (!toolsValue) return undefined + + let items: string[] + if (typeof toolsValue === "string") { + items = toolsValue.split(",").map((t) => t.trim()).filter(Boolean) + } else if (Array.isArray(toolsValue)) { + items = toolsValue.filter((t) => typeof t === "string" && t.trim().length > 0).map((t) => (t as string).trim()) + } else { + return undefined + } + + if (items.length === 0) return undefined + + const result: Record = {} + for (const tool of items) { + result[tool.toLowerCase()] = true + } + return result +} diff --git a/src/shared/resolve-agent-definition-paths.test.ts b/src/shared/resolve-agent-definition-paths.test.ts new file mode 100644 index 000000000..fcc14be7a --- /dev/null +++ b/src/shared/resolve-agent-definition-paths.test.ts @@ -0,0 +1,122 @@ +import { describe, test, expect, beforeEach, afterEach } from "bun:test" +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "fs" +import { join } from "path" +import { homedir } from "os" +import { tmpdir } from "os" + +import { resolveAgentDefinitionPaths } from "./resolve-agent-definition-paths" + +describe("resolveAgentDefinitionPaths", () => { + let tempDir: string + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "resolve-agent-def-paths-")) + }) + + afterEach(() => { + rmSync(tempDir, { recursive: true, force: true }) + }) + + describe("#given relative paths", () => { + test("#then they are resolved against baseDir", () => { + const result = resolveAgentDefinitionPaths( + ["agents/my-agent.md"], + tempDir, + null, + ) + + expect(result).toEqual([join(tempDir, "agents/my-agent.md")]) + }) + }) + + describe("#given absolute paths", () => { + test("#then they are returned as-is", () => { + const absPath = join(tempDir, "absolute-agent.md") + + const result = resolveAgentDefinitionPaths( + [absPath], + "/some/other/base", + null, + ) + + expect(result).toEqual([absPath]) + }) + }) + + describe("#given tilde-prefixed paths", () => { + test("#then ~ is expanded to homedir", () => { + const result = resolveAgentDefinitionPaths( + ["~/agents/test.md"], + tempDir, + null, + ) + + expect(result).toEqual([join(homedir(), "agents/test.md")]) + }) + }) + + describe("#given containmentDir is set", () => { + test("#then paths outside the boundary are rejected", () => { + const projectDir = join(tempDir, "project") + mkdirSync(projectDir, { recursive: true }) + + const result = resolveAgentDefinitionPaths( + ["/etc/passwd"], + projectDir, + projectDir, + ) + + expect(result).toEqual([]) + }) + + test("#then paths inside the boundary are allowed", () => { + const projectDir = join(tempDir, "project") + const agentsDir = join(projectDir, "agents") + mkdirSync(agentsDir, { recursive: true }) + writeFileSync(join(agentsDir, "a.md"), "test", "utf-8") + + const result = resolveAgentDefinitionPaths( + ["agents/a.md"], + projectDir, + projectDir, + ) + + expect(result).toEqual([join(projectDir, "agents/a.md")]) + }) + }) + + describe("#given containmentDir is null", () => { + test("#then no boundary check is applied", () => { + const result = resolveAgentDefinitionPaths( + ["/some/outside/path/agent.md"], + tempDir, + null, + ) + + expect(result).toEqual(["/some/outside/path/agent.md"]) + }) + }) + + describe("#given an empty paths array", () => { + test("#then an empty array is returned", () => { + const result = resolveAgentDefinitionPaths([], tempDir, null) + + expect(result).toEqual([]) + }) + }) + + describe("#given mixed valid and invalid paths", () => { + test("#then only valid paths within the boundary are returned", () => { + const projectDir = join(tempDir, "project") + mkdirSync(projectDir, { recursive: true }) + + const result = resolveAgentDefinitionPaths( + ["./valid.md", "/outside/boundary.md"], + projectDir, + projectDir, + ) + + expect(result).toEqual([join(projectDir, "valid.md")]) + }) + }) +}) diff --git a/src/tools/call-omo-agent/agent-resolver.test.ts b/src/tools/call-omo-agent/agent-resolver.test.ts index cadd0c99f..f2e13022b 100644 --- a/src/tools/call-omo-agent/agent-resolver.test.ts +++ b/src/tools/call-omo-agent/agent-resolver.test.ts @@ -12,8 +12,8 @@ * 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 } = require("bun:test") -const { resolveCallableAgents } = require("./agent-resolver") +const { describe, test, expect, mock, beforeEach } = require("bun:test") +const { resolveCallableAgents, clearCallableAgentsCache } = require("./agent-resolver") const { ALLOWED_AGENTS } = require("./constants") function createMockClient(agents: Array>) { @@ -33,6 +33,10 @@ function createFailingClient(error: Error = new Error("API unavailable")) { } 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 () => { diff --git a/src/tools/call-omo-agent/agent-resolver.ts b/src/tools/call-omo-agent/agent-resolver.ts index 9f87feb79..70bc4c32f 100644 --- a/src/tools/call-omo-agent/agent-resolver.ts +++ b/src/tools/call-omo-agent/agent-resolver.ts @@ -8,20 +8,37 @@ type AgentInfo = { mode?: "subagent" | "primary" | "all"; }; +const callableAgentsCache = new Map(); +const CACHE_TTL_MS = 30_000; + +export function clearCallableAgentsCache(): void { + callableAgentsCache.clear(); +} + /** * 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. * + * 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) */ export async function resolveCallableAgents( 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[], { @@ -33,7 +50,9 @@ export async function resolveCallableAgents( .map((a) => a.name.trim().toLowerCase()); const merged = new Set([...ALLOWED_AGENTS, ...dynamicAgents]); - return [...merged]; + 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( 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 7e766c28f..9d4c546e9 100644 --- a/src/tools/call-omo-agent/tools-edge-cases.test.ts +++ b/src/tools/call-omo-agent/tools-edge-cases.test.ts @@ -9,6 +9,7 @@ */ const { describe, test, expect, mock, beforeEach } = require("bun:test") const { createCallOmoAgent } = require("./tools") +const { clearCallableAgentsCache } = require("./agent-resolver") type PluginInput = { client: any; directory: string } @@ -50,6 +51,7 @@ const toolCtx = { } beforeEach(() => { + clearCallableAgentsCache() reserveSubagentSpawnMock.mockClear() reserveCommitMock.mockClear() reserveRollbackMock.mockClear() diff --git a/src/tools/call-omo-agent/tools.test.ts b/src/tools/call-omo-agent/tools.test.ts index 455e2f9aa..17491cb82 100644 --- a/src/tools/call-omo-agent/tools.test.ts +++ b/src/tools/call-omo-agent/tools.test.ts @@ -1,5 +1,6 @@ const { beforeEach, describe, test, expect, mock } = require("bun:test") const { createCallOmoAgent } = require("./tools") +const { clearCallableAgentsCache } = require("./agent-resolver") type PluginInput = { client: any; directory: string } type BackgroundManager = { @@ -72,6 +73,7 @@ const toolCtx = { } beforeEach(() => { + clearCallableAgentsCache() assertCanSpawnMock.mockClear() reserveSubagentSpawnMock.mockClear() reserveCommitMock.mockClear()