U7: the replan rebound targets a column the workflow declares (R7) — re-landed on main (#2598)

> Based on `main`, no dependencies. Re-landed after closing the stacked
chain (#2517, and #2551 below) that never reached main.

## What main already has, and what it lacks

Main independently converted the planner-lane **parameters** in this
file — and **better than I had**: it splits `plannerColumn` from
`roles.mergedPlanningColumn`, because a merged lane joins the FN-8596
arrival-order rescue but *not* the "planner column is never advanced"
shortcut. That work is main's and untouched here.

What main still lacks is the **R7 fix**: `resolveReplanTargetColumn`
returns `"triage"` **by fiat** for any workflow declaring neither legacy
id — `builtin:marketing` (ideation/backlog/drafting/…) and every fully
renamed set. A Plan Review REVISE therefore moves the card into a column
its workflow **does not declare**, for `reconcileUndeclaredTaskColumns`
to clean up after. A move the engine makes on purpose, not drift.

| Workflow | Target | Changed? |
|---|---|---|
| `builtin:coding` / stepwise | `todo` | no |
| Coding (Ideas) | `todo` | no |
| `builtin:marketing` | `backlog` (its own hold) | **yes** — was
`triage`, undeclared |
| declares no planning lane | `undefined` → park | **yes** — was
`triage` by fiat |

## The ordering the existing suite taught me

Legacy ids stay preferred **first**, and the trait resolution prefers
**hold over intake**. That is not arbitrary:

Coding (Ideas) declares `ideas` as its intake, and `ideas` is **manual
capture with no AI** (plan R10) — a rejected plan sent there stops being
replanned at all. The old code got Ideas right **by accident**: it never
recognised `ideas` as intake and fell through to `todo`. An "intake
first" trait rule would have shipped that regression dressed as a
cleanup, and three existing Ideas tests were the only thing between me
and doing it.

## An inverted comment, corrected

The function's own U11 note read: *"the second lookup asks for `todo`,
which U11 deletes… the first lookup still matches `triage` (which U11
keeps)"*.

**That is backwards.** #2515 keeps `todo` and deletes `triage`, so the
consequence is the opposite of what was written — the `todo` branch is
what saves builtin coding. Fixed rather than left, because a comment
that inverts a merge's direction sends the next reader to the wrong
branch.

## Fail-closed callers

`undefined` means "nowhere to replan" (plan U5: *skipped with a log
rather than moved arbitrarily*). All four call sites park **visibly**
rather than log a move they did not make. The scheduler's rebound still
writes `needs-replan` — deliberately, since that is what blocks dispatch
and the branch has already decided the card must not be released — with
only the *log* made conditional.

## The superseded test is deleted, not skipped

A skipped test is a guard that cannot fire. Its replacement asserts the
new contract **and** the R7 invariant directly — *"a column this
workflow declares"*, not just an id — plus a new case for a workflow
with no planning lane at all.

## Verification

| Check | Result |
|---|---|
| replan-target | 41/41 |
| with scheduler-trait-dispatch + pre-release-plan-review | 55/55 |
| `tsc --noEmit` (engine) | clean |
| `pnpm lint` | clean |
| `pnpm test:gate` | green (482 + 10 + 71) |
| `pnpm check:changesets` | clean |

`triage.test.ts` still shows main's **8 pre-existing #2515 failures** —
unchanged by this, fixed by **#2576**.

## Closing #2551

Its parameter work is superseded by main's better version; this PR
carries the only part main lacked. Same story as #2517: a stacked PR
outlived the surface it was converting.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

.changeset/replan-target-r7.md

@@ -0,0 +1,7 @@
+---
+"@runfusion/fusion": patch
+---
+
+summary: A rejected plan on a custom workflow goes back to that workflow's own planning column, not one it does not have.
+category: fix
+dev: U7 / R7, re-landed on main after the stacked chain was closed. `resolveReplanTargetColumn` returned `triage` by fiat for any workflow declaring neither legacy id (builtin:marketing, every renamed set), moving the card into an undeclared column for `reconcileUndeclaredTaskColumns` to clean up. Legacy ids stay preferred first so both coding built-ins keep their exact target; only a workflow declaring neither reaches the trait fallback, which prefers HOLD over intake (Coding (Ideas)' intake is manual-capture with no AI, so a rejected plan sent there stops being replanned). No declared lane returns undefined and all callers park visibly. Also corrects an inverted U11 note in that function: #2515 keeps `todo` and deletes `triage`, not the reverse.

packages/engine/src/__tests__/replan-target.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it, vi } from "vitest";
 import type { Task, TaskStep, TaskStore } from "@fusion/core";
-import { resolveWorkflowIrForTask, workflowHasColumn } from "@fusion/core";
+import { resolveWorkflowIrForTask } from "@fusion/core";
 import { hasAdvancedPastPlanning, isTaskStillInPlanningStage, moveTaskToReplanColumn, resolveReplanTargetColumn } from "../replan-target.js";
 
 /*
@@ -277,27 +277,53 @@ Updated to the post-merge truth, not loosened: each still pins one exact column.
     await expect(resolveReplanTargetColumn(store, "FN-1")).resolves.toBe("todo");
   });
 
-  it("targets its OWN rebound lane for a workflow declaring neither triage nor todo", async () => {
+  it("targets a workflow's OWN hold column when it declares neither legacy id", async () => {
     /*
-    FNXC:WorkflowReplan 2026-07-31-13:30 (U11 — the flagged fallback, now converted):
+    FNXC:ReplanTargetR7 2026-07-29-23:50:
-    builtin:marketing declares ideation/backlog/drafting/... — no `triage`, no `todo`.
+    CONTRACT CHANGED — deliberately, and this expectation edit IS the change rather
-    The old fallback handed it the literal `triage`, a column that lineage does not
+    than churn around it. This asserted `"triage"` for builtin:marketing, which
-    declare AND that the default lineage no longer declares either since #2515. So the
+    declares ideation/backlog/drafting/... and NO triage column: the engine moved the
-    replan move targeted a nonexistent column: the card either failed to move or landed
+    card into a column the workflow does not declare, which is the R7 violation
-    somewhere no sweep owns.
+    `reconcileUndeclaredTaskColumns` then cleaned up after.
 
-    Resolved through `resolveReboundTarget` (KTD-10: hold -> intake -> first declared),
+    The old note gave two reasons for preferring a wrong-but-legacy column, and both
-    which is the same helper every other rebound path uses. The card now lands in a
+    are now obsolete: "triage only scans triage and todo" (discovery resolves the
-    column its own workflow actually declares.
+    task's own lanes via `resolvePlannerLanes`) and "the legacy move path throws on
+    custom targets" (a move out of a non-legacy source column resolves targets from
+    the task's own workflow adjacency, FN-7591).
+
+    Asserted as "a column this workflow declares" as well as by id, because the id is
+    incidental and the INVARIANT is what matters.
     */
     const store = storeWithSelection("builtin:marketing");
     const target = await resolveReplanTargetColumn(store, "FN-1");
-    expect(target).not.toBe("triage");
+
+    expect(target).toBe("backlog");
     const ir = await resolveWorkflowIrForTask(store as never, "FN-1");
-    expect(workflowHasColumn(ir, target)).toBe(true);
+    expect((ir as unknown as { columns: Array<{ id: string }> }).columns.map((c) => c.id))
+      .toContain(target);
   });
 
