From 7cd660f5584d7228adef973c790d50f5675262e3 Mon Sep 17 00:00:00 2001 From: Phil Larson Date: Thu, 25 Jun 2026 05:08:04 -0700 Subject: [PATCH] fix(FN-783): address overlap repair review blockers --- .changeset/sharp-overlap-repair.md | 5 ++ .../core/src/__tests__/store-parsing.test.ts | 11 ++++ .../core/src/file-scope-classification.ts | 16 ++++-- packages/core/src/store.ts | 12 +++-- .../app/components/TaskDetailModal.tsx | 14 ++++- .../TaskDetailModal.rendering.test.tsx | 7 ++- .../src/__tests__/routes-tasks-ops.test.ts | 36 +++++++++++++ .../routes/register-task-workflow-routes.ts | 12 ++++- packages/engine/src/scheduler.ts | 51 +++++++++++++++---- packages/engine/src/self-healing.ts | 30 ++++++++--- 10 files changed, 166 insertions(+), 28 deletions(-) create mode 100644 .changeset/sharp-overlap-repair.md diff --git a/.changeset/sharp-overlap-repair.md b/.changeset/sharp-overlap-repair.md new file mode 100644 index 0000000000..134e0d1c6f --- /dev/null +++ b/.changeset/sharp-overlap-repair.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": patch +--- + +Fix stale overlap-blocker repair edge cases and dashboard display synchronization. diff --git a/packages/core/src/__tests__/store-parsing.test.ts b/packages/core/src/__tests__/store-parsing.test.ts index c260e32447..b0d911d608 100644 --- a/packages/core/src/__tests__/store-parsing.test.ts +++ b/packages/core/src/__tests__/store-parsing.test.ts @@ -539,6 +539,17 @@ Expected touched paths: expect(repaired?.log.at(-1)?.action).toContain(`Repaired stale overlap blocker: cleared ${blocker.id}`); }); + it("returns structured not-found result instead of throwing", async () => { + const result = await store.repairOverlapBlocker("FN-MISSING"); + + expect(result).toMatchObject({ + taskId: "FN-MISSING", + repaired: false, + statusCleared: false, + reason: "task-not-found", + }); + }); + it("rejects repair when the stored blocker still overlaps", async () => { const blocker = await store.createTask({ description: "Fusion blocker" }); const target = await store.createTask({ description: "Fusion target" }); diff --git a/packages/core/src/file-scope-classification.ts b/packages/core/src/file-scope-classification.ts index adf4f8fc7d..9054b082c7 100644 --- a/packages/core/src/file-scope-classification.ts +++ b/packages/core/src/file-scope-classification.ts @@ -22,6 +22,10 @@ export interface FileScopeClassificationResult { effectiveWriteScope: string[]; } +/* +FNXC:FileScopeClassification 2026-06-25-04:34: +Task File Scope is operator intent, not every path-like token in PROMPT.md. Keep this classifier conservative so read-only evidence, wrong-worktree safeguards, generated locks, route names, and conditional changesets do not create false write-scope leases or file-scope merge guards. +*/ const KNOWN_FILE_SCOPE_ROOT_FILES = new Set([ "makefile", "dockerfile", @@ -92,6 +96,10 @@ export function extractEffectiveWriteScopeFromPrompt(content: string): string[] } export function classifyFileScopeFromPrompt(content: string): FileScopeClassificationResult { + /* + FNXC:FileScopeClassification 2026-06-25-04:34: + Context headings inside `## File Scope` change the meaning of backticked tokens. The state machine is line-oriented on purpose: execution specs often mix write targets with forbidden paths and evidence-only metadata in the same section. + */ const section = extractFileScopeSection(content); if (!section) return { entries: [], effectiveWriteScope: [] }; @@ -103,13 +111,11 @@ export function classifyFileScopeFromPrompt(content: string): FileScopeClassific for (const rawLine of section.split("\n")) { const line = rawLine.trim(); if (!line) continue; - const lower = line.toLowerCase(); - if (INCLUDE_CONTEXT_RE.test(line) && !EXCLUDE_CONTEXT_RE.test(line) && !CONDITIONAL_CONTEXT_RE.test(line)) { context = "include"; } if (EXCLUDE_CONTEXT_RE.test(line)) { - context = lower.includes("wrong-worktree") || lower.includes("wrong worktree") ? "exclude" : "exclude"; + context = "exclude"; } if (CONDITIONAL_CONTEXT_RE.test(line)) { context = "conditional"; @@ -141,6 +147,10 @@ function classifyToken( line: string, context: "include" | "exclude" | "conditional", ): FileScopeClassificationReason { + /* + FNXC:FileScopeClassification 2026-06-25-04:34: + Classification reasons must be stable enough for diagnostics while the include/exclude decision stays binary. Preserve specific exclusion reasons after validation so review/spec gates explain why a token was ignored instead of silently shrinking File Scope. + */ if (ROUTE_OR_ACTION_RE.test(token)) return "route-or-action"; if (!isValidFileScopeEntry(token)) return "invalid-entry"; diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index 0fb099bad3..8a87dbaf50 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -10841,9 +10841,15 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS} } async repairOverlapBlocker(id: string, options: RepairOverlapBlockerOptions = {}): Promise { + /* + FNXC:OverlapRepair 2026-06-25-04:34: + Dashboard-initiated overlap repair is a narrow stale-blocker cleanup, not a general task mutation endpoint. Return structured reasons for missing tasks and cache empty scopes so route handlers can map failures predictably and repair decisions stay deterministic. + */ const dryRun = options.dryRun === true; - const task = await this.getTask(id); - if (!task) { + let task: Task; + try { + task = await this.getTask(id); + } catch { return { taskId: id, dryRun, repaired: false, statusCleared: false, reason: "task-not-found", message: `Task ${id} not found` }; } @@ -10886,7 +10892,7 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS} const scopeCache = new Map(); const getScope = async (taskId: string): Promise => { const cached = scopeCache.get(taskId); - if (cached) return cached; + if (cached !== undefined) return cached; const scope = filterRepairOverlapIgnoredPaths(await this.parseFileScopeFromPrompt(taskId), ignorePaths); scopeCache.set(taskId, scope); return scope; diff --git a/packages/dashboard/app/components/TaskDetailModal.tsx b/packages/dashboard/app/components/TaskDetailModal.tsx index 27992cb695..3319f42f75 100644 --- a/packages/dashboard/app/components/TaskDetailModal.tsx +++ b/packages/dashboard/app/components/TaskDetailModal.tsx @@ -678,6 +678,10 @@ export function TaskDetailContent({ // FN-4161: board/restart flows open the modal from slim task rows where // `githubTracking` is intentionally omitted; preserve the fetched full-detail // tracking blob instead of letting the sparse parent prop overwrite it. + const [overlapBlockedByOverride, setOverlapBlockedByOverride] = useState(undefined); + useEffect(() => { + setOverlapBlockedByOverride(undefined); + }, [task.id]); const workingTask: TaskDetail = fullDetail ? ({ ...fullDetail, @@ -692,7 +696,13 @@ export function TaskDetailContent({ paused: task.paused === undefined ? fullDetail.paused : task.paused, userPaused: task.userPaused === undefined ? fullDetail.userPaused : task.userPaused, pausedReason: task.pausedReason === undefined ? fullDetail.pausedReason : task.pausedReason, - overlapBlockedBy: task.overlapBlockedBy === undefined ? undefined : fullDetail.overlapBlockedBy, + /* + FNXC:TaskDetailOverlapRepair 2026-06-25-04:34: + SSE task props are authoritative for live blocker changes, but the Clear repair flow needs a local override while stale parent props catch up. Only fall back to fetched detail when the slim parent omitted the field entirely. + */ + overlapBlockedBy: overlapBlockedByOverride !== undefined + ? overlapBlockedByOverride + : task.overlapBlockedBy === undefined ? fullDetail.overlapBlockedBy : task.overlapBlockedBy, } as TaskDetail) : ({ ...task, prompt: "" } as TaskDetail); const canRetryTask = @@ -2474,6 +2484,7 @@ export function TaskDetailContent({ return; } if (result.task) { + setOverlapBlockedByOverride(result.task.overlapBlockedBy ?? null); setFullDetail((prev) => prev ? ({ ...prev, ...result.task } as TaskDetail) : (result.task as TaskDetail)); onTaskUpdated?.(result.task); } else { @@ -2481,6 +2492,7 @@ export function TaskDetailContent({ if (activeTaskIdRef.current !== requestTaskId) { return; } + setOverlapBlockedByOverride(updatedTask.overlapBlockedBy ?? null); setFullDetail((prev) => prev ? ({ ...prev, ...updatedTask } as TaskDetail) : updatedTask); onTaskUpdated?.(updatedTask); } diff --git a/packages/dashboard/app/components/__tests__/TaskDetailModal.rendering.test.tsx b/packages/dashboard/app/components/__tests__/TaskDetailModal.rendering.test.tsx index 742a0ff53f..048c63a0a7 100644 --- a/packages/dashboard/app/components/__tests__/TaskDetailModal.rendering.test.tsx +++ b/packages/dashboard/app/components/__tests__/TaskDetailModal.rendering.test.tsx @@ -513,7 +513,7 @@ describe("TaskDetailModal", () => { expect(screen.queryByText("File scope overlap blocker: FN-OVER (stale)")).toBeNull(); }); - it("renders clear overlap blocker button only when overlapBlockedBy is present", () => { + it("keeps clear overlap blocker button when slim live task omits overlapBlockedBy", () => { const { rerender } = render( { />, ); - expect(screen.queryByRole("button", { name: "Clear" })).toBeNull(); + expect(screen.getByRole("button", { name: "Clear" })).toBeInTheDocument(); }); it("repairs overlap blocker when clicking Clear", async () => { @@ -579,6 +579,9 @@ describe("TaskDetailModal", () => { undefined, ); }); + await waitFor(() => { + expect(screen.queryByRole("button", { name: "Clear" })).toBeNull(); + }); }); it("applies rerouted overlap blocker returned by repair API", async () => { diff --git a/packages/dashboard/src/__tests__/routes-tasks-ops.test.ts b/packages/dashboard/src/__tests__/routes-tasks-ops.test.ts index 1b8bfcc4a8..771840cc34 100644 --- a/packages/dashboard/src/__tests__/routes-tasks-ops.test.ts +++ b/packages/dashboard/src/__tests__/routes-tasks-ops.test.ts @@ -2371,6 +2371,42 @@ describe("PATCH /tasks/:id", () => { expect(store.updateTask).not.toHaveBeenCalled(); }); + it("rejects overlap blocker repair when request body is not an object", async () => { + const res = await REQUEST( + buildApp(), + "POST", + "/api/tasks/KB-001/repair-overlap-blocker", + JSON.stringify(["not", "an", "object"]), + { "Content-Type": "application/json" }, + ); + + expect(res.status).toBe(400); + expect(res.body.error).toBe("body must be an object"); + expect(store.repairOverlapBlocker).not.toHaveBeenCalled(); + }); + + it("maps missing overlap repair target to 404", async () => { + (store.repairOverlapBlocker as ReturnType).mockResolvedValue({ + taskId: "MISSING", + dryRun: false, + repaired: false, + statusCleared: false, + reason: "task-not-found", + message: "Task MISSING not found", + }); + + const res = await REQUEST( + buildApp(), + "POST", + "/api/tasks/MISSING/repair-overlap-blocker", + JSON.stringify({}), + { "Content-Type": "application/json" }, + ); + + expect(res.status).toBe(404); + expect(res.body.error).toBe("Task MISSING not found"); + }); + it("rejects overlap blocker repair when scopes still overlap", async () => { (store.repairOverlapBlocker as ReturnType).mockResolvedValue({ taskId: "KB-001", diff --git a/packages/dashboard/src/routes/register-task-workflow-routes.ts b/packages/dashboard/src/routes/register-task-workflow-routes.ts index 5c1b11b9d5..dca248835a 100644 --- a/packages/dashboard/src/routes/register-task-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-task-workflow-routes.ts @@ -3023,7 +3023,15 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork router.post("/tasks/:id/repair-overlap-blocker", async (req, res) => { try { const { store: scopedStore } = await getProjectContext(req); - const { dryRun, reason } = req.body ?? {}; + /* + FNXC:OverlapRepair 2026-06-25-04:34: + This route can clear scheduler-visible blockers. Reject non-object JSON bodies before calling the store so malformed requests cannot accidentally run a real repair with every option undefined. + */ + const body = req.body ?? {}; + if (typeof body !== "object" || body === null || Array.isArray(body)) { + throw badRequest("body must be an object"); + } + const { dryRun, reason } = body as { dryRun?: unknown; reason?: unknown }; if (dryRun !== undefined && typeof dryRun !== "boolean") { throw badRequest("dryRun must be a boolean"); } @@ -3040,7 +3048,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (result.reason === "task-not-found") { throw notFound(result.message); } - if (!result.repaired && !result.dryRun && result.reason !== "repaired") { + if (!result.repaired && !result.dryRun) { const status = result.reason === "no-overlap-blocker" ? 400 : 409; throw new ApiError(status, result.message); } diff --git a/packages/engine/src/scheduler.ts b/packages/engine/src/scheduler.ts index d1279b8b9f..73aa405159 100644 --- a/packages/engine/src/scheduler.ts +++ b/packages/engine/src/scheduler.ts @@ -265,6 +265,29 @@ export function isRunnableQueuedOverlapCandidate( return true; } +export function shouldHoldActiveFileScopeLease( + task: Task, + tasks: Task[], + options?: { + mergeRequestContractShadowEnabled?: boolean; + handoffAccepted?: boolean; + schedulingDependencyOptions?: Parameters[2]; + }, +): boolean { + /* + FNXC:OverlapScheduling 2026-06-25-04:34: + Active file-scope leases are a scheduler contract, not just a column check. Self-healing and repair paths must use this same predicate so stale `overlapBlockedBy` cleanup does not preserve blockers the scheduler would ignore on the next tick. + */ + if (task.paused || task.userPaused) return false; + if (task.column === "in-progress") { + return getUnmetSchedulingDependencies(task, tasks, options?.schedulingDependencyOptions).length === 0; + } + if (task.column !== "in-review") return false; + if (!task.worktree || task.status === "failed") return false; + if (options?.mergeRequestContractShadowEnabled === true && options.handoffAccepted === true) return false; + return true; +} + export function findHigherPriorityQueuedOverlap( candidate: QueuedOverlapCandidate, queuedScopes: QueuedOverlapCandidate[], @@ -1446,7 +1469,7 @@ export class Scheduler { const filteredScopeByTaskId = new Map(); const getFilteredFileScope = async (taskId: string): Promise => { const cached = filteredScopeByTaskId.get(taskId); - if (cached) return cached; + if (cached !== undefined) return cached; const scope = await this.store.parseFileScopeFromPrompt(taskId); const filteredScope = filterPathsByIgnoreList(scope, overlapIgnorePaths); filteredScopeByTaskId.set(taskId, filteredScope); @@ -1455,12 +1478,10 @@ export class Scheduler { if (settings.groupOverlappingFiles) { // In-progress tasks for (const t of inProgress) { + if (!shouldHoldActiveFileScopeLease(t, tasks, { schedulingDependencyOptions })) continue; const filteredScope = await getFilteredFileScope(t.id); if (isCoordinationOnlyTask(t, filteredScope)) continue; if (filteredScope.length === 0) continue; - // FN-6292: a holder waiting on scheduling deps must not lease files - // that can block its own dependency and create a circular wait. - if (getUnmetSchedulingDependencies(t, tasks, schedulingDependencyOptions).length > 0) continue; setActiveScopeLease(t.id, filteredScope, "in-progress"); } // Only live in-review tasks with a worktree belong in activeScopes. @@ -1473,7 +1494,13 @@ export class Scheduler { // 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" && Boolean(t.worktree) && !t.paused && t.status !== "failed", + (t) => t.column === "in-review" && shouldHoldActiveFileScopeLease(t, tasks, { + mergeRequestContractShadowEnabled: settings.mergeRequestContractShadowEnabled, + handoffAccepted: settings.mergeRequestContractShadowEnabled === true + ? this.store.getCompletionHandoffAcceptedMarker(t.id) !== null + : false, + schedulingDependencyOptions, + }), ); for (const t of inReviewWithWorktree) { const filteredScope = await getFilteredFileScope(t.id); @@ -2118,7 +2145,7 @@ export class Scheduler { const filteredScopeByTaskId = new Map(); const getFilteredFileScope = async (taskId: string): Promise => { const cached = filteredScopeByTaskId.get(taskId); - if (cached) return cached; + if (cached !== undefined) return cached; const scope = await this.store.parseFileScopeFromPrompt(taskId); const filteredScope = filterPathsByIgnoreList(scope, overlapIgnorePaths); filteredScopeByTaskId.set(taskId, filteredScope); @@ -2145,18 +2172,22 @@ export class Scheduler { if (settings.groupOverlappingFiles) { for (const task of tasks) { if (task.column !== "in-progress") continue; + if (!shouldHoldActiveFileScopeLease(task, tasks, { schedulingDependencyOptions })) continue; const filteredScope = await getFilteredFileScope(task.id); if (isCoordinationOnlyTask(task, filteredScope)) continue; if (filteredScope.length === 0) continue; - // FN-6292: do not let a task with unmet deps lease files that can - // keep those deps queued behind their own dependent. - if (getUnmetSchedulingDependencies(task, tasks, schedulingDependencyOptions).length > 0) continue; activeScopes.set(task.id, filteredScope); activeScopeColumns.set(task.id, task.column); } const inReviewWithWorktree = tasks.filter( - (task) => task.column === "in-review" && Boolean(task.worktree) && !task.paused && task.status !== "failed", + (task) => task.column === "in-review" && shouldHoldActiveFileScopeLease(task, tasks, { + mergeRequestContractShadowEnabled: settings.mergeRequestContractShadowEnabled, + handoffAccepted: settings.mergeRequestContractShadowEnabled === true + ? this.store.getCompletionHandoffAcceptedMarker(task.id) !== null + : false, + schedulingDependencyOptions, + }), ); for (const task of inReviewWithWorktree) { const filteredScope = await getFilteredFileScope(task.id); diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index 217fe8ec70..69a36f106d 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -74,7 +74,7 @@ import { } from "./notifier.js"; import type { GhostBugDecision } from "./triage-preflight.js"; import { DependencyBlockedTodoReporter } from "./dependency-blocked-todo-reporter.js"; -import { filterPathsByIgnoreList, getUnmetSchedulingDependencies, isCoordinationOnlyTask, pathsOverlap } from "./scheduler.js"; +import { filterPathsByIgnoreList, getUnmetSchedulingDependencies, isCoordinationOnlyTask, pathsOverlap, shouldHoldActiveFileScopeLease } from "./scheduler.js"; import { evaluateParkedAgentTaskLink, PARKED_AGENT_LINK_FRESH_RUN_MS } from "./task-agent-sync.js"; const log = createLogger("self-healing"); @@ -3653,9 +3653,13 @@ export class SelfHealingManager { const taskById = new Map(allTasks.map((t) => [t.id, t])); const overlapIgnorePaths = settings.overlapIgnorePaths ?? []; const filteredScopeByTaskId = new Map(); + /* + FNXC:OverlapSelfHealing 2026-06-25-04:34: + Completion fan-out may preserve queued overlap blockers only when the blocker still holds the scheduler's active file-scope lease. Cache empty filtered scopes too so coordination-only tasks stay deterministic within a reconciliation pass. + */ const getFilteredFileScope = async (scopeTaskId: string): Promise => { const cached = filteredScopeByTaskId.get(scopeTaskId); - if (cached) return cached; + if (cached !== undefined) return cached; const scope = await this.store.parseFileScopeFromPrompt(scopeTaskId); const filteredScope = filterPathsByIgnoreList(scope, overlapIgnorePaths); filteredScopeByTaskId.set(scopeTaskId, filteredScope); @@ -3664,8 +3668,12 @@ export class SelfHealingManager { const hasActiveFileScopeOverlapBlocker = async (dependent: Task, blockerId: string | null | undefined): Promise => { if (!blockerId) return false; const blocker = taskById.get(blockerId); - if (!blocker || blocker.paused || blocker.userPaused) return false; - if (blocker.column !== "in-progress" && !(blocker.column === "in-review" && !blocker.paused)) return false; + if (!blocker || !shouldHoldActiveFileScopeLease(blocker, allTasks, { + mergeRequestContractShadowEnabled: settings.mergeRequestContractShadowEnabled, + handoffAccepted: settings.mergeRequestContractShadowEnabled === true + ? this.store.getCompletionHandoffAcceptedMarker(blocker.id) !== null + : false, + })) return false; const dependentScope = await getFilteredFileScope(dependent.id); if (dependentScope.length === 0 || isCoordinationOnlyTask(dependent, dependentScope)) return false; const blockerScope = await getFilteredFileScope(blocker.id); @@ -4767,9 +4775,13 @@ export class SelfHealingManager { const taskById = new Map(allTasks.map((task) => [task.id, task])); const overlapIgnorePaths = settings.overlapIgnorePaths ?? []; const filteredScopeByTaskId = new Map(); + /* + FNXC:OverlapSelfHealing 2026-06-25-04:34: + Stale blockedBy cleanup must mirror scheduler lease semantics before preserving queued overlap state. Empty-scope cache hits matter here because no-write-scope advisory tasks should not repeatedly reparse specs or look active by accident. + */ const getFilteredFileScope = async (taskId: string): Promise => { const cached = filteredScopeByTaskId.get(taskId); - if (cached) return cached; + if (cached !== undefined) return cached; const scope = await this.store.parseFileScopeFromPrompt(taskId); const filteredScope = filterPathsByIgnoreList(scope, overlapIgnorePaths); filteredScopeByTaskId.set(taskId, filteredScope); @@ -4778,8 +4790,12 @@ export class SelfHealingManager { const hasActiveFileScopeOverlapBlocker = async (task: Task, blockerId: string | null | undefined): Promise => { if (!blockerId) return false; const blocker = taskById.get(blockerId); - if (!blocker || blocker.paused || blocker.userPaused) return false; - if (blocker.column !== "in-progress" && !(blocker.column === "in-review" && !blocker.paused)) return false; + if (!blocker || !shouldHoldActiveFileScopeLease(blocker, allTasks, { + mergeRequestContractShadowEnabled: settings.mergeRequestContractShadowEnabled, + handoffAccepted: settings.mergeRequestContractShadowEnabled === true + ? this.store.getCompletionHandoffAcceptedMarker(blocker.id) !== null + : false, + })) return false; const taskScope = await getFilteredFileScope(task.id); if (taskScope.length === 0 || isCoordinationOnlyTask(task, taskScope)) return false;