feat(FN-4200): prevent failed tasks from blocking overlap dispatch in sched
The scheduler now excludes failed review tasks from blocking overlap dispatch, preventing stuck tasks from stalling downstream work. A CSS fix prevents the failed badge from incorrectly overriding the in-review status in the list view, with tests covering both the scheduler logic and the badge displ Fusion-Task-Id: FN-4200
This commit is contained in:
5
.changeset/fn-4200-overlap-guard-failed-tasks.md
Normal file
5
.changeset/fn-4200-overlap-guard-failed-tasks.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Permanently-failed `in-review` tasks no longer block overlapping superseding tasks from being dispatched by the scheduler's file-scope overlap guard.
|
||||
@@ -562,6 +562,7 @@ See [Memory Plugin Contract](./memory-plugin-contract.md) for the full plan.
|
||||
|
||||
#### BlockedBy stamping invariants
|
||||
- Scheduler writes overlap-based `blockedBy` only when overlap gating is active and there is a live overlapping active scope; otherwise overlap logic does not stamp blockers.
|
||||
- Active overlap scopes exclude permanently-failed `in-review` tasks (`status === "failed"`, typically produced by `checkStuckBudget()` after `stuckKillCount > maxStuckKills`) so superseding re-implementation tasks are not indefinitely queued behind work that will never merge. (FN-4200)
|
||||
- Stamping is sticky when valid (FN-3899): if a todo task is already `queued` behind a blocker that is still active and still overlaps, the scheduler preserves that blocker and skips rewrites.
|
||||
- When the blocker must change, selection is deterministic: active overlap candidates are ordered by task ID and the first overlapping task is chosen, removing tick-order churn.
|
||||
- Writes are idempotent: scheduler updates `status/blockedBy` only when values change, reducing per-tick churn and audit noise.
|
||||
|
||||
@@ -599,10 +599,6 @@
|
||||
color: var(--text-muted);
|
||||
}
|
||||
|
||||
.list-status-badge.failed {
|
||||
background: color-mix(in srgb, var(--color-error-dark) 15%, transparent);
|
||||
color: var(--color-error-dark);
|
||||
}
|
||||
|
||||
.list-status-badge.stuck {
|
||||
background: color-mix(in srgb, var(--triage) 20%, transparent);
|
||||
@@ -639,6 +635,11 @@
|
||||
color: var(--text-muted);
|
||||
}
|
||||
|
||||
.list-status-badge.failed {
|
||||
background: var(--status-error-bg);
|
||||
color: var(--color-error-dark);
|
||||
}
|
||||
|
||||
.list-column-badge {
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
|
||||
@@ -4,6 +4,7 @@ import userEvent from "@testing-library/user-event";
|
||||
import { ListView } from "../ListView";
|
||||
import type { Task, TaskDetail } from "@fusion/core";
|
||||
import { scopedKey } from "../../utils/projectStorage";
|
||||
import { loadAllAppCss } from "../../test/cssFixture";
|
||||
|
||||
// Mock the API
|
||||
vi.mock("../../api", () => ({
|
||||
@@ -18,6 +19,7 @@ vi.mock("../../api", () => ({
|
||||
groupOverlappingFiles: true,
|
||||
autoMerge: true,
|
||||
}),
|
||||
fetchGlobalSettings: vi.fn().mockResolvedValue({}),
|
||||
fetchTaskDetail: vi.fn(),
|
||||
batchUpdateTaskModels: vi.fn(),
|
||||
fetchNodes: vi.fn().mockResolvedValue([]),
|
||||
@@ -644,6 +646,16 @@ describe("ListView", () => {
|
||||
expect(statusBadge.className).toContain("failed");
|
||||
});
|
||||
|
||||
it("keeps failed list badges on error tokens even for in-review tasks", () => {
|
||||
const css = loadAllAppCss();
|
||||
const failedRule = css.match(/\.list-status-badge\.failed\s*\{([^}]*)\}/);
|
||||
const inReviewRuleIndex = css.indexOf(".list-status-badge--in-review");
|
||||
const failedRuleIndex = css.indexOf(".list-status-badge.failed");
|
||||
|
||||
expect(failedRule?.[1]).toContain("background: var(--status-error-bg)");
|
||||
expect(failedRuleIndex).toBeGreaterThan(inReviewRuleIndex);
|
||||
});
|
||||
|
||||
it("renders paused tasks with dimmed styling", () => {
|
||||
const tasks = [createMockTask({ id: "FN-001", paused: true })];
|
||||
|
||||
|
||||
@@ -1080,7 +1080,81 @@ describe("Scheduler", () => {
|
||||
expect(updateTask).not.toHaveBeenCalledWith("FN-002", { status: "queued", blockedBy: "FN-001" });
|
||||
});
|
||||
|
||||
it("blocks todo when overlapping in-review task is not paused", async () => {
|
||||
it("excludes permanently-failed in-review tasks from active scopes", async () => {
|
||||
vi.mocked(existsSync).mockReturnValue(true);
|
||||
vi.mocked(readFile).mockResolvedValue("# Task\nDo something");
|
||||
|
||||
const tasks = [
|
||||
createMockTask({ id: "FN-001", column: "in-review", status: "failed", worktree: "/test/project/.worktrees/fn-001" }),
|
||||
createMockTask({ id: "FN-002", column: "todo" }),
|
||||
];
|
||||
|
||||
const parseScopeMock = vi.fn(async (taskId: string): Promise<string[]> => {
|
||||
if (taskId === "FN-001") return ["src/foo.ts"];
|
||||
if (taskId === "FN-002") return ["src/foo.ts"];
|
||||
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: 2,
|
||||
maxWorktrees: 4,
|
||||
groupOverlappingFiles: true,
|
||||
}),
|
||||
parseFileScopeFromPrompt: parseScopeMock,
|
||||
updateTask,
|
||||
moveTask,
|
||||
});
|
||||
|
||||
const scheduler = new Scheduler(store);
|
||||
(scheduler as any).running = true;
|
||||
await scheduler.schedule();
|
||||
|
||||
expect(moveTask).toHaveBeenCalledWith("FN-002", "in-progress", expect.objectContaining({ allocateWorktree: expect.any(Function) }));
|
||||
expect(updateTask).not.toHaveBeenCalledWith("FN-002", { status: "queued", blockedBy: "FN-001" });
|
||||
});
|
||||
|
||||
it("clears stale blockedBy when prior overlap blocker is now permanently failed", async () => {
|
||||
vi.mocked(existsSync).mockReturnValue(true);
|
||||
vi.mocked(readFile).mockResolvedValue("# Task\nDo something");
|
||||
|
||||
const tasks = [
|
||||
createMockTask({ id: "FN-FAIL", column: "in-review", status: "failed", worktree: "/test/project/.worktrees/fn-fail" }),
|
||||
createMockTask({ id: "FN-002", column: "todo", status: "queued", blockedBy: "FN-FAIL" }),
|
||||
];
|
||||
|
||||
const parseScopeMock = vi.fn(async (taskId: string): Promise<string[]> => {
|
||||
if (taskId === "FN-FAIL") return ["src/foo.ts"];
|
||||
if (taskId === "FN-002") return ["src/foo.ts"];
|
||||
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: 2,
|
||||
maxWorktrees: 4,
|
||||
groupOverlappingFiles: true,
|
||||
}),
|
||||
parseFileScopeFromPrompt: parseScopeMock,
|
||||
updateTask,
|
||||
moveTask,
|
||||
});
|
||||
|
||||
const scheduler = new Scheduler(store);
|
||||
(scheduler as any).running = true;
|
||||
await scheduler.schedule();
|
||||
|
||||
expect(moveTask).toHaveBeenCalledWith("FN-002", "in-progress", expect.objectContaining({ allocateWorktree: expect.any(Function) }));
|
||||
expect(updateTask).not.toHaveBeenCalledWith("FN-002", { status: "queued", blockedBy: "FN-FAIL" });
|
||||
});
|
||||
|
||||
it("still blocks todo when overlapping in-review task is not paused and not failed", async () => {
|
||||
vi.mocked(existsSync).mockReturnValue(true);
|
||||
vi.mocked(readFile).mockResolvedValue("# Task\nDo something");
|
||||
|
||||
|
||||
@@ -733,12 +733,17 @@ export class Scheduler {
|
||||
const filteredScope = filterPathsByIgnoreList(scope, overlapIgnorePaths);
|
||||
if (filteredScope.length > 0) activeScopes.set(t.id, filteredScope);
|
||||
}
|
||||
// Only live in-review tasks with a worktree belong in activeScopes.
|
||||
// Paused in-review tasks (e.g., failed-merge tasks awaiting human triage) cannot
|
||||
// make progress, so they must not contribute to activeScopes. Including them
|
||||
// make progress, so they must not contribute to overlap blockers; including them
|
||||
// caused a deadlock pattern where a paused task indefinitely re-stamped
|
||||
// `blockedBy` on overlapping todo tasks every scheduler tick. (FN-3867 / FN-3857)
|
||||
// Permanently-failed in-review tasks from SelfHealingManager.checkStuckBudget()
|
||||
// also keep their worktree, but after the stuck-kill budget is exhausted they
|
||||
// will never merge, so superseding re-implementation tasks (for example FN-4177
|
||||
// replaced by FN-4198) must not stay queued behind them. (FN-4200)
|
||||
const inReviewWithWorktree = tasks.filter(
|
||||
(t) => t.column === "in-review" && t.worktree && !t.paused,
|
||||
(t) => t.column === "in-review" && Boolean(t.worktree) && !t.paused && t.status !== "failed",
|
||||
);
|
||||
for (const t of inReviewWithWorktree) {
|
||||
const scope = await this.store.parseFileScopeFromPrompt(t.id);
|
||||
|
||||
Reference in New Issue
Block a user