diff --git a/.changeset/fn-7641-stranded-cards-after-merge.md b/.changeset/fn-7641-stranded-cards-after-merge.md new file mode 100644 index 0000000000..1795922fc5 --- /dev/null +++ b/.changeset/fn-7641-stranded-cards-after-merge.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix cards stranded after workspace/out-of-band merges land; node-override end no longer silently no-ops. +category: fix +dev: store.moveTask now allows proven-merge recoveryRehome from legacy columns (e.g. todo→done); nodeId='end' finalizes on durable merge proof or returns an explicit error across the dashboard route, CLI task-update tool, and store.updateTask. diff --git a/docs/task-management.md b/docs/task-management.md index 11ea9b8497..3ccdc2a402 100644 --- a/docs/task-management.md +++ b/docs/task-management.md @@ -262,6 +262,8 @@ Fusion task columns use persisted enum values as the API/filter contract. Caller - Self-healing can still auto-finalize retry-exhausted failed review tasks when it can prove their branch content already landed on the merge target, so already-merged work does not deadlock in `in-review`. - Repeated engine merge-queue drops now escalate to an explicit recoverable review failure: if auto-recovery hits `Auto-merge starvation:` in the task `error`, Fusion has already seen three consecutive enqueue attempts rejected by the engine merge queue. Operators can recover by clearing the failed state from the dashboard, which lets the usual unpause/clear flow re-attempt merge once the underlying queue wedge is resolved. - Non-recoverable state-machine errors during finalization (for example `Invalid transition: 'todo' → 'done'`) are treated as terminal review failures: recovery must not re-enqueue these tasks for merge unless task state changes prove they are recoverable. + - FNXC:AutoMergeLifecycle 2026-07-07-12:00 (FN-7641): a **proven-merge recovery rehome** is now a recoverable state-machine transition, not a terminal error. `finalizeProvenAutoMergeTask` already verifies `hasDurableMergeProof` + `getTaskHardMergeBlocker` before requesting `done` from a stale legacy column (`todo`/`in-progress`/`triage`) via `store.moveTask(id, "done", { recoveryRehome: true, preserveProgress: true })`; the store now accepts that legacy→legacy recovery rehome (previously rejected as `Invalid transition: 'todo' → 'done'`, stranding a workspace-merge-landed card in `todo` forever — NEXT-010). Only the proven-merge `recoveryRehome` adjacency check is relaxed; the `in-review → done` merge-blocker guard and normal (non-recovery) transitions are unchanged. + - FNXC:StateMachine 2026-07-07-12:00 (FN-7641): setting a task's `nodeId` override to the workflow's terminal `end` node never silently no-ops. With durable merge proof (`mergeDetails.mergeConfirmed === true`) it finalizes the card to `done` via the same recovery-rehome path above; without proof it returns an explicit, actionable error instead of writing the field and leaving the card unchanged (NEXT-322 / NEXT-375 / NEXT-340 — a human/agent merging the branch tip directly into `main`, out-of-band, then setting `nodeId='end'`, used to leave the card silently stuck in `in-review`). This contract is enforced once in `TaskStore.updateTask` / `node-override-guard.ts` and shared identically by the dashboard PATCH `/api/tasks/:id` route and the CLI `fn_task_update` tool. #### Self-healing: stranded-completed-todo recovery diff --git a/packages/cli/src/__tests__/extension.test.ts b/packages/cli/src/__tests__/extension.test.ts index 0daaca28f9..f4b72d6c6b 100644 --- a/packages/cli/src/__tests__/extension.test.ts +++ b/packages/cli/src/__tests__/extension.test.ts @@ -3519,6 +3519,65 @@ describe("fn pi extension (runnable structured-output regression slice)", () => ); expect(show.details.task.nodeId).toBeUndefined(); }); + + // FNXC:StateMachine 2026-07-07-12:00: FN-7641 Signature 2 CLI regression — nodeId='end' + // must finalize-on-proof or return an explicit isError, never a silent "Updated" no-op + // (NEXT-322 / NEXT-375 / NEXT-340). + it("finalizes an in-review task to done when setting nodeId='end' with merge proof", async () => { + const store = new TaskStore(tmpDir); + await store.init(); + const task = await store.createTask({ description: "out-of-band merge repro" }); + await store.updateTask(task.id, { steps: [{ name: "Only step", status: "done" }] }); + await store.moveTask(task.id, "todo"); + await store.moveTask(task.id, "in-progress"); + await store.moveTask(task.id, "in-review"); + await store.updateTask(task.id, { mergeDetails: { mergeConfirmed: true } }); + store.close(); + + const updateTool = api.tools.get("fn_task_update")!; + const result = await updateTool.execute( + "finalize-node-end", + { id: task.id, nodeId: "end" }, + undefined, + undefined, + makeCtx(tmpDir), + ); + + expect(result.isError).not.toBe(true); + + const showTool = api.tools.get("fn_task_show")!; + const show = await showTool.execute("show-finalized", { id: task.id }, undefined, undefined, makeCtx(tmpDir)); + expect(show.details.task.column).toBe("done"); + expect(show.details.task.nodeId).toBe("end"); + }); + + it("returns an explicit isError instead of a silent no-op when setting nodeId='end' without merge proof", async () => { + const store = new TaskStore(tmpDir); + await store.init(); + const task = await store.createTask({ description: "no proof repro" }); + await store.updateTask(task.id, { steps: [{ name: "Only step", status: "done" }] }); + await store.moveTask(task.id, "todo"); + await store.moveTask(task.id, "in-progress"); + await store.moveTask(task.id, "in-review"); + store.close(); + + const updateTool = api.tools.get("fn_task_update")!; + const result = await updateTool.execute( + "reject-node-end", + { id: task.id, nodeId: "end" }, + undefined, + undefined, + makeCtx(tmpDir), + ); + + expect(result.isError).toBe(true); + expect(result.content[0].text.toLowerCase()).toContain("merge"); + + const showTool = api.tools.get("fn_task_show")!; + const show = await showTool.execute("show-rejected", { id: task.id }, undefined, undefined, makeCtx(tmpDir)); + expect(show.details.task.column).toBe("in-review"); + expect(show.details.task.nodeId).toBeUndefined(); + }); }); describe("fn_task_retry", () => { diff --git a/packages/cli/src/extension.ts b/packages/cli/src/extension.ts index 85b4d33ce8..2fd38b1825 100644 --- a/packages/cli/src/extension.ts +++ b/packages/cli/src/extension.ts @@ -971,6 +971,16 @@ export default function kbExtension(pi: ExtensionAPI) { updatedFields.push("agentId"); } if (params.nodeId !== undefined) { + /* + FNXC:StateMachine 2026-07-07-12:00: + Signature 2 (FN-7641 / NEXT-322 / NEXT-375 / NEXT-340): nodeId='end' after an + out-of-band merge must never silently no-op here either. Pre-validate exactly like + the dashboard route so the CLI tool returns an explicit isError instead of a "success" + response that changed nothing when there is no durable merge proof. When proof exists, + `store.updateTask` below performs the real finalize-to-done move (shared logic in + TaskStore.updateTask / node-override-guard.ts), so this tool, the dashboard route, and + store.updateTask all exhibit identical behavior. + */ const normalizedNodeId = normalizeNullableStringInput(params.nodeId); const validation = validateNodeOverrideChange(task, normalizedNodeId ?? null); if (!validation.allowed) { diff --git a/packages/core/src/__tests__/node-override-guard.test.ts b/packages/core/src/__tests__/node-override-guard.test.ts index 273482f847..06f18e4e60 100644 --- a/packages/core/src/__tests__/node-override-guard.test.ts +++ b/packages/core/src/__tests__/node-override-guard.test.ts @@ -89,4 +89,97 @@ describe("validateNodeOverrideChange", () => { ); expect(result.allowed).toBe(false); }); + + // FNXC:StateMachine 2026-07-07-12:00: Signature 2 (FN-7641 / NEXT-322 / NEXT-375 / NEXT-340) + // regression — nodeId='end' must finalize-on-proof or return an explicit error, never a + // silent no-op. Covers in-review with/without merge proof, non-terminal overrides unchanged, + // clearing the override unchanged, and the still-enforced in-progress guard. + describe("terminal 'end' node override (FN-7641 Signature 2)", () => { + it("REPRO: signals requiresFinalize instead of a silent allow for in-review + merge proof", () => { + const result = validateNodeOverrideChange( + { id: "FN-322", column: "in-review", mergeDetails: { mergeConfirmed: true } }, + "end", + ); + expect(result.allowed).toBe(true); + expect(result.requiresFinalize).toBe(true); + }); + + it("REPRO: rejects nodeId='end' with an explicit error when there is NO merge proof (never silent)", () => { + const result = validateNodeOverrideChange( + { id: "FN-322", column: "in-review" }, + "end", + ); + expect(result.allowed).toBe(false); + expect(result.reason).toBe("terminal-without-merge-proof"); + expect(result.message).toContain("FN-322"); + expect(result.message).toContain("nodeId='end'"); + expect(result.message?.toLowerCase()).toContain("merge"); + }); + + it("rejects nodeId='end' with explicit error when mergeConfirmed is explicitly false", () => { + const result = validateNodeOverrideChange( + { id: "FN-375", column: "in-review", mergeDetails: { mergeConfirmed: false } }, + "end", + ); + expect(result.allowed).toBe(false); + expect(result.reason).toBe("terminal-without-merge-proof"); + }); + + it("allows nodeId='end' as a no-op when the task is already done, even without merge proof", () => { + const result = validateNodeOverrideChange({ id: "FN-340", column: "done" }, "end"); + expect(result).toEqual({ allowed: true }); + }); + + it("does not gate non-terminal nodeId overrides even with no merge proof", () => { + const result = validateNodeOverrideChange( + { id: "FN-1", column: "in-review" }, + "plan-review", + ); + expect(result).toEqual({ allowed: true }); + }); + + it("does not gate clearing the override (null) even on a terminal-eligible task with no proof", () => { + const result = validateNodeOverrideChange( + { id: "FN-1", column: "in-review", nodeId: "end" }, + null, + ); + expect(result).toEqual({ allowed: true }); + }); + + it("still blocks in-progress tasks before the terminal-node check runs", () => { + const result = validateNodeOverrideChange( + { id: "FN-1", column: "in-progress", mergeDetails: { mergeConfirmed: true } }, + "end", + ); + expect(result.allowed).toBe(false); + expect(result.reason).toBe("task-in-progress"); + }); + + it("uses a caller-supplied isTerminalNodeId resolver instead of the literal 'end' fallback", () => { + const isTerminalNodeId = (nodeId: string) => nodeId === "custom-terminal"; + + const noProof = validateNodeOverrideChange( + { id: "FN-1", column: "in-review" }, + "custom-terminal", + { isTerminalNodeId }, + ); + expect(noProof.allowed).toBe(false); + expect(noProof.reason).toBe("terminal-without-merge-proof"); + + const literalEndNotTerminalHere = validateNodeOverrideChange( + { id: "FN-1", column: "in-review" }, + "end", + { isTerminalNodeId }, + ); + expect(literalEndNotTerminalHere).toEqual({ allowed: true }); + }); + + it("todo/in-progress non-terminal cards with merge proof are unaffected by the terminal gate", () => { + const result = validateNodeOverrideChange( + { id: "FN-1", column: "todo", mergeDetails: { mergeConfirmed: true } }, + "execute", + ); + expect(result).toEqual({ allowed: true }); + }); + }); }); diff --git a/packages/core/src/__tests__/store-movement.test.ts b/packages/core/src/__tests__/store-movement.test.ts index 8757fd124c..e52e2127d9 100644 --- a/packages/core/src/__tests__/store-movement.test.ts +++ b/packages/core/src/__tests__/store-movement.test.ts @@ -1081,5 +1081,99 @@ describe("TaskStore", () => { }); }); + // FNXC:AutoMergeLifecycle 2026-07-07-12:00: Signature 1 (FN-7641 / NEXT-010) regression — + // a proven-merge recoveryRehome move from a legacy source column to `done` must succeed + // instead of throwing "Invalid transition: 'todo' -> 'done'". Covers every legacy source + // column the CAS/queue-retry paths can leave a landed task parked in (todo, in-progress, + // triage), while confirming normal (non-recovery) moves still reject illegal adjacency. + describe("moveTask — legacy proven-merge recoveryRehome to done (FN-7641)", () => { + it("RED-FIRST: reproduces the original 'todo -> done' invalid-transition symptom without recoveryRehome", async () => { + const task = await store.createTask({ description: "repro NEXT-010 without recovery flag" }); + await store.moveTask(task.id, "todo"); + await store.updateTask(task.id, { mergeDetails: { mergeConfirmed: true } }); + + await expect(store.moveTask(task.id, "done")).rejects.toThrow( + "Invalid transition: 'todo' → 'done'", + ); + }); + + it("allows a legacy todo -> done recoveryRehome move for a proven-merge task (non-workflow)", async () => { + const task = await store.createTask({ description: "NEXT-010 workspace merge finalize repro" }); + await store.moveTask(task.id, "todo"); + await store.updateTask(task.id, { mergeDetails: { mergeConfirmed: true } }); + + const moved = await store.moveTask(task.id, "done", { + recoveryRehome: true, + preserveProgress: true, + }); + + expect(moved.column).toBe("done"); + }); + + it("allows a legacy in-progress -> done recoveryRehome move for a proven-merge task", async () => { + const task = await store.createTask({ description: "NEXT-010 in-progress recovery repro" }); + await store.moveTask(task.id, "todo"); + await store.moveTask(task.id, "in-progress"); + await store.updateTask(task.id, { mergeDetails: { mergeConfirmed: true } }); + + const moved = await store.moveTask(task.id, "done", { + recoveryRehome: true, + preserveProgress: true, + }); + + expect(moved.column).toBe("done"); + }); + + it("allows a legacy triage -> done recoveryRehome move for a proven-merge task", async () => { + const task = await store.createTask({ description: "NEXT-010 triage recovery repro" }); + await store.updateTask(task.id, { mergeDetails: { mergeConfirmed: true } }); + + const moved = await store.moveTask(task.id, "done", { + recoveryRehome: true, + preserveProgress: true, + }); + + expect(moved.column).toBe("done"); + }); + + it("still allows the normal in-review -> done recovery path unchanged", async () => { + const task = await store.createTask({ description: "normal in-review recovery unaffected" }); + await store.moveTask(task.id, "todo"); + await store.moveTask(task.id, "in-progress"); + await store.moveTask(task.id, "in-review"); + await store.updateTask(task.id, { mergeDetails: { mergeConfirmed: true } }); + + const moved = await store.moveTask(task.id, "done", { + recoveryRehome: true, + preserveProgress: true, + }); + + expect(moved.column).toBe("done"); + }); + + it("does NOT relax adjacency for a normal (non-recovery) todo -> done move", async () => { + const task = await store.createTask({ description: "non-recovery move must still reject" }); + await store.moveTask(task.id, "todo"); + await store.updateTask(task.id, { mergeDetails: { mergeConfirmed: true } }); + + await expect(store.moveTask(task.id, "done")).rejects.toThrow("Invalid transition"); + }); + + it("allows a legacy todo -> done recoveryRehome move for a custom-workflow task", async () => { + const task = await store.createTask({ + description: "NEXT-010 custom workflow recovery repro", + workflowId: "builtin:coding", + }); + await store.moveTask(task.id, "todo"); + await store.updateTask(task.id, { mergeDetails: { mergeConfirmed: true } }); + + const moved = await store.moveTask(task.id, "done", { + recoveryRehome: true, + preserveProgress: true, + }); + + expect(moved.column).toBe("done"); + }); + }); }); diff --git a/packages/core/src/__tests__/task-node-override.test.ts b/packages/core/src/__tests__/task-node-override.test.ts index 6d430a0284..77ac26402d 100644 --- a/packages/core/src/__tests__/task-node-override.test.ts +++ b/packages/core/src/__tests__/task-node-override.test.ts @@ -127,4 +127,77 @@ describe("task node override persistence", () => { expect((await store.getTask(second.id)).nodeId).toBe("node-beta"); expect((await store.getTask(third.id)).nodeId).toBeUndefined(); }); + + // FNXC:StateMachine 2026-07-07-12:00: Signature 2 (FN-7641) end-to-end regression through the + // real store.updateTask surface (not just the pure guard) — nodeId='end' must finalize-on-proof + // or error, never silently no-op, for both non-workflow and custom-workflow tasks. + describe("nodeId='end' finalize-on-proof-or-error (FN-7641 Signature 2)", () => { + it("REPRO: advances an in-review task with all steps done + merge proof to done instead of no-op", async () => { + const created = await store.createTask({ description: "NEXT-322 out-of-band merge repro" }); + await store.updateTask(created.id, { steps: [{ name: "Only step", status: "done" }] }); + await store.moveTask(created.id, "todo"); + await store.moveTask(created.id, "in-progress"); + await store.moveTask(created.id, "in-review"); + await store.updateTask(created.id, { mergeDetails: { mergeConfirmed: true } }); + + const updated = await store.updateTask(created.id, { nodeId: "end" }); + + expect(updated.column).toBe("done"); + expect(updated.nodeId).toBe("end"); + }); + + it("REPRO: rejects nodeId='end' with an explicit error when there is no merge proof (never a silent no-op)", async () => { + const created = await store.createTask({ description: "NEXT-340 no proof repro" }); + await store.updateTask(created.id, { steps: [{ name: "Only step", status: "done" }] }); + await store.moveTask(created.id, "todo"); + await store.moveTask(created.id, "in-progress"); + await store.moveTask(created.id, "in-review"); + + await expect(store.updateTask(created.id, { nodeId: "end" })).rejects.toThrow( + "does not finalize a card by itself", + ); + + const unchanged = await store.getTask(created.id); + expect(unchanged.column).toBe("in-review"); + expect(unchanged.nodeId).toBeUndefined(); + }); + + it("advances a custom-workflow task (builtin:coding) with merge proof identically", async () => { + const created = await store.createTask({ + description: "custom workflow finalize repro", + workflowId: "builtin:coding", + }); + await store.updateTask(created.id, { steps: [{ name: "Only step", status: "done" }] }); + await store.moveTask(created.id, "todo"); + await store.moveTask(created.id, "in-progress"); + await store.moveTask(created.id, "in-review"); + await store.updateTask(created.id, { mergeDetails: { mergeConfirmed: true } }); + + const updated = await store.updateTask(created.id, { nodeId: "end" }); + + expect(updated.column).toBe("done"); + }); + + it("is a true no-op (no throw, stays done) when the task is already done", async () => { + const created = await store.createTask({ description: "already done repro" }); + await store.moveTask(created.id, "todo"); + await store.moveTask(created.id, "in-progress"); + await store.moveTask(created.id, "in-review"); + await store.moveTask(created.id, "done"); + + const updated = await store.updateTask(created.id, { nodeId: "end" }); + + expect(updated.column).toBe("done"); + expect(updated.nodeId).toBe("end"); + }); + + it("leaves the existing in-progress guard unchanged for a terminal nodeId with merge proof", async () => { + const created = await store.createTask({ description: "in-progress guard unaffected" }); + await store.moveTask(created.id, "todo"); + await store.moveTask(created.id, "in-progress"); + await store.updateTask(created.id, { mergeDetails: { mergeConfirmed: true } }); + + await expect(store.updateTask(created.id, { nodeId: "end" })).rejects.toThrow("in progress"); + }); + }); }); diff --git a/packages/core/src/node-override-guard.ts b/packages/core/src/node-override-guard.ts index 122a2df149..2911c387b2 100644 --- a/packages/core/src/node-override-guard.ts +++ b/packages/core/src/node-override-guard.ts @@ -1,14 +1,50 @@ -export type NodeOverrideBlockReason = "task-in-progress"; +export type NodeOverrideBlockReason = "task-in-progress" | "terminal-without-merge-proof"; export interface NodeOverrideValidationResult { allowed: boolean; reason?: NodeOverrideBlockReason; message?: string; + /** + * FNXC:StateMachine 2026-07-07-12:00: + * True when `newNodeId` resolves to the task workflow's terminal `end` node, + * the task is not already `done`, AND durable merge proof already exists + * (`mergeDetails.mergeConfirmed === true`). Callers MUST route this case + * through a finalize-to-done move (e.g. `store.moveTask(id, 'done', { + * recoveryRehome: true, preserveProgress: true })`) instead of writing + * `nodeId` as a bare field — a bare field write is exactly the Signature-2 + * silent no-op this flag exists to prevent (FN-7641 / NEXT-322 / NEXT-375 / + * NEXT-340: a human/agent merges the branch tip directly into `main`, then + * `nodeId='end'` is set and the card silently stays in `in-review` forever). + */ + requiresFinalize?: boolean; } +export interface NodeOverrideTaskInput { + column: string; + nodeId?: string; + id: string; + mergeDetails?: { mergeConfirmed?: boolean } | null; +} + +export interface NodeOverrideValidationOptions { + /** + * Resolve whether `nodeId` is the task workflow's terminal `end` node. + * Callers with access to the task's resolved workflow IR (e.g. + * `TaskStore`) should pass a real resolver keyed off `node.kind === "end"`. + * Callers without cheap IR access (dashboard route, CLI tool) may omit this + * — the default fallback below still catches the literal `nodeId === "end"` + * id used by every built-in workflow's terminal node, which covers the + * exact reported symptom and the common case. + */ + isTerminalNodeId?: (nodeId: string) => boolean; +} + +const defaultIsTerminalNodeId = (nodeId: string): boolean => nodeId === "end"; + export function validateNodeOverrideChange( - task: { column: string; nodeId?: string; id: string }, + task: NodeOverrideTaskInput, newNodeId: string | null | undefined, + options?: NodeOverrideValidationOptions, ): NodeOverrideValidationResult { if (newNodeId === undefined) { return { allowed: true }; @@ -22,5 +58,34 @@ export function validateNodeOverrideChange( }; } + /* + FNXC:StateMachine 2026-07-07-12:00: + Signature 2 (FN-7641 / NEXT-322 / NEXT-375 / NEXT-340): setting nodeId='end' after work + merged out-of-band (bypassing the merge node) must never silently no-op. Before this fix + the field was written verbatim and the card stayed wherever it was (e.g. in-review with + all steps done) with no error and no advancement. Resolve the intent explicitly instead: + a terminal `end` override with durable merge proof finalizes the card (requiresFinalize); + a terminal `end` override with NO merge proof is rejected with an actionable error so the + caller knows to confirm the merge first. Non-terminal nodeId overrides and clearing the + override (newNodeId === null) are untouched — this only gates the terminal-node case. + */ + const isTerminal = + newNodeId !== null && + (options?.isTerminalNodeId ? options.isTerminalNodeId(newNodeId) : defaultIsTerminalNodeId(newNodeId)); + if (isTerminal && task.column !== "done") { + const mergeConfirmed = task.mergeDetails?.mergeConfirmed === true; + if (mergeConfirmed) { + return { allowed: true, requiresFinalize: true }; + } + return { + allowed: false, + reason: "terminal-without-merge-proof", + message: + `Cannot set node override to '${newNodeId}' for ${task.id}: setting nodeId='end' does not finalize a card by itself. ` + + `This task has no durable merge proof (mergeDetails.mergeConfirmed is not true), so the workflow finalize path was not applied and the card was left unchanged rather than silently no-op. ` + + `If the work already merged out-of-band, confirm the merge (record mergeDetails.mergeConfirmed=true via the merge-confirm/reconcile path) and retry, or move the task to done through the normal review/merge flow instead of overriding nodeId directly.`, + }; + } + return { allowed: true }; } diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index 2958360fb8..5213620bd3 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -7675,7 +7675,28 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS} options?.recoveryRehome === true && !sourceIsLegacy && (COLUMNS as readonly string[]).includes(toColumn); - if (!isEvacuation) { + /* + FNXC:AutoMergeLifecycle 2026-07-07-12:00: + Signature 1 (FN-7641 / NEXT-010): a proven-merge recovery rehome can also run + LEGACY -> LEGACY (e.g. `todo -> done` when finalizeProvenAutoMergeTask reaches a + task whose column drifted to `todo`/`in-progress`/`triage` before workspace-merge + finalization runs). VALID_TRANSITIONS['todo'] never lists 'done' -- that adjacency + graph encodes the NORMAL flow, not proven-merge recovery -- so the legacy adjacency + check below rejected the finalizer's `store.moveTask(id, 'done', { recoveryRehome: + true, preserveProgress: true })` call with "Invalid transition: 'todo' -> 'done'. + Valid targets: in-progress, triage, archived", stranding the card in `todo` forever + even though `finalizeProvenAutoMergeTask` already verified `hasDurableMergeProof` + and `getTaskHardMergeBlocker` before calling moveTask. Bypass ONLY the adjacency + check for a recoveryRehome move between two legacy columns; the merge-blocker guard + below (fromColumn === 'in-review' && toColumn === 'done') and the finalizer's own + hard-blocker gate are untouched, so non-recovery moves and genuine merge blockers + are not weakened. + */ + const isLegacyRecoveryRehome = + options?.recoveryRehome === true && + sourceIsLegacy && + (COLUMNS as readonly string[]).includes(toColumn); + if (!isEvacuation && !isLegacyRecoveryRehome) { /* FNXC:WorkflowColumns 2026-07-05-19:30: Workflow columns graduated to always-on (no experimental flag emitted), so this "flag-OFF" @@ -8440,9 +8461,55 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS} updates: { title?: string; description?: string; priority?: TaskPriority | null; prompt?: string; worktree?: string | null; workspaceWorktrees?: import("./types.js").Task["workspaceWorktrees"]; status?: string | null; dependencies?: string[]; steps?: import("./types.js").TaskStep[]; customFields?: Record; currentStep?: number; blockedBy?: string | null; overlapBlockedBy?: string | null; assignedAgentId?: string | null; pausedByAgentId?: string | null; pausedReason?: string | null; tokenBudgetSoftAlertedAt?: string | null; worktrunkFallbackAlertedAt?: string | null; worktrunkFailure?: import("./types.js").Task["worktrunkFailure"] | null; tokenBudgetHardAlertedAt?: string | null; tokenBudgetOverride?: import("./types.js").TaskTokenBudgetOverride | null; dispatchStormCount?: number | null; lastDispatchAt?: string | null; assigneeUserId?: string | null; scopeOverride?: boolean | null; scopeOverrideReason?: string | null; scopeAutoWiden?: string[] | null; nodeId?: string | null; effectiveNodeId?: string | null; effectiveNodeSource?: string | null; checkedOutBy?: string | null; checkedOutAt?: string | null; checkoutNodeId?: string | null; checkoutRunId?: string | null; checkoutLeaseRenewedAt?: string | null; checkoutLeaseEpoch?: number | null; paused?: boolean; baseBranch?: string | null; autoMerge?: boolean | null; branch?: string | null; executionStartBranch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; executionMode?: import("./types.js").ExecutionMode | null; plannerOversightLevel?: import("./types.js").PlannerOversightLevel | null; awaitingApprovalReason?: import("./types.js").Task["awaitingApprovalReason"] | null; approvedPlanFingerprint?: string | null; mergeRetries?: number; workflowStepRetries?: number; stuckKillCount?: number | null; resumeLimboCount?: number | null; graphResumeRetryCount?: number | null; resumeLimboTipSha?: string | null; resumeLimboStepSignature?: string | null; postReviewFixCount?: number | null; recoveryRetryCount?: number | null; taskDoneRetryCount?: number | null; worktreeSessionRetryCount?: number | null; completionHandoffLimboRecoveryCount?: number | null; verificationFailureCount?: number | null; mergeConflictBounceCount?: number | null; mergeAuditBounceCount?: number | null; mergeTransientRetryCount?: number | null; branchConflictRecoveryCount?: number | null; reviewerContextRetryCount?: number | null; reviewerFallbackRetryCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; noCommitsExpected?: boolean | null; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; planningModelProvider?: string | null; planningModelId?: string | null; thinkingLevel?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; firstExecutionAt?: string | null; cumulativeActiveMs?: number | null; executionStartedAt?: string | null; executionCompletedAt?: string | null; review?: import("./types.js").TaskReview | null; reviewState?: import("./types.js").TaskReviewState | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; mergeDetails?: import("./types.js").MergeDetails | null; sourceIssue?: import("./types.js").TaskSourceIssue | null; sourceMetadataPatch?: Record | null; githubTracking?: import("./types.js").TaskGithubTracking | null; gitlabTracking?: (Omit & { item?: import("./types.js").TaskGitLabTrackedItem | null }) | null; tokenUsage?: import("./types.js").TaskTokenUsage | null; modifiedFiles?: string[] | null; workflowTransitionNotification?: import("./types.js").Task["workflowTransitionNotification"] | null; missionId?: string | null; sliceId?: string | null }, runContext?: RunMutationContext, ): Promise { + /* + FNXC:StateMachine 2026-07-07-12:00: + Signature 2 (FN-7641): resolve the nodeId='end' finalize-on-proof-or-error contract ONCE here so + the dashboard route, CLI task-update tool, and any other updateTask caller share identical + behavior via this single choke point. Read the current task and check BEFORE acquiring the + per-task lock (getTask/moveTask each acquire their own lock; nesting inside withTaskLock would + deadlock since the lock is non-reentrant). A terminal-node override with durable merge proof + finalizes the card to done via the Signature-1 recovery rehome; without proof it throws an + explicit error instead of letting updateTaskUnlocked write a no-op nodeId field. + */ + if (updates.nodeId !== undefined) { + const currentTask = await this.getTask(id).catch(() => null); + if (currentTask) { + const validation = validateNodeOverrideChange(currentTask, updates.nodeId ?? null, { + isTerminalNodeId: (nodeId) => this.isTaskTerminalNodeId(id, nodeId), + }); + if (!validation.allowed) { + throw new Error(validation.message); + } + if (validation.requiresFinalize) { + await this.moveTask(id, "done", { + moveSource: "engine", + recoveryRehome: true, + preserveProgress: true, + }); + } + } + } return this.withTaskLock(id, () => this.updateTaskUnlocked(id, updates, runContext)); } + /** + * FNXC:StateMachine 2026-07-07-12:00: + * Resolve whether `nodeId` is the task's resolved workflow terminal `end` node (kind === "end"), + * for the nodeId='end' finalize-on-proof-or-error contract (FN-7641 Signature 2). Falls back to + * the literal id check when the workflow IR cannot be resolved or does not contain the node, which + * still matches every built-in workflow's terminal node id. + */ + private isTaskTerminalNodeId(taskId: string, nodeId: string): boolean { + try { + const ir = this.resolveTaskWorkflowIrSync(taskId); + const node = ir.nodes.find((n) => n.id === nodeId); + if (node) return node.kind === "end"; + } catch { + // Fall through to the literal-id fallback below. + } + return nodeId === "end"; + } + async updateTaskAtomic( id: string, updater: ( diff --git a/packages/dashboard/src/routes/__tests__/register-task-workflow-routes.nodeid-finalize.test.ts b/packages/dashboard/src/routes/__tests__/register-task-workflow-routes.nodeid-finalize.test.ts new file mode 100644 index 0000000000..433263fbc1 --- /dev/null +++ b/packages/dashboard/src/routes/__tests__/register-task-workflow-routes.nodeid-finalize.test.ts @@ -0,0 +1,90 @@ +// @vitest-environment node +// +// FNXC:StateMachine 2026-07-07-12:00: +// FN-7641 Signature 2 route-level regression: PATCH /api/tasks/:id with +// nodeId='end' must never silently no-op. (a) with durable merge proof the +// card advances to done; (b) without merge proof the route returns an +// explicit non-2xx error; (c) neither case is a silent 2xx no-op that leaves +// the card exactly where it started with no error and no advancement +// (NEXT-322 / NEXT-375 / NEXT-340). + +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import express from "express"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { TaskStore } from "@fusion/core"; +import { createApiRoutes } from "../../routes.js"; +import { request as REQUEST } from "../../test-request.js"; + +describe("PATCH /tasks/:id nodeId='end' finalize-on-proof-or-error (FN-7641)", () => { + let store: TaskStore; + let rootDir: string; + let globalDir: string; + let app: express.Express; + + beforeEach(async () => { + rootDir = mkdtempSync(join(tmpdir(), "nodeid-finalize-root-")); + globalDir = mkdtempSync(join(tmpdir(), "nodeid-finalize-global-")); + store = new TaskStore(rootDir, globalDir, { inMemoryDb: true }); + await store.init(); + app = express(); + app.use(express.json()); + app.use("/api", createApiRoutes(store)); + }); + + afterEach(() => { + store.close(); + rmSync(rootDir, { recursive: true, force: true }); + rmSync(globalDir, { recursive: true, force: true }); + }); + + const patch = (path: string, body: unknown) => + REQUEST(app, "PATCH", path, JSON.stringify(body), { "content-type": "application/json" }); + + async function taskInReviewWithSteps() { + const task = await store.createTask({ description: "out-of-band merge repro" }); + await store.updateTask(task.id, { steps: [{ name: "Only step", status: "done" }] }); + await store.moveTask(task.id, "todo"); + await store.moveTask(task.id, "in-progress"); + await store.moveTask(task.id, "in-review"); + return task; + } + + it("(a) advances to done when merge proof exists — never a silent no-op", async () => { + const task = await taskInReviewWithSteps(); + await store.updateTask(task.id, { mergeDetails: { mergeConfirmed: true } }); + + const res = await patch(`/api/tasks/${task.id}`, { nodeId: "end" }); + + expect(res.status).toBe(200); + const body = res.body as { id: string; column: string; nodeId?: string }; + expect(body.column).toBe("done"); + expect(body.nodeId).toBe("end"); + }); + + it("(b) returns an explicit non-2xx error when there is no merge proof — never a silent no-op", async () => { + const task = await taskInReviewWithSteps(); + + const res = await patch(`/api/tasks/${task.id}`, { nodeId: "end" }); + + expect(res.status).not.toBe(200); + expect(res.status).toBeGreaterThanOrEqual(400); + expect(res.status).toBeLessThan(500); + + // (c) confirm the card was NOT silently advanced or mutated by the rejected request. + const unchanged = await store.getTask(task.id); + expect(unchanged.column).toBe("in-review"); + expect(unchanged.nodeId).toBeUndefined(); + }); + + it("leaves the existing in-progress guard behavior unchanged (still blocks, still 409)", async () => { + const task = await store.createTask({ description: "in-progress guard" }); + await store.moveTask(task.id, "todo"); + await store.moveTask(task.id, "in-progress"); + + const res = await patch(`/api/tasks/${task.id}`, { nodeId: "some-node" }); + + expect(res.status).toBe(409); + }); +}); diff --git a/packages/dashboard/src/routes/register-task-workflow-routes.ts b/packages/dashboard/src/routes/register-task-workflow-routes.ts index e4b93c4206..47c64836c6 100644 --- a/packages/dashboard/src/routes/register-task-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-task-workflow-routes.ts @@ -4306,6 +4306,16 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork updates.sourceMetadataPatch = { nearDuplicateDismissed: true }; } + /* + FNXC:WorkflowRouting 2026-07-07-12:00: + Signature 2 (FN-7641 / NEXT-322 / NEXT-375 / NEXT-340): setting nodeId='end' after a + human/agent merges the branch tip directly into main (out-of-band, bypassing the merge + node) must never silently no-op. Pre-validate here so the caller gets an explicit 409 + instead of a 200 that changed nothing when there is no durable merge proof. When proof + exists (`validation.allowed === true`), `scopedStore.updateTask` below performs the real + finalize-to-done move itself (shared logic in TaskStore.updateTask / node-override-guard.ts) + so this route, the CLI task-update tool, and store.updateTask all exhibit identical behavior. + */ if (hasBodyField("nodeId") && validatedNodeId !== undefined) { const currentTask = await scopedStore.getTask(req.params.id); if (!currentTask) { diff --git a/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts b/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts index eb5a2480e0..e8a4cdcd2f 100644 --- a/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts +++ b/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts @@ -237,6 +237,64 @@ function createMockStore(taskOverrides: Partial = {}, allTasks: Task[] = [ } describe("auto-merge proven finalization helper", () => { + /* + * FNXC:AutoMergeLifecycle 2026-07-07-12:00: + * Signature 1 (FN-7641 / NEXT-010) regression: a WORKSPACE merge ("AI merge (workspace): all + * N sub-repo(s) landed") reaches finalizeTask -> finalizeProvenAutoMergeTask the same as a + * direct merge, so the recovery-rehome move from a stranded `todo` row must succeed instead of + * throwing "Invalid transition: 'todo' -> 'done'". This exercises the workspace source label + * explicitly (the prior test below already covers the direct-ai-merge source label). + */ + it("reconciles a workspace-merge-landed todo row without invalid todo-to-done transition (NEXT-010)", async () => { + const strandedTask = { + id: "FN-WS-010", + title: "Workspace merge stranded in todo", + description: "Test", + column: "todo", + status: "queued", + error: "Invalid transition: 'todo' → 'done'. Valid targets: in-progress, triage", + blockedBy: null, + overlapBlockedBy: null, + dependencies: [], + steps: [{ status: "done" }], + currentStep: 0, + log: [{ action: "AI merge (workspace): all 2 sub-repo(s) landed" }], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + mergeDetails: { + mergeConfirmed: true, + commitSha: "workspace-landed-sha", + mergedAt: "2026-07-07T12:00:00.000Z", + landedFiles: ["packages/a/file.ts", "packages/b/file.ts"], + }, + } as Task; + const doneTask = { ...strandedTask, column: "done", status: null, error: null } as Task; + const store = createMockStore(strandedTask) as unknown as TaskStore & { + getTask: ReturnType; + moveTask: ReturnType; + emit: ReturnType; + }; + store.getTask.mockResolvedValue(strandedTask); + store.moveTask.mockResolvedValue(doneTask); + + const result = await finalizeProvenAutoMergeTask({ + store, + taskId: "FN-WS-010", + result: { task: strandedTask, ok: true, merged: true, commitSha: "workspace-landed-sha", mergeConfirmed: true } as MergeResult, + source: "workflow-graph-merge-finalize", + auditAgentId: "merger", + auditPhase: "workspace-merge-finalize", + }); + + expect(result.outcome).toBe("done"); + expect(result.task?.column).toBe("done"); + expect(store.moveTask).toHaveBeenCalledWith( + "FN-WS-010", + "done", + expect.objectContaining({ moveSource: "engine", recoveryRehome: true, preserveProgress: true }), + ); + }); + it("reconciles a landed merge-confirmed todo row without invalid todo-to-done transition", async () => { const strandedTask = { id: "FN-6897",