From e8042fa4450889614c254d5373ed0454da3d5d4d Mon Sep 17 00:00:00 2001 From: ismeth Date: Wed, 18 Feb 2026 20:56:07 +0100 Subject: [PATCH] fix(athena): harden council tool error handling and type safety Improve not-configured error message with config file path. Wrap metadataFn in try/catch for best-effort metadata. Replace unsafe as-casts with getToolContextProperty helper. Show Name (model) format in errors. Return error directly for empty member selection. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus --- src/tools/athena-council/tools.test.ts | 24 ++++++++------ src/tools/athena-council/tools.ts | 43 +++++++++++++++++++------- 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/src/tools/athena-council/tools.test.ts b/src/tools/athena-council/tools.test.ts index 8cfb957c8..e19469bec 100644 --- a/src/tools/athena-council/tools.test.ts +++ b/src/tools/athena-council/tools.test.ts @@ -39,16 +39,20 @@ function createRunningTask(id: string, sessionID = `ses-${id}`): BackgroundTask } describe("filterCouncilMembers", () => { - test("returns all members when selection is undefined", () => { + test("returns selection error when selection is undefined", () => { const result = filterCouncilMembers(configuredMembers, undefined) - expect(result.members).toEqual(configuredMembers) - expect(result.error).toBeUndefined() + expect(result.members).toEqual([]) + expect(result.error).toBe( + "athena_council runs one member per call. Pass exactly one member in members (single-item array). Available members: Claude, GPT, google/gemini-3-pro." + ) }) - test("returns all members when selection is empty", () => { + test("returns selection error when selection is empty", () => { const result = filterCouncilMembers(configuredMembers, []) - expect(result.members).toEqual(configuredMembers) - expect(result.error).toBeUndefined() + expect(result.members).toEqual([]) + expect(result.error).toBe( + "athena_council runs one member per call. Pass exactly one member in members (single-item array). Available members: Claude, GPT, google/gemini-3-pro." + ) }) test("filters members using case-insensitive name and model matching", () => { @@ -61,7 +65,7 @@ describe("filterCouncilMembers", () => { const result = filterCouncilMembers(configuredMembers, ["mistral", "xai/grok-3"]) expect(result.members).toEqual([]) expect(result.error).toBe( - "Unknown council members: mistral, xai/grok-3. Available members: Claude, GPT, google/gemini-3-pro." + "Unknown council members: mistral, xai/grok-3. Available: Claude (anthropic/claude-sonnet-4-5), GPT (openai/gpt-5.3-codex), google/gemini-3-pro." ) }) @@ -81,7 +85,7 @@ describe("createAthenaCouncilTool", () => { const result = await athenaCouncilTool.execute({ question: "How should we proceed?" }, mockToolContext) - expect(result).toBe("Athena council not configured. Add agents.athena.council.members to your config.") + expect(result).toBe("Athena council is not configured. Add council members to agents.athena.council.members in .opencode/oh-my-opencode.jsonc.") }) test("returns error when councilConfig has empty members", async () => { @@ -92,7 +96,7 @@ describe("createAthenaCouncilTool", () => { const result = await athenaCouncilTool.execute({ question: "Any concerns?" }, mockToolContext) - expect(result).toBe("Athena council not configured. Add agents.athena.council.members to your config.") + expect(result).toBe("Athena council is not configured. Add council members to agents.athena.council.members in .opencode/oh-my-opencode.jsonc.") }) test("returns helpful error when members contains invalid names", async () => { @@ -106,7 +110,7 @@ describe("createAthenaCouncilTool", () => { mockToolContext ) - expect(result).toBe("Unknown council members: unknown-model. Available members: Claude, GPT, google/gemini-3-pro.") + expect(result).toBe("Unknown council members: unknown-model. Available: Claude (anthropic/claude-sonnet-4-5), GPT (openai/gpt-5.3-codex), google/gemini-3-pro.") }) test("returns selection error when members are omitted", async () => { diff --git a/src/tools/athena-council/tools.ts b/src/tools/athena-council/tools.ts index 770754e16..afd8ad67c 100644 --- a/src/tools/athena-council/tools.ts +++ b/src/tools/athena-council/tools.ts @@ -8,6 +8,13 @@ import { waitForCouncilSessions } from "./session-waiter" import type { AthenaCouncilToolArgs } from "./types" import { storeToolMetadata } from "../../features/tool-metadata-store" +function getToolContextProperty(toolContext: unknown, key: string): unknown { + if (typeof toolContext === "object" && toolContext !== null && key in toolContext) { + return (toolContext as Record)[key] + } + return undefined +} + function isCouncilConfigured(councilConfig: CouncilConfig | undefined): councilConfig is CouncilConfig { return Boolean(councilConfig && councilConfig.members.length > 0) } @@ -27,7 +34,10 @@ export function filterCouncilMembers( selectedNames: string[] | undefined ): FilterCouncilMembersResult { if (!selectedNames || selectedNames.length === 0) { - return { members } + return { + members: [], + error: buildSingleMemberSelectionError(members), + } } const memberLookup = new Map() @@ -59,10 +69,17 @@ export function filterCouncilMembers( }) if (unresolved.length > 0) { - const availableNames = members.map((member) => member.name ?? member.model).join(", ") + const availableDescriptions = members + .map((member) => { + if (member.name) { + return `${member.name} (${member.model})` + } + return member.model + }) + .join(", ") return { members: [], - error: `Unknown council members: ${unresolved.join(", ")}. Available members: ${availableNames}.`, + error: `Unknown council members: ${unresolved.join(", ")}. Available: ${availableDescriptions}.`, } } @@ -95,7 +112,7 @@ export function createAthenaCouncilTool(args: { }, async execute(toolArgs: AthenaCouncilToolArgs, toolContext) { if (!isCouncilConfigured(councilConfig)) { - return "Athena council not configured. Add agents.athena.council.members to your config." + return "Athena council is not configured. Add council members to agents.athena.council.members in .opencode/oh-my-opencode.jsonc." } const filteredMembers = filterCouncilMembers(councilConfig.members, toolArgs.members) @@ -124,11 +141,11 @@ export function createAthenaCouncilTool(args: { const launchedMemberModel = launched?.member.model ?? "unknown" const launchedTaskId = launched?.taskId ?? "unknown" - const sessionInfos = await waitForCouncilSessions(execution.launched, backgroundManager, toolContext.abort) - const launchedSession = sessionInfos.find((session) => session.taskId === launchedTaskId) + const waitResult = await waitForCouncilSessions(execution.launched, backgroundManager, toolContext.abort) + const launchedSession = waitResult.sessions.find((session) => session.taskId === launchedTaskId) const sessionId = launchedSession?.sessionId ?? "pending" - const metadataFn = (toolContext as Record).metadata as + const metadataFn = getToolContextProperty(toolContext, "metadata") as | ((input: { title?: string; metadata?: Record }) => Promise) | undefined if (metadataFn) { @@ -141,11 +158,15 @@ export function createAthenaCouncilTool(args: { description: `Council member: ${launchedMemberName}`, }, } - await metadataFn(memberMetadata) + try { + await metadataFn(memberMetadata) - const callID = (toolContext as Record).callID - if (typeof callID === "string") { - storeToolMetadata(toolContext.sessionID, callID, memberMetadata) + const callID = getToolContextProperty(toolContext, "callID") + if (typeof callID === "string") { + storeToolMetadata(toolContext.sessionID, callID, memberMetadata) + } + } catch { + // Metadata storage is best-effort — don't mask successful council launch } }