Merge pull request #4071 from PeterPonyu/fix/4027-role-coordinator-subagent-selection
fix(team-mode): reject coordinator agents as subagent targets (#4027)
This commit is contained in:
@@ -346,3 +346,28 @@ export function isPlanFamily(category: string | undefined): boolean {
|
||||
const lowerCategory = getAgentConfigKey(category).toLowerCase().trim()
|
||||
return PLAN_FAMILY_NAMES.some((name) => lowerCategory === name)
|
||||
}
|
||||
|
||||
/**
|
||||
* Coordinator/meta agents that own the orchestration loop and must not be used as
|
||||
* arbitrary subagent targets via task(). Delegating to these creates duplicate
|
||||
* orchestration and conflicting team state (issue #4027).
|
||||
*
|
||||
* Scoped to AGENT_ELIGIBILITY_REGISTRY hard-reject entries only — sisyphus and atlas
|
||||
* are explicitly marked `verdict: "eligible"` for team membership in the registry
|
||||
* (src/features/team-mode/types.ts), so they are NOT included here. Adding them would
|
||||
* conflict with the team-mode resolver's intentional `allowPrimaryAgentDelegation: true`
|
||||
* opt-in.
|
||||
*
|
||||
* Symmetric guard to the caller-eligibility check added by PR #4065 for team_create.
|
||||
*/
|
||||
export const COORDINATOR_AGENT_NAMES = ["prometheus"]
|
||||
|
||||
/**
|
||||
* Returns true when the given agent name refers to a coordinator/meta agent that
|
||||
* should not be reachable as a subagent_type target via task().
|
||||
*/
|
||||
export function isCoordinatorAgent(agentName: string | undefined): boolean {
|
||||
if (!agentName) return false
|
||||
const normalized = getAgentConfigKey(agentName).toLowerCase().trim()
|
||||
return COORDINATOR_AGENT_NAMES.some((name) => normalized === name)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,144 @@
|
||||
/**
|
||||
* Regression test for issue #4027: coordinator agents must not be selectable as
|
||||
* subagent targets via task(). Symmetric guard to PR #4065 (team_create caller
|
||||
* eligibility) — this covers the TARGET side of delegation.
|
||||
*/
|
||||
const { describe, test, expect } = require("bun:test")
|
||||
|
||||
import { resolveSubagentExecution } from "./subagent-resolver"
|
||||
import { COORDINATOR_AGENT_NAMES } from "./constants"
|
||||
import type { ExecutorContext } from "./executor-types"
|
||||
|
||||
function makeCtx(): ExecutorContext {
|
||||
return {
|
||||
client: {
|
||||
app: { agents: async () => ({ data: [] }) },
|
||||
config: { get: async () => ({ data: {} }) },
|
||||
} as unknown as ExecutorContext["client"],
|
||||
manager: {} as unknown as ExecutorContext["manager"],
|
||||
directory: "/tmp/test",
|
||||
}
|
||||
}
|
||||
|
||||
describe("coordinator subagent guard (#4027)", () => {
|
||||
for (const coordinatorName of COORDINATOR_AGENT_NAMES) {
|
||||
test(`#given subagent_type="${coordinatorName}" #when resolveSubagentExecution is called #then it is rejected before spawning`, async () => {
|
||||
//#given
|
||||
const ctx = makeCtx()
|
||||
const args = {
|
||||
subagent_type: coordinatorName,
|
||||
prompt: "do something",
|
||||
load_skills: [],
|
||||
run_in_background: false,
|
||||
description: "test delegation",
|
||||
}
|
||||
|
||||
//#when
|
||||
const result = await resolveSubagentExecution(args, ctx, "sisyphus", "")
|
||||
|
||||
//#then
|
||||
expect(result.error).toBeDefined()
|
||||
expect(result.agentToUse).toBe("")
|
||||
expect(result.error).toContain(coordinatorName)
|
||||
expect(result.error).toContain("coordinator agent")
|
||||
})
|
||||
}
|
||||
|
||||
test("#given subagent_type=prometheus #when resolveSubagentExecution is called #then error names the agent and explains the conflict", async () => {
|
||||
//#given
|
||||
const ctx = makeCtx()
|
||||
const args = {
|
||||
subagent_type: "prometheus",
|
||||
prompt: "plan something",
|
||||
load_skills: [],
|
||||
run_in_background: false,
|
||||
description: "test delegation",
|
||||
}
|
||||
|
||||
//#when
|
||||
const result = await resolveSubagentExecution(args, ctx, "sisyphus", "")
|
||||
|
||||
//#then
|
||||
expect(result.error).toContain("prometheus")
|
||||
expect(result.error).toContain("coordinator")
|
||||
expect(result.error).toContain("duplicate")
|
||||
expect(result.agentToUse).toBe("")
|
||||
expect(result.categoryModel).toBeUndefined()
|
||||
})
|
||||
|
||||
test("#given subagent_type=hephaestus #when resolveSubagentExecution is called #then it is not blocked by coordinator guard", async () => {
|
||||
//#given
|
||||
const ctx = makeCtx()
|
||||
const args = {
|
||||
subagent_type: "hephaestus",
|
||||
prompt: "write some code",
|
||||
load_skills: [],
|
||||
run_in_background: false,
|
||||
description: "test delegation",
|
||||
}
|
||||
|
||||
//#when
|
||||
const result = await resolveSubagentExecution(args, ctx, "sisyphus", "")
|
||||
|
||||
//#then — hephaestus may fail for other reasons (API call), but NOT the coordinator guard
|
||||
expect(result.error).not.toContain("coordinator agent")
|
||||
})
|
||||
|
||||
test("#given subagent_type=sisyphus #when resolveSubagentExecution is called #then sisyphus is NOT blocked by coordinator guard (registry: eligible)", async () => {
|
||||
//#given — sisyphus is verdict:'eligible' in AGENT_ELIGIBILITY_REGISTRY; it must not be rejected by the coordinator guard
|
||||
const ctx = makeCtx()
|
||||
const args = {
|
||||
subagent_type: "sisyphus",
|
||||
prompt: "do team-mode work",
|
||||
load_skills: [],
|
||||
run_in_background: false,
|
||||
description: "test delegation",
|
||||
}
|
||||
|
||||
//#when
|
||||
const result = await resolveSubagentExecution(args, ctx, "sisyphus", "")
|
||||
|
||||
//#then — sisyphus may fail for primary-agent reasons (separate guard), but NOT the coordinator guard
|
||||
expect(result.error).not.toContain("coordinator agent")
|
||||
})
|
||||
|
||||
test("#given subagent_type=atlas #when resolveSubagentExecution is called #then atlas is NOT blocked by coordinator guard (registry: eligible)", async () => {
|
||||
//#given — atlas is verdict:'eligible' in AGENT_ELIGIBILITY_REGISTRY; it must not be rejected by the coordinator guard
|
||||
const ctx = makeCtx()
|
||||
const args = {
|
||||
subagent_type: "atlas",
|
||||
prompt: "do team-mode work",
|
||||
load_skills: [],
|
||||
run_in_background: false,
|
||||
description: "test delegation",
|
||||
}
|
||||
|
||||
//#when
|
||||
const result = await resolveSubagentExecution(args, ctx, "sisyphus", "")
|
||||
|
||||
//#then — atlas may fail for primary-agent reasons (separate guard), but NOT the coordinator guard
|
||||
expect(result.error).not.toContain("coordinator agent")
|
||||
})
|
||||
|
||||
test("#given subagent_type=prometheus AND allowPrimaryAgentDelegation=true #when resolveSubagentExecution is called #then prometheus is STILL rejected (registry hard-reject is authoritative)", async () => {
|
||||
//#given — prometheus is verdict:'hard-reject' in AGENT_ELIGIBILITY_REGISTRY; the coordinator guard must fire even when the team-mode resolver opts into primary-agent delegation
|
||||
const ctx = makeCtx()
|
||||
const args = {
|
||||
subagent_type: "prometheus",
|
||||
prompt: "plan something",
|
||||
load_skills: [],
|
||||
run_in_background: false,
|
||||
description: "test delegation",
|
||||
}
|
||||
|
||||
//#when
|
||||
const result = await resolveSubagentExecution(args, ctx, "sisyphus", "", { allowPrimaryAgentDelegation: true })
|
||||
|
||||
//#then
|
||||
expect(result.error).toContain("prometheus")
|
||||
expect(result.error).toContain("coordinator agent")
|
||||
expect(result.agentToUse).toBe("")
|
||||
})
|
||||
})
|
||||
|
||||
export {}
|
||||
@@ -1,7 +1,7 @@
|
||||
import type { DelegateTaskArgs } from "./types"
|
||||
import type { ExecutorContext } from "./executor-types"
|
||||
import type { DelegatedModelConfig } from "./types"
|
||||
import { isPlanAgent, isPlanFamily } from "./constants"
|
||||
import { isPlanAgent, isPlanFamily, isCoordinatorAgent, COORDINATOR_AGENT_NAMES } from "./constants"
|
||||
import { SISYPHUS_JUNIOR_AGENT } from "./sisyphus-junior-agent"
|
||||
import { applyCategoryParams } from "./delegated-model-config"
|
||||
import { getAvailableModelsForDelegateTask } from "./available-models"
|
||||
@@ -107,6 +107,14 @@ Create the work plan directly - that's your job as the planning agent.`,
|
||||
}
|
||||
}
|
||||
|
||||
if (isCoordinatorAgent(agentName)) {
|
||||
return {
|
||||
agentToUse: "",
|
||||
categoryModel: undefined,
|
||||
error: `Cannot delegate to coordinator agent "${agentName}" via task(). Coordinator agents (${COORDINATOR_AGENT_NAMES.join(", ")}) own the orchestration loop and must not be used as subagent targets — doing so creates duplicate coordinators and conflicting team state. Select a worker agent (e.g., sisyphus-junior via category, hephaestus, oracle) instead.`,
|
||||
}
|
||||
}
|
||||
|
||||
let agentToUse = agentName
|
||||
let categoryModel: DelegatedModelConfig | undefined
|
||||
let fallbackChain: FallbackEntry[] | undefined
|
||||
|
||||
@@ -4430,8 +4430,8 @@ describe("sisyphus-task", () => {
|
||||
{ sessionID: "p", messageID: "m", agent: "sisyphus", abort: new AbortController().signal }
|
||||
)
|
||||
|
||||
//#then
|
||||
expect(result).toContain('Cannot delegate to primary agent "prometheus" via task. Select that agent directly instead.')
|
||||
//#then — coordinator guard fires before primary-agent check; message names the agent and explains the conflict
|
||||
expect(result).toContain('Cannot delegate to coordinator agent "prometheus" via task()')
|
||||
}, { timeout: 20000 })
|
||||
|
||||
test("non-plan subagent should NOT have task permission", async () => {
|
||||
|
||||
@@ -165,7 +165,10 @@ describe("resolveSubagentExecution", () => {
|
||||
//#then
|
||||
expect(result.agentToUse).toBe("")
|
||||
expect(result.categoryModel).toBeUndefined()
|
||||
expect(result.error).toBe('Cannot delegate to primary agent "Prometheus - Plan Builder" via task. Select that agent directly instead.')
|
||||
// Prometheus is registry-hard-reject (AGENT_ELIGIBILITY_REGISTRY); the coordinator guard (#4027 / #4071) fires before
|
||||
// the primary-agent guard. Either rejection message is acceptable as long as prometheus is blocked from delegation.
|
||||
expect(result.error).toContain('"Prometheus - Plan Builder"')
|
||||
expect(result.error).toMatch(/Cannot delegate to (coordinator agent|primary agent)/)
|
||||
})
|
||||
|
||||
test("allows delegating to a primary agent when allowPrimaryAgentDelegation is enabled (team-mode path)", async () => {
|
||||
|
||||
Reference in New Issue
Block a user