FUSI-036

packages/core/src/__tests__/live-move-path-undeclared-target.test.ts

@@ -39,6 +39,7 @@ import {
 } from "../__test-utils__/pg-test-harness.js";
 import { resolveWorkflowIrForTask } from "../workflows/workflow-ir-resolver.js";
 import { workflowHasColumn } from "../workflows/workflow-transitions.js";
+import { resolveContainedBackwardTargetForTask } from "../workflows/workflow-lifecycle-traits.js";
 
 pgDescribe("live move path — which targets it accepts after the Planning merge", () => {
   const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
@@ -158,7 +159,7 @@ pgDescribe("live move path — which targets it accepts after the Planning merge
     expect(await column(task.id)).toBe("archived");
   });
 
-  it("still allows a recovery re-home to reach the workflow's REBOUND TARGET past adjacency", async () => {
+  it("REFUSES a recovery re-home out of a terminal lane, and still reaches a declared rebound target", async () => {
     /*
     FNXC:MergedPlanningColumn 2026-07-30-10:20 (PR #2601 review — greptile P2):
 
@@ -179,10 +180,39 @@ pgDescribe("live move path — which targets it accepts after the Planning merge
     the literals on the conversion backlog, so that is bug-compatibility, not an
     invariant, and pinning it would cement the bug.
 
-    What recovery genuinely needs, and what is pinned instead: a recovery re-home
-    reaches the workflow's declared rebound target even from a column ADJACENCY
-    would refuse to leave. `done -> todo` is rejected by the legacy table and must
-    still succeed under `recoveryRehome`.
+    What recovery genuinely needs, and what is pinned below instead: a stranded
+    card reaches a target the workflow DECLARES, even from a column ADJACENCY
+    would refuse to leave. That reach is the contained resolver's job now, not a
+    raw `recoveryRehome` store move — the FUSI-036 note below says why.
+
+    FNXC:LifecycleContainment 2026-09-26-00:00 (FUSI-036 — census family
+    `lifecycle-transition-forbidden`, F2):
+
+    RECOVERYREHOME IS AN ADJACENCY BYPASS, NEVER A CONTAINMENT BYPASS. This case
+    was the second census row of that family: it asked a real PG store to perform
+    `archived (rank 5) -> todo (hold, rank 1)` under `recoveryRehome: true` and
+    expected it to SUCCEED, which is a rank gap of 4.
+
+    The refusal is the contract, not a regression to relax. `recoveryRehome` skips
+    exactly two things in `task-store/moves.ts` — the unknown-column rejection for a
+    legacy recovery target, and the column-graph adjacency check — because the card
+    is already stranded in a lane its workflow may not declare. It never reaches the
+    structural deny-list, which `evaluateTransitionInvariants` runs independently of
+    every bypass flag (moves.ts:743, with the FNXC note at :736). F2 fires on the
+    rank gap; F4 would independently forbid leaving a terminal lane. No
+    `lifecycleReason` can authorize either, because the deny-list is evaluated
+    BEFORE reason registration: "an engine reason may explain a legal step backward
+    but can never authorize a structurally forbidden route"
+    (workflow-lifecycle-direction.ts:62-66). Relaxing this assertion to make it
+    green would be the barred appeasement, in the costume of a test fix.
+
+    So the two conflated claims are split. The STORE half now pins the refusal and
+    that the card did not move. The CAPABILITY half the case was written to protect
+    — a stranded card reaching a target the workflow declares — now goes through the
+    contained resolver (`resolveContainedBackwardTargetForTask`), which is the seam
+    FN-207 actually introduced for it. The engine-side wrapper
+    (`moveTaskToContainedBackwardTarget`, engine/src/execution/lifecycle-move.ts) is
+    deliberately NOT imported here: core must not reach into engine.
     */
     const store = h.store();
     /*
@@ -195,25 +225,60 @@ pgDescribe("live move path — which targets it accepts after the Planning merge
     const task = await store.createTask({ description: "recovery rehome", enabledWorkflowSteps: [] });
 
     /* `archived -> todo` is the discriminating pair: the legacy table's `archived`
-       row is `["done"]` only, so ordinary adjacency refuses it while recovery must
-       still reach the rebound target. (`done -> todo` would NOT discriminate — the
-       table permits it, so the assertion would pass with the flag removed.) */
+       row is `["done"]` only, so ordinary adjacency refuses it while a recovery
+       re-home used to be expected to reach the rebound target. (`done -> todo` would
+       NOT discriminate — the table permits it, so the assertion would pass with the
+       flag removed.) */
     await store.moveTask(task.id, "in-progress" as never, { moveSource: "user" } as never);
     await store.moveTask(task.id, "in-review" as never, { moveSource: "user" } as never);
     await store.moveTask(task.id, "done" as never, { moveSource: "user" } as never);
     await store.moveTask(task.id, "archived" as never, { moveSource: "user" } as never);
     expect(await column(task.id)).toBe("archived");
 
+    // Guard 1 of 2: ADJACENCY. The legacy `VALID_TRANSITIONS` row for `archived` is
+    // `["done"]`, so a plain engine move to `todo` is refused before any lifecycle
+    // policy is consulted. Named explicitly so it cannot be confused with the refusal below.
     await expect(
       store.moveTask(task.id, "todo" as never, { moveSource: "engine" } as never),
-    ).rejects.toThrow();
+    ).rejects.toThrow(/Valid targets/);
     expect(await column(task.id)).toBe("archived");
 
-    await store.moveTask(task.id, "todo" as never, {
-      moveSource: "engine",
-      recoveryRehome: true,
-    } as never);
+    // Guard 2 of 2: LIFECYCLE CONTAINMENT. `recoveryRehome` legitimately bypasses
+    // guard 1 — that is its whole purpose — and is then stopped by F2. The rejection
+    // names the rule and both roles so a future rank-table change fails HERE, loudly,
+    // rather than as an unexplained refusal.
+    await expect(
+      store.moveTask(task.id, "todo" as never, {
+        moveSource: "engine",
+        recoveryRehome: true,
+      } as never),
+    ).rejects.toThrow(/Forbidden lifecycle path F2: 'archived' \(archived\) → 'todo' \(hold\)/);
 
-    expect(await column(task.id)).toBe("todo");
+    // And the card held: a refused containment is a no-op, not a partial move.
+    expect(await column(task.id)).toBe("archived");
+
+    // ── The capability this case still exists to protect ──────────────────────
+    // A card stranded in a lane its workflow does not declare can still reach a
+    // target the workflow DOES declare — that is the rescue path FN-207 replaced raw
+    // `recoveryRehome` with. Measured against the default lineage (all three below
+    // verified against the live resolver, not inferred):
+    //   in-review  (review) -> in-progress (wip)     one rank, the declared target
+    //   in-progress (wip)   -> todo (hold)           one rank, the declared target
+    //   archived   (terminal) -> undefined           the card holds where it is
+    // The third is the load-bearing one: from a terminal lane the resolver declares
+    // NO backward target, which is precisely why the store-level move above has no
+    // contained target to reach and the guard is the correct answer. It mirrors the
+    // engine-side case at lifecycle-forbidden-paths.test.ts:243 ("keeps restart
+    // recovery in review when the workflow declares no WIP lane").
+    const storeForResolver = store as never;
+    await expect(
+      resolveContainedBackwardTargetForTask(storeForResolver, task.id, "in-review"),
+    ).resolves.toBe("in-progress");
+    await expect(
+      resolveContainedBackwardTargetForTask(storeForResolver, task.id, "in-progress"),
+    ).resolves.toBe("todo");
+    await expect(
+      resolveContainedBackwardTargetForTask(storeForResolver, task.id, "archived"),
+    ).resolves.toBeUndefined();
   });
 });

