diff --git a/.changeset/u12-move-targets-on-wire.md b/.changeset/u12-move-targets-on-wire.md new file mode 100644 index 0000000000..2054ff5090 --- /dev/null +++ b/.changeset/u12-move-targets-on-wire.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Move menus on custom workflows now offer exactly the moves that workflow allows. +category: fix +dev: The board-workflows payload gains a per-column `moveTargets` array from `resolveAllowedColumns` — the same resolver `moveTaskInternal` validates against. `getTaskMoveTransitions` reads it instead of approximating targets from neighbouring columns, and the `VALID_TRANSITIONS` default-column-set shortcut is deleted; `builtin-adjacency-matches-legacy-transitions.test.ts` pins the equivalence that made deleting it safe. Optional on the wire, so an older client keeps the neighbour fallback. diff --git a/packages/core/src/__tests__/builtin-adjacency-matches-legacy-transitions.test.ts b/packages/core/src/__tests__/builtin-adjacency-matches-legacy-transitions.test.ts new file mode 100644 index 0000000000..b0f957a87f --- /dev/null +++ b/packages/core/src/__tests__/builtin-adjacency-matches-legacy-transitions.test.ts @@ -0,0 +1,36 @@ +/* +FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8): +Pins the equivalence that let the dashboard's `VALID_TRANSITIONS` shortcut be DELETED. + +The move menu could not use workflow adjacency because none was on the wire, so it +approximated targets from a column's neighbours in declared order and kept a legacy +shortcut for workflows whose column-id set matched the six built-ins — because the +approximation is strictly weaker (in-progress: 4 real targets vs 2 neighbours). + +Adjacency is now on the board-workflows payload, and the shortcut is gone. That is +only safe because `resolveAllowedColumns(BUILTIN_CODING_WORKFLOW_IR, c)` is identical +to `VALID_TRANSITIONS[c]` — same members AND same order — for every built-in column. +This test is what stops that equivalence drifting silently: if the built-in workflow's +edges change without `VALID_TRANSITIONS` following, default-workflow move menus change +shape and this fails first. + +It intentionally compares ORDER too, not just membership: the menu renders targets in +the order it receives them, so a reordering is an operator-visible change. +*/ +import { describe, expect, it } from "vitest"; +import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js"; +import { resolveAllowedColumns } from "../workflow-transitions.js"; +import { COLUMNS } from "../types/board.js"; +import { VALID_TRANSITIONS } from "../types/board-config.js"; + +describe("built-in workflow adjacency vs the legacy transition table", () => { + it.each(COLUMNS)("column %s resolves the same targets, in the same order", (column) => { + expect(resolveAllowedColumns(BUILTIN_CODING_WORKFLOW_IR, column)).toEqual([ + ...VALID_TRANSITIONS[column], + ]); + }); + + it("covers every legacy column, so a new one cannot slip past this pin", () => { + expect(COLUMNS.length).toBe(Object.keys(VALID_TRANSITIONS).length); + }); +}); diff --git a/packages/dashboard/app/api/board-workflows.ts b/packages/dashboard/app/api/board-workflows.ts index addd54f75c..622f1f4101 100644 --- a/packages/dashboard/app/api/board-workflows.ts +++ b/packages/dashboard/app/api/board-workflows.ts @@ -54,6 +54,14 @@ export interface BoardWorkflowColumn { /** Optional author-defined explanatory copy from the workflow IR. */ description?: string; flags: BoardWorkflowColumnFlags; + /* + FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8): + Columns this one may move to, from the workflow's own graph adjacency + (`resolveAllowedColumns`, the same resolver `moveTaskInternal` validates against). + Optional so a client that predates the field keeps working; the move menu falls back + to approximating targets from neighbouring columns when it is absent. + */ + moveTargets?: string[]; } export interface BoardWorkflowDefinition { diff --git a/packages/dashboard/app/components/Board.tsx b/packages/dashboard/app/components/Board.tsx index ac1732d651..73f737721a 100644 --- a/packages/dashboard/app/components/Board.tsx +++ b/packages/dashboard/app/components/Board.tsx @@ -596,7 +596,7 @@ export function Board({ tasks, projectId, maxConcurrent, showWorktreeGrouping, o for (const workflow of boardWorkflows?.workflows ?? []) { map.set(workflow.id, workflow.columns .filter((column) => !column.flags.hiddenFromBoard) - .map((column) => ({ id: column.id, label: column.name, flags: column.flags }))); + .map((column) => ({ id: column.id, label: column.name, flags: column.flags, ...(column.moveTargets ? { moveTargets: column.moveTargets } : {}) }))); } return map; }, [boardWorkflows]); diff --git a/packages/dashboard/app/components/Lane.tsx b/packages/dashboard/app/components/Lane.tsx index 1732934269..4615751b3b 100644 --- a/packages/dashboard/app/components/Lane.tsx +++ b/packages/dashboard/app/components/Lane.tsx @@ -88,7 +88,7 @@ function LaneComponent(props: LaneProps) { const contextMenuColumns = useMemo( () => workflow.columns .filter((col) => !col.flags.hiddenFromBoard) - .map((col) => ({ id: col.id, label: col.name, flags: col.flags })), + .map((col) => ({ id: col.id, label: col.name, flags: col.flags, ...(col.moveTargets ? { moveTargets: col.moveTargets } : {}) })), [workflow.columns], ); const createColumnId = useMemo(() => ( diff --git a/packages/dashboard/app/components/ListView.tsx b/packages/dashboard/app/components/ListView.tsx index f734fae618..f81c440844 100644 --- a/packages/dashboard/app/components/ListView.tsx +++ b/packages/dashboard/app/components/ListView.tsx @@ -754,9 +754,86 @@ export function ListView({ const listContextMenuColumns = useMemo(() => { if (!workflowMode) return undefined; + /* + FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — PR #2525 review, greptile): + NO `moveTargets` on the shared list. In the "All workflows" view `listColumns` is a + UNION across workflows keyed by column id, so two workflows that reuse an id but + declare different edges collapse into one entry — and every task would be handed + the first workflow's adjacency. That offers moves the store rejects and hides legal + ones. Adjacency is per-workflow and must be resolved per TASK, which + `taskContextMenuColumnsByTaskId` below does; this shared list keeps labels and + flags only, where the union is harmless. + */ return listColumns.map((column) => ({ id: column.id, label: column.name, flags: column.flags })); }, [listColumns, workflowMode]); + /* + FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — PR #2525 review, greptile): + Per-task column metadata, mirroring Board's `taskContextMenuColumnsByTaskId`. Each + task gets ITS OWN workflow's columns — including that workflow's `moveTargets` — so + the aggregate view cannot serve one workflow's adjacency to another's card. Falls + back to the shared union when the task's workflow is unresolvable, which yields the + previous (neighbour-approximated) behaviour rather than a wrong answer. + */ + const taskContextMenuColumnsByTaskId = useMemo(() => { + const map = new Map(); + if (!workflowMode || !boardWorkflows) return map; + const byWorkflowId = new Map(); + for (const workflow of boardWorkflows.workflows) { + byWorkflowId.set( + workflow.id, + workflow.columns + .filter((column) => column.flags?.hiddenFromBoard !== true) + .map((column) => ({ + id: column.id, + label: column.name, + flags: column.flags, + ...(column.moveTargets ? { moveTargets: column.moveTargets } : {}), + })), + ); + } + for (const task of tasks) { + /* + FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (PR #2528 review — greptile): + VALIDATE the mapped id before trusting it. `taskWorkflowIds` can carry a STALE or + unknown entry — a workflow deleted since the payload was built, or an id the + client has not seen — and a bare `?? defaultWorkflowId` only covers the MISSING + case, not the invalid one. An unknown id then resolves to no columns, the task + silently drops back to the adjacency-free shared union, and the menu is wrong in + exactly the way this whole change exists to prevent. + + Mirrors Board's `getEffectiveTaskWorkflowId`, which already validates against the + known-workflow set for the same reason. + */ + const assigned = boardWorkflows.taskWorkflowIds[task.id]; + /* + FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (PR #2525 review — greptile): + An UNMAPPED task is unknown, not default. `buildBoardWorkflowsPayload` writes an + entry for every task it is given (null selection included), so a MISSING entry + does not mean "no selection" — it means this task is NEWER than the payload, + which happens routinely because the SSE task list updates before board-workflows + does. Assuming the default workflow there would assert the default's adjacency on + a card that may belong to another workflow entirely — precisely the wrong answer, + confidently stated, for the cards most likely to be affected (freshly created + ones, which is exactly when a workflow was chosen). + + Leave such a task without per-workflow metadata: it falls back to the shared + union and the neighbour approximation, which is the pre-existing behaviour and an + admitted guess rather than a false claim. Board additionally forces one + board-workflows refetch when it sees unmapped rendered tasks (FN-7591); porting + that self-heal to List is a real improvement and its own change. + + A PRESENT but unknown id (stale/deleted workflow) still falls back to the default + — there the entry is a real answer that has simply gone out of date. + */ + if (assigned === undefined) continue; + const workflowId = byWorkflowId.has(assigned) ? assigned : boardWorkflows.defaultWorkflowId; + const columns = workflowId ? byWorkflowId.get(workflowId) : undefined; + if (columns) map.set(task.id, columns); + } + return map; + }, [boardWorkflows, tasks, workflowMode]); + const getTaskPlanningWorkflowId = useCallback((task: Task): string | null => { const taskWorkflowId = (task as Task & { workflowId?: string | null }).workflowId; if (taskWorkflowId) return taskWorkflowId; @@ -1888,7 +1965,7 @@ export function ListView({ t, columnLabel: getListColumnLabel, currentColumnFlags: columnFlagsById.get(task.column), - workflowMoveColumns: listContextMenuColumns, + workflowMoveColumns: taskContextMenuColumnsByTaskId.get(task.id) ?? listContextMenuColumns, canRetryTask, hasDuplicateHandler: Boolean(onDuplicateTask), hasRetryHandler: Boolean(onRetryTask), @@ -2015,7 +2092,7 @@ export function ListView({ actions.push({ id: model.reviewAction.id, label: model.reviewAction.label, disabled: model.reviewAction.disabled, onSelect: model.reviewAction.onSelect }); } return actions.filter((action) => action.tone === "note" || action.disabled === true || Boolean(action.onSelect)); - }, [addToast, autoMerge, columnFlagsById, confirm, getListColumnLabel, getTaskPlanningWorkflowId, handleListContextCheckPrStatus, handleListContextEnableGithubTracking, handleListContextMove, handleListTaskArchive, handleListTaskDelete, handleListTaskRevert, isMobile, lastFetchTimeMs, listContextMenuColumns, mergeStrategy, onDuplicateTask, onMergeTask, onOpenDetail, onPlanningMode, onPauseTask, onResetTask, onRetryTask, onUnpauseTask, onArchiveTask, onRevertTask, onTasksUpdated, projectId, t, useSinglePaneList]); + }, [addToast, autoMerge, columnFlagsById, confirm, getListColumnLabel, getTaskPlanningWorkflowId, handleListContextCheckPrStatus, handleListContextEnableGithubTracking, handleListContextMove, handleListTaskArchive, handleListTaskDelete, handleListTaskRevert, isMobile, lastFetchTimeMs, listContextMenuColumns, taskContextMenuColumnsByTaskId, mergeStrategy, onDuplicateTask, onMergeTask, onOpenDetail, onPlanningMode, onPauseTask, onResetTask, onRetryTask, onUnpauseTask, onArchiveTask, onRevertTask, onTasksUpdated, projectId, t, useSinglePaneList]); const contextMenuActions = useMemo( () => (contextMenuState ? buildListContextMenuActions(contextMenuState.task) : []), diff --git a/packages/dashboard/app/components/TaskContextMenu.tsx b/packages/dashboard/app/components/TaskContextMenu.tsx index 43ff8ba350..1faa246457 100644 --- a/packages/dashboard/app/components/TaskContextMenu.tsx +++ b/packages/dashboard/app/components/TaskContextMenu.tsx @@ -3,7 +3,11 @@ import type { KeyboardEvent, PointerEvent as ReactPointerEvent, MouseEvent as Re import { Fragment, useCallback, useEffect, useRef } from "react"; import type { TFunction } from "i18next"; import type { ColumnId, Task, TaskDetail, WorkflowStepResult } from "@fusion/core"; -import { COLUMNS, VALID_TRANSITIONS, isColumn } from "@fusion/core"; +import { VALID_TRANSITIONS, isColumn } from "@fusion/core"; +// `COLUMNS` is gone from this file: deleting the default-column-set shortcut removed +// the last use. `VALID_TRANSITIONS` survives ONLY for the no-metadata load window (see +// the note at `moveTransitions`); every workflow-resolved path now reads the payload's +// own `moveTargets` adjacency. /* FNXC:ReviewLaneBypass 2026-07-09-00:00: @@ -55,6 +59,9 @@ export interface TaskContextMenuColumnMetadata { id: ColumnId; label: string; flags?: TaskContextMenuColumnFlags; + /** Columns this one may move to, from the workflow's own graph adjacency. Optional: + * a payload predating the field falls back to the neighbour approximation. */ + moveTargets?: readonly string[]; } export interface TaskReviewActionDescriptor { @@ -143,11 +150,6 @@ export function isPreExecutionHoldColumn(column: string, flags?: TaskContextMenu return column === "triage" || flags?.intake === true || flags?.hold === true; } -function isDefaultWorkflowColumnSet(columns: readonly TaskContextMenuColumnMetadata[]): boolean { - if (columns.length !== COLUMNS.length) return false; - const ids = new Set(columns.map((column) => column.id)); - return COLUMNS.every((column) => ids.has(column)); -} /* FNXC:TaskContextMenu 2026-06-30-12:42: @@ -159,33 +161,30 @@ Manual pull-request review has two separate operator intents: Start PR Review op function getWorkflowMoveTargets(task: Task | TaskDetail, columns: readonly TaskContextMenuColumnMetadata[]): ColumnId[] { const visibleColumns = columns.filter((column) => column.flags?.hiddenFromBoard !== true); /* - FNXC:TaskContextMenu 2026-07-29-00:00 (U12 — R8, KNOWN REMAINING GAP): - This `VALID_TRANSITIONS` shortcut is the LAST legacy-vocabulary read in this file and - it is deliberately KEPT, because removing it today would silently SHRINK the move - menu for every default-workflow project rather than fix anything. + FNXC:TaskContextMenu 2026-07-29-00:00 (U12 — R8): + REAL ADJACENCY, when the payload carries it. `moveTargets` comes from + `resolveAllowedColumns` — the same resolver `moveTaskInternal` validates against — so + the menu offers exactly what the store will accept, for ANY workflow. - The reason is a missing wire field, not a missing idea. `TaskContextMenuColumnMetadata` - carries id/label/flags but NO adjacency, so the workflow branch below can only guess - targets from a column's neighbours in the declared order — [previous, next]. Measured - against the real graph that is a strict loss: + This replaces the `VALID_TRANSITIONS` shortcut that used to run whenever a workflow's + column-id set matched the six built-ins. That shortcut existed because the fallback + below approximates targets from a column's NEIGHBOURS in declared order, which is + strictly weaker than the graph (in-progress: 4 real targets vs 2 neighbours), so + deleting it without adjacency would have SHRUNK every default-workflow move menu. - in-progress VALID_TRANSITIONS: in-review, todo, triage, done (4) - neighbour-derived: todo, in-review (2) - todo VALID_TRANSITIONS: in-progress, triage, archived (3) - neighbour-derived: triage, in-progress (2) - done VALID_TRANSITIONS: todo, triage, archived (3) - neighbour-derived: in-review, archived (2) + With adjacency on the wire the shortcut is not merely removable, it is redundant: + `resolveAllowedColumns(BUILTIN_CODING_WORKFLOW_IR, c)` is byte-identical to + `VALID_TRANSITIONS[c]` for all six columns, ORDER included — measured, and pinned by + `@fusion/core`'s `builtin-adjacency-matches-legacy-transitions` test so the + equivalence cannot drift silently. Custom workflows stop being guessed at. - So "delete the legacy read" here is not a cleanup — it is a UI regression that drops - real operator moves (archive from Todo, straight-to-Done from In progress). The - correct fix is to put each column's allowed targets on the board-workflows payload - and read THOSE, which changes the server, the wire shape and this file, and is its own - slice. Note the guard is keyed on the COLUMN ID SET, so a workflow that merely renames - the six built-in columns still takes this path and still gets correct targets; only a - workflow that reorders or replaces them falls through to the weaker neighbour logic. + Targets are filtered to columns this board can show, so an adjacency edge into a + hidden column never becomes a dead menu entry. */ - if (isDefaultWorkflowColumnSet(visibleColumns) && isColumn(task.column)) { - return task.column === "in-review" ? ["todo", "in-progress"] : [...VALID_TRANSITIONS[task.column]]; + const declaredTargets = columns.find((column) => column.id === task.column)?.moveTargets; + if (declaredTargets) { + const visibleIds = new Set(visibleColumns.map((column) => column.id)); + return declaredTargets.filter((target) => visibleIds.has(target)) as ColumnId[]; } const currentIndex = visibleColumns.findIndex((column) => column.id === task.column); diff --git a/packages/dashboard/app/components/TaskDetailModal.tsx b/packages/dashboard/app/components/TaskDetailModal.tsx index 30f4a12579..7b52314926 100644 --- a/packages/dashboard/app/components/TaskDetailModal.tsx +++ b/packages/dashboard/app/components/TaskDetailModal.tsx @@ -528,7 +528,7 @@ function resolveTaskWorkflowMetadata(payload: BoardWorkflowsPayload, task: Pick< const moveColumns = workflow.columns .filter((column) => column.flags.hiddenFromBoard !== true) - .map((column) => ({ id: column.id as ColumnId, label: column.name, flags: column.flags })); + .map((column) => ({ id: column.id as ColumnId, label: column.name, flags: column.flags, ...(column.moveTargets ? { moveTargets: column.moveTargets } : {}) })); const currentColumnFlags = moveColumns.find((column) => column.id === task.column)?.flags; return { id: workflow.id, name, icon: workflow.icon, fields: workflow.fields ?? null, moveColumns, currentColumnFlags }; } diff --git a/packages/dashboard/app/components/__tests__/workflow-resolved-columns.test.tsx b/packages/dashboard/app/components/__tests__/workflow-resolved-columns.test.tsx index a16561adbb..858bba0cad 100644 --- a/packages/dashboard/app/components/__tests__/workflow-resolved-columns.test.tsx +++ b/packages/dashboard/app/components/__tests__/workflow-resolved-columns.test.tsx @@ -504,6 +504,60 @@ describe("U10 — surfaces render workflow-resolved columns", () => { expect(building?.label).toBe("Back to Building"); }); + /* + FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8): + When the payload carries `moveTargets`, the menu uses the workflow's REAL graph + adjacency instead of approximating from neighbouring columns. This fixture makes + the two disagree on purpose: `staging` sits between `backlog` and `building`, so + the neighbour approximation yields exactly those two — while the declared + adjacency also allows a jump to `shipped` and forbids going back to `backlog`. + + REVERT CHECK: drop the `declaredTargets` branch and this fails — the neighbour + fallback returns ["backlog","building"], missing the legal `shipped` jump and + offering `backlog`, which the workflow's graph does not allow. That is precisely + the class of defect the old neighbour approximation shipped for every custom + workflow: menu entries the store would reject, and legal moves it never offered. + */ + it("uses the workflow's declared adjacency, not neighbouring columns", () => { + const withAdjacency: TaskContextMenuColumnMetadata[] = renamedMoveColumns.map((column) => + column.id === "staging" + ? { ...column, moveTargets: ["building", "shipped"] } + : column, + ); + const transitions = getTaskMoveTransitions( + mkTask({ id: "FN-16", column: "staging" as Task["column"] }), + t, + columnLabel, + withAdjacency, + ); + expect(transitions.map((transition) => transition.column)).toEqual(["building", "shipped"]); + }); + + it("drops an adjacency edge into a column this board cannot show", () => { + /* + Two distinct exclusions, because they fail differently (PR #2525 review — + CodeRabbit): `hidden` is a DECLARED column carrying `hiddenFromBoard`, which only + the visibility filter removes, and `nowhere` is an id the workflow does not + declare at all. Testing only the unknown id would let the hidden-column filtering + be deleted with the case still green. + */ + const withHidden: TaskContextMenuColumnMetadata[] = [ + ...renamedMoveColumns.map((column) => + column.id === "staging" + ? { ...column, moveTargets: ["building", "hidden", "nowhere"] } + : column, + ), + { id: "hidden" as ColumnId, label: "Hidden", flags: { hiddenFromBoard: true } }, + ]; + const transitions = getTaskMoveTransitions( + mkTask({ id: "FN-17", column: "staging" as Task["column"] }), + t, + columnLabel, + withHidden, + ); + expect(transitions.map((transition) => transition.column)).toEqual(["building"]); + }); + it("never offers a column the workflow does not declare", () => { const transitions = getTaskMoveTransitions( mkTask({ id: "FN-13", column: "in-review" }), diff --git a/packages/dashboard/src/routes/board-workflows.ts b/packages/dashboard/src/routes/board-workflows.ts index e6a503b962..37b8ec8ba5 100644 --- a/packages/dashboard/src/routes/board-workflows.ts +++ b/packages/dashboard/src/routes/board-workflows.ts @@ -23,6 +23,7 @@ import { getBuiltinWorkflow, isBuiltinWorkflowId, parseWorkflowIr, + resolveAllowedColumns, resolveColumnFlags, resolveWorkflowIrById, type Settings, @@ -48,6 +49,22 @@ export interface BoardWorkflowColumn { /** Optional author-defined explanatory copy; omitted keeps client lifecycle fallback. */ description?: string; flags: TraitFlags; + /* + FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8): + The columns this one may move to, resolved from the workflow's OWN graph adjacency + (`resolveAllowedColumns`) — the same function `moveTaskInternal` validates against, + so the menu offers exactly what the store will accept. + + This field exists to retire the client's two remaining legacy-vocabulary reads. The + context menu previously had no adjacency at all, so it approximated targets by a + column's NEIGHBOURS in declared order and kept a `VALID_TRANSITIONS` shortcut for + workflows whose column-id set matched the six built-ins — because the neighbour + approximation is strictly weaker (in-progress: 4 real targets vs 2 neighbours). + + Optional on the wire so a client older than this field keeps its previous behaviour + rather than losing its move menu. + */ + moveTargets?: string[]; } /** A workflow definition in use by visible cards. */ @@ -124,6 +141,7 @@ function describeColumns(ir: WorkflowIr, canonicalizeLifecycle = false): BoardWo name: displayColumnName(col.id, col.name, canonicalizeLifecycle), ...(col.description ? { description: col.description } : {}), flags: resolveColumnFlags(col), + moveTargets: resolveAllowedColumns(ir, col.id), })); }