feat(FN-5256): complete Step 2 — enforce cycle checks on writes

packages/core/src/__tests__/store-dependency-cycle.test.ts

@@ -0,0 +1,86 @@
+import { afterEach, beforeEach, describe, expect, it } from "vitest";
+import {
+  DependencyCycleError,
+  detectDependencyCycle,
+} from "../store.js";
+import { createTaskStoreTestHarness } from "./store-test-helpers.js";
+
+describe("detectDependencyCycle", () => {
+  const lookup = (graph: Record<string, string[]>) => (taskId: string) => graph[taskId];
+
+  it("detects direct self-edge", () => {
+    expect(detectDependencyCycle("A", ["A"], lookup({}))).toEqual(["A", "A"]);
+  });
+
+  it("detects 2-node cycle", () => {
+    expect(detectDependencyCycle("A", ["B"], lookup({ B: ["A"] }))).toEqual(["A", "B", "A"]);
+  });
+
+  it("detects 3-node cycle", () => {
+    expect(detectDependencyCycle("FN-5240", ["FN-5241"], lookup({
+      "FN-5241": ["FN-5242"],
+      "FN-5242": ["FN-5240"],
+    }))).toEqual(["FN-5240", "FN-5241", "FN-5242", "FN-5240"]);
+  });
+
+  it("returns null for diamond non-cycle", () => {
+    expect(detectDependencyCycle("A", ["B", "C"], lookup({ B: ["D"], C: ["D"], D: [] }))).toBeNull();
+  });
+
+  it("ignores missing dependencies", () => {
+    expect(detectDependencyCycle("A", ["MISSING"], lookup({}))).toBeNull();
+  });
+
+  it("supports candidate not yet persisted", () => {
+    expect(detectDependencyCycle("A", ["B"], lookup({ B: ["C"], C: [] }))).toBeNull();
+  });
+});
+
+describe("TaskStore dependency cycle guard", () => {
+  const harness = createTaskStoreTestHarness();
+
+  beforeEach(async () => {
+    await harness.beforeEach();
+  });
+
+  afterEach(async () => {
+    await harness.afterEach();
+  });
+
+  it("rejects cycle-forming update and preserves persisted dependencies", async () => {
+    const store = harness.store();
+    const a = await store.createTask({ title: "A", description: "A" });
+    const b = await store.createTask({ title: "B", description: "B", dependencies: [a.id] });
+
+    await expect(store.updateTask(a.id, { dependencies: [b.id] })).rejects.toBeInstanceOf(DependencyCycleError);
+
+    const refreshedA = await store.getTask(a.id);
+    expect(refreshedA.dependencies).toEqual([]);
+
+    const rows = (store as any).db.prepare(`SELECT mutationType FROM runAuditEvents WHERE taskId = ? AND mutationType = ?`).all(a.id, "task:dependency-cycle-rejected");
+    expect(rows).toHaveLength(1);
+  });
+
+  it("accepts umbrella parent depending on children with no back-edge", async () => {
+    const store = harness.store();
+    const childA = await store.createTask({ title: "child-a", description: "a" });
+    const childB = await store.createTask({ title: "child-b", description: "b" });
+
+    const parent = await store.createTask({
+      title: "umbrella",
+      description: "parent",
+      dependencies: [childA.id, childB.id],
+    });
+
+    expect(parent.dependencies).toEqual([childA.id, childB.id]);
+  });
+
+  it("accepts non-cyclic updates", async () => {
+    const store = harness.store();
+    const a = await store.createTask({ title: "A", description: "A" });
+    const b = await store.createTask({ title: "B", description: "B" });
+
+    const updated = await store.updateTask(b.id, { dependencies: [a.id] });
+    expect(updated.dependencies).toEqual([a.id]);
+  });
+});

packages/core/src/index.ts

@@ -130,7 +130,9 @@ export {
   TaskStore,
   SELF_DEFEATING_OPERATION_VERBS,
   detectSelfDefeatingDependency,
+  detectDependencyCycle,
   SelfDefeatingDependencyError,
+  DependencyCycleError,
   TaskDeletedError,
   MergeQueueTaskNotFoundError,
   MergeQueueLeaseOwnershipError,

packages/core/src/store.ts

@@ -837,6 +837,70 @@ export function detectSelfDefeatingDependency(
   };
 }
 
