From 6dcecb0c343d13d306bf7e2c68255dbb63d73848 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 12 Jul 2026 23:26:02 -0700 Subject: [PATCH] FN-7926: park completed-but-blocked tasks instead of looping execute-requeue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stops the execute → pause-abort → re-queue-to-todo infinite loop for tasks whose implementation work is done but a dependency/blockedBy blocker is still live, by diverting them into a dedicated parked state instead of feeding the FN-7863 no-progress backstop or looping forever. - Add TaskExecutor.parkCompletedBlockedTask(): when work is complete but getTaskCompletionBlocker() still reports a blocker, park the task in todo with pausedReason:"completed-work-blocked", status:"queued", preserved worktree/branch/steps, and a cleared execute-requeue signature. - Replace shouldFinalizeCompletedTask's boolean with getCompletedTaskFinalizationDecision() returning "finalize" | "blocked" | "incomplete" so both the paused-after-completion and finalization call sites can react to the new "blocked" outcome without re-entering execution. - Divert completed-but-blocked tasks before the FN-7863 execute-requeue-loop counter increments, so waiting-on-dependency states are no longer misclassified as EXECUTION_DISPATCH_LOOP_EXHAUSTED. - Add SelfHealingManager.reconcileCompletedBlockedTasks(): a bounded sweep (wired into both startup/maintenance and periodic self-healing passes) that clears the park and advances the task to review once getTaskCompletionBlockerForStore() resolves, guarded by auto-merge eligibility, user-pause, and live-execution checks; failed advances re-park rather than strand the row. - Add run-audit mutation types task:completed-blocked-parked and task:completed-blocked-advanced (ids/counts/outcomes-only metadata) plus AGENTS.md/docs/architecture.md entries documenting the new lifecycle. - Extend execute-requeue-loop-guard.test.ts with coverage for the park/advance flow, including the zero-step task edge case. Files changed: AGENTS.md | 1 + docs/architecture.md | 2 + .../execute-requeue-loop-guard.test.ts | 256 ++++++++++++++++++++- packages/engine/src/executor.ts | 85 ++++++- packages/engine/src/run-audit.ts | 4 + packages/engine/src/self-healing.ts | 95 ++++++++ 6 files changed, 432 insertions(+), 11 deletions(-) Fusion-Task-Id: FN-7926 Fusion-Task-Lineage: e47945f4-a816-447e-9ea1-7c13105d0ba9 Co-authored-by: Fusion (runfusion.ai) --- AGENTS.md | 1 + docs/architecture.md | 2 + .../execute-requeue-loop-guard.test.ts | 256 +++++++++++++++++- packages/engine/src/executor.ts | 85 +++++- packages/engine/src/run-audit.ts | 4 + packages/engine/src/self-healing.ts | 95 +++++++ 6 files changed, 432 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b9284a2226..b9df1cf183 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -229,6 +229,7 @@ Scoped exception (FN-5819): shared-branch-group members (`branchContext.assignme - FN-7884: self-healing startup recovery emits `agent:reset-error-state-on-startup` when an engine restart clears an eligible durable-agent `error` or `pauseReason:"error-retry-exhausted"` park, resets shared `heartbeatErrorRecovery` plus legacy `durableErrorRecovery` budget/cooldown metadata, clears `lastError`/exhaustion pause state, and re-arms the heartbeat. Metadata stays ids/counts/outcomes-only (`agentId`, `priorState`, optional `priorPauseReason`, `source`). This startup-only path bypasses steady-state staleness/cooldown/exhaustion gates while preserving operator-actionable, stale-module, user-paused, `error-unrecoverable`, ephemeral, disabled-runtime, and active-execution suppression. - FN-7802: self-healing emits `task:reconcile-missing-worktree-merge-active` when it proves an `in-review` merge-active task (`merging`/`merging-pr`/`merging-fix`) is stranded by an unusable-worktree session-start failure, clears stale `worktree`/`branch`/`sessionFile`, resets the worktree-session retry budget, increments `recoveryRetryCount` as the bounded stale-metadata clear counter, and requeues to `todo`; it emits `task:reconcile-missing-worktree-merge-active-no-action` when `autoMerge:false`, workspace-task ownership, or triple-proof blocks the backward move. - FN-7863: executor emits `task:execution-dispatch-loop-terminalized` when an execute-node self-requeue loop reaches `MAX_EXECUTE_REQUEUE_LOOP_CYCLES` with an unchanged progress signature; metadata stays ids/counts/outcomes-only (`taskId`, `cycleCount`, `maxCycles`, `progressSignature`, `failureValue`) and the task is visibly failed with `EXECUTION_DISPATCH_LOOP_EXHAUSTED:` while preserving worktree/branch/step progress. +- FN-7926: executor emits `task:completed-blocked-parked` when completed implementation work is held by a live `getTaskCompletionBlocker()` reason instead of re-entering the execute self-requeue loop; self-healing emits `task:completed-blocked-advanced` when the blocker clears and the parked work advances to review. Metadata stays ids/outcomes-only (`taskId`, blocker/source/prior column/status). - FN-7011: self-healing emits `task:reconcile-engine-downtime-active-timing` when startup recovery shifts active task segment anchors to exclude proven engine-process downtime, and `task:reconcile-engine-downtime-active-timing-no-action` when no active task qualifies. - FN-5419: git run-audit now includes `pull:fast-forward` and `stash:pop-conflict`; dashboard git surfaces now include the extended `POST /api/git/pull` integration-worktree path plus companion `POST /api/git/stash-resolve`, `POST /api/git/stash-drop`, and `POST /api/git/stash-apply` routes. - FN-6292: self-healing emits `task:reconcile-dependency-blocking-lease` when it rebounds an in-progress holder whose stale file-scope lease blocks an unmet dependency, and `task:reconcile-dependency-blocking-lease-no-action` when triple-proof blocks that backward move. diff --git a/docs/architecture.md b/docs/architecture.md index f77c96f3e2..505aef42ca 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -688,6 +688,7 @@ Runtime action-gate flow (v1): - `recoverPausedAbortFailures()` clears executor pause/resume abort parks only when the durable row is safe to recover. `todo`/`in-progress` rows are requeued for normal scheduling, while clean `in-review` rows (completed steps, not paused/user-paused/executing, auto-merge eligible, no confirmed or terminal merge evidence) have `status`/`error` cleared in place so review progression can continue. FN-7749 adds the manual-hold exception to the prior `autoMerge:false` guard: a benign hard-cancel pause/resume abort at a merge-region/manual-hold node is the healthy Merge & Close resting state, so already-parked rows of that exact shape are cleared in place without moving backward (FN-5147-compliant). User hard-cancel, global/user pause, terminal merge, live-execution, and other `autoMerge:false` guards remain operator-actionable. Successful recovery emits `task:auto-recover-paused-abort-park` with `preservedInReview` metadata. - Workflow graph pause/resume is node-reentrant for typed engine-internal interruptions. When `WorkflowGraphExecutor` sees the graph abort signal or a node returns `value: "aborted"`, it stamps the interrupted node and `engine-pause` abort kind into graph context. `TaskExecutor` then uses the existing bounded `graphResumeRetryCount` budget to clear the transient abort, suppress failure notification with an `Auto-recovered:` task log, and re-enter the graph/task only under the same safety guards: no user/active global pause, no merge/finalize provenance, no genuine node failure, no terminal merge value, no `autoMerge:false` protected review row, and no active execution owner. Global-pause provenance from the graph-controller abort is re-entrant once the global pause has been lifted because it represents the same in-flight node interruption. Generic legacy pause-abort parks without the typed node marker remain operator-action failures except for the narrow `in-review`/`plan` stale-replay shape: hard-cancel pause provenance, `node:plan:value === "aborted"`, no typed interrupted node, no active task/user/global pause, no terminal merge value, no confirmed merge, auto-merge eligibility, and only a clean row or the exact stale plan pause-abort failure. That path logs `stale replay ignored`, clears only the stale failure state when present, preserves `in-review`, and never re-enters planning or moves the task to `todo`. - FN-7863 adds a progress-anchored guard at the execute-node self-requeue funnel (`failedNode === "execute"` and either the live row is `todo` or the in-process self-requeue marker proves a stale `in-progress` read). The guard persists `executeRequeueLoopCount` plus `executeRequeueLoopSignature` (`currentStep` + step statuses), warns at `EXECUTE_REQUEUE_LOOP_VISIBLE_THRESHOLD = 3`, and terminalizes at `MAX_EXECUTE_REQUEUE_LOOP_CYCLES = 6` with `status:"failed"` and an `EXECUTION_DISPATCH_LOOP_EXHAUSTED:` error. It preserves worktree/branch/step progress, skips paused/user-paused/done/archived terminalization, resets on signature progress, manual retry, successful completion, forward moves, and unpause, and complements rather than replaces the scheduler's wall-clock `dispatchStormCount` guard (fast flapping stays scheduler-owned; slow no-progress flapping is caught here). + - FN-7926 keeps completed-but-blocked work out of that FN-7863 backstop. When `taskDone === true` or all implementation steps are `done`/`skipped` but `getTaskCompletionBlocker()` still reports a live `blockedBy` or unresolved dependency, the executor parks the task in `todo` with `pausedReason:"completed-work-blocked"`, `status:"queued"`, and a task-log entry containing the exact blocker reason. The park preserves worktree/branch/step progress and clears the execute-requeue signature so completed work cannot accumulate toward `EXECUTION_DISPATCH_LOOP_EXHAUSTED:`. `reconcileCompletedBlockedTasks()` runs in self-healing; once the blocker clears, and only when auto-merge processing is allowed and no live execution/user pause owns the row, it clears the park and calls the completed-task recovery handoff so the work advances to `in-review` without re-running implementation. - `reattach-orphaned-assigned-executions` is a forward-resume safety net for durable-agent assignments. During startup recovery and periodic maintenance, after orphaned-agent and stale-heartbeat-run repairs, self-healing finds `in-progress` tasks with an `assignedAgentId` whose agent has no active heartbeat run and no active executor session after the orphan grace window. It re-dispatches in place via `executor.resumeTaskForAgent(agentId)` (the same seam used by clean `HeartbeatMonitor.onRunCompleted` and guarded by executor double-execution checks), emits `task:reattach-orphaned-execution`, and never moves the task backward. This complements engine-start `executor.resumeOrphaned()` and leaves unassigned/role-based execution recovery to the existing startup/limbo/stuck-task paths. - Durable `Agent.taskId` is a running assignment for parked `todo`/`triage` task rows only when the agent has live proof: a fresh active heartbeat run or an executor-active/tracked heartbeat signal. Scheduler overlap requeues, task move sync, self-healing, and Reports Health Check share this invariant: stale durable links are cleared or rendered as stale while `status: "queued"` and `overlapBlockedBy` remain on the task row so file-scope lease blocking is not weakened. `fn_list_agents` and `fn_agent_show` render the linked task column next to `Current Task` (for example `Current Task: FN-1234 (triage)` or `Current Task: FN-1234 (not active — done)`) so parked-column planning ownership is not misread as in-progress execution drift. - Mission validation has a dedicated stale-run reaper: startup recovery and Batch 2 maintenance call `reapStaleMissionValidatorRuns()` when wired by the runtime, using `VALIDATOR_RUN_STALE_MAX_AGE_MS` (currently 6 hours). The sweep terminates ownerless `mission_validator_runs.status='running'` rows as `error`, writes the reap reason into `summary`, leaves `lastValidatorRunId` pointing at the now-terminal run, and emits run-audit telemetry with `mutationType: "mission:validator-run-reaped"` plus `runId`/`featureId`/`missionId`/`triggerType`/`elapsedMs` metadata. Active mission features move to `loopState="needs_fix"` + `lastValidatorStatus="error"` unless their parent mission is already `complete`/`archived`. @@ -751,6 +752,7 @@ Guardrails: this routine does **not** retry merges, does **not** apply to mixed/ - FN-7069: `task:reconcile-phantom-committed-reservation` records task-store startup or self-healing cleanup of committed-reservation-without-task phantoms. Metadata includes `reservationStatus: "committed"` plus pruned `activityLog` and `agents` counts; `runAuditEvents` and the committed reservation are intentionally retained for auditability and ID permanence. - FN-7074: `task:reservation-commit-rolled-back` records preventive create-path rollback when a distributed reservation was committed with the task-row insert but a later create materialization step failed. Metadata includes `{ reservationId, nodeId, reason: "failed-create", error }`; the task row/partial directory are removed and the reservation is moved to `aborted` so FN-7069 should not need to clean up a new phantom. - FN-7863: `task:execution-dispatch-loop-terminalized` records executor terminalization of a no-progress execute-node self-requeue loop. Metadata includes `{ taskId, cycleCount, maxCycles, progressSignature, failureValue }`; the task row carries `EXECUTION_DISPATCH_LOOP_EXHAUSTED:` and remains in its visible failed state with committed/step progress preserved. + - FN-7926: `task:completed-blocked-parked` records executor diversion of completed work whose `getTaskCompletionBlocker()` is still live, and `task:completed-blocked-advanced` records self-healing advancement after that blocker clears. Metadata stays ids/counts/outcomes-only (`taskId`, blocker/source/prior column/status) so operators can distinguish dependency waits from real execute-loop exhaustion. - FN-4956: Layer 3 merge-conflict arbitration now scope-partitions conflicted files before AI resolution. Out-of-scope conflicts are deterministically resolved to the integration branch (`git checkout --ours`) and unstaged, while only in-scope conflicts flow to AI. Integration branch defaults are resolved via `resolveIntegrationBranch(rootDir, settings)`. Audit events: `merge:layer3:foreign-file-skipped` and `merge:layer3:scope-override-bypass`. - FN-5655 goal anchoring observability adds `database`-domain mutation types `goal:injection-applied`, `goal:injection-skipped`, and `goal:retrieval-invoked` so Slice 2 cite-rate tracking has a prompt-independent signal. Metadata uses counts/IDs only (`count`, `lane`, `toolName`, optional `truncated`/`reason`/`notFound`) and never stores prompt bodies or goal titles/descriptions. These events surface through `GET /api/agents/:id/runs/:runId/audit` and support the existing `startTime`/`endTime` filters. diff --git a/packages/engine/src/__tests__/reliability-interactions/execute-requeue-loop-guard.test.ts b/packages/engine/src/__tests__/reliability-interactions/execute-requeue-loop-guard.test.ts index 2a6524504a..3003bab9cf 100644 --- a/packages/engine/src/__tests__/reliability-interactions/execute-requeue-loop-guard.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/execute-requeue-loop-guard.test.ts @@ -6,8 +6,11 @@ import { MAX_EXECUTE_REQUEUE_LOOP_CYCLES, TaskExecutor, } from "../../executor.js"; +import { SelfHealingManager } from "../../self-healing.js"; import { createMockStore, resetExecutorMocks } from "../executor-test-helpers.js"; +const COMPLETED_BLOCKED_PAUSE_REASON = "completed-work-blocked"; + const now = "2026-07-12T00:00:00.000Z"; function task(overrides: Partial = {}): TaskDetail { @@ -35,15 +38,39 @@ function task(overrides: Partial = {}): TaskDetail { } as TaskDetail; } -function harness(initial: TaskDetail) { +function harness(initial: TaskDetail, relatedTasks: TaskDetail[] = []) { resetExecutorMocks(); const store = createMockStore(); let live = { ...initial } as TaskDetail; - store.getTask.mockImplementation(async () => live); - store.updateTask.mockImplementation(async (_id: string, updates: Partial) => { - live = { ...live, ...updates } as TaskDetail; + const related = new Map(relatedTasks.map((candidate) => [candidate.id, candidate])); + store.getTask.mockImplementation(async (id: string) => { + if (id === live.id) return live; + const found = related.get(id); + if (!found) throw new Error(`missing task ${id}`); + return found; + }); + store.updateTask.mockImplementation(async (id: string, updates: Partial) => { + if (id === live.id) { + live = { ...live, ...updates } as TaskDetail; + return live; + } + const found = related.get(id); + if (!found) throw new Error(`missing task ${id}`); + const updated = { ...found, ...updates } as TaskDetail; + related.set(id, updated); + return updated; + }); + store.moveTask.mockImplementation(async (id: string, column: TaskDetail["column"], options?: { preservePause?: boolean }) => { + if (id !== live.id) throw new Error(`unexpected move ${id}`); + live = { + ...live, + column, + ...(options?.preservePause ? {} : { paused: false, pausedReason: undefined }), + } as TaskDetail; return live; }); + store.listTasks.mockImplementation(async () => [live, ...Array.from(related.values())]); + store.getSettings.mockResolvedValue({ autoMerge: true, globalPause: false, enginePaused: false }); store.recordRunAuditEvent = vi.fn().mockResolvedValue(undefined); const executor = new TaskExecutor(store, "/tmp/test"); return { @@ -55,6 +82,11 @@ function harness(initial: TaskDetail) { setLive(patch: Partial) { live = { ...live, ...patch } as TaskDetail; }, + setRelated(id: string, patch: Partial) { + const found = related.get(id); + if (!found) throw new Error(`missing related task ${id}`); + related.set(id, { ...found, ...patch } as TaskDetail); + }, }; } @@ -185,4 +217,220 @@ describe("execute requeue loop guard", () => { undefined, ); }); + + it.each([ + ["blockedBy", { blockedBy: "FN-BLOCKER", dependencies: [] }, "task is blocked by FN-BLOCKER"], + ["dependencies", { blockedBy: null, dependencies: ["FN-BLOCKER"] }, "task has unresolved dependencies: FN-BLOCKER"], + ])("parks completed-but-blocked tasks before the %s execute requeue loop can terminalize", async (_label, patch, blockerReason) => { + const h = harness( + task({ + id: "FN-7926-PARK", + column: "todo", + steps: [ + { name: "Preflight", status: "done" }, + { name: "Implement", status: "skipped" }, + ], + ...patch, + }), + [task({ id: "FN-BLOCKER", column: "todo" })], + ); + + await failAtExecute(h.executor, h.live); + + expect(h.live).toMatchObject({ + column: "todo", + paused: true, + pausedReason: COMPLETED_BLOCKED_PAUSE_REASON, + status: "queued", + error: null, + executeRequeueLoopCount: null, + executeRequeueLoopSignature: null, + }); + + for (let i = 0; i < MAX_EXECUTE_REQUEUE_LOOP_CYCLES + 1; i += 1) { + await failAtExecute(h.executor, h.live); + } + expect(h.store.logEntry).toHaveBeenCalledWith( + "FN-7926-PARK", + expect.stringContaining(`Completed work held — ${blockerReason}; will advance to review when blocker clears`), + undefined, + undefined, + ); + expect(h.store.updateTask).not.toHaveBeenCalledWith( + "FN-7926-PARK", + expect.objectContaining({ status: "failed", error: expect.stringMatching(/^EXECUTION_DISPATCH_LOOP_EXHAUSTED:/) }), + expect.anything(), + ); + expect(h.store.recordRunAuditEvent).not.toHaveBeenCalledWith(expect.objectContaining({ + mutationType: "task:execution-dispatch-loop-terminalized", + })); + }); + + it("parks explicit taskDone completion even when the task has zero planned steps", async () => { + const h = harness( + task({ + id: "FN-7926-TASKDONE", + column: "in-progress", + blockedBy: "FN-BLOCKER", + steps: [], + }), + [task({ id: "FN-BLOCKER", column: "todo" })], + ); + + const shouldFinalize = await (h.executor as any).shouldFinalizeCompletedTask("FN-7926-TASKDONE", true); + + expect(shouldFinalize).toBe(false); + expect(h.live).toMatchObject({ + column: "todo", + paused: true, + pausedReason: COMPLETED_BLOCKED_PAUSE_REASON, + status: "queued", + }); + }); + + it("parks the stale in-progress self-requeue marker path when completed work is blocked", async () => { + const h = harness( + task({ + id: "FN-7926-STALE", + column: "in-progress", + blockedBy: "FN-BLOCKER", + steps: [{ name: "Implement", status: "done" }], + }), + [task({ id: "FN-BLOCKER", column: "todo" })], + ); + (h.executor as any).graphRouting.add("FN-7926-STALE"); + (h.executor as any).markGraphExecuteSelfRequeued("FN-7926-STALE"); + + await failAtExecute(h.executor, h.live); + + expect(h.live).toMatchObject({ + column: "todo", + paused: true, + pausedReason: COMPLETED_BLOCKED_PAUSE_REASON, + status: "queued", + }); + expect(h.store.moveTask).toHaveBeenCalledWith("FN-7926-STALE", "todo", expect.objectContaining({ + preserveProgress: true, + preserveResumeState: true, + preserveWorktree: true, + })); + }); + + it.each([ + ["paused", { paused: true }], + ["userPaused", { userPaused: true }], + ["zero-step", { steps: [] }], + ])("does not completed-block park %s tasks", async (_label, patch) => { + const h = harness( + task({ + id: "FN-7926-GUARD", + column: "todo", + blockedBy: "FN-BLOCKER", + steps: [{ name: "Implement", status: "done" }], + ...patch, + }), + [task({ id: "FN-BLOCKER", column: "todo" })], + ); + + await failAtExecute(h.executor, h.live); + + expect(h.live.pausedReason).not.toBe(COMPLETED_BLOCKED_PAUSE_REASON); + expect(h.store.logEntry).not.toHaveBeenCalledWith( + "FN-7926-GUARD", + expect.stringContaining("Completed work held"), + expect.anything(), + expect.anything(), + ); + }); + + it("auto-advances a completed-blocked park to review when the blocker clears", async () => { + const h = harness( + task({ + id: "FN-7926-ADVANCE", + column: "todo", + blockedBy: "FN-BLOCKER", + paused: true, + pausedReason: COMPLETED_BLOCKED_PAUSE_REASON, + status: "queued", + steps: [{ name: "Implement", status: "done" }], + }), + [task({ id: "FN-BLOCKER", column: "todo" })], + ); + const recoverCompletedTask = vi.fn(async (completed: TaskDetail) => { + h.setLive({ + column: "in-review", + paused: false, + pausedReason: undefined, + status: null, + error: null, + blockedBy: null, + }); + return completed.id === "FN-7926-ADVANCE"; + }); + const healer = new SelfHealingManager(h.store, { + rootDir: "/tmp/test", + recoverCompletedTask: recoverCompletedTask as any, + getExecutingTaskIds: () => new Set(), + isTaskActive: () => false, + }); + + await (healer as any).reconcileCompletedBlockedTasks(); + expect(recoverCompletedTask).not.toHaveBeenCalled(); + + h.setRelated("FN-BLOCKER", { column: "done" }); + await (healer as any).reconcileCompletedBlockedTasks(); + + expect(recoverCompletedTask).toHaveBeenCalledWith(expect.objectContaining({ id: "FN-7926-ADVANCE" })); + expect(h.live).toMatchObject({ column: "in-review", paused: false, status: null, blockedBy: null }); + expect(h.store.logEntry).toHaveBeenCalledWith( + "FN-7926-ADVANCE", + expect.stringContaining("Auto-advanced completed blocked work to review after blocker cleared"), + ); + }); + + it("auto-advances a zero-step taskDone completed-blocked park once the blocker clears (invariant: park and advance must agree on workComplete)", async () => { + // Regression for the FN-7926 park/advance asymmetry: parkCompletedBlockedTask() + // accepts workComplete=taskDone for a task with zero planned steps (see the + // "parks explicit taskDone completion even when the task has zero planned steps" + // test above), so reconcileCompletedBlockedTasks() must be able to un-park that + // exact shape too, or the row is stranded forever behind the pause. + const h = harness( + task({ + id: "FN-7926-ADVANCE-ZEROSTEP", + column: "todo", + blockedBy: "FN-BLOCKER", + paused: true, + pausedReason: COMPLETED_BLOCKED_PAUSE_REASON, + status: "queued", + steps: [], + }), + [task({ id: "FN-BLOCKER", column: "todo" })], + ); + const recoverCompletedTask = vi.fn(async (completed: TaskDetail) => { + h.setLive({ + column: "in-review", + paused: false, + pausedReason: undefined, + status: null, + error: null, + blockedBy: null, + }); + return completed.id === "FN-7926-ADVANCE-ZEROSTEP"; + }); + const healer = new SelfHealingManager(h.store, { + rootDir: "/tmp/test", + recoverCompletedTask: recoverCompletedTask as any, + getExecutingTaskIds: () => new Set(), + isTaskActive: () => false, + }); + + await (healer as any).reconcileCompletedBlockedTasks(); + expect(recoverCompletedTask).not.toHaveBeenCalled(); + + h.setRelated("FN-BLOCKER", { column: "done" }); + await (healer as any).reconcileCompletedBlockedTasks(); + + expect(recoverCompletedTask).toHaveBeenCalledWith(expect.objectContaining({ id: "FN-7926-ADVANCE-ZEROSTEP" })); + expect(h.live).toMatchObject({ column: "in-review", paused: false, status: null, blockedBy: null }); + }); }); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 159d4a557b..693cded9fb 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -192,7 +192,7 @@ import { isMissingWorktreeSessionStartFailure, } from "./restart-recovery-coordinator.js"; import { BranchWorktreeAutoRecoveryHandler } from "./auto-recovery-handlers/branch-worktree.js"; -import { autoRecoverWorktreeSessionStartFailure, MAX_WORKTREE_SESSION_RETRIES, PAUSE_ABORT_PARK_ERROR_MARKER, PAUSE_ABORT_PARK_OPERATOR_MARKER } from "./self-healing.js"; +import { autoRecoverWorktreeSessionStartFailure, COMPLETED_BLOCKED_PAUSE_REASON, MAX_WORKTREE_SESSION_RETRIES, PAUSE_ABORT_PARK_ERROR_MARKER, PAUSE_ABORT_PARK_OPERATOR_MARKER } from "./self-healing.js"; import { ContaminationAutoRecoveryHandler } from "./auto-recovery-handlers/contamination.js"; import { createFileScopeAutoRecoveryHandler } from "./auto-recovery-handlers/file-scope.js"; import { ReadonlyViolationError, filterCustomToolsForReadonly } from "./workflow-step-tool-policy.js"; @@ -3960,15 +3960,70 @@ export class TaskExecutor { this.workflowRerunWatchdogs.set(taskId, watchdog); } - private async shouldFinalizeCompletedTask(taskId: string, taskDone: boolean): Promise { + private async parkCompletedBlockedTask(task: Task, completionBlocker: string, source: string, workComplete = this.isTaskWorkComplete(task)): Promise { + if (task.paused === true || task.userPaused === true) return false; + if (task.column === "done" || task.column === "archived") return false; + if (!workComplete) return false; + + const message = `Completed work held — ${completionBlocker}; will advance to review when blocker clears`; + /* + FNXC:WorkflowLifecycle 2026-07-12-23:13: + FN-7926: completed work with a persistent `getTaskCompletionBlocker` result must not self-requeue through the execute node. Re-running implementation cannot clear dependency/blockedBy state, so it only feeds FN-7863's generic no-progress backstop and misclassifies good work as `EXECUTION_DISPATCH_LOOP_EXHAUSTED`. Park in a scheduler-skipped todo state, preserve worktree/branch/steps, and reset the FN-7863 signature so the backstop remains reserved for genuinely incomplete no-progress loops. + */ + if (task.column !== "todo") { + await this.store.moveTask(task.id, "todo", { + preserveProgress: true, + preserveResumeState: true, + preserveWorktree: true, + moveSource: "engine", + recoveryRehome: true, + }); + } + await this.store.updateTask(task.id, { + paused: true, + pausedReason: COMPLETED_BLOCKED_PAUSE_REASON, + status: "queued", + error: null, + executeRequeueLoopCount: null, + executeRequeueLoopSignature: null, + }, this.getRunContextFor(task.id)); + executorLog.log(`${task.id}: ${message}`); + await this.store.logEntry(task.id, message, undefined, this.getRunContextFor(task.id)); + await this.store.recordRunAuditEvent?.({ + taskId: task.id, + agentId: "executor", + runId: generateSyntheticRunId("completed-blocked-park", task.id), + domain: "database", + mutationType: "task:completed-blocked-parked", + target: task.id, + metadata: { + taskId: task.id, + blocker: completionBlocker, + source, + priorColumn: task.column, + priorStatus: task.status ?? null, + }, + }); + return true; + } + + private async getCompletedTaskFinalizationDecision(taskId: string, taskDone: boolean): Promise<"finalize" | "blocked" | "incomplete"> { const task = await this.store.getTask(taskId); const completionBlocker = await this.getTaskCompletionBlocker(task); + const workComplete = taskDone || this.isTaskWorkComplete(task); if (completionBlocker) { executorLog.log(`${taskId} completion blocked — ${completionBlocker}`); - return false; + if (workComplete && await this.parkCompletedBlockedTask(task, completionBlocker, "finalization", workComplete)) { + return "blocked"; + } + return "incomplete"; } - if (taskDone) return true; - return this.isTaskWorkComplete(task); + if (workComplete) return "finalize"; + return "incomplete"; + } + + private async shouldFinalizeCompletedTask(taskId: string, taskDone: boolean): Promise { + return await this.getCompletedTaskFinalizationDecision(taskId, taskDone) === "finalize"; } private isTaskAlreadyCompleteForNonContinuableSession(task: Task, taskDone: boolean): boolean { @@ -9044,7 +9099,15 @@ export class TaskExecutor { FNXC:WorkflowLifecycle 2026-07-12-00:00: FN-7863: the scheduler's wall-clock dispatchStormCount guard only increments when re-dispatches happen inside its short window; slow execute→pause-abort→todo loops reset that counter every cycle. Count this funnel by execution-progress signature instead, warn early for board-visible monitoring, and terminalize only non-paused live tasks after the bounded no-progress cap while preserving worktree/branch/step progress. + + FNXC:WorkflowLifecycle 2026-07-12-23:14: + FN-7926 diverts completed-but-blocked rows before the FN-7863 counter increments. A stable all-done step signature plus unresolved dependency/blockedBy is a waiting state, not an implementation no-progress loop; park it with the specific blocker and let self-healing advance it when `getTaskCompletionBlocker` clears. */ + const completionBlocker = await this.getTaskCompletionBlocker(live); + if (completionBlocker && await this.parkCompletedBlockedTask(live, completionBlocker, "execute-requeue")) { + await this.persistTokenUsage(task.id); + return; + } const signature = buildExecuteRequeueLoopSignature(live); const nextCount = live.executeRequeueLoopSignature === signature ? (live.executeRequeueLoopCount ?? 0) + 1 @@ -11148,7 +11211,8 @@ export class TaskExecutor { } this.clearPausedAborted(task.id); wasPaused = true; - if (await this.shouldFinalizeCompletedTask(task.id, taskDone)) { + const finalizationDecision = await this.getCompletedTaskFinalizationDecision(task.id, taskDone); + if (finalizationDecision === "finalize") { if (await this.shouldDeferCompletionForGlobalPause(task.id, "paused after completion")) { return; } @@ -11166,6 +11230,9 @@ export class TaskExecutor { await this.handoffTaskToReview(task, "paused-after-completion"); this.clearCompletedTaskWatchdog(task.id); this.signalTaskComplete(task); + } else if (finalizationDecision === "blocked") { + await this.persistTokenUsage(task.id); + return; } else { executorLog.log(`${task.id} paused (graceful session exit) — moving to todo`); await this.store.logEntry(task.id, "Execution paused — session preserved for resume, moved to todo"); @@ -11701,7 +11768,8 @@ export class TaskExecutor { ); return; } - if (await this.shouldFinalizeCompletedTask(task.id, taskDone)) { + const finalizationDecision = await this.getCompletedTaskFinalizationDecision(task.id, taskDone); + if (finalizationDecision === "finalize") { if (await this.shouldDeferCompletionForGlobalPause(task.id, "paused after completion")) { return; } @@ -11718,6 +11786,9 @@ export class TaskExecutor { this.markCompletionFinalized(task.id); await this.handoffTaskToReview(task, "paused-after-completion"); this.signalTaskComplete(task); + } else if (finalizationDecision === "blocked") { + await this.persistTokenUsage(task.id); + return; } else { executorLog.log(`${task.id} paused — moving to todo`); if (worktreePath && existsSync(worktreePath)) { diff --git a/packages/engine/src/run-audit.ts b/packages/engine/src/run-audit.ts index 724945a239..d09795567d 100644 --- a/packages/engine/src/run-audit.ts +++ b/packages/engine/src/run-audit.ts @@ -554,6 +554,10 @@ export type DatabaseMutationType = | "task:dispatch-oscillation-terminalized" /** Metadata: { taskId, cycleCount, maxCycles, progressSignature, failureValue } */ | "task:execution-dispatch-loop-terminalized" + /** Metadata: { taskId, blocker, source, priorColumn, priorStatus } */ + | "task:completed-blocked-parked" + /** Metadata: { taskId, priorColumn, priorStatus, source } */ + | "task:completed-blocked-advanced" | "task:auto-recover-starved-refinement" /** Metadata: { rawDiffFileCount: number; attributedFileCount: number; foreignCommitCount: number; foreignCommitShas: string[]; source: string } */ | "task:worktree-contamination-detected" diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index 8273c487ca..e9c1e95dd6 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -68,6 +68,9 @@ self-healing — a real import cycle. Importing from the predicate module breaks */ import { isRepoLanded } from "./workspace-land-predicate.js"; import { findAlreadyMergedTaskCommit, getCommitTaskOwnership } from "./already-merged-detector.js"; +import { getTaskCompletionBlockerForStore } from "./task-completion.js"; + +export const COMPLETED_BLOCKED_PAUSE_REASON = "completed-work-blocked"; import { advanceIntegrationBranchRef } from "./merger-ref-update-advance.js"; import { isAiMergeContainerDir, resolveAiMergeRootPath, resolveLegacyAiMergeRootPath, resolveWorktreesDir } from "./worktree-paths.js"; import { canonicalFusionBranchName, resolveTaskWorkingBranch } from "./worktree-names.js"; @@ -1407,6 +1410,7 @@ export class SelfHealingManager { { name: "clear-stale-blocked-by", fn: () => this.clearStaleBlockedBy().then(() => undefined) }, { name: "reconcile-self-defeating-deps", fn: () => this.reconcileSelfDefeatingDependencies().then(() => undefined) }, { name: "reconcile-dependency-blocking-leases", fn: () => this.reconcileDependencyBlockingLeases().then(() => undefined) }, + { name: "reconcile-completed-blocked", fn: () => this.reconcileCompletedBlockedTasks().then(() => undefined) }, { name: "reconcile-in-review-unmet-dependencies", fn: () => this.reconcileInReviewUnmetDependencies().then(() => undefined) }, { name: "reconcile-engine-downtime-active-timing", fn: () => this.reconcileEngineDowntimeActiveTiming().then(() => undefined) }, { name: "reconcile-dependency-cycles", fn: () => this.reconcileDependencyCycles().then(() => undefined) }, @@ -2620,6 +2624,7 @@ export class SelfHealingManager { { name: "recover-stale-transition-pending", fn: () => this.runStaleTransitionPendingSweep() }, { name: "reconcile-self-defeating-deps", fn: () => this.reconcileSelfDefeatingDependencies() }, { name: "reconcile-dependency-blocking-leases", fn: () => this.reconcileDependencyBlockingLeases() }, + { name: "reconcile-completed-blocked", fn: () => this.reconcileCompletedBlockedTasks() }, { name: "reconcile-in-review-unmet-dependencies", fn: () => this.reconcileInReviewUnmetDependencies() }, // FN-6782: reclaim in-memory worktree slots whose holder is no longer // in-progress (defense-in-depth for the pause-abort leak; conservative, @@ -5601,6 +5606,96 @@ export class SelfHealingManager { }; } + async reconcileCompletedBlockedTasks(): Promise { + const settings = await this.store.getSettings(); + if (settings.globalPause || settings.enginePaused) return 0; + if (!this.options.recoverCompletedTask) return 0; + + let tasks: Task[] = []; + try { + tasks = await this.store.listTasks({ includeArchived: false, slim: true }); + } catch (err: unknown) { + const errorMessage = err instanceof Error ? err.message : String(err); + log.warn(`reconcileCompletedBlockedTasks: failed to list tasks: ${errorMessage}`); + return 0; + } + + const executingIds = this.options.getExecutingTaskIds?.() ?? new Set(); + let recovered = 0; + for (const snapshot of tasks) { + if (snapshot.deletedAt) continue; + if (snapshot.column !== "todo") continue; + if (snapshot.paused !== true || snapshot.pausedReason !== COMPLETED_BLOCKED_PAUSE_REASON) continue; + if (snapshot.userPaused === true) continue; + if (!allowsAutoMergeProcessing(snapshot, settings)) continue; + if (executingIds.has(snapshot.id) || this.options.isTaskActive?.(snapshot.id) === true) continue; + if (snapshot.worktree && activeSessionRegistry.isPathActive(snapshot.worktree)) continue; + /* + FNXC:WorkflowLifecycle 2026-07-12-23:40: + FN-7926: unlike the generic isTaskWorkComplete() convention used elsewhere in self-healing, + a zero-step task CAN legitimately reach this parked state — parkCompletedBlockedTask() + already accepts `workComplete = taskDone` for a task with no planned steps (explicit + fn_task_done() with an empty step list). Since COMPLETED_BLOCKED_PAUSE_REASON is only ever + set by that already-validated park path, re-deriving completeness by rejecting empty step + arrays here would strand those rows forever (parked but never reconciled — the same + indefinite non-terminal stall this task exists to eliminate). Only reject when steps exist + and are provably incomplete (defense-in-depth against a concurrent reopen after park). + */ + if (snapshot.steps.length > 0 && !snapshot.steps.every((step) => step.status === "done" || step.status === "skipped")) continue; + + const completionBlocker = await getTaskCompletionBlockerForStore(this.store, snapshot); + if (completionBlocker) continue; + + try { + await this.store.updateTask(snapshot.id, { + paused: false, + pausedReason: undefined, + status: null, + error: null, + blockedBy: null, + executeRequeueLoopCount: null, + executeRequeueLoopSignature: null, + }); + const fresh = await this.store.getTask(snapshot.id); + const advanced = await this.options.recoverCompletedTask(fresh); + if (!advanced) { + await this.store.updateTask(snapshot.id, { + paused: true, + pausedReason: COMPLETED_BLOCKED_PAUSE_REASON, + status: "queued", + }); + continue; + } + await this.store.logEntry( + snapshot.id, + "Auto-advanced completed blocked work to review after blocker cleared", + ); + await createRunAuditor(this.store, { + runId: generateSyntheticRunId("completed-blocked-advance", snapshot.id), + agentId: "self-healing", + taskId: snapshot.id, + taskLineageId: snapshot.lineageId, + phase: "reconcile-completed-blocked", + }).database({ + type: "task:completed-blocked-advanced" as DatabaseMutationType, + target: snapshot.id, + metadata: { + taskId: snapshot.id, + priorColumn: snapshot.column, + priorStatus: snapshot.status ?? null, + source: "self-healing", + }, + }); + recovered++; + } catch (err: unknown) { + const errorMessage = err instanceof Error ? err.message : String(err); + log.warn(`reconcileCompletedBlockedTasks: failed to advance ${snapshot.id}: ${errorMessage}`); + } + } + + return recovered; + } + async reconcileInReviewUnmetDependencies(): Promise { const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) return 0;