Phase A: workflow-owned lifecycle foundation (U1, U2, U3) by gsxdsm · Pull Request #2467 · Runfusion/Fusion

gsxdsm · 2026-07-27T07:21:38Z

Phase A (Foundation) of docs/plans/2026-07-26-001-refactor-workflow-owned-lifecycle-plan.md. Three units, one commit each. No operator-visible behavior change.

U1 — Lifecycle-column resolution seam

resolveLifecycleColumns(ir) returns { intake, hold, wip, review, complete, archived } — the first column carrying each trait, undefined for a role no column carries. resolveTaskLifecycleColumns(store, taskId, cache?) is the store-aware form; the cache is caller-owned so a sweep reads one IR per workflow rather than one per card.

A v1/column-less IR resolves to undefined for the whole struct rather than a struct of undefined roles. A caller must be able to distinguish "this workflow declares no hold column" (a real shape to honor) from "no column vocabulary at all" (skip and log) — only the second licenses conservative fallback.

Nothing consumes the seam yet; Phases B–D convert the ~207 hardcoded column literals onto it.

U2 — Delete the pre-cutover parity machinery (delete-only)

workflow-columns-settings.ts — isWorkflowColumnsEnabled had the body return true. Six live call sites branched on it, so every flag-OFF arm was dead code that read as a supported configuration. Deleted; surviving side inlined at self-healing's transitionPending sweep, the scheduler's per-column capacity diagnostic, merge-trait's policy resolver, the board-workflows payload, two task-workflow routes, and the CLI TUI's column enrichment.

workflow-parity.ts — asserted the default workflow's adjacency equals the legacy VALID_TRANSITIONS. U11 deliberately breaks that equality by merging Todo into Planning, so this is not a stale assertion to update; it is a contract against the target state. Its emitter (workflow-parity-observer.ts) is already a tombstone, so getWorkflowParitySummary and computeWorkflowColumnsGraduationReport aggregated run-audit rows nothing writes and had no caller outside TaskStore. Both store methods go with it.

flagEnabled stays on the board-workflows wire as a constant true — shipped dashboard clients still branch on it, and changing the response shape is not a deletion. U10 retires the field once no client reads it.

The legacy-tombstones ratchet is extended to both files plus seven symbols, each with the reason it is gone.

⚠️ Finding: the third listed deletion was NOT dead

The plan also lists "the flag-off inline move path" in task-store/moves.ts. It is not deleted, per U2's execution note ("any behavior change found while removing a branch means the branch was not dead").

