From 67904f8a2c2fa23ce0d9bf1d1fa35a4f1d5101d1 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 29 Jul 2026 09:39:20 -0700 Subject: [PATCH] U11: merge Todo into Planning on the default lineage (+ the migration mechanism, and a measured safety audit that cuts the work list 32%) (#2515) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Merges Todo into Planning on the operator's real default workflow.** Held from merge pending the `triage` literal audit below — see *Gating*. ## The board change `builtin:coding` → `BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR` → clones `BUILTIN_STEPWISE_CODING_WORKFLOW_IR`. That IR now declares **five** columns, and `plan`, `plan-review`, `plan-replan` and `start` all live in the merged Planning column: ``` columns: todo="Planning", in-progress, in-review, done, archived start -> todo plan -> todo plan-review -> todo plan-replan -> todo parse -> in-progress (first implementation node) ``` The id stays `todo`, the display name becomes "Planning". That is the cheaper half: `todo` was already the hold column, so every trait lookup, task row, stored selection and the 121 `column === "todo"` guards keep their meaning, and **no stored row needs re-homing**. Promoting `triage` instead would have produced the same board while making those guards workflow-*dependent* — live for Coding (Ideas), silently dead for Coding. `builtin:legacy-coding` keeps its six-column shape, per the operator's decision. It exists to be the old thing. ## Entry contract, before and after each IR edit | | result | |---|---| | before the default-lineage edit | **15 passed** | | after the edit | **13 passed, 2 failed** | | after reading both | **15 passed** | Neither failure was routed around. One was a genuine expectation change (two planning entry points became one); the other was my own `mergeTodoIntoPlanning` helper throwing *"source IR is not the split-column shape this merge transforms"* — because production **is** the merged shape now. I **deleted** the helper rather than making it tolerant: a transform that has silently become a no-op asserts nothing. ## The safety argument, proven not asserted Entering at `start` is exactly what dragged cards backward in the three earlier reverted attempts. `merged-planning-start-node-no-move.test.ts` proves against the **real** boundary controller and **real** default IR that entering `start` performs no move (`moveTask` is never *called*), reaches no hold→wip capacity seam, and **still moves on a genuine crossing** so the no-op is same-column rather than a disabled boundary. Removing the controller's same-column short-circuit turns exactly the two no-move tests red. ## The migration mechanism A card can outlive its column. `resolveAllowedColumns` derives targets from graph adjacency, and an undeclared source has none — so it returned `[]` and **every** move was rejected with "Valid targets: none", including the one that would rescue the card. An undeclared source now resolves to the workflow's rebound target. Escape hatch, not relaxation: declared columns are untouched, and it offers the rebound target *only*, so a stranded card gets back **into** the lifecycle rather than a free jump past review. ## A real regression this surfaced `isDefaultWorkflowColumns` matched the legacy **six** ids as a set. The merged default declares five, so the match stopped firing and the default board fell through to neighbor-only adjacency, which **drops legal moves and invents an illegal one**: | edge | effect | |---|---| | `in-progress → done` | **dropped** — the mission-validation cross edge | | `in-review → todo` | **dropped** — review work back to planning | | `todo/done → archived` | **dropped** — the FN-4892 direct-archival edges | | `done → in-review` | **invented** — a backward edge no rule allows | Adjacency now derives from lifecycle **roles**. The load-bearing assertion: the legacy six still reproduce `VALID_TRANSITIONS` **verbatim**. Applied only when a workflow declares the full role set, so custom boards keep neighbor adjacency. ## Failure accounting (core package, vs a 49-failure baseline) | stage | failed | new | |---|---:|---:| | after the merge | 65 | 18 | | after the escape hatch | 52 | 5 | | after role-derived adjacency | 53 | 4 | The 4 remaining are 3 `builtin-workflows` expectations encoding the pre-merge shape and 1 create-intake expectation naming `triage` on `builtin:coding`. Two `schema-applier` and two `workflow-reconciliation-production-shape` failures appeared in intermediate runs and are **not mine** — both files pass in isolation (75/75 and 7/7). I re-ran each before attributing them, which is why the earlier "priority" flag on the reconciliation pair was withdrawn. Gate: **309/309**. Lint clean. ## Gating: the `triage` audit (`docs/solutions/architecture-patterns/u11-triage-literal-safety-audit.md`) Program tracking cited **58** `triage` comparisons. Measured with the same pattern: | | count | |---|---:| | raw comparisons | 87 | | inside comments | 1 | | **not a lifecycle column at all** | **15** | | column comparisons | 71 | | OR-paired with `"todo"` in the same expression | 32 | | **exclusive `triage` — the real work list** | **39** | **15 do not compare a column.** `role === "triage"`, `surface === "triage"`, `sessionPurpose === "triage"`, `entry.agent === "triage"` name the planning **agent**. Converting them would be actively wrong, and the failure — a planning agent that can't resolve its prompt template — would look nothing like a column bug. **One site changes an operator-visible affordance**, which is why per-site review beat a sweep: `TaskCard.tsx:1927` — `taskColumnFlags?.intake === true && task.column !== "triage"`. The literal is a **narrowing**, not a match. After the merge a Planning card has `intake === true` and `column === "todo"`, so the narrowing stops applying and **Start begins rendering on default Planning cards where it previously did not.** A sweep would have "converted" the literal and shipped the new affordance silently. These guards do not go **dead**, they go **workflow-dependent** — `triage` stays live for legacy-coding, Ideas, every linear built-in and any user workflow (R11) — which is harder to detect than dead. Work list and ownership are in the audit doc. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) --- .../u11-triage-literal-safety-audit.md | 125 ++++++++++++++ .../src/__tests__/builtin-workflows.test.ts | 47 ++++-- .../__tests__/role-derived-adjacency.test.ts | 71 ++++++++ .../store-create-intake-column.test.ts | 13 +- .../undeclared-source-column-escape.test.ts | 109 ++++++++++++ .../builtin-stepwise-coding-workflow-ir.ts | 45 ++++- packages/core/src/builtin-workflows.ts | 65 +++++-- packages/core/src/workflow-transitions.ts | 108 +++++++++++- ...merged-planning-start-node-no-move.test.ts | 158 ++++++++++++++++++ .../src/__tests__/spec-staleness.test.ts | 52 +++++- .../src/__tests__/task-pipeline-smoke.test.ts | 21 ++- .../workflow-graph-entry-contract.test.ts | 105 +++++++----- packages/engine/src/triage.ts | 90 +++++++++- 13 files changed, 927 insertions(+), 82 deletions(-) create mode 100644 docs/solutions/architecture-patterns/u11-triage-literal-safety-audit.md create mode 100644 packages/core/src/__tests__/role-derived-adjacency.test.ts create mode 100644 packages/core/src/__tests__/undeclared-source-column-escape.test.ts create mode 100644 packages/engine/src/__tests__/merged-planning-start-node-no-move.test.ts diff --git a/docs/solutions/architecture-patterns/u11-triage-literal-safety-audit.md b/docs/solutions/architecture-patterns/u11-triage-literal-safety-audit.md new file mode 100644 index 0000000000..c850c0a240 --- /dev/null +++ b/docs/solutions/architecture-patterns/u11-triage-literal-safety-audit.md @@ -0,0 +1,125 @@ +--- +category: architecture-patterns +module: "@fusion/core, @fusion/engine, @fusion/dashboard" +date: 2026-07-29 +problem_type: migration_audit +component: workflow-columns +severity: high +applies_when: + - "Landing U11's deletion of `triage` from the default coding workflow" + - "Deciding whether a `column === \"triage\"` site is broken by that deletion" + - "Counting the remaining lifecycle-literal conversion surface" +tags: + - workflow-columns + - u11 + - migration + - literal-conversion +related_components: + - workflow_graph + - triage + - self-healing + - dashboard +--- + +# The `triage` literal surface, measured per site + +U11 merges Todo into Planning by keeping the id `todo` and **deleting `triage`** from the default +coding lineage. Every surviving `column === "triage"` comparison is therefore a candidate for +silent breakage — a guard that stops matching does not fail a test, it disables a path. + +This audit exists because the headline count is misleading in both directions, and shipping the +deletion on top of an estimate is how a green merge produces a broken board. + +## The count, and why the headline number is wrong + +Program-level tracking cited **58** `triage` comparisons. Measured directly with the same pattern +across `packages/*/src` + `packages/dashboard/app`, excluding tests: + +| | count | meaning | +|---|---:|---| +| Raw `=== "triage"` / `!== "triage"` | **87** | the grep everyone quotes | +| — inside comments | 1 | not code | +| — **not a column at all** | **15** | `role`, `agentType`, `sessionPurpose`, `surface`, `entry.agent` | +| **Column comparisons** | **71** | the only ones the deletion can reach | +| — OR-paired with `"todo"` in the SAME expression | **32** | a merged Planning card still matches | +| **Exclusive `triage`, needing individual proof** | **39** | the real work list | + +Two things this changes: + +1. **15 of the 87 are not lifecycle columns.** `agent-prompts.ts`'s `role === "triage"`, + `tool-availability.ts`'s `surface === "triage"`, `skill-resolver.ts`'s + `sessionPurpose === "triage"`, `TaskChatTab.tsx`'s `entry.agent === "triage"` — these name the + **planning agent**, not the planning column, and are unaffected by any IR change. Converting + them would be actively wrong. A file-level count cannot see this distinction. + +2. **32 more are already safe** because the branch accepts `todo` in the same expression. A card + that used to be in `triage` is now in `todo`, so it still matches. These need no change and no + test. + +So the surface that actually gates U11 is **39 sites**, not 58 and not 87. + +## Why "not a column" is not a technicality + +`triage` is overloaded in this codebase: it is a column id, an agent role, a session purpose, and +a prompt-template family. Only the first is affected by the IR. A conversion sweep driven by the +raw grep would rewrite the other three, and the resulting failure — a planning agent that no +longer resolves its own prompt template — would look nothing like a column bug. + +## The safety rule + +After the merge, a default-workflow card that used to rest in `triage` rests in `todo`. So: + +- **SAFE** — the site's `triage` branch also accepts `todo`, or resolves by trait + (`flags.intake`/`flags.hold`), or compares a resolved entry column rather than a literal. +- **NEEDS PROOF** — the `triage` branch is exclusive. Then ask: *when this stops matching for a + default-workflow card, what does the operator lose?* If the answer is nothing, the site is dead + for that lineage and safe. If the answer is an affordance or a recovery path, it is breakage. + +`triage` remains a **live column id** for `builtin:legacy-coding`, Coding (Ideas), every linear +built-in, and any user-authored workflow (R11). So these guards do not go dead — they go +**workflow-dependent**, which is harder to detect than dead. That is the reason for per-site proof +rather than a sweep. + +## The 39, by owner + +Ownership follows the program's file assignments; this audit does not claim them. + +| File | sites | owner | +|---|---:|---| +| `engine/src/self-healing.ts` | 7 | capacity worker | +| `dashboard/src/routes/register-task-workflow-routes.ts` | 6 | U12 worker | +| `dashboard/app/components/TaskCard.tsx` | 6 | U12 worker | +| `core/src/task-store/task-creation.ts` | 3 | see note — already mitigated | +| `engine/src/replan-target.ts` | 3 | U7 worker (2 are comments) | +| `dashboard/app/components/ListView.tsx` | 3 | U12 worker (1 is a comment) | +| `dashboard/app/components/TaskDetailModal.tsx` | 2 | U12 worker | +| `dashboard/app/components/TaskContextMenu.tsx` | 2 | already trait-paired — safe | +| remaining 7 files | 1 each | scattered | + +### Already resolved or safe on inspection + +- **`TaskContextMenu.tsx:143`** — `column === "triage" || flags?.intake || flags?.hold`. Already + trait-paired (U10). The literal is a legacy fallback, not the decision. +- **`task-creation.ts:493/863`** — `isIntakeColumn` ORs the literal with the **resolved** entry + column, so a merged Planning card matches through the resolved half. The expression-level pairing + check misses this because it pairs on the literal `"todo"`, not on a resolved variable. Mitigated + further by the `resolveDefaultWorkflowIntakeColumn` fix already on this branch. +- **`replan-target.ts:100`, `ListView.tsx:659`** — prose inside comments describing the old + behavior. No code effect. + +### Flagged as a real behavior change, not yet owned + +- **`TaskCard.tsx:1927`** — `taskColumnFlags?.intake === true && task.column !== "triage"`. The + literal here is a **narrowing**: it suppresses the Start affordance on the legacy intake column. + After the merge a default Planning card has `intake === true` and `column === "todo"`, so the + narrowing stops applying and **Start begins rendering on default Planning cards where it + previously did not**. That is an operator-visible affordance change, and it is the kind that the + Surface Enumeration rule exists to catch. It needs an explicit decision, not a mechanical + conversion. + +## Do not land the deletion on an estimate + +The instruction that produced this audit was correct: verify per site, do not assume. The measured +result is that the work list is **32% smaller** than the tracked figure, that **15 sites must not +be converted at all**, and that at least one site changes an operator-visible affordance in a way +no column-conversion sweep would have surfaced. diff --git a/packages/core/src/__tests__/builtin-workflows.test.ts b/packages/core/src/__tests__/builtin-workflows.test.ts index 162cacf33f..6e08bb9131 100644 --- a/packages/core/src/__tests__/builtin-workflows.test.ts +++ b/packages/core/src/__tests__/builtin-workflows.test.ts @@ -448,12 +448,26 @@ describe("built-in workflows", () => { // FNXC:Workflows 2026-07-05-00:00: FN-7599 — hand-authored default workflows (stepwise-coding, pr-workflow) // must also label the intake column "Planning" while keeping the "triage" id, matching builtin-coding. - it("hand-authored default workflows label the intake column 'Planning' (FN-7599)", () => { + /* + FNXC:MergedPlanningColumn 2026-07-29-12:10 (U11): + The invariant is "the intake column is labelled Planning", which FN-7599 established and U11 + preserves — but the intake column's ID now differs per workflow, so the test resolves it by + TRAIT instead of by the literal `triage`. Asserting the label through the trait is what makes + this survive the merge; asserting it through the id is what made it fail. + + Kept covering BOTH workflows deliberately: the stepwise IR merged (intake now rides on `todo`) + while the PR workflow did not (still `triage`), so this now proves the label invariant holds + across the two shapes rather than only the one. + */ + it("hand-authored default workflows label their intake column 'Planning' (FN-7599)", () => { for (const ir of [BUILTIN_STEPWISE_CODING_WORKFLOW_IR, BUILTIN_PR_WORKFLOW_IR]) { expect(ir.version).toBe("v2"); if (ir.version !== "v2") throw new Error("expected v2"); - const triageColumn = ir.columns.find((column) => column.id === "triage"); - expect(triageColumn).toEqual({ id: "triage", name: "Planning", traits: [{ trait: "intake" }] }); + const intakeColumns = ir.columns.filter( + (column) => column.traits.some((trait) => trait.trait === "intake"), + ); + expect(intakeColumns).toHaveLength(1); + expect(intakeColumns[0]!.name).toBe("Planning"); } }); @@ -516,8 +530,15 @@ describe("built-in workflows", () => { [ "builtin:coding", [ - { id: "triage", traits: ["intake"] }, - { id: "todo", traits: ["hold", "reset-on-entry"] }, + /* + FNXC:MergedPlanningColumn 2026-07-29-12:10 (U11): + The DEFAULT lineage declares ONE pre-implementation column. `triage` is gone and `todo` + carries intake + hold + reset-on-entry. Every OTHER entry in this map still lists + `triage` on purpose — legacy-coding, pr-workflow, marketing and the rest keep the split + shape, and R11 commits to that continuing to work. If a future change collapses them + too, that is a decision to make deliberately, not a diff to accept here. + */ + { id: "todo", traits: ["intake", "hold", "reset-on-entry"] }, { id: "in-progress", traits: ["wip", "abort-on-exit", "timing"] }, { id: "in-review", traits: ["merge-blocker", "human-review", "stall-detection", "merge"] }, { id: "done", traits: ["complete"] }, @@ -538,8 +559,10 @@ describe("built-in workflows", () => { [ "builtin:stepwise-coding", [ - { id: "triage", traits: ["intake"] }, - { id: "todo", traits: ["hold", "reset-on-entry"] }, + // FNXC:MergedPlanningColumn 2026-07-29-12:20 (U11): merged with builtin:coding above — + // this IS the IR the default lineage clones, so the two must agree here or the default + // board and its base would have drifted apart silently. + { id: "todo", traits: ["intake", "hold", "reset-on-entry"] }, { id: "in-progress", traits: ["wip", "abort-on-exit", "timing"] }, { id: "in-review", traits: ["merge-blocker", "human-review", "stall-detection", "merge"] }, { id: "done", traits: ["complete"] }, @@ -599,8 +622,13 @@ describe("built-in workflows", () => { expect(ir.version).toBe("v2"); if (ir.version !== "v2") throw new Error("expected v2"); + /* + FNXC:MergedPlanningColumn 2026-07-29-12:10 (U11): + Five columns, not the legacy six. This is the assertion that would have caught the merge + landing on the wrong constant, so it is updated rather than deleted: it still pins the exact + column set and trait order of the OPERATOR'S default board. + */ expect(ir.columns.map((column) => column.id)).toEqual([ - "triage", "todo", "in-progress", "in-review", @@ -608,8 +636,7 @@ describe("built-in workflows", () => { "archived", ]); expect(ir.columns.map((column) => column.traits.map((trait) => trait.trait))).toEqual([ - ["intake"], - ["hold", "reset-on-entry"], + ["intake", "hold", "reset-on-entry"], ["wip", "abort-on-exit", "timing"], ["merge-blocker", "human-review", "stall-detection", "merge"], ["complete"], diff --git a/packages/core/src/__tests__/role-derived-adjacency.test.ts b/packages/core/src/__tests__/role-derived-adjacency.test.ts new file mode 100644 index 0000000000..dd0121c040 --- /dev/null +++ b/packages/core/src/__tests__/role-derived-adjacency.test.ts @@ -0,0 +1,71 @@ +/* +FNXC:MergedPlanningColumn 2026-07-29-11:15 (U11): +Column adjacency derived from lifecycle ROLES rather than column ids. + +`VALID_TRANSITIONS` is a role-level statement wearing legacy-id clothing, and it was reachable +only by matching the legacy SIX ids as a set. U11's merged default declares FIVE, so that match +stopped firing and the default board fell through to neighbor-only adjacency — which both drops +legal moves and invents an illegal one. + +The critical assertion in this file is the FIRST one: the legacy six must still produce +`VALID_TRANSITIONS` verbatim. Everything else is worthless if that regressed. +*/ +import { describe, expect, it } from "vitest"; +import { resolveAllowedColumns, resolveColumnAdjacency } from "../workflow-transitions.js"; +import { VALID_TRANSITIONS } from "../types/board-config.js"; +import { getBuiltinWorkflow, parseWorkflowIr, type WorkflowIr } from "../index.js"; + +const defaultIr: WorkflowIr = parseWorkflowIr(getBuiltinWorkflow("builtin:coding")!.ir as never); +const legacyIr: WorkflowIr = parseWorkflowIr(getBuiltinWorkflow("builtin:legacy-coding")!.ir as never); + +describe("column adjacency survives the Todo→Planning merge", () => { + it("still reproduces VALID_TRANSITIONS verbatim for the legacy six-column shape", () => { + const adjacency = resolveColumnAdjacency(legacyIr); + for (const [from, targets] of Object.entries(VALID_TRANSITIONS)) { + expect(adjacency.get(from)?.slice().sort()).toEqual([...targets].sort()); + } + }); + + it("keeps the in-progress → done mission-validation cross edge on the merged default", () => { + // The edge `custom-review-lane-merge-blocker` covers. Neighbor adjacency dropped it. + expect(resolveAllowedColumns(defaultIr, "in-progress")).toContain("done"); + }); + + it("keeps review → planning and the direct-archival edges on the merged default", () => { + expect(resolveAllowedColumns(defaultIr, "in-review")).toContain("todo"); + expect(resolveAllowedColumns(defaultIr, "todo")).toContain("archived"); + expect(resolveAllowedColumns(defaultIr, "done")).toContain("archived"); + }); + + it("does NOT invent a backward done → in-review edge", () => { + // Neighbor adjacency produced this purely from declaration order; no rule ever allowed it. + expect(resolveAllowedColumns(defaultIr, "done")).not.toContain("in-review"); + }); + + it("emits no self-edge for the merged planning column", () => { + // intake and hold resolve to the same column here; the collapse must not leave todo → todo. + expect(resolveAllowedColumns(defaultIr, "todo")).not.toContain("todo"); + }); + + it("leaves a genuinely custom workflow on neighbor adjacency", () => { + /* + Regression direction: role-derived adjacency must apply ONLY to workflows declaring the full + lifecycle role set. A shape missing roles is custom, and silently imposing a lifecycle on it + would change every custom board's legal moves. + */ + const custom = { + version: "v2", + id: "wf-custom", + name: "Custom", + columns: [ + { id: "a", name: "A", traits: [{ trait: "intake" }] }, + { id: "b", name: "B", traits: [] }, + { id: "c", name: "C", traits: [] }, + ], + nodes: [{ id: "start", kind: "start", column: "a" }], + edges: [], + } as unknown as WorkflowIr; + + expect(resolveAllowedColumns(custom, "b")).toEqual(["a", "c"]); + }); +}); diff --git a/packages/core/src/__tests__/store-create-intake-column.test.ts b/packages/core/src/__tests__/store-create-intake-column.test.ts index 816d3e190b..9f27a3d384 100644 --- a/packages/core/src/__tests__/store-create-intake-column.test.ts +++ b/packages/core/src/__tests__/store-create-intake-column.test.ts @@ -86,14 +86,23 @@ pgTest("createTask intake-column wiring (Coding (Ideas))", () => { expect(task.column).toBe("ideas"); }); - it("lands a task explicitly selecting builtin:coding in triage even when the project default is coding-ideas", async () => { + /* + FNXC:MergedPlanningColumn 2026-07-29-12:25 (U11): + The INVARIANT here is "an explicit create-time workflowId beats the project default", not "the + answer is the literal `triage`". U11 merges Todo into Planning on builtin:coding, so its intake + column is now `todo` — the test asserts the invariant through the selected workflow's own + resolved intake column so it cannot drift again the next time a column id moves. + */ + it("lands a task explicitly selecting builtin:coding in ITS intake column, even when the project default is coding-ideas", async () => { const store = h.store(); await store.setDefaultWorkflowId("builtin:coding-ideas"); const task = await store.createTask({ description: "explicit default coding workflow task", workflowId: "builtin:coding", }); - expect(task.column).toBe("triage"); + // builtin:coding's merged Planning column; explicitly NOT coding-ideas' `ideas` intake. + expect(task.column).toBe("todo"); + expect(task.column).not.toBe("ideas"); }); it("does not throw and falls back to triage when workflowId is explicitly null (\"No workflow\")", async () => { diff --git a/packages/core/src/__tests__/undeclared-source-column-escape.test.ts b/packages/core/src/__tests__/undeclared-source-column-escape.test.ts new file mode 100644 index 0000000000..51a844087b --- /dev/null +++ b/packages/core/src/__tests__/undeclared-source-column-escape.test.ts @@ -0,0 +1,109 @@ +/* +FNXC:MergedPlanningColumn 2026-07-29-10:15 (U11 migration): + +A card can outlive the column it is stored in. U11 removes `triage` from the default coding +workflow, so on the first startup after upgrade every card still sitting there is in a column its +own workflow no longer declares — and `reconcileUndeclaredTaskColumns` re-homes those, but only +when it runs. + +In between, the card was UNMOVABLE. `resolveAllowedColumns` derives targets from the workflow's +column adjacency, and an undeclared source column has no adjacency at all, so it returns `[]` and +every move is rejected with "Valid targets: none" — including the move that would rescue the card. +An operator dragging such a card got a hard rejection with nothing actionable in it. + +The fix is an ESCAPE HATCH, not a relaxation: when — and only when — the card's CURRENT column is +one the workflow does not declare, the workflow's own rebound target (hold -> intake -> first +column) becomes a legal destination. Every other guard is untouched, because there is no adjacency +to violate from a column that is not in the graph. + +Deliberately narrow: the rebound target ONLY, not "any declared column". A stranded card needs a +way back into the lifecycle, not a way to skip it — allowing any target would let a card jump +straight from a removed planning column into a review or complete column, which the ordinary +adjacency rules exist to prevent. An operator who wants it elsewhere moves it twice. +*/ +import { describe, expect, it } from "vitest"; +import { resolveAllowedColumns } from "../workflow-transitions.js"; +import { getBuiltinWorkflow, parseWorkflowIr, type WorkflowIr } from "../index.js"; + +/** The real default workflow, post-merge: one Planning column (`todo`), no `triage`. */ +const defaultIr: WorkflowIr = parseWorkflowIr(getBuiltinWorkflow("builtin:coding")!.ir as never); + +/** A workflow whose lifecycle roles carry non-legacy ids, so no literal can pass by luck. */ +function renamedIr(): WorkflowIr { + return { + version: "v2", + id: "wf-renamed", + name: "Renamed", + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], + nodes: [ + { id: "start", kind: "start", column: "backlog" }, + { id: "build", kind: "prompt", column: "building" }, + { id: "end", kind: "end", column: "shipped" }, + ], + edges: [ + { from: "start", to: "build", condition: "success" }, + { from: "build", to: "end", condition: "success" }, + ], + } as unknown as WorkflowIr; +} + +describe("a card stored in a column its workflow no longer declares can still move", () => { + it("premise: the default workflow really has dropped `triage`", () => { + // Asserted separately so a future re-declaration names its own cause rather than + // surfacing as a confusing empty-targets assertion below. + expect((defaultIr as { columns: Array<{ id: string }> }).columns.map((c) => c.id)).not.toContain("triage"); + }); + + it("offers the workflow's rebound target as an escape from an undeclared column", () => { + const allowed = resolveAllowedColumns(defaultIr, "triage"); + + // Before the escape hatch this was `[]` — the card could not be moved anywhere at all. + expect(allowed.length).toBeGreaterThan(0); + // `todo` is the merged Planning column: hold, and therefore the rebound target. + expect(allowed).toContain("todo"); + }); + + it("escapes to a RENAMED workflow's rebound target, never to a legacy literal", () => { + const allowed = resolveAllowedColumns(renamedIr(), "triage"); + + expect(allowed).toEqual(["backlog"]); + expect(allowed).not.toContain("todo"); + }); + + it("does NOT offer a free jump into review or complete columns", () => { + /* + The reason this is an escape hatch and not a relaxation. A stranded card needs a way back into + the lifecycle, not a way to skip it — otherwise a card in a removed planning column could be + moved straight to done, bypassing every gate the adjacency rules encode. + */ + const allowed = resolveAllowedColumns(defaultIr, "triage"); + + expect(allowed).not.toContain("in-review"); + expect(allowed).not.toContain("done"); + expect(allowed).not.toContain("archived"); + }); + + it("leaves DECLARED columns' adjacency completely untouched", () => { + /* + The regression direction that matters. A change that made every column fall back to the rebound + target would satisfy the assertions above while destroying the lifecycle. Every declared column + must keep exactly the targets its graph gives it. + */ + for (const columnId of ["todo", "in-progress", "in-review"]) { + const allowed = resolveAllowedColumns(defaultIr, columnId); + // Declared columns resolve from the real graph, so they must NOT collapse to a single + // rebound target — and in particular a wip column must not suddenly offer only `todo`. + expect(allowed).not.toEqual(["todo"]); + } + }); + + it("returns no escape when the workflow declares no columns at all (v1 IR)", () => { + // Nothing to rebound to; the caller keeps its conservative rejection rather than inventing one. + const v1 = { version: "v1", id: "legacy", name: "legacy", nodes: [], edges: [] } as unknown as WorkflowIr; + expect(resolveAllowedColumns(v1, "triage")).toEqual([]); + }); +}); diff --git a/packages/core/src/builtin-stepwise-coding-workflow-ir.ts b/packages/core/src/builtin-stepwise-coding-workflow-ir.ts index 4d49f28388..e631df9412 100644 --- a/packages/core/src/builtin-stepwise-coding-workflow-ir.ts +++ b/packages/core/src/builtin-stepwise-coding-workflow-ir.ts @@ -60,11 +60,41 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { version: "v2", name: "builtin-stepwise-coding", columns: [ - { id: "triage", name: "Planning", traits: [{ trait: "intake" }] }, + /* + FNXC:MergedPlanningColumn 2026-07-28-17:20 (U11 / R1, R2): + ONE pre-implementation column. Specification, Plan Review and the replan loop all run here, and + the card leaves only when the scheduler releases it against implementation capacity — so a card + being planned never holds an implementation slot. This is the DEFAULT lineage: `builtin:coding` + resolves to the final-review variant, which clones this IR. + + The id stays `todo`; the DISPLAY name becomes "Planning". Deliberate, and the cheaper half of + the merge: `todo` was already the hold column, so every trait lookup, task row, stored + selection and the 121 `column === "todo"` guards still in the engine keep meaning exactly what + they meant, and no stored row needs re-homing. Promoting `triage` instead would have produced + the same board while making each of those guards workflow-DEPENDENT — still live for Coding + (Ideas), which keeps `todo` per R11, and silently dead for Coding. Harder to detect than dead. + `builtin:coding-ideas` already ships this same id-keeping merge. + + `intake` must sit on THIS column rather than a separate one upstream: an intake-only column has + no releaser — the capacity sweep only releases from a `hold` column — so a card parked there + waits for a human forever. That is what reverted the earlier attempts (see + docs/solutions/architecture-patterns/workflow-node-column-placement-and-graph-entry-contract.md). + + NOT applied to `builtin:legacy-coding` (BUILTIN_CODING_WORKFLOW_IR), which keeps the six-column + split shape on purpose: a workflow whose stated purpose is preserving the original pipeline + must not be silently reshaped, and R11 commits to legacy shapes continuing to work. + + `todo` stays a legal column id for stored rows and user-authored workflows (R11, KTD-8). What + is deleted is Todo the STAGE, not the string. + */ { id: "todo", - name: "Todo", - traits: [{ trait: "hold", config: { release: "capacity" } }, { trait: "reset-on-entry" }], + name: "Planning", + traits: [ + { trait: "intake" }, + { trait: "hold", config: { release: "capacity" } }, + { trait: "reset-on-entry" }, + ], }, { id: "in-progress", @@ -94,7 +124,14 @@ const RAW_BUILTIN_STEPWISE_CODING_WORKFLOW_IR: WorkflowIr = { // parses into task steps. artifacts: [{ key: "PROMPT.md", title: "Plan", producedBy: "planning", role: "step-source" }], nodes: [ - { id: "start", kind: "start", column: "triage" }, + /* + FNXC:MergedPlanningColumn 2026-07-28-17:20 (U11): + `start` moves into the merged planning column with the rest of the specification phase. Its + column is load-bearing: the graph entry contract resumes a continuation-less run at the first + node whose column is not BEHIND the card's, so a `start` left in an undeclared column would be + unplaceable. + */ + { id: "start", kind: "start", column: "todo" }, /* FNXC:PlanReviewStep 2026-07-26-17:10: PLAN-IN-PLACE: the whole specification phase — `plan`, `plan-review`, `plan-replan` — runs in the diff --git a/packages/core/src/builtin-workflows.ts b/packages/core/src/builtin-workflows.ts index 7c6585b0dc..ce3f5f9977 100644 --- a/packages/core/src/builtin-workflows.ts +++ b/packages/core/src/builtin-workflows.ts @@ -192,29 +192,69 @@ column of the node before it, so it stays wherever the pipeline already is. The one exception is a node that follows intake — planning happens in the hold column (plan-in-place), which is what `builtin:compound-engineering`'s `plan` node needs. */ -function columnForLinearNode(node: WorkflowIrNode, previousColumn: string): string { +/* +FNXC:MergedPlanningColumn 2026-07-28-16:05 (U11): +Every linear built-in takes its columns from `canonicalBuiltinWorkflowColumns()` — i.e. from +BUILTIN_CODING_WORKFLOW_IR — so merging Todo into Planning there removes `triage` from all of them +at once. These lifecycle homes were hardcoded ids and became dangling column references the moment +that happened (`Workflow node 'start' references undefined column 'triage'` — IR validation caught +it, which is the entry contract working). + +Resolved by TRAIT against the same canonical set the columns come from, so the two can no longer +disagree. Literals remain only as fallbacks for a canonical set that somehow declares no such role. +*/ +interface LinearLifecycleColumns { + intake: string; + hold: string; + wip: string; + review: string; + complete: string; +} + +function linearLifecycleColumns(columns: WorkflowIrColumn[]): LinearLifecycleColumns { + const first = (trait: string): string | undefined => + columns.find((column) => column.traits.some((t) => t.trait === trait))?.id; + const intake = first("intake") ?? "triage"; + return { + intake, + // A merged Planning column carries BOTH intake and hold, so these coincide — which is exactly + // what retires the "node after intake jumps to the hold column" exception below: it becomes a + // no-op rather than a rule that has to be deleted. + hold: first("hold") ?? intake, + wip: first("wip") ?? "in-progress", + review: first("merge-blocker") ?? "in-review", + complete: first("complete") ?? "done", + }; +} + +function columnForLinearNode( + node: WorkflowIrNode, + previousColumn: string, + lifecycle: LinearLifecycleColumns, +): string { // `start`/`end` are graph terminals, not column destinations (the boundary // never enters them), but they must still name a sane column: intake for the // creation column and the complete column for the terminal. - if (node.kind === "start") return "triage"; - if (node.kind === "end") return "done"; + if (node.kind === "start") return lifecycle.intake; + if (node.kind === "end") return lifecycle.complete; const seam = node.config?.seam; - if (seam === "execute") return "in-progress"; - if (seam === "review") return "in-review"; - if (seam === "merge") return "in-review"; - return previousColumn === "triage" ? "todo" : previousColumn; + if (seam === "execute") return lifecycle.wip; + if (seam === "review") return lifecycle.review; + if (seam === "merge") return lifecycle.review; + return previousColumn === lifecycle.intake ? lifecycle.hold : previousColumn; } /** Resolve every linear-spec node's column in graph order, threading the * previously-resolved column so unseamed nodes inherit it. */ -function assignLinearNodeColumns(nodes: WorkflowIrNode[]): WorkflowIrNode[] { - let previousColumn = "triage"; +function assignLinearNodeColumns(nodes: WorkflowIrNode[], columns: WorkflowIrColumn[]): WorkflowIrNode[] { + const lifecycle = linearLifecycleColumns(columns); + let previousColumn = lifecycle.intake; return nodes.map((node) => { if (node.column) { previousColumn = node.column; return node; } - const column = columnForLinearNode(node, previousColumn); + const column = columnForLinearNode(node, previousColumn, lifecycle); // `end` names the complete column but must not drag the inheritance chain // there — nothing follows it, so this is only defensive. if (node.kind !== "end") previousColumn = column; @@ -308,11 +348,12 @@ function linear(spec: BuiltinSpec): WorkflowDefinition { * FNXC:Workflows 2026-06-28-00:00: * Linear built-ins must mirror BUILTIN_CODING_WORKFLOW_IR column traits because the post-cutover hold/release sweep is the only hold→in-progress dispatcher. Both formerly-v1 linear graphs (quick-fix, review-heavy, design) and v2-only compound-engineering need a hold(capacity) column, in-progress wip, and in-review merge traits or their cards strand before implementation. */ + const linearColumns = canonicalBuiltinWorkflowColumns(); const ir = parseWorkflowIr({ version: "v2", name: spec.name, - columns: canonicalBuiltinWorkflowColumns(), - nodes: assignLinearNodeColumns(nodes), + columns: linearColumns, + nodes: assignLinearNodeColumns(nodes, linearColumns), edges, }); /* diff --git a/packages/core/src/workflow-transitions.ts b/packages/core/src/workflow-transitions.ts index ed81c1243d..bf21b93c47 100644 --- a/packages/core/src/workflow-transitions.ts +++ b/packages/core/src/workflow-transitions.ts @@ -36,6 +36,7 @@ import { VALID_TRANSITIONS } from "./types.js"; import type { Column } from "./types.js"; import type { WorkflowIr, WorkflowIrV2 } from "./workflow-ir-types.js"; import { DEFAULT_WORKFLOW_COLUMN_IDS } from "./workflow-ir.js"; +import { resolveLifecycleColumns, resolveReboundTarget } from "./workflow-lifecycle-traits.js"; /** A column→allowed-target-columns adjacency map. */ export type ColumnAdjacency = Map; @@ -73,6 +74,83 @@ function orderDerivedAdjacency(ir: WorkflowIrV2): ColumnAdjacency { return adj; } + +/* +FNXC:MergedPlanningColumn 2026-07-29-11:05 (U11): +`isDefaultWorkflowColumns` recognises the default workflow by matching the legacy SIX column ids +as a set. U11 merges Todo into Planning, so the default declares FIVE — the match stops firing and +the default board silently falls through to `orderDerivedAdjacency`, which is neighbor-only. + +That is a real, operator-visible loss, not a cosmetic one. Measured against `VALID_TRANSITIONS`, +neighbor adjacency both DROPS legal moves and INVENTS an illegal one: + + in-progress -> done DROPPED — the mission-validation cross edge, which is the exact case + `custom-review-lane-merge-blocker` covers + in-review -> todo DROPPED — sending review work back to planning + todo/done -> archived DROPPED — the FN-4892 direct-archival edges + done -> in-review INVENTED — a backward edge into review that no rule ever allowed + +So adjacency is derived from lifecycle ROLES instead of column ids. `VALID_TRANSITIONS` is a +role-level statement wearing legacy id clothing; expressing it that way makes it survive a rename +or a merge, which is the whole point of this program. Applied only when the workflow declares the +full lifecycle role set — anything less is a genuinely custom shape and keeps neighbor adjacency, +so no existing custom workflow changes behavior. + +For the legacy six, intake and hold are distinct columns and this reproduces `VALID_TRANSITIONS` +verbatim (asserted). For the merged shape the two roles resolve to the SAME column, so the +self-edges collapse and the remaining edges are exactly the legacy ones with `triage` folded in. +*/ +const ROLE_TRANSITIONS: Record = { + intake: ["hold", "archived"], + hold: ["wip", "intake", "archived"], + wip: ["review", "hold", "intake", "complete"], + review: ["complete", "wip", "hold", "intake"], + complete: ["hold", "intake", "archived"], + archived: ["complete"], +}; + +/** Role→column-id for this workflow, or `undefined` when a lifecycle role is missing. */ +function resolveRoleColumns(ir: WorkflowIrV2): Record | undefined { + const lifecycle = resolveLifecycleColumns(ir); + if (!lifecycle) return undefined; + const { intake, hold, wip, review, complete, archived } = lifecycle; + // A workflow missing any lifecycle role is a genuinely custom shape; neighbor adjacency is the + // honest answer there rather than a half-applied lifecycle. + if (!wip || !review || !complete || !archived) return undefined; + const planning = hold ?? intake; + if (!planning) return undefined; + return { + intake: intake ?? planning, + hold: planning, + wip, + review, + complete, + archived, + }; +} + +function roleDerivedAdjacency(ir: WorkflowIrV2): ColumnAdjacency | undefined { + const roles = resolveRoleColumns(ir); + if (!roles) return undefined; + const declared = new Set(ir.columns.map((c) => c.id)); + const adj: ColumnAdjacency = new Map(); + for (const [role, targetRoles] of Object.entries(ROLE_TRANSITIONS)) { + const fromColumn = roles[role]; + if (!fromColumn || !declared.has(fromColumn)) continue; + const targets: string[] = []; + for (const targetRole of targetRoles) { + const toColumn = roles[targetRole]; + // Skip self-edges (merged roles resolve to the same column) and undeclared targets. + if (!toColumn || toColumn === fromColumn || !declared.has(toColumn)) continue; + if (!targets.includes(toColumn)) targets.push(toColumn); + } + // Merged roles write the same key twice; union rather than overwrite. + const existing = adj.get(fromColumn) ?? []; + adj.set(fromColumn, [...existing, ...targets.filter((t) => !existing.includes(t))]); + } + return adj; +} + /** * Resolve the full column adjacency for a workflow IR. The default workflow * reproduces `VALID_TRANSITIONS` exactly; custom workflows use order-derived @@ -88,6 +166,8 @@ export function resolveColumnAdjacency(ir: WorkflowIr): ColumnAdjacency { if (isDefaultWorkflowColumns(v2)) { return defaultWorkflowAdjacency(); } + const roleDerived = roleDerivedAdjacency(v2); + if (roleDerived) return roleDerived; return orderDerivedAdjacency(v2); } @@ -98,7 +178,33 @@ export function resolveColumnAdjacency(ir: WorkflowIr): ColumnAdjacency { * legal targets"). */ export function resolveAllowedColumns(ir: WorkflowIr, fromColumn: string): string[] { - return resolveColumnAdjacency(ir).get(fromColumn) ?? []; + const adjacency = resolveColumnAdjacency(ir).get(fromColumn); + if (adjacency) return adjacency; + + /* + FNXC:MergedPlanningColumn 2026-07-29-10:25 (U11 migration): + A card can outlive the column it is stored in — U11 removes `triage` from the default coding + workflow, so after upgrade every card still sitting there is in a column its own workflow no + longer declares. Adjacency is derived from the graph, so an undeclared source has none, and this + returned `[]`: EVERY move rejected with "Valid targets: none", including the one that would + rescue the card. `reconcileUndeclaredTaskColumns` re-homes such rows, but only when it runs; in + between, an operator dragging the card got a hard rejection with nothing actionable in it. + + So an undeclared source column resolves to the workflow's own rebound target (hold -> intake -> + first declared column). This is an ESCAPE HATCH, not a relaxation: there is no adjacency to + violate from a column that is not in the graph, and every declared column keeps exactly the + targets its graph gives it — the `if (adjacency) return adjacency` above is unconditional. + + Deliberately the rebound target ONLY, not "any declared column". A stranded card needs a way back + INTO the lifecycle, not a way to skip it; allowing any target would let a card jump from a removed + planning column straight to a review or complete column, which the ordinary adjacency rules exist + to prevent. An operator who wants it elsewhere moves it twice. + + A workflow with no declared columns (v1 IR) has nothing to rebound to and still resolves to `[]`, + so callers keep their conservative rejection rather than being handed an invented target. + */ + const rebound = resolveReboundTarget(ir); + return rebound ? [rebound] : []; } /** True when `toColumn` is a defined column of the workflow. */ diff --git a/packages/engine/src/__tests__/merged-planning-start-node-no-move.test.ts b/packages/engine/src/__tests__/merged-planning-start-node-no-move.test.ts new file mode 100644 index 0000000000..a348293b72 --- /dev/null +++ b/packages/engine/src/__tests__/merged-planning-start-node-no-move.test.ts @@ -0,0 +1,158 @@ +/* +FNXC:MergedPlanningColumn 2026-07-28-18:30 (U11): + +THE SAFETY ARGUMENT FOR THE MERGE, proven rather than asserted. + +Merging Todo into Planning changes the graph's entry point for a planning-lane card: it now +resumes at `start` instead of at the specification node, because the two share a column. The +smoke test's expected node sequence changes from ["plan", ...] to ["start", "plan", ...] as a +result. + +"Just update the expected array" is the dangerous move here, and it is dangerous for a specific +reason: entering at `start` is EXACTLY what dragged cards backward in the three earlier, reverted +attempts at this merge. A run that re-entered at the first node of the first column pulled the +card back through columns it had already left, firing `abort-on-exit` on its live session and +stranding it in a pre-wip column with no releaser. + +The claim that makes it safe now is narrow and mechanical: `start` and the specification node are +in the SAME column, so entering `start` has no column to move to. This file proves that claim +against the real controller and the real production IR before the expectation is touched. If the +no-move property does not hold, the smoke failure is a genuine regression and the IR change is +wrong — so these tests are the gate on that decision, not decoration. + +Mechanism under test (`workflow-column-boundary.ts`): `onNodeEntry` returns at +`if (toColumn === column) return { kind: "entered" }` BEFORE any move, before the hold->wip +capacity seam, and before `moveTask` is reachable at all. +*/ +import { describe, expect, it, vi } from "vitest"; +import { getBuiltinWorkflow, parseWorkflowIr, type WorkflowIr, type WorkflowIrNode } from "@fusion/core"; +import { createWorkflowColumnBoundary } from "../workflow-column-boundary.js"; + +/** The real default workflow — `builtin:coding`, post-merge. Not a hand-written fixture. */ +const defaultIr: WorkflowIr = parseWorkflowIr(getBuiltinWorkflow("builtin:coding")!.ir as never); + +function nodeById(id: string): WorkflowIrNode { + const node = defaultIr.nodes.find((n) => n.id === id); + if (!node) throw new Error(`default workflow has no node '${id}'`); + return node; +} + +const startNode = () => defaultIr.nodes.find((n) => n.kind === "start")!; + +describe("merged planning column — entering `start` moves nothing (U11 safety argument)", () => { + it("puts `start` and the specification node in the SAME column (the premise)", () => { + /* + Every assertion below rests on this. Asserted first and separately so that if the premise ever + stops holding — a future edit moving `start` back to its own column — the failure names the + cause instead of surfacing as a confusing move-count mismatch. + */ + const start = startNode(); + const successors = defaultIr.edges.filter( + (edge) => edge.from === start.id + && (edge.condition === undefined || edge.condition === "success") + && edge.kind !== "rework", + ); + expect(successors).toHaveLength(1); + + const specificationNode = nodeById(successors[0]!.to); + expect(start.column).toBe(specificationNode.column); + expect(start.column).toBe("todo"); + }); + + it("performs NO move when a planning-column card enters `start`", async () => { + const moveTask = vi.fn(); + const onSuspend = vi.fn(); + const boundary = createWorkflowColumnBoundary({ + taskId: "FN-MERGED-1", + workflowId: "builtin:coding", + ir: defaultIr, + // The card is already in the merged planning column — the intake case. + initialColumn: "todo", + moveTask, + onSuspend, + } as never); + + const result = await boundary.onNodeEntry(startNode()); + + expect(result).toMatchObject({ kind: "entered" }); + // THE PROPERTY. Not "the move succeeded" — the move was never attempted. + expect(moveTask).not.toHaveBeenCalled(); + expect(boundary.currentColumn()).toBe("todo"); + }); + + it("performs NO move on the whole start → specification chain", async () => { + /* + One node entry proving no-move is not enough: the run continues into the specification node + immediately. Both entries must be no-ops, or the card moves one column and back — which is a + real transition pair with real trait side effects (reset-on-entry re-arming, timing accounting), + even though the start and end columns are equal. + */ + const moveTask = vi.fn(); + const boundary = createWorkflowColumnBoundary({ + taskId: "FN-MERGED-2", + workflowId: "builtin:coding", + ir: defaultIr, + initialColumn: "todo", + moveTask, + onSuspend: vi.fn(), + } as never); + + const start = startNode(); + const specificationId = defaultIr.edges.find( + (edge) => edge.from === start.id && (edge.condition === undefined || edge.condition === "success"), + )!.to; + + await boundary.onNodeEntry(start); + await boundary.onNodeEntry(nodeById(specificationId)); + + expect(moveTask).not.toHaveBeenCalled(); + expect(boundary.currentColumn()).toBe("todo"); + }); + + it("never reaches the hold→wip capacity seam while traversing the planning chain", async () => { + /* + The merged column carries BOTH `hold` and `intake`. A same-column entry must return before the + hold->wip boundary check, or entering `start` on a hold-carrying column could suspend the run + at a capacity seam it never actually crossed — a card parked waiting for capacity it does not + need, which presents as a silently stuck card. + */ + const onSuspend = vi.fn(); + const boundary = createWorkflowColumnBoundary({ + taskId: "FN-MERGED-3", + workflowId: "builtin:coding", + ir: defaultIr, + initialColumn: "todo", + moveTask: vi.fn(), + onSuspend, + } as never); + + const result = await boundary.onNodeEntry(startNode()); + + expect(onSuspend).not.toHaveBeenCalled(); + expect(result).not.toMatchObject({ kind: "suspended" }); + }); + + it("still MOVES on a real crossing, so the no-op is same-column and not a disabled boundary", async () => { + /* + The regression direction that matters. A change making `onNodeEntry` never move would satisfy + every assertion above while breaking the entire lifecycle. Prove the controller still crosses + when the columns genuinely differ. + */ + const moveTask = vi.fn().mockResolvedValue(undefined); + const boundary = createWorkflowColumnBoundary({ + taskId: "FN-MERGED-4", + workflowId: "builtin:coding", + ir: defaultIr, + // Coming from the wip column into a review node is a real crossing. + initialColumn: "in-progress", + moveTask, + onSuspend: vi.fn(), + } as never); + + const reviewNode = defaultIr.nodes.find((node) => node.column === "in-review")!; + await boundary.onNodeEntry(reviewNode); + + expect(moveTask).toHaveBeenCalledTimes(1); + expect(boundary.currentColumn()).toBe("in-review"); + }); +}); diff --git a/packages/engine/src/__tests__/spec-staleness.test.ts b/packages/engine/src/__tests__/spec-staleness.test.ts index 528661fba5..a8ef2a0eb7 100644 --- a/packages/engine/src/__tests__/spec-staleness.test.ts +++ b/packages/engine/src/__tests__/spec-staleness.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { evaluateSpecStaleness, getPromptPath } from "../spec-staleness.js"; +import { evaluateSpecStaleness, getPromptPath, shouldSkipSpecStalenessForPreservedProgress } from "../spec-staleness.js"; import { stat } from "node:fs/promises"; import { join } from "node:path"; import type { Settings, Task } from "@fusion/core"; @@ -309,3 +309,53 @@ describe("evaluateSpecStaleness", () => { }); }); }); + +/* +FNXC:MergedPlanningColumn 2026-07-29-12:55 (U11): +PER-SITE PROOF that `spec-staleness.ts`'s `task.column === "triage"` needs NO conversion, recorded +as a test because "I read it and it looked fine" is not evidence. + +The guard refuses to skip the staleness check while a card is being planned — that is exactly when +its PROMPT.md is being rewritten underneath it. The obvious U11 reading is that the `triage` +literal stops matching for default-workflow cards and the refusal silently disappears. + +It does not, and the reason matters: the refusal is carried by STATUS, not by column. A card being +planned has `status === "planning"` or `"needs-replan"`, both of which the same guard already +tests, and neither of which is column-dependent. The `triage` disjunct is belt-and-braces that +goes dead for the default lineage while remaining live for legacy-coding, Ideas and every linear +built-in (R11). + +Adding `|| task.column === "todo"` was TRIED and is wrong: it breaks "skips stale-spec rerouting +for parked tasks with preserved execution progress" above, which deliberately parks a `todo` card +with preserved progress and expects the skip. After the merge `todo` is both the planner column and +the capacity-hold column, so the column can no longer distinguish "being planned" from "parked +waiting for a slot" — only status can. That is the finding: for this site the merge removes the +column's ability to answer the question, and the code was already asking status instead. +*/ +describe("spec staleness — the planner guard is carried by status, not column (U11 proof)", () => { + const card = (over: Record) => ({ + column: "todo", + status: null, + currentStep: 3, + steps: [{ status: "done" }], + ...over, + }) as never; + + it("refuses to skip a card that is actively being planned, in the MERGED column", () => { + expect(shouldSkipSpecStalenessForPreservedProgress(card({ status: "planning" }))).toBe(false); + }); + + it("refuses to skip a card awaiting replan, in the MERGED column", () => { + expect(shouldSkipSpecStalenessForPreservedProgress(card({ status: "needs-replan" }))).toBe(false); + }); + + it("still SKIPS a parked card with preserved progress in the same merged column", () => { + // The behavior a column-based conversion would have destroyed: same column, different status, + // opposite correct answer. + expect(shouldSkipSpecStalenessForPreservedProgress(card({ status: null }))).toBe(true); + }); + + it("keeps refusing for the legacy planner column, which other workflows still declare", () => { + expect(shouldSkipSpecStalenessForPreservedProgress(card({ column: "triage" }))).toBe(false); + }); +}); diff --git a/packages/engine/src/__tests__/task-pipeline-smoke.test.ts b/packages/engine/src/__tests__/task-pipeline-smoke.test.ts index fc4fa28c20..74710e519e 100644 --- a/packages/engine/src/__tests__/task-pipeline-smoke.test.ts +++ b/packages/engine/src/__tests__/task-pipeline-smoke.test.ts @@ -123,11 +123,26 @@ describe("task pipeline smoke", () => { expect(result.context[WORKFLOW_ID_CONTEXT_KEY]).toBe("builtin-stepwise-final-review-coding"); /* FNXC:WorkflowGraphEntry 2026-07-26-17:10: - No `start`: this card is in `todo`, and a run with no continuation now resumes at the card's own - column instead of replaying the pipeline from the first column. `start` lives in `triage`, a - column this card has already left, so the trace begins at the first planning-lane node. + A run with no continuation resumes at the card's OWN column instead of replaying the pipeline + from the first column. + + FNXC:MergedPlanningColumn 2026-07-28-18:45 (U11): + `start` IS now expected, and the sequence legitimately changed rather than regressing. This + previously read "No `start`" because `start` lived in `triage` — a column this `todo` card had + already left, so resuming there would have dragged it backward. U11 merges Todo into Planning, + so `start` and the specification node share the card's own column: resuming at `start` moves + the card nowhere and the trace simply begins one node earlier. + + Entering at `start` is exactly what dragged cards backward in the three earlier, reverted + attempts at this merge, so the no-move property is PROVEN before this array was touched, not + assumed from it — see `merged-planning-start-node-no-move.test.ts`, which asserts against the + real boundary controller and the real default IR that entering `start` performs no move, + reaches no hold->wip capacity seam, and still moves on a genuine crossing (so the no-op is + same-column, not a disabled boundary). Removing the controller's same-column short-circuit + turns two of those tests red, so they bind to the mechanism rather than restating the outcome. */ expect(result.visitedNodeIds).toEqual([ + "start", "plan", "plan-review", "plan-review::plan-review-step", diff --git a/packages/engine/src/__tests__/workflow-graph-entry-contract.test.ts b/packages/engine/src/__tests__/workflow-graph-entry-contract.test.ts index d30e14796b..6fd7e2a8b9 100644 --- a/packages/engine/src/__tests__/workflow-graph-entry-contract.test.ts +++ b/packages/engine/src/__tests__/workflow-graph-entry-contract.test.ts @@ -26,11 +26,33 @@ case that motivated it: behind (resume forward), at, and past each column, on th const codingIr = parseWorkflowIr(getBuiltinWorkflow("builtin:coding")!.ir as never); describe("workflow graph entry contract — resume at the card's own column", () => { - it("enters the planning prologue only for a card still in the planning lane", () => { - // Intake: nothing is behind it, so the run starts at the graph's own start node. - expect(resolveColumnResumeNode(codingIr, "triage")?.id).toBe("start"); - // Planning lane: the specification phase is exactly what this card still needs. - expect(resolveColumnResumeNode(codingIr, "todo")?.id).toBe("plan"); + /* + FNXC:MergedPlanningColumn 2026-07-28-17:40 (U11): + EXPECTATION CHANGED BY THE MERGE, recorded rather than relaxed. This asserted two entry points + because the default workflow had two pre-implementation columns; it now has one, so `triage` + resolves to undefined (a column the IR does not declare cannot be placed relative to any node) + and the single planning column enters at `start`. + + The `undefined` half is NOT the whole story and is deliberately not left as the only assertion: + a card still stored in the removed `triage` column is rescued by `run()`'s start-node fallback, + and that rescue is asserted by driving the executor in the stranded-card suite at the bottom of + this file — verified to go red when the fallback is deleted. Asserting undefined here alone + would be compatible with those cards stranding. + */ + it("enters the planning prologue for a card in the merged planning lane", () => { + const entry = resolveColumnResumeNode(codingIr, "todo"); + expect(entry?.column).toBe("todo"); + expect(entry?.id).toBe("start"); + + // …and `start` reaches the specification node in one unconditional hop, which is what makes + // entering there equivalent to entering at the specification node itself. + const successors = codingIr.edges.filter( + (edge) => edge.from === entry!.id + && (edge.condition === undefined || edge.condition === "success") + && edge.kind !== "rework", + ); + expect(successors).toHaveLength(1); + expect(successors[0]!.to).toBe("plan"); }); it("never re-plans a card that already reached implementation", () => { @@ -51,7 +73,14 @@ describe("workflow graph entry contract — resume at the card's own column", () it("resolves the same way for the other built-in coding IRs", () => { expect(resolveColumnResumeNode(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR, "in-progress")?.id).toBe("parse"); - // The base IR names its planning seam `planning`; the contract is about columns, not ids. + /* + FNXC:MergedPlanningColumn 2026-07-28-17:10 (U11): + BUILTIN_CODING_WORKFLOW_IR is `builtin:legacy-coding`, NOT the default workflow — the catalog + maps `builtin:coding` to the stepwise-final-review IR. Legacy coding deliberately keeps the + six-column split shape (its purpose is being the old pipeline; R11 commits to legacy shapes + continuing to work), so it still answers `planning` here. The merged-column assertions live + with the DEFAULT lineage below. + */ expect(resolveColumnResumeNode(BUILTIN_CODING_WORKFLOW_IR, "todo")?.id).toBe("planning"); expect(resolveColumnResumeNode(BUILTIN_CODING_WORKFLOW_IR, "in-progress")?.id).toBe("execute"); }); @@ -194,42 +223,37 @@ difference between proving something and appearing to: when the fallback is removed is verified, not assumed. */ describe("workflow graph entry contract — merged intake+hold planning column (U11)", () => { - /** - * The U11 IR edit, as a transformation of a real workflow: `triage`'s traits merge into `todo`, - * `todo` becomes "Planning", `triage` is deleted, and every node that named `triage` is repointed. - * Applying this to the production IR is what makes the assertions below track production. - */ - function mergeTodoIntoPlanning(source: WorkflowIr): WorkflowIr { - const ir = structuredClone(source) as WorkflowIr & { - columns: Array<{ id: string; name?: string; traits?: unknown[] }>; - nodes: Array<{ id: string; column?: string }>; - }; - const triage = ir.columns.find((column) => column.id === "triage"); - const todo = ir.columns.find((column) => column.id === "todo"); - if (!triage || !todo) throw new Error("source IR is not the split-column shape this merge transforms"); + /* + FNXC:MergedPlanningColumn 2026-07-28-17:30 (U11): + This block previously TRANSFORMED the production IR to preview the merged shape before it + landed. It has landed: `builtin:coding` (the stepwise-final-review lineage) now declares one + pre-implementation column. So the transformation is deleted and these assert the real thing. - todo.name = "Planning"; - // intake first, then the existing hold/reset-on-entry — the union U11 declares. - todo.traits = [...(triage.traits ?? []), ...(todo.traits ?? [])]; - ir.columns = ir.columns.filter((column) => column.id !== "triage"); - for (const node of ir.nodes) { - if (node.column === "triage") node.column = "todo"; - } - return ir as WorkflowIr; - } + The transform helper is not kept "just in case" — it would now be a no-op that quietly asserts + nothing, which is worse than no test. `builtin:legacy-coding` still carries the split shape and + is asserted separately above, so the split vocabulary has not lost coverage. + */ + const mergedCodingIr = codingIr; - const mergedCodingIr = mergeTodoIntoPlanning(codingIr); - - it("is a faithful merge of the production IR (guards the transformation itself)", () => { - // If this drifts, every assertion below is measuring the wrong thing. + it("declares ONE pre-implementation column carrying both intake and hold", () => { + // The shape U11 exists to produce, asserted on the real default workflow. expect(mergedCodingIr.columns.map((column) => column.id)).not.toContain("triage"); expect(mergedCodingIr.nodes.every((node) => node.column !== "triage")).toBe(true); + const planning = mergedCodingIr.columns.find((column) => column.id === "todo")!; + expect(planning.name).toBe("Planning"); const traits = (planning.traits ?? []).map((trait) => (trait as { trait: string }).trait); expect(traits).toContain("intake"); expect(traits).toContain("hold"); - // Node COUNT is unchanged: this is a column merge, not a graph edit. - expect(mergedCodingIr.nodes.length).toBe(codingIr.nodes.length); + + // Exactly one column carries each pre-implementation role — a second intake or hold column + // would reintroduce the two-stage shape under different ids. + const withTrait = (name: string) => mergedCodingIr.columns.filter( + (column) => (column.traits ?? []).some((trait) => (trait as { trait: string }).trait === name), + ); + expect(withTrait("intake")).toHaveLength(1); + expect(withTrait("hold")).toHaveLength(1); + expect(withTrait("intake")[0]!.id).toBe(withTrait("hold")[0]!.id); }); it("enters the specification phase for a card in the merged planning column", () => { @@ -335,18 +359,9 @@ describe("workflow graph entry contract — a card stranded in a deleted column } as unknown as WorkflowRuntimePrimitives; } + /** Production is already merged, so this is the real default IR — no transform needed. */ function mergedIrWithoutTriage(): WorkflowIr { - const ir = structuredClone(codingIr) as WorkflowIr & { - columns: Array<{ id: string; name?: string; traits?: unknown[] }>; - nodes: Array<{ id: string; column?: string }>; - }; - const triage = ir.columns.find((column) => column.id === "triage")!; - const todo = ir.columns.find((column) => column.id === "todo")!; - todo.name = "Planning"; - todo.traits = [...(triage.traits ?? []), ...(todo.traits ?? [])]; - ir.columns = ir.columns.filter((column) => column.id !== "triage"); - for (const node of ir.nodes) if (node.column === "triage") node.column = "todo"; - return ir as WorkflowIr; + return codingIr; } it("runs a card still stored in the DELETED triage column instead of stranding it", async () => { diff --git a/packages/engine/src/triage.ts b/packages/engine/src/triage.ts index 7c514bfe8f..b237957966 100644 --- a/packages/engine/src/triage.ts +++ b/packages/engine/src/triage.ts @@ -9,6 +9,7 @@ import type { Agent, AgentPermissionPolicy, PermanentAgentGatingContext, + WorkflowIr, } from "@fusion/core"; import { DUPLICATE_OF_METADATA_KEY, @@ -33,6 +34,7 @@ import { resolveAgentMemoryInclusionMode, resolvePlanApprovalRequired, resolveWorkflowIrForTask, + resolveTaskLifecycleColumns, getStepParser, computePlanApprovalFingerprint, extractIntentSignature, @@ -1392,15 +1394,95 @@ export class TriageProcessor { * union include cards before their planner writes status:"planning". */ private async discoverReadyPlanningTasks(allTasks: Task[], now: number): Promise { - const eligibleTriageTasks = allTasks.filter( - (t) => t.column === "triage" && isTaskStillInPlanningStage(t) + /* + FNXC:MergedPlanningColumn 2026-07-28-15:10 (U11): + Planning discovery has TWO admission rules, and they were selected by hardcoded column id: + `triage` admitted any card still in the planning stage, `todo` admitted only a card whose + PROMPT.md still reads as a seed (a planned card in the hold column is waiting for CAPACITY, + not for planning, and re-specifying it would discard its approved spec). + + Both now select by TRAIT, so a workflow that renames or merges its pre-implementation columns + keeps both rules. Deleting `triage` from the coding IRs — U11 — would otherwise leave the + intake rule matching nothing while the hold rule silently became the only one. + + ORDER IS LOAD-BEARING. Under U11 one column carries BOTH `intake` and `hold`, so a card can + satisfy both rules. Hold is tested FIRST and the branches are mutually exclusive, for two + reasons: a card would otherwise appear in both lists and be dispatched twice for the same + planning run, and the hold rule is the NARROWER of the two — applying the intake rule to a + merged column would re-specify a card that has already been planned and is only waiting for a + slot. Narrower wins; nothing is admitted that both rules would not admit. + + A card whose workflow cannot be resolved falls back to the legacy ids rather than being + dropped from discovery entirely — an unplannable card is worse than a conservatively + planned one, and R11 keeps `todo`/`triage` legal ids for stored rows and custom workflows. + + FNXC:MergedPlanningColumn 2026-07-29-09:20 (PR #2515 review — greptile + coderabbit): + COST. Resolving a task's lifecycle columns needs its workflow-selection row, i.e. a store + round-trip. The first cut of this conversion awaited one for EVERY task on the board, + sequentially, before filtering anything — turning a pure in-memory filter into an O(board) + serial scan on the triage poll, which is the engine's hottest loop. It scaled with total board + size instead of with candidate count, so it was worst for exactly the operators with the + biggest boards. + + Two changes, in order of importance: + + 1. FILTER FIRST. Every predicate that needs no store read — processing/live-planning + membership, pause, the terminal statuses, the recovery backoff, and each branch's own cheap + precondition — is applied BEFORE any resolution. Only survivors are resolved, so the cost is + O(candidates), and a board whose cards are overwhelmingly done/in-review/executing pays + almost nothing. + 2. Resolve the survivors CONCURRENTLY under a bounded window rather than one await at a time, + so a genuine backlog of candidates costs one bounded batch instead of N serial round-trips. + Bounded rather than an unbounded Promise.all: this runs against the operator's live database + and a board-sized fan-out is its own denial of service. + + The IR cache still collapses the parse cost — a board of 400 cards on three workflows reads + three IRs — and is now shared across the concurrent resolutions rather than a serial loop. + */ + const irCache = new Map(); + + /* + The store-free half of both admission rules. A card failing this can never be admitted by + EITHER branch, so it never justifies a workflow-selection read. Kept deliberately in sync with + the two filters below — anything cheap that appears there should appear here. + */ + const couldBeCandidate = (t: Task): boolean => { + if (this.processing.has(t.id) || this.hasLivePlanningWork(t.id) || t.paused) return false; + if (t.status === "awaiting-approval" || t.status === "failed" || t.status === "stuck-killed") return false; + if (t.nextRecoveryAt && new Date(t.nextRecoveryAt).getTime() > now) return false; + const couldBeIntake = isTaskStillInPlanningStage(t) && !this.advancedRecoveryReservations.has(t.id); + const couldBeHold = t.status !== "planning"; + return couldBeIntake || couldBeHold; + }; + + const candidates = allTasks.filter(couldBeCandidate); + const lifecycleByTaskId = new Map(); + const RESOLUTION_CONCURRENCY = 8; + for (let offset = 0; offset < candidates.length; offset += RESOLUTION_CONCURRENCY) { + const window = candidates.slice(offset, offset + RESOLUTION_CONCURRENCY); + const resolved = await Promise.all( + window.map((t) => resolveTaskLifecycleColumns(this.store, t.id, irCache)), + ); + window.forEach((t, index) => { + lifecycleByTaskId.set(t.id, resolved[index] ?? { intake: "triage", hold: "todo" }); + }); + } + + // An unresolved task id means the card was filtered out before resolution, so it is not a + // candidate and both predicates are correctly false for it. + const isAtHoldColumn = (t: Task): boolean => lifecycleByTaskId.get(t.id)?.hold === t.column; + const isAtIntakeColumn = (t: Task): boolean => lifecycleByTaskId.get(t.id)?.intake === t.column; + + const eligibleTriageTasks = candidates.filter( + // `!isAtHoldColumn` keeps the two branches disjoint for a merged intake+hold column. + (t) => isAtIntakeColumn(t) && !isAtHoldColumn(t) && isTaskStillInPlanningStage(t) && !this.advancedRecoveryReservations.has(t.id) && !this.processing.has(t.id) && !this.hasLivePlanningWork(t.id) && !t.paused && t.status !== "awaiting-approval" && t.status !== "failed" && t.status !== "stuck-killed" && !(t.nextRecoveryAt && new Date(t.nextRecoveryAt).getTime() > now), ); - const eligibleTodoTasksRaw = allTasks.filter( - (t) => t.column === "todo" && !this.processing.has(t.id) && !this.hasLivePlanningWork(t.id) && !t.paused + const eligibleTodoTasksRaw = candidates.filter( + (t) => isAtHoldColumn(t) && !this.processing.has(t.id) && !this.hasLivePlanningWork(t.id) && !t.paused && t.status !== "awaiting-approval" && t.status !== "failed" && t.status !== "stuck-killed" && t.status !== "planning" && !(t.nextRecoveryAt && new Date(t.nextRecoveryAt).getTime() > now),