diff --git a/.changeset/clear-orphaned-pending-step-results.md b/.changeset/clear-orphaned-pending-step-results.md new file mode 100644 index 0000000000..73ef89ba12 --- /dev/null +++ b/.changeset/clear-orphaned-pending-step-results.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Recover in-review tasks stranded by a restart that killed an in-flight review step, instead of failing them. +category: fix +dev: New startup sweep `reconcileOrphanedPendingStepResults` wires the previously caller-less `resolveOrphanedPendingStepResults` helper; emits `task:reconcile-orphaned-pending-step-results` run-audit events. diff --git a/AGENTS.md b/AGENTS.md index f5601b55ca..62c707cb03 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -283,6 +283,7 @@ Scoped exception (FN-5819): shared-branch-group members (`branchContext.assignme - FN-8004: `agent:heartbeat-move-skipped-soft-delete` records a heartbeat move that races a soft-deleted task without parking the durable agent. Metadata remains ids/timestamps/source only (`agentId`, optional `taskId`/`deletedAt`, `moveAttemptedAt`, optional `source`); it never stores error prose. - FN-8141: the executor's `fn_task_done(outcome="blocked", reason=..., blockedBy?=[...])` honest-blocked exit emits `task:execution-blocked-parked` when an executor parks a genuinely-impossible task `failed` (`error = "BLOCKED: "`) instead of laundering it to `done` by skipping steps. It bypasses the completion/verdict/bulk-completion gates (blocked is not a completion claim), leaves steps in their true statuses, preserves worktree/branch, records `blockedBy` as real `task.dependencies` edges so the task requeues behind the blocker, and does NOT hand off to review — the parked row is honored by the executor's `status === "failed"` post-loop branch and is not auto-recovered into in-review by `recoverStrandedCompletedTodoTasks` (steps are not all done/skipped and `task.error` is set). Metadata stays ids/outcomes-only (`taskId`, `blockedBy` ids, `hasReason` boolean — never the reason prose). - FN-8305: durable symbol-lock operations emit `symbol-lock:acquired`, `symbol-lock:acquire-conflict`, `symbol-lock:renewed`, `symbol-lock:released`, `symbol-lock:reconcile-stale`, and deduplicated `symbol-lock:reconcile-stale-no-action`. Metadata is ids/counts/outcomes-only; normalized opaque symbol keys are permitted IDs, while raw symbol prose is not. +- FN-8492: the self-healing STARTUP sweep `reconcile-orphaned-pending-step-results` emits `task:reconcile-orphaned-pending-step-results` when it clears `pending` workflow-step results with no live session behind them (canonical liveness triple: `activeSessionRegistry` path, `executingTaskLock`, `isTaskActive`). It runs right after legacy adoption and before the in-review recovery steps, because an orphaned `pending` result reads as "work in flight" to the merge gate and rides the in-review stall escalator to a `failed` deadlock park. Metadata is ids/counts-only (`taskId`, `column`, `clearedCount`, `remainingCount`); user pauses and live sessions are never disturbed. - FN-8356: self-healing emits `task:reconcile-stale-duplicate-decision` when it clears a triage-marker duplicate-decision pause against a missing, deleted, done, or archived canonical. Metadata is ids/outcomes-only (`taskId`, `canonicalId`, `canonicalColumn`, `canonicalDeleted`, `priorPausedReason`); active canonical decisions and user pauses remain untouched. - U9b (R10/KTD-8): the self-healing STARTUP recovery step `adopt-legacy-task-rows` emits `task:reconcile-legacy-adoption` when it adopts a pre-cutover row through the KTD-8 adoption table (clearing a legacy `task.status` whose writer the cutover deleted so the graph re-enters at its owning node, and/or landing the one-time `reviewLevel` -> `enabledWorkflowSteps` preset backfill), and `task:reconcile-legacy-adoption-unmappable` when an UNKNOWN status parks the row `paused` for a human with its status deliberately left in place. Metadata is ids/counts/outcomes-only (`taskId`, `action`, `priorStatus`, `column`, `backfilledStepCount`, `reason`), where `reason` is a fixed adoption-table note and never row prose. Adoption runs FIRST in startup recovery (every later step reasons about `task.status`), stamps `task.legacyAdoptedAt` only on rows it actually mutates (so upgrade does not mass-write every `done` row), and never touches a user pause or a `preserve` gate. `planLegacyAdoption` in `packages/core/src/legacy-adoption.ts` is the single shared decision used by both this sweep and the store-open reconcile so the two cannot drift. - U10 (R9): the pre-graph cutover machinery is DELETED and stays deleted, ratcheted by `packages/engine/src/__tests__/legacy-tombstones.test.ts`. Gone: `workflow-cutover.ts`, `workflow-authoritative-driver.ts`, `workflow-parity-observer.ts`, the `graphCompletionInterceptors` re-entry map, triage's out-of-graph `runPlanReviewBeforeExecution` gate, and the in-session `fn_review_step` tool with its RETHINK git-reset/session-rewind, per-step conversation checkpoints, deferred reviewer provider-error channel, and review-level prompt scaffolding. Plan/code/browser review are owned EXCLUSIVELY by workflow-graph nodes — do not re-introduce a second review authority inside the implementation session; that duplicate-Plan-Review race is what the cutover removed. The tombstone test strips comments before searching, so the FNXC notes that explain each deletion are expected to remain in source while the code must not. diff --git a/packages/engine/src/__tests__/self-healing-orphaned-pending-step-results.test.ts b/packages/engine/src/__tests__/self-healing-orphaned-pending-step-results.test.ts new file mode 100644 index 0000000000..c075c1ef3e --- /dev/null +++ b/packages/engine/src/__tests__/self-healing-orphaned-pending-step-results.test.ts @@ -0,0 +1,126 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { EventEmitter } from "node:events"; +import type { Settings, Task, TaskStore, WorkflowStepResult } from "@fusion/core"; + +const { recordRunAuditEventMock } = vi.hoisted(() => ({ + recordRunAuditEventMock: vi.fn(async () => undefined), +})); +vi.mock("../run-audit.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + createRunAuditor: vi.fn(() => ({ database: recordRunAuditEventMock, git: vi.fn(), filesystem: vi.fn(), sandbox: vi.fn() })), + }; +}); + +import { SelfHealingManager } from "../self-healing.js"; + +/* +FNXC:OrphanedPendingSteps 2026-07-22-16:20 (FN-8492 incident): +An engine restart killed an in-flight pre-merge Code Review session, leaving its +`pending` workflowStepResult with no live session behind it. The merge gate read that as +"incomplete pre-merge workflow steps" and after 3 identical 30-minute stalls the deadlock +disposer parked the task `failed`. These tests pin the startup sweep that clears such +orphans — and the liveness veto that keeps it from eating a genuinely live session. +*/ + +function stepResult(overrides: Partial = {}): WorkflowStepResult { + return { + phase: "pre-merge", + source: "optional-group", + status: "passed", + workflowStepId: "plan-review", + workflowStepName: "Plan Review", + ...overrides, + } as WorkflowStepResult; +} + +function task(id: string, overrides: Partial = {}): Task { + return { + id, + title: id, + description: id, + column: "in-review", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + ...overrides, + } as Task; +} + +function storeFor(tasks: Task[]): TaskStore & EventEmitter { + const tasksById = new Map(tasks.map((entry) => [entry.id, entry])); + return Object.assign(new EventEmitter(), { + getSettings: vi.fn(async () => ({ globalPause: false, enginePaused: false } as Settings)), + listTasks: vi.fn(async () => [...tasksById.values()]), + getTask: vi.fn(async (id: string) => tasksById.get(id)), + updateTask: vi.fn(async (id: string, patch: Partial) => { + const next = { ...tasksById.get(id)!, ...patch } as Task; + tasksById.set(id, next); + return next; + }), + }) as unknown as TaskStore & EventEmitter; +} + +describe("FN-8492: reconcile orphaned pending step results at startup", () => { + beforeEach(() => vi.clearAllMocks()); + + it("clears a dead-session pending result, keeps completed ones, and audits ids/counts only", async () => { + const stranded = task("FN-1", { + workflowStepResults: [ + stepResult({ status: "passed", verdict: "APPROVE" }), + stepResult({ status: "pending", workflowStepId: "code-review", workflowStepName: "Code Review" }), + ], + }); + const store = storeFor([stranded]); + const manager = new SelfHealingManager(store, { rootDir: "/repo" }); + + expect(await manager.reconcileOrphanedPendingStepResults()).toBe(1); + const recovered = await store.getTask("FN-1"); + expect(recovered?.workflowStepResults).toHaveLength(1); + expect(recovered?.workflowStepResults?.[0]?.status).toBe("passed"); + expect(recordRunAuditEventMock).toHaveBeenCalledTimes(1); + expect(recordRunAuditEventMock).toHaveBeenCalledWith(expect.objectContaining({ + type: "task:reconcile-orphaned-pending-step-results", + target: "FN-1", + metadata: expect.objectContaining({ taskId: "FN-1", clearedCount: 1, remainingCount: 1 }), + })); + }); + + it("never clears a pending result while the task session is live (executor resumed it)", async () => { + const live = task("FN-LIVE", { + workflowStepResults: [stepResult({ status: "pending" })], + }); + const store = storeFor([live]); + const manager = new SelfHealingManager(store, { + rootDir: "/repo", + isTaskActive: (id: string) => id === "FN-LIVE", + }); + + expect(await manager.reconcileOrphanedPendingStepResults()).toBe(0); + expect((await store.getTask("FN-LIVE"))?.workflowStepResults).toHaveLength(1); + expect(recordRunAuditEventMock).not.toHaveBeenCalled(); + }); + + it("leaves user-paused tasks and tasks with no pending results untouched", async () => { + const userPaused = task("FN-PAUSED", { + userPaused: true, + paused: true, + workflowStepResults: [stepResult({ status: "pending" })], + }); + const complete = task("FN-DONE-STEPS", { + workflowStepResults: [stepResult({ status: "passed" }), stepResult({ status: "failed" })], + }); + const noResults = task("FN-NONE"); + const store = storeFor([userPaused, complete, noResults]); + const manager = new SelfHealingManager(store, { rootDir: "/repo" }); + + expect(await manager.reconcileOrphanedPendingStepResults()).toBe(0); + expect((await store.getTask("FN-PAUSED"))?.workflowStepResults).toHaveLength(1); + expect((await store.getTask("FN-DONE-STEPS"))?.workflowStepResults).toHaveLength(2); + expect(recordRunAuditEventMock).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index 0887843121..683efaa0c2 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -30,7 +30,7 @@ import { setImmediate as setImmediateCb } from "node:timers"; import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmSync, statSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { isAbsolute, join, relative, resolve } from "node:path"; -import { IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkflowColumnsEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, resolveWorkflowIrForTask, resolveReboundTarget, planLegacyAdoption, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult } from "@fusion/core"; +import { IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkflowColumnsEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, resolveWorkflowIrForTask, resolveReboundTarget, planLegacyAdoption, resolveOrphanedPendingStepResults, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult } from "@fusion/core"; import { finalizePlanningSegment } from "@fusion/core"; import type { MeshLeaseManager } from "./mesh-lease-manager.js"; import { createLogger, schedulerLog } from "./logger.js"; @@ -1379,6 +1379,12 @@ export class SelfHealingManager { // legacy `planning`/`needs-replan` row is judged by recovery rules that no longer // have a writer for that status. { name: "adopt-legacy-task-rows", fn: () => this.adoptLegacyTaskRows().then(() => undefined) }, + // FNXC:OrphanedPendingSteps 2026-07-22-16:20 (FN-8492 incident): runs right after + // adoption and BEFORE every in-review recovery step below — those reason about + // step-result completeness, and an orphaned `pending` result reads as "work in + // flight" to all of them (the merge gate included), which is the two-hour + // stall-deadlock ride this sweep exists to prevent. + { name: "reconcile-orphaned-pending-step-results", fn: () => this.reconcileOrphanedPendingStepResults().then(() => undefined) }, { name: "no-progress-no-task-done", fn: () => this.recoverNoProgressNoTaskDoneFailures().then(() => undefined) }, { name: "completed-tasks", fn: () => this.recoverCompletedTasks().then(() => undefined) }, { name: "recover-stranded-completed-todo", fn: () => this.recoverStrandedCompletedTodoTasks().then(() => undefined) }, @@ -6665,6 +6671,78 @@ export class SelfHealingManager { } } + /* + FNXC:OrphanedPendingSteps 2026-07-22-16:20 (FN-8492 incident): + Startup consumer of `resolveOrphanedPendingStepResults` — the U9b helper shipped with NO + caller (the same gap U9 left for the adoption table), so an engine restart that killed an + in-flight pre-merge step session left its `pending` workflowStepResult behind forever. + The merge gate read it as "incomplete pre-merge workflow steps", surfaced an identical + stall every 30 minutes, and after 3 stalls the deadlock disposer parked the task `failed` + (FN-8492: Code Review died in the 21:29 restart, task parked two hours later). + + Runs at STARTUP, right after legacy adoption: sessions do not survive a restart, so a + `pending` result with no live session behind it is orphaned by construction. Liveness + uses the canonical triple (activeSessionRegistry path, executingTaskLock, isTaskActive) + because runStartupRecovery runs AFTER the executor resumes orphaned sessions — a resumed + session re-registers its path and must veto the clear. User pauses are never disturbed. + */ + async reconcileOrphanedPendingStepResults(): Promise { + try { + const pageSize = 500; + let offset = 0; + let recovered = 0; + + for (;;) { + const tasks = await this.store.listTasks({ slim: true, includeArchived: false, limit: pageSize, offset }); + for (const task of tasks) { + // An operator park is authoritative; this sweep must not reach through it. + if (task.userPaused === true) continue; + const results = task.workflowStepResults; + if (!results?.some((result) => result.status === "pending")) continue; + + const livePaths = activeSessionRegistry.pathsForTask(task.id); + const hasActiveRegisteredPath = livePaths.some((path) => activeSessionRegistry.isPathActive(path)); + const sessionLive = hasActiveRegisteredPath || executingTaskLock.has(task.id) + || this.options.isTaskActive?.(task.id) === true; + + const { cleared, clearedCount } = resolveOrphanedPendingStepResults(results, () => sessionLive); + if (clearedCount === 0) continue; + + try { + await this.store.updateTask(task.id, { workflowStepResults: cleared }); + await createRunAuditor(this.store, { + runId: generateSyntheticRunId("reconcile-orphaned-pending-steps", task.id), + agentId: "self-healing", + taskId: task.id, + taskLineageId: task.lineageId, + phase: "reconcile-orphaned-pending-step-results", + }).database({ + type: "task:reconcile-orphaned-pending-step-results" as DatabaseMutationType, + target: task.id, + // ids/counts/outcomes only — never step output or reviewer prose. + metadata: { + taskId: task.id, + column: task.column, + clearedCount, + remainingCount: cleared.length, + }, + }); + recovered += 1; + } catch (error) { + log.warn(`reconcileOrphanedPendingStepResults: failed for ${task.id}: ${error instanceof Error ? error.message : String(error)}`); + } + } + if (tasks.length < pageSize) break; + offset += tasks.length; + } + if (recovered > 0) log.log(`Cleared orphaned pending step results on ${recovered} task(s)`); + return recovered; + } catch (error) { + log.error(`reconcileOrphanedPendingStepResults failed: ${error instanceof Error ? error.message : String(error)}`); + return 0; + } + } + /** * Backward lifecycle move gated on triple proof (FN-5335). * When the unproven fallback predicate fails, emits `task:finalize-no-op-review-no-action` and skips lifecycle mutation.