fix(test) + E2E: re-green main's lifecycle release leg, and prove the MERGED board + REVISE rework (#2634)

**Second batch.** Three commits, no production code. `pnpm test:gate` green, `pnpm lint` clean, all three E2E suites together **3 files / 41 tests, exit 0**. ## 1. main's lifecycle E2E is RED right now — this fixes it Independently of my work, on a detached `origin/main`: **2 failed / 18 passed**. Scenarios 1 and 2 fail with `sweep.released` **empty**. **Cause:** `seedTask` relied on task creation's PROMPT.md, which is a bootstrap seed (`"# <id>\n\n<description>"`). FN-7648's `isUnplannedForExecution` reads that file for any card resting in an intake- **or** hold-trait column and refuses to move an unplanned card into a processing column. The sweep reported `held: [{ reason: "move-rejected-or-no-slot" }]`. **That is the gate working.** The fixture was asking the scheduler to release a card that had never been specified. The fix is the one the graph-entry contract doc already prescribes: *"Scheduler/release test fixtures must model a card that cleared the gate ... A held unreviewed card is the gate working."* `seedTask` now writes a planned PROMPT.md. **Verified it repairs main, not just this branch:** applying only that file to a detached `origin/main` leaves scenarios 1 and 2 **passing**, with the 4 residual failures being scenarios 3 and 6 — which need the fixture-options commit main does not have. ### I was wrong in #2627 and this corrects it In #2627 I named the in-transaction capacity gate (#2488/#2499) as the likely cause. **It was not.** Two hypotheses died, both recorded in the code comment so nobody re-runs them: | Hypothesis | Result | |---|---| | E2E settings lack `maxConcurrent` → capacity gate rejects the move | added `maxConcurrent`/`maxWorktrees` → **still 2 failed**. Not the cause. | | the move itself is refused | a direct `moveTask(id, wip)` → **succeeded**. Never the blocker. | Only then did probing the two release gates give `isTaskBlockedOnApproval=false`, `isUnplannedForExecution=true`, and dumping the file show the stub. I've flagged the wrong lead on #2627 too — a plausible-sounding cause pointed at another worker's PR is worse than no lead. ## 2. E2E evidence: the MERGED intake+hold board U11's shape — one column carrying intake **and** hold — had no end-to-end coverage; every prior E2E drove intake and hold as separate columns. - shared fixture gains opt-in `mergedIntakeAndHold`, plus `MERGED_VOCAB` (legacy ids, so a failure is attributable to the **role** merge alone) and `MERGED_RENAMED_VOCAB` (ids move too). - lifecycle scenario 3 drives the full spine: planning runs **in place** on the dual-role column, the real `runHoldReleaseSweep` releases **from** it, the graph runs to complete. - 4 merge-safeguard cases on the merged board (finalize, proofless refusal with the same reason, merged+renamed landing no legacy id, at-most-once). ## 3. E2E evidence: a REVISE routes back through rework The plan's `InReview → InProgress: review requests changes` had **no** live-engine evidence on any board — the fixture's review seam always succeeded. Two things the engine taught me, both corrected here: - the **IR validator refused** my rework edge: it is only legal into a node with `config.reworkRegion: true`. A real contract, and the validator catching it is the system working. `exec` now declares it (the shape the builtin uses on `merge-attempt`). - my first assertion was wrong. A REVISE does **not** leave the card in wip — rework re-enters `exec` within the same run, review approves on its second call, and the card finishes at complete. The evidence is the **seam sequence** `["planning","execute","review","execute","review","merge"]`, not an intermediate column the run has already passed. Asserting the final column alone would have been satisfied by a graph that ignored the REVISE entirely. ## Both families are mutation-attributed | Scenario | Mutation | Result | |---|---|---| | 3 — merged intake+hold | `isHeldTask` treats intake/hold as exclusive | **exactly its 2 tests** fail | | 6 — REVISE → rework | disable rework re-entry in `workflow-graph-executor` | **exactly its 2 tests** fail | Both fixture options are opt-in; the two pre-existing suites are behaviourally unchanged (27 → 29 → 41 passed across the additions, no existing assertion touched). ## Still not shipped: safeguard 2's graph E2E Attempted twice, deleted both times. Attempt 1 passed and then survived mutating `merge-gate` to ignore `task.autoMerge` — the card parked on the review column's `merge-blocker` trait, not the gate. Attempt 2 removed that trait to isolate the gate, and the **control** case parked too. Isolating it needs a merge path mirroring the builtin (`merge-gate → merge node → end`) rather than a direct edge to `end` — a real redesign, not a speculative edit. The enforcement that holds today is `allowInReviewMergeProcessing` in `project-engine` (unit-mutation verified, NEW=9; gated via #2526). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-29 22:23:32 -07:00
parent eb874f3da3
commit 73338502e5
4 changed files with 348 additions and 12 deletions
--- a/docs/plans/workflow-owned-merge-stack/u9-safeguard-baseline.md
+++ b/docs/plans/workflow-owned-merge-stack/u9-safeguard-baseline.md
@@ -116,6 +116,62 @@ same family of error this program is trying to stamp out.
 Harness: baseline run → assert patch applied (a no-op patch is an abort, not a
 pass) → mutated run → `comm -13` the sorted fail-sets → restore → assert clean.