-  /* Resolution failure no longer reaches the `triage` catch: the resolver swallows
+  it("returns UNDEFINED for a workflow that declares no planning lane at all", async () => {
+    // Plan U5: "skipped with a log rather than moved arbitrarily". Inventing a column
+    // is what this change removes.
+    const store = {
+      getTaskWorkflowSelection: vi.fn(() => ({ workflowId: "custom:wip-only", stepIds: [] })),
+      getWorkflowDefinition: vi.fn(async () => ({
+        ir: {
+          version: "v2", id: "custom:wip-only", name: "wip-only", nodes: [], edges: [],
+          columns: [
+            { id: "building", name: "Wip", traits: [{ trait: "wip" }] },
+            { id: "done", name: "Done", traits: [{ trait: "complete" }] },
+          ],
+        },
+      })),
+    } as unknown as TaskStore;
+
+    await expect(resolveReplanTargetColumn(store, "FN-1")).resolves.toBeUndefined();
+  });
+
+    /* Resolution failure no longer reaches the `triage` catch: the resolver swallows
      the error and hands back the default IR, whose planner lane is now `todo`. The
      two literal `return "triage"` fallbacks survive only for workflows that declare
      neither column (see the marketing case above) — a pre-existing wart, since that
@@ -384,3 +410,57 @@ describe("replan bounces preserve the task worktree (FN-8603)", () => {
     );
   });
 });
+
+/*
+FNXC:ReplanTargetR7 2026-07-30-01:20 (PR #2598 review — greptile P1):
+`resolveReplanTargetColumn` returning `undefined` is only half a contract — what the
+CALLERS do with it is the other half, and a silent return there is worse than the R7
+move it replaced: `requestPreMergeOptionalStepFix` returns `true` unconditionally, so
+an unchanged row is reported to the graph as "remediation scheduled", the retry budget
+is never consumed, and the same failure recurs with nothing to stop it.
+
+These pin the contract at the seam rather than at each caller, because it is the seam
+every caller shares.
+*/
+describe("the no-declared-lane contract", () => {
+  function storeFor(columns: Array<Record<string, unknown>>): TaskStore {
+    return {
+      getTaskWorkflowSelection: vi.fn(() => ({ workflowId: "custom:x", stepIds: [] })),
+      getWorkflowDefinition: vi.fn(async () => ({
+        ir: { version: "v2", id: "custom:x", name: "x", nodes: [], edges: [], columns },
+      })),
+      moveTask: vi.fn(async () => undefined),
+    } as unknown as TaskStore;
+  }
+
+  const WIP_ONLY = [
+    { id: "building", name: "Wip", traits: [{ trait: "wip" }] },
+    { id: "done", name: "Done", traits: [{ trait: "complete" }] },
+  ];
+
+  it("resolves to undefined rather than inventing a column", async () => {
+    await expect(resolveReplanTargetColumn(storeFor(WIP_ONLY), "FN-1")).resolves.toBeUndefined();
+  });
+
+  it("does not move the card, and reports that it did not", async () => {
+    // The return value is what callers use as "where the card now is", so a silent
+    // no-op would be a lie they cannot detect.
+    const store = storeFor(WIP_ONLY);
+    const moved = await moveTaskToReplanColumn(store, { id: "FN-1", column: "building" });
+
+    expect(moved).toBeUndefined();
+    expect(store.moveTask).not.toHaveBeenCalled();
+  });
+
+  it("still moves and reports the column when a lane IS declared", async () => {
+    // The other side, so "never moves" cannot pass for "correctly refuses".
+    const store = storeFor([
+      { id: "drafting", name: "Hold", traits: [{ trait: "hold", config: { release: "capacity" } }] },
+      ...WIP_ONLY,
+    ]);
+    const moved = await moveTaskToReplanColumn(store, { id: "FN-1", column: "building" });
+
+    expect(moved).toBe("drafting");
+    expect(store.moveTask).toHaveBeenCalledWith("FN-1", "drafting", { preserveWorktree: true });
+  });
+});

