diff --git a/packages/core/src/__tests__/postgres/moves-intake-only-hard-cancel.pg.test.ts b/packages/core/src/__tests__/postgres/moves-intake-only-hard-cancel.pg.test.ts new file mode 100644 index 0000000000..886f1fa8a1 --- /dev/null +++ b/packages/core/src/__tests__/postgres/moves-intake-only-hard-cancel.pg.test.ts @@ -0,0 +1,106 @@ +/* +FNXC:WorkflowTaskCancellation 2026-07-30-23:20 (PR #2705 review — greptile): + +THE OPERATOR HARD CANCEL MUST FIRE ON A WORKFLOW THAT HAS NO HOLD COLUMN. + +`moveTaskInternal`'s two hard-cancel guards resolved their target as +`moveLifecycle?.hold ?? "todo"`. A workflow may legitimately declare an INTAKE column and no hold +column — Coding (Ideas)'s `ideas` lane is the shipped example — and then `"todo"` names a column +that workflow does not declare. The comparison never matches, so the card moves while its merge +request stays live and its active work items are never cancelled. + +That is a cancellation contract failing OPEN, which is the worst direction available: the operator +sees the card parked and believes the work stopped. + +WHY A LIVE STORE. The bug is in which column id the guard compares against, and that id comes from +the task's own persisted workflow. A mock that hands back a lifecycle struct would be asserting my +own assumption about what `resolveLifecycleColumns` returns for an intake-only lineage — the exact +substitution that has produced vacuous tests all through this program. This drives a real +PostgreSQL store and a real workflow definition, and asserts on OBSERVED PERSISTED STATE. + +WHAT IS ASSERTED. The completion-handoff marker is cleared only inside the guarded block, so it is +a precise witness that the block ran. Reverting the production fallback to `?? "todo"` leaves the +marker in place and fails this test. + +LANE. `.pg.test.ts`, skipped by `pgDescribe` when no PostgreSQL is reachable, so the merge gate is +unaffected. Throwaway per-file database; never port 4040. +*/ +import { beforeAll, beforeEach, afterEach, afterAll, expect, it } from "vitest"; +import "@fusion/core"; // registers the built-in column traits + +import { pgDescribe, createSharedPgTaskStoreTestHarness } from "../../__test-utils__/pg-test-harness.js"; + +pgDescribe("operator hard cancel on a workflow with an intake lane and no hold lane", () => { + const harness = createSharedPgTaskStoreTestHarness({ prefix: "fusion_moves_intake_cancel" }); + + beforeAll(harness.beforeAll); + afterAll(harness.afterAll); + beforeEach(async () => { await harness.beforeEach(); }); + afterEach(async () => { await harness.afterEach(); }); + + /** Intake, wip, review, complete — deliberately NO hold column, and no legacy ids. */ + async function intakeOnlyWorkflow(store: ReturnType) { + return store.createWorkflowDefinition({ + name: "intake-only", + ir: { + version: "v2", + name: "intake-only", + columns: [ + { id: "inbox", name: "Inbox", traits: [{ trait: "intake" }] }, + { id: "signoff", name: "Signoff", traits: [{ trait: "merge" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], + nodes: [{ id: "start", kind: "start", column: "inbox" }, { id: "end", kind: "end", column: "shipped" }], + edges: [{ from: "start", to: "end" }], + }, + } as never); + } + + it("clears the completion-handoff marker when the operator parks a review card in the INTAKE lane", async () => { + const store = harness.store(); + const definition = await intakeOnlyWorkflow(store); + const task = await store.createTask({ description: "intake-only card", workflowId: definition.id } as never); + + await store.moveTask(task.id, "signoff" as never, { bypassGuards: true } as never); + + await store.setCompletionHandoffAcceptedMarker(task.id, { source: "test" }); + expect(await store.getCompletionHandoffAcceptedMarker(task.id)).not.toBeNull(); + + /* + The operator hard cancel: review -> the pre-implementation lane, by USER. + + `bypassGuards` is here because this lineage declares no edge back from `signoff`, and that + ADJACENCY question is a different one from the side-effect contract under test. Without it the + move is rejected before reaching the guard, and the test would pass or fail on transition + legality rather than on which column id the cancel compares against. + */ + await store.moveTask(task.id, "inbox" as never, { moveSource: "user" } as never); + + store.taskCache.delete(task.id); + expect((await store.getTask(task.id)).column).toBe("inbox"); + /* + The witness. With the pre-fix `?? "todo"` fallback this guard never matched on this lineage, so + the marker survived and the merge request was left running behind a card the operator had + already parked. + */ + expect(await store.getCompletionHandoffAcceptedMarker(task.id)).toBeNull(); + }); + + it("does NOT hard-cancel an ENGINE move into the same lane", async () => { + /* + The paired negative, so the fix cannot pass by cancelling on every move into intake. Only an + operator move is a hard cancel; an engine rebound must leave the handoff alone. + */ + const store = harness.store(); + const definition = await intakeOnlyWorkflow(store); + const task = await store.createTask({ description: "engine rebound card", workflowId: definition.id } as never); + + await store.moveTask(task.id, "signoff" as never, { bypassGuards: true } as never); + await store.setCompletionHandoffAcceptedMarker(task.id, { source: "test" }); + + await store.moveTask(task.id, "inbox" as never, { moveSource: "engine", recoveryRehome: true, bypassGuards: true } as never); + + expect(await store.getCompletionHandoffAcceptedMarker(task.id)).not.toBeNull(); + }); +}); diff --git a/packages/core/src/task-store/moves.ts b/packages/core/src/task-store/moves.ts index 64fc72f37e..195a43eb85 100644 --- a/packages/core/src/task-store/moves.ts +++ b/packages/core/src/task-store/moves.ts @@ -398,6 +398,16 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum // FNXC:WorkflowColumns 2026-07-30-04:00 (U12): resolved unconditionally — the gate is gone, so // `undefined` now means only "no IR on this path or a v1 column-less IR", never "flag off". const workflowIr: WorkflowIr | undefined = await resolveTaskWorkflowIrForMove(store, id); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-23:30 (fleet: moves.ts): + ONE lifecycle resolution for the whole move, derived from the IR resolved on the line above — so + every role guard below costs nothing extra on the hottest lifecycle path in the system. The local + that used to compute this further down now aliases it rather than resolving a second time. + + `undefined` means no IR on this path or a v1 column-less IR, so each site keeps its legacy id as + the fallback: a move must behave exactly as before when there is no basis to resolve from. + */ + const moveLifecycle = workflowIr ? resolveLifecycleColumns(workflowIr) : undefined; if (task.column === toColumn) { if (internal.fromHandoff && toColumn === "in-review") { @@ -475,13 +485,13 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum return task; } - if (toColumn === "done" && store.clearDoneTransientFields(task)) { + if (toColumn === (moveLifecycle?.complete ?? "done") && store.clearDoneTransientFields(task)) { task.updatedAt = new Date().toISOString(); await store.atomicWriteTaskJson(dir, task); if (store.isWatching) store.taskCache.set(id, { ...task }); store.emit("task:updated", task); } - if (toColumn === "done") { + if (toColumn === (moveLifecycle?.complete ?? "done")) { await store.clearNearDuplicateReferencesToFailSoft(id, { column: "done", reason: "done", @@ -575,7 +585,7 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum // FNXC:WorkflowTransitionPolicy 2026-07-19-10:20: // The blocker fact is resolved only when the SOURCE column carries the // `merge-blocker` trait flag — the trait-level generalization of the legacy - // `fromColumn === "in-review"` gate. Resolving it for every complete-bound + // `fromColumn === (moveLifecycle?.review ?? "in-review")` gate. Resolving it for every complete-bound // move re-hardcoded the review lane: `getTaskMergeBlocker` rejects any // source column that is not literally "in-review", which broke the // six-column benchmark's merging → done edge and the builtin @@ -757,7 +767,7 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum } } - if (fromColumn === "in-review" && toColumn === "done" && !options?.skipMergeBlocker) { + if (fromColumn === (moveLifecycle?.review ?? "in-review") && toColumn === (moveLifecycle?.complete ?? "done") && !options?.skipMergeBlocker) { const mergeBlocker = getTaskMergeBlocker(task); if (mergeBlocker) { throw new Error(`Cannot move ${id} to done: ${mergeBlocker}`); @@ -820,7 +830,7 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum or a v1 column-less IR; the hooks treat that as "no basis" and keep the legacy names, which is the only case where a legacy literal is legitimate. */ - const moveLifecycleColumns = workflowIr ? resolveLifecycleColumns(workflowIr) : undefined; + const moveLifecycleColumns = moveLifecycle; // ── Flag-ON: route the legacy per-column side effects through the // default-workflow trait hooks (timing, reset-on-entry, abort-on-exit, // merge.onEnter). "Moved, not duplicated" applies to this path; the @@ -874,14 +884,14 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum } // Store-owned effects the hooks intentionally do NOT perform (filesystem / // store-private): clearing done transient fields + prompt-checkbox reset. - if (toColumn === "done") { + if (toColumn === (moveLifecycle?.complete ?? "done")) { store.clearDoneTransientFields(task); } if (isReopenToTodoOrTriage && !preserveStepProgress) { await store.resetPromptCheckboxes(dir); } - if (toColumn === "in-progress" && !task.worktree && options?.allocateWorktree) { + if (toColumn === (moveLifecycle?.wip ?? "in-progress") && !task.worktree && options?.allocateWorktree) { const allocator = options.allocateWorktree; const allocated = await store.withWorktreeAllocationLock(async () => { const others = await store.listTasks({ slim: true, includeArchived: false }); @@ -1039,7 +1049,7 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum // FNXC:WorkflowReviewGates 2026-07-26-16:40: see isRecognizedInReviewEntry — a // graph-owned crossing into the review column is a legitimate arrival, not a violation. - if (toColumn === "in-review" && !isRecognizedInReviewEntry(options, internal)) { + if (toColumn === (moveLifecycle?.review ?? "in-review") && !isRecognizedInReviewEntry(options, internal)) { await recordRunAuditEventWithinTransaction(tx, { taskId: id, agentId: internal.runContext?.agentId ?? "system", @@ -1190,7 +1200,18 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum } } - if (fromColumn === "in-review" && toColumn === "todo" && moveSource === "user") { + /* + FNXC:WorkflowTaskCancellation 2026-07-30-23:05 (PR #2705 review — greptile): + HOLD, THEN INTAKE, THEN THE LEGACY ID. A workflow may declare an intake column and NO hold column + (Coding (Ideas)'s `ideas` is the shipped example). `?? "todo"` then names a column that workflow + does not declare, so this comparison never matches and the operator's hard cancel silently does + nothing: the merge request stays live and the active work items are never cancelled, while the + card moves anyway. Failing OPEN on a cancellation contract is the worst available outcome. + + Same precedence the replan target settled on in #2659, and for the same reason — the + pre-implementation lane is hold when one exists and intake otherwise. + */ + if (fromColumn === (moveLifecycle?.review ?? "in-review") && toColumn === (moveLifecycle?.hold ?? moveLifecycle?.intake ?? "todo") && moveSource === "user") { const handoffAccepted = await store.getCompletionHandoffAcceptedMarker(id); const mergeRequest = await store.getMergeRequestRecordAsync(id); if (handoffAccepted && mergeRequest && mergeRequest.state !== "succeeded" && mergeRequest.state !== "cancelled") { @@ -1208,7 +1229,7 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum }); void store.clearCompletionHandoffAcceptedMarker(id); } - if (toColumn === "todo" && moveSource === "user" && (fromIsImplementation || fromColumn === "in-review")) { + if (toColumn === (moveLifecycle?.hold ?? moveLifecycle?.intake ?? "todo") && moveSource === "user" && (fromIsImplementation || fromColumn === (moveLifecycle?.review ?? "in-review"))) { // FNXC:WorkflowTaskCancellation 2026-07-21-11:51: // The task move is already committed here. Continuation cleanup is // best-effort so a storage fault cannot suppress task:moved or strand @@ -1227,7 +1248,7 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum }); } } - if (toColumn === "done") { + if (toColumn === (moveLifecycle?.complete ?? "done")) { // FNXC:RuntimeTaskOrchestrationAsync 2026-06-24-16:00: // Backend mode: clearLinkedAgentTaskIds is a sync SQLite operation; skip // it in backend mode (the agent cleanup is best-effort and handled by @@ -1322,7 +1343,7 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum ...(workflowSelectionForMove?.workflowId ? { workflowId: workflowSelectionForMove.workflowId } : {}), }); } - if (toColumn === "done") { + if (toColumn === (moveLifecycle?.complete ?? "done")) { await store.clearNearDuplicateReferencesToFailSoft(id, { column: "done", reason: "done", diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index e24fbc5df4..f473c0e2d6 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -7,7 +7,6 @@ "packages/dashboard/app/components/TaskDetailModal.tsx": 30, "packages/engine/src/scheduler.ts": 26, "packages/dashboard/src/routes/register-task-workflow-routes.ts": 20, - "packages/core/src/task-store/moves.ts": 15, "packages/core/src/store.ts": 12, "packages/engine/src/project-engine.ts": 12, "packages/engine/src/mission-execution-loop.ts": 10, @@ -70,6 +69,7 @@ "packages/core/src/task-store/archive-lifecycle-2.ts": 2, "packages/core/src/task-store/audit-ops.ts": 2, "packages/core/src/task-store/comments-ops.ts": 2, + "packages/core/src/task-store/moves.ts": 2, "packages/core/src/task-store/project-store-ops.ts": 2, "packages/core/src/task-store/reads.ts": 2, "packages/core/src/task-store/symbol-locks.ts": 2,