+## Why the merge-region nodes cannot be tested as nodes (and what U9 actually is)
+
+Three attempts to give safeguard 2 (`autoMerge:false` is terminal-until-human) a
+graph-level E2E all failed, and the third failure found the reason. It is the single
+most useful thing in this document for whoever lands the conversion.
+
+`workflow-graph-executor.ts:310` says it plainly:
+
+> Until the workflow interpreter owns merge policy end-to-end, graph execution treats
+> any entry into this region as the terminal legacy `merge` seam so observable
+> lifecycle behavior stays byte-identical with the legacy executor.
+
+`MERGE_REGION_KINDS` is `merge-gate`, `merge-attempt`, `manual-merge-hold`,
+`retry-backoff`, `recovery-router`, `branch-group-member-integration`,
+`branch-group-promotion`. **Entering any of them short-circuits to the legacy merge
+seam, so none of their handlers ever runs.** Consequences worth stating separately,
+because each one burned an attempt:
+
+- `createMergeGateHandler` (`merge-runner.ts:44`) DOES read
+  `task.autoMerge !== false && settings.autoMerge !== false` and emit
+  `auto-on`/`auto-off` — and is **never called by the graph**. A test that drives a
+  graph and expects the gate to park an `autoMerge:false` card will watch the card
+  complete, no matter how the IR is shaped.
+- The `outcome:auto-on` / `outcome:auto-off` edges the builtin coding IR declares are
+  therefore unreachable in graph execution today.
+- This is the mechanism behind the very first U9 finding
+  (`u9-merge-region-node-config-authority.test.ts`): the IR's merge-region config is
+  unread not because a handler ignores it, but because the executor bypasses the whole
+  region.
+
+**So U9's conversion, stated concretely, is: stop short-circuiting
+`MERGE_REGION_KINDS` to the legacy seam and let those nodes run.** Everything else —
+S06's capability extraction, S07's completion handoff, S08's queue processing — hangs
+off that one behavioural change, and the safeguard table above is the contract it must
+not break. Safeguard 2 in particular has NO node-level representation today, so a
+conversion that simply enables the region without carrying the `autoMerge` contract
+into it would let an `autoMerge:false` card merge on PR-readiness alone.
+
+Attempts, recorded so they are not repeated:
+1. Fixture with an auto-off edge alongside its existing bare `success` edge — the
+   `autoMerge:false` card COMPLETED, because both edges match
+   `{ outcome: "success", value: "auto-off" }` and `end` won on target-id sort. (Real
+   IR-authoring hazard; the builtin routes `outcome:auto-on` explicitly for this
+   reason.)
+2. Dropped `merge-blocker` from the review column to isolate the gate — broke the route
+   to `end`, so the CONTROL card parked too and the flag was still not the
+   discriminator.
+3. Mirrored the builtin region (`merge-gate --auto-on--> merge-attempt --> end`,
+   `--auto-off--> manual-hold`) — control passed, `autoMerge:false` still completed.
+   That is what exposed the short-circuit: the nodes are not executing.
+
+Testing safeguard 2 at the node level requires a harness that does NOT substitute
+merge-region nodes, which is a change to the executor's substitution boundary rather
+than a test addition — i.e. it becomes possible as part of the conversion, not before
+it.
+
 ## What this baseline does not cover

 - **Reviewer-lane safeguards.** This is the merge lane only. The review nodes