packages/core/src/__tests__/postgres/renamed-board-reopen.pg.test.ts

@@ -14,6 +14,14 @@ WHAT IT WOULD HAVE CAUGHT: a card bounced out of the renamed review lane kept it
 `getTaskMergeBlocker` reads that array, so the card could re-enter review and merge with
 its re-review never run.
 
+FNXC:LifecycleContainment 2026-09-26-00:00 (FUSI-036): since FN-207 that hazard is
+prevented TWICE over, and this file now pins both halves. The role-resolved clear still
+runs wherever a bounce is allowed at all (the `moveSource: "user"` cases), AND an
+automatic bounce out of a review lane into a hold lane is refused outright by the F2
+rank rule — so the shape that made the hazard reachable cannot be created by a machine.
+Neither half subsumes the other: drop the guard and the stale-result case goes red; drop
+the clear and the user-sourced case goes red.
+
 The flag-ON path is the one under test — `isWorkflowColumnsCompatibilityFlagEnabled`
 reads the RAW experimental flag, so without enabling it this suite would exercise the
 legacy inline branch (which is deliberately left name-based as the parity reference) and
@@ -27,6 +35,7 @@ import {
   createSharedPgTaskStoreTestHarness,
 } from "../../__test-utils__/pg-test-harness.js";
 import type { WorkflowIr } from "../../workflows/workflow-ir-types.js";
