From 25b3c06d2da88ade69d39349e343f5720e98b32b Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 01:20:04 -0700 Subject: [PATCH] fix(plugins): compound-engineering pipelines stalled forever on a renamed board (#3022) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #3020 — which I filed **instead of** fixing, on a rationale that turned out to be wrong. I said the plugin had no scaffolding for faking `CePipelineStore` + `taskStore` together. It does: `_harness.ts` already builds a real `PluginContext` over a live PostgreSQL layer. The gap was **two missing readers on its task-store stub**, not missing infrastructure. I checked the harness only after filing. ## The defect `TERMINAL_COLUMNS` is `{in-review, done}`, and the reconciler advances a pipeline only when **every** current-stage board task is in that set. On a board whose review and completion lanes are renamed that's false for every task, permanently: - the pipeline never advances a stage - it never creates its outbound task - it sits `running` indefinitely Nothing errors, so it reads as work that hasn't finished. Unlike the display defects in this family (#3014, #3017), the CE flow actually **stops**. ## Shape The decision is extracted to an exported `isStageTerminalColumn` because it *is* the whole decision. Left private it could only be reached through a pipeline-state + links + board-tasks fixture, and the half that needed proving is that a renamed board resolves to its own lanes through this store. It uses `resolveReviewColumns` rather than re-deriving the union — that helper is the documented review **set** (`mergeOrchestration ∪ mergeBlocker ∪ humanReview`), so a board splitting those across a merge lane and a human lane is covered without this site drifting from it. ## Two things my first attempt got wrong **The fixture spelled traits in camelCase** — `{ trait: "humanReview" }`. Trait **ids** are kebab-case (`human-review`, `merge-blocker`, `wip`); the camelCase names are the resolved **flags**. Those columns therefore resolved to *no roles at all*, silently, because an unknown trait isn't an error. `complete` is spelled identically in both vocabularies, which is exactly what made the first run look like *"complete works, review is broken"* rather than *"the fixture is wrong"* — I nearly went debugging the production union. **The harness extension is additive** and inert until a test seeds it, so all 24 existing plugin suites see the previous shape. ## Measured | check | result | |---|---| | new suite | **4/4** | | reverting to the literal-only gate | fails **exactly 2** — the renamed-terminal case, and a board declaring a NON-terminal column named `done` — while the legacy control and the WIP/intake negative still pass | | plugin suite | **24 files, 184 tests green** | | `tsc` + all five gates | clean | That second row is the one that matters: the `done`-without-`complete` board is the only shape where a real resolution and a legacy fallback disagree, so it's what separates the fix from a lucky agreement. --------- Co-authored-by: Claude Opus 5 (1M context) --- .../cli/src/plugin-sdk-core-runtime-shim.mjs | 22 ++++ .../src/__tests__/_harness.ts | 25 ++++ .../__tests__/pipeline-terminal-lanes.test.ts | 117 ++++++++++++++++++ .../src/sync/reconciler.ts | 53 +++++++- 4 files changed, 215 insertions(+), 2 deletions(-) create mode 100644 plugins/fusion-plugin-compound-engineering/src/__tests__/pipeline-terminal-lanes.test.ts diff --git a/packages/cli/src/plugin-sdk-core-runtime-shim.mjs b/packages/cli/src/plugin-sdk-core-runtime-shim.mjs index 088fc56908..93eca2d5bf 100644 --- a/packages/cli/src/plugin-sdk-core-runtime-shim.mjs +++ b/packages/cli/src/plugin-sdk-core-runtime-shim.mjs @@ -12,6 +12,28 @@ import * as postgresSchema from "../../core/src/postgres/schema/index.js"; export { postgresSchema }; +/* + * FNXC:BundledPlugins 2026-07-31-09:55: + * Lifecycle ROLE resolution, re-exported for bundled plugins. + * + * A plugin that asks "is this card in a terminal lane?" must resolve the board's roles rather than + * compare against `done`/`archived`, or it stalls forever on a renamed board. That is what the + * compound-engineering reconciler now does — but this shim is what `@fusion/core` resolves to inside + * the bundled build, so an import it does not re-export is a hard esbuild failure ("No matching + * export"), not a runtime fallback. The plugin built fine in the workspace and broke only in the CLI + * bundle. + * + * Source paths, not the package barrel, for the reason above: esbuild follows core's source here and + * the CLI must not take a private @fusion/core dependency. + */ +import { + columnsWithFlag, + resolveReviewColumns, +} from "../../core/src/workflow-lifecycle-traits.js"; +import { resolveWorkflowIrForTask } from "../../core/src/workflow-ir-resolver.js"; + +export { columnsWithFlag, resolveReviewColumns, resolveWorkflowIrForTask }; + export const FUSION_RESTART_EXIT_CODE = 86; export function superviseSpawn(command, args = [], options = {}) { diff --git a/plugins/fusion-plugin-compound-engineering/src/__tests__/_harness.ts b/plugins/fusion-plugin-compound-engineering/src/__tests__/_harness.ts index 83752642d4..df32968d19 100644 --- a/plugins/fusion-plugin-compound-engineering/src/__tests__/_harness.ts +++ b/plugins/fusion-plugin-compound-engineering/src/__tests__/_harness.ts @@ -18,6 +18,10 @@ export interface TestHarness { projectRoot: string; ctx: PluginContext; emitted: Array<{ event: string; data: unknown }>; + /** Register a workflow definition the ctx's task store can resolve by id. */ + defineWorkflow(id: string, ir: unknown): void; + /** Point a task at one of those workflows, as a real selection row would. */ + assignTaskWorkflow(taskId: string, workflowId: string): void; close(): void; } @@ -63,10 +67,29 @@ export async function makeHarness(): Promise { const emitted: Array<{ event: string; data: unknown }> = []; + /* + FNXC:WorkflowResolvedColumns 2026-07-31-04:20: + THE WORKFLOW-SELECTION SURFACE, added so lane-vocabulary behaviour is testable through this ctx. + + `resolveWorkflowIrForTask` reads a task's selection and then the named definition. The stub carried + neither, so any plugin code resolving a board's real lanes could only be exercised against the + builtin default — which is the one vocabulary where a legacy literal and a resolved answer agree, + and therefore the one that proves nothing. + + Both readers stay ABSENT until a test seeds them, so every existing test sees the previous shape. + */ + const workflowDefinitions = new Map(); + const taskWorkflowIds = new Map(); + const taskStore = { getAsyncLayer: () => layer, isBackendMode: () => true, getRootDir: () => projectRoot, + getWorkflowDefinition: (id: string) => workflowDefinitions.get(id), + getTaskWorkflowSelectionAsync: async (taskId: string) => { + const workflowId = taskWorkflowIds.get(taskId); + return workflowId ? { workflowId } : undefined; + }, } as unknown as PluginContext["taskStore"]; const ctx: PluginContext = { @@ -84,6 +107,8 @@ export async function makeHarness(): Promise { projectRoot, ctx, emitted, + defineWorkflow: (id: string, ir: unknown) => { workflowDefinitions.set(id, { id, ir }); }, + assignTaskWorkflow: (taskId: string, workflowId: string) => { taskWorkflowIds.set(taskId, workflowId); }, close: () => { rmSync(projectRoot, { recursive: true, force: true }); }, diff --git a/plugins/fusion-plugin-compound-engineering/src/__tests__/pipeline-terminal-lanes.test.ts b/plugins/fusion-plugin-compound-engineering/src/__tests__/pipeline-terminal-lanes.test.ts new file mode 100644 index 0000000000..92214cb325 --- /dev/null +++ b/plugins/fusion-plugin-compound-engineering/src/__tests__/pipeline-terminal-lanes.test.ts @@ -0,0 +1,117 @@ +/* +FNXC:WorkflowResolvedColumns 2026-07-31-04:45: +CE PIPELINES STALLED FOREVER ON A RENAMED BOARD. + +`isStageTerminalColumn` decides whether a stage's board work is finished; the reconciler advances the +pipeline only when every current-stage task answers true. It was a membership test against the +literal `{in-review, done}`, so on a board whose review and completion lanes are renamed the answer +was false for every task, permanently: the pipeline never advanced a stage, never created its +outbound task, and sat `running`. Nothing errors, which is why it reads as work that has not +finished rather than as a bug. + +The cases are DIFFERENTIAL: the same finished task under two vocabularies whose roles are identical +and only the ids differ. `shipped` and `checking` collide with no legacy id, so a surviving literal +cannot pass by luck. + +WHAT THIS COVERS AND WHAT IT DOES NOT. It drives the real resolution path — a real `PluginContext` +task store, a registered workflow definition, a real selection read — which is the half that needed +proving. The reconciler's own `every(Boolean)` wiring is a one-line call and is covered by typecheck +only; a full advancement fixture needs pipeline state plus links plus board tasks, which this plugin +has no scaffolding for. Stated rather than implied. +*/ + +import { beforeAll, afterAll, expect, it } from "vitest"; +import { makeHarness, pgDescribe, type TestHarness } from "./_harness.js"; +import { isStageTerminalColumn } from "../sync/reconciler.js"; +import type { Task } from "@fusion/core"; + +const RENAME: Record = { + todo: "drafting", + "in-progress": "building", + "in-review": "checking", + done: "shipped", +}; + +/** The builtin coding lanes with only their ids renamed, as a v2 IR. */ +const RENAMED_IR = { + version: "v2", + name: "renamed", + columns: [ + { id: RENAME.todo, name: "Drafting", traits: [{ trait: "intake" }] }, + { id: RENAME["in-progress"], name: "Building", traits: [{ trait: "wip" }] }, + { id: RENAME["in-review"], name: "Checking", traits: [{ trait: "human-review" }, { trait: "merge-blocker" }] }, + { id: RENAME.done, name: "Shipped", traits: [{ trait: "complete" }] }, + ], + nodes: [{ id: "start", kind: "start", column: RENAME.todo }, { id: "end", kind: "end", column: RENAME.done }], + edges: [{ from: "start", to: "end" }], +}; + +/* +FNXC:WorkflowResolvedColumns 2026-07-31-05:00: +TRAIT IDS ARE KEBAB-CASE; the camelCase names are the resolved FLAGS. + +`{ trait: "humanReview" }` resolves to NO flags at all — silently, because an unknown trait is not an +error — so a fixture written that way produces a column with no roles and its assertions read as a +product defect. `complete` is spelled identically in both vocabularies, which is exactly what made +the first run look like "complete works, review is broken" rather than "the fixture is wrong". +*/ +const task = (id: string, column: string) => ({ id, column, title: id, description: "t" } as unknown as Task); + +pgDescribe("CE stage-terminal detection under a renamed board vocabulary", () => { + let h: TestHarness; + + beforeAll(async () => { h = await makeHarness(); }); + afterAll(() => { h?.close(); }); + + /* Control: with no workflow registered the legacy pair still answers, so an unconverted board and + a resolution failure are byte-identical to the previous behaviour. */ + it("no resolvable workflow: the legacy pair still decides", async () => { + expect(await isStageTerminalColumn(h.ctx.taskStore, task("KB-LEGACY-1", "done"))).toBe(true); + expect(await isStageTerminalColumn(h.ctx.taskStore, task("KB-LEGACY-2", "in-review"))).toBe(true); + expect(await isStageTerminalColumn(h.ctx.taskStore, task("KB-LEGACY-3", "in-progress"))).toBe(false); + }); + + /* The defect: before the fix every one of these was false, so the pipeline never advanced. */ + it("renamed vocabulary: the renamed complete and review lanes are terminal", async () => { + h.defineWorkflow("wf-renamed", RENAMED_IR); + for (const id of ["KB-R1", "KB-R2"]) h.assignTaskWorkflow(id, "wf-renamed"); + + expect(await isStageTerminalColumn(h.ctx.taskStore, task("KB-R1", "shipped"))).toBe(true); + expect(await isStageTerminalColumn(h.ctx.taskStore, task("KB-R2", "checking"))).toBe(true); + }); + + /* + The paired negative: resolving real lanes must not degrade into "every column is terminal", which + would advance a pipeline whose board work has not started — worse than stalling, because it + propagates an outbound task for unfinished work. + */ + it("renamed vocabulary: the renamed WIP and intake lanes are NOT terminal", async () => { + h.defineWorkflow("wf-renamed", RENAMED_IR); + for (const id of ["KB-R3", "KB-R4"]) h.assignTaskWorkflow(id, "wf-renamed"); + + expect(await isStageTerminalColumn(h.ctx.taskStore, task("KB-R3", "building"))).toBe(false); + expect(await isStageTerminalColumn(h.ctx.taskStore, task("KB-R4", "drafting"))).toBe(false); + }); + + /* + A board that declares a column named `done` WITHOUT the complete trait. The literal would call it + terminal; the resolved answer must not. This is the shape that separates a real resolution from a + legacy fallback that happens to agree. + */ + it("a declared `done` column with no terminal trait is NOT terminal", async () => { + h.defineWorkflow("wf-done-not-complete", { + version: "v2", + name: "done-is-not-complete", + columns: [ + { id: "drafting", name: "Drafting", traits: [{ trait: "intake" }] }, + { id: "done", name: "Done pile (not terminal)", traits: [] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], + nodes: [{ id: "start", kind: "start", column: "drafting" }, { id: "end", kind: "end", column: "shipped" }], + edges: [{ from: "start", to: "end" }], + }); + h.assignTaskWorkflow("KB-R5", "wf-done-not-complete"); + + expect(await isStageTerminalColumn(h.ctx.taskStore, task("KB-R5", "done"))).toBe(false); + }); +}); diff --git a/plugins/fusion-plugin-compound-engineering/src/sync/reconciler.ts b/plugins/fusion-plugin-compound-engineering/src/sync/reconciler.ts index dff1628876..3931488379 100644 --- a/plugins/fusion-plugin-compound-engineering/src/sync/reconciler.ts +++ b/plugins/fusion-plugin-compound-engineering/src/sync/reconciler.ts @@ -1,4 +1,5 @@ import type { PluginContext, Task } from "@fusion/core"; +import { columnsWithFlag, resolveReviewColumns, resolveWorkflowIrForTask } from "@fusion/core"; import { listPipelineStages } from "../session/stage-registry.js"; import { createCeTaskWithLink } from "./ce-task.js"; import { @@ -41,9 +42,56 @@ import { * is needed for correctness. */ -/** Columns that mean "this stage's board work is finished" → advance the pipeline. */ +/* +FNXC:WorkflowResolvedColumns 2026-07-31-04:30: +DELIBERATE-LITERAL — the no-IR fallback for the pipeline's "this stage is finished" test. + +Retained so a task whose workflow cannot be resolved behaves exactly as before. It is no longer the +decision: gating advancement on these ids alone meant `allTerminal` was FALSE FOREVER on a board +whose review and completion lanes are renamed, so the pipeline never advanced a stage, never created +its outbound task, and sat `running` indefinitely. Nothing errors — it reads as work that has not +finished, which is why it could sit unnoticed. +*/ const TERMINAL_COLUMNS = new Set(["in-review", "done"]); +/* +FNXC:WorkflowResolvedColumns 2026-07-31-04:35: +"Finished for pipeline purposes" is a ROLE question, resolved against the task's OWN workflow. + +The set below means complete OR review — the comment at its declaration says a stage is done when its +board work reaches either. Resolved per task rather than board-wide because a column id is meaningful +only relative to its workflow, and a CE pipeline can span several. + +Falls back to the legacy pair whenever the IR cannot be resolved or declares neither trait, so an +unconverted board and a resolution failure both keep the previous behaviour rather than stalling on +an empty set — which would be the same defect wearing a different cause. + +EXPORTED because it is the whole decision. Left private it could only be reached through a full +reconcile fixture (pipeline state + links + board tasks), and the half that actually needed proving +is that a renamed board resolves to its own lanes through this store. +*/ +export async function isStageTerminalColumn( + taskStore: PluginContext["taskStore"], + task: Task, +): Promise { + try { + const ir = await resolveWorkflowIrForTask(taskStore, task.id); + if (ir) { + /* `resolveReviewColumns` is the documented review SET — mergeOrchestration ∪ mergeBlocker ∪ + humanReview — so a board that splits those across a merge lane and a human lane is covered + without this site re-deriving the union and drifting from it. */ + const terminal = new Set([ + ...columnsWithFlag(ir, "complete"), + ...resolveReviewColumns(ir), + ]); + if (terminal.size > 0) return terminal.has(task.column); + } + } catch { + /* fall through to the documented legacy pair */ + } + return TERMINAL_COLUMNS.has(task.column); +} + export interface ReconcileResult { /** Queue entries drained this sweep. */ drained: number; @@ -150,7 +198,8 @@ export class CeReconciler { // Advancement rule: every EXISTING current-stage board task has reached a // terminal column (board-authoritative read). Partial completion keeps it // running; deleted tasks are excluded above rather than counted as blocking. - const allTerminal = existing.every((t) => TERMINAL_COLUMNS.has(t.column)); + const terminalFlags = await Promise.all(existing.map((t) => isStageTerminalColumn(this.ctx.taskStore, t))); + const allTerminal = terminalFlags.every(Boolean); if (!allTerminal) { // Still running on the board — make sure our status reflects that and stop. if (state.status !== "running") {