fix(athena): council review fixes — delegation bug, dead code, test coverage
- Add background_output to council-member allowlist (fixes delegation deadlock) - Replace empty catch with error logging in prepare-council-prompt - Remove unnecessary type assertion in agent.ts - Remove dead hasAgentToolRestrictions function - Fix incorrect test assertions (undefined vs false semantics) - Add barrel export for athena module - Add guard function test coverage (5 tests) - Add parity test for triple-sync restrictions (9 tests)
This commit is contained in:
ismeth
@@ -0,0 +1,140 @@
|
||||
/**
|
||||
* Parity test: verifies Athena and council-member tool restrictions stay in sync
|
||||
* across the 3 definition surfaces.
|
||||
*
|
||||
* Surface 1: src/agents/athena/agent.ts — createAgentToolRestrictions() deny-list
|
||||
* Surface 2: src/shared/agent-tool-restrictions.ts — AGENT_RESTRICTIONS boolean map
|
||||
* Surface 3: src/agents/athena/council-member-agent.ts — createAgentToolAllowlist() array
|
||||
*
|
||||
* This test FAILS if someone adds/removes a restriction in one surface without updating the others.
|
||||
*/
|
||||
|
||||
import { describe, expect, it } from "bun:test"
|
||||
import { getAgentToolRestrictions } from "./agent-tool-restrictions"
|
||||
|
||||
// Surface 1: Athena deny-list from src/agents/athena/agent.ts
|
||||
// createAgentToolRestrictions(["write", "edit", "call_omo_agent"])
|
||||
const ATHENA_DENY_LIST = ["write", "edit", "call_omo_agent"]
|
||||
|
||||
// Surface 3: Council-member allowlist from src/agents/athena/council-member-agent.ts
|
||||
// createAgentToolAllowlist([...])
|
||||
const COUNCIL_MEMBER_ALLOWLIST = [
|
||||
"read",
|
||||
"grep",
|
||||
"glob",
|
||||
"lsp_goto_definition",
|
||||
"lsp_find_references",
|
||||
"lsp_symbols",
|
||||
"lsp_diagnostics",
|
||||
"ast_grep_search",
|
||||
"call_omo_agent",
|
||||
"background_output",
|
||||
]
|
||||
|
||||
// Tools granted to Athena by tool-config-handler.ts (not in deny-list, not in AGENT_RESTRICTIONS)
|
||||
const ATHENA_HANDLER_GRANTS = ["task", "prepare_council_prompt"]
|
||||
|
||||
describe("agent tool restrictions parity", () => {
|
||||
describe("given Athena restrictions", () => {
|
||||
describe("#when comparing deny-list (agent.ts) with boolean map (agent-tool-restrictions.ts)", () => {
|
||||
it("every tool in the deny-list has a matching false entry in AGENT_RESTRICTIONS", () => {
|
||||
const athenaRestrictions = getAgentToolRestrictions("athena")
|
||||
|
||||
for (const tool of ATHENA_DENY_LIST) {
|
||||
expect(
|
||||
athenaRestrictions[tool],
|
||||
`Tool "${tool}" is in the deny-list (agent.ts) but not false in AGENT_RESTRICTIONS["athena"]`
|
||||
).toBe(false)
|
||||
}
|
||||
})
|
||||
|
||||
it("every false entry in AGENT_RESTRICTIONS is in the deny-list", () => {
|
||||
const athenaRestrictions = getAgentToolRestrictions("athena")
|
||||
const deniedInMap = Object.entries(athenaRestrictions)
|
||||
.filter(([, value]) => value === false)
|
||||
.map(([key]) => key)
|
||||
|
||||
for (const tool of deniedInMap) {
|
||||
expect(
|
||||
ATHENA_DENY_LIST,
|
||||
`Tool "${tool}" is false in AGENT_RESTRICTIONS["athena"] but missing from deny-list (agent.ts)`
|
||||
).toContain(tool)
|
||||
}
|
||||
})
|
||||
|
||||
it("deny-list and AGENT_RESTRICTIONS false-entries have the same length", () => {
|
||||
const athenaRestrictions = getAgentToolRestrictions("athena")
|
||||
const deniedInMap = Object.entries(athenaRestrictions)
|
||||
.filter(([, value]) => value === false)
|
||||
.map(([key]) => key)
|
||||
|
||||
expect(deniedInMap.length).toBe(ATHENA_DENY_LIST.length)
|
||||
})
|
||||
})
|
||||
|
||||
describe("#when checking handler grants do not conflict with deny-list", () => {
|
||||
it("tools granted by tool-config-handler are NOT in the deny-list", () => {
|
||||
for (const tool of ATHENA_HANDLER_GRANTS) {
|
||||
expect(
|
||||
ATHENA_DENY_LIST,
|
||||
`Tool "${tool}" is granted by tool-config-handler but also in the deny-list — conflict!`
|
||||
).not.toContain(tool)
|
||||
}
|
||||
})
|
||||
|
||||
it("tools granted by tool-config-handler are NOT false in AGENT_RESTRICTIONS", () => {
|
||||
const athenaRestrictions = getAgentToolRestrictions("athena")
|
||||
|
||||
for (const tool of ATHENA_HANDLER_GRANTS) {
|
||||
expect(
|
||||
athenaRestrictions[tool],
|
||||
`Tool "${tool}" is granted by tool-config-handler but is false in AGENT_RESTRICTIONS["athena"]`
|
||||
).not.toBe(false)
|
||||
}
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("given council-member restrictions", () => {
|
||||
describe("#when comparing allowlist (council-member-agent.ts) with boolean map (agent-tool-restrictions.ts)", () => {
|
||||
it("every tool in the allowlist has a matching true entry in AGENT_RESTRICTIONS", () => {
|
||||
const councilRestrictions = getAgentToolRestrictions("council-member")
|
||||
|
||||
for (const tool of COUNCIL_MEMBER_ALLOWLIST) {
|
||||
expect(
|
||||
councilRestrictions[tool],
|
||||
`Tool "${tool}" is in the allowlist (council-member-agent.ts) but not true in AGENT_RESTRICTIONS["council-member"]`
|
||||
).toBe(true)
|
||||
}
|
||||
})
|
||||
|
||||
it("every true entry in AGENT_RESTRICTIONS is in the allowlist", () => {
|
||||
const councilRestrictions = getAgentToolRestrictions("council-member")
|
||||
const allowedInMap = Object.entries(councilRestrictions)
|
||||
.filter(([key, value]) => key !== "*" && value === true)
|
||||
.map(([key]) => key)
|
||||
|
||||
for (const tool of allowedInMap) {
|
||||
expect(
|
||||
COUNCIL_MEMBER_ALLOWLIST,
|
||||
`Tool "${tool}" is true in AGENT_RESTRICTIONS["council-member"] but missing from allowlist (council-member-agent.ts)`
|
||||
).toContain(tool)
|
||||
}
|
||||
})
|
||||
|
||||
it("allowlist and AGENT_RESTRICTIONS true-entries have the same length", () => {
|
||||
const councilRestrictions = getAgentToolRestrictions("council-member")
|
||||
const allowedInMap = Object.entries(councilRestrictions)
|
||||
.filter(([key, value]) => key !== "*" && value === true)
|
||||
.map(([key]) => key)
|
||||
|
||||
expect(allowedInMap.length).toBe(COUNCIL_MEMBER_ALLOWLIST.length)
|
||||
})
|
||||
|
||||
it("AGENT_RESTRICTIONS has wildcard deny (*: false) for council-member", () => {
|
||||
const councilRestrictions = getAgentToolRestrictions("council-member")
|
||||
expect(councilRestrictions["*"]).toBe(false)
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -1,8 +1,5 @@
|
||||
import { describe, expect, test } from "bun:test"
|
||||
import {
|
||||
getAgentToolRestrictions,
|
||||
hasAgentToolRestrictions,
|
||||
} from "./agent-tool-restrictions"
|
||||
import { getAgentToolRestrictions } from "./agent-tool-restrictions"
|
||||
|
||||
describe("agent-tool-restrictions", () => {
|
||||
test("athena restrictions include call_omo_agent", () => {
|
||||
@@ -20,9 +17,19 @@ describe("agent-tool-restrictions", () => {
|
||||
//#when
|
||||
const restrictions = getAgentToolRestrictions("council-member")
|
||||
//#then
|
||||
expect(restrictions.call_omo_agent).toBe(false)
|
||||
expect(restrictions.switch_agent).toBe(false)
|
||||
expect(restrictions.background_wait).toBe(false)
|
||||
// Wildcard deny key
|
||||
expect(restrictions["*"]).toBe(false)
|
||||
// Explicitly allowed tools
|
||||
expect(restrictions.read).toBe(true)
|
||||
expect(restrictions.grep).toBe(true)
|
||||
expect(restrictions.call_omo_agent).toBe(true)
|
||||
expect(restrictions.background_output).toBe(true)
|
||||
// Explicitly denied tools
|
||||
expect(restrictions.todowrite).toBe(false)
|
||||
expect(restrictions.todoread).toBe(false)
|
||||
// Unlisted tools are undefined (SDK applies wildcard at runtime)
|
||||
expect(restrictions.switch_agent).toBeUndefined()
|
||||
expect(restrictions.background_wait).toBeUndefined()
|
||||
})
|
||||
|
||||
test("#given dynamic council member name #when getAgentToolRestrictions #then returns council-member restrictions", () => {
|
||||
@@ -31,19 +38,21 @@ describe("agent-tool-restrictions", () => {
|
||||
//#when
|
||||
const restrictions = getAgentToolRestrictions(dynamicName)
|
||||
//#then
|
||||
expect(restrictions.write).toBe(false)
|
||||
expect(restrictions.edit).toBe(false)
|
||||
expect(restrictions.task).toBe(false)
|
||||
expect(restrictions.call_omo_agent).toBe(false)
|
||||
expect(restrictions.switch_agent).toBe(false)
|
||||
expect(restrictions.background_wait).toBe(false)
|
||||
})
|
||||
|
||||
test("hasAgentToolRestrictions returns true for athena", () => {
|
||||
//#given
|
||||
//#when
|
||||
const result = hasAgentToolRestrictions("athena")
|
||||
//#then
|
||||
expect(result).toBe(true)
|
||||
// Wildcard deny key
|
||||
expect(restrictions["*"]).toBe(false)
|
||||
// Explicitly allowed tools
|
||||
expect(restrictions.read).toBe(true)
|
||||
expect(restrictions.grep).toBe(true)
|
||||
expect(restrictions.call_omo_agent).toBe(true)
|
||||
expect(restrictions.background_output).toBe(true)
|
||||
// Explicitly denied tools
|
||||
expect(restrictions.todowrite).toBe(false)
|
||||
expect(restrictions.todoread).toBe(false)
|
||||
// Unlisted tools are undefined (SDK applies wildcard at runtime)
|
||||
expect(restrictions.switch_agent).toBeUndefined()
|
||||
expect(restrictions.write).toBeUndefined()
|
||||
expect(restrictions.edit).toBeUndefined()
|
||||
expect(restrictions.task).toBeUndefined()
|
||||
expect(restrictions.background_wait).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -72,6 +72,7 @@ const AGENT_RESTRICTIONS: Record<string, Record<string, boolean>> = {
|
||||
lsp_diagnostics: true,
|
||||
ast_grep_search: true,
|
||||
call_omo_agent: true,
|
||||
background_output: true,
|
||||
todowrite: false,
|
||||
todoread: false,
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user