From 31e49b684a7341e789febea1b75040185316eaa6 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 29 Jul 2026 22:39:14 -0700 Subject: [PATCH] TAKING default-workflow-hooks.ts + executor.ts + live-agent-count.ts + 6 dashboard files: reopen semantics by role, and the census's blind spot in both directions (13 sites) (#2628) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Batched conversion of every lifecycle-column guard I hold, plus the three the census could not see. **Six files to zero, repo-wide 60 → 49 by a comment-stripped unanchored sweep.** Each conversion has an isolated revert proof and a paired negative case, and the one code move is a separate commit from the behavior changes. ## Per-file before → after Counts from a comment-stripped, unanchored `(===|!==) ["']triage["']` sweep over `packages/*/src` + `plugins/*/src`, excluding tests. | file | before | after | note | |---|---:|---:|---| | `core/default-workflow-hooks.ts` | 4 | **0** | | | `core/task-store/moves.ts` | 5 | **4** | only the flag-ON mirror converted; the flag-OFF inline block is the parity reference and stays | | `engine/executor.ts` | 3 | **0** | **absent from the 45-guard list** — see below | | `core/live-agent-count.ts` | 2 | **0** | duplication removed; answer deliberately unchanged | | `engine/replan-target.ts` | 2 | **0** | both were comment prose, not guards | | `core/agent-prompts.ts` | 3 | **0** | ROLE comparisons, never column guards | | `engine/usage-limit-detector.ts` | 2 | **0** | ROLE comparisons | | `dashboard/app/components/DocumentsView.tsx` | 1 | **0** | real column guard | | `dashboard/app/components/TaskChatTab.tsx` | 2 | **0** | ROLE | | `dashboard/app/components/AgentLogViewer.tsx` | 1 | **0** | ROLE | | `dashboard/app/components/effective-model-resolution.ts` | 1 | **0** | ROLE | | `dashboard/app/hooks/useTasks.ts` | 1 | **0** | ROLE | | `dashboard/…/command-center/MissionControlPanel.tsx` | 1 | 1 | alias table, marked `DELIBERATE-LITERAL` with its reason | ## The census errs in BOTH directions This is the finding I would most like carried into the remaining work. - It **flagged 10 sites that were never column guards.** `role === "triage"` / `agentType === "triage"` compare an **AGENT ROLE**. The planner *lane* is named `triage` and keeps that name — U11 removed the *column*. Worse than noise: the obvious "finish the migration" edit is to rename the role, and that silently empties the planner's prompt template and mis-binds its model markers. `PLANNER_AGENT_ROLE` now names it, so the two vocabularies are distinguishable by grep and a rename fails loudly (revert proof: 4 tests, two of them pre-existing). - It **missed 3 real guards in `executor.ts`**, because the pattern matches `column`/`toColumn`/`fromColumn` and those locals are named `from` and `originColumn`. A census keyed on variable names will keep missing guards wherever a local was named for its role in the function. ## Two real defects, not tidying **1. A renamed board could merge with its re-review never run.** `default-workflow-hooks.ts` is named for the default workflow, but the store runs it on the flag-ON path for *every* workflow — the trait registry resolves hooks by trait id, not by workflow. Its reopen predicates listed the default lineage's column names, so on a renamed board **no reopen effect fired at all**. One of them clears `workflowStepResults`, which `getTaskMergeBlocker` reads: a card bounced out of review carried its old `passed` result back in, and that satisfies the merge gate. Same regression the graph-owned-crossing carve-out exists to prevent, arriving through the other door. (Two smaller ones rode along: failure state never cleared on a renamed reopen, and an operator dragging a card back to the queue never parked it, so the scheduler re-dispatched what they had just pulled back.) **I forgot the carve-out on my first pass, and that was worse than not converting.** A role-resolved clear plus a *name*-matched exemption means a renamed board takes the clear and never the exemption, destroying the remediation input the graph had just written. My own paired negative test caught it. **2. The last-resort recovery for completed-but-stranded work did not exist off the default lineage.** In `recoverCompletedTask`, `promotedFromPlannerColumn` was false on a renamed board, so finished work resting in the planning lane was never promoted — the code fell through to `handoffTaskToReview` straight from the planning column, and role adjacency has no planning → review edge, so the handoff was rejected and the card stayed stuck with its work complete. I converted the promotion **target** too: resolving the lane and then moving to a literal `in-progress` is the half-conversion I have already been burned by twice this program, where the guard starts admitting cards and the move then sends them to a column the board does not declare. ## E2E evidence `renamed-board-reopen.pg.test.ts` drives a **real PostgreSQL store** and a real `moveTask` on a workflow whose columns carry the standard traits under non-default names. The unit tests cannot show this: if `moves.ts` passed `undefined`, every unit case still passes via the no-basis fallback while the real board keeps the old behavior. **Proof it is load-bearing: forcing `moveLifecycleColumns` to `undefined` fails 2 of 3.** The executor suite covers both the split-role and the MERGED post-U11 shape. ## Revert proofs, isolated per site | change reverted | result | |---|---| | reopen predicate → literal names | 4 of 10 fail | | reopen field clears → literal names | 2 of 10 fail | | `userPaused` hold lane → literal `todo` | 1 of 10 fail | | graph carve-out → literal names | 1 of 10 fail | | store passes `undefined` lifecycle columns | 2 of 3 fail (real PG) | | `promotedFromPlannerColumn` → literals | 3 of 7 fail | | two-hop condition → `=== "triage"` | 1 of 7 fails | | promotion target → `"in-progress"` | 3 of 7 fail | | `isPlannerColumnFor` → literals | 1 of 7 fails | | live-agent-count: one arm dropped | 2 of 11 fail | | DocumentsView: trait branch removed | 3 of 7 fail | | planner role renamed to `"planner"` | 4 fail (2 pre-existing) | Every conversion is paired with a negative case (a forward move, a not-a-planner-lane card, a default-lineage card, a renamed column with no traits), so neither "always fire" nor "never fire" can pass for "resolve the role". ## Deliberately NOT converted, with reasons - **`moves.ts` flag-OFF inline block (4).** That branch *is* the legacy path, kept verbatim so the two can be parity-checked. Converting it erases the reference implementation. - **`live-agent-count.ts`'s no-flags fallback.** Reachable, and there is nothing to resolve from — `enrich…FromFlags` exists for callers with board flags rather than an IR, so a column missing from that map is the renamed case. "Not intake" is as much a guess as "todo is intake", and Running/Waiting are complements, so a card matching neither arm is reported as neither and the footer's queued total under-reports it. The real fix is at the caller; four new cases pin that flags override the legacy answer **in both directions**. What did change is the duplication: two hand-written copies of one rule now call one named function. - **`MissionControlPanel`'s `FUNNEL_STAGES`.** An alias table of column *names* where `triage` sits beside `signal` and `backlog`. Command Center aggregates across projects, so there is no single workflow to resolve traits from — the honest conversion is a data change, not a predicate change. - **`DocumentsView` with no traits.** Same no-basis rule; the documents list is full of historical columns absent from the current board. A case asserts a renamed column with no traits still reads as "working", documenting the gap rather than hiding it. ## Fixture findings Each cost a red run that looked like the code under test: - a `merge-blocker` column needs a reachable merge-class node, or `parseWorkflowIr` rejects the workflow; - a back-edge must be `kind: "rework"`, and a rework edge is legal only **into** a node with `config.reworkRegion: true`; - a workflow gets role-level transitions only when it declares wip + review + complete + **archived** plus a planning lane — without the archived column, adjacency falls back to order-derived neighbours and `checking -> queued` is not a legal move at all; - `recoverCompletedTask` only *reaches* the promotion seam when nothing is left to gate; without passed `plan-review`/`code-review` rows it re-enters the workflow graph and returns first, so a naive fixture silently tests the wrong branch and every assertion reads "no moves happened" for an unrelated reason. ## Verification - `pnpm test:gate` **71/71** - new suites: 10/10 reopen-semantics, 3/3 renamed-board-reopen (real PG), 7/7 executor-planner-lanes, 7/7 documents-status-dot, 4/4 planner-role-is-not-a-column - neighbours: 132 + 10 + 482 (gate shards), 350/351 engine planning/replan suites, 64/64 agent-prompts, 51/51 usage-limit-detector, 11/11 live-agent-count, 11/11 dashboard hook/log suites - the single engine failure (`executor-fast-mode-workflows.test.ts` › "raw fast mode still invokes non-executable review seam nodes") **reproduces with my changes stashed** — pre-existing on `origin/main` - typechecks clean for core, engine, and dashboard-app (`tsconfig.app.json`; `tsconfig.json` checks nothing under `app/`); `pnpm lint` clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) --- ...ashboard-planner-role-and-documents-dot.md | 7 + .changeset/executor-planner-lanes-resolved.md | 7 + .../live-agent-count-legacy-fallback.md | 7 + .changeset/reopen-semantics-by-role.md | 7 + .../src/__tests__/live-agent-count.test.ts | 48 +++ .../planner-role-is-not-a-column.test.ts | 63 ++++ .../postgres/renamed-board-reopen.pg.test.ts | 187 +++++++++++ .../reopen-semantics-by-role.test.ts | 243 ++++++++++++++ packages/core/src/agent-prompts.ts | 9 +- packages/core/src/default-workflow-hooks.ts | 130 +++++++- packages/core/src/index.ts | 2 +- packages/core/src/live-agent-count.ts | 48 ++- packages/core/src/task-store/moves.ts | 29 +- packages/core/src/types.ts | 4 + packages/core/src/types/task-log.ts | 20 ++ packages/dashboard/app/App.tsx | 3 + .../documents-status-dot-by-role.test.ts | 64 ++++ .../app/components/AgentLogViewer.tsx | 5 +- .../app/components/DocumentsView.tsx | 45 ++- .../dashboard/app/components/TaskChatTab.tsx | 7 +- .../command-center/MissionControlPanel.tsx | 10 + .../app/components/dashboard/MainContent.tsx | 2 + .../app/components/dashboard/types.ts | 7 + .../components/effective-model-resolution.ts | 6 +- packages/dashboard/app/hooks/useTasks.ts | 6 +- .../executor-planner-lanes-resolved.test.ts | 303 ++++++++++++++++++ packages/engine/src/executor.ts | 104 +++++- packages/engine/src/replan-target.ts | 106 +++++- packages/engine/src/triage.ts | 15 +- packages/engine/src/usage-limit-detector.ts | 8 +- 30 files changed, 1427 insertions(+), 75 deletions(-) create mode 100644 .changeset/dashboard-planner-role-and-documents-dot.md create mode 100644 .changeset/executor-planner-lanes-resolved.md create mode 100644 .changeset/live-agent-count-legacy-fallback.md create mode 100644 .changeset/reopen-semantics-by-role.md create mode 100644 packages/core/src/__tests__/planner-role-is-not-a-column.test.ts create mode 100644 packages/core/src/__tests__/postgres/renamed-board-reopen.pg.test.ts create mode 100644 packages/core/src/__tests__/reopen-semantics-by-role.test.ts create mode 100644 packages/dashboard/app/__tests__/documents-status-dot-by-role.test.ts create mode 100644 packages/engine/src/__tests__/executor-planner-lanes-resolved.test.ts diff --git a/.changeset/dashboard-planner-role-and-documents-dot.md b/.changeset/dashboard-planner-role-and-documents-dot.md new file mode 100644 index 0000000000..75f83e4e77 --- /dev/null +++ b/.changeset/dashboard-planner-role-and-documents-dot.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Task Documents shows the correct status dot for tasks on renamed or custom board columns. +category: fix +dev: `DocumentsView` takes optional per-task column traits (threaded from App's existing footer map through `MainContent`) and resolves the dot by role; five dashboard `agent === "triage"` role comparisons now use `PLANNER_AGENT_ROLE`. diff --git a/.changeset/executor-planner-lanes-resolved.md b/.changeset/executor-planner-lanes-resolved.md new file mode 100644 index 0000000000..124609353b --- /dev/null +++ b/.changeset/executor-planner-lanes-resolved.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Completed work stranded in a renamed planning column is now recovered instead of stuck there. +category: fix +dev: `recoverCompletedTask` resolves the planner lanes and the promotion target from the task's own workflow (`resolvePlannerLanes`, now shared from `replan-target.ts`), and the planning-evacuation branch of the `task:moved` handler uses the same classification via `isPlannerColumnFor`. diff --git a/.changeset/live-agent-count-legacy-fallback.md b/.changeset/live-agent-count-legacy-fallback.md new file mode 100644 index 0000000000..1c44dbf1cd --- /dev/null +++ b/.changeset/live-agent-count-legacy-fallback.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: The processing/queued footer counts now share one rule for columns with no trait flags. +category: internal +dev: `live-agent-count.ts`'s two duplicate no-flags fallbacks collapse into `isLegacyPreImplementationColumn`; deliberately still the legacy pair, with the reason recorded at the helper. `replan-target.ts` comment prose restated by role. diff --git a/.changeset/reopen-semantics-by-role.md b/.changeset/reopen-semantics-by-role.md new file mode 100644 index 0000000000..7bd6b91cbe --- /dev/null +++ b/.changeset/reopen-semantics-by-role.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Reopening a card on a renamed board now clears its stale review results, branch and failure state. +category: fix +dev: `default-workflow-hooks.ts` reopen predicates resolve intake/hold/wip/review/complete by trait from the task's own IR (passed in from `moves.ts` as `DefaultWorkflowMoveContext.lifecycleColumns`) instead of matching the default lineage's column names. `isReopenIntoPlanning` is exported so the store's former "parity mirror" calls it. The flag-OFF inline block in `moves.ts` stays name-based as the parity reference. diff --git a/packages/core/src/__tests__/live-agent-count.test.ts b/packages/core/src/__tests__/live-agent-count.test.ts index 5b8730e6e8..557dfef505 100644 --- a/packages/core/src/__tests__/live-agent-count.test.ts +++ b/packages/core/src/__tests__/live-agent-count.test.ts @@ -88,3 +88,51 @@ describe("live agent count predicates", () => { }); }); }); + +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-10:30 (Phase C convergence — live-agent-count.ts): + +The no-flags fallback is DELIBERATELY the legacy pair, and these cases exist so a future +"finish the conversion" pass cannot quietly change the answer. Running and Waiting are +complements over the same rows, so if the two former literal sites ever disagree a card +lands in both counts or in neither, and the footer's queued total misreports it. + +What is pinned: + - with NO flags, the legacy planner ids are Waiting (unchanged behavior); + - with NO flags, a renamed planner column is NOT Waiting — the known gap, whose fix is at + the caller (supply flags, or use the IR-based `enrichRunningAgentTaskShape`), not a guess + about what an absent flag set means; + - flags always WIN over the fallback, in both directions, which is what makes the caller + fix effective. +*/ +describe("the no-flags fallback keeps the legacy planner vocabulary", () => { + const bare = (column: string) => ({ id: "FN-1", column } as Parameters[0]); + + it("treats the legacy planner ids as waiting when no flags are supplied", () => { + expect(isWaitingAgentTask(enrichRunningAgentTaskShapeFromFlags(bare("triage")))).toBe(true); + expect(isWaitingAgentTask(enrichRunningAgentTaskShapeFromFlags(bare("todo")))).toBe(true); + expect(isWaitingAgentTask(enrichRunningAgentTaskShapeFromFlags(bare("in-review")))).toBe(false); + }); + + it("answers identically whether the shape was enriched or read raw", () => { + // The two former literal sites: `enrich...FromFlags` and `isWaitingAgentTask`'s own + // `??` fallback. One rule, so one answer. + for (const column of ["triage", "todo", "in-progress", "backlog"]) { + expect(isWaitingAgentTask(enrichRunningAgentTaskShapeFromFlags(bare(column)))) + .toBe(isWaitingAgentTask(bare(column))); + } + }); + + it("does NOT invent a planner lane for a renamed column with no flags", () => { + expect(isWaitingAgentTask(enrichRunningAgentTaskShapeFromFlags(bare("backlog")))).toBe(false); + }); + + it("lets supplied flags override the legacy answer in both directions", () => { + // A board that declares `todo` as a WIP column: flags win, so it is Running, not Waiting. + const wipTodo = enrichRunningAgentTaskShapeFromFlags(bare("todo"), { countsTowardWip: true }); + expect(isWaitingAgentTask(wipTodo)).toBe(false); + expect(isRunningAgentTask(wipTodo)).toBe(true); + // And the renamed planner lane becomes Waiting as soon as its flags arrive. + expect(isWaitingAgentTask(enrichRunningAgentTaskShapeFromFlags(bare("backlog"), { intake: true }))).toBe(true); + }); +}); diff --git a/packages/core/src/__tests__/planner-role-is-not-a-column.test.ts b/packages/core/src/__tests__/planner-role-is-not-a-column.test.ts new file mode 100644 index 0000000000..b94bf2d330 --- /dev/null +++ b/packages/core/src/__tests__/planner-role-is-not-a-column.test.ts @@ -0,0 +1,63 @@ +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-11:10 (Phase C convergence — vocabulary hygiene): + +THE INVARIANT: `"triage"` names TWO different things, and only one of them was removed. + + - the planner AGENT ROLE — the engine lane, its prompt templates, its usage-limit + accounting. Still called `triage`, and nothing in this program renames it. + - the intake COLUMN on the default lineage — deleted by U11 (#2515), which merged Todo + into Planning. + +WHY IT NEEDS A TEST. The lifecycle-column census greps `=== "triage"`, so every +`role === "triage"` and `agentType === "triage"` matched it and read as an un-migrated column +guard. That is not a cosmetic accounting problem: the obvious "finish the migration" edit is +to rename the role, and renaming it silently breaks prompt-template resolution (the planner +lane falls back to an empty prompt) and usage-limit lane accounting — neither of which fails +loudly. `PLANNER_AGENT_ROLE` makes the two vocabularies distinguishable by grep, and these +cases make the distinction fail loudly if someone collapses them. + +The census's error ran in BOTH directions, which is the real lesson: it flagged role +comparisons that were never column guards while MISSING real column guards in executor.ts +whose locals were named `from` and `originColumn`. A census over a shared string is only as +good as its ability to tell the vocabularies apart. +*/ +import { describe, expect, it } from "vitest"; + +import { PLANNER_AGENT_ROLE } from "../types/task-log.js"; +import { resolveAgentPrompt } from "../agent-prompts.js"; +import { resolveDefaultWorkflowIr } from "../builtin-workflows.js"; +import { resolveLifecycleColumns } from "../workflow-lifecycle-traits.js"; + +describe("the planner ROLE and the deleted intake COLUMN share a name and nothing else", () => { + it("keeps the planner role named `triage` after U11 removed the column", () => { + // If a future edit renames the role to match the column vocabulary, this fails FIRST — + // before the silent prompt-resolution and usage-accounting breakage downstream. + expect(PLANNER_AGENT_ROLE).toBe("triage"); + }); + + it("resolves the planner lane's prompt through that role", () => { + const prompt = resolveAgentPrompt(PLANNER_AGENT_ROLE); + + expect(prompt.length).toBeGreaterThan(0); + expect(prompt).toContain("task specification agent"); + }); + + it("proves the default workflow no longer declares a column by that name", () => { + // Both halves of the invariant in one assertion: the role name survives, the column + // name does not. A test that only checked the role would pass even if U11 were reverted. + const columns = (resolveDefaultWorkflowIr() as { columns?: Array<{ id?: string }> }).columns ?? []; + + expect(columns.map((c) => c.id)).not.toContain(PLANNER_AGENT_ROLE); + }); + + it("resolves the default lineage's planning lane by TRAIT, not by that name", () => { + const lifecycle = resolveLifecycleColumns(resolveDefaultWorkflowIr()); + + expect(lifecycle).toBeDefined(); + expect(lifecycle?.intake).toBeDefined(); + expect(lifecycle?.intake).not.toBe(PLANNER_AGENT_ROLE); + // Post-U11 the merged planning column carries intake AND hold, so the two roles resolve + // to the same column. That collapse is the intended end state, not a degenerate case. + expect(lifecycle?.hold).toBe(lifecycle?.intake); + }); +}); diff --git a/packages/core/src/__tests__/postgres/renamed-board-reopen.pg.test.ts b/packages/core/src/__tests__/postgres/renamed-board-reopen.pg.test.ts new file mode 100644 index 0000000000..38d5a398c4 --- /dev/null +++ b/packages/core/src/__tests__/postgres/renamed-board-reopen.pg.test.ts @@ -0,0 +1,187 @@ +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-08:45 (Phase C convergence — E2E evidence): + +THE STORE PATH, not the hook in isolation. `default-workflow-hooks.ts`'s reopen effects +are unit-covered in `reopen-semantics-by-role.test.ts`, but that proves nothing about +whether `moves.ts` actually HANDS the hooks the moving task's resolved lifecycle columns. +If it passes `undefined`, every one of those unit cases still passes (the no-basis +fallback) while the real board silently keeps the old behavior. So this drives a real +PostgreSQL store, a real `moveTask`, and a workflow whose columns carry the standard +traits under NON-default names. + +WHAT IT WOULD HAVE CAUGHT: a card bounced out of the renamed review lane kept its +`passed` review result, because the clear was gated on the literal `in-review`/`todo`. +`getTaskMergeBlocker` reads that array, so the card could re-enter review and merge with +its re-review never run. + +The flag-ON path is the one under test — `isWorkflowColumnsCompatibilityFlagEnabled` +reads the RAW experimental flag, so without enabling it this suite would exercise the +legacy inline branch (which is deliberately left name-based as the parity reference) and +prove nothing. +*/ +import { it, expect, beforeAll, beforeEach, afterEach, afterAll } from "vitest"; +import { eq } from "drizzle-orm"; + +import { + pgDescribe, + createSharedPgTaskStoreTestHarness, +} from "../../__test-utils__/pg-test-harness.js"; +import type { WorkflowIr } from "../../workflow-ir-types.js"; + +/** Standard lifecycle traits under non-default column names, with a reopen edge. */ +function renamedBoardIr(): WorkflowIr { + return { + version: "v2", + name: "test:renamed-board", + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }] }, + { + id: "queued", + name: "Queued", + traits: [{ trait: "hold", config: { release: "capacity" } }, { trait: "reset-on-entry" }], + }, + { + id: "building", + name: "Building", + traits: [ + { trait: "wip", config: { limitSetting: "maxConcurrent", countPending: true } }, + { trait: "abort-on-exit" }, + { trait: "timing" }, + ], + }, + { + id: "checking", + name: "Checking", + traits: [{ trait: "merge" }, { trait: "merge-blocker" }], + }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + /* + FOURTH FIXTURE FINDING, and the one that actually matters for reading this file: the + ARCHIVED column is not decoration. `resolveRoleColumns` returns undefined unless the + workflow declares wip + review + complete + archived + a planning lane, and without + it adjacency silently falls back to ORDER-DERIVED neighbours — under which + `checking -> queued` is not a legal move at all ("Valid targets: building, shipped"). + So a renamed board only gets role-level transitions once its role set is complete; + an incomplete one is treated as a genuinely custom shape, by design. + */ + { id: "archive", name: "Archive", traits: [{ trait: "archived" }] }, + ], + nodes: [ + { id: "start", kind: "start", column: "backlog" }, + /* + THIRD FIXTURE FINDING: a rework edge is legal only INTO a node marked + `config.reworkRegion: true` (or inside a foreach template). So a workflow that wants + a review bounce must declare its bounce TARGETS as rework-region heads — the shape is + opt-in per node, not a property of the edge alone. + */ + { id: "plan", kind: "prompt", column: "queued", config: { name: "Plan", prompt: "Specify.", reworkRegion: true } }, + { id: "build", kind: "prompt", column: "building", config: { name: "Build", prompt: "Do it.", reworkRegion: true } }, + { id: "check", kind: "prompt", column: "checking", config: { name: "Check", prompt: "Review it." } }, + /* + FIXTURE NOTE, and it is the finding of a real rule rather than boilerplate: a column + declaring `merge-blocker` must have a reachable merge-class node or `parseWorkflowIr` + rejects the whole workflow ("the merge-blocker gate can never clear without one"). My + first fixture omitted it and all three cases failed at workflow CREATION, not at the + move — a failure that looks like the code under test and is not. + */ + { id: "merge", kind: "merge-attempt", column: "checking", config: { capability: "task-merge" } }, + { id: "end", kind: "end", column: "shipped" }, + // No node in the archive column — the builtin coding IR declares its `archived` + // column the same way, and adding one only made it an unreachable node. + ], + edges: [ + { from: "start", to: "plan" }, + { from: "plan", to: "build", condition: "success" }, + { from: "build", to: "check", condition: "success" }, + { from: "check", to: "merge", condition: "success" }, + { from: "merge", to: "end", condition: "success" }, + /* + The reopen edges under test: a rejected check goes back to planning, and a renamed + board is entitled to the same bounce the default lineage has. + + SECOND FIXTURE FINDING: these MUST be `kind: "rework"`. `validateNoIllegalCycles` + exempts only rework edges from the acyclicity rule, so a plain back-edge rejects the + whole workflow. Worth knowing before writing any reopen fixture — a bounce edge in + this IR is a rework edge by definition, not an ordinary conditional one. + */ + { from: "check", to: "plan", kind: "rework", condition: "failure" }, + { from: "check", to: "build", kind: "rework", condition: "retry" }, + ], + } as WorkflowIr; +} + +pgDescribe("a renamed board gets the same reopen effects as the default lineage", () => { + const harness = createSharedPgTaskStoreTestHarness({ prefix: "fusion_renamed_reopen" }); + beforeAll(harness.beforeAll); + beforeEach(harness.beforeEach); + afterEach(harness.afterEach); + afterAll(harness.afterAll); + + beforeEach(async () => { + await harness.store().updateGlobalSettings({ experimentalFeatures: { workflowColumns: true } }); + }); + + /** Force a column directly so one move edge can be exercised in isolation. */ + async function forceColumn(taskId: string, column: string): Promise { + const store = harness.store(); + const layer = store.getAsyncLayer(); + if (!layer) throw new Error("expected async layer in backend mode"); + const { project } = await import("../../postgres/schema/index.js"); + await layer.db.update(project.tasks).set({ column }).where(eq(project.tasks.id, taskId)); + } + + async function seedCardInCheck(): Promise<{ store: ReturnType; taskId: string }> { + const store = harness.store(); + const def = await store.createWorkflowDefinition({ name: "Renamed Board", ir: renamedBoardIr() }); + const task = await store.createTask({ description: "renamed board card", workflowId: def.id }); + await store.updateTask(task.id, { + status: "failed", + error: "review rejected", + branch: "fusion/renamed", + summary: "a summary from the failed attempt", + workflowStepResults: [ + { workflowStepId: "code-review", status: "passed", completedAt: "2026-07-30T00:00:00.000Z" }, + ] as never, + }); + await forceColumn(task.id, "checking"); + return { store, taskId: task.id }; + } + + it("clears the stale review result when the renamed review lane bounces to the renamed hold lane", async () => { + const { store, taskId } = await seedCardInCheck(); + + const moved = await store.moveTask(taskId, "queued", { moveSource: "engine" }); + + expect(moved.column).toBe("queued"); + // The safety assertion: a surviving `passed` result satisfies getTaskMergeBlocker. + expect(moved.workflowStepResults ?? []).toHaveLength(0); + expect(moved.branch ?? null).toBeNull(); + expect(moved.summary ?? null).toBeNull(); + expect(moved.status ?? null).toBeNull(); + expect(moved.error ?? null).toBeNull(); + }); + + it("parks a user-source bounce into the renamed hold lane", async () => { + const { store, taskId } = await seedCardInCheck(); + + const moved = await store.moveTask(taskId, "queued", { moveSource: "user" }); + + expect(moved.userPaused).toBe(true); + }); + + it("does NOT strip results on a forward move within the renamed board", async () => { + // The paired negative: "clears on every move" must not pass for "resolves the roles". + const store = harness.store(); + const def = await store.createWorkflowDefinition({ name: "Renamed Fwd", ir: renamedBoardIr() }); + const task = await store.createTask({ description: "forward card", workflowId: def.id }); + await store.updateTask(task.id, { + workflowStepResults: [{ workflowStepId: "code-review", status: "passed" }] as never, + }); + await forceColumn(task.id, "queued"); + + const moved = await store.moveTask(task.id, "building", { moveSource: "engine" }); + + expect(moved.column).toBe("building"); + expect(moved.workflowStepResults ?? []).toHaveLength(1); + }); +}); diff --git a/packages/core/src/__tests__/reopen-semantics-by-role.test.ts b/packages/core/src/__tests__/reopen-semantics-by-role.test.ts new file mode 100644 index 0000000000..1e6a4b5dd6 --- /dev/null +++ b/packages/core/src/__tests__/reopen-semantics-by-role.test.ts @@ -0,0 +1,243 @@ +// @vitest-environment node +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-08:20 (Phase C convergence — default-workflow-hooks.ts): + +THE INVARIANT: a reopen is "live work (wip/review/complete) back into a planning lane +(intake/hold)", decided from the moving task's OWN workflow — not from the default +lineage's column names. + +WHY THIS IS A SAFETY TEST AND NOT A TIDYING TEST. `default-workflow-hooks.ts` is named +for the default workflow but the store runs it on the flag-ON path for EVERY workflow +(the trait registry resolves each hook by trait id, not by workflow). Its reopen +predicates were lists of the default lineage's names, so on a renamed board every reopen +effect silently did nothing. One of those effects clears `workflowStepResults`, and +`getTaskMergeBlocker` reads exactly that: a card bounced out of review kept its OLD +review results, and a `passed` result satisfies the merge gate. So a renamed workflow +could merge with its re-review never run — the same regression the graph-owned-crossing +carve-out in `applyReopenFieldClears` exists to prevent, arriving through the other door. + +The `todo`/`triage` names are the thing being removed here, which is why this file is +part of the triage-guard convergence and not a separate refactor: `default-workflow-hooks.ts` +held 4 of them and `moves.ts`'s flag-ON mirror held 1 more. + +WHAT IS DELIBERATELY NOT CONVERTED: the flag-OFF inline block in `moves.ts` (~line 867). +That branch IS the legacy path and is kept verbatim on purpose so the two paths can be +parity-checked; converting it would erase the reference implementation. +*/ +import { describe, it, expect, beforeEach } from "vitest"; + +import { __resetTraitRegistryForTests } from "../trait-registry.js"; +import { registerBuiltinTraits } from "../builtin-traits.js"; +import { + __resetDefaultWorkflowHooksForTests, + applyDefaultWorkflowMoveEffects, + isReopenIntoPlanning, + registerDefaultWorkflowHooks, + type DefaultWorkflowMoveContext, +} from "../default-workflow-hooks.js"; +import { resolveLifecycleColumns } from "../workflow-lifecycle-traits.js"; +import type { WorkflowIr } from "../workflow-ir-types.js"; +import type { Task } from "../types.js"; + +/** A board whose columns carry the SAME traits under DIFFERENT names. */ +const RENAMED_IR = { + version: "v2", + id: "wf-renamed", + name: "renamed", + nodes: [], + edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }] }, + { id: "queued", name: "Queued", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "checking", name: "Checking", traits: [{ trait: "merge" }, { trait: "merge-blocker" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], +} as unknown as WorkflowIr; + +/** The post-U11 default lineage: `todo` is intake AND hold; `triage` is gone (#2515). */ +const DEFAULT_IR = { + version: "v2", + id: "wf-default", + name: "default", + nodes: [], + edges: [], + columns: [ + { id: "todo", name: "Planning", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] }, + { id: "in-progress", name: "In Progress", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "in-review", name: "In Review", traits: [{ trait: "merge" }, { trait: "merge-blocker" }] }, + { id: "done", name: "Done", traits: [{ trait: "complete" }] }, + ], +} as unknown as WorkflowIr; + +function makeCtx( + ir: WorkflowIr | undefined, + fromColumn: string, + toColumn: string, + overrides: Partial = {}, +): DefaultWorkflowMoveContext { + const task = { + id: "FN-1", + column: toColumn, + columnMovedAt: "2026-07-30T00:00:00.000Z", + steps: [], + dependencies: [], + status: "failed", + error: "boom", + branch: "fusion/FN-1", + summary: "old summary", + workflowStepResults: [{ workflowStepId: "review", status: "passed" }], + } as unknown as Task; + return { + task, + fromColumn, + toColumn, + moveSource: "engine", + bypassGuards: false, + movedAt: "2026-07-30T00:00:01.000Z", + settings: undefined, + options: {}, + lifecycleColumns: ir ? resolveLifecycleColumns(ir) : undefined, + resetSteps: () => {}, + ...overrides, + }; +} + +function applyOn(ir: WorkflowIr | undefined, fromColumn: string, toColumn: string, overrides = {}) { + const ctx = makeCtx(ir, fromColumn, toColumn, overrides); + applyDefaultWorkflowMoveEffects(ctx); + return ctx.task; +} + +describe("a reopen is decided by lifecycle ROLE, not by the default lineage's names", () => { + beforeEach(() => { + __resetTraitRegistryForTests(); + __resetDefaultWorkflowHooksForTests(); + registerBuiltinTraits(); + registerDefaultWorkflowHooks(); + }); + + it("clears stale review results when a RENAMED board bounces review -> hold", () => { + // THE SAFETY CASE. Pre-fix: `checking`/`queued` matched none of the hard-coded + // names, so the `passed` review result survived the bounce and `getTaskMergeBlocker` + // would have let the card merge with its re-review never run. + const task = applyOn(RENAMED_IR, "checking", "queued"); + + expect(task.workflowStepResults).toBeUndefined(); + expect(task.branch).toBeUndefined(); + expect(task.summary).toBeUndefined(); + }); + + it("clears the failure state when a RENAMED board bounces wip -> intake", () => { + const task = applyOn(RENAMED_IR, "building", "backlog"); + + expect(task.status).toBeUndefined(); + expect(task.error).toBeUndefined(); + }); + + it("parks a user-source rebound into the renamed HOLD lane", () => { + // Pre-fix the park never happened off the default lineage, so the scheduler + // re-dispatched the card the operator had just pulled back. + const task = applyOn(RENAMED_IR, "building", "queued", { moveSource: "user" as const }); + + expect(task.userPaused).toBe(true); + }); + + it("still does the same on the DEFAULT lineage (the conversion is not a rename)", () => { + const task = applyOn(DEFAULT_IR, "in-review", "todo"); + + expect(task.workflowStepResults).toBeUndefined(); + expect(task.branch).toBeUndefined(); + expect(task.status).toBeUndefined(); + }); + + it("does NOT treat a forward move into wip as a reopen on a renamed board", () => { + // The paired negative: "clears everything always" must not be able to pass for + // "reads the roles". A backlog -> building move keeps the card's own state. + const task = applyOn(RENAMED_IR, "backlog", "building"); + + expect(task.status).toBe("failed"); + expect(task.workflowStepResults).toHaveLength(1); + }); + + it("does NOT clear results on the graph's own review -> wip crossing (carve-out survives)", () => { + const task = applyOn(RENAMED_IR, "checking", "building", { + workflowMoveSource: "workflow-graph", + }); + + expect(task.workflowStepResults).toHaveLength(1); + }); + + it("DOES clear results on an operator-dragged review -> wip crossing", () => { + const task = applyOn(RENAMED_IR, "checking", "building"); + + expect(task.workflowStepResults).toBeUndefined(); + }); +}); + +describe("no column vocabulary is the only case a legacy name is legitimate", () => { + beforeEach(() => { + __resetTraitRegistryForTests(); + __resetDefaultWorkflowHooksForTests(); + registerBuiltinTraits(); + registerDefaultWorkflowHooks(); + }); + + it("falls back to the legacy names for a v1 / column-less IR", () => { + // `resolveLifecycleColumns` returns undefined for the WHOLE struct here, which means + // "no basis to decide" — not "declares no hold column". The legacy names are all + // there is, and a pre-v2 row really does live in `in-review`/`todo`. + const task = applyOn(undefined, "in-review", "todo"); + + expect(task.workflowStepResults).toBeUndefined(); + expect(task.status).toBeUndefined(); + }); + + it("does NOT substitute a legacy name for a role the workflow genuinely lacks", () => { + // A workflow with intake but NO hold: `queued` is not a lane on this board, so a move + // there is not a reopen. Substituting `todo` would invent a lane the operator removed. + const holdlessIr = { + version: "v2", id: "wf-no-hold", name: "no-hold", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + ], + } as unknown as WorkflowIr; + + const lifecycle = resolveLifecycleColumns(holdlessIr); + + expect(isReopenIntoPlanning(lifecycle, "building", "todo")).toBe(false); + expect(isReopenIntoPlanning(lifecycle, "building", "backlog")).toBe(true); + }); +}); + +describe("the store's reopen check and the hooks' cannot disagree", () => { + beforeEach(() => { + __resetTraitRegistryForTests(); + registerBuiltinTraits(); + }); + + it("answers identically for every from/to pair on a renamed board", () => { + /* + `moves.ts` used to carry its own hand-written copy of this predicate, annotated + "parity mirror". Two copies of one rule diverge on whichever the next edit misses; + it now calls this function. This pins that there is ONE answer per pair, which is the + property the mirror was trying to have. + */ + const lifecycle = resolveLifecycleColumns(RENAMED_IR); + const columns = ["backlog", "queued", "building", "checking", "shipped"]; + + const reopens = columns.flatMap((from) => + columns.filter((to) => isReopenIntoPlanning(lifecycle, from, to)).map((to) => `${from}->${to}`), + ); + + expect(reopens.sort()).toEqual([ + "building->backlog", + "building->queued", + "checking->backlog", + "checking->queued", + "shipped->backlog", + "shipped->queued", + ]); + }); +}); diff --git a/packages/core/src/agent-prompts.ts b/packages/core/src/agent-prompts.ts index 97db90c209..3cc1016f2e 100644 --- a/packages/core/src/agent-prompts.ts +++ b/packages/core/src/agent-prompts.ts @@ -16,6 +16,9 @@ */ import type { AgentCapability, AgentPromptTemplate, AgentPromptsConfig } from "./types.js"; +// FNXC:WorkflowLifecycleColumns 2026-07-30-11:00: these are ROLE comparisons, not column +// guards — the planner LANE is named `triage` and stays named that. See PLANNER_AGENT_ROLE. +import { PLANNER_AGENT_ROLE } from "./types/task-log.js"; // --------------------------------------------------------------------------- // Built-in prompt text (canonical source for workflow seam prompts) @@ -1466,10 +1469,10 @@ export function resolveAgentPrompt( ); } - if (role === "triage" && template.builtIn && template.id === "default-triage") { + if (role === PLANNER_AGENT_ROLE && template.builtIn && template.id === "default-triage") { return `${TRIAGE_PROMPT_TEXT}\n\n${buildTriageHeartbeatGuidance(options)}`; } - if (role === "triage" && template.builtIn && template.id === "concise-triage") { + if (role === PLANNER_AGENT_ROLE && template.builtIn && template.id === "concise-triage") { return `${CONCISE_TRIAGE_PROMPT_TEXT}\n\n${buildConciseTriageHeartbeatGuidance(options)}`; } return template.prompt; @@ -1477,7 +1480,7 @@ export function resolveAgentPrompt( // Fall back to built-in default for the role const builtIn = BUILTIN_AGENT_PROMPTS.find((t) => t.role === role && t.id === `default-${role}`); - if (role === "triage" && builtIn?.id === "default-triage") { + if (role === PLANNER_AGENT_ROLE && builtIn?.id === "default-triage") { return `${TRIAGE_PROMPT_TEXT}\n\n${buildTriageHeartbeatGuidance(options)}`; } return builtIn?.prompt ?? ""; diff --git a/packages/core/src/default-workflow-hooks.ts b/packages/core/src/default-workflow-hooks.ts index d61bf6da7d..db54a88e53 100644 --- a/packages/core/src/default-workflow-hooks.ts +++ b/packages/core/src/default-workflow-hooks.ts @@ -32,6 +32,7 @@ */ import { getTraitRegistry } from "./trait-registry.js"; +import type { LifecycleColumns } from "./workflow-lifecycle-traits.js"; import type { TraitAuditWarning } from "./trait-registry.js"; import { getTaskMergeBlocker } from "./task-merge.js"; import type { Settings, Task } from "./types.js"; @@ -98,6 +99,23 @@ export interface DefaultWorkflowMoveContext { preserveWorktree?: boolean; preservePause?: boolean; }; + /** + * FNXC:WorkflowLifecycleColumns 2026-07-30-08:05 (Phase C convergence): + * The moving task's OWN lifecycle columns, resolved by trait from its workflow IR by + * the store (which already holds the IR on this path) and passed in because these + * hooks are sync and in-lock — they cannot resolve anything themselves. + * + * WHY THIS FILE NEEDED IT AT ALL. Its name says "default workflow", but the store + * runs these hooks on the flag-ON path for EVERY workflow — the trait registry + * resolves the hook by trait id, not by workflow. So the column names hard-coded + * here were the DEFAULT lineage's names being applied to a renamed board, where the + * reopen effects simply never fired. See `applyResetOnEntryEffects`. + * + * `undefined` means the workflow has no column vocabulary at all (v1 IR), which is + * NOT the same as "declares no hold column" — the hooks keep the legacy literals + * only in that no-basis case, never as a substitute for an absent role. + */ + lifecycleColumns?: LifecycleColumns | undefined; /** Reset all steps to pending + currentStep 0 (store owns the impl). */ resetSteps: () => void; } @@ -152,14 +170,51 @@ export function applyCompletionTimingEffects(ctx: DefaultWorkflowMoveContext): v } } -/** `reset-on-entry` trait (todo/triage reopen) + `abort-on-exit` userPaused - * semantics. Reproduces the legacy reopen block. */ +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-08:05 (Phase C convergence — reopen semantics): + +WHAT A "REOPEN" IS, stated once. A card leaving live work (wip / review / complete) for a +PLANNING lane (intake or hold). The three predicates below were each written as a list of +the default lineage's column names, which meant every reopen effect — status/error clear, +step reset, `workflowStepResults` clear, branch clear — was a no-op on any workflow that +renamed its columns. + +THE CONSEQUENCE WAS NOT COSMETIC. `getTaskMergeBlocker` reads `workflowStepResults`; the +executor's documented bounce invariant is "moveTask(in-review -> planning) clears ALL +results". On a renamed board that clear never happened, so a card bounced out of review +and back in carried its OLD review results — and a `passed` result satisfies the merge +gate. A renamed workflow could merge with its re-review never run. That is the same +safety regression the graph-owned-crossing carve-out above was written to prevent, +arriving through the other door. + +LEGACY IDS ARE A NO-BASIS FALLBACK, NOT A ROLE. When the struct is undefined (a v1 IR +with no column vocabulary) there is nothing to reason from and the legacy names are all +we have. When the struct EXISTS but a role is absent, the workflow genuinely has no such +lane and no substitution is made — that is the distinction `resolveLifecycleColumns` +returns `undefined`-for-the-whole-struct to preserve. +*/ +const LEGACY_PLANNING_COLUMNS = ["todo", "triage"] as const; +const LEGACY_LIVE_WORK_COLUMNS = ["in-progress", "done", "in-review"] as const; + +/** The planning lanes of THIS workflow: intake and hold. */ +function planningColumnsOf(lifecycle: LifecycleColumns | undefined): readonly string[] { + if (!lifecycle) return LEGACY_PLANNING_COLUMNS; + return [lifecycle.intake, lifecycle.hold].filter((c): c is string => typeof c === "string"); +} + +/** The lanes a card is reopened OUT of: wip, review, complete. */ +function liveWorkColumnsOf(lifecycle: LifecycleColumns | undefined): readonly string[] { + if (!lifecycle) return LEGACY_LIVE_WORK_COLUMNS; + return [lifecycle.wip, lifecycle.review, lifecycle.complete].filter( + (c): c is string => typeof c === "string", + ); +} + +/** `reset-on-entry` trait (reopen into a planning lane) + `abort-on-exit` userPaused + * semantics. Reproduces the legacy reopen block, by role rather than by name. */ export function applyResetOnEntryEffects(ctx: DefaultWorkflowMoveContext): void { const { task, fromColumn, toColumn, moveSource, options } = ctx; - const isReopenToTodoOrTriage = - (fromColumn === "in-progress" || fromColumn === "done" || fromColumn === "in-review") && - (toColumn === "todo" || toColumn === "triage"); - if (!isReopenToTodoOrTriage) return; + if (!isReopenIntoPlanning(ctx.lifecycleColumns, fromColumn, toColumn)) return; /* FNXC:WorkflowLifecycle 2026-07-12-09:05: @@ -179,8 +234,19 @@ export function applyResetOnEntryEffects(ctx: DefaultWorkflowMoveContext): void task.paused = undefined; task.pausedByAgentId = undefined; } - // abort-on-exit userPaused: only for user-source moves to todo (KTD-9). - if (moveSource === "user" && toColumn === "todo") { + /* + abort-on-exit userPaused: only for user-source moves to the HOLD lane (KTD-9). + FNXC:WorkflowLifecycleColumns 2026-07-30-08:05: `todo` was the hold lane's name on the + pre-U11 default lineage and is still its id post-U11 (#2515 merged Todo into Planning + keeping `todo`), so this reads as hold-then-intake. The role matters, not the name: an + operator dragging a card back to the queue is parking it, and on a renamed board that + park silently stopped happening — the scheduler then re-dispatched the card the + operator had just pulled back. + */ + const holdLane = ctx.lifecycleColumns + ? ctx.lifecycleColumns.hold ?? ctx.lifecycleColumns.intake + : "todo"; + if (moveSource === "user" && toColumn === holdLane) { task.userPaused = true; } else if (!options.preservePause) { task.userPaused = undefined; @@ -206,6 +272,25 @@ export function applyResetOnEntryEffects(ctx: DefaultWorkflowMoveContext): void } } +/** + * Is this move a reopen — live work (wip/review/complete) back into a planning lane + * (intake/hold)? + * + * FNXC:WorkflowLifecycleColumns 2026-07-30-08:05: EXPORTED so the store's flag-ON + * `preserveStepProgress` mirror asks the same question. Those two predicates were + * separately hand-written copies of the same column list ("Parity mirror of the gate in + * applyReopenFieldClears"), and a hand-copied predicate is a divergence waiting for + * whichever copy the next edit misses. One function cannot disagree with itself. + */ +export function isReopenIntoPlanning( + lifecycle: LifecycleColumns | undefined, + fromColumn: string, + toColumn: string, +): boolean { + return liveWorkColumnsOf(lifecycle).includes(fromColumn) + && planningColumnsOf(lifecycle).includes(toColumn); +} + /** `merge` trait onEnter (in-review): scheduler-state clearing while * preserving explicit per-task autoMerge overrides. The queue enqueue itself is * in-txn and store-owned (handoff path); the field effects mirror the legacy @@ -246,17 +331,30 @@ export function applyReopenFieldClears(ctx: DefaultWorkflowMoveContext): void { executor's documented bounce invariant ("moveTask(in-review->todo) already clears ALL results") survives unchanged. */ + const lifecycle = ctx.lifecycleColumns; + const planning = planningColumnsOf(lifecycle); + const reviewLane = lifecycle ? lifecycle.review : "in-review"; + const wipLane = lifecycle ? lifecycle.wip : "in-progress"; + const completeLane = lifecycle ? lifecycle.complete : "done"; + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-08:30 (Phase C convergence): + THE CARVE-OUT MUST BE RESOLVED TOO, and forgetting it was worse than leaving the whole + function alone. A role-resolved clear plus a NAME-matched exemption means the renamed + board takes the clear and never the exemption — so the graph's own remediation crossing + destroyed the `failed` result it had just written, which is precisely the three breakages + the note above enumerates. My own paired negative test caught this; a conversion that + moves the rule and leaves its exception behind inverts the exception. + */ const graphOwnedReviewToWip = ctx.workflowMoveSource === "workflow-graph" - && fromColumn === "in-review" - && toColumn === "in-progress"; - if ( - !graphOwnedReviewToWip - && ((fromColumn === "in-review" && (toColumn === "todo" || toColumn === "in-progress" || toColumn === "triage")) - || (fromColumn === "done" && (toColumn === "todo" || toColumn === "triage"))) - ) { + && fromColumn === reviewLane + && toColumn === wipLane; + const leftReviewForPlanningOrWip = + fromColumn === reviewLane && (planning.includes(toColumn) || toColumn === wipLane); + const leftCompleteForPlanning = fromColumn === completeLane && planning.includes(toColumn); + if (!graphOwnedReviewToWip && (leftReviewForPlanningOrWip || leftCompleteForPlanning)) { task.workflowStepResults = undefined; } - if (fromColumn === "in-review" && (toColumn === "todo" || toColumn === "triage")) { + if (fromColumn === reviewLane && planning.includes(toColumn)) { task.branch = undefined; task.executionStartBranch = undefined; task.baseCommitSha = undefined; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index f9c2bb7437..cfa0203d55 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -38,7 +38,7 @@ export type { MissionLineageApprovalResult, MissionLineageSnapshot, } from "./symbol-lock-lineage-approval.js"; -export { AGENT_VALID_TRANSITIONS, DUPLICATE_OF_METADATA_KEY, REPORT_ATTACHMENT_SOURCE, assertNotWorkspaceTaskMerge, isWorkspaceTask, WorkspaceTaskMergeError } from "./types.js"; +export { PLANNER_AGENT_ROLE, AGENT_VALID_TRANSITIONS, DUPLICATE_OF_METADATA_KEY, REPORT_ATTACHMENT_SOURCE, assertNotWorkspaceTaskMerge, isWorkspaceTask, WorkspaceTaskMergeError } from "./types.js"; export { resolveEntryPointBranchAssignment, sanitizeBranchSegment, diff --git a/packages/core/src/live-agent-count.ts b/packages/core/src/live-agent-count.ts index a07693e602..ea7cc6f89e 100644 --- a/packages/core/src/live-agent-count.ts +++ b/packages/core/src/live-agent-count.ts @@ -65,6 +65,37 @@ export function resolveColumnTerminalKind(columnId: string, ir: WorkflowIr): Col return "none"; } +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-10:20 (Phase C convergence — live-agent-count.ts): + +THE PRE-IMPLEMENTATION FALLBACK, named once instead of spelled out at two call sites. + +DELIBERATE-LITERAL, and the reason is not "we ran out of time": this is the answer used when +the caller supplies NO trait flags at all. There is nothing to resolve from. `enrich...FromFlags` +exists precisely for callers that have board-column flags rather than an IR (the dashboard +footer), and a column missing from that flag map is the renamed-or-undeclared case. + +Converting it would mean deciding what an ABSENT flag set means, and "not intake" is as much a +guess as "todo is intake" — either choice silently moves an operator-visible count. The two +counts this feeds (Running and Waiting) are complements over the same rows, so a card matching +neither arm is reported as neither running nor waiting and the footer's queued total +under-reports it. Guessing here is worse than the known legacy answer. + +The real fix for a renamed board is at the CALLER: supply flags (or use +`enrichRunningAgentTaskShape`, which takes the IR and resolves every role by trait). This +fallback only has to keep behaving exactly as it did for legacy rows. + +Both former literal sites now share this function, so the pair cannot drift apart — they were +two hand-written copies of one rule, and line 84 answering differently from line 143 would put +a card in both counts or neither. +*/ +const LEGACY_PRE_IMPLEMENTATION_COLUMN_IDS: ReadonlySet = new Set(["triage", "todo"]); + +/** Legacy-vocabulary "is this column a planner lane?", for callers that supply no traits. */ +function isLegacyPreImplementationColumn(columnId: string): boolean { + return LEGACY_PRE_IMPLEMENTATION_COLUMN_IDS.has(columnId); +} + /** Attach the workflow traits required by the pure Running and Waiting predicates. */ export function enrichRunningAgentTaskShape(task: T, ir: WorkflowIr): T & Required> { return { @@ -81,18 +112,13 @@ export function enrichRunningAgentTaskShapeFromFlags store.resetAllStepsToPending(task), + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-08:10 (Phase C convergence): + The hooks are sync and in-lock, so they cannot resolve a workflow themselves — but + this path already holds `workflowIr`, so the roles cost one trait resolution and no + extra read. Without them the hooks compared against the DEFAULT lineage's column + names on every workflow, so a renamed board got no reopen effects at all. + */ + lifecycleColumns: moveLifecycleColumns, }; - const isReopenToTodoOrTriage = - (fromColumn === "in-progress" || fromColumn === "done" || fromColumn === "in-review") && - (toColumn === "todo" || toColumn === "triage"); + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-08:10: the store's own copy of the reopen + predicate now CALLS the hooks' version instead of restating its column list. The + comment below used to say "parity mirror" — two hand-written copies of one predicate, + which is a divergence waiting for whichever copy the next edit misses. + */ + const isReopenToTodoOrTriage = isReopenIntoPlanning(moveLifecycleColumns, fromColumn, toColumn); const hasNonPendingStepProgress = task.steps.some((step) => step.status !== "pending"); const preserveStepProgress = options?.preserveResumeState || diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index f1bd48281b..4dc0f6dae6 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -346,6 +346,10 @@ export interface BatchStatusResponse { // ── task-log ────────────────────────────────────────────────────────── // FNXC:CodeOrganization 2026-07-22-14:00: Peels live in types/task-log.ts +// FNXC:WorkflowLifecycleColumns 2026-07-30-10:55: a VALUE re-export (not a type) — the +// planner AGENT ROLE name, so role comparisons stop reading as column guards. +export { PLANNER_AGENT_ROLE } from "./types/task-log.js"; + import type { StepStatus, WorkflowTransitionNotificationKind, diff --git a/packages/core/src/types/task-log.ts b/packages/core/src/types/task-log.ts index d1f200421f..b4a6d3bdca 100644 --- a/packages/core/src/types/task-log.ts +++ b/packages/core/src/types/task-log.ts @@ -89,6 +89,26 @@ export interface ActivityLogEntry { /** The set of agent roles that produce log entries. */ export type AgentRole = "triage" | "executor" | "reviewer" | "merger"; +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-10:55 (Phase C convergence — vocabulary hygiene): + +A ROLE IS NOT A COLUMN, and the string `"triage"` names both. The planning LANE — the engine +service, its prompt templates, its usage-limit accounting — is called `triage` in the +AgentRole vocabulary, and that name is not going anywhere: U11 removed the `triage` COLUMN +from the default lineage, not the planner lane. + +This constant exists so those comparisons stop reading as un-migrated column guards. The +lifecycle-column census greps `=== "triage"`, and `role === "triage"` / `agentType === +"triage"` matched it — sending reviewers to sites that were never column guards while the +pattern simultaneously MISSED real guards whose locals were named `from` or `originColumn`. +Naming the role makes the two vocabularies distinguishable by grep, which is the only way a +census over a shared string can be trusted. + +Use this at every role comparison. If you are comparing a task's COLUMN, you want a lifecycle +role resolved from the workflow IR (`resolveLifecycleColumns`), not this. +*/ +export const PLANNER_AGENT_ROLE: AgentRole = "triage"; + /* FNXC:AgentLog-EntryTypes 2026-07-15-11:20: `text` means a STREAMED DELTA FRAGMENT: renderers re-glue consecutive `text` rows with `join("")` and no separator, because that is the only way to reconstitute a streamed message (the FN-5787/5789/5803 streamed-spacing lineage). `AgentLogger` is the only producer of true deltas. diff --git a/packages/dashboard/app/App.tsx b/packages/dashboard/app/App.tsx index e6e9f45899..276e41d27a 100644 --- a/packages/dashboard/app/App.tsx +++ b/packages/dashboard/app/App.tsx @@ -1710,6 +1710,9 @@ function AppInner() { capacityRiskDismissed, capacityRiskSignal, handleDismissCapacityRisk, + // FNXC:WorkflowLifecycleColumns 2026-07-30-12:15: reuse the footer's per-task column traits + // so main-content views resolve lifecycle roles instead of matching column names. + columnFlagsByTaskId: footerColumnFlagsByTaskId, AgentsView, ChatView, CommandCenter, diff --git a/packages/dashboard/app/__tests__/documents-status-dot-by-role.test.ts b/packages/dashboard/app/__tests__/documents-status-dot-by-role.test.ts new file mode 100644 index 0000000000..3c66d048d1 --- /dev/null +++ b/packages/dashboard/app/__tests__/documents-status-dot-by-role.test.ts @@ -0,0 +1,64 @@ +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-12:25 (Phase C convergence — DocumentsView.tsx): + +The task-documents status dot encodes four LIFECYCLE ROLES: complete, archived, +pre-implementation (waiting), and everything else (working). Only the pre-implementation arm +named columns — and it named the default lineage's two — so on a renamed board a queued card +showed the "working" dot. + +Both halves are pinned here because either alone is misleading: flags must DECIDE when present +(otherwise the conversion is decoration), and the legacy names must still answer when flags are +ABSENT (the documents list spans archived and historical tasks whose columns are not on the +current board, and "not pre-implementation" would be as much a guess as the legacy pair — the +same no-basis rule as `live-agent-count.ts`). +*/ +import { describe, expect, it } from "vitest"; + +import { getTaskColumnStatusDotClass } from "../components/DocumentsView"; + +const PENDING = "status-dot status-dot--pending"; +const WORKING = "status-dot status-dot--connecting"; +const DONE = "status-dot status-dot--online"; +const ARCHIVED = "status-dot status-dot--offline"; + +describe("the documents status dot resolves lifecycle roles when traits are available", () => { + it("shows the waiting dot for a RENAMED planning column via its traits", () => { + // Pre-fix: `backlog` matched neither literal, so a queued card read as working. + expect(getTaskColumnStatusDotClass("backlog", { intake: true })).toBe(PENDING); + expect(getTaskColumnStatusDotClass("queued", { hold: true })).toBe(PENDING); + }); + + it("shows the working dot for a renamed WIP column", () => { + expect(getTaskColumnStatusDotClass("building", {})).toBe(WORKING); + }); + + it("prefers archived over complete when a column carries both", () => { + // Order matters: an archived-and-complete column is archived to an operator scanning dots. + expect(getTaskColumnStatusDotClass("shipped", { archived: true, complete: true })).toBe(ARCHIVED); + expect(getTaskColumnStatusDotClass("shipped", { complete: true })).toBe(DONE); + }); + + it("lets traits OVERRIDE a legacy name in both directions", () => { + // A board that declares `todo` as its complete column: traits win, not the name. + expect(getTaskColumnStatusDotClass("todo", { complete: true })).toBe(DONE); + // And a board that declares `done` as intake. + expect(getTaskColumnStatusDotClass("done", { intake: true })).toBe(PENDING); + }); +}); + +describe("with no traits the documented legacy names still answer", () => { + it("keeps the pre-U11 planner ids on the waiting dot", () => { + expect(getTaskColumnStatusDotClass("todo")).toBe(PENDING); + expect(getTaskColumnStatusDotClass("triage")).toBe(PENDING); + }); + + it("keeps done and archived", () => { + expect(getTaskColumnStatusDotClass("done")).toBe(DONE); + expect(getTaskColumnStatusDotClass("archived")).toBe(ARCHIVED); + }); + + it("does NOT invent a role for a renamed column with no traits", () => { + // The known, documented gap: unchanged behavior, not a guess. Supplying flags is the fix. + expect(getTaskColumnStatusDotClass("backlog")).toBe(WORKING); + }); +}); diff --git a/packages/dashboard/app/components/AgentLogViewer.tsx b/packages/dashboard/app/components/AgentLogViewer.tsx index af4554cacb..b57e435fc0 100644 --- a/packages/dashboard/app/components/AgentLogViewer.tsx +++ b/packages/dashboard/app/components/AgentLogViewer.tsx @@ -1,4 +1,7 @@ import type { AgentLogEntry } from "@fusion/core"; +// FNXC:WorkflowLifecycleColumns 2026-07-30-11:50: these are AGENT ROLE comparisons, not +// column guards — the planner LANE keeps the name `triage`; U11 removed only the COLUMN. +import { PLANNER_AGENT_ROLE } from "@fusion/core"; import { useTranslation } from "react-i18next"; import type { TFunction } from "i18next"; import { ProviderIcon } from "./ProviderIcon"; @@ -101,7 +104,7 @@ export const markdownComponents: Components = { const BOTTOM_FOLLOW_THRESHOLD_PX = 50; function getAgentDisplayName(agent: string, t: TFunction<"app">): string { - if (agent === "triage") return t("agentLog.agentNameTriage", "Plan"); + if (agent === PLANNER_AGENT_ROLE) return t("agentLog.agentNameTriage", "Plan"); return agent; } diff --git a/packages/dashboard/app/components/DocumentsView.tsx b/packages/dashboard/app/components/DocumentsView.tsx index f180b34613..238eaa21de 100644 --- a/packages/dashboard/app/components/DocumentsView.tsx +++ b/packages/dashboard/app/components/DocumentsView.tsx @@ -18,6 +18,7 @@ import { LoadingSpinner } from "./LoadingSpinner"; import { ArtifactsGallery, getArtifactCategory, type ArtifactCategory } from "./ArtifactsGallery"; import { ViewHeader } from "./ViewHeader"; import { useColumnLabel } from "../i18n/labels"; +import type { ExecutorColumnFlags } from "../hooks/useExecutorStats"; const MOBILE_BREAKPOINT = 768; @@ -42,8 +43,17 @@ const TASK_ARTIFACT_CATEGORY_ICONS: Record = other: Package, }; +/** Board-workflow column traits, indexed by task id — the shape App already builds for the footer. */ +export type DocumentsColumnFlags = Pick; + export interface DocumentsViewProps { projectId?: string; + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-12:05: optional on purpose — the status dot degrades + to the documented legacy names when a task's column has no traits (remote rows, historical + columns absent from the current board), rather than guessing. + */ + columnFlagsByTaskId?: ReadonlyMap; addToast: (message: string, type?: ToastType) => void; onOpenDetail: (task: TaskDetail) => void; onOpenArtifactTaskDetail?: (task: TaskDetail) => void; @@ -67,13 +77,38 @@ function formatFileSize(bytes: number): string { return `${(bytes / (1024 * 1024)).toFixed(1)} MB`; } -function getTaskColumnStatusDotClass(taskColumn: string): string { +/* +FNXC:WorkflowLifecycleColumns 2026-07-30-12:05 (Phase C convergence — DocumentsView.tsx): + +WHAT THE GUARD MEANT, checked rather than swapped: the four dots are LIFECYCLE ROLES — +complete, archived, pre-implementation (waiting), and everything else (working). Only the +pre-implementation arm named columns, and it named the default lineage's two, so on a renamed +board a queued card showed the "working" dot. + +FLAGS FIRST, legacy names only with NO BASIS. `flags` are the board-workflow column traits the +dashboard already threads to the footer (`ExecutorColumnFlags`); when present they decide, and +they cover renamed and custom columns. When ABSENT there is nothing to resolve from — the +documents list spans archived and historical tasks whose columns are not in the current board +map — and "not pre-implementation" would be as much a guess as the legacy pair. Same rule as +`live-agent-count.ts`'s no-flags fallback, and same reason: an invented answer moves what the +operator sees. +*/ +export function getTaskColumnStatusDotClass(taskColumn: string, flags?: DocumentsColumnFlags): string { + if (flags) { + if (flags.archived) return "status-dot status-dot--offline"; + if (flags.complete) return "status-dot status-dot--online"; + if (flags.intake || flags.hold) return "status-dot status-dot--pending"; + return "status-dot status-dot--connecting"; + } if (taskColumn === "done") return "status-dot status-dot--online"; if (taskColumn === "archived") return "status-dot status-dot--offline"; - if (taskColumn === "todo" || taskColumn === "triage") return "status-dot status-dot--pending"; + if (LEGACY_PRE_IMPLEMENTATION_COLUMNS.has(taskColumn)) return "status-dot status-dot--pending"; return "status-dot status-dot--connecting"; } +/** The pre-U11 planner column ids, used only when a column has no trait flags. */ +const LEGACY_PRE_IMPLEMENTATION_COLUMNS: ReadonlySet = new Set(["triage", "todo"]); + function getTaskArtifactCategoryLabel(t: TFunction<"app">, category: ArtifactCategory): string { switch (category) { case "image": return t("documents.artifactCategoryImage", "Image"); @@ -213,7 +248,7 @@ function TaskArtifactInlineViewer({ artifact, projectId, content, loading, error ); } -export function DocumentsView({ projectId, addToast, onOpenDetail, onOpenArtifactTaskDetail, onSendSelectionToTask }: DocumentsViewProps) { +export function DocumentsView({ projectId, columnFlagsByTaskId, addToast, onOpenDetail, onOpenArtifactTaskDetail, onSendSelectionToTask }: DocumentsViewProps) { const { t } = useTranslation("app"); // FNXC:ArtifactsView 2026-07-11-11:30: Artifacts is the first tab and the landing tab — the view is the artifact gallery first, with project files and task documents as secondary tabs. const [activeTab, setActiveTab] = useState("artifacts"); @@ -1048,7 +1083,9 @@ export function DocumentsView({ projectId, addToast, onOpenDetail, onOpenArtifac