--- a/packages/engine/src/tests/_workflow-vocabulary-fixture.ts
+++ b/packages/engine/src/tests/_workflow-vocabulary-fixture.ts
@@ -53,8 +53,51 @@ export interface LifecycleIrOptions {
     suite's IR stays byte-identical to what it was written against — a shared
     fixture must not silently change an existing suite's subject. */
  readonly mergeOrchestration?: boolean;
+  /* FNXC:MergedPlanningColumn 2026-07-29-23:50 (U9 E2E evidence — the merged board):
+     Adds the `intake` trait to the HOLD column, so ONE column carries intake + hold —
+     which is exactly the shape U11 shipped on the default lineage (Planning, id `todo`,
+     no `triage` column at all). Until now every E2E here drove a board with intake and
+     hold as SEPARATE columns, so nothing proved the merged shape end-to-end; a guard
+     that silently keys on "the column that is only a hold" passes on the default and
+     renamed vocabularies and goes wrong only here.
+
+     OPT-IN for the same reason as `mergeOrchestration`: a shared fixture must not
+     silently change an existing suite's subject. */
+  readonly mergedIntakeAndHold?: boolean;
+  /* FNXC:ReviewRework 2026-07-30-01:10 (U9 E2E evidence — the review half):
+     Adds the `review --failure--> exec` rework edge, so a REVISE verdict routes the
+     card BACK to the wip column. Without it the review node has only a success edge
+     and every E2E here drives review as a pass-through, which means the plan's
+     `InReview --> InProgress: review requests changes` transition had no live-engine
+     evidence on ANY board.
+
+     Opt-in like the other options: this changes the graph's reachable shape, and a
+     shared fixture must not alter an existing suite's subject. */
+  readonly reviewRework?: boolean;
 }

