U12 part 2: bind the three U5 reconciliation guards — USER-VISIBLE (and one path that couldn't run under PostgreSQL at all) (#2512)
## U12 part 2 — the three U5 reconciliation guards now actually fire
USER-VISIBLE. Taken on standing authority; here is exactly what changed
for operators.
All three read the RAW `experimentalFeatures.workflowColumns` key via
`store.workflowColumnsFlagOn()`. Nothing in production writes it, so all
three have been inert since the workflow-columns cutover.
| Guard | Before (every real project) | After |
|---|---|---|
| Workflow edit removing an **occupied** column | Save succeeded; cards
left in a column the workflow no longer declares | Save fails with
`OccupiedColumnsError` unless `rehomeTo` is supplied |
| Workflow **delete** | Occupant capture returned `[]`; cards sat in the
deleted workflow's columns until the next engine start | Cards move to
the default workflow's entry column as part of the delete |
| Workflow **switch** | Never reconciled; the `reconciliation` field in
the declared return type was never populated | Card in an undeclared
column moves to the resolved target; a declared column is preserved |
Both consumers already handle the new outcomes and needed no change:
`register-workflow-routes.ts` maps `OccupiedColumnsError` to a
structured 409 carrying per-column occupant counts, and
`fn_workflow_update` returns a retryable structured result. The
dashboard editor's `rehomeTo` retry flow becomes reachable for the first
time. I only updated two stale "flag-ON" comments there — that code was
correct all along and simply never fired.
### What an operator actually sees (USER-VISIBLE — read this bit)
Four changes to what the board and the API do. Nothing here is silent.
1. **Editing a workflow to remove a column that has cards in it now
FAILS.** Previously the save succeeded and the cards were left in a
column their workflow no longer declared. The dashboard shows the
existing 409 with per-column occupant counts and prompts for a re-home
target; retrying with `rehomeTo` moves the cards and saves. Removing an
EMPTY column is unaffected.
2. **Deleting a workflow moves its cards immediately** to the default
workflow's entry column, instead of leaving them until the next engine
start.
3. **Switching a task's workflow moves the card** when the new workflow
does not declare its current column. A card whose column IS declared
stays exactly where it is. The API response now carries the
`reconciliation` summary it always promised.
4. **A switch whose re-home would be REJECTED is now refused before
anything is written.** If the destination column is at its WIP limit,
the switch fails with a structured 409 (`workflow-switch-rehome-failed`)
naming the task, both columns and the reason — and **nothing changes**:
the task keeps its current workflow AND its current column. Retry after
making room. Previously this combination committed the selection and
then silently reported a move that never happened, leaving selection and
column disagreeing.
**Can a torn card still happen? Yes, in one narrow case, and here is how
you recover.** If the destination fills in the window between the
pre-flight and the move, the selection is already committed and the card
ends up in a column its new workflow does not declare. That case is not
silent: it writes a `task:workflow-switch-torn` run-audit row, and the
error carries `selectionCommitted: true` with both columns. Recovery:
make room in the destination and move the card there, or switch the task
back — and if neither happens, the R7 startup sweep
`reconcileUndeclaredTaskColumns` re-homes it on the next engine start.
The card is never lost; it is visible in a lane the board may not draw
until one of those runs.
The one thing to watch after merge: (1) converts a previously-silent
success into a visible failure, so an operator mid-edit on a busy
workflow will start seeing a 409 they never saw before. That is the
point — the alternative was stranding their cards — but it is the change
most likely to generate a "this used to work" report.
### The thing that made this more than a gate removal
Un-gating the switch guard surfaced that
`selectTaskWorkflowAndReconcileImpl` read the task through
`store.readTaskFromDb` — the **synchronous SQLite** reader, which throws
under PostgreSQL:
```
TaskStore.db: SQLite Database is not available in backend mode
```
The flag returned before that line, so the gate was hiding a path that
**could not execute at all in the production backend**, not merely a
disabled feature. Ported to the async `readTaskRow`. Found by the new
tests, not by reading the code.
### Review round 2 (both findings real, both fixed)
**Torn write with no alarm — fixed by ORDERING, not by a louder
message.** My first attempt only made the error loud, which left the
torn state intact. The real fix is that the deterministic rejection
cause (destination at its WIP limit) is now checked BEFORE
`selectTaskWorkflow` commits, by resolving the target IR straight from
`workflowId` instead of through the task's selection. Nothing commits on
that path.
For the residual race the failure is loud AND recorded: `rehomeOccupant`
now returns `{ moved, error? }` (additive; sweep callers ignore it), the
switch writes a `task:workflow-switch-torn` run-audit row, and throws
`WorkflowSwitchRehomeFailedError` with `committed: true`. Consumers
translate it: the dashboard route returns a structured 409 with
`selectionCommitted`, and `fn_task_set_workflow` returns the same fields
— no more generic "something went wrong".
**Fabricated column for a deleted task.** My first fix fell back to
`fromColumn` when the final read found no row, so a task soft-deleted
mid-switch was reported as having its old column *preserved*. Absent now
reads as absent (the optional `reconciliation` is omitted). Extracted as
the pure `buildSwitchReconciliation` seam because the window is not
reachable through the public call — `selectTaskWorkflow` rejects an
already-deleted task up front — so it is a genuine race, and I test the
decision directly rather than asserting it from reading the code.
### Revert-proof, measured
New `workflow-reconciliation-production-shape.pg.test.ts` — 6 cases,
with the flag **never written**, which is the configuration every real
project has. Each flip reverted individually:
- re-gate the edit guard → **2 failures** (OccupiedColumnsError case;
rehomeTo re-home case)
- re-gate the delete capture → **1 failure** (card stays in
`custom-hold`)
- restore the switch early return → **2 failures** (`reconciliation`
undefined; card does not move)
- all three in place → **6/6 green**
Round-2 fixes, also measured:
- restore the `fromColumn` fallback → the "row is gone" case fails
(reports `preserved: true` for a deleted task)
- drop the `!outcome.moved` throw → the capacity-blocked case fails
(resolves instead of raising)
- **move the capacity pre-flight back AFTER the commit → the case fails
on the SELECTION assertion** (expected `WF-002`, received `WF-001`),
i.e. it proves the ordering, not the wording
The pre-existing coverage in `workflow-authoritative-reads.pg.test.ts`
reached the occupied-column guard by **writing the flag ON itself** —
same pattern as the ListView/Board suites in part 1. Its flag write is
removed; it now runs in the production shape.
### Where I nearly got this wrong
My first revert harness was buggy and I briefly concluded the delete
re-home was **redundant** — I had probed the stored column and seen
`triage` with what I thought was the flip reverted. It wasn't.
`workflow-ops.ts` contains two identical `const occupantTaskIds = await
store.listWorkflowOccupantTaskIds(id, false)` lines (field-reconcile
block, delete path), so my first-match edit reverted the wrong one.
Re-run anchored on surrounding context, the delete case fails as
predicted. Recorded in the test header as a caution. I also chased and
**refuted** a scarier hypothesis along the way — that an unrelated
`updateTask` coerces a custom column back to `triage`. It does not; the
column survives.
### Deliberately NOT in this PR
The v1-IR rollback-compat persistence (`downgradeIrToV1IfPure`) on the
workflow UPDATE path. It shared the same `flagOn` variable, which is how
it surfaced: **one flag read was feeding two unrelated decisions, so the
flag has more decision sites than call sites** — my earlier 9-site
inventory undercounted. It chooses the stored *shape* of the graph
rather than gating a guard, so it is a persistence-format change with a
different blast radius. It now reads the flag explicitly, behaviour
unchanged, for a follow-up.
The `moves.ts` group remains U2b's.
### Verification
`pnpm test:gate` (307 + 10 + 71), `pnpm lint`, `pnpm verify:fast` (17
steps), both typechecks green. Full `packages/core` PostgreSQL suite:
**1042 passed, 3 failed** — `central-archive-secrets.test.ts`
(log-prefix assertion) and
`workflow-settings-project-identity.pg.test.ts` (×2, project-id
resolution). I confirmed the identical 3 failures on a stashed clean
tree: pre-existing, unrelated. No Fusion instance booted.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Workflow edits now prevent removal of occupied columns unless cards
are moved to a specified destination.
* Cards are automatically re-homed when workflows are deleted or
switched.
* Workflow switches now check destination capacity before committing and
provide clear conflict details when re-homing fails.
* Reconciliation results now indicate whether cards were moved or
preserved.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
7
.changeset/u12-u5-reconciliation-guards.md
Normal file
7
.changeset/u12-u5-reconciliation-guards.md
Normal file
@@ -0,0 +1,7 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
summary: Workflow edits, deletes, and switches now reconcile the cards sitting in the affected columns.
|
||||||
|
category: fix
|
||||||
|
dev: The three U5 guards (`updateWorkflowDefinition` occupied-column block, `deleteWorkflowDefinition` occupant re-home, `selectTaskWorkflowAndReconcile` switch reconciliation) were gated on the retired raw `experimentalFeatures.workflowColumns` key and had never fired in production. Removing an occupied column now returns a 409 `OccupiedColumnsError` unless `rehomeTo` is supplied; deleting a workflow re-homes its cards immediately rather than at next engine start; switching workflows moves a card whose column the new workflow does not declare and returns a `reconciliation` summary. Also ports the switch path off the synchronous SQLite reader, which throws under PostgreSQL.
|
||||||
@@ -1444,6 +1444,30 @@ export default function kbExtension(pi: ExtensionAPI) {
|
|||||||
await store.selectTaskWorkflowAndReconcile(task.id, workflowId);
|
await store.selectTaskWorkflowAndReconcile(task.id, workflowId);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
const message = error instanceof Error ? error.message : String(error);
|
const message = error instanceof Error ? error.message : String(error);
|
||||||
|
/*
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — PR #2512 review):
|
||||||
|
TRANSLATE the switch re-home failure here too. `fn_task_update` is a
|
||||||
|
second switch consumer alongside `fn_task_set_workflow`, and a bare
|
||||||
|
message string gives the caller no way to tell "nothing changed, retry
|
||||||
|
after making room" from "the selection committed and the task is now
|
||||||
|
INCONSISTENT" — which is exactly the distinction that decides whether it
|
||||||
|
may treat the switch as done.
|
||||||
|
*/
|
||||||
|
const typed = error as { name?: string; committed?: boolean; taskId?: string; workflowId?: string; fromColumn?: string; intendedColumn?: string };
|
||||||
|
if (typed?.name === "WorkflowSwitchRehomeFailedError") {
|
||||||
|
return {
|
||||||
|
content: [{ type: "text", text: `ERROR: ${message}` }],
|
||||||
|
isError: true,
|
||||||
|
details: {
|
||||||
|
code: "workflow-switch-rehome-failed",
|
||||||
|
taskId: typed.taskId,
|
||||||
|
workflowId: typed.workflowId,
|
||||||
|
fromColumn: typed.fromColumn,
|
||||||
|
intendedColumn: typed.intendedColumn,
|
||||||
|
selectionCommitted: typed.committed === true,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
return {
|
return {
|
||||||
content: [{ type: "text", text: `ERROR: ${message}` }],
|
content: [{ type: "text", text: `ERROR: ${message}` }],
|
||||||
isError: true,
|
isError: true,
|
||||||
|
|||||||
@@ -31,7 +31,14 @@ pgDescribe("PostgreSQL workflow authoritative reads", () => {
|
|||||||
|
|
||||||
it("blocks removal of a PostgreSQL-occupied workflow column", async () => {
|
it("blocks removal of a PostgreSQL-occupied workflow column", async () => {
|
||||||
const store = h.store();
|
const store = h.store();
|
||||||
await store.updateGlobalSettings({ experimentalFeatures: { workflowColumns: true } });
|
/*
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — R9):
|
||||||
|
The `experimentalFeatures: { workflowColumns: true }` write is DELETED. It was the
|
||||||
|
only way this test reached the guard, and it is a configuration no production
|
||||||
|
project has — so this case passed while the guard was inert for every real
|
||||||
|
operator. The guard is no longer flag-gated, so the test now runs in the
|
||||||
|
production shape and means what it always claimed to mean.
|
||||||
|
*/
|
||||||
const ir = workflowWithCustomColumn();
|
const ir = workflowWithCustomColumn();
|
||||||
const workflow = await store.createWorkflowDefinition({ name: "Occupancy", ir, layout: {} });
|
const workflow = await store.createWorkflowDefinition({ name: "Occupancy", ir, layout: {} });
|
||||||
const task = await store.createTask({ description: "occupies custom column" });
|
const task = await store.createTask({ description: "occupies custom column" });
|
||||||
|
|||||||
@@ -0,0 +1,241 @@
|
|||||||
|
/*
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — R9, R12):
|
||||||
|
The three U5 (R20) workflow-lifecycle reconciliation guards, exercised in the
|
||||||
|
PRODUCTION SHAPE — that is, with `experimentalFeatures.workflowColumns` NEVER
|
||||||
|
written by the test.
|
||||||
|
|
||||||
|
WHY THAT MATTERS, and why this file is separate from
|
||||||
|
`workflow-authoritative-reads.pg.test.ts`. Every one of these guards used to be
|
||||||
|
gated on `store.workflowColumnsFlagOn()`, which reads the RAW
|
||||||
|
`experimentalFeatures.workflowColumns` key. No production writer sets that key, so
|
||||||
|
all three were inert for every real project:
|
||||||
|
|
||||||
|
- removing an OCCUPIED column from a workflow silently succeeded, stranding the
|
||||||
|
cards in a column their workflow no longer declared;
|
||||||
|
- deleting a workflow captured an EMPTY occupant list, so its cards were left in
|
||||||
|
that workflow's columns until the next engine startup sweep;
|
||||||
|
- switching a task's workflow never reconciled its column, and the
|
||||||
|
`reconciliation` field the API promises was never populated.
|
||||||
|
|
||||||
|
The pre-existing coverage reached these guards by writing the flag ON itself, which
|
||||||
|
is precisely why the gap was invisible: the tests passed against a configuration no
|
||||||
|
operator has. Every case below therefore asserts through the PUBLIC store seams with
|
||||||
|
the flag ABSENT.
|
||||||
|
|
||||||
|
REVERT CHECK — each case fails if its flip is undone:
|
||||||
|
- restore `flagOn &&` on the edit guard -> "blocks a workflow edit ..." fails,
|
||||||
|
because the update resolves instead of rejecting with OccupiedColumnsError, and
|
||||||
|
"re-homes occupants ..." fails because the cards never move.
|
||||||
|
- restore `flagOn ? : []` on the delete capture -> "re-homes a deleted workflow's
|
||||||
|
occupants ..." fails, because the card stays in `custom-hold`.
|
||||||
|
- restore the `workflowColumnsFlagOn()` early return on switch -> both switch cases
|
||||||
|
fail, because `reconciliation` comes back undefined and the card does not move.
|
||||||
|
I ran each of those three reverts individually against this file and confirmed the
|
||||||
|
matching failures. Note for anyone repeating it: `workflow-ops.ts` contains TWO
|
||||||
|
identical `const occupantTaskIds = await store.listWorkflowOccupantTaskIds(id, false)`
|
||||||
|
lines — one in the field-reconcile block, one in the delete path — so a first-match
|
||||||
|
edit reverts the wrong one and the delete case then passes against what looks like
|
||||||
|
reverted code. Anchor on surrounding context. Measured output is in the PR description.
|
||||||
|
*/
|
||||||
|
import { afterAll, afterEach, beforeAll, beforeEach, expect, it } from "vitest";
|
||||||
|
import { BUILTIN_CODING_WORKFLOW_IR } from "../../builtin-coding-workflow-ir.js";
|
||||||
|
import type { WorkflowIrV2 } from "../../workflow-ir-types.js";
|
||||||
|
import {
|
||||||
|
createSharedPgTaskStoreTestHarness,
|
||||||
|
pgDescribe,
|
||||||
|
type SharedPgTaskStoreHarness,
|
||||||
|
} from "../../__test-utils__/pg-test-harness.js";
|
||||||
|
|
||||||
|
/** The built-in coding workflow plus one extra column, so a test can occupy a
|
||||||
|
* column that the DEFAULT workflow does not declare. */
|
||||||
|
function workflowWithCustomColumn(name: string): WorkflowIrV2 {
|
||||||
|
const ir = structuredClone(BUILTIN_CODING_WORKFLOW_IR) as WorkflowIrV2;
|
||||||
|
ir.name = name;
|
||||||
|
ir.columns.push({ id: "custom-hold", name: "Custom hold", traits: [] });
|
||||||
|
return ir;
|
||||||
|
}
|
||||||
|
|
||||||
|
pgDescribe("U5 workflow reconciliation guards — production shape (no workflowColumns flag)", () => {
|
||||||
|
const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
|
||||||
|
prefix: "fusion_u5_prod_shape",
|
||||||
|
});
|
||||||
|
|
||||||
|
beforeAll(h.beforeAll);
|
||||||
|
beforeEach(h.beforeEach);
|
||||||
|
afterEach(h.afterEach);
|
||||||
|
afterAll(h.afterAll);
|
||||||
|
|
||||||
|
/** Create a workflow with a custom column and park one task in it. */
|
||||||
|
async function seedOccupiedCustomColumn(workflowName: string) {
|
||||||
|
const store = h.store();
|
||||||
|
const ir = workflowWithCustomColumn(workflowName);
|
||||||
|
const workflow = await store.createWorkflowDefinition({ name: workflowName, ir, layout: {} });
|
||||||
|
const task = await store.createTask({ description: `occupies ${workflowName}` });
|
||||||
|
await store.selectTaskWorkflow(task.id, workflow.id);
|
||||||
|
await store.moveTask(task.id, "custom-hold", {
|
||||||
|
moveSource: "engine",
|
||||||
|
bypassGuards: true,
|
||||||
|
recoveryRehome: true,
|
||||||
|
});
|
||||||
|
expect((await store.getTask(task.id)).column).toBe("custom-hold");
|
||||||
|
return { store, ir, workflow, task };
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── Guard 1: workflow edit that removes an occupied column ────────────────
|
||||||
|
|
||||||
|
it("blocks a workflow edit that removes an occupied column, with no flag set", async () => {
|
||||||
|
const { store, ir, workflow } = await seedOccupiedCustomColumn("Edit guard");
|
||||||
|
|
||||||
|
const nextIr = structuredClone(ir);
|
||||||
|
nextIr.columns = nextIr.columns.filter((column) => column.id !== "custom-hold");
|
||||||
|
|
||||||
|
await expect(store.updateWorkflowDefinition(workflow.id, { ir: nextIr })).rejects.toMatchObject({
|
||||||
|
name: "OccupiedColumnsError",
|
||||||
|
workflowId: workflow.id,
|
||||||
|
});
|
||||||
|
|
||||||
|
// The rejection must be a real abort: the IR is unchanged, so a failed save
|
||||||
|
// cannot half-apply and leave the column gone with the cards still in it.
|
||||||
|
const after = await store.getWorkflowDefinition(workflow.id);
|
||||||
|
expect((after!.ir as WorkflowIrV2).columns.map((c) => c.id)).toContain("custom-hold");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("re-homes occupants into rehomeTo when the edit supplies one, with no flag set", async () => {
|
||||||
|
const { store, ir, workflow, task } = await seedOccupiedCustomColumn("Edit rehome");
|
||||||
|
|
||||||
|
const nextIr = structuredClone(ir);
|
||||||
|
nextIr.columns = nextIr.columns.filter((column) => column.id !== "custom-hold");
|
||||||
|
|
||||||
|
await store.updateWorkflowDefinition(workflow.id, { ir: nextIr, rehomeTo: "todo" });
|
||||||
|
|
||||||
|
// The card lands in the column the editor chose, not wherever it happened to sit.
|
||||||
|
expect((await store.getTask(task.id)).column).toBe("todo");
|
||||||
|
const after = await store.getWorkflowDefinition(workflow.id);
|
||||||
|
expect((after!.ir as WorkflowIrV2).columns.map((c) => c.id)).not.toContain("custom-hold");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("allows an edit that removes an UNOCCUPIED column, with no flag set", async () => {
|
||||||
|
const store = h.store();
|
||||||
|
const ir = workflowWithCustomColumn("Unoccupied");
|
||||||
|
const workflow = await store.createWorkflowDefinition({ name: "Unoccupied", ir, layout: {} });
|
||||||
|
|
||||||
|
const nextIr = structuredClone(ir);
|
||||||
|
nextIr.columns = nextIr.columns.filter((column) => column.id !== "custom-hold");
|
||||||
|
|
||||||
|
// No occupants -> no rejection, no rehomeTo required. This is the case that must
|
||||||
|
// NOT regress into a blanket "you may never remove a column" error.
|
||||||
|
await store.updateWorkflowDefinition(workflow.id, { ir: nextIr });
|
||||||
|
const after = await store.getWorkflowDefinition(workflow.id);
|
||||||
|
expect((after!.ir as WorkflowIrV2).columns.map((c) => c.id)).not.toContain("custom-hold");
|
||||||
|
});
|
||||||
|
|
||||||
|
// ── Guard 2: workflow delete ──────────────────────────────────────────────
|
||||||
|
|
||||||
|
it("re-homes a deleted workflow's occupants to the default entry column, with no flag set", async () => {
|
||||||
|
const { store, workflow, task } = await seedOccupiedCustomColumn("Delete guard");
|
||||||
|
|
||||||
|
await store.deleteWorkflowDefinition(workflow.id);
|
||||||
|
|
||||||
|
// Immediately after the delete — not at the next engine startup sweep — the card
|
||||||
|
// must be out of the vanished column and in the default workflow's entry column.
|
||||||
|
expect((await store.getTask(task.id)).column).toBe("triage");
|
||||||
|
|
||||||
|
});
|
||||||
|
|
||||||
|
// ── Guard 3: workflow switch ──────────────────────────────────────────────
|
||||||
|
|
||||||
|
it("reconciles a task's column when switching to a workflow that lacks it, with no flag set", async () => {
|
||||||
|
const { store, task } = await seedOccupiedCustomColumn("Switch source");
|
||||||
|
|
||||||
|
// The built-in coding workflow does not declare `custom-hold`, so switching to it
|
||||||
|
// must move the card rather than leave it in a lane the target cannot draw.
|
||||||
|
const target = await store.createWorkflowDefinition({
|
||||||
|
name: "Switch target",
|
||||||
|
ir: structuredClone(BUILTIN_CODING_WORKFLOW_IR) as WorkflowIrV2,
|
||||||
|
layout: {},
|
||||||
|
});
|
||||||
|
|
||||||
|
const result = await store.selectTaskWorkflowAndReconcile(task.id, target.id);
|
||||||
|
|
||||||
|
expect(result.reconciliation).toBeDefined();
|
||||||
|
expect(result.reconciliation!.preserved).toBe(false);
|
||||||
|
expect(result.reconciliation!.fromColumn).toBe("custom-hold");
|
||||||
|
expect((await store.getTask(task.id)).column).toBe(result.reconciliation!.toColumn);
|
||||||
|
expect((await store.getTask(task.id)).column).not.toBe("custom-hold");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("refuses the switch BEFORE committing the selection when the destination is full (PR #2512 review)", async () => {
|
||||||
|
const store = h.store();
|
||||||
|
|
||||||
|
/*
|
||||||
|
`rehomeOccupant` deliberately swallows a rejected move ("a full target column
|
||||||
|
rejects, which we audit and skip"), so the switch used to report the column it
|
||||||
|
ASKED for. Induce that: give the target workflow's entry column a WIP limit of 1
|
||||||
|
and fill it, so the re-home is rejected and the card stays put.
|
||||||
|
*/
|
||||||
|
const targetIr = structuredClone(BUILTIN_CODING_WORKFLOW_IR) as WorkflowIrV2;
|
||||||
|
targetIr.name = "capped-target";
|
||||||
|
const entry = targetIr.columns.find((c) => c.id === "triage")!;
|
||||||
|
entry.traits = [...entry.traits, { trait: "wip", config: { limit: 1 } }];
|
||||||
|
const target = await store.createWorkflowDefinition({ name: "Capped target", ir: targetIr, layout: {} });
|
||||||
|
|
||||||
|
// Occupy the single slot in the target workflow's entry column.
|
||||||
|
const filler = await store.createTask({ description: "fills the cap" });
|
||||||
|
await store.selectTaskWorkflow(filler.id, target.id);
|
||||||
|
expect((await store.getTask(filler.id)).column).toBe("triage");
|
||||||
|
|
||||||
|
// A card parked in a column the target workflow does not declare.
|
||||||
|
const { task } = await seedOccupiedCustomColumn("Capacity source");
|
||||||
|
|
||||||
|
const before = await store.getTaskWorkflowSelectionAsync(task.id);
|
||||||
|
|
||||||
|
/*
|
||||||
|
The ORDERING is the fix (PR #2512 review). The destination is full, so the switch
|
||||||
|
must be refused BEFORE the selection commits — leaving a consistent card — rather
|
||||||
|
than committing the selection and then discovering the re-home cannot happen.
|
||||||
|
*/
|
||||||
|
await expect(store.selectTaskWorkflowAndReconcile(task.id, target.id)).rejects.toMatchObject({
|
||||||
|
name: "WorkflowSwitchRehomeFailedError",
|
||||||
|
taskId: task.id,
|
||||||
|
workflowId: target.id,
|
||||||
|
fromColumn: "custom-hold",
|
||||||
|
intendedColumn: "triage",
|
||||||
|
committed: false,
|
||||||
|
});
|
||||||
|
|
||||||
|
// NOTHING was written: same column AND same workflow selection as before. This is
|
||||||
|
// the assertion that distinguishes the ordering fix from a louder error message —
|
||||||
|
// it fails if the selection is committed before the capacity pre-flight.
|
||||||
|
expect((await store.getTask(task.id)).column).toBe("custom-hold");
|
||||||
|
expect((await store.getTaskWorkflowSelectionAsync(task.id))?.workflowId).toBe(before?.workflowId);
|
||||||
|
expect((await store.getTaskWorkflowSelectionAsync(task.id))?.workflowId).not.toBe(target.id);
|
||||||
|
});
|
||||||
|
|
||||||
|
/*
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — PR #2512 review):
|
||||||
|
The soft-delete-mid-switch case is NOT here. `selectTaskWorkflow` rejects an
|
||||||
|
already-deleted task up front with `TaskDeletedError`, so the window between the
|
||||||
|
switch's first read and its final one cannot be driven from outside the call. It is
|
||||||
|
covered directly against the pure seam in
|
||||||
|
`__tests__/workflow-switch-reconciliation-report.test.ts`.
|
||||||
|
*/
|
||||||
|
it("preserves a task's column when the new workflow DOES declare it, with no flag set", async () => {
|
||||||
|
const store = h.store();
|
||||||
|
const task = await store.createTask({ description: "stays put" });
|
||||||
|
await store.moveTask(task.id, "todo", { moveSource: "engine", bypassGuards: true, recoveryRehome: true });
|
||||||
|
|
||||||
|
const target = await store.createWorkflowDefinition({
|
||||||
|
name: "Declares todo",
|
||||||
|
ir: structuredClone(BUILTIN_CODING_WORKFLOW_IR) as WorkflowIrV2,
|
||||||
|
layout: {},
|
||||||
|
});
|
||||||
|
|
||||||
|
const result = await store.selectTaskWorkflowAndReconcile(task.id, target.id);
|
||||||
|
|
||||||
|
// Reconciliation is not a licence to move every switched card — a declared column
|
||||||
|
// is left exactly where it is.
|
||||||
|
expect(result.reconciliation).toBeDefined();
|
||||||
|
expect(result.reconciliation!.preserved).toBe(true);
|
||||||
|
expect((await store.getTask(task.id)).column).toBe("todo");
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,41 @@
|
|||||||
|
/*
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — PR #2512 review, greptile P1):
|
||||||
|
Direct coverage of what a completed workflow switch REPORTS.
|
||||||
|
|
||||||
|
Why a unit test rather than a store-level one: the case that matters — the task is
|
||||||
|
soft-deleted BETWEEN the switch's first read and its final one — is not reachable
|
||||||
|
through the public call, because `selectTaskWorkflow` rejects an already-deleted task
|
||||||
|
up front with `TaskDeletedError`. It is a genuine race. Testing the decision directly
|
||||||
|
is honest; asserting it from reading the code is not.
|
||||||
|
|
||||||
|
REVERT CHECK: restore the old `afterRow ? String(afterRow.column) : fromColumn`
|
||||||
|
fallback and the "vanished row" case fails — it reports `{ preserved: true, toColumn:
|
||||||
|
fromColumn }`, fabricating a live column for a row that is gone.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import { buildSwitchReconciliation } from "../workflow-reconciliation.js";
|
||||||
|
|
||||||
|
describe("workflow switch reconciliation reporting", () => {
|
||||||
|
it("reports the ACTUAL column, not the intended one", () => {
|
||||||
|
// The re-home landed somewhere other than the source: report where it is.
|
||||||
|
expect(buildSwitchReconciliation("custom-hold", "triage")).toEqual({
|
||||||
|
preserved: false,
|
||||||
|
fromColumn: "custom-hold",
|
||||||
|
toColumn: "triage",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("reports preserved when the card did not move", () => {
|
||||||
|
expect(buildSwitchReconciliation("todo", "todo")).toEqual({
|
||||||
|
preserved: true,
|
||||||
|
fromColumn: "todo",
|
||||||
|
toColumn: "todo",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("omits the reconciliation entirely when the row is gone", () => {
|
||||||
|
// A soft-delete racing the switch leaves no readable row. Absent must read as
|
||||||
|
// absent — never as "preserved in its old column".
|
||||||
|
expect(buildSwitchReconciliation("todo", undefined)).toBeUndefined();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -471,6 +471,7 @@ export type { ColumnCapacity } from "./workflow-capacity.js";
|
|||||||
// ── U5: workflow lifecycle reconciliation (switch / edit / delete) ───────────
|
// ── U5: workflow lifecycle reconciliation (switch / edit / delete) ───────────
|
||||||
export {
|
export {
|
||||||
OccupiedColumnsError,
|
OccupiedColumnsError,
|
||||||
|
WorkflowSwitchRehomeFailedError,
|
||||||
InvalidRehomeTargetError,
|
InvalidRehomeTargetError,
|
||||||
IncompatibleFieldChangeError,
|
IncompatibleFieldChangeError,
|
||||||
resolveEntryColumnId,
|
resolveEntryColumnId,
|
||||||
|
|||||||
@@ -107,7 +107,7 @@ import { getTaskSelectClauseImpl2, createTaskPersistSerializationContextImpl, ge
|
|||||||
import { getTaskSelectClauseWithActivityLogLimitImpl, getChangedTaskColumnsImpl, getSoftDeletedWriteConflictImpl, readTaskJsonImpl, writeConfigImpl, _maybeAutoArchiveSameAgentDuplicateBackendImpl, updateBranchGroupImpl, updatePrEntityImpl, listTasksForGithubTrackingReconcileImpl, listTasksForGitlabTrackingReconcileImpl, renewCheckoutLeaseImpl, updateTaskAtomicImpl, getWorkflowPromptOverridesImpl, updateWorkflowSettingValuesImpl, rollbackConfigurationImpl, cancelActiveWorkflowWorkItemsForTaskImpl, setCompletionHandoffAcceptedMarkerImpl, reconcileLegacyAutoMergeStampsImpl, recoverExpiredMergeQueueLeasesImpl, rewriteDependentsForRemovalImpl, cleanupBranchForTaskImpl, addAttachmentImpl, deleteAttachmentImpl, registerArtifactImpl, updatePrInfoImpl, unlinkGithubIssueImpl, cleanupArchivedTasksImpl, generatePromptFromArchiveEntryImpl, listWorkflowOccupantTaskIdsImpl, listApprovedCliAutonomyAdaptersImpl, closeImpl, getActivityLogImpl } from "./task-store/task-mutation-ops.js";
|
import { getTaskSelectClauseWithActivityLogLimitImpl, getChangedTaskColumnsImpl, getSoftDeletedWriteConflictImpl, readTaskJsonImpl, writeConfigImpl, _maybeAutoArchiveSameAgentDuplicateBackendImpl, updateBranchGroupImpl, updatePrEntityImpl, listTasksForGithubTrackingReconcileImpl, listTasksForGitlabTrackingReconcileImpl, renewCheckoutLeaseImpl, updateTaskAtomicImpl, getWorkflowPromptOverridesImpl, updateWorkflowSettingValuesImpl, rollbackConfigurationImpl, cancelActiveWorkflowWorkItemsForTaskImpl, setCompletionHandoffAcceptedMarkerImpl, reconcileLegacyAutoMergeStampsImpl, recoverExpiredMergeQueueLeasesImpl, rewriteDependentsForRemovalImpl, cleanupBranchForTaskImpl, addAttachmentImpl, deleteAttachmentImpl, registerArtifactImpl, updatePrInfoImpl, unlinkGithubIssueImpl, cleanupArchivedTasksImpl, generatePromptFromArchiveEntryImpl, listWorkflowOccupantTaskIdsImpl, listApprovedCliAutonomyAdaptersImpl, closeImpl, getActivityLogImpl } from "./task-store/task-mutation-ops.js";
|
||||||
import { getOrCreateForProjectImpl, listGoalCitationsImpl, atomicWriteTaskJsonWithAuditImpl, duplicateTaskImpl, listStrandedRefinementsImpl, tryClaimCheckoutImpl, evaluateWorkflowMovePoliciesImpl, recordRunAuditEventImpl, getRunAuditEventsImpl, dequeueMergeQueueOnColumnExitImpl, updateIssueInfoImpl, listWorkflowStepsImpl, getWorkflowStepImpl, createWorkflowDefinitionImpl, countActiveInCapacitySlotSyncImpl, countActiveInCapacitySlotAsyncImpl, generateSpecifiedPromptImpl, recordActivityImpl, getEvalStoreImpl } from "./task-store/project-store-ops.js";
|
import { getOrCreateForProjectImpl, listGoalCitationsImpl, atomicWriteTaskJsonWithAuditImpl, duplicateTaskImpl, listStrandedRefinementsImpl, tryClaimCheckoutImpl, evaluateWorkflowMovePoliciesImpl, recordRunAuditEventImpl, getRunAuditEventsImpl, dequeueMergeQueueOnColumnExitImpl, updateIssueInfoImpl, listWorkflowStepsImpl, getWorkflowStepImpl, createWorkflowDefinitionImpl, countActiveInCapacitySlotSyncImpl, countActiveInCapacitySlotAsyncImpl, generateSpecifiedPromptImpl, recordActivityImpl, getEvalStoreImpl } from "./task-store/project-store-ops.js";
|
||||||
import { markLegacyAutoMergeStampsOnceImpl, appendAgentLogImpl, importLegacyAgentLogsImpl, cleanupNoOpTaskMovedActivityRowsOnceImpl, backfillCommitAssociationDiffStatsImpl } from "./task-store/workflow-integrity.js";
|
import { markLegacyAutoMergeStampsOnceImpl, appendAgentLogImpl, importLegacyAgentLogsImpl, cleanupNoOpTaskMovedActivityRowsOnceImpl, backfillCommitAssociationDiffStatsImpl } from "./task-store/workflow-integrity.js";
|
||||||
import { saveWorkflowRunBranchImpl, clearNearDuplicateReferencesToImpl, selectNextTaskForAgentImpl, pauseTaskImpl, clearLinkedAgentTaskIdsImpl, listArtifactsImpl, rehomeOccupantImpl } from "./task-store/branch-group-ops.js";
|
import { saveWorkflowRunBranchImpl, clearNearDuplicateReferencesToImpl, selectNextTaskForAgentImpl, pauseTaskImpl, clearLinkedAgentTaskIdsImpl, listArtifactsImpl, rehomeOccupantImpl, type RehomeOccupantResult } from "./task-store/branch-group-ops.js";
|
||||||
import { taskToArchiveEntryImpl, deleteTaskBackendImpl, deleteTaskIfBackendImpl, archiveTaskBackendImpl, unarchiveTaskImpl, restoreFromArchiveImpl, listArchivedTasksImpl } from "./task-store/archive-lifecycle-2.js";
|
import { taskToArchiveEntryImpl, deleteTaskBackendImpl, deleteTaskIfBackendImpl, archiveTaskBackendImpl, unarchiveTaskImpl, restoreFromArchiveImpl, listArchivedTasksImpl } from "./task-store/archive-lifecycle-2.js";
|
||||||
import { pruneOperationalLogsAsync, pruneAgentLogFilesAsync, type OperationalLogPruneResult } from "./task-store/async-maintenance.js";
|
import { pruneOperationalLogsAsync, pruneAgentLogFilesAsync, type OperationalLogPruneResult } from "./task-store/async-maintenance.js";
|
||||||
import { reconcilePhantomCommittedReservationsAsync } from "./task-store/async-phantom-reservations.js";
|
import { reconcilePhantomCommittedReservationsAsync } from "./task-store/async-phantom-reservations.js";
|
||||||
@@ -2409,7 +2409,7 @@ Issue #2149 requires read-only type filtering to occur in the file-store before
|
|||||||
public async occupantsByColumnForWorkflow( workflowId: string, includeNullSelection: boolean, ): Promise<Map<string, number>> {
|
public async occupantsByColumnForWorkflow( workflowId: string, includeNullSelection: boolean, ): Promise<Map<string, number>> {
|
||||||
return occupantsByColumnForWorkflowImpl(this, workflowId, includeNullSelection);
|
return occupantsByColumnForWorkflowImpl(this, workflowId, includeNullSelection);
|
||||||
}
|
}
|
||||||
public async rehomeOccupant( taskId: string, targetColumn: string, reason: "workflow-switch" | "workflow-delete" | "workflow-edit-rehome", metadata: Record<string, unknown>, ): Promise<void> {
|
public async rehomeOccupant( taskId: string, targetColumn: string, reason: "workflow-switch" | "workflow-delete" | "workflow-edit-rehome", metadata: Record<string, unknown>, ): Promise<RehomeOccupantResult> {
|
||||||
return rehomeOccupantImpl(this, taskId, targetColumn, reason, metadata);
|
return rehomeOccupantImpl(this, taskId, targetColumn, reason, metadata);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -279,7 +279,23 @@ export async function listArtifactsImpl(store: TaskStore, options?: { type?: Art
|
|||||||
return listArtifactsAsync(store.asyncLayer!.db, options);
|
return listArtifactsAsync(store.asyncLayer!.db, options);
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function rehomeOccupantImpl(store: TaskStore, taskId: string, targetColumn: string, reason: "workflow-switch" | "workflow-delete" | "workflow-edit-rehome", metadata: Record<string, unknown>,): Promise<void> {
|
/*
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — PR #2513 review):
|
||||||
|
Returns the OUTCOME instead of `void`. This function deliberately swallows a rejected
|
||||||
|
move ("a full target column rejects, which we audit and skip"), which is correct for
|
||||||
|
the sweep-style callers that re-home many cards best-effort — but for the workflow
|
||||||
|
SWITCH it produced a torn write with no alarm: the new selection had already
|
||||||
|
committed, so the selection said one thing and the card's column said another and
|
||||||
|
nothing reported it. Callers that need to know now can; the audit event is unchanged.
|
||||||
|
*/
|
||||||
|
export interface RehomeOccupantResult {
|
||||||
|
/** True when the card actually landed in `targetColumn`. */
|
||||||
|
readonly moved: boolean;
|
||||||
|
/** Why the move was rejected, when it was. */
|
||||||
|
readonly error?: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
export async function rehomeOccupantImpl(store: TaskStore, taskId: string, targetColumn: string, reason: "workflow-switch" | "workflow-delete" | "workflow-edit-rehome", metadata: Record<string, unknown>,): Promise<RehomeOccupantResult> {
|
||||||
/*
|
/*
|
||||||
FNXC:PostgresWorkflowEvacuation 2026-07-14-17:49:
|
FNXC:PostgresWorkflowEvacuation 2026-07-14-17:49:
|
||||||
Re-homing is an async workflow mutation and must read its current task through the authoritative PostgreSQL path; otherwise ON→OFF evacuation discovers custom-column cards but the SQLite-only read prevents every move.
|
Re-homing is an async workflow mutation and must read its current task through the authoritative PostgreSQL path; otherwise ON→OFF evacuation discovers custom-column cards but the SQLite-only read prevents every move.
|
||||||
@@ -292,7 +308,7 @@ export async function rehomeOccupantImpl(store: TaskStore, taskId: string, targe
|
|||||||
} catch {
|
} catch {
|
||||||
current = undefined;
|
current = undefined;
|
||||||
}
|
}
|
||||||
if (!current) return;
|
if (!current) return { moved: false, error: "task not readable" };
|
||||||
const fromColumn = current.column;
|
const fromColumn = current.column;
|
||||||
if (fromColumn === targetColumn) {
|
if (fromColumn === targetColumn) {
|
||||||
// Already in the target column — nothing to move, but still record the
|
// Already in the target column — nothing to move, but still record the
|
||||||
@@ -306,7 +322,8 @@ export async function rehomeOccupantImpl(store: TaskStore, taskId: string, targe
|
|||||||
target: taskId,
|
target: taskId,
|
||||||
metadata: { ...metadata, reason, fromColumn, toColumn: targetColumn, moved: false },
|
metadata: { ...metadata, reason, fromColumn, toColumn: targetColumn, moved: false },
|
||||||
});
|
});
|
||||||
return;
|
// Already in the target column: nothing to move, and nothing failed.
|
||||||
|
return { moved: true };
|
||||||
}
|
}
|
||||||
const abortRan = await runReconciliationAbort({ taskId, fromColumn, reason });
|
const abortRan = await runReconciliationAbort({ taskId, fromColumn, reason });
|
||||||
let moved = false;
|
let moved = false;
|
||||||
@@ -337,4 +354,5 @@ export async function rehomeOccupantImpl(store: TaskStore, taskId: string, targe
|
|||||||
target: taskId,
|
target: taskId,
|
||||||
metadata: { ...metadata, reason, fromColumn, toColumn: targetColumn, abortRan, moved, error },
|
metadata: { ...metadata, reason, fromColumn, toColumn: targetColumn, abortRan, moved, error },
|
||||||
});
|
});
|
||||||
|
return { moved, ...(error !== undefined ? { error } : {}) };
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -10,7 +10,9 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import { TaskStore } from "../store.js";
|
import { TaskStore } from "../store.js";
|
||||||
import {resolveEntryColumnId} from "../workflow-reconciliation.js";
|
import {resolveEntryColumnId, WorkflowSwitchRehomeFailedError, buildSwitchReconciliation} from "../workflow-reconciliation.js";
|
||||||
|
import {resolveColumnCapacity, resolveCapacityPoolId} from "../workflow-capacity.js";
|
||||||
|
import {readTaskRow as readTaskRowAsync} from "./async-persistence.js";
|
||||||
import { pruneAgentLogFiles as pruneAgentLogFileEntries, readAgentLogEntriesByTimeRange } from "../agent-log-file-store.js";
|
import { pruneAgentLogFiles as pruneAgentLogFileEntries, readAgentLogEntriesByTimeRange } from "../agent-log-file-store.js";
|
||||||
import { BUILTIN_WORKFLOWS, DEFAULT_WORKFLOW_ID, resolveDefaultWorkflowIr, getBuiltinWorkflow, getRequiredPluginIdForBuiltinWorkflow, isBuiltinWorkflowDeprecated, isBuiltinWorkflowEnabled, isBuiltinWorkflowId, isBuiltinWorkflowPluginGated } from "../builtin-workflows.js";
|
import { BUILTIN_WORKFLOWS, DEFAULT_WORKFLOW_ID, resolveDefaultWorkflowIr, getBuiltinWorkflow, getRequiredPluginIdForBuiltinWorkflow, isBuiltinWorkflowDeprecated, isBuiltinWorkflowEnabled, isBuiltinWorkflowId, isBuiltinWorkflowPluginGated } from "../builtin-workflows.js";
|
||||||
import { type DistributedTaskIdAllocator } from "../distributed-task-id.js";
|
import { type DistributedTaskIdAllocator } from "../distributed-task-id.js";
|
||||||
@@ -716,26 +718,187 @@ export async function selectTaskWorkflowAndReconcileImpl(store: TaskStore,
|
|||||||
enabledWorkflowSteps: string[];
|
enabledWorkflowSteps: string[];
|
||||||
reconciliation?: { preserved: boolean; fromColumn: string; toColumn: string };
|
reconciliation?: { preserved: boolean; fromColumn: string; toColumn: string };
|
||||||
}> {
|
}> {
|
||||||
const enabledWorkflowSteps = await store.selectTaskWorkflow(taskId, workflowId);
|
/*
|
||||||
if (!(await store.workflowColumnsFlagOn())) {
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — PR #2512 review, greptile P1):
|
||||||
return { enabledWorkflowSteps };
|
PRE-FLIGHT BEFORE COMMITTING. The ordering, not the message, is the fix.
|
||||||
|
|
||||||
|
`selectTaskWorkflow` COMMITS the new selection. If the re-home that follows is then
|
||||||
|
rejected, the card's selection says one thing and its column says another, and
|
||||||
|
self-healing later has to GUESS which is authoritative. So the deterministic
|
||||||
|
rejection cause — the destination column being at its WIP limit — is checked HERE,
|
||||||
|
before anything is written. On that path nothing commits: the task keeps its old
|
||||||
|
workflow AND its old column, which is a consistent card, and the operator gets a
|
||||||
|
typed error naming the full column.
|
||||||
|
|
||||||
|
The target IR is resolved straight from `workflowId` rather than through the task's
|
||||||
|
selection, which is precisely what let this move ahead of the commit.
|
||||||
|
*/
|
||||||
|
const preRow = await readTaskRowAsync(store.asyncLayer!, taskId, { includeDeleted: false });
|
||||||
|
if (preRow) {
|
||||||
|
const fromColumnPre = String(preRow.column);
|
||||||
|
const targetDef = await store.getWorkflowDefinition(workflowId);
|
||||||
|
if (targetDef) {
|
||||||
|
const targetIr = parseWorkflowIr(targetDef.ir);
|
||||||
|
const pre = resolveSwitchReconciliation(targetIr, fromColumnPre);
|
||||||
|
if (!pre.preserved && pre.targetColumn !== fromColumnPre) {
|
||||||
|
const settingsForCapacity = await store.getSettingsFast();
|
||||||
|
const capacity = resolveColumnCapacity(targetIr, pre.targetColumn, settingsForCapacity);
|
||||||
|
if (capacity.limit !== undefined && Number.isFinite(capacity.limit)) {
|
||||||
|
const occupied = await store.asyncLayer!.transactionImmediate(async (tx) =>
|
||||||
|
store.countActiveInCapacitySlotAsync({
|
||||||
|
tx,
|
||||||
|
targetColumn: pre.targetColumn,
|
||||||
|
workflowId: resolveCapacityPoolId(workflowId),
|
||||||
|
countPending: capacity.countPending === true,
|
||||||
|
excludeTaskId: taskId,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
if (occupied >= capacity.limit) {
|
||||||
|
throw new WorkflowSwitchRehomeFailedError({
|
||||||
|
taskId,
|
||||||
|
workflowId,
|
||||||
|
fromColumn: fromColumnPre,
|
||||||
|
intendedColumn: pre.targetColumn,
|
||||||
|
reason: `target column is at its limit (${occupied}/${capacity.limit})`,
|
||||||
|
committed: false,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const enabledWorkflowSteps = await store.selectTaskWorkflow(taskId, workflowId);
|
||||||
|
/*
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — R9, USER-VISIBLE):
|
||||||
|
The early return on the raw `workflowColumns` flag is DELETED. It read a key no
|
||||||
|
production writer sets, so switching a task's workflow NEVER reconciled its
|
||||||
|
column: the card kept sitting in its old column even when the new workflow does
|
||||||
|
not declare that column, and the `reconciliation` field this function promises in
|
||||||
|
its return type was never populated for a real project.
|
||||||
|
|
||||||
|
Operator-visible consequence, deliberate: switching a task to a workflow that does
|
||||||
|
not declare its current column now moves the card into that workflow's resolved
|
||||||
|
target column (`resolveSwitchReconciliation`), and API/dashboard callers start
|
||||||
|
receiving the `reconciliation` summary they already have handling for. A card whose
|
||||||
|
column IS declared by the new workflow is preserved in place, unchanged.
|
||||||
|
*/
|
||||||
const newIr = await resolveWorkflowIrForTask(store, taskId);
|
const newIr = await resolveWorkflowIrForTask(store, taskId);
|
||||||
const current = store.readTaskFromDb(taskId, { includeDeleted: false });
|
/*
|
||||||
if (!current) return { enabledWorkflowSteps };
|
FNXC:PostgresCutover 2026-07-28-00:00 (U12):
|
||||||
const fromColumn = current.column;
|
ASYNC read, not `store.readTaskFromDb`. The synchronous reader resolves through
|
||||||
|
`TaskStore.db`, which THROWS under the PostgreSQL runtime ("SQLite Database is not
|
||||||
|
available in backend mode"). The old flag gate returned before ever reaching this
|
||||||
|
line, so un-gating the switch reconciliation surfaced a path that could not run at
|
||||||
|
all in the production backend — the flag was hiding an unported read, not just a
|
||||||
|
disabled feature. Caught by the production-shape tests in
|
||||||
|
`__tests__/postgres/workflow-reconciliation-production-shape.pg.test.ts`.
|
||||||
|
|
||||||
|
NOT a missing SQLite fallback (PR #2513 review — CodeRabbit). `backendMode` is
|
||||||
|
defined as "the mandatory production AsyncDataLayer was injected", this module
|
||||||
|
already dereferences `store.asyncLayer!` in 15 other places, and the sibling
|
||||||
|
workflow-definition operations carry FNXC:SqliteDualPathCleanup notes stating they
|
||||||
|
require an AsyncDataLayer. Re-adding the synchronous reader as a fallback would
|
||||||
|
reintroduce exactly the throw this line fixes.
|
||||||
|
*/
|
||||||
|
const currentRow = await readTaskRowAsync(store.asyncLayer!, taskId, { includeDeleted: false });
|
||||||
|
if (!currentRow) return { enabledWorkflowSteps };
|
||||||
|
const fromColumn = String(currentRow.column);
|
||||||
const decision = resolveSwitchReconciliation(newIr, fromColumn);
|
const decision = resolveSwitchReconciliation(newIr, fromColumn);
|
||||||
if (!decision.preserved && decision.targetColumn !== fromColumn) {
|
if (!decision.preserved && decision.targetColumn !== fromColumn) {
|
||||||
await store.rehomeOccupant(taskId, decision.targetColumn, "workflow-switch", { workflowId });
|
const outcome = await store.rehomeOccupant(taskId, decision.targetColumn, "workflow-switch", { workflowId });
|
||||||
|
/*
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — PR #2513 review):
|
||||||
|
FAIL LOUDLY. `rehomeOccupant` swallows a rejected move by design, which is right
|
||||||
|
for the best-effort sweep callers but wrong here: the new selection has ALREADY
|
||||||
|
committed, so a swallowed rejection is a torn write — selection and column
|
||||||
|
disagree and nobody is told. Throw instead, leaving the recoverable state in
|
||||||
|
place (the R7 startup sweep re-homes an undeclared column) rather than rolling
|
||||||
|
back a committed selection.
|
||||||
|
*/
|
||||||
|
if (!outcome.moved) {
|
||||||
|
/*
|
||||||
|
RESIDUAL RACE ONLY. The capacity pre-flight above already rejects the
|
||||||
|
deterministic case before any commit, so reaching here means the destination
|
||||||
|
filled up (or the move was otherwise rejected) between the pre-flight and the
|
||||||
|
move. The selection IS committed at this point, so this IS a torn card — and it
|
||||||
|
is RECORDED, not merely thrown, so self-healing and an operator both find the
|
||||||
|
divergence in run-audit instead of having to infer it from a card in a lane the
|
||||||
|
board cannot draw. Metadata stays ids/columns/outcomes-only.
|
||||||
|
*/
|
||||||
|
/*
|
||||||
|
AWAITED, not fire-and-forget (PR #2512 review — CodeRabbit). The whole claim of
|
||||||
|
this branch is that the divergence is a fact on disk; `void`-ing the write and
|
||||||
|
throwing on the next line meant the one artifact self-healing is meant to find
|
||||||
|
could silently be absent. The write is awaited and its own failure is swallowed
|
||||||
|
so it can never mask the rejection the caller actually needs to see.
|
||||||
|
|
||||||
|
NO ERROR PROSE (PR #2512 review — CodeRabbit). `outcome.error` is a propagated
|
||||||
|
`err.message`; persisting it would contradict this file's own "ids/columns/
|
||||||
|
outcomes-only" claim and the project rule that run-audit never stores error
|
||||||
|
prose. The bounded outcome code goes here; the human-readable reason travels on
|
||||||
|
the thrown error, which is not persisted.
|
||||||
|
*/
|
||||||
|
try {
|
||||||
|
await store.recordRunAuditEvent({
|
||||||
|
taskId,
|
||||||
|
agentId: "system",
|
||||||
|
runId: `workflow-switch-torn-${taskId}`,
|
||||||
|
domain: "database",
|
||||||
|
mutationType: "task:workflow-switch-torn",
|
||||||
|
target: taskId,
|
||||||
|
metadata: {
|
||||||
|
workflowId,
|
||||||
|
fromColumn,
|
||||||
|
intendedColumn: decision.targetColumn,
|
||||||
|
selectionCommitted: true,
|
||||||
|
outcome: "rehome-rejected",
|
||||||
|
},
|
||||||
|
});
|
||||||
|
} catch {
|
||||||
|
// An audit-write failure must not replace the rejection being reported.
|
||||||
|
}
|
||||||
|
throw new WorkflowSwitchRehomeFailedError({
|
||||||
|
taskId,
|
||||||
|
workflowId,
|
||||||
|
fromColumn,
|
||||||
|
intendedColumn: decision.targetColumn,
|
||||||
|
committed: true,
|
||||||
|
...(outcome.error !== undefined ? { reason: outcome.error } : {}),
|
||||||
|
});
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return {
|
/*
|
||||||
enabledWorkflowSteps,
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — PR #2512 review, greptile P1):
|
||||||
reconciliation: {
|
Report the column the task ACTUALLY has, not the one we asked for.
|
||||||
preserved: decision.preserved,
|
`rehomeOccupant` deliberately swallows a rejected move — "a full target column
|
||||||
fromColumn,
|
rejects, which we audit and skip" — and returns void, so reporting
|
||||||
toColumn: decision.targetColumn,
|
`decision.targetColumn` claimed a move that may never have happened. A caller
|
||||||
},
|
(dashboard switch, `fn_task_set_workflow`) would then show the card in a column it
|
||||||
};
|
is not in.
|
||||||
|
|
||||||
|
Re-reading also makes the reported result honest under the concurrency windows
|
||||||
|
raised in the same review: this call resolves the IR and re-homes AFTER
|
||||||
|
`selectTaskWorkflow` released its task lock, so a racing move can land in between.
|
||||||
|
Re-reading cannot close that window — it makes the response describe the outcome
|
||||||
|
rather than the intention, so a caller is never told a move succeeded when the
|
||||||
|
card sits elsewhere. Closing the window itself needs the switch to hold the task
|
||||||
|
lock across selection + reconciliation, which changes the locking contract and is
|
||||||
|
left as a separate, testable change rather than smuggled into a flag flip.
|
||||||
|
*/
|
||||||
|
/*
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — PR #2512 review, greptile P1):
|
||||||
|
ABSENT IS ABSENT, decided by the pure `buildSwitchReconciliation` seam. An earlier
|
||||||
|
version fell back to `fromColumn`, so a task SOFT-DELETED between the first read
|
||||||
|
and this one was reported as having its old column PRESERVED — a live column
|
||||||
|
fabricated for a row that is gone.
|
||||||
|
*/
|
||||||
|
const afterRow = await readTaskRowAsync(store.asyncLayer!, taskId, { includeDeleted: false });
|
||||||
|
const reconciliation = buildSwitchReconciliation(
|
||||||
|
fromColumn,
|
||||||
|
afterRow ? String(afterRow.column) : undefined,
|
||||||
|
);
|
||||||
|
return { enabledWorkflowSteps, ...(reconciliation ? { reconciliation } : {}) };
|
||||||
}
|
}
|
||||||
|
|
||||||
export function pruneAgentLogFilesImpl(store: TaskStore, retentionDays: number): { prunedFiles: number; prunedEntries: number; freedBytes: number } {
|
export function pruneAgentLogFilesImpl(store: TaskStore, retentionDays: number): { prunedFiles: number; prunedEntries: number; freedBytes: number } {
|
||||||
|
|||||||
@@ -177,12 +177,27 @@ export async function updateWorkflowDefinitionImpl(store: TaskStore, id: string,
|
|||||||
if (isBuiltinWorkflowId(id)) throw new Error("Built-in workflows cannot be edited");
|
if (isBuiltinWorkflowId(id)) throw new Error("Built-in workflows cannot be edited");
|
||||||
/* FNXC:SqliteDualPathCleanup 2026-07-26-14:08: workflow definition deletes require AsyncDataLayer. */
|
/* FNXC:SqliteDualPathCleanup 2026-07-26-14:08: workflow definition deletes require AsyncDataLayer. */
|
||||||
const layer: AsyncDataLayer = store.asyncLayer!;
|
const layer: AsyncDataLayer = store.asyncLayer!;
|
||||||
// U5 (R20): flag-ON edits that remove an occupied column block with a typed
|
/*
|
||||||
// OccupiedColumnsError unless `rehomeTo` is supplied. Computed before taking
|
U5 (R20): an edit that removes an OCCUPIED column blocks with a typed
|
||||||
// the config lock (pure DB reads) so the lock body stays focused.
|
OccupiedColumnsError unless `rehomeTo` is supplied. Computed before taking the
|
||||||
const flagOn = await store.workflowColumnsFlagOn();
|
config lock (pure DB reads) so the lock body stays focused.
|
||||||
|
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — R9, USER-VISIBLE):
|
||||||
|
The `flagOn` conjunct is DELETED. It read the RAW
|
||||||
|
`experimentalFeatures.workflowColumns` key, which no production writer sets, so
|
||||||
|
this guard has NEVER fired for a real project: removing a column with cards in it
|
||||||
|
silently succeeded and left those cards in a column their workflow no longer
|
||||||
|
declares. The typed rejection and the `rehomeTo` re-home are the whole point of
|
||||||
|
the guard; gating them on a retired flag made the API contract a fiction.
|
||||||
|
|
||||||
|
Operator-visible consequence, deliberate: saving a workflow edit that drops an
|
||||||
|
occupied column now FAILS with OccupiedColumnsError instead of succeeding. The
|
||||||
|
dashboard editor's re-home flow (which passes `rehomeTo`) becomes reachable for
|
||||||
|
the first time. Cards are moved by the editor's explicit choice rather than
|
||||||
|
stranded silently.
|
||||||
|
*/
|
||||||
let pendingRehome: { rehomeTo: string; occupantTaskIds: string[] } | undefined;
|
let pendingRehome: { rehomeTo: string; occupantTaskIds: string[] } | undefined;
|
||||||
if (flagOn && updates.ir !== undefined) {
|
if (updates.ir !== undefined) {
|
||||||
const existingForCheck = await store.getWorkflowDefinition(id);
|
const existingForCheck = await store.getWorkflowDefinition(id);
|
||||||
if (!existingForCheck) throw new Error(`Workflow '${id}' not found`);
|
if (!existingForCheck) throw new Error(`Workflow '${id}' not found`);
|
||||||
const nextIrForCheck = parseWorkflowIr(updates.ir);
|
const nextIrForCheck = parseWorkflowIr(updates.ir);
|
||||||
@@ -295,7 +310,20 @@ export async function updateWorkflowDefinitionImpl(store: TaskStore, id: string,
|
|||||||
name: next.name,
|
name: next.name,
|
||||||
description: next.description,
|
description: next.description,
|
||||||
icon: next.icon ?? null,
|
icon: next.icon ?? null,
|
||||||
ir: flagOn ? next.ir : downgradeIrToV1IfPure(next.ir),
|
/*
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — deliberately NOT flipped here):
|
||||||
|
This is the v1-IR rollback-compat persistence decision (#1405), not an
|
||||||
|
enforcement gate — it chooses the STORED SHAPE of the graph so an older binary
|
||||||
|
could still load the row. It shared the U5 `flagOn` variable that this change
|
||||||
|
deletes, which is why it is spelled out separately now rather than silently
|
||||||
|
inheriting the flip: one flag read was feeding two unrelated decisions, so the
|
||||||
|
flag has more decision sites than call sites.
|
||||||
|
|
||||||
|
Retiring the downgrade is a persistence-format change with a different blast
|
||||||
|
radius than a guard, and it needs its own round-trip evidence. Left reading the
|
||||||
|
raw flag, unchanged in behaviour, for a follow-up.
|
||||||
|
*/
|
||||||
|
ir: (await store.workflowColumnsFlagOn()) ? next.ir : downgradeIrToV1IfPure(next.ir),
|
||||||
layout: next.layout,
|
layout: next.layout,
|
||||||
updatedAt: next.updatedAt,
|
updatedAt: next.updatedAt,
|
||||||
}).where(eq(schema.project.workflows.id, id));
|
}).where(eq(schema.project.workflows.id, id));
|
||||||
@@ -338,11 +366,24 @@ export async function deleteWorkflowDefinitionImpl(store: TaskStore, id: string)
|
|||||||
if (isBuiltinWorkflowId(id)) throw new Error("Built-in workflows cannot be deleted");
|
if (isBuiltinWorkflowId(id)) throw new Error("Built-in workflows cannot be deleted");
|
||||||
/* FNXC:SqliteDualPathCleanup 2026-07-26-14:08: workflow definition deletes require AsyncDataLayer. */
|
/* FNXC:SqliteDualPathCleanup 2026-07-26-14:08: workflow definition deletes require AsyncDataLayer. */
|
||||||
const layer: AsyncDataLayer = store.asyncLayer!;
|
const layer: AsyncDataLayer = store.asyncLayer!;
|
||||||
// U5 (R20): flag-ON, capture the occupant task ids BEFORE the cascade clears
|
/*
|
||||||
// their selection rows, so we can re-home them to the DEFAULT workflow's
|
U5 (R20): capture the occupant task ids BEFORE the cascade clears their selection
|
||||||
// entry column once their selection resolves back to the default (KTD-1).
|
rows, so we can re-home them to the DEFAULT workflow's entry column once their
|
||||||
const flagOn = await store.workflowColumnsFlagOn();
|
selection resolves back to the default (KTD-1).
|
||||||
const occupantTaskIds = flagOn ? await store.listWorkflowOccupantTaskIds(id, false) : [];
|
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — R9, USER-VISIBLE):
|
||||||
|
The `flagOn ? … : []` gate is DELETED. Reading the retired raw flag meant the
|
||||||
|
capture returned an empty list for every real project, so the re-home below was
|
||||||
|
dead: deleting a workflow left its cards sitting in that workflow's columns with
|
||||||
|
their selection cleared, resolving to the default workflow which does not declare
|
||||||
|
those columns. The startup sweep `reconcileUndeclaredTaskColumns` would eventually
|
||||||
|
re-home them, but only on the next engine start — until then the cards sat in
|
||||||
|
lanes the board could not draw.
|
||||||
|
|
||||||
|
Operator-visible consequence, deliberate: deleting a workflow now moves its cards
|
||||||
|
to the default workflow's entry column immediately, instead of at next startup.
|
||||||
|
*/
|
||||||
|
const occupantTaskIds = await store.listWorkflowOccupantTaskIds(id, false);
|
||||||
|
|
||||||
|
|
||||||
// FNXC:PostgresCutover 2026-06-28: async deletes for backend mode
|
// FNXC:PostgresCutover 2026-06-28: async deletes for backend mode
|
||||||
@@ -394,7 +435,7 @@ export async function deleteWorkflowDefinitionImpl(store: TaskStore, id: string)
|
|||||||
// workflow's entry column. Their selection rows are already cleared above,
|
// workflow's entry column. Their selection rows are already cleared above,
|
||||||
// so they now resolve to the built-in default workflow (KTD-1); the re-home
|
// so they now resolve to the built-in default workflow (KTD-1); the re-home
|
||||||
// move preserves task fields (preserveProgress) and emits one audit per card.
|
// move preserves task fields (preserveProgress) and emits one audit per card.
|
||||||
if (flagOn && occupantTaskIds.length > 0) {
|
if (occupantTaskIds.length > 0) {
|
||||||
const defaultEntry = resolveEntryColumnId(BUILTIN_CODING_WORKFLOW_IR);
|
const defaultEntry = resolveEntryColumnId(BUILTIN_CODING_WORKFLOW_IR);
|
||||||
if (defaultEntry) {
|
if (defaultEntry) {
|
||||||
for (const taskId of occupantTaskIds) {
|
for (const taskId of occupantTaskIds) {
|
||||||
|
|||||||
@@ -101,6 +101,85 @@ export function resolveSwitchReconciliation(
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — PR #2513 review):
|
||||||
|
A workflow SWITCH commits the new selection first, then re-homes the card. When the
|
||||||
|
destination rejects the move (capacity), `rehomeOccupant` swallows the error — so the
|
||||||
|
selection said one thing and the card's column said another, and nothing reported it.
|
||||||
|
A torn write with no alarm is the one outcome that must not survive.
|
||||||
|
|
||||||
|
The state is RECOVERABLE and deliberately left in place rather than rolled back: the
|
||||||
|
selection is committed and the card sits in a column the new workflow does not
|
||||||
|
declare, which is exactly what the R7 startup sweep `reconcileUndeclaredTaskColumns`
|
||||||
|
repairs. What was missing is the alarm, so this error IS the alarm — it names the
|
||||||
|
task, both columns, and the underlying rejection so an operator can retry or make
|
||||||
|
room instead of discovering it later from a card in a lane that cannot be drawn.
|
||||||
|
*/
|
||||||
|
export class WorkflowSwitchRehomeFailedError extends Error {
|
||||||
|
readonly taskId: string;
|
||||||
|
readonly workflowId: string;
|
||||||
|
readonly fromColumn: string;
|
||||||
|
readonly intendedColumn: string;
|
||||||
|
readonly reason?: string;
|
||||||
|
/** True when the workflow selection was already COMMITTED — i.e. the card is torn
|
||||||
|
* and needs recovery. False when the switch was rejected before any write, which
|
||||||
|
* leaves the card fully consistent and is the ordinary case. */
|
||||||
|
readonly committed: boolean;
|
||||||
|
constructor(args: {
|
||||||
|
taskId: string;
|
||||||
|
workflowId: string;
|
||||||
|
fromColumn: string;
|
||||||
|
intendedColumn: string;
|
||||||
|
reason?: string;
|
||||||
|
committed: boolean;
|
||||||
|
}) {
|
||||||
|
super(
|
||||||
|
args.committed
|
||||||
|
? `Task '${args.taskId}' was switched to workflow '${args.workflowId}', but re-homing it ` +
|
||||||
|
`from '${args.fromColumn}' to '${args.intendedColumn}' was rejected` +
|
||||||
|
`${args.reason ? `: ${args.reason}` : ""}. The workflow selection IS COMMITTED and the ` +
|
||||||
|
`card remains in '${args.fromColumn}', which that workflow does not declare — the card ` +
|
||||||
|
`is inconsistent. Make room in '${args.intendedColumn}' and move the card there, or ` +
|
||||||
|
`switch the task back; startup reconciliation will otherwise re-home it.`
|
||||||
|
: `Cannot switch task '${args.taskId}' to workflow '${args.workflowId}': it would have to ` +
|
||||||
|
`move from '${args.fromColumn}' to '${args.intendedColumn}'` +
|
||||||
|
`${args.reason ? `, but ${args.reason}` : ", but that move was rejected"}. Nothing was ` +
|
||||||
|
`changed — the task keeps its current workflow and column. Make room in ` +
|
||||||
|
`'${args.intendedColumn}' and retry.`,
|
||||||
|
);
|
||||||
|
this.name = "WorkflowSwitchRehomeFailedError";
|
||||||
|
this.taskId = args.taskId;
|
||||||
|
this.workflowId = args.workflowId;
|
||||||
|
this.fromColumn = args.fromColumn;
|
||||||
|
this.intendedColumn = args.intendedColumn;
|
||||||
|
this.committed = args.committed;
|
||||||
|
if (args.reason !== undefined) this.reason = args.reason;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Decide what a completed workflow switch should REPORT, given the column the task
|
||||||
|
* actually has afterwards (`undefined` when the row could not be read).
|
||||||
|
*
|
||||||
|
* FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — PR #2512 review, greptile P1):
|
||||||
|
* Extracted as a pure seam because the case that matters is not reachable through
|
||||||
|
* the public call. `selectTaskWorkflow` rejects a soft-deleted task up front with
|
||||||
|
* `TaskDeletedError`, so "the task is soft-deleted BETWEEN the first read and the
|
||||||
|
* final one" is a genuine race that cannot be driven from outside — and it is the
|
||||||
|
* case where the previous fallback fabricated a live column for a row that is gone.
|
||||||
|
* Racing a lifecycle read against a soft-delete is a documented hazard here (the
|
||||||
|
* soft-delete verification matrix; FN-8004). Testing the decision directly is honest;
|
||||||
|
* asserting it from reading the code is not.
|
||||||
|
*/
|
||||||
|
export function buildSwitchReconciliation(
|
||||||
|
fromColumn: string,
|
||||||
|
actualColumn: string | undefined,
|
||||||
|
): { preserved: boolean; fromColumn: string; toColumn: string } | undefined {
|
||||||
|
// Absent is absent — never synthesize a column from the stale pre-switch read.
|
||||||
|
if (actualColumn === undefined) return undefined;
|
||||||
|
return { preserved: actualColumn === fromColumn, fromColumn, toColumn: actualColumn };
|
||||||
|
}
|
||||||
|
|
||||||
// ── (b) Workflow edit removing an occupied column ────────────────────────────
|
// ── (b) Workflow edit removing an occupied column ────────────────────────────
|
||||||
|
|
||||||
/** Per-column occupant count for a blocked edit/delete. */
|
/** Per-column occupant count for a blocked edit/delete. */
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import type { WorkflowDefinition, WorkflowDefinitionKind, WorkflowIr, WorkflowIrNode, WorkflowSettingDefinition, TaskStore } from "@fusion/core";
|
import type { WorkflowDefinition, WorkflowDefinitionKind, WorkflowIr, WorkflowIrNode, WorkflowSettingDefinition, TaskStore } from "@fusion/core";
|
||||||
import { ColumnTraitValidationError, OccupiedColumnsError, InvalidRehomeTargetError, WorkflowIrError, ColumnAgentBindingError, WorkflowSettingRejectionError, SCHEMA_VERSION, assertColumnTraitsValid, layoutForIr, listTraits, listStepParsers, parseWorkflowIr, resolvePlanningSettingsModel, stripApprovalBypassFlags, resolveWorkflowIrById, resolveEffectiveSettingValues, findOrphanedSettingValues, isBuiltinWorkflowId, getBuiltinWorkflow, BUILTIN_WORKFLOW_SETTINGS, AgentStore, validateColumnAgentBindings, resolveWorkflowOptionalSteps, enumeratePromptBearingWorkflowNodes, normalizeWorkflowIcon } from "@fusion/core";
|
import { ColumnTraitValidationError, OccupiedColumnsError, InvalidRehomeTargetError, WorkflowIrError, ColumnAgentBindingError, WorkflowSettingRejectionError, SCHEMA_VERSION, assertColumnTraitsValid, layoutForIr, listTraits, listStepParsers, parseWorkflowIr, resolvePlanningSettingsModel, stripApprovalBypassFlags, resolveWorkflowIrById, resolveEffectiveSettingValues, findOrphanedSettingValues, isBuiltinWorkflowId, getBuiltinWorkflow, BUILTIN_WORKFLOW_SETTINGS, AgentStore, validateColumnAgentBindings, resolveWorkflowOptionalSteps, enumeratePromptBearingWorkflowNodes, normalizeWorkflowIcon, WorkflowSwitchRehomeFailedError } from "@fusion/core";
|
||||||
import { buildSessionSkillContextSync, createFnAgent as engineCreateFnAgent, validateCodeNodeSources, validateWorkflowIrDryRun } from "@fusion/engine";
|
import { buildSessionSkillContextSync, createFnAgent as engineCreateFnAgent, validateCodeNodeSources, validateWorkflowIrDryRun } from "@fusion/engine";
|
||||||
import { ApiError, badRequest, conflict, notFound, rateLimited } from "../api-error.js";
|
import { ApiError, badRequest, conflict, notFound, rateLimited } from "../api-error.js";
|
||||||
// FNXC:TaskLookup404 2026-07-26-11:40: shared task-miss -> 404 mapping seam.
|
// FNXC:TaskLookup404 2026-07-26-11:40: shared task-miss -> 404 mapping seam.
|
||||||
@@ -431,9 +431,17 @@ export function registerWorkflowRoutes(ctx: ApiRoutesContext): void {
|
|||||||
res.json(updated);
|
res.json(updated);
|
||||||
} catch (err: unknown) {
|
} catch (err: unknown) {
|
||||||
if (err instanceof ApiError) throw err;
|
if (err instanceof ApiError) throw err;
|
||||||
// U5 (R20): a flag-ON edit removing an occupied column blocks with a typed
|
/*
|
||||||
// error. Surface it as a structured 409 carrying the per-column occupant
|
U5 (R20): an edit removing an occupied column blocks with a typed error.
|
||||||
// counts so the client can prompt for a `rehomeTo` target and retry.
|
Surface it as a structured 409 carrying the per-column occupant counts so the
|
||||||
|
client can prompt for a `rehomeTo` target and retry.
|
||||||
|
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — R9):
|
||||||
|
Was "a flag-ON edit". The store-side guard is no longer gated on the retired
|
||||||
|
`workflowColumns` flag, so this 409 — and the editor's re-home prompt behind it
|
||||||
|
— are reachable for the first time. The handler itself is unchanged; it was
|
||||||
|
correct and simply never fired.
|
||||||
|
*/
|
||||||
if (err instanceof OccupiedColumnsError) {
|
if (err instanceof OccupiedColumnsError) {
|
||||||
throw conflict(err.message, { workflowId: err.workflowId, occupancies: err.occupancies });
|
throw conflict(err.message, { workflowId: err.workflowId, occupancies: err.occupancies });
|
||||||
}
|
}
|
||||||
@@ -639,7 +647,9 @@ export function registerWorkflowRoutes(ctx: ApiRoutesContext): void {
|
|||||||
throw badRequest("workflowId must be a string or null");
|
throw badRequest("workflowId must be a string or null");
|
||||||
}
|
}
|
||||||
let enabledWorkflowSteps: string[] = [];
|
let enabledWorkflowSteps: string[] = [];
|
||||||
// U5 (R20) switch reconciliation: when the workflowColumns flag is ON, the
|
// FNXC:WorkflowColumns 2026-07-28-00:00 (U12): the flag gate is gone — this
|
||||||
|
// reconciliation now runs for every project.
|
||||||
|
// U5 (R20) switch reconciliation: the
|
||||||
// store re-homes the card to the new workflow's entry column (aborting
|
// store re-homes the card to the new workflow's entry column (aborting
|
||||||
// in-flight work first) unless the new workflow defines its current column.
|
// in-flight work first) unless the new workflow defines its current column.
|
||||||
// The re-home outcome rides on the response so the UI can reflect the move.
|
// The re-home outcome rides on the response so the UI can reflect the move.
|
||||||
@@ -655,6 +665,29 @@ export function registerWorkflowRoutes(ctx: ApiRoutesContext): void {
|
|||||||
if (selectErr instanceof Error && /not found/i.test(selectErr.message)) {
|
if (selectErr instanceof Error && /not found/i.test(selectErr.message)) {
|
||||||
throw notFound(selectErr.message);
|
throw notFound(selectErr.message);
|
||||||
}
|
}
|
||||||
|
/*
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — PR #2512 review):
|
||||||
|
TRANSLATE the switch re-home failure instead of letting it fall through to a
|
||||||
|
generic 500. Without this the operator sees "something went wrong" with no
|
||||||
|
indication that the switch was refused, why, or whether their card moved.
|
||||||
|
|
||||||
|
`committed` is the field that matters: false means nothing was written and the
|
||||||
|
card is intact (the ordinary case — the destination column is full, caught by
|
||||||
|
the pre-flight before any commit), so 409 "retry after making room". True means
|
||||||
|
the selection committed and the re-home then lost a race, so the card IS torn
|
||||||
|
and the payload says so explicitly along with both columns.
|
||||||
|
*/
|
||||||
|
if (selectErr instanceof WorkflowSwitchRehomeFailedError) {
|
||||||
|
throw conflict(selectErr.message, {
|
||||||
|
code: "workflow-switch-rehome-failed",
|
||||||
|
taskId: selectErr.taskId,
|
||||||
|
workflowId: selectErr.workflowId,
|
||||||
|
fromColumn: selectErr.fromColumn,
|
||||||
|
intendedColumn: selectErr.intendedColumn,
|
||||||
|
selectionCommitted: selectErr.committed,
|
||||||
|
...(selectErr.reason !== undefined ? { reason: selectErr.reason } : {}),
|
||||||
|
});
|
||||||
|
}
|
||||||
throw selectErr;
|
throw selectErr;
|
||||||
}
|
}
|
||||||
emitWorkflowSseEvent("workflow:updated", { taskId: req.params.taskId, workflowId }, projectId);
|
emitWorkflowSseEvent("workflow:updated", { taskId: req.params.taskId, workflowId }, projectId);
|
||||||
|
|||||||
@@ -2871,6 +2871,30 @@ export function createWorkflowSelectTool(store: TaskStore, currentTaskId: string
|
|||||||
};
|
};
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
} catch (err: any) {
|
} catch (err: any) {
|
||||||
|
/*
|
||||||
|
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — PR #2512 review):
|
||||||
|
TRANSLATE the switch re-home failure so the agent gets an actionable, retryable
|
||||||
|
result instead of an opaque message. `selectionCommitted` is the field that
|
||||||
|
matters: false means nothing was written and the task is intact (destination
|
||||||
|
column full, caught before any commit) so the agent can make room and retry;
|
||||||
|
true means the selection committed and the re-home then lost a race, so the
|
||||||
|
task is INCONSISTENT and the agent must not treat the switch as done.
|
||||||
|
*/
|
||||||
|
if (err?.name === "WorkflowSwitchRehomeFailedError") {
|
||||||
|
return {
|
||||||
|
content: [{ type: "text" as const, text: `ERROR: ${err.message}` }],
|
||||||
|
details: {
|
||||||
|
code: "workflow-switch-rehome-failed",
|
||||||
|
taskId: err.taskId,
|
||||||
|
workflowId: err.workflowId,
|
||||||
|
fromColumn: err.fromColumn,
|
||||||
|
intendedColumn: err.intendedColumn,
|
||||||
|
selectionCommitted: err.committed === true,
|
||||||
|
...(err.reason !== undefined ? { reason: err.reason } : {}),
|
||||||
|
},
|
||||||
|
isError: true,
|
||||||
|
};
|
||||||
|
}
|
||||||
return {
|
return {
|
||||||
content: [{ type: "text" as const, text: `ERROR: Failed to select workflow: ${err?.message ?? err}` }],
|
content: [{ type: "text" as const, text: `ERROR: Failed to select workflow: ${err?.message ?? err}` }],
|
||||||
details: {},
|
details: {},
|
||||||
|
|||||||
Reference in New Issue
Block a user