packages/engine/src/replan-target.ts

@@ -1,6 +1,7 @@
 import type { Task, TaskStore } from "@fusion/core";
-import { resolveLifecycleColumns, resolveReboundTarget, resolveWorkflowIrForTask, workflowHasColumn } from "@fusion/core";
+import { resolveLifecycleColumns, resolveWorkflowIrForTask, workflowHasColumn } from "@fusion/core";
 import type { WorkflowIr } from "@fusion/core";
+import { schedulerLog } from "./logger.js";
 
 /*
 FNXC:WorkflowReplan 2026-07-12-23:15:
@@ -315,7 +316,7 @@ export function isTaskStillInPlanningStage(
   return !hasAdvancedPastPlanning(task, plannerColumn, roles);
 }
 
-export async function resolveReplanTargetColumn(store: TaskStore, taskId: string): Promise<string> {
+export async function resolveReplanTargetColumn(store: TaskStore, taskId: string): Promise<string | undefined> {
   try {
     const ir = await resolveWorkflowIrForTask(store, taskId);
     /*
@@ -360,16 +361,33 @@ export async function resolveReplanTargetColumn(store: TaskStore, taskId: string
     (hold -> intake -> first declared), so the card lands in a column its own workflow
     declares and the replan lanes stay consistent with the rebound lanes.
     */