That path is gated on isWorkflowColumnsCompatibilityFlagEnabled (store.ts:38) — a different function from the always-true public helper. It reads the raw experimentalFeatures.workflowColumns setting, which nothing in production sets (settings-schema.ts:396 — "no default flags are emitted"; zero non-test writers; the operator's own ~/.fusion/settings.json has no such key). So useWorkflow is false for effectively every real project: the flag-OFF inline side effects are the live default move path and the flag-ON default-workflow-hooks path is the dead one. The code says so itself at moves.ts:638.

Deleting that branch would swap every project onto an untravelled code path — a behavior change, not a deletion.

Carry this into Phases B and C, stated plainly so the plan's error is not repeated:

The inline move path in moves.ts is LIVE. default-workflow-hooks.ts (the trait-hook path) is DEAD. KTD-6 asserted the inverse. Until the convergence unit lands, nothing may assume trait hooks run — a guard, sweep, or subscriber written against applyDefaultWorkflowMoveEffects would never fire in production and would still pass its tests.

Convergence is not attempted here. It is its own unit (Phase A2) with a proper equivalence proof, per operator decision.

U3's emit point is on the LIVE path — the seam is not born dead

Worth stating explicitly because it is the failure mode that would make every later subscriber silently never fire: the TaskTransitioned emit is not inside the if (useWorkflow) branch. That block closes at moves.ts:1212; the emit sits at :1214, beside the existing store.emit("task:moved", …), on the unconditional post-commit path. It therefore fires on both the live inline path and the dead hooks path, and the convergence unit inherits the obligation to keep it firing on whichever path survives — same events, same order, same payloads.

The graph-side emitters (NodeEntered, RunSuspended) carry the same risk from a different direction: the bus refuses an invalid payload silently by design, so an emitter regression would stop the event with no test failure. They are asserted end-to-end through the real bus — "did a subscriber actually receive it", not "was emit called" — because a spy passes on a refused payload. The moveTaskInternalImpl emit does not yet have that end-to-end assertion against a real store move; that proof belongs to the convergence unit, which has to build the both-paths fixture anyway.

U3 — Post-commit event seam with a transactional outbox

The bus is not a queue, not a transaction participant, and not a delivery guarantee. Durable follow-on work uses the transactional outbox — a workflow_work_items row written inside the transition transaction (the shape createCompletionHandoffWorkflowWork already uses). "Emit after commit, let a subscriber enqueue the work" has a crash window where a process dies between commit and subscriber, leaving no event and no work-item row, so required work is skipped permanently with nothing to recover from. Post-commit subscribers therefore carry only losable reactions.

Emission is consequently lossy and isolated by design: a throwing or rejecting subscriber is caught and logged, cannot roll back the transition, and cannot stop the others. Deliveries append to one serial chain, so two transitions on a task deliver in commit order.

The ids/outcomes-only rule is mechanised, not documented — run-audit's equivalent lives only in prose and has been violated repeatedly. A payload carrying an object body or a prose string is refused at the emit boundary and never reaches a subscriber or log sink. It degrades rather than throws: the emitter is post-commit, so a shape bug must not become a lifecycle failure.

Emit points: TaskTransitioned from the single post-commit point in moveTaskInternalImpl; NodeEntered and RunSuspended from the graph column boundary, the latter after the durable continuation is persisted so an observed suspension implies a resumable run.

registerWorkflowEventSubscribers (engine) is empty on purpose — U7/U8/U10 move real reactions onto it, each with the characterization test proving the reaction was non-authoritative first.

Verification

pnpm test:gate — green (2/10, 16/299, 1/71).
pnpm lint, pnpm build, tsc --noEmit on core and engine — green.
U1: 20 tests in workflow-lifecycle-traits.test.ts, including the fully-renamed-workflow case (fails if the resolver falls back to a literal) and a shared-cache read-count assertion.
U2: legacy-tombstones.test.ts green with the extended ratchet; board-workflows, merge-trait, workflow-graph-executor-parity, and move-hook suites green with no expectation edits.
U3: 20 bus-invariant unit tests (isolation, ordering, the allowed-key and required-key halves of the ids-only rule, lossiness) plus 3 end-to-end emitter-delivery tests; 5 outbox tests against a real PostgreSQL work-item table (crash survival, rollback, at-least-once redelivery on lease expiry, idempotent handler → one effect, dropped-subscriber vs. durable work). A hand-written fake of the lease predicate would only prove the fake redelivers.

Not verified: the moveTaskInternalImpl emit is confirmed on the unconditional post-commit path by structure and by the surrounding tests, but is not yet asserted end-to-end against a real store move on both flag settings — that is Phase A2's fixture. The engine subscriber registry ships empty by design, so no production subscriber exercises the bus end-to-end yet. settings-defaults.test.ts has one pre-existing failure on main (a logger-prefix mismatch in the mergeIntegrationWorktree=cwd-main warning) — confirmed present on a clean tree, unrelated to this branch.

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Workflow lifecycle columns are now derived from workflow definitions, supporting renamed and custom workflows.
- Added post-commit lifecycle events for task transitions, node entry, and run suspend/resume with validated payloads.
- Follow-on processing for lifecycle emissions is now more robust (rollback-safe, at-least-once delivery, idempotent handling).
Bug Fixes
- Workflow board responses, task enrichment, and promotion no longer depend on workflow-columns feature-flag gating.
- Subscriber failures no longer impact committed workflow transitions.

Phase A/U1 of the workflow-owned-lifecycle program. ~207 production sites decide the task lifecycle by comparing `task.column` against a hardcoded id. Such a guard does not fail when the column moves underneath it — it silently stops matching, disabling a recovery path with a green suite. Phases B-D convert those sites onto one resolver so the conversion is mechanical rather than per-site IR plumbing. `resolveLifecycleColumns(ir)` returns `{ intake, hold, wip, review, complete, archived }` — the first column carrying each trait, `undefined` for a role no column carries (never substituted from an unrelated column). `resolveTaskLifecycleColumns(store, taskId, cache?)` is the store-aware form over the existing `resolveWorkflowIrForTask`; the cache is caller-owned so a sweep reads one IR per workflow rather than one per card. A v1/column-less IR resolves to `undefined` for the WHOLE struct rather than a struct of undefined roles: a caller must distinguish "this workflow declares no hold column" (a real shape to honor) from "no column vocabulary at all" (skip and log). Only the latter licenses conservative fallback. No behavior change — nothing consumes the seam yet.

…arity harness (U2) Phase A/U2 of the workflow-owned-lifecycle program (R9). Delete-only: two pre-cutover modules whose branches were provably unreachable. `workflow-columns-settings.ts` held `isWorkflowColumnsEnabled`, whose body was `return true`. Six live call sites still branched on it, so every flag-OFF arm was dead code that read as a supported configuration — a future reader adding a lifecycle branch would naturally gate it on the "flag" and ship a dead arm. Deleted, surviving side inlined at: self-healing's transitionPending sweep, scheduler's per-column capacity diagnostic, merge-trait's policy resolver, the board-workflows payload, two task-workflow routes, and the CLI TUI's column enrichment. `workflow-parity.ts` asserted the default workflow's adjacency EQUALS the legacy `VALID_TRANSITIONS`. U11 deliberately breaks that equality by merging Todo into Planning, so this is not a stale assertion to update — it is a contract against the target state. Its emitter (`workflow-parity-observer.ts`) is already a tombstone, so `getWorkflowParitySummary` and `computeWorkflowColumnsGraduationReport` aggregated run-audit rows nothing writes; neither had a caller outside TaskStore. Both store methods go with it. `flagEnabled` stays on the board-workflows WIRE as a constant `true`: shipped dashboard clients still branch on it, and changing the response shape is not a deletion. U10 retires the field once no client reads it. The tombstone ratchet is extended to both files plus seven symbols, each with the reason it is gone — a failure message that only says "symbol found" invites re-deletion without understanding, which is how this machinery came back once already. NOT deleted, and reported as a finding: the plan also lists the flag-off inline move path in `task-store/moves.ts`. That path is gated on `isWorkflowColumnsCompatibilityFlagEnabled` — a DIFFERENT function reading the raw `experimentalFeatures.workflowColumns` setting, which nothing in production sets. It is therefore the LIVE default move path (the code says so at moves.ts:638), and the flag-ON hooks path is the dead one. Removing it would swap every project onto an untravelled path: a behavior change, not a deletion.

…box (U3) Phase A/U3 of the workflow-owned-lifecycle program (R5, R6). One place transitions are announced, one registry of subscribers — the seam later units move imperative cross-service reactions behind. Today, when a task transitions, every service that must react is invoked directly from the transition site; that is the main reason `executor.ts` is 21k lines. WHAT THE BUS IS NOT. Not a queue, not a transaction participant, not a delivery guarantee. Durable follow-on work uses the TRANSACTIONAL OUTBOX — a `workflow_work_items` row written INSIDE the transition transaction, the shape `createCompletionHandoffWorkflowWork` already uses. "Emit after commit, let a subscriber enqueue the work" has a crash window: a process that dies between the commit and the subscriber leaves no event AND no work-item row, so required work is skipped permanently with nothing to recover from. Post-commit subscribers therefore carry only losable reactions — notify, board refresh, analytics. Emission is consequently lossy and isolated by design: a throwing (or rejecting) subscriber is caught and logged, cannot roll back the transition, and cannot stop the other subscribers. Deliveries are appended to one serial chain so two transitions on a task deliver in commit order, which is what lets a subscriber maintain derived state without its own sequencing. The ids/outcomes-only rule is MECHANISED, not documented. run-audit's equivalent lives only in prose and has been violated repeatedly; these payloads reach plugin subscribers, so a payload carrying an object body or a prose string is refused at the emit boundary and never reaches a subscriber or a log sink. It degrades rather than throws — the emitter is post-commit, so a shape bug must not surface as a lifecycle failure. Emit points: `TaskTransitioned` from the single post-commit point in `moveTaskInternalImpl` (beside the existing `task:moved` store event, which U7 migrates onto the bus); `NodeEntered` and `RunSuspended` from the graph's column boundary controller, the latter after the durable continuation is persisted so an observed suspension implies a resumable run. `registerWorkflowEventSubscribers` in @fusion/engine is the registration point and is EMPTY on purpose: U7/U8/U10 move real reactions onto it, each with the characterization test proving the reaction was non-authoritative before it moved. Landing it pre-populated would convert reactions in the same commit that introduces the mechanism they rely on. Tests: bus isolation/ordering/ids-only/lossiness as unit tests; the outbox half — crash survival, rollback, at-least-once redelivery on lease expiry, and an idempotent handler producing one effect across two deliveries — against a REAL PostgreSQL work-item table, because a hand-written fake of the lease predicate would only prove the fake redelivers.

coderabbitai · 2026-07-27T07:22:39Z

📝 Walkthrough

Walkthrough

Workflow lifecycle columns now resolve from traits, workflow-column flag gates are removed, and legacy parity APIs are deleted. Core adds a validated post-commit event bus, engine boundaries emit lifecycle events, and PostgreSQL tests cover transactional outbox durability and delivery semantics.

Changes

Workflow-owned lifecycle

Layer / File(s)	Summary
Lifecycle resolution and flag cutover `packages/core/...`, `packages/dashboard/...`, `packages/cli/...`, `packages/engine/...`	Lifecycle roles resolve from workflow traits, task resolution supports shared caching, and workflow-column flag gates are removed from dashboard, CLI, merge, scheduling, and recovery paths.
Lifecycle event contract and bus `packages/core/types/workflow-events.ts`, `packages/core/workflow-events.ts`, `packages/engine/workflow-event-subscribers.ts`	Adds lifecycle event types, ids-only payload validation, serialized isolated delivery, global bus access, and engine subscriber registration.
Post-commit emission and outbox validation `packages/core/task-store/moves.ts`, `packages/engine/workflow-column-boundary.ts`, `packages/core/__tests__/postgres/*`	Emits `TaskTransitioned`, `NodeEntered`, and `RunSuspended` at workflow boundaries and validates transactional outbox durability, rollback, redelivery, idempotency, and subscriber loss.
Parity API removal and tombstones `packages/core/...`, `packages/engine/__tests__/*`	Removes workflow parity reports and exports, deletes related store implementations, and updates tombstone and executor tests.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TaskTransition
  participant WorkflowEventBus
  participant EngineSubscriber
  participant PostgreSQL
  TaskTransition->>PostgreSQL: Commit transition and durable work item
  TaskTransition->>WorkflowEventBus: Emit TaskTransitioned
  WorkflowEventBus->>EngineSubscriber: Deliver validated event
  PostgreSQL-->>TaskTransition: Redeliver leased work item

Possibly related PRs

Runfusion/Fusion#2335: Modifies the workflow column-boundary model used by this PR.
Runfusion/Fusion#2341: Modifies lifecycle traits and task transition paths used by this PR.
Runfusion/Fusion#2378: Changes the workflow column-boundary suspension and entry paths where this PR emits events.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 68.18% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the PR’s main theme and scope across U1, U2, and U3.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feature/workflow-owned-lifecycle

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

greptile-apps · 2026-07-27T07:24:34Z

Greptile Summary

Phase A adds workflow-owned lifecycle foundations without changing current operator behavior.

Adds trait-based lifecycle-column resolution, including task-aware workflow lookup and caller-owned caching.
Removes obsolete workflow-column flag and parity machinery while retaining the existing compatibility move path.
Introduces validated, isolated post-commit lifecycle events while keeping authoritative follow-on work in the transactional outbox.
Emits task-transition, node-entry, and run-suspension events at their corresponding lifecycle boundaries.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
packages/core/src/workflow-lifecycle-traits.ts	Adds trait-driven lifecycle role resolution and delegates task-aware lookup to the canonical workflow IR resolver.
packages/core/src/workflow-events.ts	Adds a serial, post-commit event bus with payload validation and isolated subscriber failures.
packages/core/src/types/workflow-events.ts	Defines lifecycle event contracts and enforces per-type allowed and required payload keys.
packages/core/src/task-store/moves.ts	Emits TaskTransitioned only after the task transition transaction commits, with compatibility-aware workflow identity.
packages/engine/src/workflow-column-boundary.ts	Emits node-entry and suspension events while ensuring durable suspension state is persisted first.
packages/engine/src/workflow-event-subscribers.ts	Establishes the engine subscriber registration seam without introducing authoritative event-driven reactions.
packages/core/src/store.ts	Removes obsolete workflow parity store methods and updates move-task wiring for lifecycle event emission.
packages/dashboard/src/routes/board-workflows.ts	Removes dead feature-flag branching while preserving the shipped flagEnabled response field.
packages/cli/src/commands/dashboard.ts	Makes workflow-column enrichment explicitly unconditional, matching the previously always-enabled behavior.

Sequence Diagram

sequenceDiagram
  participant Engine as Workflow Engine
  participant Store as Task Store
  participant DB as PostgreSQL
  participant Bus as Lifecycle Event Bus
  participant Subscriber as Non-authoritative Subscriber

  Engine->>Store: Move task / cross node boundary
  Store->>DB: Begin transition transaction
  Store->>DB: Persist lifecycle state
  opt Durable follow-on work required
    Store->>DB: Insert workflow_work_items row
  end
  DB-->>Store: Commit
  Store->>Bus: Emit lifecycle event
  Bus->>Bus: Validate IDs/outcomes-only payload
  Bus-->>Subscriber: Deliver serially after commit
  Subscriber-->>Bus: Success, throw, or reject
  Bus->>Bus: Isolate subscriber failure

_{Reviews (3): Last reviewed commit: "fix(core): enforce required event keys, ..." | Re-trigger Greptile}

coderabbitai

Actionable comments posted: 4

🧹 Nitpick comments (6)

packages/dashboard/src/routes/board-workflows.ts (1)
169-184: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

getSettings is no longer read — consider dropping it from the store constraint.

Nothing in the body touches settings now, so the "getSettings" member of the Pick<TaskStore, …> only forces callers and test doubles to keep supplying it. Keeping settingsOverride in the signature is fine (documented), but the store type can narrow.
♻️ Narrow the store type
-  store: Pick<TaskStore, "getWorkflowDefinition" | "getTaskWorkflowSelection" | "getSettings" | "listWorkflowDefinitions"> &
+  store: Pick<TaskStore, "getWorkflowDefinition" | "getTaskWorkflowSelection" | "listWorkflowDefinitions"> &
     Partial<Pick<TaskStore, "getTaskWorkflowSelectionAsync">>,
🤖 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 169 - 184,
Remove "getSettings" from the store constraint in buildBoardWorkflowsPayload,
leaving the other required TaskStore members and the optional
getTaskWorkflowSelectionAsync unchanged. Keep the settingsOverride parameter and
its existing signature behavior intact.
packages/core/src/workflow-lifecycle-traits.ts (1)
151-165: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Resolve each column's flags once instead of six times.

first() re-runs registry.resolveColumnFlags(c) per role, so a workflow with N columns performs up to 6N resolutions — and resolveLifecycleColumns is not memoized, so a caller-cached sweep over 400 cards still pays this per card (the cache only holds the IR). A single pass over the columns keeps the struct identical.
♻️ Single-pass flag resolution
-  const registry = getTraitRegistry();
-  const first = (flag: keyof TraitFlags): string | undefined =>
-    columns.find((c) => registry.resolveColumnFlags(c)[flag] === true)?.id;
+  const registry = getTraitRegistry();
+  const resolved = columns.map((c) => [c.id, registry.resolveColumnFlags(c)] as const);
+  const first = (flag: keyof TraitFlags): string | undefined =>
+    resolved.find(([, flags]) => flags[flag] === true)?.[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/core/src/workflow-lifecycle-traits.ts` around lines 151 - 165,
Update resolveLifecycleColumns to resolve each column’s flags once during a
single pass, storing the first matching column ID for each lifecycle role in the
returned LifecycleColumns fields. Remove the first helper’s repeated
registry.resolveColumnFlags calls while preserving the existing field mapping
and undefined result for workflows with no columns.
packages/core/src/__tests__/workflow-events.test.ts (1)
91-109: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Prefer a controllable deferred over a real 20 ms sleep for the ordering proof.

The bus serializes via its promise chain, so ordering can be demonstrated with a deferred the test resolves explicitly (or fake timers) instead of wall-clock delay — same assertion, no timing surface.

As per coding guidelines: "Prefer narrow seams, in-memory fakes, shared harnesses, fake timers, and targeted assertions".
🤖 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__/workflow-events.test.ts` around lines 91 - 109,
Replace the real 20 ms timeout in the async subscriber of “delivers two seams on
one task in commit order even when subscribers are async” with a controllable
deferred or fake-timer gate that the test explicitly resolves. Keep the
subscriber blocked until that signal, then release it before awaiting
bus.drain(), preserving the existing commit-order assertion without relying on
wall-clock timing.
Source: Coding guidelines
packages/core/src/__tests__/postgres/workflow-events-outbox.pg.test.ts (1)
63-68: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Use execFileSync with an argument array instead of a shell string.

The values interpolated here are module-controlled (generated db names, literal statements, a developer-supplied FUSION_PG_TEST_URL_BASE), so this isn't currently exploitable — but the hand-rolled " escaping is fragile and will break the first time a statement contains a quote or $. An argument array removes both the shell and the escaping.
♻️ Drop the shell
-import { execSync } from "node:child_process";
+import { execFileSync } from "node:child_process";
 function adminExec(statement: string): void {
-  execSync(
-    `psql "${PG_TEST_URL_BASE}/postgres" -v ON_ERROR_STOP=1 -c "${statement.replace(/"/g, '\\"')}"`,
-    { stdio: "pipe", env: process.env },
-  );
+  execFileSync(
+    "psql",
+    [`${PG_TEST_URL_BASE}/postgres`, "-v", "ON_ERROR_STOP=1", "-c", statement],
+    { stdio: "pipe", env: process.env },
+  );
 }
🤖 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__/postgres/workflow-events-outbox.pg.test.ts`
around lines 63 - 68, Update adminExec to use execFileSync with the psql
executable and a separate argument array, including the database URL,
ON_ERROR_STOP option, command option, and statement as distinct arguments.
Remove the shell command string and manual quote escaping while preserving the
existing synchronous execution options and environment.
Source: Linters/SAST tools
packages/cli/src/commands/dashboard.ts (1)
1070-1087: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Stale heading above the changed block.

The section heading still reads "resolve per-task workflow column flags for the TUI (flag-ON only)" even though the flag and its early return are gone — the new FNXC note directly contradicts 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/cli/src/commands/dashboard.ts` around lines 1070 - 1087, Update the
section heading above resolveTaskColumnInfo to remove the stale “flag-ON only”
qualifier and describe unconditional per-task workflow column flag resolution,
aligning it with the removed flag parameter and early return.
packages/dashboard/src/routes/register-task-workflow-routes.ts (1)
1112-1120: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Dead getSettingsFast() call now that buildBoardWorkflowsPayload ignores settingsOverride.

With the flag-OFF short-circuit removed, settings is fetched here solely to pass as buildBoardWorkflowsPayload's settingsOverride, but that function now does void settingsOverride; and hardcodes flagEnabled = true — the value is never read. This leaves an unnecessary settings round-trip on every /tasks/board-workflows board-load request.
♻️ Proposed cleanup
-      const settings = await scopedStore.getSettingsFast();
       // Resolve over the same (non-archived) board list the client renders.
       const tasks = await scopedStore.listTasks({ slim: true, includeArchived: false });
       const taskIds = tasks.map((t) => t.id);
-      const payload = await buildBoardWorkflowsPayload(scopedStore, taskIds, settings);
+      const payload = await buildBoardWorkflowsPayload(scopedStore, taskIds);
🤖 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
1112 - 1120, Remove the unused scopedStore.getSettingsFast() call and the
resulting settings argument from the board-workflows route flow around
buildBoardWorkflowsPayload; update that function invocation to match its current
contract while preserving task ID collection and payload generation.

🤖 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/task-store/moves.ts`:
- Around line 1235-1244: Update the TaskTransitioned emission in the
move-handling flow to avoid reporting the hardcoded "builtin:coding" value when
the compatibility flag is off and the task’s workflow was not genuinely
resolved. Prefer omitting workflowId in that unresolved path, and make the
conditional spread reflect actual resolution rather than the always-truthy
effectiveWorkflowIdForMove value; preserve the real workflow ID when it is
available.

In `@packages/core/src/types/workflow-events.ts`:
- Around line 144-156: Update findWorkflowEventShapeViolations to validate each
event’s allowed and required keys based on its type before scalar validation.
Reject unknown fields and missing required fields according to the declared
workflow event schema, while preserving the existing scalar and array checks for
permitted keys.

In `@packages/engine/src/workflow-column-boundary.ts`:
- Around line 290-307: The workflow boundary handler currently returns before
emitting NodeEntered for columnless nodes. Move the NodeEntered emission ahead
of the early return, and only include the column property when toColumn is
present, preserving the existing event fields and behavior for column-based
nodes.

In `@packages/engine/src/workflow-event-subscribers.ts`:
- Around line 65-80: Update registerWorkflowEventSubscribers so it captures the
unsubscribe functions returned by this invocation’s bus.subscribe calls and
returns a cleanup that invokes only those functions. Do not return the global
unregisterWorkflowEventSubscribers function, ensuring cleanup from an earlier
registration cannot remove subscribers created by a later registration.

---

Nitpick comments:
In `@packages/cli/src/commands/dashboard.ts`:
- Around line 1070-1087: Update the section heading above resolveTaskColumnInfo
to remove the stale “flag-ON only” qualifier and describe unconditional per-task
workflow column flag resolution, aligning it with the removed flag parameter and
early return.

In `@packages/core/src/__tests__/postgres/workflow-events-outbox.pg.test.ts`:
- Around line 63-68: Update adminExec to use execFileSync with the psql
executable and a separate argument array, including the database URL,
ON_ERROR_STOP option, command option, and statement as distinct arguments.
Remove the shell command string and manual quote escaping while preserving the
existing synchronous execution options and environment.

In `@packages/core/src/__tests__/workflow-events.test.ts`:
- Around line 91-109: Replace the real 20 ms timeout in the async subscriber of
“delivers two seams on one task in commit order even when subscribers are async”
with a controllable deferred or fake-timer gate that the test explicitly
resolves. Keep the subscriber blocked until that signal, then release it before
awaiting bus.drain(), preserving the existing commit-order assertion without
relying on wall-clock timing.

In `@packages/core/src/workflow-lifecycle-traits.ts`:
- Around line 151-165: Update resolveLifecycleColumns to resolve each column’s
flags once during a single pass, storing the first matching column ID for each
lifecycle role in the returned LifecycleColumns fields. Remove the first
helper’s repeated registry.resolveColumnFlags calls while preserving the
existing field mapping and undefined result for workflows with no columns.

In `@packages/dashboard/src/routes/board-workflows.ts`:
- Around line 169-184: Remove "getSettings" from the store constraint in
buildBoardWorkflowsPayload, leaving the other required TaskStore members and the
optional getTaskWorkflowSelectionAsync unchanged. Keep the settingsOverride
parameter and its existing signature behavior intact.

In `@packages/dashboard/src/routes/register-task-workflow-routes.ts`:
- Around line 1112-1120: Remove the unused scopedStore.getSettingsFast() call
and the resulting settings argument from the board-workflows route flow around
buildBoardWorkflowsPayload; update that function invocation to match its current
contract while preserving task ID collection and payload generation.

🪄 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: b5b13f9f-c451-4cd1-aba1-4c61bab3ddbd

📥 Commits

Reviewing files that changed from the base of the PR and between 8b039a5 and 926d54f.

📒 Files selected for processing (27)

.changeset/workflow-owned-lifecycle-phase-a.md
packages/cli/src/commands/dashboard.ts
packages/core/src/__tests__/postgres/workflow-events-outbox.pg.test.ts
packages/core/src/__tests__/settings-defaults.test.ts
packages/core/src/__tests__/workflow-events.test.ts
packages/core/src/__tests__/workflow-lifecycle-traits.test.ts
packages/core/src/__tests__/workflow-parity.test.ts
packages/core/src/index.gate.ts
packages/core/src/index.ts
packages/core/src/store.ts
packages/core/src/task-store/moves.ts
packages/core/src/task-store/project-store-ops.ts
packages/core/src/task-store/task-artifacts-ops.ts
packages/core/src/types/workflow-events.ts
packages/core/src/workflow-columns-settings.ts
packages/core/src/workflow-events.ts
packages/core/src/workflow-lifecycle-traits.ts
packages/core/src/workflow-parity.ts
packages/dashboard/src/routes/board-workflows.ts
packages/dashboard/src/routes/register-task-workflow-routes.ts
packages/engine/src/__tests__/legacy-tombstones.test.ts
packages/engine/src/__tests__/workflow-graph-executor-parity.test.ts
packages/engine/src/merge-trait.ts
packages/engine/src/scheduler.ts
packages/engine/src/self-healing.ts
packages/engine/src/workflow-column-boundary.ts
packages/engine/src/workflow-event-subscribers.ts

💤 Files with no reviewable changes (5)

packages/core/src/tests/workflow-parity.test.ts
packages/core/src/workflow-columns-settings.ts
packages/core/src/workflow-parity.ts
packages/core/src/task-store/project-store-ops.ts
packages/core/src/tests/settings-defaults.test.ts

Four real defects in the new U3 seam plus one U1 hot-path fix. 1. IDS-ONLY WAS ONLY HALF ENFORCED (major). The validator checked value shape but accepted any unknown key, so a short single-line `error: "auth failed"`, `prompt`, or `modelId` passed every rule and would reach a plugin subscriber — exactly the leak the rule exists to prevent. Added a per-type key allow-list derived from the declared interfaces; an unknown key and an unrecognised event type are both refused outright. The value-shape cases now assert on DECLARED keys, since an undeclared key is refused first and would otherwise mask a value-rule regression behind an `unknown-key` verdict. 2. STALE CLEANUP COULD UNSUBSCRIBE A LATER REGISTRATION (major). `registerWorkflowEventSubscribers` returned the module-global `unregisterWorkflowEventSubscribers`, so a handle captured before an engine restart would silently remove the set registered after it — leaving the process with no reactions and no error. The returned cleanup is now scoped to its own call, and clears the module handle only when it still points at that registration. 3. `workflowId` MISREPORTED ON THE DEFAULT PATH. `effectiveWorkflowIdForMove` reads the task's real selection only when the compat flag is on, and that flag is off for effectively every real project (the U2 finding), so a custom-workflow task's transition would have been stamped `builtin:coding`. Latent only because no subscriber reads it yet — a wrong value in a brand-new wire field. Now omitted rather than guessed. 4. `NodeEntered` SKIPPED COLUMNLESS NODES. The emit sat below the columnless early return, contradicting its own "announce the node entry, not the column crossing" comment and never firing for `end`. Moved above the return, with `column` omitted when absent — which is why the field is optional. 5. `resolveLifecycleColumns` resolved every column's traits once PER ROLE (up to 6N per call) and is not memoized, so a Phase B sweep sharing an IR cache across 400 cards would still pay it per card. Single pass now. Skipped: narrowing `getSettings` out of `buildBoardWorkflowsPayload`'s store constraint — it ripples into callers and test doubles for no behavior gain, and the parameter is already documented as retained for signature stability. New: `workflow-event-subscribers.test.ts` covers idempotent re-registration and the stale-cleanup case, both of which fail silently when wrong.

gsxdsm · 2026-07-27T07:44:16Z

All four actionable CodeRabbit findings are fixed in 5c8df6e (CI green on that commit); the inline threads above are anchored to pre-fix line numbers.

Enforce the declared event schema (major) — fixed. Added a per-type key allow-list in types/workflow-events.ts; unknown keys and unrecognised event types are now refused outright, so a short error/prompt/modelId no longer passes. The value-shape tests moved onto declared keys so an unknown-key verdict can't mask a value-rule regression.
Return a registration-scoped unsubscribe (major) — fixed. The returned cleanup closes over its own call's handles and clears the module handle only while it still points at that registration. Covered by the new workflow-event-subscribers.test.ts stale-cleanup case; this failed silently before.
workflowId misreports on the compat-flag-OFF path — fixed, and thanks: this is real precisely because the compat flag is off by default (see the U2 finding in the PR body). Now omitted rather than guessed.
Emit NodeEntered for columnless nodes — fixed. The emit moved above the columnless early return with column omitted when absent, which is what the optional field was for.

Nitpicks: took the single-pass flag resolution in resolveLifecycleColumns (it is on the Phase B sweep hot path and was up to 6N per call). Skipped narrowing getSettings out of buildBoardWorkflowsPayload's store constraint — it ripples into callers and test doubles for no behavior gain, and the parameter is already documented as retained for signature stability.

…t silent refusal Completes the schema-enforcement finding on #2467. ALLOWED_EVENT_KEYS was a CEILING with no floor: an event missing `taskId`, `type`, or `at` validated clean and was delivered. A subscriber keying derived state on `event.taskId` would then write under `undefined` rather than fail — the quiet-corruption mode this seam is supposed to be immune to. Added REQUIRED_EVENT_KEYS per type and a `missing-required-key` violation, with a case per declared event type. `undefined` counts as missing, not present. The emitters build payloads with conditional spreads so an unresolved field is absent, but a caller writing `{ taskId: maybeId }` produces the explicitly-undefined form; both are the same bug. Required keys are checked after the per-key pass so a payload that is both malformed and incomplete reports every reason at once. Required-ness is a sibling literal rather than derived from the allow-list, because it is exactly the part a type cannot express at runtime: `column` on NodeEntered and `fromColumn`/`toColumn` on RunSuspended are allowed but genuinely optional (a columnless node has no column), so the two lists differ on purpose. `runId`/`workflowId` stay optional — both are legitimately unresolvable at some emit sites, which is what U3's own workflowId fix relies on. ANTI-"BORN DEAD" GUARD. The bus refuses an invalid payload SILENTLY by design (the emitter is post-commit; throwing there would turn a shape bug into a lifecycle fault). Combined with stricter validation that means an emitter regression — a dropped nodeId, a renamed field, a stray `error` — would stop the event firing with no test failure anywhere, and every subscriber built on it would quietly never run. The boundary's real emits are now asserted end-to-end through the real bus: not "was emit called" (a spy passes on a refused payload) but "did a subscriber actually receive it", covering the column-bearing node, the columnless node, and the capacity suspension.

…aths (#2468) **Stacked on #2467** (base is `feature/workflow-owned-lifecycle`, not `main`). Phase A2 steps 1 and 2. **Step 3 — make one path authoritative and delete the other — is NOT done.** It is blocked on a measured divergence, escalated to the operator. This PR is the evidence that decision needs. ## The setup `moves.ts` branches on `useWorkflow = isWorkflowColumnsCompatibilityFlagEnabled(settings)`, which reads the raw `experimentalFeatures.workflowColumns` key. Nothing in production writes it, so the **inline branch is LIVE** and **`default-workflow-hooks.ts` is DEAD**. Because only one implementation runs, equivalence cannot be observed by running the suite normally — the dead path is never entered. Every case here forces both paths explicitly through one shared fixture and compares a 19-field observation, not "it moved". ## Step 1-2 result: side-effect equivalence is PROVEN Eight behaviors, field-by-field identical across both paths: | Behavior | Verdict | |---|---| | `in-progress → todo` user reopen field clears | equivalent | | Engine-source reopen does not set `userPaused` | equivalent | | `preserveStatus` keeps status/error | equivalent | | `preservePause` keeps an operator park (FN-7851) | equivalent | | Timing / `cumulativeActiveMs` across exit and re-entry | equivalent | | `preserveResumeState` step progress | equivalent | | `preserveWorktree` | equivalent | | Default worktree clear on reopen | equivalent | ### Why this is a proof and not a green suite Two independent guards, both of which caught a real silent failure in this PR's own development: - **The forcing mechanism is self-checked.** `assertPathActive` probes an undeclared target column — whose rejection message differs per path — before every case. The first version of this suite wrote the flag with `updateSettings` instead of `updateGlobalSettings` (`experimentalFeatures` is global-scoped, which is exactly why `moves.ts` reads it through `getSettingsFast()`), and reported **nine passing "equivalence" cases while running the inline path twice**. The check then caught a second failure: `updateGlobalSettings` *merges*, so resetting with `{}` left a previous `true` in place and leaked the hooks path into seven cases that believed they were on inline. - **The suite is mutation-tested.** Deleting `task.blockedBy = undefined` from `applyResetOnEntryEffects` fails the reopen case, naming the field. Restored before commit. Timestamps are compared by presence rather than value — the two runs happen at different wall-clock instants by construction — but a path that forgets to stamp `executionCompletedAt`, or wrongly clears `firstExecutionAt`, still fails. ## ⚠️ Read this before writing any both-paths test **A differential harness that cannot prove which path it is on will report a tautology, confidently, and in green.** This suite hit that twice in one afternoon: 1. **Global-scoped key written to project scope.** The first version set the flag with `updateSettings`. `experimentalFeatures` is **global**-scoped — which is exactly why `moves.ts` reads it through `getSettingsFast()` (merged global + project). The write was silently accepted and never reached `useWorkflow`. Result: **nine passing "equivalence" cases while running the inline path twice.** 2. **Merge-on-write leaking a stale `true`.** `updateGlobalSettings` *merges*, so resetting with `{ experimentalFeatures: {} }` left the previous `workflowColumns: true` in place. Result: the hooks path leaked into **seven cases that believed they were on inline.** Neither failure produced a red test. Both were caught only by `assertPathActive` — a per-case probe that moves a task to an undeclared column and asserts on the rejection *message*, which differs per path (`Valid targets: …` inline vs `Unknown column for this workflow` on hooks). This is the same failure class as a spy passing on a refused payload (see #2467): **the observation confirms the assumption instead of the behavior.** The rule that generalizes: > When a test forces a code path, assert that the path is active using a signal only that path can produce — before every case, not once in setup. A forcing mechanism that can fail silently makes every assertion downstream worthless. Any future work touching both move paths needs this probe or it will get a confident wrong answer. ## Why step 3 is blocked ### Divergence: rejection type and message | | Inline (live) | Hooks (dead) | |---|---|---| | Validates against | legacy `VALID_TRANSITIONS` | the task's own workflow | | Throws | bare `Error` | `TransitionRejectionError` with a machine-readable `rejection` | | Message | `Valid targets: …` | `Unknown column for this workflow` | Both reject, so neither is "broken" — but they are not interchangeable. Making either authoritative changes what every catch site observes, including the flag-OFF characterization suite that pins the bare-Error contract and the callers that branch on `rejection.code`. ### Unproven, recorded as an honest negative: in-transaction capacity The capacity block sits inside `if (useWorkflow && workflowIr && fromColumn !== toColumn)`, so it **cannot** run on the live path. The natural inference is "convergence turns store-level capacity rejection on for every project at once" — a serious blast radius, since `capacity-exhausted` is what the graph column boundary parks on and what the promote route surfaces to operators. **That inference did not survive measurement.** With `maxConcurrent: 1` and an already-occupied wip column, the second move was **accepted on both paths**. Something further in — `resolveColumnCapacity`'s limit resolution, or what `countActiveInCapacitySlotAsync` counts as an occupant — keeps the check from firing even flag-on. This suite does not establish which. So the capacity blast radius is **unquantified, not absent**. The test pins today's observed behavior so the investigation starts from a fact rather than from the code reading; if a future change makes it reject, that failure is the signal to reopen the question. ## Verification - 10/10 green against real PostgreSQL, with the path flip proven live by `assertPathActive` on every case. - Mutation-tested (see above). - `pnpm lint` and `tsc --noEmit` (core) green. **Not verified:** whether the capacity gate would activate under some other configuration; the plugin column-gate and post-commit plugin-hook divergences (also inside the `useWorkflow` gate) are identified structurally but not characterized here. 🤖 Generated with [Claude Code](https://claude.com/claude-code)  ## Summary by CodeRabbit * **New Features** * Workflow lifecycle updates now provide more consistent task state notifications and workflow activity handling. * Task lists display resolved workflow column names and lifecycle details more reliably. * **Bug Fixes** * Workflow-based task promotion and board views no longer depend on an obsolete feature setting. * Improved recovery for tasks left in transitional states. * Preserved task status, pause, progress, timing, and worktree behavior across workflow transitions. * **Reliability** * Added stronger validation and durable handling for workflow events and follow-up processing.  --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ratcheted, not fixed (#2469) **Stacked on #2468**, which is stacked on #2467. Base is `feature/workflow-move-path-convergence`. **Answer: the A2 observation was real. The documented invariant is broken.** Not a false alarm, not a harness artifact. `workflow-capacity.ts` states enforcement "runs INSIDE `moveTaskInternal`'s transaction and is **NEVER bypassable** (not a guard — runs regardless of bypassGuards/recoveryRehome/moveSource)". It does not hold for default-workflow tasks, for two independent reasons. ## R1 — Pool-id sentinel mismatch (the defect) | Site | Sentinel for "no workflow selection" | |---|---| | `moves.ts:319` (in-txn check, **asks**) | `?? "builtin:coding"` | | `countActiveInCapacitySlotAsyncImpl` (**answers**) | `?? DEFAULT_WORKFLOW_POOL_ID` → `"__default-workflow__"` | | `hold-release.ts:116` (sweep, second enforcement point) | `?? DEFAULT_WORKFLOW_POOL_ID` ✅ | The check asks for occupants of a pool that no occupant is ever bucketed into, so the count comes back `0` and the limit can never bind. The sweep is correct, so the two enforcement points **disagree about pool identity** — precisely what the module docstring says is impossible ("the two enforcement points can never disagree on what a limit *is*, only on the live count"). Note the shape of the bug: it is not a missing check. The check runs, queries correctly, and returns a confidently wrong answer. ## R2 — The `useWorkflow` gate The whole block sits inside `if (useWorkflow && workflowIr && fromColumn !== toColumn)` (`moves.ts:921`), and `useWorkflow` reads the raw `experimentalFeatures.workflowColumns` key nothing in production sets. **On the live path the check cannot run at all**, so R1 is latent today and becomes reachable the moment A2 converges onto the flag-ON side. ## How this was established, not guessed Three of my assumptions failed earlier in this program, so this one is pinned by a **discriminating experiment** rather than a code reading: | Case | Path | Selection | Result | |---|---|---|---| | DEFECT (R2) | inline (live) | none | accepted — check cannot run | | DEFECT (R1) | hooks | none | accepted — sentinels disagree | | **DISCRIMINATOR** | hooks | explicit `builtin:coding` | **refused, `capacity-exhausted`** | The third case changes nothing but sentinel agreement. That rules out "capacity is simply not wired" and isolates the cause to the mismatch. Both-path forcing reuses A2's `assertPathActive` probe. Without it this suite would silently run one path twice and report a tautology — the failure mode that produced sixteen false passes across A2's two harness bugs. ## The deliverable: an invariant ratchet The three cases above assert today's wrong behavior, so on their own they would let the defect live forever. A fourth case states the invariant **as written** and is marked `it.fails`: - **today** — the body fails, so `it.fails` passes; CI stays green while honestly recording the breach; - **when fixed** — the body passes, `it.fails` *fails*, forcing whoever lands the fix to flip it and the two `DEFECT:` expectations. That is "a test that fails if the invariant is broken" in the only shape that does not park a permanently-red test in CI. ## Blast radius (step 4) — why I did not fix it The fix is one line: make `moves.ts:319` use `DEFAULT_WORKFLOW_POOL_ID`, matching the sweep. The consequences are not one line. **Today: zero.** `useWorkflow` is false everywhere, so the corrected check still cannot run on the live path. The sentinel fix is safe to land in isolation. **At A2 convergence: every default-workflow task move into `in-progress` becomes capacity-checked against `maxConcurrent` (default 2), for the first time.** Affected movers: - **The graph column boundary** catches `capacity-exhausted` and *parks the run*. Runs that previously proceeded would begin suspending — this is a scheduling behavior change across every project, not an error path. - **`executor.ts`, `project-engine.ts`, `pr-comment-handler.ts`** each move tasks into `in-progress` and would begin seeing a rejection they have never seen. - **Operator drags and the promote route** would start refusing beyond `maxConcurrent`. Scheduler-side admission (`maxConcurrent`) is a separate, still-live control, so this is not "capacity is unenforced today" — it is "the store-level check the graph and promote paths are written against returns 0 and never binds". **Recommendation:** land the sentinel fix on its own (provably inert today), with the ratchet flipped in the same commit, *before* A2 convergence — so convergence does not simultaneously switch paths and switch on a previously-dead enforcement. ## Verification 4 cases green against real PostgreSQL (3 passed + 1 expected-fail), path flip proven live on every case. `pnpm lint` and `tsc --noEmit` green. ## Follow-up: both "not verified" items are now answered Recorded here rather than left as open questions, since this is where anyone investigating capacity will look. **1. Does the sync/SQLite counter carry the same mismatch? — YES, identically, but it is unreachable.** `countActiveInCapacitySlotSyncImpl` (`project-store-ops.ts:767`) buckets rows the same way as the async one: ```ts const effectiveWorkflowId = row.wid ?? TaskStore.DEFAULT_WORKFLOW_POOL_ID; ``` So it disagrees with `moves.ts:319` in exactly the same way. **However** its only caller is the public `TaskStore.countActiveInCapacitySlotSync` wrapper, which has no in-repo caller at all — it is dead API surface. The mismatch is real but currently unreachable, which makes it a landmine for whoever wires it up rather than an active defect. Fixing the sentinel should fix both call sites together. **2. Are custom workflows with an explicit numeric `limit` affected? — NO, and this is already proven by the discriminator above.** The mismatch fires **only when the selection row is absent** — that is what the `??` fallback is for. A custom workflow necessarily *has* a selection row; that is what makes it custom. The DISCRIMINATOR case adds an explicit selection and the rejection appears, which is exactly the custom-workflow shape. So the defect is scoped to **no-selection (default-workflow) tasks only**. The explicit-numeric-`limit` question turns out to be orthogonal: `resolveColumnBudgetKey` returning `col:${columnId}` decides *which columns share a budget*, not which pool id is passed to the counter. It does not interact with the sentinel at all. Net effect on blast radius: **narrower than first stated.** Only default-workflow (no-selection) tasks slip the limit today. Custom-workflow tasks are already enforced — meaning the fix does not switch enforcement on for them, it only closes the gap for the default workflow. 🤖 Generated with [Claude Code](https://claude.com/claude-code)  ## Summary by CodeRabbit * **Tests** * Added coverage for workflow column capacity enforcement during transactions. * Documented scenarios where capacity limits are bypassed, including tasks without workflow selection. * Verified that explicit workflow selection correctly rejects moves when capacity is exhausted. * Added a tracked failing test for the expected invariant once enforcement is corrected.

…guards, red-green) (#2479) **Stacked on #2469** → #2468 → #2467. Base is `feature/workflow-capacity-ground-truth`. This is **slice B1 of Phase B, not all of Phase B.** Sizing escalation sent separately; the census is below. ## Why this is a slice Measured census of code lines referencing a lifecycle column literal (comments excluded): | Unit | Files | Sites | |---|---|---:| | U4 | `self-healing.ts` | 203 | | U5 | `executor.ts` 171, `scheduler.ts` 55, `replan-target.ts` 20, `merger-ai.ts` 5, `hold-release.ts` 4, `mesh-lease-manager.ts` 4, `task-agent-sync.ts` 3 | 262 | | U6 | `moves.ts` 34, `default-workflow-hooks.ts` 13, `board-config.ts` 9, `blocker-fanout.ts` 6, `task-priority.ts` 5, `dependency-blocked-todo-report.ts` 2, `stale-paused-todo.ts` 1 | 70 | | | **Total** | **535** | The plan's "~207" counts the guard category only. Under the phase's non-negotiable rule — a test that **fails before** conversion, per guard — that is ~200 red-green cycles. Doing it as one sweep would reproduce exactly the failure this phase exists to prevent: converted guards nobody proved still fire. `moves.ts` and `default-workflow-hooks.ts` stay **parked** per the dispatch constraint (move-path convergence and the pool-id sentinel are on an operator decision). ## Guards converted (4), each red-green Every case below was written **first** and observed failing against the literal implementation. | Module | Guard | Before → After | |---|---|---| | `stale-paused-todo.ts` | stall detection | `column !== "todo"` → resolved **hold** column | | `blocker-fanout.ts` | active | `ACTIVE_COLUMNS.has(col)` → `!terminalColumns.has(col)` | | `blocker-fanout.ts` | hold-wait metric | `col === "todo"` → resolved **hold** column | | `task-priority.ts` | unblock active | `UNBLOCK_ACTIVE_COLUMNS` **deleted**, folded into the terminal set | Three of the seven new cases are **regression floors** that pass before and after. One of them earned its keep immediately: it failed on my own fixture (`activeCount` vs the public `totalCount`), catching a bad test rather than bad code — which is the point of asserting the default path alongside the renamed one. ### The `task-priority` finding `UNBLOCK_ACTIVE_COLUMNS` and `DONE_COLUMNS` encoded **one concept twice**, two lines apart, and disagreed for any custom column: dependency counting treated a `drafting` card as unmet (correct) while the active check treated it as inactive (wrong), zeroing the blocker's unblock weight. The enumeration wasn't just legacy-shaped — it contradicted its own neighbour. ## ⚠️ Behavior change, not a pure refactor Inverting active from enumeration to exclusion means **a card in a column that is neither terminal nor in the legacy enum now counts as active where it previously did not.** That is the plan's stated intent, but it is a real change for any project already using a custom column — **Coding (Ideas)' `ideas` column is the in-tree case.** Fan-out counts and unblock weights for such cards will rise. ## Verification - Four affected suites green (45 tests), each conversion observed red→green. - `pnpm lint`, `tsc --noEmit` (core) green. **Not verified / not done, stated plainly:** - **Call sites are not wired.** These modules now *accept* resolved roles; every parameter still defaults to the legacy set, so at the call sites the vocabulary is unchanged. A caller that cannot resolve a workflow keeps literal behavior. Threading `resolveLifecycleColumns` through `reads.ts` and `self-healing.ts` is follow-on work — until then the guards are *convertible*, not *converted end-to-end*. - `dependency-blocked-todo-report.ts` and `board-config.ts` are untouched in this slice. - 19 core-suite failures exist on this branch; all confirmed **pre-existing** by stashing and re-running on a clean tree (`duplicate-guard`, `log-severity-spam-contract`, `settings-parity`, `task-delete-caller-attribution`, `settings-defaults`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- **Supersedes #2470**, which GitHub force-closed when its base branch was deleted by the merge of #2469 and refuses to reopen. Same head branch, same commits (rebased onto `main`), now based on `main` directly. The two P1 review threads on #2470 were resolved there — one of them with a correction noting the threading half landed in code that was subsequently deleted as a dead feature in #2477.  ## Summary by CodeRabbit * **Bug Fixes** * Dependency and blocker reports now correctly recognize custom hold, active, and terminal workflow columns. * Blockers in renamed terminal columns are no longer incorrectly reported as active. * Stale paused-task badges and self-healing now work with workflow-specific hold columns. * Mixed boards with different workflow column names are handled consistently. * Existing default workflow behavior remains compatible, including fallback handling when workflow details cannot be resolved. * **Enhancements** * Reporting and task-priority calculations now support configurable single or multiple hold and terminal columns.

…d the definitive answer on the raw flag (2 reads left, both U2b's) (#2535) ## U12 part 8 — deletes the lossy `normalizeColumn`, and ratchets it shut Independent of the #2525 → #2528 → #2530 stack; touches only `@fusion/core` exports. This closes **one of the two `@deprecated (workflowColumns, U12)` markers** the unit was named for. ### The hazard `normalizeColumn` coerced an arbitrary value to a **legacy** column, rewriting every workflow-defined custom id to `triage`. Silent data loss for any project whose workflow declares a column outside the six built-ins — and it sat one line away from `normalizeColumnId`, which sanitises structurally and passes real ids through. The dashboard picked the wrong one for its entire task-ingest path until that was diagnosed; `useTasks.ts` and `routes-trait-rekey.test.ts` still carry the notes from that fix. So this is not a hypothetical footgun — it already fired once, on the surface where it mattered most. Deleted rather than left deprecated because it has **zero callers anywhere in the workspace**. It was pure exported hazard: a lossy coercion next to its safe twin, waiting to be picked again. ### The ratchet is the point `no-lossy-column-coercion-export.test.ts` bans the **behaviour, not the identifier**: it walks every exported single-argument function whose name mentions "column" and fails if one maps a valid custom id onto a different legacy id. Re-adding `normalizeColumn` under any name trips it. Verified by actually reintroducing the function — **two of the three cases fail, including the name-agnostic one**. That last detail is what stops it being a guard that checks nothing. Coverage stated plainly: deleting an unused export has no behaviour to revert-check. The compile is the proof it had no callers; the ratchet is the proof it cannot return. --- ## Answering the standing question: does anything still read the raw `workflowColumns` flag? **Yes. Exactly two sites, and both are U2b's.** I am not able to close this out, and here is the complete list rather than a summary: ``` packages/core/src/store.ts:38,43 ← the definition packages/core/src/task-store/moves.ts:9,363 ← `useWorkflow` packages/core/src/task-store/workflow-task-create-ops.ts:11,351 ← move-policy preflight ``` That is the whole list in production code. Everything else that greps is a comment, a test that writes the flag deliberately to exercise the dead path, or the unrelated `workflowColumns.*` i18n namespace for the Columns editor panel. **Why I have not deleted the settings key.** It cannot go while those two read it — the key is what they read. And the two are not separable from each other: `workflow-task-create-ops.ts:351` computes the `movePolicyPreflight` that `moves.ts` consumes and validates, and un-gating the preflight alone would start evaluating workflow move policies (with their plugin-gate side effects) while the branch that consumes the result stays off. That is a behaviour change with no consumer, which is worse than either state. **Status of the blocker.** U2b has not landed. `main` at `919f68f9b` still has both reads; the program's merged history goes `#2466 → #2467 → #2468 (characterisation only) → #2469 → #2479 → #2500 → #2512 → #2513`, with no convergence PR. PR #2468 was Phase A2 **steps 1–2 only** — the differential characterisation — and the convergence that deletes one of the two move paths was never merged. So the honest state of the unit: everything U12 owns is done except the two reads that U2b owns, and the settings key that cannot be deleted until they are gone. If you want me to take U2b itself, say so — I have the inventory and the divergence list, and I would want the current U2b worker stood down from `moves.ts` first. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…st read goes — answer: 2 reads left, key cannot be deleted (#2537) ## U12 part 9 — the flag census now answers itself Independent of the #2530 rebase; adds one test file, no production changes. ## The answer, first: NO, the settings key cannot be deleted yet **Three files reference the raw flag on current main (`3ff98aae5`):** ``` packages/core/src/store.ts ← declares it packages/core/src/task-store/moves.ts:363 ← U2b: `useWorkflow` packages/core/src/task-store/workflow-task-create-ops.ts:351 ← U2b: move-policy preflight ``` Everything else that greps is a comment, a test writing the flag deliberately to reach the dead path, or the unrelated `workflowColumns.*` i18n namespace for the Columns editor panel. **Why I can't remove them.** Both are on the move path and belong to **U2b**, which carries an equivalence-proof obligation because the two move implementations it arbitrates have never both run in production. They are also **not separable from each other**: `workflow-task-create-ops.ts:351` computes the `movePolicyPreflight` that `moves.ts` consumes and validates, so un-gating it alone would start evaluating workflow move policies — with their plugin-gate side effects — while the branch consuming the result stays off. That is a behaviour change with no consumer, which is worse than either end state. **U2b has not landed.** Program history on main runs `#2466 → #2467 → #2468 → #2469 → #2479 → #2500 → #2512 → #2513 → #2525 → #2528 → #2535`. #2468 was Phase A2 **steps 1–2 only** — the differential characterisation. No convergence PR exists. ## Why this is a PR and not another status message You have asked this question three times. I have answered it three times by grepping, and each answer was a number nobody could re-derive later — including me, which is why I re-ran the audit from scratch each time. That is exactly the shape this program keeps finding: a fact everyone believes, maintained by nobody. So the census is now a test. It **fails in both directions**, deliberately: - **A new read appears** → someone re-gated behaviour on a flag that is `false` for every real project, so the feature behind it will not run. That is the defect class U12 spent its length finding (the capacity gate, the U5 guards, the move policies — all looked enforced, none were). - **The last read disappears** → U2b has landed, and the settings key can finally go. The removal steps are written at the assertion. The second case is the one that matters. It converts "remember to delete the settings key someday" into a failing test at the exact moment that becomes possible, instead of a note in a PR body that ages out. ## Verified in both directions, not assumed - Adding a reference in `lifecycle-ops.ts` → fails with `+ "packages/core/src/task-store/lifecycle-ops.ts"`. - Dropping `moves.ts` from the allowlist → fails with `+ "packages/core/src/task-store/moves.ts"`. Equality rather than subset is what makes the second case possible; a subset check would let the last reader vanish silently and leave the key orphaned forever. Two supporting assertions, both there because of failure modes this program has already hit: - **No production code WRITES the key.** That is the premise the entire unit rests on — if a writer appears, every "this branch is unreachable" conclusion in U12 needs revisiting. - **The scan sees >200 files.** A broken path glob would otherwise make every assertion vacuously green: a guard reporting success without checking anything. ## Verification `pnpm test:gate` (414 + 10 + 71), `pnpm lint`, `pnpm verify:fast`, core typecheck green. ## Standing offer If you want U12 actually closed rather than ratcheted, the remaining work is U2b's convergence. I have the inventory and the divergence list its characterisation suite does not yet cover (plugin column gates, the `transitionPending` marker, `workflowId` in `task:move` run-audit, move-policy preflight). I would want the current U2b worker stood down from `moves.ts` first — two writers on the file this whole program pivots on is the one hazard I would not take on my own authority.  ## Summary by CodeRabbit * **Tests** * Added a new automated Vitest “census ratchet” to ensure only an approved, fixed set of production reads is made for the workflow columns compatibility flag. * Added checks that disallow hardcoded `workflowColumns: true/false` assignments in production sources. * Added allowlist validation, including per-file occurrence counts, required rationale text length, and confirmation that referenced files exist.  --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

The committed snapshot weighted six test files that no longer exist. Each traces to a deliberate deletion — #2461 (meta-task auto-archive), #2467 (workflow-owned lifecycle foundation), #2477 (dependency-blocked-todo feature) — and the snapshot kept their durations. Why it matters beyond a red test: the planner distributes work by these durations, so phantom weight is handed to a shard that has no such file to run. The shard finishes early while its siblings carry the real load, and the watchdog budget is derived from the same numbers — the shape behind the Full Suite SIGKILLs that looked like hangs and were undercounted budgets. Data hygiene only, no judgment: an entry is removed if and only if its path does not exist. No durations were edited and capturedAt is untouched and inside the staleness budget. The guard was working correctly the whole time — it named all six. It is the manifest-style failure in reverse: here the data was stale and the assertion was right. Positive control: adding one phantom path back fails the guard (11 pass / 1 fail), so this still catches the next deletion that forgets its timings. Fusion-Task-Id: KB-SELF-HEALING-QUERIES

…tom weight skews shard balance and watchdog budgets) (#3043) Third of the seven `scripts/__tests__` failures I diagnosed on #3035. This one is the mirror image of the release-check manifest: there the assertion was stale; **here the assertion was right the whole time and the data rotted.** ## Six phantom paths The committed shard-timing snapshot weighted six test files that no longer exist. Every one traces to a deliberate deletion: | deleted in | files | |---|---| | #2461 — meta-task auto-archive removal | `meta-chain-auto-close`, `meta-archive-guard-composition`, `self-healing-meta-archive-guards` | | #2467 — workflow-owned lifecycle foundation | `workflow-parity` | | #2477 — dependency-blocked-todo removal | `dependency-blocked-todo-report`, `dependency-blocked-todo-reporter` | The features went; their durations stayed. ## Why this is more than a red test The shard planner distributes work **by these durations**. Phantom weight is handed to a shard that has no such file to run: that shard finishes early while its siblings carry the real load. The watchdog budget is derived from the same numbers — which is the shape behind Full Suite runs being SIGKILLed at budgets that looked like hangs and were actually undercounted. ## What I changed, and what I did not Data hygiene only: an entry is removed **if and only if its path does not exist**. No duration was edited, and `capturedAt` is untouched and still inside the staleness budget — so this is not a re-measurement smuggled in as a cleanup, and the freshness assertion keeps whatever teeth it had. I did **not** regenerate the snapshot. That needs a real full-suite measurement run, it would rewrite every number, and the drift here is six dead paths, not wrong timings. ## Positive control Adding a single phantom path back fails the guard (11 pass / 1 fail), naming the file. So it still catches the next deletion that forgets its timings — which, on this evidence, is the normal way this file rots. ## Verification `node --test scripts/__tests__/ci-test-shard-timings.test.mjs` **12 passed** · `pnpm test:gate` 161 + 13 + 487 + 71 · lint — green. Diff is 6 deleted lines. Remaining from the seven: the release-check manifest (measured on #3035 — 10 of 16 seams dead, needs sizing as a real unit), `verify-fast` and `engine-vitest-gate-policy` (list drift), `plugin-authoring-docs` (a TOC anchor for a heading containing `&`), `release-prompt-gate` (dry-run exit ordering).

gsxdsm added 3 commits July 27, 2026 00:09

coderabbitai Bot reviewed Jul 27, 2026

View reviewed changes

Comment thread packages/core/src/task-store/moves.ts

Comment thread packages/core/src/types/workflow-events.ts

Comment thread packages/engine/src/workflow-column-boundary.ts

Comment thread packages/engine/src/workflow-event-subscribers.ts Outdated

This was referenced Jul 27, 2026

Phase A2 (steps 1-2): differential characterization of the two move paths #2468

Merged

Phase A3: the in-txn capacity invariant does NOT hold — root-caused, ratcheted, not fixed #2469

Merged

coderabbitai Bot mentioned this pull request Jul 27, 2026

docs: correct KTD-6 — the "flag-off" move path is live, the hooks path is dead #2466

Merged

gsxdsm mentioned this pull request Jul 27, 2026

Phase B slice B1: lifecycle column roles in the U6 policy modules (4 guards, red-green) #2470

Closed

gsxdsm merged commit 4158cf1 into main Jul 27, 2026
7 checks passed

gsxdsm deleted the feature/workflow-owned-lifecycle branch July 27, 2026 20:30

gsxdsm mentioned this pull request Jul 27, 2026

Phase B slice B1: lifecycle column roles in the U6 policy modules (4 guards, red-green) #2479

Merged

coderabbitai Bot mentioned this pull request Jul 29, 2026

U7 PR4: triage discovers and releases by the task's own workflow columns (measured: 15 real sites, not the 50 a grep reports) #2517

Closed

gsxdsm mentioned this pull request Jul 31, 2026

fix(ci): prune six phantom paths from the shard-timing snapshot (phantom weight skews shard balance and watchdog budgets) #3043

Merged

coderabbitai Bot mentioned this pull request Aug 8, 2026

test(dashboard): align dependency link mocks with prompt refresh #3360

Open

Navigation Menu

Uh oh!

Phase A: workflow-owned lifecycle foundation (U1, U2, U3) - #2467

Phase A: workflow-owned lifecycle foundation (U1, U2, U3)#2467
gsxdsm merged 5 commits into
mainfrom
feature/workflow-owned-lifecycle

gsxdsm commented Jul 27, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 27, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

greptile-apps Bot commented Jul 27, 2026 •

edited

Loading

Important Files Changed

Sequence Diagram

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gsxdsm commented Jul 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

gsxdsm commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

U1 — Lifecycle-column resolution seam

U2 — Delete the pre-cutover parity machinery (delete-only)

⚠️ Finding: the third listed deletion was NOT dead

U3's emit point is on the LIVE path — the seam is not born dead

U3 — Post-commit event seam with a transactional outbox

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

greptile-apps Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gsxdsm commented Jul 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

gsxdsm commented Jul 27, 2026 •

edited

Loading

coderabbitai Bot commented Jul 27, 2026 •

edited

Loading

greptile-apps Bot commented Jul 27, 2026 •

edited

Loading