diff --git a/src/tools/team-spawn.ts b/src/tools/team-spawn.ts index f2b71fa..90e3281 100644 --- a/src/tools/team-spawn.ts +++ b/src/tools/team-spawn.ts @@ -445,6 +445,11 @@ export async function executeTeamSpawn( }) const branchInfo = worktreeBranch ? ` (branch: ${worktreeBranch})` : "" + // Isolation is a claim the lead relies on, so say plainly when it did not + // happen. The toast alone left "spawned" as the only signal (issue #40). + const isolationInfo = useWorktree && !worktreeBranch + ? ` (WARNING: worktree creation failed — ${args.name} shares your working directory, so edits can collide)` + : "" const planInfo = usePlanApproval ? " [plan mode — will send plan for approval]" : "" const claimInfo = claimedTaskContent ? ` (claimed task: ${args.claim_task})` @@ -452,5 +457,5 @@ export async function executeTeamSpawn( // Reset circuit breaker on success spawnFailures.delete(teamInfo.teamId) log(`spawn:done name=${args.name} sessionId=${childSessionId}`) - return `Teammate "${args.name}" spawned (agent: ${agent})${branchInfo}${planInfo}${claimInfo}. They are working on: ${args.prompt.slice(0, 120)}${args.prompt.length > 120 ? "..." : ""}` + return `Teammate "${args.name}" spawned (agent: ${agent})${branchInfo}${isolationInfo}${planInfo}${claimInfo}. They are working on: ${args.prompt.slice(0, 120)}${args.prompt.length > 120 ? "..." : ""}` } diff --git a/src/tools/team-tasks-add.ts b/src/tools/team-tasks-add.ts index cd9334e..200ee4c 100644 --- a/src/tools/team-tasks-add.ts +++ b/src/tools/team-tasks-add.ts @@ -19,6 +19,24 @@ export async function executeTeamTasksAdd( ): Promise { const teamInfo = requireTeamMember(deps, sessionId) + // Validate every dependency before inserting anything. A typo'd id used to + // create a task that was blocked, unclaimable, and uncompletable — dead + // until the team was purged, because no tool edits depends_on (issue #40). + const unknown: string[] = [] + for (const task of args.tasks) { + for (const depId of task.depends_on ?? []) { + const dep = deps.db.query("SELECT id FROM team_task WHERE id = ? AND team_id = ?") + .get(depId, teamInfo.teamId) + if (!dep) unknown.push(depId) + } + } + if (unknown.length > 0) { + throw new Error( + `Unknown depends_on id${unknown.length !== 1 ? "s" : ""}: ${[...new Set(unknown)].join(", ")}. ` + + `Use team_tasks_list to get real task ids (they look like task_xxx). Nothing was added.` + ) + } + const ids: string[] = [] const now = Date.now() diff --git a/src/v2-client.ts b/src/v2-client.ts index e70b3c0..7e632bb 100644 --- a/src/v2-client.ts +++ b/src/v2-client.ts @@ -119,7 +119,17 @@ export function createV2Client(ctx: V2Context, deps: V2ClientDeps = {}): PluginC // and agent rules apply later), which strips team tools from // read-only agents. Re-assert here: rules() evaluates after the // agent's rules, restoring the tools. OQ-V2-rules. - await ctx.permission.rules({ sessionID: created.id, permissions }) + // + // opencode >= 2.0.4 removed ctx.permission.rules, so the call is + // feature-detected. Without the guard every team_spawn died with + // "ctx.permission.rules is not a function" (issue #40). The re-assert + // is skipped, not replaced: agent rules still win on 2.x, which + // costs read-only agents their team tools. + if (typeof ctx.permission?.rules === "function") { + await ctx.permission.rules({ sessionID: created.id, permissions }) + } else { + console.warn("[ensemble] ctx.permission.rules unavailable; skipping permission re-assert") + } } return { data: { id: created.id } } }, diff --git a/test/tools/team-spawn.test.ts b/test/tools/team-spawn.test.ts index 609a6ca..24f49ee 100644 --- a/test/tools/team-spawn.test.ts +++ b/test/tools/team-spawn.test.ts @@ -449,6 +449,37 @@ describe("team_spawn", () => { expect(result).toContain("branch:") }) + // issue #40: worktree.create failing was downgraded to a TUI toast, so the + // tool said only "spawned" while the teammate worked in the shared directory. + test("reports in the tool result when worktree creation failed (issue #40)", async () => { + deps.client.worktree.create = async () => { throw new Error("no worktree strategy configured") } + + const result = await executeTeamSpawn(deps, { + name: "alice", agent: "build", prompt: "Fix the tests", + }, "lead-sess") + + expect(result).toContain("worktree") + expect(result).toMatch(/failed|shared/i) + + const row = deps.db.query("SELECT worktree_dir, worktree_branch FROM team_member WHERE name = ?") + .get("alice") as Record + expect(row.worktree_dir).toBeNull() + expect(row.worktree_branch).toBeNull() + }) + + test("says the teammate is isolated only when a worktree really exists (issue #40)", async () => { + const ok = await executeTeamSpawn(deps, { + name: "alice", agent: "build", prompt: "Fix the tests", + }, "lead-sess") + expect(ok).toContain("branch:") + + deps.client.worktree.create = async () => { throw new Error("nope") } + const degraded = await executeTeamSpawn(deps, { + name: "bob", agent: "build", prompt: "Fix the tests", + }, "lead-sess") + expect(degraded).not.toContain("branch:") + }) + test("uses team id for worktree names so same-name teams in different projects do not collide", async () => { deps.db.run( "INSERT OR IGNORE INTO project (id, name, path, status, time_created, time_updated) VALUES (?, ?, ?, 'active', ?, ?)", diff --git a/test/tools/team-tasks.test.ts b/test/tools/team-tasks.test.ts index 5f052e6..7764ef6 100644 --- a/test/tools/team-tasks.test.ts +++ b/test/tools/team-tasks.test.ts @@ -82,6 +82,46 @@ describe("team_tasks_add", () => { .rejects.toThrow("not in a team") }) + // issue #40: a typo in depends_on used to create a task that was blocked, + // unclaimable, and uncompletable — dead until the team was purged. + test("rejects depends_on ids that match no task in the team", async () => { + await expect(executeTeamTasksAdd(deps, { tasks: [ + { content: "Task B", priority: "medium", depends_on: ["t1"] }, + ] }, "sess-alice")).rejects.toThrow("t1") + const rows = deps.db.query("SELECT * FROM team_task WHERE team_id = ?").all("t1") + expect(rows).toHaveLength(0) + }) + + test("rejects the whole call when any task has an unknown dependency, inserting nothing", async () => { + const result1 = await executeTeamTasksAdd(deps, { tasks: [ + { content: "Task A", priority: "high" }, + ] }, "sess-alice") + const taskAId = result1.match(/task_\S+/)![0] + + await expect(executeTeamTasksAdd(deps, { tasks: [ + { content: "Good", priority: "low", depends_on: [taskAId] }, + { content: "Bad", priority: "low", depends_on: ["task_nope"] }, + ] }, "sess-alice")).rejects.toThrow("task_nope") + + const rows = deps.db.query("SELECT content FROM team_task WHERE team_id = ?").all("t1") as Array<{ content: string }> + expect(rows.map(r => r.content)).toEqual(["Task A"]) + }) + + test("rejects a dependency that exists only in another team", async () => { + insertTeam(deps.db, "t2", "other-team", "lead2-sess") + insertMember(deps.db, "t2", "bob", "sess-bob") + deps.db.run( + "INSERT INTO team_task (id, team_id, content, status, priority, time_created, time_updated) VALUES (?, ?, ?, 'pending', 'low', ?, ?)", + ["task_foreign", "t2", "Foreign task", Date.now(), Date.now()], + ) + + await expect(executeTeamTasksAdd(deps, { tasks: [ + { content: "Borrowed", priority: "low", depends_on: ["task_foreign"] }, + ] }, "sess-alice")).rejects.toThrow("task_foreign") + const rows = deps.db.query("SELECT * FROM team_task WHERE team_id = ?").all("t1") + expect(rows).toHaveLength(0) + }) + test("lead can add tasks", async () => { const result = await executeTeamTasksAdd(deps, { tasks: [ { content: "Lead task", priority: "high" }, diff --git a/test/v2-client.test.ts b/test/v2-client.test.ts index eac4aec..3e302a5 100644 --- a/test/v2-client.test.ts +++ b/test/v2-client.test.ts @@ -94,6 +94,29 @@ describe("v2-client adapter (issue #36, DRY: reuses all tool logic)", () => { }) }) + test("session.create survives opencode 2.x, where ctx.permission.rules is gone (issue #40)", async () => { + // opencode >= 2.0.4 removed the permission.rules plugin API. Calling it + // unconditionally made every team_spawn throw + // "ctx.permission.rules is not a function". + const { calls, ctx } = mockCtx() + delete (ctx as { permission?: Record }).permission?.rules + expect((ctx as { permission?: { rules?: unknown } }).permission?.rules).toBeUndefined() + + const client = createV2Client(ctx as never) + const result = await client.session.create({ + parentID: "ses_lead", + title: "alice (@build teammate)", + permission: [{ permission: "team_message", pattern: "*", action: "allow" }], + }) + + expect(result.data?.id).toBe("ses_new123") + // Permissions still travel on session.create; only the re-assert is skipped. + expect(calls[0]?.input).toMatchObject({ + permissions: [{ action: "team_message", resource: "*", effect: "allow" }], + }) + expect(calls.some((c) => c.method === "rules")).toBe(false) + }) + test("session.create maps parentID/title/permissions to V2 shape", async () => { const { calls: calls2, ctx: ctx2 } = mockCtx() const client2 = createV2Client(ctx2 as never)