-    return resolveReboundTarget(ir) ?? "triage";
+    /*
+    FNXC:ReplanTargetR7 2026-07-31-15:30 (CORRECTS my own #2659, superseded by #2598):
+    PREFER HOLD, THEN INTAKE, THEN NOTHING — never an arbitrary column.
+
+    #2659 (mine) used `resolveReboundTarget`, whose third fallback is "first declared
+    column". For a wip-only workflow that returns the WIP column, so a needs-replan
+    card would be moved INTO an execution lane where the scheduler can pick it up
+    against the spec that was just rejected. That is worse than the literal it
+    replaced, and #2598's review surfaced it.
+
+    Hold before intake is deliberate: Coding (Ideas) declares `ideas` as its intake
+    and `ideas` is manual capture with NO AI, so a rejected plan sent there stops
+    being replanned at all. The old literal got Ideas right by accident — it never
+    recognised `ideas` as intake and fell through to `todo`.
+
+    `undefined` means "this workflow declares nowhere to replan"; callers park the
+    card visibly rather than reporting a move that did not happen.
+    */
+    const roles = resolveLifecycleColumns(ir);
+    return roles?.hold ?? roles?.intake;
   } catch {
     /*
-    NO IR MEANS NOTHING TO RESOLVE, so this keeps the legacy literal deliberately
+    Unreachable in practice: `resolveWorkflowIrForTask` is TOTAL — every failure path
-    rather than guessing. It is reached only when resolution THROWS — not when it
+    returns the default coding IR rather than throwing. Kept as belt-and-braces and
-    falls back to the default IR, which returns a real workflow and takes the `todo`
+    documented so nobody writes a test for a state that cannot occur.
-    branch above. Changing it to another literal would trade one arbitrary column for
-    another without evidence about the workflow.
     */
-    return "triage";
+    return undefined;
   }
 }
 
@@ -403,10 +421,23 @@ export async function moveTaskToReplanColumn(
   store: TaskStore,
   task: Pick<Task, "id" | "column">,
   target?: string,
-): Promise<string> {
+): Promise<string | undefined> {
   const replanColumn = target ?? await resolveReplanTargetColumn(store, task.id);
+  /*
+  FNXC:ReplanTargetR7 2026-07-31-15:35 (PR #2598):
+  NO DECLARED REPLAN COLUMN: do NOT move, and say so. Every caller treats the return
+  value as "where the card now is", so a silent no-op would be a lie — returning
+  `undefined` forces the caller to state the outcome instead of assuming one.
+  */
+  if (!replanColumn) {
+    schedulerLog.warn(
+      `${task.id}: replan rebound skipped — the task's workflow declares no intake or hold `
+      + `column to replan in; card left in ${task.column}`,
+    );
+    return undefined;
+  }
   if (task.column !== replanColumn) {
-    await store.moveTask(task.id, replanColumn, { preserveWorktree: true });
+    await store.moveTask(task.id, replanColumn as Task["column"], { preserveWorktree: true });
   }
   return replanColumn;
 }