fix(agent-teams): close latest review gaps for auth and race safety
This commit is contained in:
parent
79c3823762
commit
2103061123
@ -212,7 +212,7 @@ export function createReadInboxTool(): ToolDefinition {
|
|||||||
unread_only: tool.schema.boolean().optional().describe("Return only unread messages"),
|
unread_only: tool.schema.boolean().optional().describe("Return only unread messages"),
|
||||||
mark_as_read: tool.schema.boolean().optional().describe("Mark returned messages as read"),
|
mark_as_read: tool.schema.boolean().optional().describe("Mark returned messages as read"),
|
||||||
},
|
},
|
||||||
execute: async (args: Record<string, unknown>): Promise<string> => {
|
execute: async (args: Record<string, unknown>, context: TeamToolContext): Promise<string> => {
|
||||||
try {
|
try {
|
||||||
const input = TeamReadInboxInputSchema.parse(args)
|
const input = TeamReadInboxInputSchema.parse(args)
|
||||||
const teamError = validateTeamName(input.team_name)
|
const teamError = validateTeamName(input.team_name)
|
||||||
@ -223,7 +223,15 @@ export function createReadInboxTool(): ToolDefinition {
|
|||||||
if (agentError) {
|
if (agentError) {
|
||||||
return JSON.stringify({ error: agentError })
|
return JSON.stringify({ error: agentError })
|
||||||
}
|
}
|
||||||
readTeamConfigOrThrow(input.team_name)
|
const config = readTeamConfigOrThrow(input.team_name)
|
||||||
|
const actor = resolveSenderFromContext(config, context)
|
||||||
|
if (!actor) {
|
||||||
|
return JSON.stringify({ error: "unauthorized_reader_session" })
|
||||||
|
}
|
||||||
|
|
||||||
|
if (actor !== "team-lead" && actor !== input.agent_name) {
|
||||||
|
return JSON.stringify({ error: "unauthorized_reader_session" })
|
||||||
|
}
|
||||||
|
|
||||||
const messages = readInbox(
|
const messages = readInbox(
|
||||||
input.team_name,
|
input.team_name,
|
||||||
|
|||||||
@ -118,8 +118,15 @@ export function updateTeamTask(teamName: string, taskId: string, patch: TeamTask
|
|||||||
|
|
||||||
if (patch.status && patch.status !== "deleted") {
|
if (patch.status && patch.status !== "deleted") {
|
||||||
ensureForwardStatusTransition(currentTask.status, patch.status)
|
ensureForwardStatusTransition(currentTask.status, patch.status)
|
||||||
const effectiveBlockedBy = Array.from(new Set([...(currentTask.blockedBy ?? []), ...(patch.addBlockedBy ?? [])]))
|
}
|
||||||
ensureDependenciesCompleted(patch.status, effectiveBlockedBy, readTask)
|
|
||||||
|
const effectiveStatus = patch.status ?? currentTask.status
|
||||||
|
const effectiveBlockedBy = Array.from(new Set([...(currentTask.blockedBy ?? []), ...(patch.addBlockedBy ?? [])]))
|
||||||
|
const shouldValidateDependencies =
|
||||||
|
(patch.status !== undefined || (patch.addBlockedBy?.length ?? 0) > 0) && effectiveStatus !== "deleted"
|
||||||
|
|
||||||
|
if (shouldValidateDependencies) {
|
||||||
|
ensureDependenciesCompleted(effectiveStatus, effectiveBlockedBy, readTask)
|
||||||
}
|
}
|
||||||
|
|
||||||
let nextTask: TeamTask = { ...currentTask }
|
let nextTask: TeamTask = { ...currentTask }
|
||||||
|
|||||||
@ -90,10 +90,10 @@ export async function spawnTeammate(params: SpawnTeammateParams): Promise<TeamTe
|
|||||||
throw new Error("teammate_create_failed")
|
throw new Error("teammate_create_failed")
|
||||||
}
|
}
|
||||||
|
|
||||||
ensureInbox(params.teamName, params.name)
|
|
||||||
sendPlainInboxMessage(params.teamName, "team-lead", params.name, params.prompt, "initial_prompt", teammate.color)
|
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
ensureInbox(params.teamName, params.name)
|
||||||
|
sendPlainInboxMessage(params.teamName, "team-lead", params.name, params.prompt, "initial_prompt", teammate.color)
|
||||||
|
|
||||||
const resolvedModel = parseModel(params.model)
|
const resolvedModel = parseModel(params.model)
|
||||||
const launched = await params.manager.launch({
|
const launched = await params.manager.launch({
|
||||||
description: `[team:${params.teamName}] ${params.name}`,
|
description: `[team:${params.teamName}] ${params.name}`,
|
||||||
|
|||||||
@ -8,7 +8,7 @@ import {
|
|||||||
TeamToolContext,
|
TeamToolContext,
|
||||||
isTeammateMember,
|
isTeammateMember,
|
||||||
} from "./types"
|
} from "./types"
|
||||||
import { getTeamMember, readTeamConfigOrThrow, removeTeammate, writeTeamConfig } from "./team-config-store"
|
import { getTeamMember, readTeamConfigOrThrow, removeTeammate, updateTeamConfig, writeTeamConfig } from "./team-config-store"
|
||||||
import { cancelTeammateRun, spawnTeammate } from "./teammate-runtime"
|
import { cancelTeammateRun, spawnTeammate } from "./teammate-runtime"
|
||||||
import { resetOwnerTasks } from "./team-task-store"
|
import { resetOwnerTasks } from "./team-task-store"
|
||||||
|
|
||||||
@ -130,7 +130,16 @@ export function createProcessShutdownTool(manager: BackgroundManager): ToolDefin
|
|||||||
}
|
}
|
||||||
|
|
||||||
await cancelTeammateRun(manager, member)
|
await cancelTeammateRun(manager, member)
|
||||||
writeTeamConfig(input.team_name, removeTeammate(config, input.agent_name))
|
|
||||||
|
updateTeamConfig(input.team_name, (current) => {
|
||||||
|
const refreshedMember = getTeamMember(current, input.agent_name)
|
||||||
|
if (!refreshedMember || !isTeammateMember(refreshedMember)) {
|
||||||
|
return current
|
||||||
|
}
|
||||||
|
|
||||||
|
return removeTeammate(current, input.agent_name)
|
||||||
|
})
|
||||||
|
|
||||||
resetOwnerTasks(input.team_name, input.agent_name)
|
resetOwnerTasks(input.team_name, input.agent_name)
|
||||||
|
|
||||||
return JSON.stringify({ success: true, message: `${input.agent_name} removed` })
|
return JSON.stringify({ success: true, message: `${input.agent_name} removed` })
|
||||||
|
|||||||
@ -657,6 +657,112 @@ describe("agent-teams tools functional", () => {
|
|||||||
expect(result.error).toBe("team_not_found")
|
expect(result.error).toBe("team_not_found")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("read_inbox denies cross-member access for non-lead sessions", async () => {
|
||||||
|
//#given
|
||||||
|
const { manager } = createMockManager()
|
||||||
|
const tools = createAgentTeamsTools(manager)
|
||||||
|
const leadContext = createContext()
|
||||||
|
|
||||||
|
await executeJsonTool(tools, "team_create", { team_name: "core" }, leadContext)
|
||||||
|
await executeJsonTool(
|
||||||
|
tools,
|
||||||
|
"spawn_teammate",
|
||||||
|
{
|
||||||
|
team_name: "core",
|
||||||
|
name: "worker_1",
|
||||||
|
prompt: "Handle release prep",
|
||||||
|
},
|
||||||
|
leadContext,
|
||||||
|
)
|
||||||
|
|
||||||
|
const teammateContext = createContext("ses-worker-1")
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const unauthorized = await executeJsonTool(
|
||||||
|
tools,
|
||||||
|
"read_inbox",
|
||||||
|
{
|
||||||
|
team_name: "core",
|
||||||
|
agent_name: "team-lead",
|
||||||
|
},
|
||||||
|
teammateContext,
|
||||||
|
) as { error?: string }
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(unauthorized.error).toBe("unauthorized_reader_session")
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const ownInbox = await executeJsonTool(
|
||||||
|
tools,
|
||||||
|
"read_inbox",
|
||||||
|
{
|
||||||
|
team_name: "core",
|
||||||
|
agent_name: "worker_1",
|
||||||
|
},
|
||||||
|
teammateContext,
|
||||||
|
) as unknown[]
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(Array.isArray(ownInbox)).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("cannot add pending blockers to already in-progress task without status change", async () => {
|
||||||
|
//#given
|
||||||
|
const { manager } = createMockManager()
|
||||||
|
const tools = createAgentTeamsTools(manager)
|
||||||
|
const context = createContext()
|
||||||
|
|
||||||
|
await executeJsonTool(tools, "team_create", { team_name: "core" }, context)
|
||||||
|
|
||||||
|
const blocker = await executeJsonTool(
|
||||||
|
tools,
|
||||||
|
"team_task_create",
|
||||||
|
{
|
||||||
|
team_name: "core",
|
||||||
|
subject: "Blocker",
|
||||||
|
description: "Unfinished blocker",
|
||||||
|
},
|
||||||
|
context,
|
||||||
|
) as { id: string }
|
||||||
|
|
||||||
|
const mainTask = await executeJsonTool(
|
||||||
|
tools,
|
||||||
|
"team_task_create",
|
||||||
|
{
|
||||||
|
team_name: "core",
|
||||||
|
subject: "Main",
|
||||||
|
description: "Main task",
|
||||||
|
},
|
||||||
|
context,
|
||||||
|
) as { id: string }
|
||||||
|
|
||||||
|
await executeJsonTool(
|
||||||
|
tools,
|
||||||
|
"team_task_update",
|
||||||
|
{
|
||||||
|
team_name: "core",
|
||||||
|
task_id: mainTask.id,
|
||||||
|
status: "in_progress",
|
||||||
|
},
|
||||||
|
context,
|
||||||
|
)
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = await executeJsonTool(
|
||||||
|
tools,
|
||||||
|
"team_task_update",
|
||||||
|
{
|
||||||
|
team_name: "core",
|
||||||
|
task_id: mainTask.id,
|
||||||
|
add_blocked_by: [blocker.id],
|
||||||
|
},
|
||||||
|
context,
|
||||||
|
) as { error?: string }
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(result.error).toBe(`blocked_by_incomplete:${blocker.id}:pending`)
|
||||||
|
})
|
||||||
|
|
||||||
test("binds sender to calling context and rejects sender spoofing", async () => {
|
test("binds sender to calling context and rejects sender spoofing", async () => {
|
||||||
//#given
|
//#given
|
||||||
const { manager } = createMockManager()
|
const { manager } = createMockManager()
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user