feat(FN-3924): enforce blockedBy blocker invariants in scheduler and recove
Implements invariant enforcement for blocked-by relationships in the scheduler and self-healing recovery, preventing tasks from being blocked by stale or invalid dependencies. The feature adds tests for both scheduler and self-healing behavior, updates the CLI extension and architecture docs, and sh Fusion-Task-Id: FN-3924
This commit is contained in:
@@ -422,6 +422,68 @@ describe("Scheduler", () => {
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-3811", { blockedBy: null, status: null });
|
||||
});
|
||||
|
||||
it("FN-3924: does not repoint cleared dependency blocker to unrelated overlap task", async () => {
|
||||
vi.mocked(existsSync).mockReturnValue(true);
|
||||
vi.mocked(readFile).mockResolvedValue("# Task\nDo something");
|
||||
|
||||
const dep = createMockTask({ id: "FN-DEP", column: "done" });
|
||||
const unrelated = createMockTask({
|
||||
id: "FN-3170",
|
||||
column: "in-review",
|
||||
worktree: "/test/project/.worktrees/fn-3170",
|
||||
});
|
||||
const dependent = createMockTask({
|
||||
id: "FN-3919",
|
||||
column: "todo",
|
||||
status: "queued",
|
||||
blockedBy: "FN-DEP",
|
||||
dependencies: ["FN-DEP"],
|
||||
});
|
||||
const tasks = [dep, unrelated, dependent];
|
||||
|
||||
const listTasks = vi.fn(async (options?: { column?: string; includeArchived?: boolean }) => {
|
||||
if (options?.column === "todo") {
|
||||
return tasks.filter((task) => task.column === "todo");
|
||||
}
|
||||
return tasks;
|
||||
});
|
||||
|
||||
const updateTask = vi.fn(async (id: string, patch: Partial<Task>) => {
|
||||
const task = tasks.find((candidate) => candidate.id === id);
|
||||
if (task) Object.assign(task, patch);
|
||||
return (task ?? createMockTask({ id })) as Task;
|
||||
});
|
||||
|
||||
const store = createMockStore({
|
||||
listTasks,
|
||||
getTask: vi.fn(async (id: string) => (tasks.find((task) => task.id === id) ?? createMockTask({ id })) as any),
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
groupOverlappingFiles: true,
|
||||
}),
|
||||
parseFileScopeFromPrompt: vi.fn(async (taskId: string): Promise<string[]> => {
|
||||
if (taskId === "FN-3170") return ["packages/dashboard/app/App.tsx"];
|
||||
if (taskId === "FN-3919") return ["packages/dashboard/app/App.tsx"];
|
||||
return ["packages/core/src/index.ts"];
|
||||
}),
|
||||
updateTask,
|
||||
moveTask: vi.fn().mockResolvedValue(undefined),
|
||||
});
|
||||
|
||||
const scheduler = new Scheduler(store);
|
||||
const movedHandler = (store.on as any).mock.calls.find((call: any) => call[0] === "task:moved")?.[1];
|
||||
await movedHandler({ task: dep, from: "in-progress", to: "done" });
|
||||
|
||||
expect(updateTask).toHaveBeenCalledWith("FN-3919", { blockedBy: null, status: null });
|
||||
|
||||
(scheduler as any).running = true;
|
||||
await scheduler.schedule();
|
||||
|
||||
expect(updateTask).not.toHaveBeenCalledWith("FN-3919", { status: "queued", blockedBy: "FN-3170" });
|
||||
expect(tasks.find((task) => task.id === "FN-3919")?.blockedBy ?? null).toBeNull();
|
||||
});
|
||||
|
||||
it("does not trigger scheduling for non-done task:moved events", async () => {
|
||||
const store = createMockStore({
|
||||
listTasks: vi.fn().mockResolvedValue([
|
||||
@@ -786,7 +848,7 @@ describe("Scheduler", () => {
|
||||
await scheduler.schedule();
|
||||
|
||||
// Dependency-blocked urgent task should be queued, not started.
|
||||
expect(updateTask).toHaveBeenCalledWith("FN-100", { status: "queued" });
|
||||
expect(updateTask).toHaveBeenCalledWith("FN-100", { status: "queued", blockedBy: "FN-900" });
|
||||
// Overlap-blocked urgent task should be queued with blocker id.
|
||||
expect(updateTask).toHaveBeenCalledWith("FN-103", { status: "queued", blockedBy: "FN-001" });
|
||||
// Paused and recovery-gated urgent tasks never enter scheduling.
|
||||
@@ -1406,7 +1468,7 @@ describe("Scheduler", () => {
|
||||
|
||||
// Task with unmet deps should be queued, not validated
|
||||
// Since KB-006 is not done, KB-005 should not be validated
|
||||
expect(updateTask).toHaveBeenCalledWith("FN-005", { status: "queued" });
|
||||
expect(updateTask).toHaveBeenCalledWith("FN-005", { status: "queued", blockedBy: "FN-006" });
|
||||
// No filesystem validation should occur (no move to triage)
|
||||
expect(moveTask).not.toHaveBeenCalledWith("FN-005", "triage");
|
||||
});
|
||||
|
||||
@@ -461,8 +461,8 @@ describe("SelfHealingManager", () => {
|
||||
enginePaused: false,
|
||||
} as unknown as Settings);
|
||||
vi.mocked(store.listTasks).mockResolvedValue([
|
||||
{ id: "A", column: "todo", blockedBy: "B", paused: false, mergeRetries: 0 } as unknown as Task,
|
||||
{ id: "B", column: "done", blockedBy: null, paused: false, mergeRetries: 0 } as unknown as Task,
|
||||
{ id: "A", column: "todo", blockedBy: "B", paused: false, mergeRetries: 0, dependencies: [] } as unknown as Task,
|
||||
{ id: "B", column: "done", blockedBy: null, paused: false, mergeRetries: 0, dependencies: [] } as unknown as Task,
|
||||
]);
|
||||
|
||||
await manager.runStartupRecovery();
|
||||
@@ -489,8 +489,8 @@ describe("SelfHealingManager", () => {
|
||||
enginePaused: false,
|
||||
} as unknown as Settings);
|
||||
vi.mocked(store.listTasks).mockResolvedValue([
|
||||
{ id: "A", column: "todo", blockedBy: "B", paused: false, mergeRetries: 0 } as unknown as Task,
|
||||
{ id: "B", column: "done", blockedBy: null, paused: false, mergeRetries: 0 } as unknown as Task,
|
||||
{ id: "A", column: "todo", blockedBy: "B", paused: false, mergeRetries: 0, dependencies: [] } as unknown as Task,
|
||||
{ id: "B", column: "done", blockedBy: null, paused: false, mergeRetries: 0, dependencies: [] } as unknown as Task,
|
||||
]);
|
||||
|
||||
await manager.runStartupRecovery();
|
||||
@@ -3513,6 +3513,7 @@ describe("clearStaleBlockedBy", () => {
|
||||
paused: false,
|
||||
blockedBy: null,
|
||||
mergeRetries: 0,
|
||||
dependencies: [],
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
@@ -3597,6 +3598,22 @@ describe("clearStaleBlockedBy", () => {
|
||||
manager.stop();
|
||||
});
|
||||
|
||||
it("clears blockedBy when dependency task has no unresolved deps but blockedBy points elsewhere", async () => {
|
||||
const store = createRunningStore();
|
||||
const taskA = createTask("A", { blockedBy: "FN-400", dependencies: ["FN-DEP"] });
|
||||
const overlapBlocker = createTask("FN-400", { column: "in-progress" });
|
||||
const dependency = createTask("FN-DEP", { column: "done" });
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([taskA, overlapBlocker, dependency]);
|
||||
|
||||
const manager = new SelfHealingManager(store, { rootDir: "/tmp/test-project" });
|
||||
const recovered = await manager.clearStaleBlockedBy();
|
||||
|
||||
expect(recovered).toBe(1);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("A", { blockedBy: null, status: null });
|
||||
expect(store.logEntry).toHaveBeenCalledWith("A", expect.stringContaining("not among unresolved dependencies"));
|
||||
manager.stop();
|
||||
});
|
||||
|
||||
it("does not clear blockedBy when blocker is in-review and not paused/failed", async () => {
|
||||
const store = createRunningStore();
|
||||
const taskA = createTask("A", { blockedBy: "FN-500" });
|
||||
|
||||
@@ -279,31 +279,53 @@ export class Scheduler {
|
||||
}
|
||||
}
|
||||
|
||||
// FN-3895: complement periodic stale-blockedBy self-healing with immediate
|
||||
// unblock when a blocker reaches a terminal completion column.
|
||||
// FN-3895/FN-3924: complement periodic stale-blockedBy self-healing with immediate
|
||||
// blocker reconciliation when a potential blocker reaches a terminal completion column.
|
||||
// Invariant: blockedBy must reference a *current* unresolved blocker, else be null.
|
||||
if (to === "done" || to === "archived") {
|
||||
try {
|
||||
const settings = await this.store.getSettings();
|
||||
if (!settings.globalPause && !settings.enginePaused) {
|
||||
const todoTasks = await this.store.listTasks({ column: "todo", slim: true });
|
||||
const allTasks = await this.store.listTasks({ slim: true, includeArchived: true });
|
||||
const taskById = new Map(allTasks.map((candidate) => [candidate.id, candidate]));
|
||||
for (const dependent of todoTasks) {
|
||||
if (dependent.blockedBy !== task.id) continue;
|
||||
const mentionsCompletedTask = dependent.dependencies.includes(task.id);
|
||||
const currentlyBlockedByCompletedTask = dependent.blockedBy === task.id;
|
||||
if (!mentionsCompletedTask && !currentlyBlockedByCompletedTask) continue;
|
||||
|
||||
const unresolvedDeps = dependent.dependencies.filter((depId) => {
|
||||
const dep = taskById.get(depId);
|
||||
return dep && dep.column !== "done" && dep.column !== "in-review" && dep.column !== "archived";
|
||||
});
|
||||
|
||||
try {
|
||||
await this.store.updateTask(dependent.id, { blockedBy: null, status: null });
|
||||
await this.store.logEntry(
|
||||
dependent.id,
|
||||
`Auto-unblocked: blocker ${task.id} reached ${to}`,
|
||||
);
|
||||
if (unresolvedDeps.length > 0) {
|
||||
await this.store.updateTask(dependent.id, {
|
||||
status: "queued",
|
||||
blockedBy: unresolvedDeps[0],
|
||||
});
|
||||
await this.store.logEntry(
|
||||
dependent.id,
|
||||
`Auto-reblocked: unresolved dependency ${unresolvedDeps[0]} remains after ${task.id} reached ${to}`,
|
||||
);
|
||||
} else {
|
||||
await this.store.updateTask(dependent.id, { blockedBy: null, status: null });
|
||||
await this.store.logEntry(
|
||||
dependent.id,
|
||||
`Auto-unblocked: blocker ${task.id} reached ${to}`,
|
||||
);
|
||||
}
|
||||
} catch (error) {
|
||||
schedulerLog.error(
|
||||
`Failed to auto-unblock dependent ${dependent.id} for blocker ${task.id}`,
|
||||
`Failed to reconcile dependent ${dependent.id} for blocker ${task.id}`,
|
||||
error,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
schedulerLog.error(`Failed event-driven unblock pass for blocker ${task.id}`, error);
|
||||
schedulerLog.error(`Failed event-driven blocker reconciliation for ${task.id}`, error);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -729,7 +751,10 @@ export class Scheduler {
|
||||
});
|
||||
|
||||
if (unmetDeps.length > 0) {
|
||||
await this.store.updateTask(task.id, { status: "queued" });
|
||||
await this.store.updateTask(task.id, {
|
||||
status: "queued",
|
||||
blockedBy: unmetDeps[0],
|
||||
});
|
||||
this.options.onBlocked?.(task, unmetDeps);
|
||||
continue;
|
||||
}
|
||||
@@ -775,7 +800,16 @@ export class Scheduler {
|
||||
}
|
||||
}
|
||||
if (overlappingTaskId) {
|
||||
await this.store.updateTask(task.id, { status: "queued", blockedBy: overlappingTaskId });
|
||||
// Keep blockedBy tied to explicit unresolved dependencies when a task has
|
||||
// dependency edges; avoid repointing dependency-unblocked tasks to unrelated
|
||||
// overlap ids (FN-3924). For dependency-free tasks, blockedBy may reference
|
||||
// the active overlap blocker.
|
||||
await this.store.updateTask(
|
||||
task.id,
|
||||
task.dependencies.length > 0
|
||||
? { status: "queued", blockedBy: null }
|
||||
: { status: "queued", blockedBy: overlappingTaskId },
|
||||
);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1108,6 +1108,11 @@ export class SelfHealingManager {
|
||||
const blocker = taskById.get(blockerId);
|
||||
let reason: string | null = null;
|
||||
|
||||
const unresolvedDeps = task.dependencies.filter((depId) => {
|
||||
const dep = taskById.get(depId);
|
||||
return dep && dep.column !== "done" && dep.column !== "in-review" && dep.column !== "archived";
|
||||
});
|
||||
|
||||
if (!blocker) {
|
||||
reason = `blocker ${blockerId} missing`;
|
||||
} else if (blocker.column === "done") {
|
||||
@@ -1122,6 +1127,8 @@ export class SelfHealingManager {
|
||||
(blocker.mergeRetries ?? 0) >= MAX_AUTO_MERGE_RETRIES
|
||||
) {
|
||||
reason = `blocker ${blockerId} in-review + failed (mergeRetries ${blocker.mergeRetries ?? 0}/${MAX_AUTO_MERGE_RETRIES})`;
|
||||
} else if (task.dependencies.length > 0 && !unresolvedDeps.includes(blockerId)) {
|
||||
reason = `blocker ${blockerId} not among unresolved dependencies`;
|
||||
}
|
||||
|
||||
if (!reason) continue;
|
||||
|
||||
Reference in New Issue
Block a user