+/**
+ * The MERGED board: intake and hold are one column, as on the operator's real default
+ * workflow after U11. Ids deliberately overlap the legacy enum (`todo` is genuinely the
+ * merged Planning id there) so this is not a rename test — it isolates the merge of two
+ * ROLES onto one column from any change of vocabulary.
+ */
+export const MERGED_VOCAB: Vocabulary = {
+  hold: "todo",
+  wip: "in-progress",
+  review: "in-review",
+  complete: "done",
+};
+
+/** A merged board that ALSO renames: both variables move at once, which is the shape a
+ *  custom workflow author actually produces. */
+export const MERGED_RENAMED_VOCAB: Vocabulary = {
+  hold: "planning",
+  wip: "building",
+  review: "checking",
+  complete: "shipped",
+};
+
 export function lifecycleIr(v: Vocabulary, id: string, options: LifecycleIrOptions = {}): WorkflowIr {
  return {
    version: "v2",
@@ -64,7 +107,10 @@ export function lifecycleIr(v: Vocabulary, id: string, options: LifecycleIrOptio
      {
        id: v.hold,
        name: "Hold",
-        traits: [{ trait: "hold", config: { release: "capacity" } }],
+        traits: [
+          ...(options.mergedIntakeAndHold ? [{ trait: "intake" }] : []),
+          { trait: "hold", config: { release: "capacity" } },
+        ],
        /* U4 workflow-declared recovery policy (#2478). Declared on the HOLD column of both
           vocabularies from the one builder, so the reconciler's role resolution is exercised
           against a renamed column with nothing else differing. */
@@ -89,7 +135,18 @@ export function lifecycleIr(v: Vocabulary, id: string, options: LifecycleIrOptio
    nodes: [
      { id: "start", kind: "start", column: v.hold },
      { id: "plan", kind: "prompt", column: v.hold, config: { seam: "planning" } },
-      { id: "exec", kind: "prompt", column: v.wip, config: { seam: "execute" } },
+      {
+        id: "exec",
+        kind: "prompt",
+        column: v.wip,
+        /* `reworkRegion` is required by the IR validator for any rework-edge TARGET
+           ("only legal ... into a top-level rework region head") — the same shape the
+           builtin coding IR uses on `merge-attempt`. Declared only when the rework edge
+           exists, so the non-rework IR stays byte-identical. */
+        config: options.reviewRework
+          ? { seam: "execute", reworkRegion: true, maxReworkCycles: 3 }
+          : { seam: "execute" },
+      },
      { id: "review", kind: "prompt", column: v.review, config: { seam: "review" } },
      /* A real merge-class node. The IR validator REFUSES a `merge-blocker` column with no
         reachable merge-class node ("the gate can never clear without one") — discovered by this
@@ -105,6 +162,9 @@ export function lifecycleIr(v: Vocabulary, id: string, options: LifecycleIrOptio
      { from: "exec", to: "review", condition: "success" },
      { from: "review", to: "merge-gate", condition: "success" },
      { from: "merge-gate", to: "end", condition: "success" },
+      ...(options.reviewRework
+        ? [{ from: "review", to: "exec", condition: "failure", kind: "rework" }]
+        : []),
    ],
  } as WorkflowIr;
 }
--- a/packages/engine/src/tests/workflow-lifecycle-live-e2e.pg.test.ts
+++ b/packages/engine/src/tests/workflow-lifecycle-live-e2e.pg.test.ts
@@ -52,7 +52,7 @@ import {
 import { WorkflowGraphTaskRunner, type WorkflowColumnBoundaryHooks } from "../workflow-graph-task-runner.js";
 import { createExecutorColumnBoundaryHooks } from "../workflow-column-boundary-hooks.js";
 import { runHoldReleaseSweep } from "../hold-release.js";
-import { DEFAULT_VOCAB, RENAMED_VOCAB, HOLD_STALENESS_MS, lifecycleIr, type Vocabulary } from "./_workflow-vocabulary-fixture.js";
+import { DEFAULT_VOCAB, RENAMED_VOCAB, MERGED_VOCAB, MERGED_RENAMED_VOCAB, HOLD_STALENESS_MS, lifecycleIr, type Vocabulary } from "./_workflow-vocabulary-fixture.js";
 import { SelfHealingManager } from "../self-healing.js";
 import { reconcileRecovery } from "../recovery-reconciler.js";

@@ -67,6 +67,25 @@ interface SeamLog {
   ANY entry into the merge region (merge-gate included) onto the legacy `merge` seam, so the walk
   below genuinely reaches the merge lane before the terminal column. Scripting it is the same
   substitution `testMode` makes; the column move that follows is real. */
+/*
+FNXC:ReviewRework 2026-07-30-01:20 (U9 E2E evidence — the review half):
+Seams whose REVIEW fails its first call and succeeds afterwards, so the graph takes
+the `review --failure--> exec` rework edge exactly once. `OK` everywhere else, so the
+only behavioural difference from `scriptedSeams` is the verdict.
+*/
+function revisingSeams(log: SeamLog) {
+  const base = scriptedSeams(log);
+  let reviewCalls = 0;
+  return {
+    ...base,
+    review: async () => {
+      log.calls.push("review");
+      reviewCalls += 1;
+      return reviewCalls === 1 ? { outcome: "failure" as const, value: "REVISE" } : OK;
+    },
+  };
+}
+
 function scriptedSeams(log: SeamLog) {
  const seam = (name: string) => async () => {
    log.calls.push(name);
@@ -101,8 +120,8 @@ pgDescribe("live lifecycle E2E: real graph + real PostgreSQL store", () => {
   *  `createWorkflowDefinition` allocates its own `WF-###` and IGNORES the `id` in the input —
   *  binding a task to the id we passed in silently resolves to the DEFAULT builtin IR, which is
   *  exactly how a renamed-workflow fixture can pass while testing nothing. */
-  async function seedWorkflow(v: Vocabulary, key: string): Promise<{ workflowId: string; ir: WorkflowIr }> {
-    const ir = lifecycleIr(v, `custom:${key}`);
+  async function seedWorkflow(v: Vocabulary, key: string, merged = false, reviewRework = false): Promise<{ workflowId: string; ir: WorkflowIr }> {
+    const ir = lifecycleIr(v, `custom:${key}`, { mergedIntakeAndHold: merged, reviewRework });
    const created = await h.store().createWorkflowDefinition({
      name: `Lifecycle ${key}`,
      kind: "workflow",
@@ -119,6 +138,36 @@ pgDescribe("live lifecycle E2E: real graph + real PostgreSQL store", () => {
      { taskId, applyDefaultWorkflowSteps: false } as never,
    );
    await store.writeTaskWorkflowSelection(taskId, workflowId, []);
+    /*
+    FNXC:WorkflowLifecycleColumns 2026-07-30-04:10 (release-leg fixture fix):
+    The card needs a PLANNED PROMPT.md before the sweep will release it. Task creation
+    leaves a bootstrap seed ("# <id>\n\n<description>"), and FN-7648's
+    `isUnplannedForExecution` reads that file for any card resting in an intake- OR
+    hold-trait column and refuses to move an unplanned card into a processing column —
+    so the sweep reported `held: [{ reason: "move-rejected-or-no-slot" }]` and released
+    nothing, which is the gate WORKING, not a scheduler defect.
+
+    Diagnosed rather than guessed, and two hypotheses died on the way: adding
+    `maxConcurrent`/`maxWorktrees` to the E2E settings changed nothing (so the
+    in-transaction capacity gate from #2488/#2499 was NOT the cause), and a direct
+    `moveTask(id, wip)` succeeded (so the move itself was never the blocker). Probing the
+    two release gates showed `isTaskBlockedOnApproval=false`,
+    `isUnplannedForExecution=true`, and dumping the file showed the stub.
+
+    This is the fixture modelling a card that cleared specification, which
+    `docs/solutions/architecture-patterns/workflow-node-column-placement-and-graph-entry-contract.md`
+    states outright: "Scheduler/release test fixtures must model a card that cleared the
+    gate ... A held unreviewed card is the gate working."
+    */
+    const { writeFileSync, mkdirSync } = await import("node:fs");
+    const { join } = await import("node:path");
+    const dir = join(store.getTasksDir(), taskId);
+    mkdirSync(dir, { recursive: true });
+    writeFileSync(
+      join(dir, "PROMPT.md"),
+      `# ${taskId}\n\n## Context\nA planned spec, so the release sweep does not classify this card as an unplanned seed.\n\n## Steps\n### Step 1\n- [ ] do the planned work\n`,
+      "utf-8",
+    );
    store.taskCache.delete(taskId);
    return task as Task;
  }
@@ -165,7 +214,13 @@ pgDescribe("live lifecycle E2E: real graph + real PostgreSQL store", () => {
    });
  }

-  function makeRunner(taskId: string, workflowId: string, log: SeamLog, moveMarks: string[] = []) {
+  function makeRunner(
+    taskId: string,
+    workflowId: string,
+    log: SeamLog,
+    moveMarks: string[] = [],
+    seamsFactory?: (l: SeamLog) => unknown,
+  ) {
    const store = h.store();
    const runId = `${taskId}:workflow`;
    return new WorkflowGraphTaskRunner({
@@ -176,7 +231,7 @@ pgDescribe("live lifecycle E2E: real graph + real PostgreSQL store", () => {
        getTask: (id: string) => store.getTask(id),
      },
      runId,
-      seams: scriptedSeams(log) as never,
+      seams: (seamsFactory ?? scriptedSeams)(log) as never,
      runCustomNode: async () => {
        throw new Error("no custom node should run in this lifecycle shape");
      },
@@ -195,8 +250,8 @@ pgDescribe("live lifecycle E2E: real graph + real PostgreSQL store", () => {
   * The full lifecycle, driven for one vocabulary. Returns everything observed so the two
   * vocabularies can be compared field-for-field rather than eyeballed.
   */
-  async function driveLifecycle(taskId: string, v: Vocabulary, key: string) {
-    const { workflowId } = await seedWorkflow(v, key);
+  async function driveLifecycle(taskId: string, v: Vocabulary, key: string, merged = false) {
+    const { workflowId } = await seedWorkflow(v, key, merged);
    await seedTask(taskId, v, workflowId);

    const events: WorkflowLifecycleEvent[] = [];
@@ -305,6 +360,47 @@ pgDescribe("live lifecycle E2E: real graph + real PostgreSQL store", () => {
    });
  });

+  /*
+  FNXC:MergedPlanningColumn 2026-07-30-00:20 (U9 E2E evidence — the merged board):
+  Scenarios 1 and 2 both drive a board where intake and hold are SEPARATE columns.
+  The operator's real default workflow no longer looks like that: U11 merged them into
+  one Planning column. The release path is where that matters — `runHoldReleaseSweep`
+  has to release a card FROM a column that is simultaneously the intake column, and a
+  resolver that treats "intake" and "hold" as mutually exclusive roles would leave the
+  card parked forever with no error.
+
+  Two variants: MERGED_VOCAB keeps legacy ids so a failure is attributable to the ROLE
+  merge alone, and MERGED_RENAMED_VOCAB moves ids too, which is what a custom workflow
+  author actually produces.
+  */
+  describe("scenario 3 — MERGED intake+hold column (U11's shape)", () => {
+    it("releases and completes a card whose hold column is ALSO the intake column", async () => {
+      const r = await driveLifecycle("FN-E2E-MERGED", MERGED_VOCAB, "merged-vocab", true);
+
+      expect(r.columnsObserved.atCreate).toBe(MERGED_VOCAB.hold);
+      // Planning runs IN PLACE on the merged column — it must not be treated as a
+      // pre-intake staging lane the card has to leave first.
+      expect(r.columnsObserved.afterPlanning).toBe(MERGED_VOCAB.hold);
+      // The claim this scenario exists for: capacity release works from a dual-role column.
+      expect(r.sweep.released).toContain("FN-E2E-MERGED");
+      expect(r.columnsObserved.afterRelease).toBe(MERGED_VOCAB.wip);
+      expect(r.columnsObserved.afterRun).toBe(MERGED_VOCAB.complete);
+      expect(r.seamCalls).toEqual(["planning", "execute", "review", "merge"]);
+    });
+
+    it("does the same on a board that is BOTH merged and renamed", async () => {
+      const r = await driveLifecycle("FN-E2E-MR", MERGED_RENAMED_VOCAB, "merged-renamed", true);
+
+      expect(r.columnsObserved.atCreate).toBe(MERGED_RENAMED_VOCAB.hold);
+      expect(r.sweep.released).toContain("FN-E2E-MR");
+      expect(r.columnsObserved.afterRelease).toBe(MERGED_RENAMED_VOCAB.wip);
+      expect(r.columnsObserved.afterRun).toBe(MERGED_RENAMED_VOCAB.complete);
+      // No leg may touch a legacy id on this board.
+      const legacy = new Set(["triage", "todo", "in-progress", "in-review", "done", "archived"]);
+      for (const observed of Object.values(r.columnsObserved)) expect(legacy.has(observed)).toBe(false);
+    });
+  });
+
  describe("scenario 4 — the post-commit event seam under a real move", () => {
    it("delivers a well-formed TaskTransitioned to a real subscriber for a RENAMED move", async () => {
      const r = await driveLifecycle("FN-E2E-5", RENAMED_VOCAB, "events-renamed");
@@ -657,6 +753,77 @@ pgDescribe("live lifecycle E2E: real graph + real PostgreSQL store", () => {
      }
    });
  });
+  /*
+  FNXC:ReviewRework 2026-07-30-01:30 (U9 E2E evidence — the review half):
+  The plan's target lifecycle includes `InReview --> InProgress: review requests
+  changes`, and until now NO board proved it against a live engine: the fixture's
+  review seam always succeeded, so every E2E drove review as a pass-through.
+
+  This drives a real REVISE. The graph takes the `review --failure--> exec` rework
+  edge, which is a BACKWARD column move (review -> wip) through the real boundary
+  controller and the real `store.moveTask` — the move class most likely to be refused
+  by an adjacency or trait guard, and the one a rework loop cannot work without.
+
+  Run on the merged board as well as the renamed one: rework re-enters the wip column,
+  whose trait set differs on a merged board, so a guard that resolves the rework target
+  by elimination ("the column that is not intake and not review") behaves differently
+  there.
+  */
+  describe("scenario 6 — a REVISE verdict routes the card back to wip", () => {
+    async function driveRevise(taskId: string, v: Vocabulary, key: string, merged: boolean) {
+      const { workflowId } = await seedWorkflow(v, key, merged, true);
+      await seedTask(taskId, v, workflowId);
+      const log: SeamLog = { calls: [] };
+      const marks: string[] = [];
+
+      // Leg 1: plan in the hold column, park at the capacity boundary.
+      await makeRunner(taskId, workflowId, log, marks, revisingSeams).run(await detail(taskId), settings);
+      // Leg 2: the real sweep releases into wip.
+      await runHoldReleaseSweep(h.store(), { now: () => Date.now() });
+      const afterRelease = await persistedColumn(taskId);
+      // Leg 3: execute -> review -> REVISE -> rework back to exec.
+      const items = await h.store().listWorkflowWorkItemsForTask(taskId, { kinds: ["task"] });
+      const resumeNode = items.find((i) => ["held", "runnable", "running"].includes(i.state))?.nodeId;
+      const leg3 = await makeRunner(taskId, workflowId, log, marks, revisingSeams)
+        .run(await detail(taskId), settings, resumeNode);
+      return { afterRelease, afterRevise: await persistedColumn(taskId), calls: log.calls, leg3 };
+    }
+
+    /*
+    MEASURED, and it corrected the assertion I first wrote. A REVISE does not leave the
+    card resting in wip: the rework edge re-enters `exec` WITHIN THE SAME run, review is
+    called again, approves, and the card finishes at complete. So the observable proof
+    that rework happened is the SEAM SEQUENCE — execute appears twice, the second time
+    after a review — not an intermediate column, which the run has already moved past by
+    the time the leg returns.
+
+    Asserting the final column alone would have been satisfied by a graph that ignored
+    the REVISE entirely and went straight to merge, which is exactly the failure this
+    scenario is for.
+    */
+    it("re-enters exec on a REVISE and only completes after the second review (renamed board)", async () => {
+      const r = await driveRevise("FN-E2E-REV", RENAMED_VOCAB, "revise-renamed", false);
+
+      expect(r.afterRelease).toBe(RENAMED_VOCAB.wip);
+      // The rework edge was traversed: execute ran a SECOND time, after a review.
+      expect(r.calls).toEqual(["planning", "execute", "review", "execute", "review", "merge"]);
+      // And the loop resolved rather than spinning — the card reached complete.
+      expect(r.afterRevise).toBe(RENAMED_VOCAB.complete);
+    });
+
+    it("does the same on a MERGED board, without bouncing to the dual-role column", async () => {
+      const r = await driveRevise("FN-E2E-REV-M", MERGED_VOCAB, "revise-merged", true);
+
+      expect(r.afterRelease).toBe(MERGED_VOCAB.wip);
+      expect(r.calls).toEqual(["planning", "execute", "review", "execute", "review", "merge"]);
+      expect(r.afterRevise).toBe(MERGED_VOCAB.complete);
+      // Rework must re-enter wip, not the dual-role Planning column: on a merged board
+      // intake and hold share an id, so an elimination-based target resolution lands
+      // there. A second "planning" call in the sequence above would reveal that.
+      expect(r.calls.filter((c) => c === "planning")).toHaveLength(1);
+    });
+  });
+
 });

 /*
--- a/packages/engine/src/tests/workflow-merge-safeguards-live-e2e.pg.test.ts
+++ b/packages/engine/src/tests/workflow-merge-safeguards-live-e2e.pg.test.ts
@@ -41,7 +41,7 @@ import {
  type SharedPgTaskStoreHarness,
 } from "../../../core/src/__test-utils__/pg-test-harness.js";
 import { finalizeProvenAutoMergeTask } from "../auto-merge-finalization.js";
-import { DEFAULT_VOCAB, RENAMED_VOCAB, lifecycleIr, type Vocabulary } from "./_workflow-vocabulary-fixture.js";
+import { DEFAULT_VOCAB, RENAMED_VOCAB, MERGED_VOCAB, MERGED_RENAMED_VOCAB, lifecycleIr, type Vocabulary } from "./_workflow-vocabulary-fixture.js";

 pgDescribe("live merge safeguards E2E: real store, real refusals", () => {
  const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
@@ -55,11 +55,11 @@ pgDescribe("live merge safeguards E2E: real store, real refusals", () => {

  /** The store allocates its own `WF-###` and ignores the id in the input; binding to
   *  the one we passed would silently resolve to the DEFAULT builtin IR instead. */
-  async function seedWorkflow(v: Vocabulary, key: string): Promise<string> {
+  async function seedWorkflow(v: Vocabulary, key: string, merged = false): Promise<string> {
    const created = await h.store().createWorkflowDefinition({
      name: `Merge safeguards ${key}`,
      kind: "workflow",
-      ir: lifecycleIr(v, `custom:${key}`, { mergeOrchestration: true }),
+      ir: lifecycleIr(v, `custom:${key}`, { mergeOrchestration: true, mergedIntakeAndHold: merged }),
    } as never);
    return (created as { id: string }).id;
  }
@@ -257,4 +257,57 @@ pgDescribe("live merge safeguards E2E: real store, real refusals", () => {
    expect(column).toBe(RENAMED_VOCAB.review);
    expect(["todo", "in-progress", "in-review", "done", "archived"]).not.toContain(column);
  });
+  /* ── The MERGED board — U11's actual shape ────────────────────────────────────
+     Every case above drives a board where intake and hold are SEPARATE columns. The
+     operator's real default workflow no longer looks like that: U11 merged them into
+     one Planning column (id `todo`, no `triage` at all). A guard that keys on "the
+     column that is only a hold" passes on the default AND renamed vocabularies and
+     goes wrong only here, which is precisely why this needed its own family rather
+     than another row in an existing one.
+
+     Two variants on purpose: MERGED_VOCAB keeps the legacy ids (so a failure is
+     attributable to the ROLE merge alone), MERGED_RENAMED_VOCAB moves both variables
+     at once (the shape a custom workflow author actually produces). */
+  it("finalizes a proven-merged card on a MERGED intake+hold board", async () => {
+    const wf = await seedWorkflow(MERGED_VOCAB, "merged", true);
+    await seedProvenMergedTask("FN-SG-MERGED", MERGED_VOCAB, wf);
+
+    const result = await finalize("FN-SG-MERGED");
+
+    expect(result.outcome).toBe("done");
+    expect(await persistedColumn("FN-SG-MERGED")).toBe(MERGED_VOCAB.complete);
+  });
+
+  it("refuses a proofless card on a MERGED board, with the same reason", async () => {
+    const wf = await seedWorkflow(MERGED_VOCAB, "merged-noproof", true);
+    await seedProvenMergedTask("FN-SG-MERGED-NOPROOF", MERGED_VOCAB, wf, { mergeDetails: null });
+
+    const result = await finalize("FN-SG-MERGED-NOPROOF");
+
+    expect({ outcome: result.outcome, reason: (result as { reason?: string }).reason })
+      .toEqual({ outcome: "blocked", reason: "missing-merge-confirmation" });
+    expect(await persistedColumn("FN-SG-MERGED-NOPROOF")).toBe(MERGED_VOCAB.review);
+  });
+
+  it("finalizes on a board that is BOTH merged and renamed, landing no legacy id", async () => {
+    const wf = await seedWorkflow(MERGED_RENAMED_VOCAB, "merged-renamed", true);
+    await seedProvenMergedTask("FN-SG-MR", MERGED_RENAMED_VOCAB, wf);
+
+    const result = await finalize("FN-SG-MR");
+
+    expect(result.outcome).toBe("done");
+    const column = await persistedColumn("FN-SG-MR");
+    expect(column).toBe(MERGED_RENAMED_VOCAB.complete);
+    expect(["todo", "triage", "in-progress", "in-review", "done", "archived"]).not.toContain(column);
+  });
+
+  it("at-most-once holds on a merged+renamed board too", async () => {
+    const wf = await seedWorkflow(MERGED_RENAMED_VOCAB, "merged-once", true);
+    await seedProvenMergedTask("FN-SG-MR-ONCE", MERGED_RENAMED_VOCAB, wf);
+
+    expect((await finalize("FN-SG-MR-ONCE")).outcome).toBe("done");
+    expect((await finalize("FN-SG-MR-ONCE")).outcome).toBe("already-done");
+    expect(await persistedColumn("FN-SG-MR-ONCE")).toBe(MERGED_RENAMED_VOCAB.complete);
+  });
+
 });