+export class DependencyCycleError extends Error {
+  readonly code = "DEPENDENCY_CYCLE" as const;
+
+  constructor(
+    readonly taskId: string,
+    readonly cyclePath: readonly string[],
+  ) {
+    super(`Dependency cycle detected for ${taskId}: ${cyclePath.join(" → ")}`);
+    this.name = "DependencyCycleError";
+  }
+}
+
+export function detectDependencyCycle(
+  candidateTaskId: string,
+  candidateDependencies: readonly string[],
+  lookupDependencies: (taskId: string) => readonly string[] | undefined,
+): string[] | null {
+  const visited = new Set<string>();
+
+  for (const dep of candidateDependencies) {
+    if (dep === candidateTaskId) {
+      return [candidateTaskId, candidateTaskId];
+    }
+
+    const initialDeps = lookupDependencies(dep);
+    if (!initialDeps) continue;
+
+    const stack: Array<{ taskId: string; deps: readonly string[]; index: number }> = [
+      { taskId: dep, deps: initialDeps, index: 0 },
+    ];
+    const path = [candidateTaskId, dep];
+
+    while (stack.length > 0) {
+      const top = stack[stack.length - 1]!;
+      if (top.index >= top.deps.length) {
+        stack.pop();
+        path.pop();
+        continue;
+      }
+
+      const next = top.deps[top.index++]!;
+      if (next === candidateTaskId) {
+        return [...path, candidateTaskId];
+      }
+
+      if (visited.has(next)) {
+        continue;
+      }
+
+      const nextDeps = lookupDependencies(next);
+      if (!nextDeps) {
+        visited.add(next);
+        continue;
+      }
+
+      visited.add(next);
+      stack.push({ taskId: next, deps: nextDeps, index: 0 });
+      path.push(next);
+    }
+  }
+
+  return null;
+}
+
 export class MergeQueueTaskNotFoundError extends Error {
   constructor(public readonly taskId: string) {
     super(`Cannot enqueue merge queue entry for missing task ${taskId}`);
@@ -3213,6 +3277,50 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
     return resolved.length > 0 ? resolved : undefined;
   }
 
+  private async buildActiveTaskDependencyLookup(overrides?: Map<string, readonly string[]>): Promise<Map<string, readonly string[]>> {
+    const tasks = await this.listTasks({ includeArchived: false });
+    const lookup = new Map<string, readonly string[]>();
+    for (const task of tasks) {
+      lookup.set(task.id, task.dependencies ?? []);
+    }
+    if (overrides) {
+      for (const [taskId, deps] of overrides.entries()) {
+        lookup.set(taskId, deps);
+      }
+    }
+    return lookup;
+  }
+
+  private recordDependencyCycleRejectedAudit(
+    taskId: string,
+    cyclePath: readonly string[],
+    source: "createTask" | "createTaskWithReservedId" | "updateTask" | "replication",
+  ): void {
+    this.insertRunAuditEventRow({
+      taskId,
+      domain: "database",
+      mutationType: source === "replication" ? "task:dependency-cycle-rejected-replication" : "task:dependency-cycle-rejected",
+      target: taskId,
+      metadata: { taskId, cyclePath, source },
+    });
+  }
+
+  private async assertNoDependencyCycle(
+    taskId: string,
+    dependencies: readonly string[],
+    source: "createTask" | "createTaskWithReservedId" | "updateTask" | "replication",
+    overrides?: Map<string, readonly string[]>,
+  ): Promise<void> {
+    const lookup = await this.buildActiveTaskDependencyLookup(overrides);
+    const cyclePath = detectDependencyCycle(taskId, dependencies, (candidateId) => lookup.get(candidateId));
+    if (!cyclePath) return;
+    this.recordDependencyCycleRejectedAudit(taskId, cyclePath, source);
+    if (source === "replication") {
+      storeLog.warn("Skipping replicated task create due to dependency cycle", { taskId, cyclePath });
+      return;
+    }
+    throw new DependencyCycleError(taskId, cyclePath);
+  }
   async createTask(
     input: TaskCreateInput,
     options?: {
@@ -3302,6 +3410,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
         if (input.dependencies?.includes(taskId)) {
           throw new Error(`Task ${taskId} cannot depend on itself`);
         }
+        await this.assertNoDependencyCycle(taskId, input.dependencies ?? [], "createTask");
         return this._createTaskInternal(
           input,
           title,
@@ -3405,6 +3514,8 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
       throw new Error(`Task ${id} cannot depend on itself`);
     }
 
+    await this.assertNoDependencyCycle(id, input.dependencies ?? [], "createTaskWithReservedId");
+
     this.assertTaskIdAvailable(id);
 
     const title = input.title?.trim() || undefined;
@@ -3456,6 +3567,20 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
       throw replicationCollisionError(payload.taskId);
     }
 
+    if (payload.input.dependencies?.includes(payload.taskId)) {
+      this.recordDependencyCycleRejectedAudit(payload.taskId, [payload.taskId, payload.taskId], "replication");
+      storeLog.warn("Skipping replicated task create due to self dependency", { taskId: payload.taskId });
+      return { task: payload.input as Task, applied: false };
+    }
+
+    const lookup = await this.buildActiveTaskDependencyLookup(new Map([[payload.taskId, payload.input.dependencies ?? []]]));
+    const replicationCycle = detectDependencyCycle(payload.taskId, payload.input.dependencies ?? [], (candidateId) => lookup.get(candidateId));
+    if (replicationCycle) {
+      this.recordDependencyCycleRejectedAudit(payload.taskId, replicationCycle, "replication");
+      storeLog.warn("Skipping replicated task create due to dependency cycle", { taskId: payload.taskId, cyclePath: replicationCycle });
+      return { task: payload.input as Task, applied: false };
+    }
+
     const task = await this.createTaskWithReservedId(payload.input, {
       taskId: payload.taskId,
       createdAt: payload.createdAt,
@@ -4944,6 +5069,14 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
       if (updates.dependencies?.includes(id)) {
         throw new Error(`Task ${id} cannot depend on itself`);
       }
+      if (updates.dependencies !== undefined) {
+        await this.assertNoDependencyCycle(
+          id,
+          updates.dependencies,
+          "updateTask",
+          new Map([[id, updates.dependencies]]),
+        );
+      }
 
       const dir = this.taskDir(id);
       const task = await this.readTaskJson(dir);