mirror of
https://mirror.skon.top/github.com/code-yeongyu/oh-my-opencode
synced 2026-04-23 02:23:52 +08:00
refactor(athena): remove redundant council_read tool, use Read instead
council_read was a sandboxed readFile wrapper for .sisyphus/ archives. Now that council_finalize extracts member responses into plain files, the standard Read tool serves the same purpose. Removes the tool, its tests, and updates Athena's prompt to reference Read directly.
This commit is contained in:
@@ -31,7 +31,7 @@ import {
|
||||
createHashlineEditTool,
|
||||
createPrepareCouncilPromptTool,
|
||||
} from "../tools"
|
||||
import { createCouncilFinalize, createCouncilRead } from "../tools/council-archive"
|
||||
import { createCouncilFinalize } from "../tools/council-archive"
|
||||
import { contextCollector } from "../features/context-injector"
|
||||
import { getMainSessionID } from "../features/claude-code-session-state"
|
||||
import { filterDisabledTools } from "../shared/disabled-tools"
|
||||
@@ -283,7 +283,6 @@ export function createToolRegistry(args: {
|
||||
...hashlineToolsRecord,
|
||||
prepare_council_prompt: createPrepareCouncilPromptTool(ctx.directory),
|
||||
council_finalize: createCouncilFinalize(ctx.directory, { contextCollector }),
|
||||
council_read: createCouncilRead(ctx.directory),
|
||||
}
|
||||
|
||||
for (const toolDefinition of Object.values(allTools)) {
|
||||
|
||||
@@ -5,7 +5,6 @@ import { mkdtemp, mkdir, writeFile, readFile, rm, stat } from "node:fs/promises"
|
||||
import { join } from "node:path"
|
||||
import { tmpdir } from "node:os"
|
||||
import { createCouncilFinalize } from "./create-council-finalize"
|
||||
import { createCouncilRead } from "./create-council-read"
|
||||
import type { CouncilFinalizeResult } from "./types"
|
||||
import type { BackgroundTask } from "../../features/background-agent"
|
||||
import type { BackgroundOutputManager } from "../background-task/clients"
|
||||
@@ -88,7 +87,7 @@ afterEach(async () => {
|
||||
describe("council archive integration flow", () => {
|
||||
describe("#given 3 council members with valid output files", () => {
|
||||
describe("#when finalize is called and then each archive is read", () => {
|
||||
it("#then creates archive with correct structure and council_read extracts content from archives", async () => {
|
||||
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" },
|
||||
@@ -137,23 +136,13 @@ describe("council archive integration flow", () => {
|
||||
expect(archiveContent).toBe(agents[i].response)
|
||||
}
|
||||
|
||||
const readTool = createCouncilRead(tmpDir)
|
||||
|
||||
for (let i = 0; i < agents.length; i++) {
|
||||
const readResult = await readTool.execute({ file_path: result.members[i].archive_file! }, toolContext)
|
||||
const parsed = JSON.parse(readResult)
|
||||
|
||||
expect(parsed.has_response).toBe(true)
|
||||
expect(parsed.response_complete).toBe(true)
|
||||
expect(parsed.result).toBe(agents[i].response)
|
||||
}
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a member with incomplete tags (no closing tag)", () => {
|
||||
describe("#when finalize is called and archive is read", () => {
|
||||
it("#then finalize marks incomplete but council_read still returns raw archived content", async () => {
|
||||
it("#then finalize marks incomplete but archive still contains raw content", async () => {
|
||||
const taskId = "bg_partial"
|
||||
await writeFile(
|
||||
join(tmpDir, ".sisyphus", "task-outputs", `${taskId}.md`),
|
||||
@@ -176,13 +165,6 @@ describe("council archive integration flow", () => {
|
||||
const archiveContent = await readFile(join(tmpDir, member.archive_file!), "utf-8")
|
||||
expect(archiveContent).toBe("Analysis still in progress...")
|
||||
|
||||
const readTool = createCouncilRead(tmpDir)
|
||||
const readResult = await readTool.execute({ file_path: member.archive_file! }, toolContext)
|
||||
const parsed = JSON.parse(readResult)
|
||||
|
||||
expect(parsed.has_response).toBe(true)
|
||||
expect(parsed.response_complete).toBe(true)
|
||||
expect(parsed.result).toBe("Analysis still in progress...")
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -248,7 +230,7 @@ describe("council archive integration flow", () => {
|
||||
|
||||
describe("#given a very large council response exceeding 8000 chars", () => {
|
||||
describe("#when finalize is called and then archive is read", () => {
|
||||
it("#then finalize stores full output and council_read returns full response", async () => {
|
||||
it("#then finalize stores full output and archive contains full response", async () => {
|
||||
const largeResponse = "A".repeat(9000)
|
||||
const taskId = "bg_large"
|
||||
await writeFile(
|
||||
@@ -271,13 +253,6 @@ describe("council archive integration flow", () => {
|
||||
const fullContent = await readFile(join(tmpDir, member.archive_file!), "utf-8")
|
||||
expect(fullContent).toHaveLength(9000)
|
||||
|
||||
const readTool = createCouncilRead(tmpDir)
|
||||
const readResult = await readTool.execute({ file_path: member.archive_file! }, toolContext)
|
||||
const parsed = JSON.parse(readResult)
|
||||
|
||||
expect(parsed.has_response).toBe(true)
|
||||
expect(parsed.response_complete).toBe(true)
|
||||
expect(parsed.result).toHaveLength(9000)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -5,7 +5,6 @@ import { mkdtemp, mkdir, writeFile, readFile } from "node:fs/promises"
|
||||
import { join } from "node:path"
|
||||
import { tmpdir } from "node:os"
|
||||
import { createCouncilFinalize } from "./create-council-finalize"
|
||||
import { createCouncilRead } from "./create-council-read"
|
||||
import type { CouncilFinalizeResult } from "./types"
|
||||
|
||||
function mockTaskOutput(agent: string, responseBody: string, complete = true): string {
|
||||
@@ -132,7 +131,7 @@ describe("createCouncilFinalize", () => {
|
||||
})
|
||||
|
||||
describe("#given large response exceeding 8000 chars", () => {
|
||||
it("#then keeps full content in archive and council_read returns full response", async () => {
|
||||
it("#then keeps full content in archive readable via readFile", async () => {
|
||||
const largeResponse = "x".repeat(9000)
|
||||
await writeFile(
|
||||
join(tmpDir, ".sisyphus", "task-outputs", "bg_large.md"),
|
||||
@@ -153,14 +152,9 @@ describe("createCouncilFinalize", () => {
|
||||
expect(member).not.toHaveProperty("result")
|
||||
expect(member).not.toHaveProperty("result_truncated")
|
||||
|
||||
const readTool = createCouncilRead(tmpDir)
|
||||
const readResult = await readTool.execute({ file_path: member.archive_file! }, mockCtx)
|
||||
const parsed = JSON.parse(readResult)
|
||||
|
||||
expect(parsed.has_response).toBe(true)
|
||||
expect(parsed.response_complete).toBe(true)
|
||||
expect(parsed.result).toHaveLength(9000)
|
||||
expect(parsed.result).toBe(largeResponse)
|
||||
const archiveContent = await readFile(join(tmpDir, member.archive_file!), "utf-8")
|
||||
expect(archiveContent).toHaveLength(9000)
|
||||
expect(archiveContent).toBe(largeResponse)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -1,129 +0,0 @@
|
||||
/// <reference types="bun-types" />
|
||||
|
||||
import { describe, expect, it, beforeEach, afterEach } from "bun:test"
|
||||
import { mkdtemp, writeFile, mkdir, rm } from "node:fs/promises"
|
||||
import { tmpdir } from "node:os"
|
||||
import { join } from "node:path"
|
||||
import { createCouncilRead } from "./create-council-read"
|
||||
|
||||
let tempDir: string
|
||||
let sisyphusDir: string
|
||||
|
||||
beforeEach(async () => {
|
||||
tempDir = await mkdtemp(join(tmpdir(), "council-read-test-"))
|
||||
sisyphusDir = join(tempDir, ".sisyphus")
|
||||
await mkdir(sisyphusDir, { recursive: true })
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await rm(tempDir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
const toolContext = {
|
||||
sessionID: "test-session",
|
||||
messageID: "test-message",
|
||||
agent: "test-agent",
|
||||
abort: new AbortController().signal,
|
||||
}
|
||||
|
||||
describe("createCouncilRead", () => {
|
||||
describe("#given an archive file with clean extracted content", () => {
|
||||
it("#then returns has_response true, response_complete true, and raw file content", async () => {
|
||||
const archivePath = join(sisyphusDir, "member-1.txt")
|
||||
await writeFile(archivePath, "Full analysis here")
|
||||
|
||||
const tool = createCouncilRead(tempDir)
|
||||
const relativePath = `.sisyphus/member-1.txt`
|
||||
|
||||
const result = await tool.execute({ file_path: relativePath }, toolContext)
|
||||
const parsed = JSON.parse(result)
|
||||
|
||||
expect(parsed.has_response).toBe(true)
|
||||
expect(parsed.response_complete).toBe(true)
|
||||
expect(parsed.result).toBe("Full analysis here")
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given an archive file containing tag-like text", () => {
|
||||
it("#then returns raw content without re-parsing tags", async () => {
|
||||
const archivePath = join(sisyphusDir, "member-2.txt")
|
||||
await writeFile(archivePath, "<COUNCIL_MEMBER_RESPONSE>do not parse this</COUNCIL_MEMBER_RESPONSE>")
|
||||
|
||||
const tool = createCouncilRead(tempDir)
|
||||
const relativePath = `.sisyphus/member-2.txt`
|
||||
|
||||
const result = await tool.execute({ file_path: relativePath }, toolContext)
|
||||
const parsed = JSON.parse(result)
|
||||
|
||||
expect(parsed.has_response).toBe(true)
|
||||
expect(parsed.response_complete).toBe(true)
|
||||
expect(parsed.result).toBe("<COUNCIL_MEMBER_RESPONSE>do not parse this</COUNCIL_MEMBER_RESPONSE>")
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given an archive file with empty content", () => {
|
||||
it("#then returns empty result content", async () => {
|
||||
const archivePath = join(sisyphusDir, "member-3.txt")
|
||||
await writeFile(archivePath, "")
|
||||
|
||||
const tool = createCouncilRead(tempDir)
|
||||
const relativePath = `.sisyphus/member-3.txt`
|
||||
|
||||
const result = await tool.execute({ file_path: relativePath }, toolContext)
|
||||
const parsed = JSON.parse(result)
|
||||
|
||||
expect(parsed.has_response).toBe(true)
|
||||
expect(parsed.response_complete).toBe(true)
|
||||
expect(parsed.result).toBe("")
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a path outside .sisyphus/", () => {
|
||||
it("#then returns Access denied error", async () => {
|
||||
const tool = createCouncilRead(tempDir)
|
||||
const result = await tool.execute({ file_path: "/etc/passwd" }, toolContext)
|
||||
const parsed = JSON.parse(result)
|
||||
|
||||
expect(parsed.error).toBe("Access denied: path must be within .sisyphus/")
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a missing file within .sisyphus/", () => {
|
||||
it("#then returns has_response false with File not found error", async () => {
|
||||
const tool = createCouncilRead(tempDir)
|
||||
const relativePath = `.sisyphus/nonexistent-file.txt`
|
||||
|
||||
const result = await tool.execute({ file_path: relativePath }, toolContext)
|
||||
const parsed = JSON.parse(result)
|
||||
|
||||
expect(parsed.has_response).toBe(false)
|
||||
expect(parsed.error).toContain("File not found")
|
||||
expect(parsed.error).toContain(relativePath)
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a path traversal attempt", () => {
|
||||
it("#then returns Access denied error", async () => {
|
||||
const tool = createCouncilRead(tempDir)
|
||||
const result = await tool.execute({ file_path: "../../../etc/passwd" }, toolContext)
|
||||
const parsed = JSON.parse(result)
|
||||
|
||||
expect(parsed.error).toBe("Access denied: path must be within .sisyphus/")
|
||||
})
|
||||
})
|
||||
|
||||
describe("#given a Windows-style path under .sisyphus", () => {
|
||||
it("#then normalizes separators and reads the file successfully", async () => {
|
||||
const archivePath = join(sisyphusDir, "windows-path.txt")
|
||||
await writeFile(archivePath, "Windows path response")
|
||||
|
||||
const tool = createCouncilRead(tempDir)
|
||||
const result = await tool.execute({ file_path: ".sisyphus\\windows-path.txt" }, toolContext)
|
||||
const parsed = JSON.parse(result)
|
||||
|
||||
expect(parsed.has_response).toBe(true)
|
||||
expect(parsed.response_complete).toBe(true)
|
||||
expect(parsed.result).toBe("Windows path response")
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -1,42 +0,0 @@
|
||||
import { tool, type ToolDefinition } from "@opencode-ai/plugin"
|
||||
import { readFile } from "node:fs/promises"
|
||||
import { resolve, sep } from "node:path"
|
||||
|
||||
function normalizeInputPath(pathValue: string): string {
|
||||
return pathValue.replace(/\\/g, "/")
|
||||
}
|
||||
|
||||
function isPathWithinDirectory(pathToCheck: string, directory: string): boolean {
|
||||
const normalizedDir = directory.endsWith(sep) ? directory : `${directory}${sep}`
|
||||
return pathToCheck === directory || pathToCheck.startsWith(normalizedDir)
|
||||
}
|
||||
|
||||
export function createCouncilRead(basePath?: string): ToolDefinition {
|
||||
return tool({
|
||||
description:
|
||||
"Read a council archive file and return its raw member response content.",
|
||||
args: {
|
||||
file_path: tool.schema.string().describe("Path to the archive file (must be within .sisyphus/)"),
|
||||
},
|
||||
async execute(args: { file_path: string }) {
|
||||
const normalizedInputPath = normalizeInputPath(args.file_path)
|
||||
if (!normalizedInputPath.startsWith(".sisyphus/")) {
|
||||
return JSON.stringify({ error: "Access denied: path must be within .sisyphus/" }, null, 2)
|
||||
}
|
||||
|
||||
try {
|
||||
const base = basePath ?? process.cwd()
|
||||
const absPath = resolve(base, normalizedInputPath)
|
||||
const absSisyphusRoot = resolve(base, ".sisyphus")
|
||||
if (!isPathWithinDirectory(absPath, absSisyphusRoot)) {
|
||||
return JSON.stringify({ error: "Access denied: path must be within .sisyphus/" }, null, 2)
|
||||
}
|
||||
|
||||
const content = await readFile(absPath, "utf-8")
|
||||
return JSON.stringify({ has_response: true, response_complete: true, result: content }, null, 2)
|
||||
} catch {
|
||||
return JSON.stringify({ has_response: false, error: `File not found: ${normalizedInputPath}` }, null, 2)
|
||||
}
|
||||
},
|
||||
})
|
||||
}
|
||||
@@ -1,5 +1,4 @@
|
||||
export { createCouncilFinalize } from "./create-council-finalize"
|
||||
export { createCouncilRead } from "./create-council-read"
|
||||
export { extractCouncilResponse, OPENING_TAG, CLOSING_TAG } from "./council-response-extractor"
|
||||
export type { CouncilResponseExtraction } from "./council-response-extractor"
|
||||
export type { CouncilFinalizeArgs, CouncilMemberResult, CouncilFinalizeResult } from "./types"
|
||||
|
||||
@@ -47,7 +47,7 @@ export {
|
||||
} from "./task"
|
||||
export { createHashlineEditTool } from "./hashline-edit"
|
||||
export { createPrepareCouncilPromptTool } from "./prepare-council-prompt"
|
||||
export { createCouncilFinalize, createCouncilRead } from "./council-archive"
|
||||
export { createCouncilFinalize } from "./council-archive"
|
||||
|
||||
export function createBackgroundTools(manager: BackgroundManager, client: OpencodeClient): Record<string, ToolDefinition> {
|
||||
const outputManager: BackgroundOutputManager = manager
|
||||
|
||||
Reference in New Issue
Block a user