U11 PR1: pin the merged intake+hold column contract before the IR moves (a mutation proved the first 7 tests insufficient) (#2495)

First of several small PRs for **U11** (merge Todo into Planning). **Tests only — no production change.** It lands the precondition so the IR edit arrives on proven substrate instead of an assumption. ## Decision taken (reversible, proceeding on it) **The surviving Planning column keeps the id `todo`; `triage` is deleted.** Same board the operator asked for — one column labelled "Planning", no "Todo" — via the cheaper and safer half. Measured, comments excluded, non-test, `packages/*/src` + `dashboard/app`: | | guards | writes | fallbacks | total | |---|---:|---:|---:|---:| | `"todo"` | 121 | 68 | 9 | 323 | | `"triage"` | 90 | 30 | 21 | 304 | Deleting `triage` instead of `todo` also means **no data migration** (every live card in `todo` is already in the surviving column) and **no guard changes meaning** (`column === "todo"` still denotes the hold column). Under the plan's letter the opposite is true, and worse than "dead": because Coding (Ideas) keeps `todo` per R10/R11, a surviving `column === "todo"` guard would stay live for Ideas cards while silently never matching for Coding cards — workflow-dependent, not dead. This is also a proven in-tree pattern rather than a new idea: **`builtin:coding-ideas` already ships this exact merge** — id `todo`, display name "Planning", `hold(capacity)` + `reset-on-entry`, plan-in-place. Consequence worth flagging: **U11 no longer waits on Phase B.** The 121 `todo` guards keep their meaning, so converting them becomes U12 cleanup rather than a U11 blocker. ## What this PR pins Nothing in tree has ever carried `intake` and `hold` on one column. Every built-in splits them. KTD-1 asserts the merged shape works; that assertion was untested. ## The result, reported as found **All 11 assertions passed on the first run against unmodified sources.** The merged column is already supported by trait resolution, the capacity sweep, and the release gate. **I could not make the first seven fail**, so they are a regression floor — not evidence of a fix, and I am not claiming them as one. What makes them worth keeping is that they are *differential*: the same scenario runs against the split-role vocabulary and the merged one and asserts the role-level outcomes are **equal**, so a literal creeping into any path fails the merged half while the split half stays green. ## The finding **The first seven tests were not enough, and proving that is the point of this PR.** A mutation encoding the plausible-but-wrong belief *"an intake column has no releaser"*: ```diff - if (currentFlags.intake !== true && currentFlags.hold !== true) return false; + if (currentFlags.intake === true) return false; + if (currentFlags.hold !== true) return false; ``` left **all seven green**. That belief is not hypothetical — it is stated verbatim in `builtin-plan-review-group.ts`'s own FNXC comment as the reason Plan Review lives in `todo` rather than `triage` today. Under U11 the planning column **is** an intake column, so any code encoding it silently stops holding unplanned cards and they release into implementation with a bootstrap stub for a spec. The gap: nothing reached `isUnplannedForExecution`. The mock store had no `getTasksDir`, so both halves of the gate returned early — the tests were exercising less than they appeared to. The fourth block drives it with a real temp dir and a real bootstrap `PROMPT.md`. **Re-running the same mutation now fails exactly one test — the merged-column one — while its split-shape twin stays green.** That discrimination is what the suite is for. ## Verification 26 tests green across this file plus `hold-release-renamed-columns`, `hold-release-instrumentation`, and `pre-release-plan-review`. Lint clean. No production file touched, so there is nothing to regress. ## Next PRs in this unit 1. Entry-contract test for `start` in a hold-carrying column (the specific interaction the earlier, reverted attempt got wrong). 2. The ~10-line IR change itself — deliberately last, per KTD-7. 3. The intake-lane `triage` conversion: the 21 `?? "triage"` creation defaults are the dangerous ones, since they would silently create cards into a column that no longer exists. `self-healing.ts` (11 triage guards + 4 writes) is the main worker's file — I am not touching it and will hand over the line list rather than race them. 🤖 Generated with [Claude Code](https://claude.com/claude-code)  ## Summary by CodeRabbit * **Tests** * Added coverage for merged Planning column behavior across split and merged workflow configurations. * Verified intake and hold resolution, rebound targeting, capacity hold/release outcomes, and execution gating. * Confirmed planned cards are released appropriately while cards already in progress are not unnecessarily held.  Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-28 11:36:05 -07:00
parent a271f1868f
commit 8492278fdd
1 changed files with 308 additions and 0 deletions
--- a/packages/engine/src/tests/merged-intake-hold-column.test.ts
+++ b/packages/engine/src/tests/merged-intake-hold-column.test.ts
@@ -0,0 +1,308 @@
+/*
+FNXC:MergedPlanningColumn 2026-07-28-09:10 (U11 precondition):
+
+U11 merges Todo into Planning: ONE pre-implementation column that both intakes new
+cards and holds them for capacity. KTD-1 asserts that shape works. Nothing in tree
+has ever exercised it — every built-in splits the two roles across two columns
+(`triage` intake + `todo` hold; Coding (Ideas) `ideas` intake + `todo` hold), so
+"a column carrying intake AND hold" is an untested configuration, not an
+established one.
+
+These tests pin the contract BEFORE the IR changes, so the ~10-line IR edit lands
+on proven substrate rather than on an assumption. They are differential: the SAME
+scenario runs against the split-role vocabulary (today's shape) and the merged one
+(U11's shape) and asserts the role-level outcomes match. A difference is therefore
+attributable to the merge itself and to nothing else.
+
+The failure mode being guarded is not a wrong decision but NO decision — a column
+whose role a path fails to recognize makes the card invisible to that path, which
+presents as a quietly stuck card rather than an error. That is how the earlier
+attempt at this merge stranded cards (see
+docs/solutions/architecture-patterns/workflow-node-column-placement-and-graph-entry-contract.md).
+
+Harness deliberately mirrors `hold-release-renamed-columns.test.ts` (Phase B slice
+B2) so the two differential suites stay comparable.
+*/
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import type { Task, TaskStore, WorkflowIr } from "@fusion/core";
+import { resolveLifecycleColumns, resolveReboundTarget, columnsWithFlag } from "@fusion/core";
+
+import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { buildBootstrapPrompt } from "@fusion/core";
+
+import { runHoldReleaseSweep, resetHoldReleaseInstrumentation, isUnplannedForExecution } from "../hold-release.js";
+import { schedulerLog } from "../logger.js";
+
+const WF = "custom:wf";
+
+function task(over: Partial<Task> = {}): Task {
+  return {
+    id: "FN-1",
+    title: "t",
+    description: "",
+    column: "todo",
+    status: null,
+    dependencies: [],
+    steps: [],
+    currentStep: 0,
+    log: [],
+    createdAt: "2026-01-01T00:00:00.000Z",
+    updatedAt: "2026-01-01T00:00:00.000Z",
+    columnMovedAt: "2026-01-01T00:00:00.000Z",
+    ...over,
+  } as Task;
+}
+
+/**
+ * SPLIT — today's built-in shape: intake and hold are different columns.
+ * MERGED — U11's shape: one column carries both. `planning` is deliberately NOT
+ * a legacy id, so a surviving `=== "todo"` / `=== "triage"` comparison cannot
+ * pass by luck.
+ */
+const SPLIT = { intake: "triage", hold: "todo", wip: "in-progress", complete: "done" };
+const MERGED = { intake: "planning", hold: "planning", wip: "in-progress", complete: "done" };
+
+function ir(names: { intake: string; hold: string; wip: string; complete: string }): WorkflowIr {
+  const preImplementation = names.intake === names.hold
+    ? [{
+        id: names.hold,
+        name: "Planning",
+        traits: [
+          { trait: "intake" },
+          { trait: "hold", config: { release: "capacity" } },
+          { trait: "reset-on-entry" },
+        ],
+      }]
+    : [
+        { id: names.intake, name: "Planning", traits: [{ trait: "intake" }] },
+        {
+          id: names.hold,
+          name: "Todo",
+          traits: [{ trait: "hold", config: { release: "capacity" } }, { trait: "reset-on-entry" }],
+        },
+      ];
+
+  return {
+    version: "v2",
+    id: WF,
+    nodes: [
+      { id: "start", kind: "start", column: names.intake },
+      { id: "planning", kind: "prompt", column: names.hold },
+      { id: "execute", kind: "prompt", column: names.wip },
+      { id: "end", kind: "end", column: names.complete },
+    ],
+    edges: [
+      { from: "start", to: "planning", condition: "success" },
+      { from: "planning", to: "execute", condition: "success" },
+      { from: "execute", to: "end", condition: "success" },
+    ],
+    columns: [
+      ...preImplementation,
+      { id: names.wip, name: "In progress", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
+      { id: names.complete, name: "Done", traits: [{ trait: "complete" }] },
+    ],
+  } as unknown as WorkflowIr;
+}
+
+function storeWith(tasks: Task[], workflowIr: WorkflowIr, settings: Record<string, unknown>): TaskStore {
+  const selection = { workflowId: WF, stepIds: [] };
+  return {
+    getSettings: vi.fn(async () => settings),
+    listTasks: vi.fn(async () => tasks),
+    getTask: vi.fn(async (id: string) => tasks.find((t) => t.id === id) ?? null),
+    moveTaskIf: vi.fn(async (id: string, column: string) => {
+      const cur = tasks.find((t) => t.id === id)!;
+      cur.column = column;
+      return { task: cur, moved: true };
+    }),
+    logEntry: vi.fn(async () => undefined),
+    recordRunAuditEvent: vi.fn(async () => undefined),
+    getCompletionHandoffAcceptedMarker: vi.fn(async () => null),
+    getTaskWorkflowSelection: vi.fn(() => selection),
+    getTaskWorkflowSelectionAsync: vi.fn(async () => selection),
+    getWorkflowDefinition: vi.fn(async () => ({ ir: workflowIr })),
+  } as unknown as TaskStore;
+}
+
+/** Run the capacity scenario and report in ROLE terms so the two shapes compare directly. */
+async function capacityScenario(names: typeof SPLIT) {
+  const held = task({ id: "H", column: names.hold });
+  const occupant = task({ id: "O", column: names.wip });
+  const store = storeWith([held, occupant], ir(names), { maxConcurrent: 1 });
+
+  const saturated = await runHoldReleaseSweep(store, { now: () => 1_000_000 });
+  occupant.column = names.complete;
+  const freed = await runHoldReleaseSweep(store, { now: () => 1_045_000 });
+
+  return {
+    heldWhileSaturated: saturated.held.some((h) => h.taskId === "H"),
+    heldReason: saturated.held.find((h) => h.taskId === "H")?.reason,
+    releasedWhileSaturated: saturated.released,
+    releasedOnceFreed: freed.released,
+    landedInWipRole: held.column === names.wip,
+  };
+}
+
+describe("a column carrying BOTH intake and hold (U11's merged Planning column)", () => {
+  beforeEach(() => {
+    resetHoldReleaseInstrumentation();
+    vi.restoreAllMocks();
+    vi.spyOn(schedulerLog, "log").mockImplementation(() => {});
+    vi.spyOn(schedulerLog, "debug").mockImplementation(() => {});
+    vi.spyOn(schedulerLog, "warn").mockImplementation(() => {});
+  });
+
+  describe("trait resolution", () => {
+    it("resolves intake and hold to the SAME column without either role being lost", () => {
+      const columns = resolveLifecycleColumns(ir(MERGED));
+
+      expect(columns).toBeDefined();
+      expect(columns?.intake).toBe("planning");
+      expect(columns?.hold).toBe("planning");
+      expect(columns?.wip).toBe("in-progress");
+      expect(columns?.complete).toBe("done");
+    });
+
+    it("reports the merged column under both trait queries", () => {
+      const merged = ir(MERGED);
+
+      expect(columnsWithFlag(merged, "intake")).toContain("planning");
+      expect(columnsWithFlag(merged, "hold")).toContain("planning");
+    });
+
+    it("rebounds to the merged column (hold wins, and it is the same column as intake)", () => {
+      expect(resolveReboundTarget(ir(MERGED))).toBe("planning");
+      // Split shape prefers hold over intake — the merged shape must not change that preference,
+      // it just makes the two answers coincide.
+      expect(resolveReboundTarget(ir(SPLIT))).toBe("todo");
+    });
+  });
+
+  describe("capacity hold and release", () => {
+    it("holds and releases identically whether the pre-implementation roles are split or merged", async () => {
+      const split = await capacityScenario(SPLIT);
+      resetHoldReleaseInstrumentation();
+      const merged = await capacityScenario(MERGED);
+
+      // The split run is not vacuously equal: it really did hold, then release.
+      expect(split.heldWhileSaturated).toBe(true);
+      expect(split.releasedOnceFreed).toEqual(["H"]);
+      expect(split.landedInWipRole).toBe(true);
+
+      // …and the merged column produces the identical role-level outcome.
+      expect(merged).toEqual(split);
+    });
+
+    it("does not make a card in the merged column INVISIBLE to the sweep", async () => {
+      /*
+      The failure this unit most fears. An unrecognized pre-implementation column
+      produces no decision at all — the card is neither held nor released, which
+      presents as a quietly stuck card rather than an error. Assert a positive
+      decision was recorded, not merely "it wasn't released".
+      */
+      const held = task({ id: "H", column: MERGED.hold });
+      const occupant = task({ id: "O", column: MERGED.wip });
+      const store = storeWith([held, occupant], ir(MERGED), { maxConcurrent: 1 });
+
+      const result = await runHoldReleaseSweep(store, { now: () => 1_000_000 });
+
+      expect(result.held.map((h) => h.taskId)).toContain("H");
+      expect(result.held.find((h) => h.taskId === "H")?.reason).toBe("downstream-full");
+    });
+
+    it("releases a merged-column card straight into the wip column when capacity is free", async () => {
+      const held = task({ id: "H", column: MERGED.hold });
+      const store = storeWith([held], ir(MERGED), { maxConcurrent: 5 });
+
+      const result = await runHoldReleaseSweep(store, { now: () => 1_000_000 });
+
+      expect(result.released).toEqual(["H"]);
+      expect(held.column).toBe(MERGED.wip);
+    });
+
+    it("does not release a merged-column card that is user-paused", async () => {
+      const held = task({ id: "H", column: MERGED.hold, userPaused: true, paused: true } as Partial<Task>);
+      const store = storeWith([held], ir(MERGED), { maxConcurrent: 5 });
+
+      const result = await runHoldReleaseSweep(store, { now: () => 1_000_000 });
+
+      expect(result.released).not.toContain("H");
+      expect(held.column).toBe(MERGED.hold);
+    });
+  });
+  /*
+  FNXC:MergedPlanningColumn 2026-07-28-09:55 (U11 precondition):
+
+  `isUnplannedForExecution` decides whether a pre-implementation card may cross into
+  execution, and it is the path U11 depends on most: BOTH of its halves ask "what kind
+  of column is this card resting in".
+
+  This block exists because the first cut of this file did NOT reach either half — a
+  mutation encoding the plausible-but-wrong belief "an intake column has no releaser"
+  (`if (currentFlags.intake === true) return false`) left all seven earlier tests GREEN.
+  That belief is not hypothetical: it is stated verbatim in
+  builtin-plan-review-group.ts's own FNXC comment as the reason plan review lives in
+  `todo` today. Under U11 the planning column IS an intake column, so anything encoding
+  that belief silently stops holding unplanned cards and they release into
+  implementation with a bootstrap stub for a spec.
+  */
+  describe("the release gate (isUnplannedForExecution)", () => {
+    let tasksDir: string;
+
+    beforeEach(() => {
+      tasksDir = mkdtempSync(join(tmpdir(), "fusion-u11-merged-"));
+    });
+
+    afterEach(() => {
+      rmSync(tasksDir, { recursive: true, force: true });
+    });
+
+    /** Write the bootstrap stub PROMPT.md that marks a card as not yet specified. */
+    function seedUnplannedPrompt(taskId: string, title: string, description: string) {
+      mkdirSync(join(tasksDir, taskId), { recursive: true });
+      writeFileSync(join(tasksDir, taskId, "PROMPT.md"), buildBootstrapPrompt(taskId, title, description), "utf-8");
+    }
+
+    function seedPlannedPrompt(taskId: string) {
+      mkdirSync(join(tasksDir, taskId), { recursive: true });
+      writeFileSync(join(tasksDir, taskId, "PROMPT.md"), "# Real spec\n\nActual planned work.\n", "utf-8");
+    }
+
+    function gateStore(): TaskStore {
+      return {
+        getTasksDir: () => tasksDir,
+        getSettings: vi.fn(async () => ({})),
+      } as unknown as TaskStore;
+    }
+
+    it("holds an UNPLANNED card resting in the merged intake+hold column", async () => {
+      const card = task({ id: "U1", title: "Unplanned", description: "d", column: MERGED.hold });
+      seedUnplannedPrompt("U1", "Unplanned", "d");
+
+      await expect(isUnplannedForExecution(gateStore(), card, ir(MERGED))).resolves.toBe(true);
+    });
+
+    it("reaches the same verdict for the split shape (the merge changes nothing)", async () => {
+      const card = task({ id: "U1", title: "Unplanned", description: "d", column: SPLIT.hold });
+      seedUnplannedPrompt("U1", "Unplanned", "d");
+
+      await expect(isUnplannedForExecution(gateStore(), card, ir(SPLIT))).resolves.toBe(true);
+    });
+
+    it("releases a PLANNED card from the merged column (the gate is not a blanket hold)", async () => {
+      const card = task({ id: "P1", title: "Planned", description: "d", column: MERGED.hold });
+      seedPlannedPrompt("P1");
+
+      await expect(isUnplannedForExecution(gateStore(), card, ir(MERGED))).resolves.toBe(false);
+    });
+
+    it("does not gate a card already in the wip column", async () => {
+      const card = task({ id: "W1", title: "Working", description: "d", column: MERGED.wip });
+      seedUnplannedPrompt("W1", "Working", "d");
+
+      await expect(isUnplannedForExecution(gateStore(), card, ir(MERGED))).resolves.toBe(false);
+    });
+  });
+});