root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(KB-287): add self-dependency validation to task store

Browse Source 
- Validate that tasks cannot depend on themselves in TaskStore.addDependency()

- Add comprehensive tests for self-dependency validation

- Fix mock isolation issues in store tests
This commit is contained in:
gsxdsm
2026-03-31 09:14:29 -07:00
parent b97c713427
commit 202ec6b687
2 changed files with 60 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

63
packages/core/src/store.test.ts
View File

@@ -564,35 +564,71 @@ describe("TaskStore", () => {
const task = await createTestTask();
expect(task.dependencies).toEqual([]);
const updated = await store.updateTask(task.id, { dependencies: ["KB-001", "KB-002"] });
expect(updated.dependencies).toEqual(["KB-001", "KB-002"]);
const updated = await store.updateTask(task.id, { dependencies: ["KB-999", "KB-002"] });
expect(updated.dependencies).toEqual(["KB-999", "KB-002"]);
// Verify persistence
const fetched = await store.getTask(task.id);
expect(fetched.dependencies).toEqual(["KB-001", "KB-002"]);
expect(fetched.dependencies).toEqual(["KB-999", "KB-002"]);
});
it("replaces existing dependencies", async () => {
const task = await store.createTask({ description: "Dep task", dependencies: ["KB-001"] });
expect(task.dependencies).toEqual(["KB-001"]);
const task = await store.createTask({ description: "Dep task", dependencies: ["KB-999"] });
expect(task.dependencies).toEqual(["KB-999"]);
const updated = await store.updateTask(task.id, { dependencies: ["KB-002", "KB-003"] });
expect(updated.dependencies).toEqual(["KB-002", "KB-003"]);
});
it("clears dependencies with empty array", async () => {
const task = await store.createTask({ description: "Dep task", dependencies: ["KB-001"] });
expect(task.dependencies).toEqual(["KB-001"]);
const task = await store.createTask({ description: "Dep task", dependencies: ["KB-999"] });
expect(task.dependencies).toEqual(["KB-999"]);
const updated = await store.updateTask(task.id, { dependencies: [] });
expect(updated.dependencies).toEqual([]);
});
it("leaves dependencies unchanged when not provided", async () => {
const task = await store.createTask({ description: "Dep task", dependencies: ["KB-001"] });
const task = await store.createTask({ description: "Dep task", dependencies: ["KB-999"] });
const updated = await store.updateTask(task.id, { title: "New title" });
expect(updated.dependencies).toEqual(["KB-001"]);
expect(updated.dependencies).toEqual(["KB-999"]);
});
});
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 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`);
});
it("updateTask should throw when setting dependencies to include self", async () => {
const task = await createTestTask();
expect(task.dependencies).toEqual([]);
await expect(store.updateTask(task.id, { dependencies: [task.id, "KB-002"] }))
.rejects.toThrow(`Task ${task.id} cannot depend on itself`);
// 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 () => {
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`);
// Verify the task was not modified
const fetched = await store.getTask(task.id);
expect(fetched.dependencies).toEqual(["KB-999"]);
});
});
@@ -626,16 +662,16 @@ describe("TaskStore", () => {
});
it("does NOT move when dependencies are removed from a todo task", async () => {
const task = await store.createTask({ description: "Todo task", column: "todo", dependencies: ["KB-001"] });
const task = await store.createTask({ description: "Todo task", column: "todo", dependencies: ["KB-999"] });
const updated = await store.updateTask(task.id, { dependencies: [] });
expect(updated.column).toBe("todo");
});
it("does NOT move when dependencies are replaced with same set", async () => {
const task = await store.createTask({ description: "Todo task", column: "todo", dependencies: ["KB-001"] });
const task = await store.createTask({ description: "Todo task", column: "todo", dependencies: ["KB-999"] });
const updated = await store.updateTask(task.id, { dependencies: ["KB-001"] });
const updated = await store.updateTask(task.id, { dependencies: ["KB-999"] });
expect(updated.column).toBe("todo");
});
@@ -2502,6 +2538,9 @@ describe("TaskStore", () => {
});
it("uses provided title when available (does not call AI)", async () => {
// Clear mock to ensure we're not picking up calls from previous tests
mockPrompt.mockClear();
const task = await store.createTask({
title: "Custom Title",
description: "This is the description that should not become the title",

9
packages/core/src/store.ts
View File

@@ -378,6 +378,10 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
}
const id = await this.allocateId();
// Validate that task doesn't depend on itself
if (input.dependencies?.includes(id)) {
throw new Error(`Task ${id} cannot depend on itself`);
}
// Generate title from description if not provided
const generatedTitle = await generateTitleFromDescription(input.description, this.rootDir);
const title = input.title?.trim() || generatedTitle;
@@ -645,6 +649,11 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
updates: { title?: string; description?: string; prompt?: string; worktree?: string; status?: string | null; dependencies?: string[]; blockedBy?: string | null; paused?: boolean; baseBranch?: string; size?: "S" | "M" | "L"; reviewLevel?: number; mergeRetries?: number; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; error?: string | null; summary?: string | null },
): 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`);
}
const dir = this.taskDir(id);
const task = await this.readTaskJson(dir);