diff --git a/.changeset/u12-u5-reconciliation-guards.md b/.changeset/u12-u5-reconciliation-guards.md new file mode 100644 index 0000000000..84bea9a2a3 --- /dev/null +++ b/.changeset/u12-u5-reconciliation-guards.md @@ -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. diff --git a/packages/cli/src/extension.ts b/packages/cli/src/extension.ts index a57439cf22..b571a90218 100644 --- a/packages/cli/src/extension.ts +++ b/packages/cli/src/extension.ts @@ -1444,6 +1444,30 @@ export default function kbExtension(pi: ExtensionAPI) { await store.selectTaskWorkflowAndReconcile(task.id, workflowId); } catch (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 { content: [{ type: "text", text: `ERROR: ${message}` }], isError: true, diff --git a/packages/core/src/__tests__/postgres/workflow-authoritative-reads.pg.test.ts b/packages/core/src/__tests__/postgres/workflow-authoritative-reads.pg.test.ts index c6ea44a6f2..cb728f1cdb 100644 --- a/packages/core/src/__tests__/postgres/workflow-authoritative-reads.pg.test.ts +++ b/packages/core/src/__tests__/postgres/workflow-authoritative-reads.pg.test.ts @@ -31,7 +31,14 @@ pgDescribe("PostgreSQL workflow authoritative reads", () => { it("blocks removal of a PostgreSQL-occupied workflow column", async () => { 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 workflow = await store.createWorkflowDefinition({ name: "Occupancy", ir, layout: {} }); const task = await store.createTask({ description: "occupies custom column" }); diff --git a/packages/core/src/__tests__/postgres/workflow-reconciliation-production-shape.pg.test.ts b/packages/core/src/__tests__/postgres/workflow-reconciliation-production-shape.pg.test.ts new file mode 100644 index 0000000000..46b8050a17 --- /dev/null +++ b/packages/core/src/__tests__/postgres/workflow-reconciliation-production-shape.pg.test.ts @@ -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"); + }); +}); diff --git a/packages/core/src/__tests__/workflow-switch-reconciliation-report.test.ts b/packages/core/src/__tests__/workflow-switch-reconciliation-report.test.ts new file mode 100644 index 0000000000..722c42bec9 --- /dev/null +++ b/packages/core/src/__tests__/workflow-switch-reconciliation-report.test.ts @@ -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(); + }); +}); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 2976730122..8fc0fa1097 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -471,6 +471,7 @@ export type { ColumnCapacity } from "./workflow-capacity.js"; // ── U5: workflow lifecycle reconciliation (switch / edit / delete) ─────────── export { OccupiedColumnsError, + WorkflowSwitchRehomeFailedError, InvalidRehomeTargetError, IncompatibleFieldChangeError, resolveEntryColumnId, diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index a01a4fb6e0..09c2b357e2 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -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 { 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 { 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 { pruneOperationalLogsAsync, pruneAgentLogFilesAsync, type OperationalLogPruneResult } from "./task-store/async-maintenance.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> { return occupantsByColumnForWorkflowImpl(this, workflowId, includeNullSelection); } - public async rehomeOccupant( taskId: string, targetColumn: string, reason: "workflow-switch" | "workflow-delete" | "workflow-edit-rehome", metadata: Record, ): Promise { + public async rehomeOccupant( taskId: string, targetColumn: string, reason: "workflow-switch" | "workflow-delete" | "workflow-edit-rehome", metadata: Record, ): Promise { return rehomeOccupantImpl(this, taskId, targetColumn, reason, metadata); } diff --git a/packages/core/src/task-store/branch-group-ops.ts b/packages/core/src/task-store/branch-group-ops.ts index 7babb1d5bb..0f043e837f 100644 --- a/packages/core/src/task-store/branch-group-ops.ts +++ b/packages/core/src/task-store/branch-group-ops.ts @@ -279,7 +279,23 @@ export async function listArtifactsImpl(store: TaskStore, options?: { type?: Art 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,): Promise { +/* +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,): Promise { /* 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. @@ -292,7 +308,7 @@ export async function rehomeOccupantImpl(store: TaskStore, taskId: string, targe } catch { current = undefined; } - if (!current) return; + if (!current) return { moved: false, error: "task not readable" }; const fromColumn = current.column; if (fromColumn === targetColumn) { // 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, 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 }); let moved = false; @@ -337,4 +354,5 @@ export async function rehomeOccupantImpl(store: TaskStore, taskId: string, targe target: taskId, metadata: { ...metadata, reason, fromColumn, toColumn: targetColumn, abortRan, moved, error }, }); + return { moved, ...(error !== undefined ? { error } : {}) }; } diff --git a/packages/core/src/task-store/workflow-definitions.ts b/packages/core/src/task-store/workflow-definitions.ts index 35dbb1cd5e..abb63a16cb 100644 --- a/packages/core/src/task-store/workflow-definitions.ts +++ b/packages/core/src/task-store/workflow-definitions.ts @@ -10,7 +10,9 @@ */ 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 { BUILTIN_WORKFLOWS, DEFAULT_WORKFLOW_ID, resolveDefaultWorkflowIr, getBuiltinWorkflow, getRequiredPluginIdForBuiltinWorkflow, isBuiltinWorkflowDeprecated, isBuiltinWorkflowEnabled, isBuiltinWorkflowId, isBuiltinWorkflowPluginGated } from "../builtin-workflows.js"; import { type DistributedTaskIdAllocator } from "../distributed-task-id.js"; @@ -716,26 +718,187 @@ export async function selectTaskWorkflowAndReconcileImpl(store: TaskStore, enabledWorkflowSteps: string[]; reconciliation?: { preserved: boolean; fromColumn: string; toColumn: string }; }> { - const enabledWorkflowSteps = await store.selectTaskWorkflow(taskId, workflowId); - if (!(await store.workflowColumnsFlagOn())) { - return { enabledWorkflowSteps }; + /* + FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — PR #2512 review, greptile P1): + 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 current = store.readTaskFromDb(taskId, { includeDeleted: false }); - if (!current) return { enabledWorkflowSteps }; - const fromColumn = current.column; + /* + FNXC:PostgresCutover 2026-07-28-00:00 (U12): + 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); 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, - reconciliation: { - preserved: decision.preserved, - fromColumn, - toColumn: decision.targetColumn, - }, - }; + /* + FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — PR #2512 review, greptile P1): + Report the column the task ACTUALLY has, not the one we asked for. + `rehomeOccupant` deliberately swallows a rejected move — "a full target column + rejects, which we audit and skip" — and returns void, so reporting + `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 } { diff --git a/packages/core/src/task-store/workflow-ops.ts b/packages/core/src/task-store/workflow-ops.ts index c6d93a796a..e32895b907 100644 --- a/packages/core/src/task-store/workflow-ops.ts +++ b/packages/core/src/task-store/workflow-ops.ts @@ -177,12 +177,27 @@ export async function updateWorkflowDefinitionImpl(store: TaskStore, id: string, if (isBuiltinWorkflowId(id)) throw new Error("Built-in workflows cannot be edited"); /* FNXC:SqliteDualPathCleanup 2026-07-26-14:08: workflow definition deletes require AsyncDataLayer. */ 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 - // the config lock (pure DB reads) so the lock body stays focused. - const flagOn = await store.workflowColumnsFlagOn(); + /* + U5 (R20): an edit that removes an OCCUPIED column blocks with a typed + OccupiedColumnsError unless `rehomeTo` is supplied. Computed before taking the + 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; - if (flagOn && updates.ir !== undefined) { + if (updates.ir !== undefined) { const existingForCheck = await store.getWorkflowDefinition(id); if (!existingForCheck) throw new Error(`Workflow '${id}' not found`); const nextIrForCheck = parseWorkflowIr(updates.ir); @@ -295,7 +310,20 @@ export async function updateWorkflowDefinitionImpl(store: TaskStore, id: string, name: next.name, description: next.description, 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, updatedAt: next.updatedAt, }).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"); /* FNXC:SqliteDualPathCleanup 2026-07-26-14:08: workflow definition deletes require AsyncDataLayer. */ 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 - // entry column once their selection resolves back to the default (KTD-1). - const flagOn = await store.workflowColumnsFlagOn(); - const occupantTaskIds = flagOn ? await store.listWorkflowOccupantTaskIds(id, false) : []; + /* + U5 (R20): capture the occupant task ids BEFORE the cascade clears their selection + rows, so we can re-home them to the DEFAULT workflow's entry column once their + selection resolves back to the default (KTD-1). + + 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 @@ -394,7 +435,7 @@ export async function deleteWorkflowDefinitionImpl(store: TaskStore, id: string) // 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 // 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); if (defaultEntry) { for (const taskId of occupantTaskIds) { diff --git a/packages/core/src/workflow-reconciliation.ts b/packages/core/src/workflow-reconciliation.ts index e417c79a9e..1765af8bb6 100644 --- a/packages/core/src/workflow-reconciliation.ts +++ b/packages/core/src/workflow-reconciliation.ts @@ -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 ──────────────────────────── /** Per-column occupant count for a blocked edit/delete. */ diff --git a/packages/dashboard/src/routes/register-workflow-routes.ts b/packages/dashboard/src/routes/register-workflow-routes.ts index 244d978a86..d8ca4065ab 100644 --- a/packages/dashboard/src/routes/register-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-workflow-routes.ts @@ -1,5 +1,5 @@ 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 { ApiError, badRequest, conflict, notFound, rateLimited } from "../api-error.js"; // 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); } catch (err: unknown) { 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 - // counts so the client can prompt for a `rehomeTo` target and retry. + /* + U5 (R20): an edit removing an occupied column blocks with a typed error. + 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) { 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"); } 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 // 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. @@ -655,6 +665,29 @@ export function registerWorkflowRoutes(ctx: ApiRoutesContext): void { if (selectErr instanceof Error && /not found/i.test(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; } emitWorkflowSseEvent("workflow:updated", { taskId: req.params.taskId, workflowId }, projectId); diff --git a/packages/engine/src/agent-tools.ts b/packages/engine/src/agent-tools.ts index c324389337..cb51f569ac 100644 --- a/packages/engine/src/agent-tools.ts +++ b/packages/engine/src/agent-tools.ts @@ -2871,6 +2871,30 @@ export function createWorkflowSelectTool(store: TaskStore, currentTaskId: string }; // eslint-disable-next-line @typescript-eslint/no-explicit-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 { content: [{ type: "text" as const, text: `ERROR: Failed to select workflow: ${err?.message ?? err}` }], details: {},