feat: workflow-defined custom columns via composable traits - #1418
Conversation
- WorkflowNodeEditor overlay was missing the `open` class, so the graph editor mounted with display:none — clicking the button just dismissed the steps view. Add `open` so the overlay renders. - Add fusion-plugin-compound-engineering and fusion-plugin-roadmap to BUILTIN_PLUGINS so they show under Settings → Built-in Plugins. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…persede interpreter-cutover plan
… nodes, v1 upgrade (U1)
…, typed rejections, default-workflow hook parity (U4)
…anch state, schema v107 (U13)
… in an undefined column (U5)
…igurable strategy/fileScope, lost-work guards non-configurable (U7)
…ed columns (KTD-1)
…sweep with reservation-first ordering (U6)
…ucket — no card silently dropped (U11)
…ated gates, live-dependent disable guard (U8)
…join nodes with inline validation (U10)
…it-keyed columns, typed drag rejections, hold promote (U9)
…-side trait validation, plugin post-commit hooks, docs + changeset (U12)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds v2 Workflow IR, trait registry and builtin traits, typed transition contract and transitionPending marker with DB migrations (106/107), TaskStore flag‑ON move path (guards/gates/capacity/markers/post‑commit hooks), hold/release sweep and split/join execution, merge/plugin trait plumbing, dashboard multi‑lane board and v2 editor, CLI TUI graceful degradation, i18n updates, and extensive tests. ChangesWorkflow Columns, Traits, and Surfaces
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related issues
Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
packages/engine/src/scheduler.ts (1)
1215-1270:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRefresh the task snapshot after the hold/release sweep.
taskswas loaded earlier at Line 1166, but Lines 1215-1270 still deriveactiveWorktrees,available,perColumnGates, and the todo queue from that pre-sweep snapshot. WhenrunHoldReleaseSweepPass()releases a held card intoin-progress, this pass can still think capacity is available and attempt an extra dispatch, while also emitting stale concurrency diagnostics for that tick. Re-list tasks after the sweep, or move the initial snapshot below it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/scheduler.ts` around lines 1215 - 1270, The current pass uses the earlier "tasks" snapshot (used to compute activeWorktrees, agentSlots, available, perColumnGates and to call computeConcurrencyGateDiagnostic) even after runHoldReleaseSweepPass() may have released cards into "in-progress"; refresh the task list after the sweep (or move the initial snapshot to after runHoldReleaseSweepPass()) so that activeWorktrees, inProgress, semaphore-available calculations, perColumnGates (isWorkflowColumnsEnabled/DEFAULT_WORKFLOW_POOL_ID) and the concurrency diagnostics reflect the post-sweep state and avoid double-dispatching or stale diagnostics.packages/dashboard/src/routes/register-task-workflow-routes.ts (1)
1359-1384:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftStop hard-coding legacy columns in manual moves.
POST /tasks/:id/movestill rejects anything outsideCOLUMNSand only provisions a worktree for literal"in-progress". With workflow-defined columns enabled, board drags to custom processing columns will fail here beforemoveTask()can apply the new transition logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/src/routes/register-task-workflow-routes.ts` around lines 1359 - 1384, The code currently hard-codes the literal "in-progress" when deciding to allocate a worktree which breaks workflow-defined columns; instead, determine whether the requested target column should be treated as the processing/in-progress column from the workflow/settings and only then allocate. Replace the check (column as Column) === "in-progress" with logic that loads the workflow/settings (use scopedStore.getSettings() and the workflow metadata or scopedStore API that exposes workflow columns) or call an existing helper on moveTask that identifies the processing/in-progress column for the active workflow; keep the same allocateWorktree assignment and call to planTaskWorktreePath(existing, rootDir, settings.worktreeNaming, reservedNames, settings) when that computed check is true.packages/engine/src/workflow-graph-task-runner.ts (1)
87-117:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClear branch progress before any early fallback return.
branchProgress.clear()currently runs only after workflow resolution. Early fallback paths can leak stale progress intogetBranchProgress().Suggested fix
public async run( task: TaskDetail, settings: Pick<Settings, "experimentalFeatures"> | undefined, ): Promise<WorkflowGraphTaskRunResult> { + this.branchProgress.clear(); if (!isExperimentalFeatureEnabled(settings, "workflowGraphExecutor")) { return this.fallBack(task.id, "flag-off"); } ... - this.branchProgress.clear();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/workflow-graph-task-runner.ts` around lines 87 - 117, Move the branchProgress.clear() call to the very start of run() so it always runs before any early fallback; specifically, clear this.branchProgress at the top of WorkflowGraphTaskRunner.run (before the experimental feature check and before retrieving selection) to ensure stale state cannot be returned by getBranchProgress() when any of the fallBack(...) paths (e.g., "flag-off", "selection-error", "no-selection", "workflow-load-error", "workflow-missing") are taken.packages/engine/src/merger.ts (1)
4934-4966:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
scopeOverrideeffective for workflow custom file-scope rules.
hasCustomRulesnow suppresses the existingtask.scopeOverridebypass, so a task with an explicit file-scope override will still fail enforcement whenever a workflow suppliesfileScopeRules. That removes the documented per-task escape hatch instead of just swapping the declared scope source.Suggested fix
- if (!hasCustomRules && task.scopeOverride === true) { + if (task.scopeOverride === true) { const reasonSuffix = task.scopeOverrideReason?.trim() ? ` — reason: ${task.scopeOverrideReason.trim()}` : "";Based on learnings: Per-task opt-out exists:
task.scopeOverride = true(log the reason) for File Scope invariant violations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/merger.ts` around lines 4934 - 4966, The current logic ignores per-task scopeOverride when customScopeRules are present because the early return only runs if !hasCustomRules; change the control flow so a task-level opt-out (task.scopeOverride === true) always short-circuits regardless of customScopeRules: check task.scopeOverride first, log the same message via store.appendAgentLog (using taskId and task.scopeOverrideReason) and return immediately; then proceed to determine declaredScope using customScopeRules or store.parseFileScopeFromPrompt as before. Ensure you still preserve the existing guard that parseFileScopeFromPrompt may be missing.packages/dashboard/app/components/Column.tsx (2)
382-385:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftWorkflow-mode bulk moves still target legacy column ids.
These actions are now exposed on trait-based custom columns, but
handleReplanAll()still sends every card to"triage"andhandleMoveAllToTodo()still sends every card to"todo". Any workflow that does not literally define those ids will reject or misroute the bulk action. The destination needs to come from the resolved workflow column metadata, not hard-coded legacy ids.Also applies to: 406-409, 480-482
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/Column.tsx` around lines 382 - 385, handleReplanAll and handleMoveAllToTodo currently hard-code legacy column ids ("triage" and "todo") when calling onMoveTask, which breaks trait-based custom workflows; change these bulk-move callers to derive the destination ColumnType/id from the resolved workflow column metadata (the same source used to render custom columns) instead of using string literals. Locate uses of onMoveTask(..., "triage" as ColumnType) and onMoveTask(..., "todo" as ColumnType) (also at the other noted blocks) and replace the hard-coded second arg with the appropriate resolved column identifier (e.g., lookup the workflow's triage/todo equivalent from the resolved columns map or column metadata used by the Column component) so each task is moved to the actual workflow-specific destination. Ensure types still match ColumnType and handle missing mappings by skipping or reporting an error.
418-420:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCustom workflow columns can throw before the confirm dialog opens.
COLUMN_LABELS[column]is undefined for workflow-defined ids, so.toLowerCase()will crash when a user opens Stop All / Move All to Todo on a custom processing or review column. Reuse the same resolved label fallback you already use in the header.Suggested fix
+ const resolvedColumnLabel = workflowMode + ? (columnDisplayName ?? COLUMN_LABELS[column] ?? column) + : COLUMN_LABELS[column]; + const handlePauseAll = useCallback(async () => { @@ title: t("column.stopAllTitle", "Stop All Tasks"), - message: t("column.stopAllMessage", "Stop all {{count}} {{columnLabel}} task{{plural}}?", { count: pauseEligibleCount, columnLabel: COLUMN_LABELS[column].toLowerCase(), plural: pauseEligibleCount === 1 ? "" : "s" }), + message: t("column.stopAllMessage", "Stop all {{count}} {{columnLabel}} task{{plural}}?", { count: pauseEligibleCount, columnLabel: resolvedColumnLabel.toLowerCase(), plural: pauseEligibleCount === 1 ? "" : "s" }), @@ title: t("column.moveAllToTodoTitle", "Move All to Todo"), - message: t("column.moveAllToTodoMessage", "Move all {{count}} {{columnLabel}} task{{plural}} to Todo?", { count: tasks.length, columnLabel: COLUMN_LABELS[column].toLowerCase(), plural: tasks.length === 1 ? "" : "s" }), + message: t("column.moveAllToTodoMessage", "Move all {{count}} {{columnLabel}} task{{plural}} to Todo?", { count: tasks.length, columnLabel: resolvedColumnLabel.toLowerCase(), plural: tasks.length === 1 ? "" : "s" }),Also applies to: 447-448, 524-524, 572-572
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/Column.tsx` around lines 418 - 420, COLUMN_LABELS[column].toLowerCase() can throw for custom workflow column ids; replace direct access with a safe resolved label and use it in the confirm() calls (the same fallback the header uses). Add a local safeLabel variable (e.g. const safeLabel = (COLUMN_LABELS[column] ?? /* header's resolved label fallback */ ?? String(column)).toLowerCase()) and use safeLabel instead of COLUMN_LABELS[column].toLowerCase() in the confirm invocations (the confirm call shown and the other occurrences mentioned at the same spots). Ensure you update all four locations (the Stop All / Move All prompts) to use this safe resolved label.packages/cli/src/commands/dashboard-tui/app.tsx (1)
143-143:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFive columns no longer fit the 80-column breakpoint.
Adding
OTHER_BUCKETmakes the board render 5 columns, but the wide-mode cutoff is still 80. On 80-99 column terminals, each lane clamps to 20 chars, so the board now needs at least ~102 cols and will overflow off-screen instead of collapsing to the narrow single-column view.Suggested fix
- const isNarrow = cols < NARROW_THRESHOLD; + const minBoardCols = (KANBAN_COLUMNS.length * 20) + 2; + const isNarrow = cols < Math.max(NARROW_THRESHOLD, minBoardCols);Also applies to: 1733-1736
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/commands/dashboard-tui/app.tsx` at line 143, The fixed NARROW_THRESHOLD (80) no longer accounts for the new OTHER_BUCKET which can make the board render five columns; change the logic that decides narrow vs wide mode so it computes required width dynamically instead of using the hardcoded NARROW_THRESHOLD. Replace the constant check with a computed condition (e.g., isNarrow = cols < Math.max(80, buckets.length * MIN_LANE_WIDTH + padding)) where MIN_LANE_WIDTH is 20 and padding is the extra chars between lanes; update any uses of NARROW_THRESHOLD and the rendering code that references OTHER_BUCKET/board column count to use this computed isNarrow value so 5-column boards only switch to narrow mode when terminal width truly cannot fit them.
🟡 Minor comments (11)
packages/core/src/__tests__/transition-parity.test.ts-243-265 (1)
243-265:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThis case cannot detect
transitionPendingdouble-counting.With
maxConcurrent = 1, both the correct implementation and one that countst1twice still rejectt2, becauset1.column === "in-progress"already occupies the only slot. If this is meant to prove the pending-marker path is counted exactly once, usemaxConcurrent = 2and assert a second move succeeds before a third is rejected.As per coding guidelines
Regression tests must assert the general invariant across ALL known surfaces, not only the single reported reproduction (FN-5893).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/__tests__/transition-parity.test.ts` around lines 243 - 265, Update the test "U6 capacity counts cards mid-transitionPending (they hold their slot from commit time)" to verify the marker path is counted exactly once by using maxConcurrent = 2 and asserting two moves succeed then the third is rejected: set store.updateSettings({ maxConcurrent: 2 }), seed three tasks (t1, t2, t3), move t1 into "in-progress" and inject the transitionPending marker into the DB for t1 (same db.prepare(...) call already present), then attempt to move t2 into "in-progress" and assert it succeeds, and finally attempt to move t3 and assert it throws a TransitionRejectionError with rejection.code === "capacity-exhausted"; keep the same use of seedInColumn, store.moveTask, and TransitionRejectionError for locating the relevant code paths.packages/core/src/__tests__/goals-schema.test.ts-93-94 (1)
93-94:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the test title to match the asserted schema version.
The test name still says
101while the assertion expects106, which makes failures harder to interpret.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/__tests__/goals-schema.test.ts` around lines 93 - 94, Update the test name string so it accurately describes the asserted schema version: change the it(...) description that currently says "reports schema version 101" to reflect version 106; locate the test containing expect(db.getSchemaVersion()).toBe(106) and edit the it(...) title accordingly (e.g., "reports schema version 106").packages/i18n/locales/es/errors.json-1-1 (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRestore removed Spanish error keys instead of shipping an empty locale map.
Line 1 drops all
eserror translations, which degrades i18n behavior for existing error paths.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/i18n/locales/es/errors.json` at line 1, The Spanish errors locale file errors.json was replaced with an empty object, removing all `es` error translation keys; restore the removed Spanish error keys by reverting errors.json to include the original translation map (or copy the keys from the canonical source, e.g., the `en` errors file) and provide Spanish translations for each key so the keys in errors.json match the same identifiers used elsewhere (ensure you update the same keys present in other locale files and keep the file as a non-empty JSON object).packages/i18n/locales/en/common.json-50-50 (1)
50-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix singular pluralization for
toolCallsCount_one.
toolCallsCount_onecurrently resolves to a plural phrase (“1 tool calls”).💬 Proposed fix
- "toolCallsCount_one": "{{count}} tool calls", + "toolCallsCount_one": "{{count}} tool call",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/i18n/locales/en/common.json` at line 50, The singular pluralization string for the i18n key toolCallsCount_one is wrong (it reads as a plural). Update the value of toolCallsCount_one in the locales file so it uses the singular form (e.g., " {{count}} tool call" or an equivalent singular phrase) while keeping toolCallsCount_other unchanged; locate the key toolCallsCount_one in the JSON and replace its string with the singular wording.packages/dashboard/app/components/__tests__/Lane.test.tsx-129-140 (1)
129-140:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert drag-over cancellation state in the rejection path.
This case says drop is prevented, but it never verifies the
dragOverevent was not canceled. Add an explicit assertion so the test cannot pass ifpreventDefault()starts being called.Suggested fix
- const preventDefault = vi.fn(); - fireEvent.dragOver(ipColumn, { dataTransfer: { dropEffect: "" }, preventDefault }); + const notPrevented = fireEvent.dragOver(ipColumn, { dataTransfer: { dropEffect: "" } }); + expect(notPrevented).toBe(true); // Rejection → preventDefault NOT called → the browser refuses the drop. expect(props.canDropTask).toHaveBeenCalledWith("FN-DRAG", "in-progress", "builtin:coding");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/__tests__/Lane.test.tsx` around lines 129 - 140, The test for the rejection path must assert the dragOver event's preventDefault was NOT called; update the "prevents the drop (no-move) when canDropTask returns a rejection key" test to include an assertion that the local preventDefault mock (created before fireEvent.dragOver) was not invoked after calling fireEvent.dragOver on the element, while keeping the existing checks for props.canDropTask and the inline feedback; reference the preventDefault mock, props.canDropTask, and the fireEvent.dragOver call to locate where to add expect(preventDefault).not.toHaveBeenCalled().packages/dashboard/app/components/TaskCard.tsx-496-497 (1)
496-497:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMemo comparator can miss branch-progress badge updates.
Line 496 only tracks
branchProgresslength. If completed vs non-completed statuses change without a length change, the badge text can remain stale.Suggested fix
- ((previousTask as TaskWithBranchProgress).branchProgress?.length ?? 0) === - ((nextTask as TaskWithBranchProgress).branchProgress?.length ?? 0) && + ((previousTask as TaskWithBranchProgress).branchProgress?.length ?? 0) === + ((nextTask as TaskWithBranchProgress).branchProgress?.length ?? 0) && + ((previousTask as TaskWithBranchProgress).branchProgress?.filter((b) => b.status === "completed").length ?? 0) === + ((nextTask as TaskWithBranchProgress).branchProgress?.filter((b) => b.status === "completed").length ?? 0) &&🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/TaskCard.tsx` around lines 496 - 497, The memo comparator currently only compares ((previousTask as TaskWithBranchProgress).branchProgress?.length ?? 0) to ((nextTask as TaskWithBranchProgress).branchProgress?.length ?? 0), which misses updates where branchProgress items change status but keep the same length; update the comparator used in TaskCard.tsx (the block that receives previousTask and nextTask) to perform a deeper comparison of branchProgress — e.g. compare the arrays' contents (statuses/ids) or use a stable serialization like JSON.stringify on (previousTask as TaskWithBranchProgress).branchProgress and (nextTask as TaskWithBranchProgress).branchProgress — so badge text updates when items change even if length does not.packages/cli/src/commands/dashboard-tui/app.tsx-1201-1203 (1)
1201-1203:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winThe “Other” bucket still windows cards as if they were 4 rows tall.
Those cards now always render
secondaryLabel, so their minimum height is 5 rows, not 4.visibleCountstill divides by 4, which can over-render this bucket and clip the bottom card or the↓ N morehint.Suggested fix
- const visibleCount = Math.max(1, Math.floor(cardRowsBudget / 4)); + const estimatedCardRows = isOther ? 5 : 4; + const visibleCount = Math.max(1, Math.floor(cardRowsBudget / estimatedCardRows));Also applies to: 1233-1237, 1275-1275
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/commands/dashboard-tui/app.tsx` around lines 1201 - 1203, The "Other" bucket's visibleCount calculation assumes each card is 4 rows tall but cards now always render secondaryLabel, making them 5 rows tall; update the calculation that uses visibleCount to compute rows for the Other bucket (the variable named visibleCount and any code computing counts for the "Other" bucket) to use a cardHeight variable (e.g., const cardHeight = secondaryLabel ? 5 : 4) and divide/ceil by cardHeight instead of 4, and apply the same change to the other occurrences where visibleCount is derived (the other spots referenced around the secondaryLabel rendering) so the bucket won't over-render or clip the bottom card/hint.packages/dashboard/src/__tests__/workflow-routes.test.ts-117-120 (1)
117-120:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert the specific composition violation, not just “some 400 happened.”
Both Residual A tests stay green on any save-time validation failure. That means a parser error or unrelated workflow validation regression could satisfy these assertions without proving the
complete+wipincompatibility is still rejected on both create and update surfaces. Please assert a stable violation field here as well (for example the violation code, message key, or offending column/trait pair).Based on learnings "Regression tests must assert the general invariant across ALL known surfaces, not only the single reported reproduction (FN-5893)".
Also applies to: 158-158
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/src/__tests__/workflow-routes.test.ts` around lines 117 - 120, The test currently only asserts a 400 and that some violations exist; instead assert the specific "complete + wip" incompatibility violation so unrelated validation failures won't mask regressions. In the workflow-routes.test.ts assertions that inspect details and details.violations (the block around the current expect(res.status).toBe(400) and the similar block at the other occurrence) replace the loose length check with an assertion that the violations array contains an object matching the known invariant (e.g. a violation entry with a stable identifier like a violation code or message key and/or the offending trait pair: "complete" and "wip"); use expect(...).toContainEqual(expect.objectContaining({ code: '<INCOMPATIBLE_TRAITS_CODE>' }) ) or match the messageKey or fields indicating traitA: 'complete', traitB: 'wip' for both the create and update test locations mentioned.packages/dashboard/app/components/Board.tsx-12-13 (1)
12-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winScope lane collapse persistence by
projectId.The persisted collapse state is global even though the comment says “per project”. As written, collapsing a workflow in one project will silently collapse the same workflow id after switching to another project in the same browser session.
Proposed fix
-const LANE_COLLAPSE_STORAGE_KEY = "kb-dashboard-lane-collapsed"; +const getLaneCollapseStorageKey = (projectId?: string) => + `kb-dashboard-lane-collapsed:${projectId ?? "global"}`; ... - const [collapsedLanes, setCollapsedLanes] = useState<ReadonlySet<string>>(() => { + const [collapsedLanes, setCollapsedLanes] = useState<ReadonlySet<string>>(() => { if (typeof window === "undefined") return new Set(); try { - const raw = window.localStorage.getItem(LANE_COLLAPSE_STORAGE_KEY); + const raw = window.localStorage.getItem(getLaneCollapseStorageKey(projectId)); const parsed = raw ? (JSON.parse(raw) as unknown) : null; if (Array.isArray(parsed)) return new Set(parsed.filter((x): x is string => typeof x === "string")); } catch { /* ignore corrupt persisted state */ } return new Set(); }); ... - window.localStorage.setItem(LANE_COLLAPSE_STORAGE_KEY, JSON.stringify([...next])); + window.localStorage.setItem(getLaneCollapseStorageKey(projectId), JSON.stringify([...next]));Also applies to: 273-283, 319-332
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/Board.tsx` around lines 12 - 13, The LANE_COLLAPSE_STORAGE_KEY is currently global causing collapse state to leak across projects; update the persistence to scope by projectId by composing the storage key with the current projectId (e.g., `${LANE_COLLAPSE_STORAGE_KEY}:${projectId}`) wherever the key is used (the constant LANE_COLLAPSE_STORAGE_KEY and the functions/hooks that read/write collapse state around lines ~273-283 and ~319-332). Change reads, writes and initializations to use the project-scoped key and ensure any deserialization/merge logic still works per-project so collapsing a lane only affects that project.packages/core/src/plugin-types.ts-848-853 (1)
848-853:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject restricted flag key presence, not only truthy values.
This check currently allows restricted keys when set to falsy values (e.g.
complete: false), which still violates the “may not be declared” contract.Suggested fix
- for (const restricted of PLUGIN_TRAIT_RESTRICTED_FLAGS) { - if (flags[restricted]) { + for (const restricted of PLUGIN_TRAIT_RESTRICTED_FLAGS) { + if (restricted in flags) { errors.push( `${prefix}.flags.${restricted} is a restricted (built-in-only) flag and may not be declared by a plugin trait`, ); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/plugin-types.ts` around lines 848 - 853, The loop over PLUGIN_TRAIT_RESTRICTED_FLAGS should reject the presence of a restricted key in the flags object regardless of its truthiness; change the condition from checking flags[restricted] to testing ownership (e.g. Object.prototype.hasOwnProperty.call(flags, restricted) or `restricted in flags`) so any declared key (even falsy values like false or null) pushes the same error into errors; update the check around the existing errors.push call that references prefix, flags and restricted accordingly.docs/plans/2026-06-03-003-feat-workflow-custom-columns-traits-plan.md-177-177 (1)
177-177:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language to the fenced code block to satisfy markdownlint (MD040).
Suggested fix
-``` +```text [Planning col] [Todo col] [In-Progress col] [Review col] [Done col] ... -``` +```🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/plans/2026-06-03-003-feat-workflow-custom-columns-traits-plan.md` at line 177, The fenced code block containing the ASCII board (the block starting with the triple backticks before the "[Planning col] [Todo col] ..." line) is missing a language tag and triggers markdownlint MD040; fix it by adding a language identifier (e.g., "text") immediately after the opening ``` so the fence reads ```text, leaving the block contents unchanged.
🧹 Nitpick comments (6)
packages/core/src/__tests__/move-task-characterization.test.ts (1)
24-24: ⚡ Quick winDerive
ALL_COLUMNSfromVALID_TRANSITIONSto keep parity coverage complete.Line 24 hardcodes the transition surface, so newly added transition keys can bypass this characterization suite until the list is manually updated.
♻️ Proposed refactor
-const ALL_COLUMNS: Column[] = ["triage", "todo", "in-progress", "in-review", "done", "archived"]; +const ALL_COLUMNS = Object.keys(VALID_TRANSITIONS) as Column[];As per coding guidelines, "Regression tests must assert the general invariant across ALL known surfaces, not only the single reported reproduction (FN-5893)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/__tests__/move-task-characterization.test.ts` at line 24, Replace the hardcoded ALL_COLUMNS array with a derived list built from the keys of VALID_TRANSITIONS so the test covers every known transition surface; specifically, compute ALL_COLUMNS by extracting and sorting (or preserving deterministic order from) Object.keys(VALID_TRANSITIONS) and casting to Column[] (or mapping to Column) instead of the literal array, ensuring the test uses that derived symbol rather than the manual list.packages/core/src/transition-types.ts (1)
28-42: ⚡ Quick winMake the rejection-code list the single source of truth.
TransitionRejectionCodeandTRANSITION_REJECTION_CODESare duplicated by hand. If a future code is added to the union but not the array, it will still type-check, butisTransitionRejectionCode()will reject it at runtime and deserialization will start returningnullfor a persisted valid code.♻️ Proposed refactor
-export type TransitionRejectionCode = - | "guard-rejected" - | "capacity-exhausted" - | "unknown-column" - | "workflow-mismatch" - | "merge-blocked"; - /** The full, immutable set of rejection codes (handy for exhaustive validation). */ -export const TRANSITION_REJECTION_CODES: readonly TransitionRejectionCode[] = [ +export const TRANSITION_REJECTION_CODES = [ "guard-rejected", "capacity-exhausted", "unknown-column", "workflow-mismatch", "merge-blocked", ] as const; + +export type TransitionRejectionCode = (typeof TRANSITION_REJECTION_CODES)[number];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/transition-types.ts` around lines 28 - 42, TransitionRejectionCode and TRANSITION_REJECTION_CODES are duplicated; make the array the single source of truth by keeping TRANSITION_REJECTION_CODES as the readonly tuple of literals and deriving the union type from it (e.g., set TransitionRejectionCode = typeof TRANSITION_REJECTION_CODES[number]) so new codes only need to be added to TRANSITION_REJECTION_CODES; update any runtime checks/deserializers (e.g., isTransitionRejectionCode) to rely on TRANSITION_REJECTION_CODES for validation.packages/core/src/__tests__/builtin-workflows.test.ts (1)
19-25: ⚡ Quick winBroaden this regression assertion to all known default-workflow surfaces.
This currently validates only
BUILTIN_CODING_WORKFLOW_IR; please also assert the same column-id invariant ongetBuiltinWorkflow("builtin:coding")?.irso registry wiring can’t drift silently.As per coding guidelines: “Regression tests must assert the general invariant across ALL known surfaces, not only the single reported reproduction (FN-5893)”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/__tests__/builtin-workflows.test.ts` around lines 19 - 25, Extend the test so the column-id invariant is asserted on both surfaces: keep the existing check for BUILTIN_CODING_WORKFLOW_IR.columns.map(c=>c.id) against DEFAULT_WORKFLOW_COLUMN_IDS and add the same expectation for getBuiltinWorkflow("builtin:coding")?.ir.columns.map(c=>c.id) (first ensure the call returns a workflow and its ir is defined), so the registry wiring can’t drift silently; reference the existing symbols BUILTIN_CODING_WORKFLOW_IR, getBuiltinWorkflow("builtin:coding")?.ir, and DEFAULT_WORKFLOW_COLUMN_IDS when adding the second assertion.packages/dashboard/app/components/__tests__/TaskCard.test.tsx (1)
496-512: ⚡ Quick winBroaden branch-progress assertions to cover all known status surfaces
Line 496 currently validates one mixed state (
completed+running) and Line 509 validates absence only; please add a small table-driven case set covering the known branch statuses so the completed-count invariant is protected across all supported surfaces.As per coding guidelines, "Regression tests must assert the general invariant across ALL known surfaces, not only the single reported reproduction (FN-5893)".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/app/components/__tests__/TaskCard.test.tsx` around lines 496 - 512, Add table-driven tests for TaskCard's branch-progress badge to assert the completed-count invariant across all known branch statuses: update the existing tests around TaskCard (the "renders a per-branch progress badge..." and the "does not render..." cases) to iterate over a small matrix of branchProgress scenarios (e.g., all completed, none completed, mixed completed/failed/running/skipped) and for each case render <TaskCard task={...} ... /> (use makeTask() to build base Task and override branchProgress) then assert the badge shows the correct "X/Y" completed count or is absent when appropriate; ensure you reference the branchProgress property and testId "branch-progress-badge" in each subcase so the completed-count invariant is covered for all known status surfaces.packages/engine/src/__tests__/workflow-graph-fanout.test.ts (1)
79-102: ⚡ Quick winThese
collectcases don't pin the early-advance invariant.Both tests resolve the lagging branch before awaiting the run, so they still pass if the executor waits for all branches before leaving the split. Please assert the intended behavior on both early-exit surfaces here (
mode: "any"and quorum) by checking that the tail has already run while the laggard is still pending. As per coding guidelines, "Regression tests must assert the general invariant across ALL known surfaces, not only the single reported reproduction (FN-5893)".Also applies to: 125-168
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/engine/src/__tests__/workflow-graph-fanout.test.ts` around lines 79 - 102, Update the test "mode:any with collect — first completion fires join; slower branch finishes without re-firing" (and the similar quorum test around lines 125-168) to assert the early-advance invariant: after starting the run (executor.run(...)) and yielding one microtask (await Promise.resolve()), check that tail has already been invoked and slowFinished is still false before resolving the lagging branch; then resolve slow and await the run to completion and assert final outcome and that tail was called only once. Use the existing symbols slow, tail, slowFinished, run and executor.run(...) to locate and modify the assertions so the test verifies tail ran while the laggard was still pending.packages/dashboard/src/routes/board-workflows.ts (1)
163-166: ⚡ Quick winConsider parallel workflow description for better performance.
The current implementation awaits each
describeWorkflowsequentially. When many workflows are referenced, this could add unnecessary latency.⚡ Proposed parallel execution
- const workflows: BoardWorkflowDefinition[] = []; - for (const workflowId of referenced) { - workflows.push(await describeWorkflow(store, workflowId)); - } + const workflows = await Promise.all( + Array.from(referenced).map((workflowId) => describeWorkflow(store, workflowId)) + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dashboard/src/routes/board-workflows.ts` around lines 163 - 166, The loop builds workflows by calling describeWorkflow(store, workflowId) sequentially causing unnecessary latency; replace the for-await pattern with parallel execution by mapping referenced to promises (e.g. referenced.map(id => describeWorkflow(store, id))) and await Promise.all(...) to produce the BoardWorkflowDefinition[] (assign the resulting array to workflows) so all describeWorkflow calls run concurrently while preserving the same store and workflowId arguments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 35fd859f-bbb2-4bcf-acbe-6dca203e233a
📒 Files selected for processing (131)
.changeset/workflow-custom-columns-traits.md.changeset/workflow-graph-editor-and-bundled-plugins.mdCONCEPTS.mddocs/PLUGIN_AUTHORING.mddocs/architecture.mddocs/plans/2026-06-03-002-feat-workflow-interpreter-cutover-plan.mddocs/plans/2026-06-03-003-feat-workflow-custom-columns-traits-plan.mddocs/residual-review-findings/gsxdsm-custom-columns.mddocs/workflow-steps.mdeslint.config.mjspackages/cli/src/commands/dashboard-tui/__tests__/app.test.tsxpackages/cli/src/commands/dashboard-tui/__tests__/bucket-mapping.test.tspackages/cli/src/commands/dashboard-tui/app.tsxpackages/cli/src/commands/dashboard-tui/bucket-mapping.tspackages/cli/src/commands/dashboard-tui/state.tspackages/cli/src/commands/dashboard.tspackages/core/src/__tests__/builtin-coding-workflow-ir.test.tspackages/core/src/__tests__/builtin-traits.test.tspackages/core/src/__tests__/builtin-workflows.test.tspackages/core/src/__tests__/db-migrate.test.tspackages/core/src/__tests__/db.test.tspackages/core/src/__tests__/default-workflow-hooks.test.tspackages/core/src/__tests__/goals-schema.test.tspackages/core/src/__tests__/insight-store.test.tspackages/core/src/__tests__/merge-request-record.test.tspackages/core/src/__tests__/migration-workflow-columns.test.tspackages/core/src/__tests__/mission-store.test.tspackages/core/src/__tests__/move-task-characterization.test.tspackages/core/src/__tests__/run-audit.test.tspackages/core/src/__tests__/store-merge-queue.test.tspackages/core/src/__tests__/task-documents.test.tspackages/core/src/__tests__/trait-registry.test.tspackages/core/src/__tests__/transition-parity.test.tspackages/core/src/__tests__/transition-types.test.tspackages/core/src/__tests__/workflow-ir.test.tspackages/core/src/__tests__/workflow-reconciliation.test.tspackages/core/src/builtin-coding-workflow-ir.tspackages/core/src/builtin-traits.tspackages/core/src/db.tspackages/core/src/default-workflow-hooks.tspackages/core/src/index.tspackages/core/src/plugin-gate-verdict.tspackages/core/src/plugin-loader.tspackages/core/src/plugin-types.tspackages/core/src/store.tspackages/core/src/trait-registry.tspackages/core/src/trait-types.tspackages/core/src/transition-pending.tspackages/core/src/transition-types.tspackages/core/src/types.tspackages/core/src/workflow-capacity.tspackages/core/src/workflow-columns-settings.tspackages/core/src/workflow-definition-types.tspackages/core/src/workflow-ir-types.tspackages/core/src/workflow-ir.tspackages/core/src/workflow-parity.tspackages/core/src/workflow-reconciliation.tspackages/core/src/workflow-transitions.tspackages/dashboard/app/api/legacy.tspackages/dashboard/app/components/Board.tsxpackages/dashboard/app/components/Column.tsxpackages/dashboard/app/components/Lane.csspackages/dashboard/app/components/Lane.tsxpackages/dashboard/app/components/PluginManager.tsxpackages/dashboard/app/components/TaskCard.tsxpackages/dashboard/app/components/TaskDetailModal.tsxpackages/dashboard/app/components/WorkflowColumnPanel.tsxpackages/dashboard/app/components/WorkflowNodeEditor.csspackages/dashboard/app/components/WorkflowNodeEditor.tsxpackages/dashboard/app/components/WorkflowResultsTab.tsxpackages/dashboard/app/components/WorkflowSelector.tsxpackages/dashboard/app/components/__tests__/Board.test.tsxpackages/dashboard/app/components/__tests__/Column.test.tsxpackages/dashboard/app/components/__tests__/Lane.test.tsxpackages/dashboard/app/components/__tests__/TaskCard.test.tsxpackages/dashboard/app/components/__tests__/WorkflowNodeEditor.test.tsxpackages/dashboard/app/components/__tests__/WorkflowSelector.test.tsxpackages/dashboard/app/components/__tests__/workflow-flow-mapping.test.tspackages/dashboard/app/components/nodes/WorkflowNodeTypes.tsxpackages/dashboard/app/components/workflow-flow-mapping.tspackages/dashboard/src/__tests__/workflow-routes.test.tspackages/dashboard/src/routes/__tests__/board-workflows.test.tspackages/dashboard/src/routes/__tests__/register-task-workflow-routes.move-bypassguards.test.tspackages/dashboard/src/routes/board-workflows.tspackages/dashboard/src/routes/register-task-workflow-routes.tspackages/dashboard/src/routes/register-workflow-routes.tspackages/dashboard/vitest.config.tspackages/engine/src/__tests__/hold-release.test.tspackages/engine/src/__tests__/merge-trait.test.tspackages/engine/src/__tests__/plugin-traits.test.tspackages/engine/src/__tests__/scheduler.test.tspackages/engine/src/__tests__/workflow-graph-fanout.test.tspackages/engine/src/concurrency.tspackages/engine/src/hold-release.tspackages/engine/src/index.tspackages/engine/src/merge-trait.tspackages/engine/src/merger.tspackages/engine/src/plugin-runner.tspackages/engine/src/plugin-trait-adapter.tspackages/engine/src/run-audit.tspackages/engine/src/scheduler.tspackages/engine/src/workflow-graph-branches.tspackages/engine/src/workflow-graph-executor.tspackages/engine/src/workflow-graph-task-runner.tspackages/i18n/locales/en/app.jsonpackages/i18n/locales/en/cli.jsonpackages/i18n/locales/en/common.jsonpackages/i18n/locales/en/errors.jsonpackages/i18n/locales/es/app.jsonpackages/i18n/locales/es/cli.jsonpackages/i18n/locales/es/common.jsonpackages/i18n/locales/es/errors.jsonpackages/i18n/locales/fr/app.jsonpackages/i18n/locales/fr/cli.jsonpackages/i18n/locales/fr/common.jsonpackages/i18n/locales/fr/errors.jsonpackages/i18n/locales/ko/app.jsonpackages/i18n/locales/ko/cli.jsonpackages/i18n/locales/ko/common.jsonpackages/i18n/locales/ko/errors.jsonpackages/i18n/locales/zh-CN/app.jsonpackages/i18n/locales/zh-CN/cli.jsonpackages/i18n/locales/zh-CN/common.jsonpackages/i18n/locales/zh-CN/errors.jsonpackages/i18n/locales/zh-TW/app.jsonpackages/i18n/locales/zh-TW/cli.jsonpackages/i18n/locales/zh-TW/common.jsonpackages/i18n/locales/zh-TW/errors.jsonpackages/i18n/src/i18next-resources.d.tspackages/i18n/src/resources.d.tspackages/plugin-sdk/src/index.ts
…pe-only exports, restore pruned i18n keys
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/i18n/locales/en/common.json`:
- Around line 61-62: The pluralization keys toolCallsCount_one and
toolCallsCount_other are inconsistent: update the value for "toolCallsCount_one"
to use the singular form (e.g., "{{count}} tool call") while keeping
"toolCallsCount_other" as "{{count}} tool calls" so that the singular case
renders correctly for the key toolCallsCount (used by the i18n pluralization
system).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c4f7863-8ec5-40ef-8bb2-29c3a9298d87
📒 Files selected for processing (10)
packages/cli/src/__tests__/task-plan.test.tspackages/cli/src/__tests__/task-steer.test.tspackages/i18n/locales/en/common.jsonpackages/i18n/locales/es/common.jsonpackages/i18n/locales/fr/common.jsonpackages/i18n/locales/ko/common.jsonpackages/i18n/locales/zh-CN/common.jsonpackages/i18n/locales/zh-TW/common.jsonpackages/plugin-sdk/src/index.tsplugins/fusion-plugin-roadmap/src/store/__tests__/roadmap-store.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/i18n/locales/es/common.json
| "toolCallsCount_one": "{{count}} tool calls", | ||
| "toolCallsCount_other": "{{count}} tool calls", |
There was a problem hiding this comment.
Fix singular pluralization form for tool-call count.
Line 61 should use singular text; current _one renders grammatically incorrect output (e.g., “1 tool calls”).
Suggested patch
- "toolCallsCount_one": "{{count}} tool calls",
+ "toolCallsCount_one": "{{count}} tool call",
"toolCallsCount_other": "{{count}} tool calls",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "toolCallsCount_one": "{{count}} tool calls", | |
| "toolCallsCount_other": "{{count}} tool calls", | |
| "toolCallsCount_one": "{{count}} tool call", | |
| "toolCallsCount_other": "{{count}} tool calls", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/i18n/locales/en/common.json` around lines 61 - 62, The pluralization
keys toolCallsCount_one and toolCallsCount_other are inconsistent: update the
value for "toolCallsCount_one" to use the singular form (e.g., "{{count}} tool
call") while keeping "toolCallsCount_other" as "{{count}} tool calls" so that
the singular case renders correctly for the key toolCallsCount (used by the i18n
pluralization system).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/__tests__/run-audit.test.ts`:
- Around line 586-587: The test title is inconsistent with its assertion: update
the test description string in the failing test (the it(...) block named "schema
version is bumped to 40") to match the asserted schema version
(expect(db.getSchemaVersion()).toBe(107)); locate the it(...) that contains
expect(db.getSchemaVersion()) and rename its description to "schema version is
bumped to 107" (or to whatever version the assertion should reflect) so the test
name accurately reflects the assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 69fd61e1-e163-4fd7-9e45-dea1ba84ff38
📒 Files selected for processing (7)
packages/core/src/__tests__/goals-schema.test.tspackages/core/src/__tests__/insight-store.test.tspackages/core/src/__tests__/merge-request-record.test.tspackages/core/src/__tests__/mission-store.test.tspackages/core/src/__tests__/run-audit.test.tspackages/core/src/__tests__/store-merge-queue.test.tspackages/core/src/__tests__/task-documents.test.ts
✅ Files skipped from review due to trivial changes (2)
- packages/core/src/tests/goals-schema.test.ts
- packages/core/src/tests/store-merge-queue.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/tests/mission-store.test.ts
| it("schema version is bumped to 40", () => { | ||
| expect(db.getSchemaVersion()).toBe(105); | ||
| expect(db.getSchemaVersion()).toBe(107); |
There was a problem hiding this comment.
Rename the test to match the asserted schema version.
The test name still says 40 while the assertion expects 107, which is misleading when failures are triaged.
Suggested fix
- it("schema version is bumped to 40", () => {
+ it("schema version is bumped to 107", () => {
expect(db.getSchemaVersion()).toBe(107);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("schema version is bumped to 40", () => { | |
| expect(db.getSchemaVersion()).toBe(105); | |
| expect(db.getSchemaVersion()).toBe(107); | |
| it("schema version is bumped to 107", () => { | |
| expect(db.getSchemaVersion()).toBe(107); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/__tests__/run-audit.test.ts` around lines 586 - 587, The
test title is inconsistent with its assertion: update the test description
string in the failing test (the it(...) block named "schema version is bumped to
40") to match the asserted schema version
(expect(db.getSchemaVersion()).toBe(107)); locate the it(...) that contains
expect(db.getSchemaVersion()) and rename its description to "schema version is
bumped to 107" (or to whatever version the assertion should reflect) so the test
name accurately reflects the assertion.
…select, CRUD, trait catalog (#1408)
Summary
Boards are no longer locked to the fixed
triage → todo → in-progress → in-review → donepipeline. A workflow now defines its own columns, each carrying composable traits (WIP limits, holds, human-review gates, merge orchestration, …), and the dashboard renders one lane per workflow in use. The engine inverts into a capability substrate: transitions, capacity, hold/release, merge policy, and recovery semantics are all resolved from the task's workflow instead of hardcoded column names.Everything ships behind
experimentalFeatures.workflowColumns(default off). Flag-off behavior is byte-identical, proven by a 160-case characterization suite that runs against both flag states.What's now possible
manual | timer | capacity | dependency | external-eventrelease).split/joingraph nodes run branches concurrently (all | any | quorum(n)joins, fail-fast or collect), with crash-recoverable per-branch state. Seam nodes (execute/merge) are validator-banned inside branches.strict | warn | off | custom) move into the merge trait; the three lost-work guards stay capability-level and unreachable from config.gate/onEnter/onExit/releaseCondition) through one registry;complete/archivedflags and sync guards are built-in-only.Key design decisions
moveTaskInternalstays the single transition authoritytransitionPendingmarkerMigration
Additive-only schema (v106
tasks.transitionPending, v107workflow_run_branches), forward-only, idempotent. A flag-on integrity pass audits and re-homes any task whose column is invalid in its resolved workflow. Flag-off after flag-on is rollback-safe (tested), with one caveat tracked in #1405 (v2 IR rows are unreadable by pre-upgrade binaries).Testing
~1,000 new/updated tests across core/engine/dashboard/cli: 160-case move characterization (both flag states), transition parity vs
VALID_TRANSITIONS, capacity/hold-release with fake timers, fan-out (quorum/fail-fast/crash-resume), plugin traits against real engine wiring, migration/rollback safety, multi-lane board + editor component suites, TUI bucket mapping. Multi-agent code review (13 reviewers + 9 validators) applied 14 fixes on top; lint and all package typechecks clean. Browser-verified live: flag-off board byte-identical, workflow editor's read-only built-in banner, duplicate-to-customize, and the column/traits panel.Residual Review Findings
All 17 findings have been fixed on this branch:
Fixes #1401, fixes #1402, fixes #1403, fixes #1404, fixes #1405, fixes #1406, fixes #1407, fixes #1408, fixes #1409, fixes #1410, fixes #1411, fixes #1412, fixes #1413, fixes #1414, fixes #1415, fixes #1416, fixes #1417.
Post-Deploy Monitoring & Validation
experimentalFeatures.workflowColumnsis enabled per-install.transitionPendingmarkers persisting > minutes (SELECT id FROM tasks WHERE transitionPending IS NOT NULL) — recovery sweep is not yet implemented ([P1] Implement transitionPending recovery sweep (markers leak capacity slots after crash) #1401);merge:dependency-parity-diffaudit events (dual-accept drift — blocks graduation);task:workflow-reconcileaudits (unexpected re-homes); sweep log noise fromhold-release.computeWorkflowColumnsGraduationReport) returns zero blockers before any default-flip.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests