From 7d2749cfe1797908bfe03daf1413c1d4f3f736e5 Mon Sep 17 00:00:00 2001 From: ismeth Date: Sun, 1 Mar 2026 16:48:12 +0100 Subject: [PATCH] fix(athena): address 11 audit findings (H2,H3,H5,M1-M4,M8-M11) Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus --- src/agents/athena/constants.ts | 7 + .../athena/council-member-agent.test.ts | 78 +++++++- src/agents/athena/council-member-agent.ts | 3 +- .../athena/council-runtime-guidance.test.ts | 170 ++++++++++++++++++ src/agents/athena/index.ts | 1 + .../council-member-agents.test.ts | 71 ++++++++ .../council-continuation-enforcer.test.ts | 6 +- .../council-continuation-enforcer.ts | 18 +- .../council-response-checker.test.ts | 8 +- .../council-response-checker.ts | 16 +- .../council-finalize-helpers.ts | 54 ++++++ .../council-flow.integration.test.ts | 26 +-- .../council-response-extractor.test.ts | 63 +++++-- .../council-response-extractor.ts | 5 + .../create-council-finalize.test.ts | 88 ++++----- .../create-council-finalize.ts | 107 ++--------- src/tools/council-archive/index.ts | 2 +- .../council-archive/meta-yaml-formatter.ts | 41 +++++ src/tools/council-archive/types.ts | 2 +- .../prepare-council-prompt/tools.test.ts | 28 +++ src/tools/prepare-council-prompt/tools.ts | 29 +-- 21 files changed, 610 insertions(+), 213 deletions(-) create mode 100644 src/agents/athena/constants.ts create mode 100644 src/agents/athena/council-runtime-guidance.test.ts create mode 100644 src/tools/council-archive/council-finalize-helpers.ts create mode 100644 src/tools/council-archive/meta-yaml-formatter.ts diff --git a/src/agents/athena/constants.ts b/src/agents/athena/constants.ts new file mode 100644 index 000000000..e65d6324a --- /dev/null +++ b/src/agents/athena/constants.ts @@ -0,0 +1,7 @@ +export const COUNCIL_DEFAULTS = { + CLEANUP_DELAY_MS: 30 * 60 * 1000, + BACKGROUND_WAIT_TIMEOUT_MS: 30000, + STUCK_THRESHOLD_SECONDS: 120, + MEMBER_MAX_RUNNING_SECONDS: 1800, + ARCHIVE_ID_BYTES: 2, +} as const diff --git a/src/agents/athena/council-member-agent.test.ts b/src/agents/athena/council-member-agent.test.ts index 82fe1a2c1..fa75d865f 100644 --- a/src/agents/athena/council-member-agent.test.ts +++ b/src/agents/athena/council-member-agent.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test" -import { COUNCIL_MEMBER_PROMPT } from "./council-member-agent" +import { COUNCIL_MEMBER_PROMPT, createCouncilMemberAgent } from "./council-member-agent" describe("COUNCIL_MEMBER_PROMPT", () => { describe("#given the prompt constant", () => { @@ -44,3 +44,79 @@ describe("COUNCIL_MEMBER_PROMPT", () => { }) }) }) + +describe("createCouncilMemberAgent", () => { + describe("#given a model string", () => { + describe("#when creating a council member agent", () => { + const agent = createCouncilMemberAgent("openai/gpt-5-nano") + + it("#then returns an object with the given model", () => { + expect(agent.model).toBe("openai/gpt-5-nano") + }) + + it("#then has temperature 0.1", () => { + expect(agent.temperature).toBe(0.1) + }) + + it("#then has the COUNCIL_MEMBER_PROMPT as prompt", () => { + expect(agent.prompt).toBe(COUNCIL_MEMBER_PROMPT) + }) + + it("#then has mode subagent", () => { + expect(agent.mode).toBe("subagent") + }) + + it("#then has tool restrictions with permission object", () => { + expect(agent.permission).toBeDefined() + }) + + it("#then allows read tool", () => { + const perm = agent.permission as Record + expect(perm.read).toBe("allow") + }) + + it("#then allows grep tool", () => { + const perm = agent.permission as Record + expect(perm.grep).toBe("allow") + }) + + it("#then allows glob tool", () => { + const perm = agent.permission as Record + expect(perm.glob).toBe("allow") + }) + + it("#then allows lsp_goto_definition tool", () => { + const perm = agent.permission as Record + expect(perm.lsp_goto_definition).toBe("allow") + }) + + it("#then allows ast_grep_search tool", () => { + const perm = agent.permission as Record + expect(perm.ast_grep_search).toBe("allow") + }) + + it("#then denies all other tools via wildcard", () => { + const perm = agent.permission as Record + expect(perm["*"]).toBe("deny") + }) + + it("#then explicitly denies todowrite", () => { + const perm = agent.permission as Record + expect(perm.todowrite).toBe("deny") + }) + + it("#then explicitly denies todoread", () => { + const perm = agent.permission as Record + expect(perm.todoread).toBe("deny") + }) + }) + }) + + describe("#given the factory function", () => { + describe("#when checking the static mode property", () => { + it("#then has mode 'subagent'", () => { + expect(createCouncilMemberAgent.mode).toBe("subagent") + }) + }) + }) +}) diff --git a/src/agents/athena/council-member-agent.ts b/src/agents/athena/council-member-agent.ts index e449de73a..06e206e2e 100644 --- a/src/agents/athena/council-member-agent.ts +++ b/src/agents/athena/council-member-agent.ts @@ -36,7 +36,8 @@ Example: Your analysis here... -If you do not wrap your response in tags, your analysis will not be included in the synthesis.` +If you do not wrap your response in tags, your analysis will not be included in the synthesis. +Your response inside the tags must be at least 100 characters of substantive content. Empty or trivially short responses will be treated as missing and will not count toward quorum.` export const COUNCIL_SOLO_ADDENDUM = ` ## Solo Analysis Mode diff --git a/src/agents/athena/council-runtime-guidance.test.ts b/src/agents/athena/council-runtime-guidance.test.ts new file mode 100644 index 000000000..1fb556cd0 --- /dev/null +++ b/src/agents/athena/council-runtime-guidance.test.ts @@ -0,0 +1,170 @@ +import { describe, expect, it } from "bun:test" +import { resolveCouncilIntent, buildAthenaRuntimeGuidance, getValidCouncilIntents } from "./council-runtime-guidance" + +describe("council-runtime-guidance", () => { + describe("resolveCouncilIntent", () => { + describe("#given valid uppercase intents", () => { + describe("#when called with each valid intent", () => { + const validIntents = ["DIAGNOSE", "AUDIT", "PLAN", "EVALUATE", "EXPLAIN", "CREATE", "PERSPECTIVES", "FREEFORM"] as const + + for (const intent of validIntents) { + it(`#then returns "${intent}" for "${intent}"`, () => { + expect(resolveCouncilIntent(intent)).toBe(intent) + }) + } + }) + }) + + describe("#given lowercase intents", () => { + describe("#when called with lowercase versions", () => { + it("#then normalizes 'diagnose' to 'DIAGNOSE'", () => { + expect(resolveCouncilIntent("diagnose")).toBe("DIAGNOSE") + }) + + it("#then normalizes 'audit' to 'AUDIT'", () => { + expect(resolveCouncilIntent("audit")).toBe("AUDIT") + }) + + it("#then normalizes 'freeform' to 'FREEFORM'", () => { + expect(resolveCouncilIntent("freeform")).toBe("FREEFORM") + }) + }) + }) + + describe("#given mixed case intents", () => { + describe("#when called with mixed case", () => { + it("#then normalizes 'Audit' to 'AUDIT'", () => { + expect(resolveCouncilIntent("Audit")).toBe("AUDIT") + }) + + it("#then normalizes 'DiAgNoSe' to 'DIAGNOSE'", () => { + expect(resolveCouncilIntent("DiAgNoSe")).toBe("DIAGNOSE") + }) + + it("#then normalizes 'Perspectives' to 'PERSPECTIVES'", () => { + expect(resolveCouncilIntent("Perspectives")).toBe("PERSPECTIVES") + }) + }) + }) + + describe("#given invalid inputs", () => { + describe("#when called with an unrecognized intent", () => { + it("#then returns null for 'INVALID'", () => { + expect(resolveCouncilIntent("INVALID")).toBeNull() + }) + + it("#then returns null for 'COMPARISON'", () => { + expect(resolveCouncilIntent("COMPARISON")).toBeNull() + }) + }) + + describe("#when called with undefined", () => { + it("#then returns null", () => { + expect(resolveCouncilIntent(undefined)).toBeNull() + }) + }) + + describe("#when called with empty string", () => { + it("#then returns null", () => { + expect(resolveCouncilIntent("")).toBeNull() + }) + }) + }) + }) + + describe("buildAthenaRuntimeGuidance", () => { + describe("#given a valid intent", () => { + describe("#when building guidance for DIAGNOSE", () => { + it("#then wraps content in athena_runtime_guidance tags", () => { + const result = buildAthenaRuntimeGuidance("DIAGNOSE") + expect(result).toContain("") + expect(result).toContain("") + }) + + it("#then contains the intent name", () => { + const result = buildAthenaRuntimeGuidance("DIAGNOSE") + expect(result).toContain("intent: DIAGNOSE") + }) + + it("#then contains DIAGNOSE-specific content about root cause", () => { + const result = buildAthenaRuntimeGuidance("DIAGNOSE") + expect(result).toContain("root cause") + }) + }) + + describe("#when building guidance for AUDIT", () => { + it("#then contains the AUDIT intent name", () => { + const result = buildAthenaRuntimeGuidance("AUDIT") + expect(result).toContain("intent: AUDIT") + }) + + it("#then contains AUDIT-specific synthesis rules", () => { + const result = buildAthenaRuntimeGuidance("AUDIT") + expect(result).toContain("AUDIT synthesis") + }) + }) + + describe("#when building guidance for FREEFORM", () => { + it("#then contains FREEFORM intent name", () => { + const result = buildAthenaRuntimeGuidance("FREEFORM") + expect(result).toContain("intent: FREEFORM") + }) + + it("#then contains FREEFORM-specific content", () => { + const result = buildAthenaRuntimeGuidance("FREEFORM") + expect(result).toContain("FREEFORM synthesis") + }) + }) + + describe("#when building guidance for each intent", () => { + const allIntents = ["DIAGNOSE", "AUDIT", "PLAN", "EVALUATE", "EXPLAIN", "CREATE", "PERSPECTIVES", "FREEFORM"] as const + + for (const intent of allIntents) { + it(`#then ${intent} guidance contains runtime_synthesis_rules`, () => { + const result = buildAthenaRuntimeGuidance(intent) + expect(result).toContain("runtime_synthesis_rules") + }) + + it(`#then ${intent} guidance contains runtime_action_paths`, () => { + const result = buildAthenaRuntimeGuidance(intent) + expect(result).toContain("runtime_action_paths") + }) + + it(`#then ${intent} guidance contains source: council_finalize`, () => { + const result = buildAthenaRuntimeGuidance(intent) + expect(result).toContain("source: council_finalize") + }) + } + }) + }) + }) + + describe("getValidCouncilIntents", () => { + describe("#given the function is called", () => { + describe("#when retrieving valid intents", () => { + it("#then returns an array of 8 intents", () => { + const intents = getValidCouncilIntents() + expect(intents).toHaveLength(8) + }) + + it("#then contains all expected intent values", () => { + const intents = getValidCouncilIntents() + expect(intents).toContain("DIAGNOSE") + expect(intents).toContain("AUDIT") + expect(intents).toContain("PLAN") + expect(intents).toContain("EVALUATE") + expect(intents).toContain("EXPLAIN") + expect(intents).toContain("CREATE") + expect(intents).toContain("PERSPECTIVES") + expect(intents).toContain("FREEFORM") + }) + + it("#then returns a readonly array", () => { + const intents1 = getValidCouncilIntents() + const intents2 = getValidCouncilIntents() + expect(intents1).toBe(intents2) + }) + }) + }) + }) +}) diff --git a/src/agents/athena/index.ts b/src/agents/athena/index.ts index 88adce0c2..971242d17 100644 --- a/src/agents/athena/index.ts +++ b/src/agents/athena/index.ts @@ -7,3 +7,4 @@ export { resolveCouncilIntent, } from "./council-runtime-guidance" export type { CouncilIntent } from "./council-runtime-guidance" +export { COUNCIL_DEFAULTS } from "./constants" diff --git a/src/agents/builtin-agents/council-member-agents.test.ts b/src/agents/builtin-agents/council-member-agents.test.ts index 39ddd7956..e028074ad 100644 --- a/src/agents/builtin-agents/council-member-agents.test.ts +++ b/src/agents/builtin-agents/council-member-agents.test.ts @@ -83,4 +83,75 @@ describe("council-member-agents", () => { expect(result.registeredKeys).toHaveLength(0) expect(result.agents).toEqual({}) }) + + test("returns skippedMembers with reason for invalid model format", () => { + //#given + const config = { + members: [ + { model: "openai/gpt-5.3-codex", name: "GPT" }, + { model: "no-slash", name: "Bad" }, + { model: "anthropic/claude-opus-4-6", name: "Claude" }, + ], + retry_on_fail: 0, + retry_failed_if_others_finished: false, + cancel_retrying_on_quorum: true, + stuck_threshold_seconds: 120, + member_max_running_seconds: 1800, + } + //#when + const result = registerCouncilMemberAgents(config) + //#then + expect(result.skippedMembers).toHaveLength(1) + expect(result.skippedMembers[0].name).toBe("Bad") + expect(result.skippedMembers[0].reason).toContain("Invalid model format") + expect(result.skippedMembers[0].reason).toContain("no-slash") + }) + + test("returns skippedMembers with reason for duplicate names", () => { + //#given + const config = { + members: [ + { model: "openai/gpt-5.3-codex", name: "Alpha" }, + { model: "anthropic/claude-opus-4-6", name: "Beta" }, + { model: "google/gemini-3-pro", name: "alpha" }, + ], + retry_on_fail: 0, + retry_failed_if_others_finished: false, + cancel_retrying_on_quorum: true, + stuck_threshold_seconds: 120, + member_max_running_seconds: 1800, + } + //#when + const result = registerCouncilMemberAgents(config) + //#then + expect(result.registeredKeys).toHaveLength(2) + expect(result.skippedMembers).toHaveLength(1) + expect(result.skippedMembers[0].name).toBe("alpha") + expect(result.skippedMembers[0].reason).toContain("Duplicate name") + }) + + test("returns skippedMembers combining both invalid model and duplicate reasons", () => { + //#given + const config = { + members: [ + { model: "openai/gpt-5.3-codex", name: "GPT" }, + { model: "bad-model", name: "Invalid" }, + { model: "anthropic/claude-opus-4-6", name: "Claude" }, + { model: "google/gemini-3-pro", name: "gpt" }, + ], + retry_on_fail: 0, + retry_failed_if_others_finished: false, + cancel_retrying_on_quorum: true, + stuck_threshold_seconds: 120, + member_max_running_seconds: 1800, + } + //#when + const result = registerCouncilMemberAgents(config) + //#then + expect(result.skippedMembers).toHaveLength(2) + expect(result.skippedMembers[0].name).toBe("Invalid") + expect(result.skippedMembers[0].reason).toContain("Invalid model format") + expect(result.skippedMembers[1].name).toBe("gpt") + expect(result.skippedMembers[1].reason).toContain("Duplicate name") + }) }) diff --git a/src/features/background-agent/council-continuation-enforcer.test.ts b/src/features/background-agent/council-continuation-enforcer.test.ts index b2235b185..59bc09027 100644 --- a/src/features/background-agent/council-continuation-enforcer.test.ts +++ b/src/features/background-agent/council-continuation-enforcer.test.ts @@ -140,13 +140,13 @@ describe("hasCouncilResponseTag", () => { }) }) - describe("#given assistant message contains the response tag", () => { + describe("#given assistant message contains a complete council response", () => { it("#then should return true", () => { //#given const messages = [ { info: { role: "assistant" }, - parts: [{ type: "text", text: "My analysis " }], + parts: [{ type: "text", text: "This is a comprehensive analysis of the council member findings. The investigation reveals multiple significant patterns across the entire codebase that require careful refactoring." }], }, ] @@ -170,7 +170,7 @@ describe("hasCouncilResponseTag", () => { }, { info: { role: "assistant" }, - parts: [{ type: "text", text: "final answer " }], + parts: [{ type: "text", text: "This is a comprehensive analysis of the council member findings. The investigation reveals multiple significant patterns across the entire codebase that require careful refactoring." }], }, ] diff --git a/src/features/background-agent/council-continuation-enforcer.ts b/src/features/background-agent/council-continuation-enforcer.ts index 5b57540d8..bd29652e9 100644 --- a/src/features/background-agent/council-continuation-enforcer.ts +++ b/src/features/background-agent/council-continuation-enforcer.ts @@ -6,12 +6,11 @@ import { createInternalAgentTextPart, } from "../../shared" import { setSessionTools } from "../../shared/session-tools-store" +import { extractCouncilResponse } from "../../tools/council-archive/council-response-extractor" import { COUNCIL_MEMBER_KEY_PREFIX } from "../../agents/builtin-agents/council-member-agents" type OpencodeClient = PluginInput["client"] -const COUNCIL_RESPONSE_TAG = "" - const CONTINUATION_PROMPT = "You have not yet produced your final . Continue your analysis and wrap your findings in tags. If you are waiting for background tasks, use background_wait to block until they complete, then produce your response." @@ -28,17 +27,18 @@ export function resetCouncilNudgeCount(taskId: string): void { } export function hasCouncilResponseTag(sessionMessages: Array<{ info?: { role?: string }; parts?: Array<{ type?: string; text?: string }> }>): boolean { - for (let i = sessionMessages.length - 1; i >= 0; i--) { - const msg = sessionMessages[i] + const assistantTexts: string[] = [] + for (const msg of sessionMessages) { if (msg.info?.role !== "assistant") continue - const parts = msg.parts ?? [] - for (const part of parts) { - if (part.type === "text" && part.text?.includes(COUNCIL_RESPONSE_TAG)) { - return true + for (const part of msg.parts ?? []) { + if (part.type === "text" && part.text) { + assistantTexts.push(part.text) } } } - return false + if (assistantTexts.length === 0) return false + const extraction = extractCouncilResponse(assistantTexts.join("\n")) + return extraction.has_response && extraction.response_complete } export function sendCouncilContinuationNudge( diff --git a/src/features/background-agent/council-response-checker.test.ts b/src/features/background-agent/council-response-checker.test.ts index 0c85c6672..8df990e39 100644 --- a/src/features/background-agent/council-response-checker.test.ts +++ b/src/features/background-agent/council-response-checker.test.ts @@ -13,13 +13,13 @@ function createMockClient( } describe("sessionHasCouncilResponse", () => { - describe("#given assistant message with closing council tag", () => { + describe("#given assistant message with complete council response", () => { it("#when tag is in text part #then should return true", async () => { //#given const client = createMockClient([ { info: { role: "assistant" }, - parts: [{ type: "text", text: "Some response" }], + parts: [{ type: "text", text: "This is a comprehensive analysis of the council member findings. The investigation reveals multiple significant patterns across the entire codebase that require careful refactoring." }], }, ]) @@ -102,7 +102,7 @@ describe("sessionHasCouncilResponse", () => { }) }) - describe("#given assistant message with tag buried in longer text", () => { + describe("#given assistant message with complete council response in longer text", () => { it("#when tag appears mid-text #then should return true", async () => { //#given const client = createMockClient([ @@ -111,7 +111,7 @@ describe("sessionHasCouncilResponse", () => { parts: [ { type: "text", - text: "Here is my analysis of the situation.\n\nLong content here.\n\n\n\nMore text after.", + text: "Here is my analysis. This is a comprehensive analysis of the council member findings. The investigation reveals multiple significant patterns across the entire codebase that require careful refactoring. More text after.", }, ], }, diff --git a/src/features/background-agent/council-response-checker.ts b/src/features/background-agent/council-response-checker.ts index a8e552ab4..f611a1dd3 100644 --- a/src/features/background-agent/council-response-checker.ts +++ b/src/features/background-agent/council-response-checker.ts @@ -1,10 +1,9 @@ import type { PluginInput } from "@opencode-ai/plugin" import { log, normalizeSDKResponse } from "../../shared" +import { hasCouncilResponseTag } from "./council-continuation-enforcer" type OpencodeClient = PluginInput["client"] -const COUNCIL_RESPONSE_TAG = "" - export async function sessionHasCouncilResponse( client: OpencodeClient, sessionID: string, @@ -20,18 +19,7 @@ export async function sessionHasCouncilResponse( { preferResponseOnMissingData: true }, ) - for (let i = messages.length - 1; i >= 0; i--) { - const msg = messages[i] - if (msg.info?.role !== "assistant") continue - const parts = msg.parts ?? [] - for (const part of parts) { - if (part.type === "text" && part.text?.includes(COUNCIL_RESPONSE_TAG)) { - return true - } - } - } - - return false + return hasCouncilResponseTag(messages) } catch (error) { log("[council-response-checker] Error checking session for response tag:", { sessionID, diff --git a/src/tools/council-archive/council-finalize-helpers.ts b/src/tools/council-archive/council-finalize-helpers.ts new file mode 100644 index 000000000..632eab461 --- /dev/null +++ b/src/tools/council-archive/council-finalize-helpers.ts @@ -0,0 +1,54 @@ +import { readFile, writeFile, rename } from "node:fs/promises" +import { join, isAbsolute, resolve, relative } from "node:path" +import { log } from "../../shared/logger" + +export const TASK_ID_PATTERN = /^[a-zA-Z0-9_-]+$/ + +export function slugify(text: string): string { + return text + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, "") +} + +export function toPosixPath(pathValue: string): string { + return pathValue.replace(/\\/g, "/") +} + +export function extractAgentFromFrontmatter(content: string): string | null { + const fmMatch = content.match(/^---\n([\s\S]*?)\n---/) + if (!fmMatch) return null + const agentLine = fmMatch[1].match(/^agent:\s*(.+)$/m) + return agentLine ? agentLine[1].trim() : null +} + +export function isPathEscaping(expectedRoot: string, targetPath: string): boolean { + const rel = relative(expectedRoot, targetPath) + return rel.startsWith("..") || isAbsolute(rel) +} + +export async function movePromptFile( + promptFilePath: string, + base: string, + absArchiveDir: string, + relArchiveDir: string, +): Promise { + try { + const promptFilename = "council-prompt.md" + const absPromptSrc = isAbsolute(promptFilePath) ? promptFilePath : resolve(base, promptFilePath) + const expectedPromptRoot = join(base, ".sisyphus", "tmp") + if (isPathEscaping(expectedPromptRoot, absPromptSrc)) { + log("[council-finalize] Rejected prompt_file outside .sisyphus/tmp/", { promptFile: promptFilePath }) + return undefined + } + const absPromptDest = join(absArchiveDir, promptFilename) + await rename(absPromptSrc, absPromptDest).catch(async () => { + const content = await readFile(absPromptSrc, "utf-8") + await writeFile(absPromptDest, content, "utf-8") + }) + return toPosixPath(join(relArchiveDir, promptFilename)) + } catch (err) { + log("[council-finalize] Failed to move prompt file", { promptFile: promptFilePath, error: String(err) }) + return undefined + } +} diff --git a/src/tools/council-archive/council-flow.integration.test.ts b/src/tools/council-archive/council-flow.integration.test.ts index c5355a196..eb0f2b5c4 100644 --- a/src/tools/council-archive/council-flow.integration.test.ts +++ b/src/tools/council-archive/council-flow.integration.test.ts @@ -89,9 +89,9 @@ describe("council archive integration flow", () => { describe("#when finalize is called and then each archive is read", () => { it("#then creates archive with correct structure and archives are readable", async () => { const agents = [ - { id: "bg_opus", agent: "Council: Claude Opus", response: "Opus deep analysis of architecture" }, - { id: "bg_gpt", agent: "Council: GPT-5", response: "GPT pragmatic code review" }, - { id: "bg_gemini", agent: "Council: Gemini", response: "Gemini creative alternative approach" }, + { id: "bg_opus", agent: "Council: Claude Opus", response: "Opus deep analysis of architecture: This is a detailed analysis from Opus model covering the full scope of the council question." }, + { id: "bg_gpt", agent: "Council: GPT-5", response: "GPT pragmatic code review: This is a detailed analysis from GPT model covering the full scope of the council question." }, + { id: "bg_gemini", agent: "Council: Gemini", response: "Gemini creative alternative approach: This is a detailed analysis from Gemini model covering the full scope of the council question." }, ] for (const a of agents) { @@ -104,7 +104,7 @@ describe("council archive integration flow", () => { const finalizeTool = createCouncilFinalize(tmpDir) const resultStr = await finalizeTool.execute( - { task_ids: agents.map((a) => a.id), name: "test" }, + { task_ids: agents.map((a) => a.id), name: "test", intent: "FREEFORM" }, toolContext, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -152,7 +152,7 @@ describe("council archive integration flow", () => { const finalizeTool = createCouncilFinalize(tmpDir) const resultStr = await finalizeTool.execute( - { task_ids: [taskId], name: "partial" }, + { task_ids: [taskId], name: "partial", intent: "FREEFORM" }, toolContext, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -180,7 +180,7 @@ describe("council archive integration flow", () => { const finalizeTool = createCouncilFinalize(tmpDir) const resultStr = await finalizeTool.execute( - { task_ids: ["bg_notags"], name: "notags" }, + { task_ids: ["bg_notags"], name: "notags", intent: "FREEFORM" }, toolContext, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -197,18 +197,18 @@ describe("council archive integration flow", () => { it("#then 2 members succeed and 1 has error 'Task output file not found'", async () => { await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_first.md"), - mockTaskOutput("Council: First", "First analysis"), + mockTaskOutput("Council: First", "First analysis: This is a detailed analysis from First model covering the full scope of the council question."), "utf-8", ) await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_third.md"), - mockTaskOutput("Council: Third", "Third analysis"), + mockTaskOutput("Council: Third", "Third analysis: This is a detailed analysis from Third model covering the full scope of the council question."), "utf-8", ) const finalizeTool = createCouncilFinalize(tmpDir) const resultStr = await finalizeTool.execute( - { task_ids: ["bg_first", "bg_missing", "bg_third"], name: "partial" }, + { task_ids: ["bg_first", "bg_missing", "bg_third"], name: "partial", intent: "FREEFORM" }, toolContext, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -241,7 +241,7 @@ describe("council archive integration flow", () => { const finalizeTool = createCouncilFinalize(tmpDir) const resultStr = await finalizeTool.execute( - { task_ids: [taskId], name: "large" }, + { task_ids: [taskId], name: "large", intent: "FREEFORM" }, toolContext, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -263,7 +263,7 @@ describe("council archive integration flow", () => { const taskId = "bg_with_meta" await writeFile( join(tmpDir, ".sisyphus", "task-outputs", `${taskId}.md`), - mockTaskOutput("Council: Opus", "Analysis result"), + mockTaskOutput("Council: Opus", "Analysis result: This is a detailed analysis from Opus model covering the full scope of the council question."), "utf-8", ) @@ -277,6 +277,7 @@ describe("council archive integration flow", () => { { task_ids: [taskId], name: "meta-test", + intent: "FREEFORM", question: "What is the best architecture for this app?", prompt_file: promptFile, }, @@ -304,7 +305,7 @@ describe("council archive integration flow", () => { const taskId = "bg_question_only" await writeFile( join(tmpDir, ".sisyphus", "task-outputs", `${taskId}.md`), - mockTaskOutput("Council: GPT", "GPT analysis"), + mockTaskOutput("Council: GPT", "GPT analysis: This is a detailed analysis from GPT model covering the full scope of the council question."), "utf-8", ) @@ -313,6 +314,7 @@ describe("council archive integration flow", () => { { task_ids: [taskId], name: "question-only", + intent: "FREEFORM", question: "How should we handle auth?", }, toolContext, diff --git a/src/tools/council-archive/council-response-extractor.test.ts b/src/tools/council-archive/council-response-extractor.test.ts index 86c29dc2e..03f2c1aa6 100644 --- a/src/tools/council-archive/council-response-extractor.test.ts +++ b/src/tools/council-archive/council-response-extractor.test.ts @@ -4,12 +4,12 @@ import { extractCouncilResponse } from "./council-response-extractor" describe("extractCouncilResponse", () => { describe("#given complete COUNCIL_MEMBER_RESPONSE tags", () => { it("#then returns has_response true, response_complete true, and the content", () => { - const result = extractCouncilResponse("analysis here") + const result = extractCouncilResponse("" + "a".repeat(100) + "") expect(result).toEqual({ has_response: true, response_complete: true, - result: "analysis here", + result: "a".repeat(100), }) }) }) @@ -39,11 +39,11 @@ describe("extractCouncilResponse", () => { }) describe("#given empty content between tags", () => { - it("#then returns has_response true, response_complete true, and empty string result", () => { + it("#then returns has_response false, response_complete true, and empty string result", () => { const result = extractCouncilResponse("") expect(result).toEqual({ - has_response: true, + has_response: false, response_complete: true, result: "", }) @@ -53,13 +53,13 @@ describe("extractCouncilResponse", () => { describe("#given multiple tag pairs", () => { it("#then returns content from the last opening tag", () => { const text = - "first analysis\nSome interim text\nfinal analysis" + `${"first".repeat(20)}\nSome interim text\n${"final".repeat(20)}` const result = extractCouncilResponse(text) expect(result).toEqual({ has_response: true, response_complete: true, - result: "final analysis", + result: "final".repeat(20), }) }) }) @@ -72,14 +72,14 @@ describe("extractCouncilResponse", () => { "Now here is my actual response:", "", "## Finding 1: Tag discussion in body", - "The extractor uses lastIndexOf to find the opening tag.", + "The extractor uses lastIndexOf to find the opening tag, which ensures the last response is extracted.", "", ].join("\n") const result = extractCouncilResponse(text) expect(result).toEqual({ has_response: true, response_complete: true, - result: "## Finding 1: Tag discussion in body\nThe extractor uses lastIndexOf to find the opening tag.", + result: "## Finding 1: Tag discussion in body\nThe extractor uses lastIndexOf to find the opening tag, which ensures the last response is extracted.", }) }) }) @@ -97,11 +97,11 @@ describe("extractCouncilResponse", () => { }) describe("#given whitespace-only content between tags", () => { - it("#then returns has_response true, response_complete true, and empty string result", () => { + it("#then returns has_response false, response_complete true, and empty string result", () => { const result = extractCouncilResponse(" ") expect(result).toEqual({ - has_response: true, + has_response: false, response_complete: true, result: "", }) @@ -110,13 +110,52 @@ describe("extractCouncilResponse", () => { describe("#given content with surrounding text before the opening tag", () => { it("#then returns only the tagged content", () => { - const text = "Some preamble text\nthe actual response" + const text = `Some preamble text\n${"a".repeat(100)}` const result = extractCouncilResponse(text) expect(result).toEqual({ has_response: true, response_complete: true, - result: "the actual response", + result: "a".repeat(100), + }) + }) + }) + + describe("#given 99-char content between tags (below MIN_RESPONSE_LENGTH)", () => { + it("#then returns has_response false, response_complete true", () => { + const content = "a".repeat(99) + const result = extractCouncilResponse(`${content}`) + + expect(result).toEqual({ + has_response: false, + response_complete: true, + result: content, + }) + }) + }) + + describe("#given 100-char content between tags (exactly MIN_RESPONSE_LENGTH)", () => { + it("#then returns has_response true, response_complete true", () => { + const content = "a".repeat(100) + const result = extractCouncilResponse(`${content}`) + + expect(result).toEqual({ + has_response: true, + response_complete: true, + result: content, + }) + }) + }) + + describe("#given 101-char content between tags (above MIN_RESPONSE_LENGTH)", () => { + it("#then returns has_response true, response_complete true", () => { + const content = "a".repeat(101) + const result = extractCouncilResponse(`${content}`) + + expect(result).toEqual({ + has_response: true, + response_complete: true, + result: content, }) }) }) diff --git a/src/tools/council-archive/council-response-extractor.ts b/src/tools/council-archive/council-response-extractor.ts index 76c439e70..67f0c91fa 100644 --- a/src/tools/council-archive/council-response-extractor.ts +++ b/src/tools/council-archive/council-response-extractor.ts @@ -1,3 +1,5 @@ +export const MIN_RESPONSE_LENGTH = 100 + export const OPENING_TAG = "" export const CLOSING_TAG = "" @@ -22,5 +24,8 @@ export function extractCouncilResponse(fullText: string): CouncilResponseExtract } const content = fullText.slice(contentStart, closingAfterLastOpen).trim() + if (content.length < MIN_RESPONSE_LENGTH) { + return { has_response: false, response_complete: true, result: content } + } return { has_response: true, response_complete: true, result: content } } diff --git a/src/tools/council-archive/create-council-finalize.test.ts b/src/tools/council-archive/create-council-finalize.test.ts index 50e1c2967..b3d1d26b4 100644 --- a/src/tools/council-archive/create-council-finalize.test.ts +++ b/src/tools/council-archive/create-council-finalize.test.ts @@ -44,9 +44,9 @@ describe("createCouncilFinalize", () => { describe("#given 3 members with valid output files", () => { it("#then creates full archive with all has_response true", async () => { const agents = [ - { id: "bg_001", agent: "Council: Claude Opus", response: "Opus analysis" }, - { id: "bg_002", agent: "Council: GPT-5", response: "GPT analysis" }, - { id: "bg_003", agent: "Council: Gemini", response: "Gemini analysis" }, + { id: "bg_001", agent: "Council: Claude Opus", response: "Opus analysis: This is a detailed analysis from Opus model covering the full scope of the council question." }, + { id: "bg_002", agent: "Council: GPT-5", response: "GPT analysis: This is a detailed analysis from GPT model covering the full scope of the council question." }, + { id: "bg_003", agent: "Council: Gemini", response: "Gemini analysis: This is a detailed analysis from Gemini model covering the full scope of the council question." }, ] for (const a of agents) { @@ -59,7 +59,7 @@ describe("createCouncilFinalize", () => { const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: agents.map((a) => a.id), name: "test" }, + { task_ids: agents.map((a) => a.id), name: "test", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -73,14 +73,12 @@ describe("createCouncilFinalize", () => { expect(member.task_id).toBe(agents[i].id) expect(member.has_response).toBe(true) expect(member.response_complete).toBe(true) - expect(member).not.toHaveProperty("result") - expect(member).not.toHaveProperty("result_truncated") expect(member.error).toBeUndefined() expect(member.archive_file).toBeDefined() } const opusArchive = await readFile(join(tmpDir, result.members[0].archive_file!), "utf-8") - expect(opusArchive).toBe("Opus analysis") + expect(opusArchive).toBe("Opus analysis: This is a detailed analysis from Opus model covering the full scope of the council question.") const metaContent = await readFile(join(tmpDir, result.meta_file), "utf-8") expect(metaContent).toContain("archive_name: council-test-") @@ -96,18 +94,18 @@ describe("createCouncilFinalize", () => { it("#then returns partial success with error for missing member", async () => { await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_001.md"), - mockTaskOutput("Council: Claude Opus", "Opus findings"), + mockTaskOutput("Council: Claude Opus", "Opus findings: This is a detailed analysis from Opus model covering the full scope of the council question."), "utf-8", ) await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_003.md"), - mockTaskOutput("Council: Gemini", "Gemini findings"), + mockTaskOutput("Council: Gemini", "Gemini findings: This is a detailed analysis from Gemini model covering the full scope of the council question."), "utf-8", ) const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: ["bg_001", "bg_002", "bg_003"], name: "partial" }, + { task_ids: ["bg_001", "bg_002", "bg_003"], name: "partial", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -116,8 +114,6 @@ describe("createCouncilFinalize", () => { expect(result.members[0].has_response).toBe(true) expect(result.members[0].archive_file).toBeDefined() - expect(result.members[0]).not.toHaveProperty("result") - expect(result.members[0]).not.toHaveProperty("result_truncated") expect(result.members[1].has_response).toBe(false) expect(result.members[1].error).toBe("Task output file not found") @@ -125,8 +121,6 @@ describe("createCouncilFinalize", () => { expect(result.members[2].has_response).toBe(true) expect(result.members[2].archive_file).toBeDefined() - expect(result.members[2]).not.toHaveProperty("result") - expect(result.members[2]).not.toHaveProperty("result_truncated") }) }) @@ -141,7 +135,7 @@ describe("createCouncilFinalize", () => { const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: ["bg_large"], name: "large" }, + { task_ids: ["bg_large"], name: "large", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -149,8 +143,6 @@ describe("createCouncilFinalize", () => { const member = result.members[0] expect(member.has_response).toBe(true) expect(member.archive_file).toBeDefined() - expect(member).not.toHaveProperty("result") - expect(member).not.toHaveProperty("result_truncated") const archiveContent = await readFile(join(tmpDir, member.archive_file!), "utf-8") expect(archiveContent).toHaveLength(9000) @@ -159,7 +151,7 @@ describe("createCouncilFinalize", () => { }) describe("#given empty response between tags", () => { - it("#then returns has_response true with empty string result", async () => { + it("#then returns has_response false with empty string result", async () => { const emptyOutput = [ "---", "task_id: bg_empty", @@ -182,17 +174,13 @@ describe("createCouncilFinalize", () => { const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: ["bg_empty"], name: "empty" }, + { task_ids: ["bg_empty"], name: "empty", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) const member = result.members[0] - expect(member.has_response).toBe(true) - expect(member.response_complete).toBe(true) - expect(member.archive_file).toBeDefined() - expect(member).not.toHaveProperty("result") - expect(member).not.toHaveProperty("result_truncated") + expect(member.has_response).toBe(false) }) }) @@ -200,18 +188,18 @@ describe("createCouncilFinalize", () => { it("#then each member gets a unique archive_file keyed by task id", async () => { await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_alpha.md"), - mockTaskOutput("Agent A+B", "First response"), + mockTaskOutput("Agent A+B", "First response: This is a detailed analysis from Agent A+B covering the full scope of the council question."), "utf-8", ) await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_beta.md"), - mockTaskOutput("Agent A B", "Second response"), + mockTaskOutput("Agent A B", "Second response: This is a detailed analysis from Agent A B covering the full scope of the council question."), "utf-8", ) const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: ["bg_alpha", "bg_beta"], name: "collision" }, + { task_ids: ["bg_alpha", "bg_beta"], name: "collision", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -233,7 +221,7 @@ describe("createCouncilFinalize", () => { it("#then registers critical custom context for Athena runtime guidance", async () => { await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_intent.md"), - mockTaskOutput("Council: GPT-5", "Plan proposal"), + mockTaskOutput("Council: GPT-5", "Plan proposal: This is a detailed analysis from GPT model covering the full scope of the council question."), "utf-8", ) @@ -273,7 +261,7 @@ describe("createCouncilFinalize", () => { it("#then emits diagnose action options for hephaestus and sisyphus", async () => { await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_diagnose.md"), - mockTaskOutput("Council: Claude", "Root cause found"), + mockTaskOutput("Council: Claude", "Root cause found: This is a detailed analysis from Claude model covering the full scope of the council question."), "utf-8", ) @@ -305,7 +293,7 @@ describe("createCouncilFinalize", () => { it("#then emits audit processing mode and batching guidance", async () => { await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_audit.md"), - mockTaskOutput("Council: Claude", "Audit findings"), + mockTaskOutput("Council: Claude", "Audit findings: This is a detailed analysis from Claude model covering the full scope of the council question."), "utf-8", ) @@ -352,7 +340,7 @@ describe("createCouncilFinalize", () => { it("#then emits informational write-to-document path without atlas delegation", async () => { await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_eval.md"), - mockTaskOutput("Council: Claude", "Option comparison"), + mockTaskOutput("Council: Claude", "Option comparison: This is a detailed analysis from Claude model covering the full scope of the council question."), "utf-8", ) @@ -394,13 +382,13 @@ describe("createCouncilFinalize", () => { it("#then rejects task IDs with path traversal characters", async () => { await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_valid.md"), - mockTaskOutput("Agent", "Valid response"), + mockTaskOutput("Agent", "Valid response: This is a detailed analysis from Agent covering the full scope of the council question."), "utf-8", ) const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: ["../../etc/passwd", "bg_valid", "foo/bar"], name: "traversal" }, + { task_ids: ["../../etc/passwd", "bg_valid", "foo/bar"], name: "traversal", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -416,13 +404,13 @@ describe("createCouncilFinalize", () => { it("#then sanitizes name with path traversal characters", async () => { await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_safe.md"), - mockTaskOutput("Agent", "Response"), + mockTaskOutput("Agent", "Response: This is a detailed analysis from Agent covering the full scope of the council question here."), "utf-8", ) const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: ["bg_safe"], name: "../../etc" }, + { task_ids: ["bg_safe"], name: "../../etc", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -432,11 +420,11 @@ describe("createCouncilFinalize", () => { }) it("#then sanitizes name with absolute path", async () => { - await writeFile(join(tmpDir, ".sisyphus", "task-outputs", "bg_abs.md"), mockTaskOutput("Agent", "Response"), "utf-8") + await writeFile(join(tmpDir, ".sisyphus", "task-outputs", "bg_abs.md"), mockTaskOutput("Agent", "Response: This is a detailed analysis from Agent covering the full scope of the council question here."), "utf-8") const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: ["bg_abs"], name: "/absolute/path" }, + { task_ids: ["bg_abs"], name: "/absolute/path", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -446,11 +434,11 @@ describe("createCouncilFinalize", () => { }) it("#then sanitizes name with dot-dot-slash in the middle", async () => { - await writeFile(join(tmpDir, ".sisyphus", "task-outputs", "bg_mid.md"), mockTaskOutput("Agent", "Response"), "utf-8") + await writeFile(join(tmpDir, ".sisyphus", "task-outputs", "bg_mid.md"), mockTaskOutput("Agent", "Response: This is a detailed analysis from Agent covering the full scope of the council question here."), "utf-8") const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: ["bg_mid"], name: "foo/../bar" }, + { task_ids: ["bg_mid"], name: "foo/../bar", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -460,11 +448,11 @@ describe("createCouncilFinalize", () => { }) it("#then accepts valid task IDs", async () => { - await writeFile(join(tmpDir, ".sisyphus", "task-outputs", "valid-id_123.md"), mockTaskOutput("Agent", "Response"), "utf-8") + await writeFile(join(tmpDir, ".sisyphus", "task-outputs", "valid-id_123.md"), mockTaskOutput("Agent", "Response: This is a detailed analysis from Agent covering the full scope of the council question here."), "utf-8") const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: ["valid-id_123"], name: "valid" }, + { task_ids: ["valid-id_123"], name: "valid", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -477,13 +465,13 @@ describe("createCouncilFinalize", () => { it("#then rejects prompt_file with absolute path outside workspace", async () => { await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_prompt.md"), - mockTaskOutput("Agent", "Response"), + mockTaskOutput("Agent", "Response: This is a detailed analysis from Agent covering the full scope of the council question here."), "utf-8", ) const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: ["bg_prompt"], name: "prompt-test", prompt_file: "/etc/passwd" }, + { task_ids: ["bg_prompt"], name: "prompt-test", prompt_file: "/etc/passwd", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -496,13 +484,13 @@ describe("createCouncilFinalize", () => { it("#then rejects prompt_file with relative traversal", async () => { await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_rel.md"), - mockTaskOutput("Agent", "Response"), + mockTaskOutput("Agent", "Response: This is a detailed analysis from Agent covering the full scope of the council question here."), "utf-8", ) const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: ["bg_rel"], name: "rel-test", prompt_file: "../../etc/passwd" }, + { task_ids: ["bg_rel"], name: "rel-test", prompt_file: "../../etc/passwd", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -517,13 +505,13 @@ describe("createCouncilFinalize", () => { await writeFile(join(tmpDir, ".sisyphus", "tmp", "athena-council-test.md"), "Test prompt", "utf-8") await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_ok.md"), - mockTaskOutput("Agent", "Response"), + mockTaskOutput("Agent", "Response: This is a detailed analysis from Agent covering the full scope of the council question here."), "utf-8", ) const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: ["bg_ok"], name: "valid-prompt", prompt_file: ".sisyphus/tmp/athena-council-test.md" }, + { task_ids: ["bg_ok"], name: "valid-prompt", prompt_file: ".sisyphus/tmp/athena-council-test.md", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -535,7 +523,7 @@ describe("createCouncilFinalize", () => { it("#then rejects task IDs with backslash characters", async () => { const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: ["bg\\evil"], name: "backslash" }, + { task_ids: ["bg\\evil"], name: "backslash", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) @@ -548,13 +536,13 @@ describe("createCouncilFinalize", () => { it("#then handles name that slugifies to empty string", async () => { await writeFile( join(tmpDir, ".sisyphus", "task-outputs", "bg_empty_name.md"), - mockTaskOutput("Agent", "Response"), + mockTaskOutput("Agent", "Response: This is a detailed analysis from Agent covering the full scope of the council question here."), "utf-8", ) const toolDef = createCouncilFinalize(tmpDir) const resultStr = await toolDef.execute( - { task_ids: ["bg_empty_name"], name: "///..." }, + { task_ids: ["bg_empty_name"], name: "///...", intent: "FREEFORM" }, mockCtx, ) const result: CouncilFinalizeResult = JSON.parse(resultStr) diff --git a/src/tools/council-archive/create-council-finalize.ts b/src/tools/council-archive/create-council-finalize.ts index f0f9d0478..30af61361 100644 --- a/src/tools/council-archive/create-council-finalize.ts +++ b/src/tools/council-archive/create-council-finalize.ts @@ -1,84 +1,24 @@ import { tool, type ToolDefinition } from "@opencode-ai/plugin" -import { readFile, writeFile, mkdir, rename } from "node:fs/promises" -import { join, isAbsolute, resolve, relative } from "node:path" +import { readFile, writeFile, mkdir } from "node:fs/promises" +import { join } from "node:path" import { randomBytes } from "node:crypto" import { extractCouncilResponse } from "./council-response-extractor" +import { TASK_ID_PATTERN, slugify, toPosixPath, extractAgentFromFrontmatter, isPathEscaping, movePromptFile } from "./council-finalize-helpers" +import { formatMetaYaml, type MetaMember } from "./meta-yaml-formatter" import { buildAthenaRuntimeGuidance, getValidCouncilIntents, resolveCouncilIntent, + COUNCIL_DEFAULTS, } from "../../agents/athena" import { log } from "../../shared/logger" import type { ContextCollector } from "../../features/context-injector" import type { CouncilFinalizeArgs, CouncilMemberResult, CouncilFinalizeResult } from "./types" - -interface MetaMember { - task_id: string - member: string - member_slug: string - task_output_path: string - archive_file: string - has_response: boolean - response_complete: boolean -} - type RegisterContext = Pick type CouncilFinalizeToolContext = { sessionID?: string } - -const TASK_ID_PATTERN = /^[a-zA-Z0-9_-]+$/ -function slugify(text: string): string { - return text - .toLowerCase() - .replace(/[^a-z0-9]+/g, "-") - .replace(/^-+|-+$/g, "") -} - -function toPosixPath(pathValue: string): string { - return pathValue.replace(/\\/g, "/") -} - -function extractAgentFromFrontmatter(content: string): string | null { - const fmMatch = content.match(/^---\n([\s\S]*?)\n---/) - if (!fmMatch) return null - const agentLine = fmMatch[1].match(/^agent:\s*(.+)$/m) - return agentLine ? agentLine[1].trim() : null -} - -function formatMetaYaml(archiveName: string, createdAt: string, members: MetaMember[], question?: string, promptFile?: string): string { - const lines: string[] = [ - `archive_name: ${archiveName}`, - `created_at: ${createdAt}`, - ] - - if (question) { - lines.push(`question: |`) - for (const qLine of question.split("\n")) { - lines.push(` ${qLine}`) - } - } - - if (promptFile) { - lines.push(`prompt_file: ${promptFile}`) - } - - lines.push("members:") - - for (const m of members) { - lines.push(` - task_id: ${m.task_id}`) - lines.push(` member: "${m.member}"`) - lines.push(` member_slug: ${m.member_slug}`) - lines.push(` task_output_path: ${m.task_output_path}`) - lines.push(` archive_file: ${m.archive_file}`) - lines.push(` has_response: ${m.has_response}`) - lines.push(` response_complete: ${m.response_complete}`) - } - - return lines.join("\n") + "\n" -} - export function createCouncilFinalize( basePath?: string, options?: { contextCollector?: RegisterContext } @@ -95,14 +35,13 @@ export function createCouncilFinalize( name: tool.schema.string().describe("Council name used in the archive directory name"), intent: tool.schema .string() - .optional() .describe(`Classified question intent used for runtime Athena guidance injection. Valid intents: ${getValidCouncilIntents().join(", ")}`), question: tool.schema.string().optional().describe("Original user question that triggered the council"), prompt_file: tool.schema.string().optional().describe("Path to the council prompt temp file (will be moved into the archive)"), }, async execute(args: CouncilFinalizeArgs, toolContext: CouncilFinalizeToolContext) { const resolvedIntent = resolveCouncilIntent(args.intent) - if (args.intent && !resolvedIntent) { + if (!resolvedIntent) { return `Invalid intent: "${args.intent}". Valid intents: ${getValidCouncilIntents().join(", ")}.` } @@ -120,16 +59,14 @@ export function createCouncilFinalize( } const base = basePath ?? process.cwd() - const hexId = randomBytes(2).toString("hex") + const hexId = randomBytes(COUNCIL_DEFAULTS.ARCHIVE_ID_BYTES).toString("hex") const safeName = slugify(args.name) || "unnamed" const archiveName = `council-${safeName}-${hexId}` const relArchiveDir = join(".sisyphus", "athena", archiveName) const relArchiveDirForOutput = toPosixPath(relArchiveDir) const absArchiveDir = join(base, relArchiveDir) - const expectedArchiveRoot = join(base, ".sisyphus", "athena") - const relFromArchiveRoot = relative(expectedArchiveRoot, absArchiveDir) - if (relFromArchiveRoot.startsWith("..") || isAbsolute(relFromArchiveRoot)) { + if (isPathEscaping(join(base, ".sisyphus", "athena"), absArchiveDir)) { return `Security error: archive directory would escape .sisyphus/athena/` } @@ -162,9 +99,7 @@ export function createCouncilFinalize( const relTaskOutputForOutput = toPosixPath(relTaskOutput) const absTaskOutput = join(base, relTaskOutput) - const expectedTaskOutputRoot = join(base, ".sisyphus", "task-outputs") - const relFromTaskOutputRoot = relative(expectedTaskOutputRoot, absTaskOutput) - if (relFromTaskOutputRoot.startsWith("..") || isAbsolute(relFromTaskOutputRoot)) { + if (isPathEscaping(join(base, ".sisyphus", "task-outputs"), absTaskOutput)) { members.push({ task_id: taskId, member: "unknown", @@ -241,27 +176,9 @@ export function createCouncilFinalize( }) } - let relPromptFile: string | undefined - if (args.prompt_file) { - try { - const promptFilename = "council-prompt.md" - const absPromptSrc = isAbsolute(args.prompt_file) ? args.prompt_file : resolve(base, args.prompt_file) - const expectedPromptRoot = join(base, ".sisyphus", "tmp") - const relFromPromptRoot = relative(expectedPromptRoot, absPromptSrc) - if (relFromPromptRoot.startsWith("..") || isAbsolute(relFromPromptRoot)) { - log("[council-finalize] Rejected prompt_file outside .sisyphus/tmp/", { promptFile: args.prompt_file }) - } else { - const absPromptDest = join(absArchiveDir, promptFilename) - await rename(absPromptSrc, absPromptDest).catch(async () => { - const content = await readFile(absPromptSrc, "utf-8") - await writeFile(absPromptDest, content, "utf-8") - }) - relPromptFile = toPosixPath(join(relArchiveDir, promptFilename)) - } - } catch (err) { - log("[council-finalize] Failed to move prompt file", { promptFile: args.prompt_file, error: String(err) }) - } - } + const relPromptFile = args.prompt_file + ? await movePromptFile(args.prompt_file, base, absArchiveDir, relArchiveDir) + : undefined const relMetaFile = join(relArchiveDir, "meta.yaml") const relMetaFileForOutput = toPosixPath(relMetaFile) diff --git a/src/tools/council-archive/index.ts b/src/tools/council-archive/index.ts index d0258cdc4..7ac1bf6e5 100644 --- a/src/tools/council-archive/index.ts +++ b/src/tools/council-archive/index.ts @@ -1,4 +1,4 @@ export { createCouncilFinalize } from "./create-council-finalize" -export { extractCouncilResponse, OPENING_TAG, CLOSING_TAG } from "./council-response-extractor" +export { extractCouncilResponse, OPENING_TAG, CLOSING_TAG, MIN_RESPONSE_LENGTH } from "./council-response-extractor" export type { CouncilResponseExtraction } from "./council-response-extractor" export type { CouncilFinalizeArgs, CouncilMemberResult, CouncilFinalizeResult } from "./types" diff --git a/src/tools/council-archive/meta-yaml-formatter.ts b/src/tools/council-archive/meta-yaml-formatter.ts new file mode 100644 index 000000000..081f79882 --- /dev/null +++ b/src/tools/council-archive/meta-yaml-formatter.ts @@ -0,0 +1,41 @@ +export interface MetaMember { + task_id: string + member: string + member_slug: string + task_output_path: string + archive_file: string + has_response: boolean + response_complete: boolean +} + +export function formatMetaYaml(archiveName: string, createdAt: string, members: MetaMember[], question?: string, promptFile?: string): string { + const lines: string[] = [ + `archive_name: ${archiveName}`, + `created_at: ${createdAt}`, + ] + + if (question) { + lines.push(`question: |`) + for (const qLine of question.split("\n")) { + lines.push(` ${qLine}`) + } + } + + if (promptFile) { + lines.push(`prompt_file: ${promptFile}`) + } + + lines.push("members:") + + for (const m of members) { + lines.push(` - task_id: ${m.task_id}`) + lines.push(` member: "${m.member}"`) + lines.push(` member_slug: ${m.member_slug}`) + lines.push(` task_output_path: ${m.task_output_path}`) + lines.push(` archive_file: ${m.archive_file}`) + lines.push(` has_response: ${m.has_response}`) + lines.push(` response_complete: ${m.response_complete}`) + } + + return lines.join("\n") + "\n" +} diff --git a/src/tools/council-archive/types.ts b/src/tools/council-archive/types.ts index ae6e3d598..eea6fc1f0 100644 --- a/src/tools/council-archive/types.ts +++ b/src/tools/council-archive/types.ts @@ -1,7 +1,7 @@ export interface CouncilFinalizeArgs { task_ids: string[] name: string - intent?: string + intent: string question?: string prompt_file?: string } diff --git a/src/tools/prepare-council-prompt/tools.test.ts b/src/tools/prepare-council-prompt/tools.test.ts index 40b6dfb0a..99fee73a0 100644 --- a/src/tools/prepare-council-prompt/tools.test.ts +++ b/src/tools/prepare-council-prompt/tools.test.ts @@ -197,5 +197,33 @@ describe("createPrepareCouncilPromptTool", () => { expect(intentIdx).toBeLessThan(questionIdx) }) }) + + describe("#when called with lowercase intent 'audit'", () => { + it("#then resolves correctly and produces file with AUDIT addendum", async () => { + tmpDir = await mkdtemp(join(tmpdir(), "council-test-")) + const toolDef = createPrepareCouncilPromptTool(tmpDir) + const result = await toolDef.execute({ prompt: "Review security", intent: "audit" }, mockContext) + + expect(result).not.toContain("Invalid intent") + expect(result).toContain("intent: AUDIT") + const filePath = extractFilePath(result) + const content = await readFile(filePath, "utf-8") + expect(content).toContain("## Analysis Intent: AUDIT") + }) + }) + + describe("#when called with mixed case intent 'Diagnose'", () => { + it("#then resolves correctly and produces file with DIAGNOSE addendum", async () => { + tmpDir = await mkdtemp(join(tmpdir(), "council-test-")) + const toolDef = createPrepareCouncilPromptTool(tmpDir) + const result = await toolDef.execute({ prompt: "Why is the API failing?", intent: "Diagnose" }, mockContext) + + expect(result).not.toContain("Invalid intent") + expect(result).toContain("intent: DIAGNOSE") + const filePath = extractFilePath(result) + const content = await readFile(filePath, "utf-8") + expect(content).toContain("## Analysis Intent: DIAGNOSE") + }) + }) }) }) diff --git a/src/tools/prepare-council-prompt/tools.ts b/src/tools/prepare-council-prompt/tools.ts index e052d4ca8..e48c67667 100644 --- a/src/tools/prepare-council-prompt/tools.ts +++ b/src/tools/prepare-council-prompt/tools.ts @@ -3,9 +3,9 @@ import { randomUUID } from "node:crypto" import { writeFile, unlink, mkdir, readdir, stat } from "node:fs/promises" import { join } from "node:path" import { log } from "../../shared/logger" -import { COUNCIL_SOLO_ADDENDUM, COUNCIL_DELEGATION_ADDENDUM, COUNCIL_INTENT_ADDENDUMS, getValidCouncilIntents, type CouncilIntent } from "../../agents/athena" +import { COUNCIL_SOLO_ADDENDUM, COUNCIL_DELEGATION_ADDENDUM, COUNCIL_INTENT_ADDENDUMS, getValidCouncilIntents, resolveCouncilIntent, type CouncilIntent, COUNCIL_DEFAULTS } from "../../agents/athena" -const CLEANUP_DELAY_MS = 30 * 60 * 1000 +const CLEANUP_DELAY_MS = COUNCIL_DEFAULTS.CLEANUP_DELAY_MS const COUNCIL_TMP_DIR = ".sisyphus/tmp" const COUNCIL_FILE_PREFIX = "athena-council-" @@ -24,12 +24,18 @@ async function cleanupStaleTempFiles(directory: string): Promise { await unlink(filePath) log("[prepare-council-prompt] Cleaned up stale temp file", { filePath }) } - } catch { - // File may have been deleted between readdir and stat + } catch (err: unknown) { + const code = (err as NodeJS.ErrnoException).code + if (code !== "ENOENT") { + log("[prepare-council-prompt] Unexpected error during temp file cleanup", { filePath, error: String(err), code }) + } } } - } catch { - // Directory may not exist yet — nothing to clean + } catch (err: unknown) { + const code = (err as NodeJS.ErrnoException).code + if (code !== "ENOENT") { + log("[prepare-council-prompt] Unexpected error reading temp directory", { directory: tmpDir, error: String(err), code }) + } } } @@ -78,12 +84,15 @@ Returns the file path to reference in subsequent task() calls.` return `Invalid mode: "${args.mode}". Valid modes: "solo", "delegation".` } - const validIntents = getValidCouncilIntents() - if (args.intent !== undefined && !(validIntents as readonly string[]).includes(args.intent.toUpperCase())) { - return `Invalid intent: "${args.intent}". Valid intents: ${validIntents.map((i) => `"${i}"`).join(", ")}.` + if (args.intent !== undefined) { + const resolved = resolveCouncilIntent(args.intent) + if (!resolved) { + const validIntents = getValidCouncilIntents() + return `Invalid intent: "${args.intent}". Valid intents: ${validIntents.map((i) => `"${i}"`).join(", ")}.` + } } - const resolvedIntent = args.intent?.toUpperCase() + const resolvedIntent = args.intent ? resolveCouncilIntent(args.intent) : undefined const mode = args.mode === "delegation" ? "delegation" : "solo"