feat(FN-3899): stabilize blockedBy overlap stamping and add recovery utilit
Stabilizes blocked-by overlap stamping in the scheduler and adds a standalone `recover-stale-blocked-by` utility script to clear stale upstream blockers, with corresponding scheduler tests and architecture documentation. Fusion-Task-Id: FN-3899
This commit is contained in:
@@ -1052,6 +1052,125 @@ describe("Scheduler", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("blockedBy stability — FN-3899", () => {
|
||||
it("preserves a still-valid queued blocker instead of repointing to another active task", async () => {
|
||||
vi.mocked(existsSync).mockReturnValue(true);
|
||||
vi.mocked(readFile).mockResolvedValue("# Task\nDo something");
|
||||
|
||||
const tasks = [
|
||||
createMockTask({ id: "FN-A", column: "in-progress" }),
|
||||
createMockTask({ id: "FN-B", column: "in-progress" }),
|
||||
createMockTask({ id: "FN-T", column: "todo", status: "queued", blockedBy: "FN-B" }),
|
||||
];
|
||||
|
||||
const parseScopeMock = vi.fn(async (taskId: string): Promise<string[]> => {
|
||||
if (taskId === "FN-A") return ["packages/engine/src/merger.ts", "packages/dashboard/app/components/Header.tsx"];
|
||||
if (taskId === "FN-B") return ["packages/dashboard/app/App.tsx"];
|
||||
if (taskId === "FN-T") return ["packages/dashboard/app/App.tsx"];
|
||||
return [];
|
||||
});
|
||||
|
||||
const updateTask = vi.fn().mockResolvedValue(undefined);
|
||||
const moveTask = vi.fn().mockResolvedValue(undefined);
|
||||
const store = createMockStore({
|
||||
listTasks: vi.fn().mockResolvedValue(tasks),
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 10,
|
||||
maxWorktrees: 10,
|
||||
groupOverlappingFiles: true,
|
||||
}),
|
||||
parseFileScopeFromPrompt: parseScopeMock,
|
||||
updateTask,
|
||||
moveTask,
|
||||
});
|
||||
|
||||
const scheduler = new Scheduler(store);
|
||||
(scheduler as any).running = true;
|
||||
await scheduler.schedule();
|
||||
|
||||
expect(updateTask).not.toHaveBeenCalledWith("FN-T", { status: "queued", blockedBy: "FN-A" });
|
||||
expect(moveTask).not.toHaveBeenCalledWith("FN-T", "in-progress", expect.anything());
|
||||
});
|
||||
|
||||
it("recomputes stale queued blockers when the recorded blocker is no longer overlapping", async () => {
|
||||
vi.mocked(existsSync).mockReturnValue(true);
|
||||
vi.mocked(readFile).mockResolvedValue("# Task\nDo something");
|
||||
|
||||
const tasks = [
|
||||
createMockTask({ id: "FN-A", column: "in-progress" }),
|
||||
createMockTask({ id: "FN-B", column: "in-progress" }),
|
||||
createMockTask({ id: "FN-T", column: "todo", status: "queued", blockedBy: "FN-A" }),
|
||||
];
|
||||
|
||||
const parseScopeMock = vi.fn(async (taskId: string): Promise<string[]> => {
|
||||
if (taskId === "FN-A") return ["packages/engine/src/merger.ts"];
|
||||
if (taskId === "FN-B") return ["packages/dashboard/app/App.tsx"];
|
||||
if (taskId === "FN-T") return ["packages/dashboard/app/App.tsx"];
|
||||
return [];
|
||||
});
|
||||
|
||||
const updateTask = vi.fn().mockResolvedValue(undefined);
|
||||
const store = createMockStore({
|
||||
listTasks: vi.fn().mockResolvedValue(tasks),
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 10,
|
||||
maxWorktrees: 10,
|
||||
groupOverlappingFiles: true,
|
||||
}),
|
||||
parseFileScopeFromPrompt: parseScopeMock,
|
||||
updateTask,
|
||||
moveTask: vi.fn().mockResolvedValue(undefined),
|
||||
});
|
||||
|
||||
const scheduler = new Scheduler(store);
|
||||
(scheduler as any).running = true;
|
||||
await scheduler.schedule();
|
||||
|
||||
expect(updateTask).toHaveBeenCalledWith("FN-T", { status: "queued", blockedBy: "FN-B" });
|
||||
});
|
||||
|
||||
it("does not stamp blockedBy for todos without overlap, including empty scopes", async () => {
|
||||
vi.mocked(existsSync).mockReturnValue(true);
|
||||
vi.mocked(readFile).mockResolvedValue("# Task\nDo something");
|
||||
|
||||
const tasks = [
|
||||
createMockTask({ id: "FN-A", column: "in-progress" }),
|
||||
createMockTask({ id: "FN-T1", column: "todo" }),
|
||||
createMockTask({ id: "FN-T2", column: "todo" }),
|
||||
];
|
||||
|
||||
const parseScopeMock = vi.fn(async (taskId: string): Promise<string[]> => {
|
||||
if (taskId === "FN-A") return ["packages/engine/src/merger.ts"];
|
||||
if (taskId === "FN-T1") return ["packages/dashboard/app/App.tsx"];
|
||||
if (taskId === "FN-T2") return [];
|
||||
return [];
|
||||
});
|
||||
|
||||
const updateTask = vi.fn().mockResolvedValue(undefined);
|
||||
const moveTask = vi.fn().mockResolvedValue(undefined);
|
||||
const store = createMockStore({
|
||||
listTasks: vi.fn().mockResolvedValue(tasks),
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 10,
|
||||
maxWorktrees: 10,
|
||||
groupOverlappingFiles: true,
|
||||
}),
|
||||
parseFileScopeFromPrompt: parseScopeMock,
|
||||
updateTask,
|
||||
moveTask,
|
||||
});
|
||||
|
||||
const scheduler = new Scheduler(store);
|
||||
(scheduler as any).running = true;
|
||||
await scheduler.schedule();
|
||||
|
||||
expect(updateTask).not.toHaveBeenCalledWith("FN-T1", { status: "queued", blockedBy: "FN-A" });
|
||||
expect(updateTask).not.toHaveBeenCalledWith("FN-T2", { status: "queued", blockedBy: "FN-A" });
|
||||
expect(moveTask).toHaveBeenCalledWith("FN-T1", "in-progress", expect.objectContaining({ allocateWorktree: expect.any(Function) }));
|
||||
expect(moveTask).toHaveBeenCalledWith("FN-T2", "in-progress", expect.objectContaining({ allocateWorktree: expect.any(Function) }));
|
||||
});
|
||||
});
|
||||
|
||||
describe("worktree reservation", () => {
|
||||
it("assigns a planned worktree path before moving a task to in-progress", async () => {
|
||||
vi.mocked(existsSync).mockReturnValue(true);
|
||||
|
||||
@@ -792,24 +792,32 @@ export class Scheduler {
|
||||
overlapIgnorePaths,
|
||||
);
|
||||
if (taskScope.length > 0) {
|
||||
let overlappingTaskId: string | null = null;
|
||||
for (const [ipId, ipScope] of activeScopes) {
|
||||
if (this.pathsOverlap(taskScope, ipScope)) {
|
||||
overlappingTaskId = ipId;
|
||||
break;
|
||||
}
|
||||
}
|
||||
const activeScopeEntries = Array.from(activeScopes.entries()).sort(([aId], [bId]) => aId.localeCompare(bId));
|
||||
const currentBlockerScope = task.blockedBy ? activeScopes.get(task.blockedBy) : undefined;
|
||||
const hasValidCurrentBlocker =
|
||||
Boolean(task.blockedBy)
|
||||
&& Boolean(currentBlockerScope)
|
||||
&& this.pathsOverlap(taskScope, currentBlockerScope!);
|
||||
|
||||
/**
|
||||
* blockedBy stamping invariants:
|
||||
* - sticky when still valid: preserve an existing active overlapping blocker
|
||||
* - deterministic when changing: pick the first overlapping active task by sorted taskId
|
||||
* - idempotent writes only: update DB only when blockedBy/status must change
|
||||
*/
|
||||
const overlappingTaskId = hasValidCurrentBlocker
|
||||
? task.blockedBy
|
||||
: activeScopeEntries.find(([, ipScope]) => this.pathsOverlap(taskScope, ipScope))?.[0] ?? null;
|
||||
|
||||
if (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 },
|
||||
);
|
||||
const targetBlockedBy = task.dependencies.length > 0 ? null : overlappingTaskId;
|
||||
if (task.status !== "queued" || task.blockedBy !== targetBlockedBy) {
|
||||
await this.store.updateTask(task.id, { status: "queued", blockedBy: targetBlockedBy });
|
||||
}
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user