diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6f16fcb6cf..bee7db8dca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,12 +52,19 @@ jobs: bun test src/hooks/atlas bun test src/hooks/compaction-context-injector bun test src/features/tmux-subagent + bun test src/shared/session-utils.test.ts + bun test src/tools/skill/tools.test.ts - name: Run remaining tests run: | - # Run all other tests (mock-heavy ones are re-run but that's acceptable) + # Run all other tests + # IMPORTANT: src/shared is listed as individual files to exclude + # session-utils.test.ts which uses mock.module("node:fs") and would + # pollute the fs module for all other tests in the same process. + # session-utils.test.ts is already covered in the isolated mock-heavy step above. + SHARED_TESTS=$(find src/shared -name '*.test.ts' ! -name 'session-utils.test.ts' | sort) bun test bin script src/cli src/config src/mcp src/index.test.ts \ - src/agents src/tools src/shared \ + src/agents src/tools $SHARED_TESTS \ src/hooks/anthropic-context-window-limit-recovery \ src/hooks/claude-code-compatibility \ src/hooks/context-injection \ diff --git a/src/config/schema.test.ts b/src/config/schema.test.ts index 704a7ea81b..00a61f42d6 100644 --- a/src/config/schema.test.ts +++ b/src/config/schema.test.ts @@ -722,14 +722,78 @@ describe("GitMasterConfigSchema", () => { } }) - test("rejects number for commit_footer", () => { - //#given - const config = { commit_footer: 123 } + test("rejects number for commit_footer", () => { + //#given + const config = { commit_footer: 123 } - //#when - const result = GitMasterConfigSchema.safeParse(config) + //#when + const result = GitMasterConfigSchema.safeParse(config) - //#then + //#then + expect(result.success).toBe(false) + }) +}) + +describe("agent_display_names schema", () => { + test("should accept agent_display_names with string values", () => { + // given + const config = { + agent_display_names: { sisyphus: "Builder", oracle: "Debugger" }, + } + + // when + const result = OhMyOpenCodeConfigSchema.safeParse(config) + + // then + expect(result.success).toBe(true) + if (result.success) { + expect(result.data.agent_display_names).toEqual({ + sisyphus: "Builder", + oracle: "Debugger", + }) + } + }) + + test("should reject agent_display_names with non-string values", () => { + // given + const config = { + agent_display_names: { sisyphus: 123 }, + } + + // when + const result = OhMyOpenCodeConfigSchema.safeParse(config) + + // then expect(result.success).toBe(false) }) + + test("should accept empty agent_display_names object", () => { + // given + const config = { + agent_display_names: {}, + } + + // when + const result = OhMyOpenCodeConfigSchema.safeParse(config) + + // then + expect(result.success).toBe(true) + if (result.success) { + expect(result.data.agent_display_names).toEqual({}) + } + }) + + test("should accept undefined agent_display_names (optional field)", () => { + // given + const config = {} + + // when + const result = OhMyOpenCodeConfigSchema.safeParse(config) + + // then + expect(result.success).toBe(true) + if (result.success) { + expect(result.data.agent_display_names).toBeUndefined() + } + }) }) diff --git a/src/config/schema/oh-my-opencode-config.ts b/src/config/schema/oh-my-opencode-config.ts index be0ebd9149..e823e96330 100644 --- a/src/config/schema/oh-my-opencode-config.ts +++ b/src/config/schema/oh-my-opencode-config.ts @@ -32,8 +32,10 @@ export const OhMyOpenCodeConfigSchema = z.object({ disabled_hooks: z.array(HookNameSchema).optional(), disabled_commands: z.array(BuiltinCommandNameSchema).optional(), /** Disable specific tools by name (e.g., ["todowrite", "todoread"]) */ - disabled_tools: z.array(z.string()).optional(), - agents: AgentOverridesSchema.optional(), + disabled_tools: z.array(z.string()).optional(), + /** Map agent canonical names to custom display names (e.g., { "sisyphus": "Builder" }) */ + agent_display_names: z.record(z.string(), z.string()).optional(), + agents: AgentOverridesSchema.optional(), categories: CategoriesConfigSchema.optional(), claude_code: ClaudeCodeConfigSchema.optional(), sisyphus_agent: SisyphusAgentConfigSchema.optional(), diff --git a/src/hooks/todo-continuation-enforcer/idle-event.ts b/src/hooks/todo-continuation-enforcer/idle-event.ts index 86d3e504f3..a42e07e378 100644 --- a/src/hooks/todo-continuation-enforcer/idle-event.ts +++ b/src/hooks/todo-continuation-enforcer/idle-event.ts @@ -5,6 +5,7 @@ import { readBoulderState } from "../../features/boulder-state" import { subagentSessions } from "../../features/claude-code-session-state" import type { ToolPermission } from "../../features/hook-message-injector" import { log } from "../../shared/logger" +import { toCanonical } from "../../shared/agent-name-aliases" import { ABORT_WINDOW_MS, @@ -112,12 +113,12 @@ export async function handleSessionIdle(args: { path: { id: sessionID }, }) const messages = (messagesResp.data ?? []) as Array<{ info?: MessageInfo }> - for (let i = messages.length - 1; i >= 0; i--) { - const info = messages[i].info - if (info?.agent === "compaction") { - hasCompactionMessage = true - continue - } + for (let i = messages.length - 1; i >= 0; i--) { + const info = messages[i].info + if (toCanonical(info?.agent ?? "") === "compaction") { + hasCompactionMessage = true + continue + } if (info?.agent || info?.model || (info?.modelID && info?.providerID)) { resolvedInfo = { agent: info.agent, @@ -131,12 +132,12 @@ export async function handleSessionIdle(args: { log(`[${HOOK_NAME}] Failed to fetch messages for agent check`, { sessionID, error: String(error) }) } - log(`[${HOOK_NAME}] Agent check`, { sessionID, agentName: resolvedInfo?.agent, skipAgents, hasCompactionMessage }) + log(`[${HOOK_NAME}] Agent check`, { sessionID, agentName: resolvedInfo?.agent, skipAgents, hasCompactionMessage }) - if (resolvedInfo?.agent && skipAgents.includes(resolvedInfo.agent)) { - log(`[${HOOK_NAME}] Skipped: agent in skipAgents list`, { sessionID, agent: resolvedInfo.agent }) - return - } + if (resolvedInfo?.agent && skipAgents.includes(toCanonical(resolvedInfo.agent))) { + log(`[${HOOK_NAME}] Skipped: agent in skipAgents list`, { sessionID, agent: resolvedInfo.agent }) + return + } if (hasCompactionMessage && !resolvedInfo?.agent) { log(`[${HOOK_NAME}] Skipped: compaction occurred but no agent info resolved`, { sessionID }) return diff --git a/src/hooks/todo-continuation-enforcer/todo-continuation-enforcer.test.ts b/src/hooks/todo-continuation-enforcer/todo-continuation-enforcer.test.ts index 760165dd79..84e864828e 100644 --- a/src/hooks/todo-continuation-enforcer/todo-continuation-enforcer.test.ts +++ b/src/hooks/todo-continuation-enforcer/todo-continuation-enforcer.test.ts @@ -4,6 +4,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test" import type { BackgroundManager } from "../../features/background-agent" import { setMainSession, subagentSessions, _resetForTesting } from "../../features/claude-code-session-state" +import { initializeAgentNameAliases, resetAgentNameAliases } from "../../shared/agent-name-aliases" import { createTodoContinuationEnforcer } from "." type TimerCallback = (...args: any[]) => void @@ -212,19 +213,21 @@ describe("todo-continuation-enforcer", () => { } as any } - beforeEach(() => { - fakeTimers = createFakeTimers() - _resetForTesting() - promptCalls = [] - toastCalls = [] - mockMessages = [] - }) - - afterEach(() => { - fakeTimers.restore() - _resetForTesting() - cleanupBoulderFile() - }) + beforeEach(() => { + fakeTimers = createFakeTimers() + _resetForTesting() + resetAgentNameAliases() + promptCalls = [] + toastCalls = [] + mockMessages = [] + }) + + afterEach(() => { + fakeTimers.restore() + _resetForTesting() + resetAgentNameAliases() + cleanupBoulderFile() + }) test("should inject continuation when idle with incomplete todos", async () => { fakeTimers.restore() @@ -1415,25 +1418,147 @@ describe("todo-continuation-enforcer", () => { expect(promptCalls).toHaveLength(0) }) - test("should still inject for background task session regardless of boulder state", async () => { - fakeTimers.restore() - // given - background task session with no boulder entry - setMainSession("main-session") - const bgTaskSession = "bg-task-boulder-test" - subagentSessions.add(bgTaskSession) - cleanupBoulderFile() + test("should still inject for background task session regardless of boulder state", async () => { + fakeTimers.restore() + // given - background task session with no boulder entry + setMainSession("main-session") + const bgTaskSession = "bg-task-boulder-test" + subagentSessions.add(bgTaskSession) + cleanupBoulderFile() + + const hook = createTodoContinuationEnforcer(createMockPluginInput(), {}) + + // when - background task session goes idle + await hook.handler({ + event: { type: "session.idle", properties: { sessionID: bgTaskSession } }, + }) + + await wait(2500) + + // then - continuation still injected (background tasks bypass boulder check) + expect(promptCalls.length).toBe(1) + expect(promptCalls[0].sessionID).toBe(bgTaskSession) + }, { timeout: 15000 }) + + test("should canonicalize agent name when checking skipAgents list", async () => { + // given - session with renamed prometheus agent + const sessionID = "main-renamed-prometheus" + setupMainSessionWithBoulder(sessionID) + + initializeAgentNameAliases( + { prometheus: "Prometheus (Planner)" }, + ["prometheus", "compaction", "sisyphus"] + ) + + const mockMessagesWithRenamedPrometheus = [ + { info: { id: "msg-1", role: "user", agent: "Prometheus (Planner)" } }, + { info: { id: "msg-2", role: "assistant", agent: "Prometheus (Planner)" } }, + ] + + const mockInput = { + client: { + session: { + todo: async () => ({ + data: [{ id: "1", content: "Task 1", status: "pending", priority: "high" }], + }), + messages: async () => ({ data: mockMessagesWithRenamedPrometheus }), + prompt: async (opts: any) => { + promptCalls.push({ + sessionID: opts.path.id, + agent: opts.body.agent, + model: opts.body.model, + text: opts.body.parts[0].text, + }) + return {} + }, + promptAsync: async (opts: any) => { + promptCalls.push({ + sessionID: opts.path.id, + agent: opts.body.agent, + model: opts.body.model, + text: opts.body.parts[0].text, + }) + return {} + }, + }, + tui: { showToast: async () => ({}) }, + }, + directory: "/tmp/test", + } as any - const hook = createTodoContinuationEnforcer(createMockPluginInput(), {}) + const hook = createTodoContinuationEnforcer(mockInput, { + skipAgents: ["prometheus"], + }) - // when - background task session goes idle - await hook.handler({ - event: { type: "session.idle", properties: { sessionID: bgTaskSession } }, - }) + // when - session goes idle with renamed prometheus agent + await hook.handler({ + event: { type: "session.idle", properties: { sessionID } }, + }) - await wait(2500) + await fakeTimers.advanceBy(3000) - // then - continuation still injected (background tasks bypass boulder check) - expect(promptCalls.length).toBe(1) - expect(promptCalls[0].sessionID).toBe(bgTaskSession) - }, { timeout: 15000 }) + // then - no continuation (renamed agent canonicalized and matched in skipAgents) + expect(promptCalls).toHaveLength(0) + }) + + test("should canonicalize compaction agent name when filtering messages", async () => { + // given - session where compaction agent is renamed + const sessionID = "main-renamed-compaction" + setupMainSessionWithBoulder(sessionID) + + initializeAgentNameAliases( + { compaction: "Compaction Agent" }, + ["prometheus", "compaction", "sisyphus"] + ) + + const mockMessagesWithRenamedCompaction = [ + { info: { id: "msg-1", role: "user", agent: "sisyphus" } }, + { info: { id: "msg-2", role: "assistant", agent: "sisyphus" } }, + { info: { id: "msg-3", role: "assistant", agent: "Compaction Agent" } }, + ] + + const mockInput = { + client: { + session: { + todo: async () => ({ + data: [{ id: "1", content: "Task 1", status: "pending", priority: "high" }], + }), + messages: async () => ({ data: mockMessagesWithRenamedCompaction }), + prompt: async (opts: any) => { + promptCalls.push({ + sessionID: opts.path.id, + agent: opts.body.agent, + model: opts.body.model, + text: opts.body.parts[0].text, + }) + return {} + }, + promptAsync: async (opts: any) => { + promptCalls.push({ + sessionID: opts.path.id, + agent: opts.body.agent, + model: opts.body.model, + text: opts.body.parts[0].text, + }) + return {} + }, + }, + tui: { showToast: async () => ({}) }, + }, + directory: "/tmp/test", + } as any + + const hook = createTodoContinuationEnforcer(mockInput, {}) + + // when - session goes idle + await hook.handler({ + event: { type: "session.idle", properties: { sessionID } }, + }) + + await fakeTimers.advanceBy(2500) + + // then - continuation uses sisyphus (renamed compaction agent was filtered) + expect(promptCalls.length).toBe(1) + expect(promptCalls[0].agent).toBe("sisyphus") + }) }) diff --git a/src/index.test.ts b/src/index.test.ts index 8d2c6d9760..8f0c15b981 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -1,4 +1,6 @@ -import { describe, expect, it } from "bun:test" +import { describe, expect, it, beforeEach } from "bun:test" +import { initializeAgentNameAliases, toCanonical, resetAgentNameAliases } from "./shared/agent-name-aliases" + /** * Tests for conditional tool registration logic in index.ts * @@ -79,12 +81,55 @@ describe("look_at tool conditional registration", () => { // given lookAt is null (agent disabled) // when spreading into tool object // then look_at should NOT be included - it("excludes look_at when lookAt is null", () => { - const lookAt = null - const tools = { - ...(lookAt ? { look_at: lookAt } : {}), - } - expect(tools).not.toHaveProperty("look_at") - }) - }) + it("excludes look_at when lookAt is null", () => { + const lookAt = null + const tools = { + ...(lookAt ? { look_at: lookAt } : {}), + } + expect(tools).not.toHaveProperty("look_at") + }) + }) + + describe("disabled_agents with canonicalization", () => { + beforeEach(() => { + resetAgentNameAliases() + }) + + // given disabled_agents contains canonical name "multimodal-looker" + // when checking if agent is enabled with toCanonical + // then should return false (disabled) + it("returns false when multimodal-looker is disabled (canonical name)", () => { + const disabledAgents: string[] = ["multimodal-looker"] + const isEnabled = !disabledAgents.some( + (agent) => toCanonical(agent).toLowerCase() === "multimodal-looker" + ) + expect(isEnabled).toBe(false) + }) + + // given disabled_agents contains display name alias for multimodal-looker + // when checking if agent is enabled with toCanonical + // then should return false (disabled via alias) + it("returns false when multimodal-looker is disabled (via display name alias)", () => { + initializeAgentNameAliases( + { "multimodal-looker": "Image Analyzer" }, + ["multimodal-looker"] + ) + const disabledAgents: string[] = ["Image Analyzer"] + const isEnabled = !disabledAgents.some( + (agent) => toCanonical(agent).toLowerCase() === "multimodal-looker" + ) + expect(isEnabled).toBe(false) + }) + + // given disabled_agents is empty + // when checking if agent is enabled with toCanonical + // then should return true (enabled by default) + it("returns true when disabled_agents is empty (with canonicalization)", () => { + const disabledAgents: string[] = [] + const isEnabled = !disabledAgents.some( + (agent) => toCanonical(agent).toLowerCase() === "multimodal-looker" + ) + expect(isEnabled).toBe(true) + }) + }) }) diff --git a/src/index.ts b/src/index.ts index 69cae8cd5e..7d8d24d3c3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -22,6 +22,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { startTmuxCheck() const pluginConfig = loadPluginConfig(ctx.directory, ctx) + const disabledHooks = new Set(pluginConfig.disabled_hooks ?? []) const isHookEnabled = (hookName: HookName): boolean => !disabledHooks.has(hookName) diff --git a/src/plugin-config.ts b/src/plugin-config.ts index b95942fd4c..bd73376243 100644 --- a/src/plugin-config.ts +++ b/src/plugin-config.ts @@ -54,6 +54,7 @@ export function mergeConfigs( return { ...base, ...override, + agent_display_names: { ...(base.agent_display_names ?? {}), ...(override.agent_display_names ?? {}) }, agents: deepMerge(base.agents, override.agents), categories: deepMerge(base.categories, override.categories), disabled_agents: [ diff --git a/src/plugin-handlers/agent-display-name-rekeyer.test.ts b/src/plugin-handlers/agent-display-name-rekeyer.test.ts new file mode 100644 index 0000000000..32355f471e --- /dev/null +++ b/src/plugin-handlers/agent-display-name-rekeyer.test.ts @@ -0,0 +1,229 @@ +/// + +import { describe, test, expect, spyOn, beforeEach, afterEach } from "bun:test" +import { rekeyAgentsByDisplayNames } from "./agent-display-name-rekeyer" +import * as aliasModule from "../shared/agent-name-aliases" +import * as displayNamesModule from "../shared/agent-display-names" +import * as shared from "../shared" + +beforeEach(() => { + aliasModule.resetAgentNameAliases() + displayNamesModule.resetAgentDisplayNames() + spyOn(shared, "log" as any).mockImplementation(() => {}) +}) + +afterEach(() => { + aliasModule.resetAgentNameAliases() + displayNamesModule.resetAgentDisplayNames() + ;(shared.log as any)?.mockRestore?.() +}) + +describe("rekeyAgentsByDisplayNames", () => { + test("basic re-keying: sisyphus -> Bob", () => { + //#given + const config: Record = { + agent: { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + }, + } + const agentResult: Record = { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + } + const displayNames = { sisyphus: "Bob" } + + //#when + rekeyAgentsByDisplayNames({ config, agentResult, displayNames }) + + //#then + const agent = config.agent as Record + expect(agent["Bob"]).toBeDefined() + expect(agent["sisyphus"]).toBeUndefined() + expect(agentResult["Bob"]).toBeDefined() + expect(agentResult["sisyphus"]).toBeUndefined() + }) + + test("default_agent is re-keyed: sisyphus -> Bob", () => { + //#given + const config: Record = { + default_agent: "sisyphus", + agent: { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + }, + } + const agentResult: Record = { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + } + const displayNames = { sisyphus: "Bob" } + + //#when + rekeyAgentsByDisplayNames({ config, agentResult, displayNames }) + + //#then + expect(config.default_agent).toBe("Bob") + }) + + test("no-op when displayNames is undefined", () => { + //#given + const config: Record = { + agent: { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + }, + } + const agentResult: Record = { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + } + + //#when + rekeyAgentsByDisplayNames({ config, agentResult, displayNames: undefined }) + + //#then + const agent = config.agent as Record + expect(agent["sisyphus"]).toBeDefined() + expect(agentResult["sisyphus"]).toBeDefined() + }) + + test("no-op when displayNames is empty {}", () => { + //#given + const config: Record = { + agent: { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + }, + } + const agentResult: Record = { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + } + + //#when + rekeyAgentsByDisplayNames({ config, agentResult, displayNames: {} }) + + //#then + const agent = config.agent as Record + expect(agent["sisyphus"]).toBeDefined() + expect(agentResult["sisyphus"]).toBeDefined() + }) + + test("multiple re-keys: sisyphus -> Bob, oracle -> Advisor", () => { + //#given + const config: Record = { + agent: { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + oracle: { name: "oracle", prompt: "test", mode: "subagent" }, + }, + } + const agentResult: Record = { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + oracle: { name: "oracle", prompt: "test", mode: "subagent" }, + } + const displayNames = { sisyphus: "Bob", oracle: "Advisor" } + + //#when + rekeyAgentsByDisplayNames({ config, agentResult, displayNames }) + + //#then + const agent = config.agent as Record + expect(agent["Bob"]).toBeDefined() + expect(agent["sisyphus"]).toBeUndefined() + expect(agent["Advisor"]).toBeDefined() + expect(agent["oracle"]).toBeUndefined() + expect(agentResult["Bob"]).toBeDefined() + expect(agentResult["Advisor"]).toBeDefined() + }) + + test("non-existent agent in displayNames: warning logged, no crash", () => { + //#given + const config: Record = { + agent: { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + }, + } + const agentResult: Record = { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + } + const displayNames = { nonexistent: "Ghost" } + const logSpy = shared.log as ReturnType + + //#when + rekeyAgentsByDisplayNames({ config, agentResult, displayNames }) + + //#then - should not crash, should log warning + const agent = config.agent as Record + expect(agent["sisyphus"]).toBeDefined() + expect(agent["Ghost"]).toBeUndefined() + const logCalls = logSpy.mock.calls.map((c: unknown[]) => c[0]) + const hasWarning = logCalls.some( + (msg: string) => typeof msg === "string" && msg.includes("nonexistent") + ) + expect(hasWarning).toBe(true) + }) + + test("unchanged agents remain: agents not in displayNames keep canonical keys", () => { + //#given + const config: Record = { + agent: { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + oracle: { name: "oracle", prompt: "test", mode: "subagent" }, + librarian: { name: "librarian", prompt: "test", mode: "subagent" }, + }, + } + const agentResult: Record = { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + oracle: { name: "oracle", prompt: "test", mode: "subagent" }, + librarian: { name: "librarian", prompt: "test", mode: "subagent" }, + } + const displayNames = { sisyphus: "Bob" } + + //#when + rekeyAgentsByDisplayNames({ config, agentResult, displayNames }) + + //#then + const agent = config.agent as Record + expect(agent["Bob"]).toBeDefined() + expect(agent["sisyphus"]).toBeUndefined() + expect(agent["oracle"]).toBeDefined() + expect(agent["librarian"]).toBeDefined() + expect(agentResult["oracle"]).toBeDefined() + expect(agentResult["librarian"]).toBeDefined() + }) + + test("agentResult is also re-keyed in sync with config.agent", () => { + //#given + const sisyphusData = { name: "sisyphus", prompt: "test", mode: "primary", permission: { task: "allow" } } + const config: Record = { + agent: { sisyphus: { ...sisyphusData } }, + } + const agentResult: Record = { + sisyphus: { ...sisyphusData }, + } + const displayNames = { sisyphus: "Bob" } + + //#when + rekeyAgentsByDisplayNames({ config, agentResult, displayNames }) + + //#then + expect(agentResult["Bob"]).toBeDefined() + expect(agentResult["sisyphus"]).toBeUndefined() + const rekeyed = agentResult["Bob"] as Record + expect(rekeyed.permission).toEqual({ task: "allow" }) + }) + + test("does not create entries in config.agent when canonical key missing from config", () => { + //#given - agentResult has sisyphus but config.agent does not + const config: Record = { + agent: {}, + } + const agentResult: Record = { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + } + const displayNames = { sisyphus: "Bob" } + + //#when + rekeyAgentsByDisplayNames({ config, agentResult, displayNames }) + + //#then - agentResult re-keyed, config.agent untouched (no creation) + expect(agentResult["Bob"]).toBeDefined() + expect(agentResult["sisyphus"]).toBeUndefined() + const agent = config.agent as Record + expect(agent["Bob"]).toBeUndefined() + expect(Object.keys(agent)).toHaveLength(0) + }) +}) diff --git a/src/plugin-handlers/agent-display-name-rekeyer.ts b/src/plugin-handlers/agent-display-name-rekeyer.ts new file mode 100644 index 0000000000..f573db713a --- /dev/null +++ b/src/plugin-handlers/agent-display-name-rekeyer.ts @@ -0,0 +1,46 @@ +import { log } from "../shared" +import { initializeAgentDisplayNames } from "../shared/agent-display-names" +import { + initializeAgentNameAliases, + getCanonicalToRegisteredMap, +} from "../shared/agent-name-aliases" + +export function rekeyAgentsByDisplayNames(params: { + config: Record + agentResult: Record + displayNames: Record | undefined +}): void { + const { config, agentResult, displayNames } = params + + if (!displayNames || Object.keys(displayNames).length === 0) return + + initializeAgentDisplayNames(displayNames) + + const { warnings } = initializeAgentNameAliases( + displayNames, + Object.keys(agentResult), + ) + + for (const warning of warnings) { + log(warning) + } + + const canonicalToRegistered = getCanonicalToRegisteredMap() + const agentMap = config.agent as Record | undefined + + for (const [canonical, registered] of canonicalToRegistered) { + if (agentResult[canonical] !== undefined) { + agentResult[registered] = agentResult[canonical] + delete agentResult[canonical] + } + + if (agentMap && agentMap[canonical] !== undefined) { + agentMap[registered] = agentMap[canonical] + delete agentMap[canonical] + } + + if (config.default_agent === canonical) { + config.default_agent = registered + } + } +} diff --git a/src/plugin-handlers/config-handler.test.ts b/src/plugin-handlers/config-handler.test.ts index bca4ce4d1f..64460e3946 100644 --- a/src/plugin-handlers/config-handler.test.ts +++ b/src/plugin-handlers/config-handler.test.ts @@ -1053,9 +1053,87 @@ describe("per-agent todowrite/todoread deny when task_system enabled", () => { //#when await handler(config) - //#then - const agentResult = config.agent as Record }> - expect(agentResult.sisyphus?.permission?.todowrite).toBeUndefined() - expect(agentResult.sisyphus?.permission?.todoread).toBeUndefined() + //#then + const agentResult = config.agent as Record }> + expect(agentResult.sisyphus?.permission?.todowrite).toBeUndefined() + expect(agentResult.sisyphus?.permission?.todoread).toBeUndefined() + }) +}) + +describe("Agent display name re-keying", () => { + test("re-keys agents in config.agent when agent_display_names is provided", async () => { + //#given + const createBuiltinAgentsMock = agents.createBuiltinAgents as unknown as { + mockResolvedValue: (value: Record) => void + } + createBuiltinAgentsMock.mockResolvedValue({ + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + oracle: { name: "oracle", prompt: "test", mode: "subagent" }, + }) + + const pluginConfig: OhMyOpenCodeConfig = { + agent_display_names: { + sisyphus: "MyCustomSisyphus", + oracle: "MyOracle", + }, + } + const config: Record = { + model: "anthropic/claude-opus-4-6", + agent: { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + oracle: { name: "oracle", prompt: "test", mode: "subagent" }, + }, + } + const handler = createConfigHandler({ + ctx: { directory: "/tmp" }, + pluginConfig, + modelCacheState: { + anthropicContext1MEnabled: false, + modelContextLimitsCache: new Map(), + }, + }) + + //#when + await handler(config) + + //#then - agents should be re-keyed by display names + const agentConfig = config.agent as Record + expect(agentConfig["MyCustomSisyphus"]).toBeDefined() + expect(agentConfig["MyOracle"]).toBeDefined() + expect(agentConfig.sisyphus).toBeUndefined() + expect(agentConfig.oracle).toBeUndefined() + }) + + test("does not re-key when agent_display_names is undefined", async () => { + //#given + const createBuiltinAgentsMock = agents.createBuiltinAgents as unknown as { + mockResolvedValue: (value: Record) => void + } + createBuiltinAgentsMock.mockResolvedValue({ + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + }) + + const pluginConfig: OhMyOpenCodeConfig = {} + const config: Record = { + model: "anthropic/claude-opus-4-6", + agent: { + sisyphus: { name: "sisyphus", prompt: "test", mode: "primary" }, + }, + } + const handler = createConfigHandler({ + ctx: { directory: "/tmp" }, + pluginConfig, + modelCacheState: { + anthropicContext1MEnabled: false, + modelContextLimitsCache: new Map(), + }, + }) + + //#when + await handler(config) + + //#then - agents should keep original keys + const agentConfig = config.agent as Record + expect(agentConfig.sisyphus).toBeDefined() }) }) diff --git a/src/plugin-handlers/config-handler.ts b/src/plugin-handlers/config-handler.ts index 5eb7f242b8..91e495b1a8 100644 --- a/src/plugin-handlers/config-handler.ts +++ b/src/plugin-handlers/config-handler.ts @@ -7,6 +7,7 @@ import { applyMcpConfig } from "./mcp-config-handler"; import { applyProviderConfig } from "./provider-config-handler"; import { loadPluginComponents } from "./plugin-components-loader"; import { applyToolConfig } from "./tool-config-handler"; +import { rekeyAgentsByDisplayNames } from "./agent-display-name-rekeyer"; export { resolveCategoryConfig } from "./category-config-resolver"; @@ -32,6 +33,7 @@ export function createConfigHandler(deps: ConfigHandlerDeps) { }); applyToolConfig({ config, pluginConfig, agentResult }); + rekeyAgentsByDisplayNames({ config, agentResult, displayNames: pluginConfig.agent_display_names }); await applyMcpConfig({ config, pluginConfig, pluginComponents }); await applyCommandConfig({ config, pluginConfig, pluginComponents }); diff --git a/src/plugin-handlers/index.ts b/src/plugin-handlers/index.ts index fa9bde977f..28b57127aa 100644 --- a/src/plugin-handlers/index.ts +++ b/src/plugin-handlers/index.ts @@ -8,3 +8,4 @@ export * from "./plugin-components-loader"; export * from "./category-config-resolver"; export * from "./prometheus-agent-config-builder"; export * from "./agent-priority-order"; +export * from "./agent-display-name-rekeyer"; diff --git a/src/plugin/tool-registry.ts b/src/plugin/tool-registry.ts index 7236ddc489..c38ed32ff8 100644 --- a/src/plugin/tool-registry.ts +++ b/src/plugin/tool-registry.ts @@ -28,7 +28,7 @@ import { } from "../tools" import { getMainSessionID } from "../features/claude-code-session-state" import { filterDisabledTools } from "../shared/disabled-tools" -import { log } from "../shared" +import { toCanonical, log } from "../shared" import type { Managers } from "../create-managers" import type { SkillContext } from "./skill-context" @@ -51,7 +51,7 @@ export function createToolRegistry(args: { const callOmoAgent = createCallOmoAgent(ctx, managers.backgroundManager) const isMultimodalLookerEnabled = !(pluginConfig.disabled_agents ?? []).some( - (agent) => agent.toLowerCase() === "multimodal-looker", + (agent) => toCanonical(agent).toLowerCase() === "multimodal-looker", ) const lookAt = isMultimodalLookerEnabled ? createLookAt(ctx) : null diff --git a/src/shared/agent-config-integration.test.ts b/src/shared/agent-config-integration.test.ts index e663da9fc5..cbe347e205 100644 --- a/src/shared/agent-config-integration.test.ts +++ b/src/shared/agent-config-integration.test.ts @@ -1,6 +1,6 @@ -import { describe, test, expect } from "bun:test" +import { describe, test, expect, afterEach, beforeEach } from "bun:test" import { migrateAgentNames } from "./migration" -import { getAgentDisplayName } from "./agent-display-names" +import { getAgentDisplayName, initializeAgentDisplayNames, resetAgentDisplayNames } from "./agent-display-names" import { AGENT_MODEL_REQUIREMENTS } from "./model-requirements" describe("Agent Config Integration", () => { @@ -84,6 +84,10 @@ describe("Agent Config Integration", () => { }) describe("Display name resolution", () => { + beforeEach(() => { + resetAgentDisplayNames() + }) + test("returns correct display names for all builtin agents", () => { // given - lowercase config keys const agents = ["sisyphus", "atlas", "prometheus", "metis", "momus", "oracle", "librarian", "explore", "multimodal-looker"] @@ -169,6 +173,10 @@ describe("Agent Config Integration", () => { }) describe("End-to-end config flow", () => { + beforeEach(() => { + resetAgentDisplayNames() + }) + test("old config migrates and displays correctly", () => { // given - old format config const oldConfig = { @@ -217,8 +225,72 @@ describe("Agent Config Integration", () => { const atlasDisplay = getAgentDisplayName("atlas") // then - display names are correct - expect(sisyphusDisplay).toBe("Sisyphus (Ultraworker)") - expect(atlasDisplay).toBe("Atlas (Plan Execution Orchestrator)") + expect(sisyphusDisplay).toBe("Sisyphus (Ultraworker)") + expect(atlasDisplay).toBe("Atlas (Plan Execution Orchestrator)") + }) + }) + + describe("Plugin initialization flow", () => { + afterEach(() => { + resetAgentDisplayNames() + }) + + test("initializeAgentDisplayNames applies user overrides", () => { + // given - user-provided display name overrides + const userOverrides = { + sisyphus: "My Custom Sisyphus", + oracle: "My Custom Oracle", + } + + // when - initialization is called + initializeAgentDisplayNames(userOverrides) + + // then - overrides are applied + expect(getAgentDisplayName("sisyphus")).toBe("My Custom Sisyphus") + expect(getAgentDisplayName("oracle")).toBe("My Custom Oracle") + + // then - non-overridden agents still use defaults + expect(getAgentDisplayName("atlas")).toBe("Atlas (Plan Execution Orchestrator)") + }) + + test("initializeAgentDisplayNames handles empty overrides", () => { + // given - empty overrides + const emptyOverrides = {} + + // when - initialization is called + initializeAgentDisplayNames(emptyOverrides) + + // then - defaults are still used + expect(getAgentDisplayName("sisyphus")).toBe("Sisyphus (Ultraworker)") + expect(getAgentDisplayName("atlas")).toBe("Atlas (Plan Execution Orchestrator)") + }) + + test("initializeAgentDisplayNames case-insensitive override matching", () => { + // given - overrides with different case + const overrides = { + Sisyphus: "Custom Sisyphus", + ATLAS: "Custom Atlas", + } + + // when - initialization is called + initializeAgentDisplayNames(overrides) + + // then - case-insensitive matching works + expect(getAgentDisplayName("sisyphus")).toBe("Custom Sisyphus") + expect(getAgentDisplayName("atlas")).toBe("Custom Atlas") + expect(getAgentDisplayName("SISYPHUS")).toBe("Custom Sisyphus") + }) + + test("resetAgentDisplayNames clears overrides", () => { + // given - initialized overrides + initializeAgentDisplayNames({ sisyphus: "Custom" }) + expect(getAgentDisplayName("sisyphus")).toBe("Custom") + + // when - reset is called + resetAgentDisplayNames() + + // then - defaults are restored + expect(getAgentDisplayName("sisyphus")).toBe("Sisyphus (Ultraworker)") }) }) }) diff --git a/src/shared/agent-display-names.test.ts b/src/shared/agent-display-names.test.ts index 628de8b8c4..89d0c2e924 100644 --- a/src/shared/agent-display-names.test.ts +++ b/src/shared/agent-display-names.test.ts @@ -1,5 +1,5 @@ -import { describe, it, expect } from "bun:test" -import { AGENT_DISPLAY_NAMES, getAgentDisplayName } from "./agent-display-names" +import { describe, it, expect, beforeEach } from "bun:test" +import { AGENT_DISPLAY_NAMES, getAgentDisplayName, initializeAgentDisplayNames, resetAgentDisplayNames } from "./agent-display-names" describe("getAgentDisplayName", () => { it("returns display name for lowercase config key (new format)", () => { @@ -155,4 +155,77 @@ describe("AGENT_DISPLAY_NAMES", () => { // then contains all expected mappings expect(AGENT_DISPLAY_NAMES).toEqual(expectedMappings) }) -}) \ No newline at end of file +}) + +describe("config-aware display names", () => { + beforeEach(() => { + resetAgentDisplayNames() + }) + + it("returns override when initialized with custom name", () => { + // given override for sisyphus + initializeAgentDisplayNames({ sisyphus: "Builder" }) + + // when getAgentDisplayName called + const result = getAgentDisplayName("sisyphus") + + // then returns custom name + expect(result).toBe("Builder") + }) + + it("preserves defaults for non-overridden agents", () => { + // given override for sisyphus only + initializeAgentDisplayNames({ sisyphus: "Builder" }) + + // when getAgentDisplayName called for atlas + const result = getAgentDisplayName("atlas") + + // then returns default name + expect(result).toBe("Atlas (Plan Execution Orchestrator)") + }) + + it("handles case-insensitive override keys", () => { + // given override with uppercase key + initializeAgentDisplayNames({ Sisyphus: "Builder" }) + + // when getAgentDisplayName called with lowercase + const result = getAgentDisplayName("sisyphus") + + // then returns custom name + expect(result).toBe("Builder") + }) + + it("allows override for unknown agents", () => { + // given override for custom agent + initializeAgentDisplayNames({ "my-agent": "My Agent" }) + + // when getAgentDisplayName called + const result = getAgentDisplayName("my-agent") + + // then returns custom name + expect(result).toBe("My Agent") + }) + + it("preserves all defaults with empty overrides", () => { + // given empty overrides + initializeAgentDisplayNames({}) + + // when getAgentDisplayName called for sisyphus + const result = getAgentDisplayName("sisyphus") + + // then returns default name + expect(result).toBe("Sisyphus (Ultraworker)") + }) + + it("clears overrides after reset", () => { + // given override + initializeAgentDisplayNames({ sisyphus: "Builder" }) + + // when resetAgentDisplayNames called + resetAgentDisplayNames() + + // then getAgentDisplayName returns default + const result = getAgentDisplayName("sisyphus") + expect(result).toBe("Sisyphus (Ultraworker)") + }) +}) diff --git a/src/shared/agent-display-names.ts b/src/shared/agent-display-names.ts index 82c08b2c8c..82c683ce3b 100644 --- a/src/shared/agent-display-names.ts +++ b/src/shared/agent-display-names.ts @@ -16,22 +16,43 @@ export const AGENT_DISPLAY_NAMES: Record = { "multimodal-looker": "multimodal-looker", } +let userOverrides: Record = {} + +/** + * Initialize agent display name overrides from configuration. + * @param overrides - Map of agent config keys to custom display names + */ +export function initializeAgentDisplayNames(overrides: Record): void { + userOverrides = overrides +} + +/** + * Reset agent display name overrides to empty state. + */ +export function resetAgentDisplayNames(): void { + userOverrides = {} +} + /** * Get display name for an agent config key. * Uses case-insensitive lookup for backward compatibility. * Returns original key if not found. + * @param configKey - Agent config key (e.g., "sisyphus", "atlas") + * @returns Display name with UI/log suffix, or original key if not found */ export function getAgentDisplayName(configKey: string): string { - // Try exact match first + const lowerKey = configKey.toLowerCase() + + for (const [k, v] of Object.entries(userOverrides)) { + if (k.toLowerCase() === lowerKey) return v + } + const exactMatch = AGENT_DISPLAY_NAMES[configKey] if (exactMatch !== undefined) return exactMatch - // Fall back to case-insensitive search - const lowerKey = configKey.toLowerCase() for (const [k, v] of Object.entries(AGENT_DISPLAY_NAMES)) { if (k.toLowerCase() === lowerKey) return v } - // Unknown agent: return original key return configKey } \ No newline at end of file diff --git a/src/shared/agent-name-aliases.test.ts b/src/shared/agent-name-aliases.test.ts new file mode 100644 index 0000000000..e4808309a2 --- /dev/null +++ b/src/shared/agent-name-aliases.test.ts @@ -0,0 +1,234 @@ +import { describe, it, expect, beforeEach } from "bun:test" +import { + initializeAgentNameAliases, + toCanonical, + toRegistered, + getCanonicalToRegisteredMap, + hasAliases, + resetAgentNameAliases, +} from "./agent-name-aliases" + +const ALL_CANONICAL = [ + "sisyphus", + "atlas", + "prometheus", + "sisyphus-junior", + "metis", + "momus", + "oracle", + "librarian", + "explore", + "multimodal-looker", + "hephaestus", +] + +describe("agent-name-aliases", () => { + beforeEach(() => { + resetAgentNameAliases() + }) + + describe("round-trip", () => { + it("toRegistered(toCanonical(alias)) returns alias", () => { + //#given sisyphus mapped to Bob + initializeAgentNameAliases({ sisyphus: "Bob" }, ALL_CANONICAL) + + //#when round-tripping alias through canonical + const canonical = toCanonical("Bob") + const registered = toRegistered(canonical) + + //#then returns the alias + expect(canonical).toBe("sisyphus") + expect(registered).toBe("Bob") + }) + + it("toCanonical(toRegistered(canonical)) returns canonical", () => { + //#given sisyphus mapped to Bob + initializeAgentNameAliases({ sisyphus: "Bob" }, ALL_CANONICAL) + + //#when round-tripping canonical through registered + const registered = toRegistered("sisyphus") + const canonical = toCanonical(registered) + + //#then returns the canonical + expect(registered).toBe("Bob") + expect(canonical).toBe("sisyphus") + }) + }) + + describe("identity (no mapping)", () => { + it("toCanonical returns canonical as-is when no alias exists", () => { + //#given no aliases initialized + initializeAgentNameAliases(undefined, ALL_CANONICAL) + + //#when toCanonical called with canonical name + const result = toCanonical("sisyphus") + + //#then returns same name + expect(result).toBe("sisyphus") + }) + + it("toRegistered returns unknown name as-is when no mapping", () => { + //#given no aliases initialized + initializeAgentNameAliases(undefined, ALL_CANONICAL) + + //#when toRegistered called with unknown name + const result = toRegistered("unknown") + + //#then returns same name + expect(result).toBe("unknown") + }) + }) + + describe("case-insensitive", () => { + it("toCanonical resolves alias regardless of case", () => { + //#given sisyphus mapped to Bob + initializeAgentNameAliases({ sisyphus: "Bob" }, ALL_CANONICAL) + + //#when toCanonical called with uppercase alias + const result = toCanonical("BOB") + + //#then returns canonical name + expect(result).toBe("sisyphus") + }) + + it("toRegistered resolves canonical regardless of case", () => { + //#given sisyphus mapped to Bob + initializeAgentNameAliases({ sisyphus: "Bob" }, ALL_CANONICAL) + + //#when toRegistered called with uppercase canonical + const result = toRegistered("SISYPHUS") + + //#then returns registered alias + expect(result).toBe("Bob") + }) + }) + + describe("collision detection", () => { + it("warns on duplicate alias values", () => { + //#given two agents mapped to the same alias + const config = { sisyphus: "Worker", hephaestus: "Worker" } + + //#when initializing + const { warnings } = initializeAgentNameAliases(config, ALL_CANONICAL) + + //#then returns warning about collision + expect(warnings.length).toBeGreaterThan(0) + expect(warnings[0]).toContain("Worker") + }) + + it("warns when alias collides with existing canonical name", () => { + //#given sisyphus mapped to "oracle" (which is a canonical name) + const config = { sisyphus: "oracle" } + + //#when initializing + const { warnings } = initializeAgentNameAliases(config, ALL_CANONICAL) + + //#then returns warning about canonical collision + expect(warnings.length).toBeGreaterThan(0) + expect(warnings[0]).toContain("oracle") + }) + }) + + describe("non-existent agent", () => { + it("warns when canonical name does not exist", () => { + //#given alias for non-existent agent + const config = { nonexistent: "Ghost" } + + //#when initializing + const { warnings } = initializeAgentNameAliases(config, ALL_CANONICAL) + + //#then returns warning about unknown canonical + expect(warnings.length).toBeGreaterThan(0) + expect(warnings[0]).toContain("nonexistent") + }) + }) + + describe("empty/undefined config", () => { + it("hasAliases returns false with undefined config", () => { + //#given undefined config + initializeAgentNameAliases(undefined, ALL_CANONICAL) + + //#when checking hasAliases + //#then returns false + expect(hasAliases()).toBe(false) + }) + + it("hasAliases returns false with empty config", () => { + //#given empty config + initializeAgentNameAliases({}, ALL_CANONICAL) + + //#when checking hasAliases + //#then returns false + expect(hasAliases()).toBe(false) + }) + }) + + describe("multiple aliases", () => { + it("all resolve correctly", () => { + //#given multiple aliases + const config = { + sisyphus: "Bob", + atlas: "Alice", + oracle: "Oscar", + } + initializeAgentNameAliases(config, ALL_CANONICAL) + + //#when resolving each alias + //#then all resolve to correct canonical + expect(toCanonical("Bob")).toBe("sisyphus") + expect(toCanonical("Alice")).toBe("atlas") + expect(toCanonical("Oscar")).toBe("oracle") + + //#then all canonical names resolve to aliases + expect(toRegistered("sisyphus")).toBe("Bob") + expect(toRegistered("atlas")).toBe("Alice") + expect(toRegistered("oracle")).toBe("Oscar") + + //#then unaliased agents remain unchanged + expect(toRegistered("prometheus")).toBe("prometheus") + }) + }) + + describe("getCanonicalToRegisteredMap", () => { + it("returns map of all aliases", () => { + //#given aliases configured + initializeAgentNameAliases({ sisyphus: "Bob", atlas: "Alice" }, ALL_CANONICAL) + + //#when getting map + const map = getCanonicalToRegisteredMap() + + //#then map contains all aliases + expect(map.size).toBe(2) + expect(map.get("sisyphus")).toBe("Bob") + expect(map.get("atlas")).toBe("Alice") + }) + + it("returns empty map when no aliases", () => { + //#given no aliases + initializeAgentNameAliases(undefined, ALL_CANONICAL) + + //#when getting map + const map = getCanonicalToRegisteredMap() + + //#then map is empty + expect(map.size).toBe(0) + }) + }) + + describe("reset", () => { + it("clears all state", () => { + //#given aliases configured + initializeAgentNameAliases({ sisyphus: "Bob" }, ALL_CANONICAL) + expect(hasAliases()).toBe(true) + + //#when reset called + resetAgentNameAliases() + + //#then all state is cleared + expect(hasAliases()).toBe(false) + expect(toCanonical("Bob")).toBe("bob") + expect(toRegistered("sisyphus")).toBe("sisyphus") + expect(getCanonicalToRegisteredMap().size).toBe(0) + }) + }) +}) diff --git a/src/shared/agent-name-aliases.ts b/src/shared/agent-name-aliases.ts new file mode 100644 index 0000000000..f04db38bd2 --- /dev/null +++ b/src/shared/agent-name-aliases.ts @@ -0,0 +1,76 @@ +// Bidirectional alias mapping for custom agent names. +// Maps user-defined display names (aliases) back to canonical agent names for SDK routing, +// and canonical names forward to their registered aliases for UI display. + +let aliasToCanonical = new Map() +let canonicalToAlias = new Map() + +export function initializeAgentNameAliases( + displayNames: Record | undefined, + allCanonicalNames: string[], +): { warnings: string[] } { + const warnings: string[] = [] + aliasToCanonical = new Map() + canonicalToAlias = new Map() + + if (!displayNames) return { warnings } + + const canonicalSet = new Set(allCanonicalNames.map((n) => n.toLowerCase())) + const seenAliases = new Map() + + for (const [canonical, alias] of Object.entries(displayNames)) { + const canonicalLower = canonical.toLowerCase() + const aliasLower = alias.toLowerCase() + + if (!canonicalSet.has(canonicalLower)) { + warnings.push( + `Agent name alias: "${canonical}" is not a known agent name, skipping`, + ) + continue + } + + const existingOwner = seenAliases.get(aliasLower) + if (existingOwner) { + warnings.push( + `Agent name alias: "${alias}" is used by both "${existingOwner}" and "${canonical}", skipping duplicate`, + ) + continue + } + + if (canonicalSet.has(aliasLower) && aliasLower !== canonicalLower) { + warnings.push( + `Agent name alias: "${alias}" for "${canonical}" collides with existing canonical name "${alias}"`, + ) + continue + } + + seenAliases.set(aliasLower, canonical) + aliasToCanonical.set(aliasLower, canonicalLower) + canonicalToAlias.set(canonicalLower, alias) + } + + return { warnings } +} + +export function toCanonical(name: string): string { + const lower = name.toLowerCase() + return aliasToCanonical.get(lower) ?? lower +} + +export function toRegistered(name: string): string { + const lower = name.toLowerCase() + return canonicalToAlias.get(lower) ?? name +} + +export function getCanonicalToRegisteredMap(): ReadonlyMap { + return canonicalToAlias +} + +export function hasAliases(): boolean { + return canonicalToAlias.size > 0 +} + +export function resetAgentNameAliases(): void { + aliasToCanonical = new Map() + canonicalToAlias = new Map() +} diff --git a/src/shared/agent-tool-restrictions.test.ts b/src/shared/agent-tool-restrictions.test.ts new file mode 100644 index 0000000000..87ca31dbb2 --- /dev/null +++ b/src/shared/agent-tool-restrictions.test.ts @@ -0,0 +1,114 @@ +import { describe, it, expect, beforeEach } from "bun:test" +import { + getAgentToolRestrictions, + hasAgentToolRestrictions, +} from "./agent-tool-restrictions" +import { + initializeAgentNameAliases, + resetAgentNameAliases, +} from "./agent-name-aliases" + +const ALL_CANONICAL = [ + "sisyphus", + "atlas", + "prometheus", + "sisyphus-junior", + "metis", + "momus", + "oracle", + "librarian", + "explore", + "multimodal-looker", + "hephaestus", +] + +describe("agent-tool-restrictions", () => { + beforeEach(() => { + resetAgentNameAliases() + }) + + describe("getAgentToolRestrictions", () => { + it("returns restrictions for canonical agent name", () => { + //#given explore agent has restrictions + const restrictions = getAgentToolRestrictions("explore") + + //#then returns the exploration denylist + expect(restrictions).toEqual({ + write: false, + edit: false, + task: false, + call_omo_agent: false, + }) + }) + + it("returns restrictions for custom alias when mapped", () => { + //#given explore mapped to Scout + initializeAgentNameAliases({ explore: "Scout" }, ALL_CANONICAL) + + //#when getting restrictions for Scout + const restrictions = getAgentToolRestrictions("Scout") + + //#then returns explore's restrictions + expect(restrictions).toEqual({ + write: false, + edit: false, + task: false, + call_omo_agent: false, + }) + }) + + it("returns empty object for unknown agent", () => { + //#given unknown agent name + const restrictions = getAgentToolRestrictions("unknown-agent") + + //#then returns empty object + expect(restrictions).toEqual({}) + }) + + it("handles case-insensitive lookup for canonical names", () => { + //#given explore agent + const restrictionsLower = getAgentToolRestrictions("explore") + const restrictionsUpper = getAgentToolRestrictions("EXPLORE") + + //#then both return same restrictions + expect(restrictionsLower).toEqual(restrictionsUpper) + }) + }) + + describe("hasAgentToolRestrictions", () => { + it("returns true for agent with restrictions", () => { + //#given explore agent has restrictions + const has = hasAgentToolRestrictions("explore") + + //#then returns true + expect(has).toBe(true) + }) + + it("returns true for custom alias with restrictions", () => { + //#given explore mapped to Scout + initializeAgentNameAliases({ explore: "Scout" }, ALL_CANONICAL) + + //#when checking Scout + const has = hasAgentToolRestrictions("Scout") + + //#then returns true + expect(has).toBe(true) + }) + + it("returns false for unknown agent", () => { + //#given unknown agent name + const has = hasAgentToolRestrictions("unknown-agent") + + //#then returns false + expect(has).toBe(false) + }) + + it("returns false for agent without restrictions", () => { + //#given sisyphus agent (no restrictions defined) + const has = hasAgentToolRestrictions("sisyphus") + + //#then returns false + expect(has).toBe(false) + }) + }) +}) diff --git a/src/shared/agent-tool-restrictions.ts b/src/shared/agent-tool-restrictions.ts index 865251d6fc..d7145c3dbd 100644 --- a/src/shared/agent-tool-restrictions.ts +++ b/src/shared/agent-tool-restrictions.ts @@ -4,6 +4,8 @@ * true = tool allowed, false = tool denied. */ +import { toCanonical } from "./agent-name-aliases" + const EXPLORATION_AGENT_DENYLIST: Record = { write: false, edit: false, @@ -45,13 +47,15 @@ const AGENT_RESTRICTIONS: Record> = { } export function getAgentToolRestrictions(agentName: string): Record { - return AGENT_RESTRICTIONS[agentName] - ?? Object.entries(AGENT_RESTRICTIONS).find(([key]) => key.toLowerCase() === agentName.toLowerCase())?.[1] + const canonical = toCanonical(agentName) + return AGENT_RESTRICTIONS[canonical] + ?? Object.entries(AGENT_RESTRICTIONS).find(([key]) => key.toLowerCase() === canonical.toLowerCase())?.[1] ?? {} } export function hasAgentToolRestrictions(agentName: string): boolean { - const restrictions = AGENT_RESTRICTIONS[agentName] - ?? Object.entries(AGENT_RESTRICTIONS).find(([key]) => key.toLowerCase() === agentName.toLowerCase())?.[1] + const canonical = toCanonical(agentName) + const restrictions = AGENT_RESTRICTIONS[canonical] + ?? Object.entries(AGENT_RESTRICTIONS).find(([key]) => key.toLowerCase() === canonical.toLowerCase())?.[1] return restrictions !== undefined && Object.keys(restrictions).length > 0 } diff --git a/src/shared/index.ts b/src/shared/index.ts index 07d8bd8605..63fe357bcc 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -49,3 +49,4 @@ export * from "./port-utils" export * from "./git-worktree" export * from "./safe-create-hook" export * from "./truncate-description" +export * from "./agent-name-aliases" diff --git a/src/shared/session-utils.test.ts b/src/shared/session-utils.test.ts new file mode 100644 index 0000000000..ae49bed0bd --- /dev/null +++ b/src/shared/session-utils.test.ts @@ -0,0 +1,96 @@ +import { describe, test, expect, beforeEach, afterEach, spyOn, mock } from "bun:test" +import * as fs from "node:fs" +import { initializeAgentNameAliases, resetAgentNameAliases } from "./agent-name-aliases" + +//#given mocked fs module to control getMessageDir behavior +mock.module("node:fs", () => ({ + existsSync: mock(() => true), + readdirSync: mock(() => []), +})) + +import * as sessionUtils from "./session-utils" + +describe("session-utils", () => { + //#given mocked hook-message-injector dependencies + //#when testing isCallerOrchestrator + //#then all tests should pass with proper cleanup + + let injectorSpy: ReturnType | null = null + + beforeEach(() => { + resetAgentNameAliases() + }) + + afterEach(() => { + //#then restore all mocks to prevent test pollution + if (injectorSpy) { + injectorSpy.mockRestore() + injectorSpy = null + } + }) + + test("returns false when sessionID is undefined", () => { + //#when sessionID is undefined + //#then isCallerOrchestrator returns false + expect(sessionUtils.isCallerOrchestrator(undefined)).toBe(false) + }) + + test("returns false when no message directory exists", () => { + //#given getMessageDir returns null (no message directory found) + //#when calling isCallerOrchestrator with a sessionID + //#then it returns false + const injectorModule = require("../features/hook-message-injector") + injectorSpy = spyOn(injectorModule, "findNearestMessageWithFields").mockReturnValue(null) + expect(sessionUtils.isCallerOrchestrator("ses_test")).toBe(false) + }) + + test("returns true when nearest message agent is atlas", () => { + //#given findNearestMessageWithFields returns an object with agent: "atlas" + const injectorModule = require("../features/hook-message-injector") + injectorSpy = spyOn(injectorModule, "findNearestMessageWithFields").mockReturnValue({ agent: "atlas" }) + //#when calling isCallerOrchestrator + //#then it returns true + expect(sessionUtils.isCallerOrchestrator("ses_test")).toBe(true) + }) + + test("returns true for Atlas with different casing", () => { + //#given findNearestMessageWithFields returns an object with agent: "Atlas" (uppercase) + const injectorModule = require("../features/hook-message-injector") + injectorSpy = spyOn(injectorModule, "findNearestMessageWithFields").mockReturnValue({ agent: "Atlas" }) + //#when calling isCallerOrchestrator + //#then it returns true (case-insensitive comparison) + expect(sessionUtils.isCallerOrchestrator("ses_test")).toBe(true) + }) + + test("returns false when nearest message agent is not atlas", () => { + //#given findNearestMessageWithFields returns an object with agent: "sisyphus" + const injectorModule = require("../features/hook-message-injector") + injectorSpy = spyOn(injectorModule, "findNearestMessageWithFields").mockReturnValue({ agent: "sisyphus" }) + //#when calling isCallerOrchestrator + //#then it returns false + expect(sessionUtils.isCallerOrchestrator("ses_test")).toBe(false) + }) + + test("canonicalizes renamed atlas agent when checking orchestrator", () => { + //#given agent name aliases are initialized with atlas -> "Master Orchestrator" + initializeAgentNameAliases( + { atlas: "Master Orchestrator" }, + ["atlas", "sisyphus", "prometheus"], + ) + //#and findNearestMessageWithFields returns the renamed agent name + const injectorModule = require("../features/hook-message-injector") + injectorSpy = spyOn(injectorModule, "findNearestMessageWithFields").mockReturnValue({ agent: "Master Orchestrator" }) + //#when calling isCallerOrchestrator + //#then it canonicalizes the name and returns true + expect(sessionUtils.isCallerOrchestrator("ses_test")).toBe(true) + }) + + test("returns false when agent field is missing from nearest message", () => { + //#given findNearestMessageWithFields returns an object without agent field + const injectorModule = require("../features/hook-message-injector") + injectorSpy = spyOn(injectorModule, "findNearestMessageWithFields").mockReturnValue({}) + //#when calling isCallerOrchestrator + //#then it returns false (agent defaults to empty string, not "atlas") + expect(sessionUtils.isCallerOrchestrator("ses_test")).toBe(false) + }) +}) diff --git a/src/shared/session-utils.ts b/src/shared/session-utils.ts index eb98397437..cd10451171 100644 --- a/src/shared/session-utils.ts +++ b/src/shared/session-utils.ts @@ -3,6 +3,7 @@ import * as os from "node:os" import { existsSync, readdirSync } from "node:fs" import { join } from "node:path" import { findNearestMessageWithFields, MESSAGE_STORAGE } from "../features/hook-message-injector" +import { toCanonical } from "./agent-name-aliases" export function getMessageDir(sessionID: string): string | null { if (!existsSync(MESSAGE_STORAGE)) return null @@ -23,5 +24,5 @@ export function isCallerOrchestrator(sessionID?: string): boolean { const messageDir = getMessageDir(sessionID) if (!messageDir) return false const nearest = findNearestMessageWithFields(messageDir) - return nearest?.agent?.toLowerCase() === "atlas" + return toCanonical(nearest?.agent ?? "").toLowerCase() === "atlas" } diff --git a/src/tools/background-task/create-background-output.ts b/src/tools/background-task/create-background-output.ts index aaca3cb165..48fe5436e2 100644 --- a/src/tools/background-task/create-background-output.ts +++ b/src/tools/background-task/create-background-output.ts @@ -8,6 +8,7 @@ import { delay } from "./delay" import { formatFullSession } from "./full-session-format" import { formatTaskResult } from "./task-result-format" import { formatTaskStatus } from "./task-status-format" +import { toCanonical } from "../../shared" const SISYPHUS_JUNIOR_AGENT = "sisyphus-junior" @@ -27,7 +28,7 @@ function resolveToolCallID(ctx: ToolContextWithMetadata): string | undefined { } function formatResolvedTitle(task: BackgroundTask): string { - const label = task.agent === SISYPHUS_JUNIOR_AGENT && task.category ? task.category : task.agent + const label = toCanonical(task.agent) === SISYPHUS_JUNIOR_AGENT && task.category ? task.category : task.agent return `${label} - ${task.description}` } diff --git a/src/tools/background-task/modules/background-output.ts b/src/tools/background-task/modules/background-output.ts index 70f522843d..8ef7867cf6 100644 --- a/src/tools/background-task/modules/background-output.ts +++ b/src/tools/background-task/modules/background-output.ts @@ -7,6 +7,7 @@ import { delay } from "./utils" import { storeToolMetadata } from "../../../features/tool-metadata-store" import type { BackgroundTask } from "../../../features/background-agent" import type { ToolContextWithMetadata } from "./utils" +import { toCanonical } from "../../../shared" const SISYPHUS_JUNIOR_AGENT = "sisyphus-junior" @@ -30,7 +31,7 @@ function resolveToolCallID(ctx: ToolContextWithCallId): string | undefined { } function formatResolvedTitle(task: BackgroundTask): string { - const label = task.agent === SISYPHUS_JUNIOR_AGENT && task.category + const label = toCanonical(task.agent) === SISYPHUS_JUNIOR_AGENT && task.category ? task.category : task.agent return `${label} - ${task.description}` diff --git a/src/tools/call-omo-agent/sync-executor.ts b/src/tools/call-omo-agent/sync-executor.ts index f64310fbda..95b2fc3a30 100644 --- a/src/tools/call-omo-agent/sync-executor.ts +++ b/src/tools/call-omo-agent/sync-executor.ts @@ -1,6 +1,6 @@ import type { CallOmoAgentArgs } from "./types" import type { PluginInput } from "@opencode-ai/plugin" -import { log } from "../../shared" +import { log, toRegistered } from "../../shared" import { getAgentToolRestrictions } from "../../shared" import { createOrGetSession } from "./session-creator" import { waitForCompletion } from "./completion-poller" @@ -27,18 +27,18 @@ export async function executeSync( log(`[call_omo_agent] Sending prompt to session ${sessionID}`) log(`[call_omo_agent] Prompt text:`, args.prompt.substring(0, 100)) - try { - await (ctx.client.session as any).promptAsync({ - path: { id: sessionID }, - body: { - agent: args.subagent_type, - tools: { - ...getAgentToolRestrictions(args.subagent_type), - task: false, - }, - parts: [{ type: "text", text: args.prompt }], - }, - }) + try { + await (ctx.client.session as any).promptAsync({ + path: { id: sessionID }, + body: { + agent: toRegistered(args.subagent_type), + tools: { + ...getAgentToolRestrictions(args.subagent_type), + task: false, + }, + parts: [{ type: "text", text: args.prompt }], + }, + }) } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error) log(`[call_omo_agent] Prompt error:`, errorMessage) diff --git a/src/tools/call-omo-agent/tools.ts b/src/tools/call-omo-agent/tools.ts index dbcfcf970e..7e07bb9b98 100644 --- a/src/tools/call-omo-agent/tools.ts +++ b/src/tools/call-omo-agent/tools.ts @@ -2,7 +2,7 @@ import { tool, type PluginInput, type ToolDefinition } from "@opencode-ai/plugin import { ALLOWED_AGENTS, CALL_OMO_AGENT_DESCRIPTION } from "./constants" import type { AllowedAgentType, CallOmoAgentArgs, ToolContextWithMetadata } from "./types" import type { BackgroundManager } from "../../features/background-agent" -import { log } from "../../shared" +import { log, toCanonical } from "../../shared" import { executeBackground } from "./background-executor" import { executeSync } from "./sync-executor" @@ -32,17 +32,13 @@ export function createCallOmoAgent( const toolCtx = toolContext as ToolContextWithMetadata log(`[call_omo_agent] Starting with agent: ${args.subagent_type}, background: ${args.run_in_background}`) - // Case-insensitive agent validation - allows "Explore", "EXPLORE", "explore" etc. - if ( - !ALLOWED_AGENTS.some( - (name) => name.toLowerCase() === args.subagent_type.toLowerCase(), - ) - ) { - return `Error: Invalid agent type "${args.subagent_type}". Only ${ALLOWED_AGENTS.join(", ")} are allowed.` - } + // Case-insensitive agent validation - allows "Explore", "EXPLORE", "explore" etc. + const canonicalAgent = toCanonical(args.subagent_type) + if (!ALLOWED_AGENTS.includes(canonicalAgent as AllowedAgentType)) { + return `Error: Invalid agent type "${args.subagent_type}". Only ${ALLOWED_AGENTS.join(", ")} are allowed.` + } - const normalizedAgent = args.subagent_type.toLowerCase() as AllowedAgentType - args = { ...args, subagent_type: normalizedAgent } + args = { ...args, subagent_type: canonicalAgent as AllowedAgentType } if (args.run_in_background) { if (args.session_id) { diff --git a/src/tools/delegate-task/subagent-resolver.ts b/src/tools/delegate-task/subagent-resolver.ts index 0447416d9b..e627e2f242 100644 --- a/src/tools/delegate-task/subagent-resolver.ts +++ b/src/tools/delegate-task/subagent-resolver.ts @@ -6,6 +6,7 @@ import { parseModelString } from "./model-string-parser" import { AGENT_MODEL_REQUIREMENTS } from "../../shared/model-requirements" import { getAvailableModelsForDelegateTask } from "./available-models" import { resolveModelForDelegateTask } from "./model-selection" +import { toCanonical } from "../../shared/agent-name-aliases" export async function resolveSubagentExecution( args: DelegateTaskArgs, @@ -21,7 +22,7 @@ export async function resolveSubagentExecution( const agentName = args.subagent_type.trim() - if (agentName.toLowerCase() === SISYPHUS_JUNIOR_AGENT.toLowerCase()) { + if (toCanonical(agentName).toLowerCase() === SISYPHUS_JUNIOR_AGENT.toLowerCase()) { return { agentToUse: "", categoryModel: undefined, @@ -31,7 +32,7 @@ Sisyphus-Junior is spawned automatically when you specify a category. Pick the a } } - if (isPlanFamily(agentName) && isPlanFamily(parentAgent)) { + if (isPlanFamily(toCanonical(agentName)) && isPlanFamily(toCanonical(parentAgent ?? ""))) { return { agentToUse: "", categoryModel: undefined, @@ -80,10 +81,10 @@ Create the work plan directly - that's your job as the planning agent.`, agentToUse = matchedAgent.name - const agentNameLower = agentToUse.toLowerCase() - const agentOverride = agentOverrides?.[agentNameLower as keyof typeof agentOverrides] - ?? (agentOverrides ? Object.entries(agentOverrides).find(([key]) => key.toLowerCase() === agentNameLower)?.[1] : undefined) - const agentRequirement = AGENT_MODEL_REQUIREMENTS[agentNameLower] + const canonicalLower = toCanonical(agentToUse).toLowerCase() + const agentOverride = agentOverrides?.[canonicalLower as keyof typeof agentOverrides] + ?? (agentOverrides ? Object.entries(agentOverrides).find(([key]) => key.toLowerCase() === canonicalLower)?.[1] : undefined) + const agentRequirement = AGENT_MODEL_REQUIREMENTS[canonicalLower] if (agentOverride?.model || agentRequirement || matchedAgent.model) { const availableModels = await getAvailableModelsForDelegateTask(client) diff --git a/src/tools/delegate-task/sync-continuation.test.ts b/src/tools/delegate-task/sync-continuation.test.ts index d3ff6d05b3..32288741e1 100644 --- a/src/tools/delegate-task/sync-continuation.test.ts +++ b/src/tools/delegate-task/sync-continuation.test.ts @@ -41,6 +41,10 @@ describe("executeSyncContinuation - toast cleanup error paths", () => { const { __resetTimingConfig } = require("./timing") __resetTimingConfig() + //#given - reset agent name aliases to prevent state leakage + const { resetAgentNameAliases } = require("../../shared/agent-name-aliases") + resetAgentNameAliases() + mock.restore() resetToastManager?.() @@ -357,6 +361,148 @@ describe("executeSyncContinuation - toast cleanup error paths", () => { expect(removeTaskCalls.length).toBe(0) }) + test("canonicalizes agent name when checking isPlanFamily for prometheus alias", async () => { + //#given + const { initializeAgentNameAliases } = require("../../shared/agent-name-aliases") + const { __setTimingConfig } = require("./timing") + __setTimingConfig({ + POLL_INTERVAL_MS: 10, + MIN_STABILITY_TIME_MS: 0, + STABILITY_POLLS_REQUIRED: 1, + MAX_POLL_TIME_MS: 100, + }) + + initializeAgentNameAliases({ prometheus: "Bob" }, ["prometheus", "plan", "sisyphus"]) + + const { executeSyncContinuation } = require("./sync-continuation") + + const mockClient = { + session: { + messages: async () => ({ + data: [ + { info: { id: "msg_001", role: "user", time: { created: 1000 }, agent: "Bob" } }, + { + info: { id: "msg_002", role: "assistant", time: { created: 2000 }, finish: "end_turn" }, + parts: [{ type: "text", text: "Response" }], + }, + ], + }), + promptAsync: async () => ({}), + status: async () => ({ + data: { ses_test: { type: "idle" } }, + }), + }, + } + + const deps = { + pollSyncSession: async () => null, + fetchSyncResult: async () => ({ ok: true as const, textContent: "Result" }), + } + + const mockCtx = { + sessionID: "parent-session", + callID: "call-123", + metadata: () => {}, + } + + const mockExecutorCtx = { + client: mockClient, + } + + const args = { + session_id: "ses_test_12345678", + prompt: "test prompt", + description: "test task", + load_skills: [], + run_in_background: false, + } + + //#when + let promptArgs: any + const originalPrompt = mockClient.session.promptAsync + mockClient.session.promptAsync = mock(async (input: any) => { + promptArgs = input + return originalPrompt.call(mockClient.session) + }) + + const result = await executeSyncContinuation(args, mockCtx, mockExecutorCtx, deps) + + //#then - should recognize "Bob" as prometheus (plan family) and allow task + expect(promptArgs.body.tools.task).toBe(true) + expect(result).toContain("Task continued and completed") + }) + + test("canonicalizes agent name when checking isPlanFamily for plan alias", async () => { + //#given + const { initializeAgentNameAliases } = require("../../shared/agent-name-aliases") + const { __setTimingConfig } = require("./timing") + __setTimingConfig({ + POLL_INTERVAL_MS: 10, + MIN_STABILITY_TIME_MS: 0, + STABILITY_POLLS_REQUIRED: 1, + MAX_POLL_TIME_MS: 100, + }) + + initializeAgentNameAliases({ plan: "Planner" }, ["prometheus", "plan", "sisyphus"]) + + const { executeSyncContinuation } = require("./sync-continuation") + + const mockClient = { + session: { + messages: async () => ({ + data: [ + { info: { id: "msg_001", role: "user", time: { created: 1000 }, agent: "Planner" } }, + { + info: { id: "msg_002", role: "assistant", time: { created: 2000 }, finish: "end_turn" }, + parts: [{ type: "text", text: "Response" }], + }, + ], + }), + promptAsync: async () => ({}), + status: async () => ({ + data: { ses_test: { type: "idle" } }, + }), + }, + } + + const deps = { + pollSyncSession: async () => null, + fetchSyncResult: async () => ({ ok: true as const, textContent: "Result" }), + } + + const mockCtx = { + sessionID: "parent-session", + callID: "call-123", + metadata: () => {}, + } + + const mockExecutorCtx = { + client: mockClient, + } + + const args = { + session_id: "ses_test_12345678", + prompt: "test prompt", + description: "test task", + load_skills: [], + run_in_background: false, + } + + //#when + let promptArgs: any + const originalPrompt = mockClient.session.promptAsync + mockClient.session.promptAsync = mock(async (input: any) => { + promptArgs = input + return originalPrompt.call(mockClient.session) + }) + + const result = await executeSyncContinuation(args, mockCtx, mockExecutorCtx, deps) + + //#then - should recognize "Planner" as plan (plan family) and allow task + expect(promptArgs.body.tools.task).toBe(true) + expect(result).toContain("Task continued and completed") + }) + test("includes subagent in task_metadata when agent info is present in session messages", async () => { //#given - mock session messages with agent info on the last assistant message const mockClient = { diff --git a/src/tools/delegate-task/sync-continuation.ts b/src/tools/delegate-task/sync-continuation.ts index 95f6baec8d..b0486a4f02 100644 --- a/src/tools/delegate-task/sync-continuation.ts +++ b/src/tools/delegate-task/sync-continuation.ts @@ -9,6 +9,7 @@ import { promptWithModelSuggestionRetry } from "../../shared/model-suggestion-re import { findNearestMessageWithFields } from "../../features/hook-message-injector" import { formatDuration } from "./time-formatter" import { syncContinuationDeps, type SyncContinuationDeps } from "./sync-continuation-deps" +import { toCanonical } from "../../shared/agent-name-aliases" export async function executeSyncContinuation( args: DelegateTaskArgs, @@ -76,7 +77,7 @@ export async function executeSyncContinuation( resumeVariant = resumeMessage?.model?.variant } - const allowTask = isPlanFamily(resumeAgent) + const allowTask = isPlanFamily(toCanonical(resumeAgent ?? "")) await promptWithModelSuggestionRetry(client, { path: { id: args.session_id! }, diff --git a/src/tools/delegate-task/sync-prompt-sender.test.ts b/src/tools/delegate-task/sync-prompt-sender.test.ts index 7b26ae2394..5e7a50ad39 100644 --- a/src/tools/delegate-task/sync-prompt-sender.test.ts +++ b/src/tools/delegate-task/sync-prompt-sender.test.ts @@ -1,6 +1,11 @@ -const { describe, test, expect, mock } = require("bun:test") +const { describe, test, expect, mock, afterEach } = require("bun:test") +const { resetAgentNameAliases } = require("../../shared/agent-name-aliases") describe("sendSyncPrompt", () => { + afterEach(() => { + resetAgentNameAliases() + }) + test("applies agent tool restrictions for explore agent", async () => { //#given const { sendSyncPrompt } = require("./sync-prompt-sender") @@ -120,4 +125,92 @@ describe("sendSyncPrompt", () => { expect(promptAsync).toHaveBeenCalled() expect(promptArgs.body.tools.call_omo_agent).toBe(true) }) + + test("canonicalizes agent name when checking isPlanFamily for prometheus alias", async () => { + //#given + const { sendSyncPrompt } = require("./sync-prompt-sender") + const { initializeAgentNameAliases } = require("../../shared/agent-name-aliases") + + // Initialize aliases: "Bob" -> "prometheus" + initializeAgentNameAliases({ prometheus: "Bob" }, ["prometheus", "plan", "sisyphus"]) + + let promptArgs: any + const promptAsync = mock(async (input: any) => { + promptArgs = input + return { data: {} } + }) + + const mockClient = { + session: { + promptAsync, + }, + } + + const input = { + sessionID: "test-session", + agentToUse: "Bob", // Alias for prometheus + args: { + description: "test task", + prompt: "test prompt", + category: "quick", + run_in_background: false, + load_skills: [], + }, + systemContent: undefined, + categoryModel: undefined, + toastManager: null, + taskId: undefined, + } + + //#when + await sendSyncPrompt(mockClient as any, input) + + //#then - should recognize "Bob" as prometheus (plan family) and allow task + expect(promptAsync).toHaveBeenCalled() + expect(promptArgs.body.tools.task).toBe(true) + }) + + test("canonicalizes agent name when checking isPlanFamily for plan alias", async () => { + //#given + const { sendSyncPrompt } = require("./sync-prompt-sender") + const { initializeAgentNameAliases } = require("../../shared/agent-name-aliases") + + // Initialize aliases: "Planner" -> "plan" + initializeAgentNameAliases({ plan: "Planner" }, ["prometheus", "plan", "sisyphus"]) + + let promptArgs: any + const promptAsync = mock(async (input: any) => { + promptArgs = input + return { data: {} } + }) + + const mockClient = { + session: { + promptAsync, + }, + } + + const input = { + sessionID: "test-session", + agentToUse: "Planner", // Alias for plan + args: { + description: "test task", + prompt: "test prompt", + category: "quick", + run_in_background: false, + load_skills: [], + }, + systemContent: undefined, + categoryModel: undefined, + toastManager: null, + taskId: undefined, + } + + //#when + await sendSyncPrompt(mockClient as any, input) + + //#then - should recognize "Planner" as plan (plan family) and allow task + expect(promptAsync).toHaveBeenCalled() + expect(promptArgs.body.tools.task).toBe(true) + }) }) diff --git a/src/tools/delegate-task/sync-prompt-sender.ts b/src/tools/delegate-task/sync-prompt-sender.ts index 815fe38036..90a0ce53b7 100644 --- a/src/tools/delegate-task/sync-prompt-sender.ts +++ b/src/tools/delegate-task/sync-prompt-sender.ts @@ -3,6 +3,7 @@ import { isPlanFamily } from "./constants" import { promptWithModelSuggestionRetry } from "../../shared/model-suggestion-retry" import { formatDetailedError } from "./error-formatting" import { getAgentToolRestrictions } from "../../shared/agent-tool-restrictions" +import { toCanonical } from "../../shared/agent-name-aliases" export async function sendSyncPrompt( client: OpencodeClient, @@ -17,7 +18,7 @@ export async function sendSyncPrompt( } ): Promise { try { - const allowTask = isPlanFamily(input.agentToUse) + const allowTask = isPlanFamily(toCanonical(input.agentToUse)) await promptWithModelSuggestionRetry(client, { path: { id: input.sessionID }, body: { diff --git a/src/tools/look-at/multimodal-agent-metadata.ts b/src/tools/look-at/multimodal-agent-metadata.ts index e24c8b6fba..1e8143e2d0 100644 --- a/src/tools/look-at/multimodal-agent-metadata.ts +++ b/src/tools/look-at/multimodal-agent-metadata.ts @@ -1,6 +1,6 @@ import type { PluginInput } from "@opencode-ai/plugin" import { MULTIMODAL_LOOKER_AGENT } from "./constants" -import { log } from "../../shared" +import { log, toCanonical } from "../../shared" type AgentModel = { providerID: string; modelID: string } @@ -42,7 +42,7 @@ export async function resolveMultimodalLookerAgentMetadata( const agents = Array.isArray(agentsRaw) ? agentsRaw.map(toAgentInfo).filter(Boolean) : [] const matched = agents.find( - (agent) => agent?.name?.toLowerCase() === MULTIMODAL_LOOKER_AGENT.toLowerCase() + (agent) => toCanonical(agent?.name ?? "").toLowerCase() === MULTIMODAL_LOOKER_AGENT.toLowerCase() ) return { diff --git a/src/tools/look-at/tools.ts b/src/tools/look-at/tools.ts index 0d5c1c0b7f..f6f6cfb9c3 100644 --- a/src/tools/look-at/tools.ts +++ b/src/tools/look-at/tools.ts @@ -3,7 +3,7 @@ import { pathToFileURL } from "node:url" import { tool, type PluginInput, type ToolDefinition } from "@opencode-ai/plugin" import { LOOK_AT_DESCRIPTION, MULTIMODAL_LOOKER_AGENT } from "./constants" import type { LookAtArgs } from "./types" -import { log, promptSyncWithModelSuggestionRetry } from "../../shared" +import { log, promptSyncWithModelSuggestionRetry, toRegistered } from "../../shared" import { extractLatestAssistantText } from "./assistant-message-extractor" import type { LookAtArgsWithAlias } from "./look-at-arguments" import { normalizeArgs, validateArgs } from "./look-at-arguments" @@ -110,7 +110,7 @@ Original error: ${createResult.error}` await promptSyncWithModelSuggestionRetry(ctx.client, { path: { id: sessionID }, body: { - agent: MULTIMODAL_LOOKER_AGENT, + agent: toRegistered(MULTIMODAL_LOOKER_AGENT), tools: { task: false, call_omo_agent: false,