Q47: task parents cannot form a cycle
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
b164677ea7
commit
d5d68ad358
5 files changed
+86
-13
No files matched your search
@@ -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 }>;
|
||||
|
||||
@@ -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<Tab>("overview");
|
||||
const [editing, setEditing] = useState(false);
|
||||
const [editError, setEditError] = useState<string | null>(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({
|
||||
<div className="row" style={{ alignItems: "flex-start", gap: 10, marginBottom: 8 }}>
|
||||
{task.priority === "high" && <span style={{ width: 3, height: 18, background: "var(--red)", borderRadius: 2, marginTop: 2, flexShrink: 0 }} />}
|
||||
<h2 className="modal-title" style={{ margin: 0, flex: 1 }}>{task.title}</h2>
|
||||
<IconButton icon="edit" label={t("Edit")} onClick={() => setEditing(true)} />
|
||||
<IconButton icon="edit" label={t("Edit")} onClick={() => { setEditError(null); setEditing(true); }} />
|
||||
<IconButton icon="x" label={t("Close")} onClick={onClose} />
|
||||
</div>
|
||||
<div className="row" style={{ flexWrap: "wrap", gap: 6, marginBottom: 12 }}>
|
||||
@@ -127,7 +129,7 @@ export function TaskDetailDialog({
|
||||
{advanceTo && <Button tone="primary" onClick={() => onAdvance(task)}><Icon name="check" size={16} /> {prettyStatus(advanceTo)}</Button>}
|
||||
</div>
|
||||
|
||||
{editing && <EditOverlay task={task} agents={agents} allTasks={allTasks} onSubmit={submitEdit} onCancel={() => setEditing(false)} />}
|
||||
{editing && <EditOverlay task={task} agents={agents} allTasks={allTasks} error={editError} onSubmit={submitEdit} onCancel={() => { setEditing(false); setEditError(null); }} />}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
@@ -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<HTMLFormElement>) => 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 (
|
||||
<div style={{ position: "absolute", inset: 0, background: "var(--bg)", borderRadius: 14, padding: 24, overflow: "auto", zIndex: 2 }}>
|
||||
<form onSubmit={onSubmit}>
|
||||
@@ -256,6 +259,7 @@ function EditOverlay({ task, agents, allTasks, onSubmit, onCancel }: {
|
||||
{otherTasks.map((t) => <option key={t.id} value={t.id}>{t.title}</option>)}
|
||||
</select>
|
||||
</Field>
|
||||
{error && <div className="form-error" role="alert">{error}</div>}
|
||||
<div className="modal-actions">
|
||||
<Button onClick={onCancel}>{t("Cancel")}</Button>
|
||||
<Button tone="primary" type="submit">{t("Save")}</Button>
|
||||
|
||||
@@ -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<string> {
|
||||
const found = new Set<string>();
|
||||
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;
|
||||
}
|
||||
@@ -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([]);
|
||||
});
|
||||
});
|
||||
@@ -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<string, string> = {};
|
||||
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();
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user