feat(FN-5486): merge fusion/fn-5486
This commit is contained in:
@@ -6,7 +6,7 @@ import { existsSync } from "node:fs";
|
||||
import * as projectMemory from "../project-memory.js";
|
||||
import { AgentStore } from "../agent-store.js";
|
||||
import { CentralDatabase } from "../central-db.js";
|
||||
import { TaskStore, TaskHasDependentsError } from "../store.js";
|
||||
import { DependencyCycleError, TaskStore, TaskHasDependentsError } from "../store.js";
|
||||
import { setTaskCreatedHook } from "../task-creation-hooks.js";
|
||||
import { buildResearchDocumentKey, type Task } from "../types.js";
|
||||
import { createTaskStoreTestHarness, makeTmpDir } from "./store-test-helpers.js";
|
||||
@@ -842,12 +842,21 @@ describe("TaskStore", () => {
|
||||
store.createTaskWithReservedId({ description: "duplicate" }, { taskId: "FN-9003" }),
|
||||
).rejects.toThrow("Task ID already exists: FN-9003");
|
||||
|
||||
await expect(
|
||||
store.createTaskWithReservedId(
|
||||
let error: unknown;
|
||||
try {
|
||||
await store.createTaskWithReservedId(
|
||||
{ description: "self dep", dependencies: ["FN-9004"] },
|
||||
{ taskId: "FN-9004" },
|
||||
),
|
||||
).rejects.toThrow("Task FN-9004 cannot depend on itself");
|
||||
);
|
||||
} catch (caught) {
|
||||
error = caught;
|
||||
}
|
||||
|
||||
expect(error).toBeInstanceOf(DependencyCycleError);
|
||||
expect(error).toMatchObject({
|
||||
taskId: "FN-9004",
|
||||
cyclePath: ["FN-9004", "FN-9004"],
|
||||
});
|
||||
});
|
||||
|
||||
it("applyReplicatedTaskCreate does not auto-apply default workflow steps", async () => {
|
||||
|
||||
@@ -136,9 +136,69 @@ describe("TaskStore dependency cycle guard", () => {
|
||||
const store = harness.store();
|
||||
const a = await store.createTask({ title: "A", description: "A" });
|
||||
|
||||
await expect(store.updateTask(a.id, { dependencies: [a.id] })).rejects.toThrow(
|
||||
`Task ${a.id} cannot depend on itself`,
|
||||
);
|
||||
let error: unknown;
|
||||
try {
|
||||
await store.updateTask(a.id, { dependencies: [a.id] });
|
||||
} catch (caught) {
|
||||
error = caught;
|
||||
}
|
||||
|
||||
expect(error).toBeInstanceOf(DependencyCycleError);
|
||||
expect(error).toMatchObject({
|
||||
name: "DependencyCycleError",
|
||||
taskId: a.id,
|
||||
cyclePath: [a.id, a.id],
|
||||
});
|
||||
expect((error as DependencyCycleError).message).toContain(`${a.id} → ${a.id}`);
|
||||
|
||||
const refreshedA = await store.getTask(a.id);
|
||||
expect(refreshedA.dependencies).toEqual([]);
|
||||
|
||||
const rows = (store as any).db
|
||||
.prepare("SELECT metadata FROM runAuditEvents WHERE taskId = ? AND mutationType = ?")
|
||||
.all(a.id, "task:dependency-cycle-rejected") as Array<{ metadata: string | { source?: string } }>;
|
||||
expect(rows).toHaveLength(1);
|
||||
const metadata = typeof rows[0].metadata === "string" ? JSON.parse(rows[0].metadata) : rows[0].metadata;
|
||||
expect(metadata.source).toBe("updateTask");
|
||||
});
|
||||
|
||||
it("rejects createTaskWithReservedId self-loop with typed cycle contract", async () => {
|
||||
const store = harness.store();
|
||||
|
||||
let error: unknown;
|
||||
try {
|
||||
await store.createTaskWithReservedId(
|
||||
{ title: "self", description: "self", dependencies: ["FN-SELF-1"] },
|
||||
{ taskId: "FN-SELF-1" },
|
||||
);
|
||||
} catch (caught) {
|
||||
error = caught;
|
||||
}
|
||||
|
||||
expect(error).toBeInstanceOf(DependencyCycleError);
|
||||
expect(error).toMatchObject({
|
||||
taskId: "FN-SELF-1",
|
||||
cyclePath: ["FN-SELF-1", "FN-SELF-1"],
|
||||
});
|
||||
|
||||
const rows = (store as any).db
|
||||
.prepare("SELECT metadata FROM runAuditEvents WHERE taskId = ? AND mutationType = ?")
|
||||
.all("FN-SELF-1", "task:dependency-cycle-rejected") as Array<{ metadata: string | { source?: string } }>;
|
||||
expect(rows).toHaveLength(1);
|
||||
const metadata = typeof rows[0].metadata === "string" ? JSON.parse(rows[0].metadata) : rows[0].metadata;
|
||||
expect(metadata.source).toBe("createTaskWithReservedId");
|
||||
|
||||
await expect(store.getTask("FN-SELF-1")).rejects.toThrow("Task FN-SELF-1 not found");
|
||||
});
|
||||
|
||||
it("prioritizes self-edge cycle path when mixed with other dependencies", async () => {
|
||||
const store = harness.store();
|
||||
const a = await store.createTask({ title: "A", description: "A" });
|
||||
|
||||
await expect(store.updateTask(a.id, { dependencies: [a.id, "FN-NONEXISTENT"] })).rejects.toMatchObject({
|
||||
taskId: a.id,
|
||||
cyclePath: [a.id, a.id],
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects incremental update that closes a loop and preserves state", async () => {
|
||||
|
||||
@@ -6,7 +6,7 @@ import { existsSync } from "node:fs";
|
||||
import * as projectMemory from "../project-memory.js";
|
||||
import { AgentStore } from "../agent-store.js";
|
||||
import { CentralDatabase } from "../central-db.js";
|
||||
import { TaskStore, TaskHasDependentsError } from "../store.js";
|
||||
import { DependencyCycleError, TaskStore, TaskHasDependentsError } from "../store.js";
|
||||
import { buildResearchDocumentKey, type Task } from "../types.js";
|
||||
import { createSharedTaskStoreTestHarness, makeTmpDir } from "./store-test-helpers.js";
|
||||
|
||||
@@ -76,34 +76,43 @@ describe("TaskStore", () => {
|
||||
|
||||
|
||||
describe("self-dependency validation", () => {
|
||||
it("createTask should throw when dependencies include self", async () => {
|
||||
// We can't know the ID before creation, so we test the update scenario
|
||||
// or test that the check exists in the code path
|
||||
const expectSelfLoopError = async (taskId: string, dependencies: string[]) => {
|
||||
let error: unknown;
|
||||
try {
|
||||
await store.updateTask(taskId, { dependencies });
|
||||
} catch (caught) {
|
||||
error = caught;
|
||||
}
|
||||
|
||||
expect(error).toBeInstanceOf(DependencyCycleError);
|
||||
expect(error).toMatchObject({
|
||||
name: "DependencyCycleError",
|
||||
taskId,
|
||||
cyclePath: [taskId, taskId],
|
||||
});
|
||||
};
|
||||
|
||||
it("createTask should reject self-dependency update with DependencyCycleError", async () => {
|
||||
const task = await createTestTask();
|
||||
// After creation, task.id is known (e.g., KB-001)
|
||||
// Now try to update it to depend on itself
|
||||
await expect(store.updateTask(task.id, { dependencies: [task.id] }))
|
||||
.rejects.toThrow(`Task ${task.id} cannot depend on itself`);
|
||||
await expectSelfLoopError(task.id, [task.id]);
|
||||
});
|
||||
|
||||
it("updateTask should throw when setting dependencies to include self", async () => {
|
||||
it("updateTask should reject mixed self + other deps with self cyclePath first", async () => {
|
||||
const task = await createTestTask();
|
||||
expect(task.dependencies).toEqual([]);
|
||||
|
||||
await expect(store.updateTask(task.id, { dependencies: [task.id, "FN-002"] }))
|
||||
.rejects.toThrow(`Task ${task.id} cannot depend on itself`);
|
||||
await expectSelfLoopError(task.id, [task.id, "FN-002"]);
|
||||
|
||||
// Verify the task was not modified
|
||||
const fetched = await store.getTask(task.id);
|
||||
expect(fetched.dependencies).toEqual([]);
|
||||
});
|
||||
|
||||
it("updateTask should throw when updating dependencies to add self (when task already has other dependencies)", async () => {
|
||||
it("updateTask should reject existing dep + self with self cyclePath", async () => {
|
||||
const task = await store.createTask({ description: "Dep task", dependencies: ["KB-999"] });
|
||||
expect(task.dependencies).toEqual(["KB-999"]);
|
||||
|
||||
await expect(store.updateTask(task.id, { dependencies: ["KB-999", task.id] }))
|
||||
.rejects.toThrow(`Task ${task.id} cannot depend on itself`);
|
||||
await expectSelfLoopError(task.id, ["KB-999", task.id]);
|
||||
|
||||
// Verify the task was not modified
|
||||
const fetched = await store.getTask(task.id);
|
||||
|
||||
@@ -3422,9 +3422,6 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
|
||||
const task = await this.createTaskWithDistributedReservation(input, {
|
||||
createTaskWithId: async (taskId) => {
|
||||
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,
|
||||
@@ -3525,10 +3522,6 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
throw new Error("taskId is required");
|
||||
}
|
||||
|
||||
if (input.dependencies?.includes(id)) {
|
||||
throw new Error(`Task ${id} cannot depend on itself`);
|
||||
}
|
||||
|
||||
await this.assertNoDependencyCycle(id, input.dependencies ?? [], "createTaskWithReservedId");
|
||||
|
||||
this.assertTaskIdAvailable(id);
|
||||
@@ -5090,10 +5083,6 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
runContext?: RunMutationContext,
|
||||
): Promise<Task> {
|
||||
return this.withTaskLock(id, async () => {
|
||||
// Validate that task doesn't depend on itself
|
||||
if (updates.dependencies?.includes(id)) {
|
||||
throw new Error(`Task ${id} cannot depend on itself`);
|
||||
}
|
||||
if (updates.dependencies !== undefined) {
|
||||
await this.assertNoDependencyCycle(
|
||||
id,
|
||||
|
||||
Reference in New Issue
Block a user