FN-7641: fix cards stranded after out-of-band/workspace merges by allowing proven-merge rehome
Fixes a state-machine bug family where cards got stranded after out-of-band or workspace merges landed: store.moveTask now allows a proven-merge recoveryRehome to cross legacy columns (e.g. todo→done), and nodeId='end' finalize no longer silently no-ops — it finalizes on durable merge proof or returns an explicit error, consistently across the dashboard route, the CLI task-update tool, and store.updateTask. - packages/core/src/store.ts: allow proven-merge recoveryRehome moves across legacy columns (e.g. todo→done) instead of rejecting them - packages/core/src/node-override-guard.ts: nodeId='end' finalize now checks for durable merge proof and returns an explicit error instead of silently no-op'ing - packages/dashboard/src/routes/register-task-workflow-routes.ts: dashboard workflow route surfaces the new explicit finalize error/behavior - packages/cli/src/extension.ts: CLI task-update tool surfaces the same explicit finalize error/behavior - docs/task-management.md: documented the updated finalize/rehome behavior - Added regression tests across core (node-override-guard, store-movement, task-node-override), dashboard (register-task-workflow-routes.nodeid-finalize), engine (merger-merge-lifecycle), and CLI (extension) covering the stranded-card invariant - Added changeset for @runfusion/fusion (patch) Files changed: .changeset/fn-7641-stranded-cards-after-merge.md | 7 ++ docs/task-management.md | 2 + packages/cli/src/__tests__/extension.test.ts | 59 ++++++++++++++ packages/cli/src/extension.ts | 10 +++ .../core/src/__tests__/node-override-guard.test.ts | 93 +++++++++++++++++++++ packages/core/src/__tests__/store-movement.test.ts | 94 ++++++++++++++++++++++ .../core/src/__tests__/task-node-override.test.ts | 73 +++++++++++++++++ packages/core/src/node-override-guard.ts | 69 +++++++++++++++- packages/core/src/store.ts | 69 +++++++++++++++- ...er-task-workflow-routes.nodeid-finalize.test.ts | 90 +++++++++++++++++++++ .../src/routes/register-task-workflow-routes.ts | 10 +++ .../src/__tests__/merger-merge-lifecycle.test.ts | 58 +++++++++++++ 12 files changed, 631 insertions(+), 3 deletions(-) Fusion-Task-Id: FN-7641 Fusion-Task-Lineage: 48ea7851-ee68-48f1-92f9-302d0da5acff Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
7
.changeset/fn-7641-stranded-cards-after-merge.md
Normal file
7
.changeset/fn-7641-stranded-cards-after-merge.md
Normal file
@@ -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.
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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 });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 };
|
||||
}
|
||||
|
||||
@@ -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<string, unknown>; 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<string, unknown> | null; githubTracking?: import("./types.js").TaskGithubTracking | null; gitlabTracking?: (Omit<import("./types.js").TaskGitLabTracking, "item"> & { 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<Task> {
|
||||
/*
|
||||
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: (
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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) {
|
||||
|
||||
@@ -237,6 +237,64 @@ function createMockStore(taskOverrides: Partial<Task> = {}, 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<typeof vi.fn>;
|
||||
moveTask: ReturnType<typeof vi.fn>;
|
||||
emit: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
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",
|
||||
|
||||
Reference in New Issue
Block a user