diff --git a/.changeset/merge-confirmed-finalization.md b/.changeset/merge-confirmed-finalization.md new file mode 100644 index 0000000000..037376adc9 --- /dev/null +++ b/.changeset/merge-confirmed-finalization.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: A task whose branch already merged can no longer get stuck as failed with unfinished steps. +category: fix +dev: `getMergeConfirmedFinalizationBlocker` (core) exempts incomplete `steps` at all four merge-confirmed finalization sites once landing is proven, while a no-op merge that landed no content still blocks. Unfinished steps are logged as `MergeConfirmedFinalizeUnfinishedSteps` rather than dropped. diff --git a/packages/core/src/__tests__/task-merge.test.ts b/packages/core/src/__tests__/task-merge.test.ts index 59f360e174..7e01d11f28 100644 --- a/packages/core/src/__tests__/task-merge.test.ts +++ b/packages/core/src/__tests__/task-merge.test.ts @@ -1,6 +1,8 @@ import { describe, it, expect } from "vitest"; import type { PrInfo, StepStatus } from "../types.js"; import { + getMergeConfirmedFinalizationBlocker, + getUnfinishedStepTitles, isPreMergeStepsNotRunBlocker, PreMergeStepsNotRunError, PRE_MERGE_STEPS_NOT_RUN_BLOCKER, @@ -1085,3 +1087,68 @@ describe("isTaskBlockedOnApproval", () => { expect(isTaskBlockedOnApproval({ paused: false, pausedReason: AWAITING_APPROVAL_PAUSE_REASON, status: undefined })).toBe(false); }); }); + +/* +FNXC:MergeConfirmedFinalization 2026-08-23-21:40 (FN-9193 aftermath). + +ORIGINAL SYMPTOM: FN-9193's branch landed on main as eaa1d47c, but a Code Review revision request +had reset its steps while the approved merge was in flight. The card was left `mergeConfirmed: true` +WITH incomplete steps, every finalization site refused with "task has incomplete steps", and it sat +`failed` for five hours. Restarting it made it worse: replanning issued seven fresh `pending` steps, +so the retry re-created the exact condition that was blocking it — a loop with no exit. + +ASSERTION: incomplete steps never block a finalization whose landing the caller has already proven, +while a genuinely rejecting pre-merge review still does. +*/ +describe("getMergeConfirmedFinalizationBlocker", () => { + const landedWithUnfinishedWork = { + ...baseTask, + steps: [ + { name: "Preflight", status: "in-progress" as StepStatus }, + { name: "Remove the dead CSS custom-property reference", status: "pending" as StepStatus }, + ], + }; + + it("does not block finalization for incomplete steps", () => { + // The hard blocker still refuses these — that difference IS the fix. + expect(getTaskHardMergeBlocker(landedWithUnfinishedWork)).toBe("task has incomplete steps"); + expect(getMergeConfirmedFinalizationBlocker(landedWithUnfinishedWork)).toBeUndefined(); + }); + + it("survives the restart loop that re-plans fresh pending steps", () => { + const replanned = { + ...baseTask, + steps: Array.from({ length: 7 }, (_, i) => ({ name: `Step ${i}`, status: "pending" as StepStatus })), + }; + expect(getMergeConfirmedFinalizationBlocker(replanned)).toBeUndefined(); + }); + + /* A no-op merge with no commit sha landed NOTHING, so incomplete steps stay an honest blocker — + and the executor's no-op branch depends on that reason still firing. */ + it("still blocks incomplete steps when the merge landed no content", () => { + expect(getMergeConfirmedFinalizationBlocker({ + ...landedWithUnfinishedWork, + mergeDetails: { noOpMerge: true } as never, + })).toBe("task has incomplete steps"); + // A no-op that still produced a commit did land something; the exemption applies. + expect(getMergeConfirmedFinalizationBlocker({ + ...landedWithUnfinishedWork, + mergeDetails: { noOpMerge: true, commitSha: "abc123" } as never, + })).toBeUndefined(); + }); + + it("still blocks on a failed pre-merge review", () => { + // A review that actually rejected this content is a real signal even after landing; the + // FN-7720 operator bypass is the sanctioned way past it. + expect(getMergeConfirmedFinalizationBlocker({ + ...landedWithUnfinishedWork, + workflowStepResults: [{ workflowStepId: "code-review", workflowStepName: "Code Review", status: "failed", phase: "pre-merge" }], + })).toBe("task has failed pre-merge workflow steps"); + }); + + it("names the unfinished steps so they are recorded, not dropped", () => { + expect(getUnfinishedStepTitles(landedWithUnfinishedWork)) + .toEqual(["Preflight", "Remove the dead CSS custom-property reference"]); + expect(getUnfinishedStepTitles({ steps: [{ name: "done work", status: "done" as StepStatus }] })).toEqual([]); + }); +}); diff --git a/packages/core/src/index.gate.ts b/packages/core/src/index.gate.ts index 06bce54fca..8890dc3a18 100644 --- a/packages/core/src/index.gate.ts +++ b/packages/core/src/index.gate.ts @@ -1090,6 +1090,8 @@ export { PreMergeStepsNotRunError, PRE_MERGE_STEPS_NOT_RUN_BLOCKER, getTaskHardMergeBlocker, + getMergeConfirmedFinalizationBlocker, + getUnfinishedStepTitles, REVIEW_ELIGIBLE_SENTINEL_COLUMN, MERGE_CONFIRMED_TRANSIENT_STATUSES, clearMergeConfirmedTransientStatus, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a4e00bc4dd..57f9580792 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1271,6 +1271,8 @@ export { PreMergeStepsNotRunError, PRE_MERGE_STEPS_NOT_RUN_BLOCKER, getTaskHardMergeBlocker, + getMergeConfirmedFinalizationBlocker, + getUnfinishedStepTitles, REVIEW_ELIGIBLE_SENTINEL_COLUMN, MERGE_CONFIRMED_TRANSIENT_STATUSES, clearMergeConfirmedTransientStatus, diff --git a/packages/core/src/merge/task-merge.ts b/packages/core/src/merge/task-merge.ts index 071c3a27f8..ff577a3a79 100644 --- a/packages/core/src/merge/task-merge.ts +++ b/packages/core/src/merge/task-merge.ts @@ -590,6 +590,48 @@ export function getTaskHardMergeBlocker( }); } +/* +FNXC:MergeConfirmedFinalization 2026-08-23-21:40 (FN-9193 aftermath — the wedge that outlived the race): +A CARD WHOSE BRANCH IS ALREADY ON THE TARGET MUST ALWAYS BE ABLE TO FINALIZE. FN-9193 landed +eaa1d47c on main and was then left `mergeConfirmed: true` WITH incomplete steps, because a Code +Review revision request reset its steps while the approved merge was in flight. Every finalization +site evaluated `getTaskHardMergeBlocker`, which counts incomplete steps, so the card could never +reach `done` — it sat `failed` re-reading its own contradiction for five hours, and a RESTART made it +strictly worse: replanning issued seven fresh `pending` steps, so the retry that was supposed to +rescue the card re-created the exact condition blocking it. A self-defeating loop with no exit. + +Incomplete steps are not a safety property here. Holding the card out of `done` does not un-merge +anything; the code is live on the target branch either way. The only thing the hold buys is an +inconsistent board and an alarming failed card. Landing proof is established BEFORE this check by +the callers' `hasDurableMergeProof` / reachability verification, so this is not a laundering path: +`mergeConfirmed` alone never reaches here. + +What still blocks: a failed or pending PRE-MERGE workflow step (a review that actually rejected this +content is a real signal even post-landing, and the operator-bypass path exists for it). What no +longer blocks: incomplete `steps`, which describe implementation work that the landed branch has +already superseded. Callers must surface the unfinished steps rather than silently dropping them. +*/ +export function getMergeConfirmedFinalizationBlocker( + task: Pick, + options: { reviewColumns?: ReadonlySet; requiredPreMergeStepIds?: ReadonlySet } = {}, +): string | undefined { + /* + The exemption is scoped to a merge that actually LANDED CONTENT. A no-op merge with no commit sha + landed nothing, so its card is not "already done however you look at it" — there incomplete steps + are the honest signal that the work is unfinished, and the executor's own no-op branch + (`merge-confirmed-finalize.ts`) depends on that blocker still firing. + */ + const landedNothing = task.mergeDetails?.noOpMerge === true && !task.mergeDetails?.commitSha; + return getTaskHardMergeBlocker(landedNothing ? task : { ...task, steps: [] }, options); +} + +/** Non-terminal steps on a card being finalized after a proven merge — recorded, never silently dropped. */ +export function getUnfinishedStepTitles(task: Pick): string[] { + return (task.steps ?? []) + .filter((step) => NON_TERMINAL_STEP_STATUSES.has(step.status)) + .map((step, index) => step.name?.trim() || `step ${index + 1}`); +} + export function getTaskDoneBypassBlocker( task: Pick, ): string | undefined { diff --git a/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts b/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts index 652c2a1192..34ba84a58d 100644 --- a/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts +++ b/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts @@ -484,8 +484,21 @@ describe("auto-merge proven finalization helper", () => { })); }); - it("blocks workflow finalization while planned steps are still incomplete", async () => { - const strandedTask = { + /* + FNXC:MergeConfirmedFinalization 2026-08-23-21:40 (FN-9193 — DELIBERATE INVERSION of this test): + This case previously asserted that incomplete steps BLOCK a proven merge's finalization. That is + the behaviour that wedged FN-9193: its branch landed on main as eaa1d47c, a Code Review revision + request had reset its steps while the approved merge was in flight, and the card was then left + `mergeConfirmed` WITH incomplete steps — unfinalizable, parked `failed` for five hours. Restarting + it re-planned seven fresh `pending` steps, so the retry re-created the block it was meant to clear. + + Holding a landed card out of `done` un-merges nothing; the code is on the target branch either way. + So the exemption is now asserted here, scoped to merges that actually landed content — the sibling + case below keeps the blocker for a no-op merge that landed nothing, which is the protective half + the original test was really carrying. + */ + it("finalizes a proven merge even when planned steps are still incomplete", async () => { + const landedTask = { id: "FN-INCOMPLETE", title: "Incomplete workflow", description: "Test", @@ -498,27 +511,66 @@ describe("auto-merge proven finalization helper", () => { updatedAt: new Date().toISOString(), mergeDetails: { mergeConfirmed: true, commitSha: "abc123", landedFiles: ["packages/engine/src/executor.ts"] }, } as Task; - const store = createMockStore(strandedTask) as unknown as TaskStore & { + const store = createMockStore(landedTask) as unknown as TaskStore & { + getTask: ReturnType; + updateTask: ReturnType; + moveTask: ReturnType; + logEntry: ReturnType; + recordRunAuditEvent: ReturnType; + }; + store.getTask.mockResolvedValue(landedTask); + + const result = await finalizeProvenAutoMergeTask({ + store, + taskId: "FN-INCOMPLETE", + result: { task: landedTask, ok: true, merged: true, commitSha: "abc123", mergeConfirmed: true } as MergeResult, + source: "workflow-graph-merge-finalize", + rootDir: "/repo", + }); + + expect(result).not.toEqual(expect.objectContaining({ reason: "task has incomplete steps" })); + expect(store.updateTask).not.toHaveBeenCalledWith("FN-INCOMPLETE", expect.objectContaining({ + error: "Merge confirmed but finalization blocked: task has incomplete steps", + })); + // The unfinished work is recorded on the task rather than silently dropped. + expect(store.logEntry).toHaveBeenCalledWith( + "FN-INCOMPLETE", + expect.stringContaining("unfinished step"), + "MergeConfirmedFinalizeUnfinishedSteps", + ); + }); + + it("still blocks finalization when a no-op merge landed no content", async () => { + const noOpTask = { + id: "FN-NOOP", + title: "No-op merge", + description: "Test", + column: "in-progress", + dependencies: [], + steps: [{ status: "done" }, { status: "pending" }], + currentStep: 1, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + mergeDetails: { mergeConfirmed: true, noOpMerge: true, landedFiles: [] }, + } as unknown as Task; + const store = createMockStore(noOpTask) as unknown as TaskStore & { getTask: ReturnType; updateTask: ReturnType; moveTask: ReturnType; recordRunAuditEvent: ReturnType; }; - store.getTask.mockResolvedValue(strandedTask); + store.getTask.mockResolvedValue(noOpTask); const result = await finalizeProvenAutoMergeTask({ store, - taskId: "FN-INCOMPLETE", - result: { task: strandedTask, ok: true, merged: true, commitSha: "abc123", mergeConfirmed: true } as MergeResult, + taskId: "FN-NOOP", + result: { task: noOpTask, ok: true, merged: true, mergeConfirmed: true, noOp: true } as MergeResult, source: "workflow-graph-merge-finalize", rootDir: "/repo", }); expect(result).toEqual(expect.objectContaining({ outcome: "blocked", reason: "task has incomplete steps" })); - expect(store.updateTask).toHaveBeenCalledWith("FN-INCOMPLETE", expect.objectContaining({ - status: "failed", - error: "Merge confirmed but finalization blocked: task has incomplete steps", - })); expect(store.moveTask).not.toHaveBeenCalled(); }); diff --git a/packages/engine/src/merge/auto-merge-finalization.ts b/packages/engine/src/merge/auto-merge-finalization.ts index 9d0d7ba08e..4a64c9612e 100644 --- a/packages/engine/src/merge/auto-merge-finalization.ts +++ b/packages/engine/src/merge/auto-merge-finalization.ts @@ -1,5 +1,6 @@ import { - getTaskHardMergeBlocker, + getMergeConfirmedFinalizationBlocker, + getUnfinishedStepTitles, resolveWorkflowIrForTask, resolveCompleteColumn, resolveMergeOrchestrationColumn, @@ -285,7 +286,10 @@ export async function finalizeProvenAutoMergeTask({ return { outcome: "blocked", task: latest, previousColumn: latest.column, reason }; } - const hardBlocker = getTaskHardMergeBlocker({ + /* FNXC:MergeConfirmedFinalization 2026-08-23-21:40 (FN-9193): landing is already proven above by + `hasDurableMergeProof`, so incomplete steps must not hold the card out of `done` — see the + core helper for why that hold was self-defeating. Unfinished steps are logged, not dropped. */ + const hardBlocker = getMergeConfirmedFinalizationBlocker({ ...latest, /* FNXC:WorkflowMerge 2026-06-29-09:15: @@ -300,6 +304,14 @@ export async function finalizeProvenAutoMergeTask({ status: clearMergeConfirmedTransientStatus(latest.status), error: undefined, }); + const unfinishedSteps = getUnfinishedStepTitles(latest); + if (unfinishedSteps.length > 0) { + await store.logEntry( + taskId, + `Finalizing proven merge with ${unfinishedSteps.length} unfinished step(s) — the branch already landed, so these did not run: ${unfinishedSteps.slice(0, 8).join("; ")}`, + "MergeConfirmedFinalizeUnfinishedSteps", + ).catch(() => undefined); + } if (hardBlocker) { // FNXC:MergeReliability 2026-08-11-21:39: A blocker discovered before finalization // still writes task lifecycle state, so an orphan must reject rather than return a blocked result. diff --git a/packages/engine/src/project-engine.ts b/packages/engine/src/project-engine.ts index c413aec7d3..ca795be035 100644 --- a/packages/engine/src/project-engine.ts +++ b/packages/engine/src/project-engine.ts @@ -33,7 +33,8 @@ import { emitOverseerRecoveryAttempt, emitOverseerRetry, emitOverseerSteering, - getTaskHardMergeBlocker, + getMergeConfirmedFinalizationBlocker, + getUnfinishedStepTitles, PreMergeStepsNotRunError, PRE_MERGE_STEPS_NOT_RUN_BLOCKER, classifyMergeSweepAdmission, @@ -4369,7 +4370,7 @@ export class ProjectEngine { continue; } } // end !isWorkspaceTask reachability gate (B2): workspace tasks skip the root-cwd commitSha check - const blockerReason = getTaskHardMergeBlocker({ + const blockerReason = getMergeConfirmedFinalizationBlocker({ ...(task as Task), /* FNXC:WorkflowResolvedColumns 2026-07-30-18:05 (this parked ALREADY-MERGED work as failed): @@ -4383,7 +4384,13 @@ export class ProjectEngine { and for the reason recorded there: `"in-review"` is the review-eligible SENTINEL for this helper, not a lifecycle column, so a merge-confirmed card evaluates the same blocker set on a custom workflow as on the builtin one. The column identity of an already-landed card - is not what this check is for — paused / error / incomplete steps still apply. + is not what this check is for. + + FNXC:MergeConfirmedFinalization 2026-08-23-21:40 (FN-9193): incomplete steps NO LONGER + apply here. The fast path above already proved this merge landed, and holding a landed + card out of `done` for unfinished steps is what left FN-9193 permanently unfinalizable — + a restart then replanned fresh pending steps and re-created the very block it was meant + to clear. Paused/error/pre-merge-step blockers still apply. */ column: REVIEW_ELIGIBLE_SENTINEL_COLUMN, // Merge-confirmed tasks have already landed. Treat stale merge @@ -4393,6 +4400,14 @@ export class ProjectEngine { status: clearMergeConfirmedTransientStatus(task.status), error: undefined, }); + const unfinishedFastPathSteps = getUnfinishedStepTitles(task as Task); + if (unfinishedFastPathSteps.length > 0) { + await store.logEntry( + taskId, + `Finalizing proven merge with ${unfinishedFastPathSteps.length} unfinished step(s) — the branch already landed, so these did not run: ${unfinishedFastPathSteps.slice(0, 8).join("; ")}`, + "MergeConfirmedFinalizeUnfinishedSteps", + ).catch(() => undefined); + } if (blockerReason) { await store.updateTask(taskId, { status: "failed", diff --git a/packages/engine/src/self-healing.ts b/packages/engine/src/self-healing.ts index ed432e24dc..17df66a721 100644 --- a/packages/engine/src/self-healing.ts +++ b/packages/engine/src/self-healing.ts @@ -30,7 +30,7 @@ import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmSync, import { readFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { basename, dirname, isAbsolute, join, relative, resolve } from "node:path"; -import { loadWorkspaceConfig, type TaskMoveLanes, resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, hasSharedBranchMemberAutoMergeHold, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isLiveSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, getBuiltinWorkflow, isBuiltinWorkflowId, resolveWorkflowIrForTask, resolveWorkflowIrForTaskWithProvenance, resolveReboundTarget, resolveReboundTargetForTask, columnsWithFlag, resolveLifecycleColumns, resolveTaskLifecycleColumns, workflowHasColumn, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, 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, type WorkflowIr, +import { loadWorkspaceConfig, type TaskMoveLanes, resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, hasSharedBranchMemberAutoMergeHold, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getMergeConfirmedFinalizationBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isLiveSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, getBuiltinWorkflow, isBuiltinWorkflowId, resolveWorkflowIrForTask, resolveWorkflowIrForTaskWithProvenance, resolveReboundTarget, resolveReboundTargetForTask, columnsWithFlag, resolveLifecycleColumns, resolveTaskLifecycleColumns, workflowHasColumn, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, 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, type WorkflowIr, resolveNearDuplicateCanonicalFlags, LEGACY_COLUMN_IDS_BY_ROLE, TERMINAL_ROLES, @@ -12131,8 +12131,11 @@ const movedTask = await this.store.moveTask(task.id, completeLane); mergeTargetSource: mergeTarget.source, }; - /* Wired: unwired, this would decline every card the widened read now finds. */ - const hardBlocker = getTaskHardMergeBlocker({ + /* Wired: unwired, this would decline every card the widened read now finds. + FNXC:MergeConfirmedFinalization 2026-08-23-21:40 (FN-9193): this sweep has already + proven the content landed on the base branch, so incomplete steps must not hold the + card — that hold is what left FN-9193 unfinalizable, and a restart re-created it. */ + const hardBlocker = getMergeConfirmedFinalizationBlocker({ ...task, steps: task.steps ?? [], workflowStepResults: task.workflowStepResults, @@ -12395,8 +12398,10 @@ const movedTask = await this.store.moveTask(task.id, completeLane); mergeTargetSource: mergeTarget.source, }; - /* Wired with THIS card's review lanes — see the note on `ownReviewLanesForAlreadyMerged` above. */ - const hardBlocker = getTaskHardMergeBlocker({ + /* Wired with THIS card's review lanes — see the note on `ownReviewLanesForAlreadyMerged` above. + FNXC:MergeConfirmedFinalization 2026-08-23-21:40 (FN-9193): merged content is already + proven on the base branch here, so incomplete steps must not block finalization. */ + const hardBlocker = getMergeConfirmedFinalizationBlocker({ ...task, steps: task.steps ?? [], workflowStepResults: task.workflowStepResults,