From accb8741556cb2dacf9c82a27c20aa4672582310 Mon Sep 17 00:00:00 2001 From: Nguyen Khac Trung Kien Date: Sun, 8 Feb 2026 14:55:26 +0700 Subject: [PATCH] fix(agent-teams): close delete race and preserve parent-agent fallback --- .../agent-teams/team-config-store.test.ts | 63 ++++++++++++++++--- src/tools/agent-teams/team-config-store.ts | 16 +++-- .../teammate-parent-context.test.ts | 12 ++++ .../agent-teams/teammate-parent-context.ts | 2 +- src/tools/delegate-task/types.ts | 2 +- 5 files changed, 79 insertions(+), 16 deletions(-) diff --git a/src/tools/agent-teams/team-config-store.test.ts b/src/tools/agent-teams/team-config-store.test.ts index 7b6a50e06..b908a9e0e 100644 --- a/src/tools/agent-teams/team-config-store.test.ts +++ b/src/tools/agent-teams/team-config-store.test.ts @@ -5,7 +5,14 @@ import { tmpdir } from "node:os" import { join } from "node:path" import { acquireLock } from "../../features/claude-tasks/storage" import { getTeamDir, getTeamTaskDir, getTeamsRootDir } from "./paths" -import { createTeamConfig, deleteTeamData, teamExists } from "./team-config-store" +import { + createTeamConfig, + deleteTeamData, + readTeamConfigOrThrow, + teamExists, + upsertTeammate, + writeTeamConfig, +} from "./team-config-store" describe("agent-teams team config store", () => { let originalCwd: string @@ -31,15 +38,18 @@ describe("agent-teams team config store", () => { const lock = acquireLock(getTeamDir("core")) expect(lock.acquired).toBe(true) - //#when - const deleteWhileLocked = () => deleteTeamData("core") + try { + //#when + const deleteWhileLocked = () => deleteTeamData("core") - //#then - expect(deleteWhileLocked).toThrow("team_lock_unavailable") - expect(teamExists("core")).toBe(true) + //#then + expect(deleteWhileLocked).toThrow("team_lock_unavailable") + expect(teamExists("core")).toBe(true) + } finally { + //#when + lock.release() + } - //#when - lock.release() deleteTeamData("core") //#then @@ -91,4 +101,41 @@ describe("agent-teams team config store", () => { expect(existsSync(teamDir)).toBe(true) }) + test("deleteTeamData fails if team has active teammates", () => { + //#given + const config = readTeamConfigOrThrow("core") + const updated = upsertTeammate(config, { + agentId: "teammate@core", + name: "teammate", + agentType: "sisyphus", + category: "test", + model: "sisyphus", + prompt: "test prompt", + color: "#000000", + planModeRequired: false, + joinedAt: Date.now(), + cwd: process.cwd(), + subscriptions: [], + backendType: "native", + isActive: true, + sessionID: "ses-sub", + }) + writeTeamConfig("core", updated) + + //#when + const deleteWithTeammates = () => deleteTeamData("core") + + //#then + expect(deleteWithTeammates).toThrow("team_has_active_members") + expect(teamExists("core")).toBe(true) + + //#when - cleanup teammate to allow afterEach to succeed + const cleared = { ...updated, members: updated.members.filter(m => m.name === "team-lead") } + writeTeamConfig("core", cleared) + deleteTeamData("core") + + //#then + expect(teamExists("core")).toBe(false) + }) + }) diff --git a/src/tools/agent-teams/team-config-store.ts b/src/tools/agent-teams/team-config-store.ts index 7c733e539..6667a807d 100644 --- a/src/tools/agent-teams/team-config-store.ts +++ b/src/tools/agent-teams/team-config-store.ts @@ -1,10 +1,5 @@ import { existsSync, rmSync } from "node:fs" -import { - acquireLock, - ensureDir, - readJsonSafe, - writeJsonAtomic, -} from "../../features/claude-tasks/storage" +import { acquireLock, ensureDir, readJsonSafe, writeJsonAtomic } from "../../features/claude-tasks/storage" import { getTeamConfigPath, getTeamDir, @@ -175,6 +170,15 @@ export function assignNextColor(config: TeamConfig): string { export function deleteTeamData(teamName: string): void { assertValidTeamName(teamName) withTeamLock(teamName, () => { + const config = readJsonSafe(getTeamConfigPath(teamName), TeamConfigSchema) + if (!config) { + throw new Error("team_not_found") + } + + if (listTeammates(config).length > 0) { + throw new Error("team_has_active_members") + } + withTeamTaskLock(teamName, () => { const teamDir = getTeamDir(teamName) const taskDir = getTeamTaskDir(teamName) diff --git a/src/tools/agent-teams/teammate-parent-context.test.ts b/src/tools/agent-teams/teammate-parent-context.test.ts index 6654d4918..ec4ecc307 100644 --- a/src/tools/agent-teams/teammate-parent-context.test.ts +++ b/src/tools/agent-teams/teammate-parent-context.test.ts @@ -21,4 +21,16 @@ describe("agent-teams teammate parent context", () => { expect(parentToolContext.messageID).toBe("msg-main") expect(parentToolContext.agent).toBe("sisyphus") }) + + test("leaves agent undefined if missing in tool context", () => { + //#when + const parentToolContext = buildTeamParentToolContext({ + sessionID: "ses-main", + messageID: "msg-main", + abort: new AbortController().signal, + }) + + //#then + expect(parentToolContext.agent).toBeUndefined() + }) }) diff --git a/src/tools/agent-teams/teammate-parent-context.ts b/src/tools/agent-teams/teammate-parent-context.ts index 129e9ba12..a9966dd11 100644 --- a/src/tools/agent-teams/teammate-parent-context.ts +++ b/src/tools/agent-teams/teammate-parent-context.ts @@ -7,7 +7,7 @@ export function buildTeamParentToolContext(context: TeamToolContext): ToolContex return { sessionID: context.sessionID, messageID: context.messageID, - agent: context.agent ?? "sisyphus", + agent: context.agent, abort: context.abort ?? new AbortController().signal, } } diff --git a/src/tools/delegate-task/types.ts b/src/tools/delegate-task/types.ts index 13d1973a4..10aa72f90 100644 --- a/src/tools/delegate-task/types.ts +++ b/src/tools/delegate-task/types.ts @@ -26,7 +26,7 @@ export interface DelegateTaskArgs { export interface ToolContextWithMetadata { sessionID: string messageID: string - agent: string + agent?: string abort: AbortSignal metadata?: (input: { title?: string; metadata?: Record }) => void | Promise /**