+import { getTaskMergeBlocker } from "../../merge/task-merge.js";
 
 /** Standard lifecycle traits under non-default column names, with a reopen edge. */
 function renamedBoardIr(): WorkflowIr {
@@ -151,13 +160,102 @@ pgDescribe("a renamed board gets the same reopen effects as the default lineage"
     return { store, taskId: task.id };
   }
 
-  it("clears the stale review result when the renamed review lane bounces to the renamed hold lane", async () => {
+  it("REFUSES the engine-sourced bounce out of the renamed review lane, and the card stays merge-blocked", async () => {
+    /*
+    FNXC:LifecycleContainment 2026-09-26-00:00 (FUSI-036 — census family
+    `lifecycle-transition-forbidden`, F2):
+
+    THE REFUSAL IS THE HAZARD CONTROL, NOT A LOSS OF COVERAGE. This case is the
+    first census row of its family: the store was asked to bounce `checking
+    (review, rank 3) -> queued (hold, rank 1)` under `moveSource: "engine"` and
+    expected to land. That is a rank gap of 2, so F2 fires, and the move supplies
+    no `lifecycleReason` at all — so even the sanctioned-reason check would reject
+    it independently. `AGENTS.md` allows a backward step out of review only via
+    Code Review / verification / merge-fix REVISE, and only to WIP, never to a hold
+    lane. The guard is right; the expectation was the regression.
+
+    WHICH OF THE TWO HALVES OF THE ORIGINAL CASE IS ACTUALLY TRUE AFTER THE
+    REFUSAL — the spec asked for this to be measured, not assumed, so it was:
+
+      1. The reopen clear does NOT run on a refused move. `evaluateTransitionInvariants`
+         throws at moves.ts:751, long before `applyDefaultWorkflowMoveEffects` at
+         moves.ts:1052, so `workflowStepResults`, `branch`, `summary`, `status`,
+         and `error` all survive. The assertions below inverted from "emptied" to
+         "intact" for that reason, and are kept rather than deleted.
+
+      2. The card is still MERGE-BLOCKED where it sits, and the hazard is
+         unreachable anyway. `getTaskMergeBlocker` reads `workflowStepResults`, and
+         the surviving `passed` result is what would let a card merge without a
+         re-review — but only for a card sitting OUTSIDE the review lane. This one
+         never left `checking`, so the result is in the lane it was earned in, and
+         the card is blocked by its own `failed` status. The negative half is
+         measured too, and it is the real proof: the same card, read as if it had
+         moved AND had its status cleared, is merge-ELIGIBLE (`undefined`). That is
+         the exact shape the reopen clear used to prevent, and it is now
+         unreachable from an automatic path because the move that would create it
+         is refused.
+
+    A renamed review lane obeys the same rank rule as `in-review` because roles come
+    from each column's own trait flags, not from column ids. That is the property
+    this file exists to prove, and F2 firing on `checking` is now its proof.
+
+    The guard is scoped, not blanket: the sibling case below moves the same card
+    with `moveSource: "user"` and still lands, because
+    `evaluateLifecycleDirectionPostcondition` returns `null` immediately for a
+    non-engine, non-scheduler source. A human may still pull a card back to a hold
+    lane; an automatic path may not.
+    */
     const { store, taskId } = await seedCardInCheck();
 
-    const moved = await store.moveTask(taskId, "queued", { moveSource: "engine" });
+    await expect(
+      store.moveTask(taskId, "queued", { moveSource: "engine" }),
+    ).rejects.toThrow(/Forbidden lifecycle path F2: 'checking' \(review\) → 'queued' \(hold\)/);
+
+    // The card did not move, and the reopen clear never ran (half 1 above), so
+    // every field the original case watched for clearing is still exactly as seeded.
+    const held = await store.getTask(taskId);
+    expect(held.column).toBe("checking");
+    expect(held.workflowStepResults ?? []).toHaveLength(1);
+    expect(held.branch ?? null).not.toBeNull();
+    expect(held.summary ?? null).not.toBeNull();
+    expect(held.status ?? null).not.toBeNull();
+    expect(held.error ?? null).not.toBeNull();
+
+    // The safety property, stated as an assertion (half 2 above). Resolved review
+    // lanes, exactly as `moves.ts` hands them to the merge door, so this is the
+    // board's own review identity and not the literal `in-review` fallback.
+    const reviewColumns = new Set(["checking"]);
+    expect(getTaskMergeBlocker(held as never, { reviewColumns })).toBeTruthy();
+
+    // The negative control that makes the positive one mean something: the SAME
+    // card with a stale `passed` result, read as though it had left the review lane
+    // with its status cleared, is merge-eligible. That is the hazard the reopen
+    // clear used to prevent by emptying `workflowStepResults`, and it is exactly
+    // what the refusal now makes unreachable.
+    const hazard = { ...held, column: "queued", status: undefined, error: undefined };
+    expect(getTaskMergeBlocker(hazard as never, {
+      skipColumnIdentityCheck: true,
+      requiredPreMergeStepIds: new Set(),
+    })).toBeUndefined();
+  });
+
+  /*
+  The reopen-hook coverage the refused case can no longer carry, kept where the
+  guard does not apply. `moveSource: "user"` returns `null` from the containment
+  postcondition immediately, so the bounce lands and
+  `applyReopenFieldClears` runs its role-resolved clear: the `passed` result,
+  branch, summary, status, and error are all dropped. Without this case the file
+  would pin the refusal but no longer prove the clear works anywhere — a guard that
+  refuses everything would pass it.
+  */
+  it("still clears the stale review result on a user-sourced bounce into the renamed hold lane", async () => {
+    const { store, taskId } = await seedCardInCheck();
+
+    const moved = await store.moveTask(taskId, "queued", { moveSource: "user" });
 
     expect(moved.column).toBe("queued");
-    // The safety assertion: a surviving `passed` result satisfies getTaskMergeBlocker.
+    // The safety assertion, on the path where a human is doing the pulling:
+    // a surviving `passed` result satisfies getTaskMergeBlocker.
     expect(moved.workflowStepResults ?? []).toHaveLength(0);
     expect(moved.branch ?? null).toBeNull();
     expect(moved.summary ?? null).toBeNull();