From d5d68ad3585a8efa43f3feafedce96ae12319a28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 8 Oct 2026 08:19:41 -0400 Subject: [PATCH] Q47: task parents cannot form a cycle Co-Authored-By: Claude Opus 5.5 --- src/main/services/organization-service.ts | 19 +++++++--- .../src/pages/work/TaskDetailDialog.tsx | 18 ++++++---- src/renderer/src/pages/work/task-helpers.ts | 16 +++++++++ ...n-blocker.test.ts => task-helpers.test.ts} | 10 ++++-- tests/unit/task-hierarchy.test.ts | 36 +++++++++++++++++++ 5 files changed, 86 insertions(+), 13 deletions(-) rename tests/unit/{run-blocker.test.ts => task-helpers.test.ts} (66%) create mode 100644 tests/unit/task-hierarchy.test.ts diff --git a/src/main/services/organization-service.ts b/src/main/services/organization-service.ts index ea181a5a..01bf0e7f 100644 --- a/src/main/services/organization-service.ts +++ b/src/main/services/organization-service.ts @@ -492,10 +492,7 @@ export class OrganizationService { updateTask(input: UpdateTaskInput): void { const now = new Date().toISOString(); if (input.assigneeAgentId) this.verifyOptionalOrgEntity("agents", input.assigneeAgentId, input.orgId, "Assignee agent"); - if (input.parentTaskId) { - const parent = this.db.prepare("select id from tasks where id = ? and org_id = ?").get(input.parentTaskId, input.orgId); - if (!parent) throw new Error("Parent task was not found."); - } + if (input.parentTaskId) this.assertParentAllowed(input.orgId, input.taskId, input.parentTaskId, input.title.trim()); const result = this.db.prepare(` update tasks set title = ?, description = ?, assignee_agent_id = ?, risk = ?, priority = ?, parent_task_id = ?, updated_at = ? where id = ? and org_id = ? @@ -545,6 +542,20 @@ export class OrganizationService { }); } + private assertParentAllowed(orgId: string, taskId: string, parentTaskId: string, title: string): void { + if (parentTaskId === taskId) throw new Error(`"${title}" cannot be its own parent.`); + const parent = this.db.prepare("select title from tasks where id = ? and org_id = ?").get(parentTaskId, orgId) as { title: string } | undefined; + if (!parent) throw new Error("Parent task was not found."); + const loop = this.db.prepare(` + with recursive ancestors(id) as ( + select parent_task_id from tasks where id = ? + union select t.parent_task_id from tasks t join ancestors a on t.id = a.id where t.parent_task_id is not null + ) + select 1 from ancestors where id = ? limit 1 + `).get(parentTaskId, taskId); + if (loop) throw new Error(`"${title}" cannot sit under "${parent.title}": "${parent.title}" is already beneath "${title}".`); + } + private assertNoOpenSubtasks(orgId: string, taskId: string): void { const open = this.db.prepare("select title from tasks where org_id = ? and parent_task_id = ? and status <> 'done' order by created_at") .all(orgId, taskId) as Array<{ title: string }>; diff --git a/src/renderer/src/pages/work/TaskDetailDialog.tsx b/src/renderer/src/pages/work/TaskDetailDialog.tsx index 746e68f9..7a2c4fb8 100644 --- a/src/renderer/src/pages/work/TaskDetailDialog.tsx +++ b/src/renderer/src/pages/work/TaskDetailDialog.tsx @@ -13,7 +13,7 @@ import type { } from "../../../../shared/domain"; import { TASK_STATUS } from "../../../../shared/domain"; import { auditEventLabel, formatRelative } from "../types"; -import { nextStatus, prettyRunState, prettyStatus, providerFor, runBlocker, runsForTask, runTone } from "./task-helpers"; +import { descendantIds, nextStatus, prettyRunState, prettyStatus, providerFor, runBlocker, runsForTask, runTone } from "./task-helpers"; import type { SnapshotActions } from "../../app/useSnapshot"; import { t } from "../../i18n/i18n"; @@ -59,6 +59,7 @@ export function TaskDetailDialog({ }) { const [tab, setTab] = useState("overview"); const [editing, setEditing] = useState(false); + const [editError, setEditError] = useState(null); if (!task) return null; const assignee = agents.find((a) => a.id === task.assigneeAgentId) ?? null; @@ -76,6 +77,7 @@ export function TaskDetailDialog({ event.preventDefault(); const form = new FormData(event.currentTarget); const val = (key: string) => String(form.get(key) ?? "").trim(); + setEditError(null); void actions.run(() => window.agentCompany.updateTask({ orgId, taskId: task.id, @@ -85,7 +87,7 @@ export function TaskDetailDialog({ risk: (val("risk") || "low") as "low" | "medium" | "high", priority: (val("priority") || "normal") as "low" | "normal" | "high", parentTaskId: val("parentTaskId") || null, - })).then((r) => { if (r) setEditing(false); }); + })).then((r) => { if (r) setEditing(false); }, (error: unknown) => setEditError(error instanceof Error ? error.message : String(error))); }; const deleteTask = () => { @@ -99,7 +101,7 @@ export function TaskDetailDialog({
{task.priority === "high" && }

{task.title}

- setEditing(true)} /> + { setEditError(null); setEditing(true); }} />
@@ -127,7 +129,7 @@ export function TaskDetailDialog({ {advanceTo && }
- {editing && setEditing(false)} />} + {editing && { setEditing(false); setEditError(null); }} />} ); @@ -221,11 +223,12 @@ function ActivityTab({ audit, approvals }: { audit: AuditEventRecord[]; approval ); } -function EditOverlay({ task, agents, allTasks, onSubmit, onCancel }: { - task: TaskRecord; agents: AgentRecord[]; allTasks: TaskRecord[]; +function EditOverlay({ task, agents, allTasks, error, onSubmit, onCancel }: { + task: TaskRecord; agents: AgentRecord[]; allTasks: TaskRecord[]; error: string | null; onSubmit: (e: FormEvent) => void; onCancel: () => void; }) { - const otherTasks = allTasks.filter((t) => t.id !== task.id && t.orgId === task.orgId); + const beneath = descendantIds(task.id, allTasks); + const otherTasks = allTasks.filter((t) => t.id !== task.id && t.orgId === task.orgId && !beneath.has(t.id)); return (
@@ -256,6 +259,7 @@ function EditOverlay({ task, agents, allTasks, onSubmit, onCancel }: { {otherTasks.map((t) => )} + {error &&
{error}
}
diff --git a/src/renderer/src/pages/work/task-helpers.ts b/src/renderer/src/pages/work/task-helpers.ts index 0b1454e3..43b678aa 100644 --- a/src/renderer/src/pages/work/task-helpers.ts +++ b/src/renderer/src/pages/work/task-helpers.ts @@ -62,3 +62,19 @@ export function latestTaskRun(taskId: string, taskRuns: TaskRunRecord[]): TaskRu export function runsForTask(taskId: string, taskRuns: TaskRunRecord[]): TaskRunRecord[] { return taskRuns.filter((run) => run.taskId === taskId); } + +/** Every task beneath this one; none of them may become its parent. */ +export function descendantIds(taskId: string, tasks: TaskRecord[]): Set { + const found = new Set(); + const frontier = [taskId]; + while (frontier.length > 0) { + const current = frontier.pop(); + for (const task of tasks) { + if (task.parentTaskId === current && !found.has(task.id)) { + found.add(task.id); + frontier.push(task.id); + } + } + } + return found; +} diff --git a/tests/unit/run-blocker.test.ts b/tests/unit/task-helpers.test.ts similarity index 66% rename from tests/unit/run-blocker.test.ts rename to tests/unit/task-helpers.test.ts index 967f6bab..604fb0f6 100644 --- a/tests/unit/run-blocker.test.ts +++ b/tests/unit/task-helpers.test.ts @@ -1,12 +1,12 @@ import { describe, expect, test } from "vitest"; -import { runBlocker } from "../../src/renderer/src/pages/work/task-helpers"; +import { descendantIds, runBlocker } from "../../src/renderer/src/pages/work/task-helpers"; import type { ProviderRecord, TaskRecord, TaskRunRecord } from "../../src/shared/domain"; const provider = { enabled: true, authState: "configured" } as ProviderRecord; const task = (status: TaskRecord["status"]) => ({ id: "tsk_1", status }) as TaskRecord; const run = (state: string, taskId = "tsk_1") => ({ taskId, state }) as TaskRunRecord; -describe("run blocker", () => { +describe("work task helpers", () => { test("a manual run needs a provider, an open task and no session in flight", () => { expect(runBlocker(task("in_progress"), provider, [run("completed"), run("running", "tsk_other")])).toBeNull(); expect(runBlocker(task("in_progress"), null, [])).toBe("Connect a provider to run"); @@ -14,4 +14,10 @@ describe("run blocker", () => { expect(runBlocker(task("backlog"), provider, [run("queued")])).toBe("A session is already queued or running"); expect(runBlocker(task("review"), provider, [run("running")])).toBe("A session is already queued or running"); }); + + test("descendants are found through every level, so the parent picker can leave them out", () => { + const tree = [["b", "a"], ["c", "b"], ["d", null], ["e", "c"]].map(([id, parentTaskId]) => ({ id, parentTaskId }) as TaskRecord); + expect([...descendantIds("a", tree)].sort()).toEqual(["b", "c", "e"]); + expect([...descendantIds("d", tree)]).toEqual([]); + }); }); diff --git a/tests/unit/task-hierarchy.test.ts b/tests/unit/task-hierarchy.test.ts new file mode 100644 index 00000000..639809d6 --- /dev/null +++ b/tests/unit/task-hierarchy.test.ts @@ -0,0 +1,36 @@ +import { randomUUID } from "node:crypto"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, test } from "vitest"; +import { DatabaseConnection } from "../../src/main/database/connection"; +import { OrganizationService } from "../../src/main/services/organization-service"; +import { RISK_LEVEL, TASK_PRIORITY, TASK_STATUS } from "../../src/shared/domain"; + +describe("task hierarchy", () => { + test("a task cannot become its own parent or the child of its own descendant", () => { + const connection = new DatabaseConnection(join(tmpdir(), `ac-tree-${randomUUID()}.db`)); + const organization = new OrganizationService(connection.db); + let orgId = ""; + const ids: Record = {}; + connection.transaction(() => { + orgId = organization.createOrganization({ organizationName: "Acme", employeeName: "Dana", mission: "m", goal: "g" }); + for (const title of ["A", "B", "C"]) { + ids[title] = organization.createTask({ + orgId, title, description: "d", status: TASK_STATUS.BACKLOG, assigneeAgentId: null, + risk: RISK_LEVEL.LOW, projectId: null, goalId: null, workspaceId: null, priority: TASK_PRIORITY.NORMAL, requiresFilesystem: false, + }); + } + }); + const reparent = (title: string, parent: string | null) => organization.updateTask({ + orgId, taskId: ids[title], title, description: "d", assigneeAgentId: null, risk: RISK_LEVEL.LOW, priority: TASK_PRIORITY.NORMAL, parentTaskId: parent, + }); + reparent("B", ids.A); + reparent("C", ids.B); + expect(() => reparent("A", ids.A)).toThrow('"A" cannot be its own parent.'); + expect(() => reparent("A", ids.C)).toThrow('"A" cannot sit under "C": "C" is already beneath "A".'); + expect(connection.db.prepare("select parent_task_id from tasks where id = ?").get(ids.A)).toEqual({ parent_task_id: null }); + reparent("C", ids.A); + reparent("C", null); + connection.close(); + }); +});