diff --git a/.changeset/duplicate-archive-renamed-lane.md b/.changeset/duplicate-archive-renamed-lane.md new file mode 100644 index 0000000000..10028f513d --- /dev/null +++ b/.changeset/duplicate-archive-renamed-lane.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Duplicate archiving, CLI merge completion, and stuck-task recovery work on boards with renamed columns. +category: fix +dev: Also `cli/commands/task-lifecycle`, whose two merge-completion paths passed a hardcoded `"done"`. `duplicate-intake` and `duplicate-guard` passed a hardcoded `"archived"` to `moveTask`. Since the workflow-column rejection went live, a board without that column rejects the move, so the duplicate stays on the board — already stamped `deterministicDuplicateOf`. Both now resolve the `archived`-trait column from the task's workflow, falling back to the legacy id. Four auto-recovery requeues (contamination, foreign-only contamination x2, and the restart path) passed a hardcoded `"todo"` to `moveTask` for the same reason; on a board without that column the move was rejected and the recovery never completed, leaving the task stuck in exactly the state the recovery exists to clear. All four now resolve the rebound target from the task's own workflow. diff --git a/docs/solutions/architecture-patterns/hardcoded-movetask-destinations-are-census-invisible.md b/docs/solutions/architecture-patterns/hardcoded-movetask-destinations-are-census-invisible.md new file mode 100644 index 0000000000..b2347fa48f --- /dev/null +++ b/docs/solutions/architecture-patterns/hardcoded-movetask-destinations-are-census-invisible.md @@ -0,0 +1,146 @@ +--- +category: architecture-patterns +module: workflow-resolved-columns +date: 2026-07-30 +problem_type: systemic_gap +component: engine +severity: high +applies_when: + - "Converting a lifecycle-column guard whose body performs a moveTask" + - "Reading the lifecycle-column census total as the remaining work" + - "Auditing what a renamed board breaks" +tags: + - workflow-resolved-columns + - column-census + - move-task + - census-invisible +--- + +# A guard and its `moveTask` are two halves of one conversion, and the census only counts one + +## The shape + +```ts +if (task.column !== "in-review") { … return; } // the census counts THIS +… +await this.store.moveTask(taskId, "in-progress"); // and cannot see THIS +``` + +The census is an AST scan for **comparisons** against a lifecycle id. A `moveTask` destination is a +**call argument**, so no entry in the backlog ever points at one. Converting the guard alone is worse +than leaving both: the handler starts *admitting* work on a renamed board and then tries to move the +card into a lane that board may not declare. + +Hit twice in one week, both times only because the guard next to it was being converted: + +- `auto-recovery-handlers/branch-worktree.ts` — the counted guard was `task.column === "in-progress"`; + the invisible half was `moveTask(task.id, "todo", …)`, requeuing into a lane that may not exist (PR + #2797). +- `pr-comment-handler.ts` — the counted guard dropped a GitHub "changes requested" review; the + invisible half was `moveTask(taskId, "in-progress")` (PR #2807). + +## The measurement + +Across `packages/core`, `packages/engine`, `packages/dashboard`, `packages/cli` and `plugins`, +excluding `__tests__`/`*.test.*` and comment lines: + +| | count | +| --- | ---: | +| hardcoded `moveTask` destinations in production | **51** | +| …passing `recoveryRehome: true` — **deliberate**, see below | 22 | +| …plain, i.e. rejected on a board that does not declare the target | **29** | + +The plain 29 at the time of measurement, by file. **13 have since been converted** on this branch and +in PRs #2797/#2807 — the parenthesised entries are done, and the count stands at **16 remaining**: + +```text + 6 packages/engine/src/self-healing.ts (all 6 sit in query-gated sweeps — see below) + 3 plugins/fusion-plugin-even-realities-glasses/src/agent-actions.ts + 2 packages/cli/src/extension.ts + 2 packages/engine/src/replan-target.ts (both are COMMENT lines, not call sites) + 1 packages/dashboard/app/utils/appLifecycle.ts + 1 packages/engine/src/project-engine.ts + ---- converted ---- + 3 packages/engine/src/executor.ts (done) + 2 packages/engine/src/project-engine.ts (done) + 2 packages/engine/src/recovery/foreign-only-contamination.ts (done) + 2 packages/cli/src/commands/task-lifecycle.ts (done) + 1 packages/core/src/duplicate-intake.ts (done) + 1 packages/core/src/duplicate-guard.ts (done) + 1 packages/engine/src/auto-recovery-handlers/contamination.ts (done) + 1 packages/engine/src/restart-recovery-coordinator.ts (done) + 1 packages/engine/src/pr-comment-handler.ts (done, #2807) +``` + +**`self-healing.ts`'s 6 are deliberately last.** Every one sits inside a sweep whose task list comes from +a hardcoded `listTasks({ column: … })` filter, so on a renamed board the sweep returns no rows and the +`moveTask` below it is never reached. Converting those destinations changes nothing observable until the +query layer is fixed — see the sibling doc on self-healing. Converting them first would look like +progress and deliver none. + +Three shared resolvers now cover the converted sites, in `workflow-lifecycle-traits.ts` beside +`resolveTaskLifecycleColumns`: `resolveReboundTargetForTask`, `resolveArchiveTargetForTask`, +`resolveWipTargetForTask`. Use them rather than re-deriving a destination per call site. + +Re-measure with: + +```bash +grep -rn 'moveTask(' packages/*/src packages/dashboard/app packages/cli/src plugins \ + --include=*.ts --include=*.tsx | grep -v __tests__ | grep -v '\.test\.' +``` +then split on whether `recoveryRehome: true` appears in the option object. + +## The 22 are NOT defects — do not "fix" them + +`moves.ts` deliberately exempts them: + +```ts +const recoveryToLegacy = + options?.recoveryRehome === true && (COLUMNS as readonly string[]).includes(toColumn); +if (!workflowHasColumn(workflowIr, toColumn) && !recoveryToLegacy) { throw … } +``` + +The comment there records why (#1411): a custom-workflow card stranded in an undeclared column must +still be rescuable to a legacy safe-landing column, or it can never be recovered at all. A sweep that +"converts" these re-homes removes the rescue path. + +## Why this got sharper recently + +The `workflowHasColumn(workflowIr, toColumn)` rejection used to sit inside a block gated on +`isWorkflowColumnsCompatibilityFlagEnabled`, which reads a raw settings key **nothing in production +writes** — so the check did not execute and the legacy `VALID_TRANSITIONS` table decided instead. U12 +hoisted it out of that dead branch, and it is now live and unconditional whenever the workflow resolves. +Proven on a real store by `packages/core/src/__tests__/live-move-path-undeclared-target.test.ts`: + +```text +moveTask(card in "todo" -> "triage") now REJECTS: /Unknown column for this workflow/ +``` + +That changed the failure mode of all 29. **Before**, a hardcoded destination silently landed the card in +an undeclared column — invisible to every trait-driven sweep until reconciliation re-homed it. **Now** it +throws. Whether that surfaces or disappears depends entirely on whether the caller catches, which is +per-site and is **not** measured here — do not read "29" as "29 crashes". + +## What to do + +1. **Convert the pair or neither.** When a census entry sits in a function that also performs a + `moveTask`, the destination is in scope for the same change. Resolve it — `resolveReboundTarget(ir)` + for a rebound, the appropriate `columnsWithFlag(ir, …)` lane otherwise — and keep the legacy id as + the fallback. +2. **Guard the move.** Even a resolved destination can be rejected (a deleted task, a guard, capacity). + Catch at the move, record an audit row naming the actual failure, and do not let a recovery handler + die on it — see `branch-worktree.ts`, where the rejection is classified from + `TransitionRejectionError.rejection.code` rather than by message match. +3. **Do not clear state before the move.** If the move can be rejected, anything cleared beforehand is + lost with no requeue. `branch-worktree.ts` cleared `branch`/`baseCommitSha` first and destroyed the + only pointers back to the work on a rejected move. + +## Related + +- `docs/solutions/architecture-patterns/self-healing-sweeps-are-blind-on-a-renamed-board.md` — the other + census-invisible class, where a hardcoded `listTasks({ column })` filter means the guard never runs at + all. +- `docs/solutions/test-failures/optional-flags-seam-hides-unconverted-column-guards.md` — the census + counts syntax; its "literal COLLECTION" section is the third invisible class (array/`Set` membership). +- `packages/core/src/__tests__/live-move-path-undeclared-target.test.ts` — the live proof that the + rejection now fires. diff --git a/packages/cli/src/__tests__/complete-target-renamed-lane.test.ts b/packages/cli/src/__tests__/complete-target-renamed-lane.test.ts new file mode 100644 index 0000000000..9f0e6551d3 --- /dev/null +++ b/packages/cli/src/__tests__/complete-target-renamed-lane.test.ts @@ -0,0 +1,68 @@ +/* +FNXC:WorkflowResolvedColumns 2026-07-30-20:25 (census-invisible moveTask destinations): +The CLI's merge-completion paths (`finalizePullRequestMerge`, `finalizeNoOpMergeTask`) both passed a +hardcoded `"done"` to `moveTask`. The destination is a call ARGUMENT, so the lifecycle-column census — +an AST scan for comparisons — never pointed at either. + +Since U12 hoisted the `workflowHasColumn` rejection out of its dead flag-gated branch, a board that does +not declare `done` REJECTS that move. Both callers run `updateTask({ status: null, mergeRetries: 0 })` +FIRST, so on a rejection the merge has already landed and the bookkeeping is already cleared while the +card never reaches its complete lane: the operator sees a merged branch, a card still sitting in review, +and a reset retry counter. + +SCOPE, stated rather than implied: this covers the RESOLVER, not the two call sites. Both enclosing +functions are private to the module and reachable only through `processPullRequest`, which needs a live +GitHub surface; exporting them purely to test the wiring would be a worse trade than saying plainly what +is and is not covered. Both call sites now route through this one helper, so they cannot drift from each +other — the same argument as triage's two copies of the terminal filter. + +REVERT CHECK, measured: with the body replaced by a bare `return "done"`, the renamed case fails. +*/ +import { describe, expect, it, vi } from "vitest"; +import type { TaskStore, WorkflowIr } from "@fusion/core"; +import { resolveCompleteTargetForTask } from "../commands/task-lifecycle.js"; + +function storeWith(ir: WorkflowIr | undefined): TaskStore { + return { + getTaskWorkflowSelectionAsync: vi.fn(async () => (ir ? { workflowId: "cli-lifecycle", stepIds: [] } : undefined)), + getTaskWorkflowSelection: vi.fn(() => (ir ? { workflowId: "cli-lifecycle", stepIds: [] } : undefined)), + getWorkflowDefinition: vi.fn(async (id: string) => (id === "cli-lifecycle" && ir ? { ir } : undefined)), + } as unknown as TaskStore; +} + +/** Minimal IR: one hold lane and a complete lane whose id is NOT the legacy one. */ +const RENAMED_IR = { + version: "v2", + id: "cli-lifecycle", + name: "cli", + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], + nodes: [{ id: "start", kind: "start", column: "backlog" }], + edges: [], +} as unknown as WorkflowIr; + +describe("resolveCompleteTargetForTask", () => { + it("resolves the workflow's OWN complete lane", async () => { + await expect(resolveCompleteTargetForTask(storeWith(RENAMED_IR), "FN-1")).resolves.toBe("shipped"); + }); + + it("falls back to the legacy id when no workflow resolves", async () => { + /* + Load-bearing: `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather than throwing, and the + built-in complete lane IS `done` — so this also pins that a default board is byte-identical. + */ + await expect(resolveCompleteTargetForTask(storeWith(undefined), "FN-1")).resolves.toBe("done"); + }); + + it("falls back to the legacy id when the workflow lookup throws", async () => { + const store = { + getTaskWorkflowSelectionAsync: vi.fn(async () => { throw new Error("store unavailable"); }), + getTaskWorkflowSelection: vi.fn(() => undefined), + getWorkflowDefinition: vi.fn(async () => undefined), + } as unknown as TaskStore; + + await expect(resolveCompleteTargetForTask(store, "FN-1")).resolves.toBe("done"); + }); +}); diff --git a/packages/cli/src/commands/task-lifecycle.ts b/packages/cli/src/commands/task-lifecycle.ts index 7cd6596448..d7a21fa6df 100644 --- a/packages/cli/src/commands/task-lifecycle.ts +++ b/packages/cli/src/commands/task-lifecycle.ts @@ -34,6 +34,35 @@ import { WorkspaceTaskMergeError, } from "@fusion/core"; import type { Settings, TaskDetail, PrInfo, MergeResult, BranchGroup, BranchGroupPrState, Task } from "@fusion/core"; +import { resolveWorkflowIrForTask, resolveCompleteColumn } from "@fusion/core"; + +/* +FNXC:WorkflowResolvedColumns 2026-07-30-20:10 (census-invisible moveTask destinations): +Resolve THIS task's complete lane, falling back to the legacy id. + +Both merge-completion paths below passed a hardcoded `"done"` to `moveTask`. The destination is a call +ARGUMENT, so the lifecycle-column census — an AST scan for comparisons — never pointed at either. Since +U12 hoisted the `workflowHasColumn` rejection out of its dead flag-gated branch, a board that does not +declare `done` REJECTS the move. + +That matters here because both callers run `updateTask({ status: null, mergeRetries: 0 })` FIRST: on a +rejection the merge has already landed and the bookkeeping is already cleared, but the card never +reaches its complete lane — so the operator sees a merged branch and a card still sitting in review, +with the retry counter reset. + +Unioned with the legacy id because `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather than +throwing. +*/ +export async function resolveCompleteTargetForTask(store: TaskStore, taskId: string): Promise { + try { + const ir = await resolveWorkflowIrForTask(store, taskId); + if (ir) { + const complete = resolveCompleteColumn(ir); + if (complete) return complete; + } + } catch { /* degraded: legacy id */ } + return "done"; +} import { activeSessionRegistry, resolveIntegrationBranch } from "@fusion/engine"; import type { CreateGroupPrFn, @@ -658,7 +687,7 @@ async function finalizePullRequestMerge( ): Promise { await cleanupMergedTaskArtifacts(cwd, task, { pool }); await store.updateTask(task.id, { status: null, mergeRetries: 0 }); - const movedTask = await store.moveTask(task.id, "done"); + const movedTask = await store.moveTask(task.id, await resolveCompleteTargetForTask(store, task.id)); const mergedTask = movedTask ?? (await store.getTask(task.id)); await store.logEntry(task.id, message, `PR #${prInfo.number}: ${prInfo.url}`); const settings = await store.getSettings(); @@ -696,7 +725,7 @@ async function finalizeNoOpMergeTask( const branch = task.branch ?? getTaskBranchName(task.id); await cleanupMergedTaskArtifacts(cwd, task, { pool }); await store.updateTask(task.id, { status: null, mergeRetries: 0 }); - const movedTask = await store.moveTask(task.id, "done"); + const movedTask = await store.moveTask(task.id, await resolveCompleteTargetForTask(store, task.id)); const mergedTask = movedTask ?? (await store.getTask(task.id)); await store.logEntry(task.id, reason, `Branch ${branch} has no commits relative to the base branch; nothing to merge.`); store.emit("task:merged", { diff --git a/packages/cli/src/extension.ts b/packages/cli/src/extension.ts index 7d46271175..48f1ec2c86 100644 --- a/packages/cli/src/extension.ts +++ b/packages/cli/src/extension.ts @@ -1947,9 +1947,11 @@ export default function kbExtension(pi: ExtensionAPI) { ...buildManualRetryResetPatch({ resetMergeRetries: true }), }); await store.logEntry(params.id, `Retry requested via Fusion extension (unusable worktree session-start recovery → todo, preserving progress${retryLogSuffix})`); - await store.moveTask(params.id, "todo", { preserveProgress: true }); + /* FNXC:WorkflowResolvedColumns 2026-07-30-22:20: census-invisible moveTask DESTINATION — a call argument, not a comparison. This is an OPERATOR-triggered Retry: on a board that does not declare `todo` the move is REJECTED and the retry fails in the operator's face. The reply text below uses the SAME resolved value so it cannot name a lane the card did not go to. */ + const retryTarget = await fusionCore.resolveReboundTargetForTask(store, params.id); + await store.moveTask(params.id, retryTarget, { preserveProgress: true }); return { - content: [{ type: "text", text: `Retried ${params.id} → todo (unusable worktree session metadata cleared)` }], + content: [{ type: "text", text: `Retried ${params.id} → ${retryTarget} (unusable worktree session metadata cleared)` }], details: { taskId: params.id, newColumn: 'todo' }, }; } @@ -1969,9 +1971,11 @@ export default function kbExtension(pi: ExtensionAPI) { ? `Retry requested via Fusion extension (stranded in-review execution retry → todo, preserving progress${retryLogSuffix})` : `Retry requested via Fusion extension (execution failure in-review → todo, preserving progress${retryLogSuffix})`, ); - await store.moveTask(params.id, "todo", { preserveProgress: true }); + /* FNXC:WorkflowResolvedColumns 2026-07-30-22:20: census-invisible moveTask DESTINATION — same operator Retry path as above. */ + const executionRetryTarget = await fusionCore.resolveReboundTargetForTask(store, params.id); + await store.moveTask(params.id, executionRetryTarget, { preserveProgress: true }); return { - content: [{ type: "text", text: `Retried ${params.id} → todo (execution failure, preserving step progress)` }], + content: [{ type: "text", text: `Retried ${params.id} → ${executionRetryTarget} (execution failure, preserving step progress)` }], details: { taskId: params.id, newColumn: 'todo' }, }; } diff --git a/packages/core/src/__tests__/duplicate-guard.test.ts b/packages/core/src/__tests__/duplicate-guard.test.ts index 6edc4334b5..51b0f3cb45 100644 --- a/packages/core/src/__tests__/duplicate-guard.test.ts +++ b/packages/core/src/__tests__/duplicate-guard.test.ts @@ -305,6 +305,51 @@ describe("reconcileDeterministicDuplicate", () => { })); }); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-19:45 (census-invisible moveTask destinations): + DIFFERENTIAL over the archive lane's id. The case above asserts `moveTask("FN-2", "archived")` — the + LEGACY id, which is exactly what the hardcoded destination passed, so it was green before this + conversion and would stay green for a broken one. + + The destination of a `moveTask` is a call ARGUMENT, so the lifecycle-column census (an AST scan for + comparisons) never pointed at it. Since U12 hoisted the `workflowHasColumn` rejection out of its dead + flag-gated branch, a board that does not declare `archived` REJECTS this move instead of silently + landing the card there — so the duplicate is never archived and keeps sitting on the operator's board + as live work, already stamped `deterministicDuplicateOf`. + + REVERT CHECK, measured: with the literal `"archived"` restored, this fails — `moveTask` is called with + `"archived"` on a board whose archive lane is `boxed`. + */ + it("archives a deterministic duplicate into the workflow's OWN archive lane", async () => { + const canonicalTs = new Date(Date.now() - 2_000).toISOString(); + const createdTs = new Date().toISOString(); + const canonical = mkTask({ id: "FN-1", title: INPUT.title, description: INPUT.description, column: "todo", createdAt: canonicalTs, updatedAt: canonicalTs, source: { sourceType: "api", sourceMetadata: { contentFingerprint: "fp" } } }); + const created = mkTask({ id: "FN-2", title: INPUT.title, description: INPUT.description, column: "todo", createdAt: createdTs, updatedAt: createdTs, source: { sourceType: "api", sourceMetadata: { contentFingerprint: "fp" } } }); + const { store } = makeStore([canonical, created]); + vi.spyOn(store, "findRecentTasksByContentFingerprint").mockResolvedValueOnce([canonical, created]); + /* A workflow whose archive lane is NOT the legacy id. Everything else is irrelevant to this path. */ + const ir = { + version: "v2", id: "dup-lifecycle", name: "dup", + columns: [ + { id: "todo", name: "Todo", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] }, + { id: "boxed", name: "Boxed", traits: [{ trait: "archived" }] }, + ], + nodes: [{ id: "start", kind: "start", column: "todo" }], + edges: [], + }; + Object.assign(store, { + getTaskWorkflowSelectionAsync: vi.fn(async () => ({ workflowId: "dup-lifecycle", stepIds: [] })), + getTaskWorkflowSelection: vi.fn(() => ({ workflowId: "dup-lifecycle", stepIds: [] })), + getWorkflowDefinition: vi.fn(async (id: string) => (id === "dup-lifecycle" ? { ir } : undefined)), + }); + + const result = await reconcileDeterministicDuplicate(store, { createdTask: created, fingerprint: "fp" }); + + expect(result).toEqual({ outcome: "archived", canonical }); + expect(store.moveTask).toHaveBeenCalledWith("FN-2", "boxed"); + expect(store.moveTask).not.toHaveBeenCalledWith("FN-2", "archived"); + }); + it("fails open when archive move throws", async () => { const canonicalTs = new Date(Date.now() - 2_000).toISOString(); const createdTs = new Date().toISOString(); diff --git a/packages/core/src/__tests__/move-target-resolvers.test.ts b/packages/core/src/__tests__/move-target-resolvers.test.ts new file mode 100644 index 0000000000..352cb438ce --- /dev/null +++ b/packages/core/src/__tests__/move-target-resolvers.test.ts @@ -0,0 +1,102 @@ +/* +FNXC:WorkflowResolvedColumns 2026-07-30-21:00 (census-invisible moveTask destinations): +The two MOVE-TARGET resolvers, which are the other half of a lifecycle conversion. + +The census is an AST scan for COMPARISONS, so a `moveTask` DESTINATION — a call argument — is invisible +to it. Seven production call sites now route through these two functions instead of a hardcoded id; one +definition each, so they cannot drift apart. See +`docs/solutions/architecture-patterns/hardcoded-movetask-destinations-are-census-invisible.md`. + +The fallbacks are load-bearing, not defensive padding: `resolveWorkflowIrForTask` degrades to the +BUILT-IN IR rather than throwing, and the built-in board's rebound/archive lanes ARE `todo`/`archived` — +so the fallback cases below also pin that a default board is byte-identical after the conversion. + +REVERT CHECK, measured: replacing either body with a bare `return ""` fails its renamed case. +*/ +import { describe, expect, it, vi } from "vitest"; +import type { WorkflowIr } from "../workflow-ir-types.js"; +import type { WorkflowIrResolverStore } from "../workflow-ir-resolver.js"; +import { resolveReboundTargetForTask, resolveArchiveTargetForTask, resolveWipTargetForTask } from "../workflow-lifecycle-traits.js"; + +function storeWith(ir: WorkflowIr | undefined): WorkflowIrResolverStore { + return { + getTaskWorkflowSelectionAsync: vi.fn(async () => (ir ? { workflowId: "wf", stepIds: [] } : undefined)), + getTaskWorkflowSelection: vi.fn(() => (ir ? { workflowId: "wf", stepIds: [] } : undefined)), + getWorkflowDefinition: vi.fn(async (id: string) => (id === "wf" && ir ? { ir } : undefined)), + } as unknown as WorkflowIrResolverStore; +} + +const throwingStore = { + getTaskWorkflowSelectionAsync: vi.fn(async () => { throw new Error("store unavailable"); }), + getTaskWorkflowSelection: vi.fn(() => { throw new Error("store unavailable"); }), + getWorkflowDefinition: vi.fn(async () => undefined), +} as unknown as WorkflowIrResolverStore; + +/** A board sharing no ids with the legacy vocabulary. */ +const RENAMED: WorkflowIr = { + version: "v2", + id: "wf", + name: "renamed", + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "boxed", name: "Boxed", traits: [{ trait: "archived" }] }, + ], + nodes: [{ id: "start", kind: "start", column: "backlog" }], + edges: [], +} as unknown as WorkflowIr; + +/* +FNXC:WorkflowResolvedColumns 2026-07-30-19:30 (#2808 review — coderabbit): +The two `.not.toBe(legacyId)` cases are DELETED, and the comment that called one of them +"Non-vacuous" had it exactly backwards. + +Each sat directly beneath a positive case asserting `.toBe("backlog")` / `.toBe("boxed")`. An exact +equality is strictly stronger than a negation: nothing can satisfy `toBe("backlog")` and still return +`"todo"`. So the negatives could not fail unless the positive had already failed, and they would have +passed for any wrong-but-not-legacy id — which is the weakness they claimed to be guarding against. + +The remaining four cases pin all four outcomes exactly: the resolved lane, and the legacy fallback for +both an unresolvable workflow and a throwing lookup. +*/ +describe("resolveReboundTargetForTask", () => { + it("resolves the board's own hold lane", async () => { + await expect(resolveReboundTargetForTask(storeWith(RENAMED), "FN-1")).resolves.toBe("backlog"); + }); + + it("falls back to the legacy id when no workflow resolves", async () => { + await expect(resolveReboundTargetForTask(storeWith(undefined), "FN-1")).resolves.toBe("todo"); + }); + + it("falls back to the legacy id when the lookup throws", async () => { + await expect(resolveReboundTargetForTask(throwingStore, "FN-1")).resolves.toBe("todo"); + }); +}); + +describe("resolveArchiveTargetForTask", () => { + it("resolves the board's own archive lane", async () => { + await expect(resolveArchiveTargetForTask(storeWith(RENAMED), "FN-1")).resolves.toBe("boxed"); + }); + + it("falls back to the legacy id when no workflow resolves", async () => { + await expect(resolveArchiveTargetForTask(storeWith(undefined), "FN-1")).resolves.toBe("archived"); + }); + + it("falls back to the legacy id when the lookup throws", async () => { + await expect(resolveArchiveTargetForTask(throwingStore, "FN-1")).resolves.toBe("archived"); + }); +}); + +describe("resolveWipTargetForTask", () => { + it("resolves the board's own wip lane", async () => { + await expect(resolveWipTargetForTask(storeWith(RENAMED), "FN-1")).resolves.toBe("building"); + }); + + it("falls back to the legacy id when no workflow resolves", async () => { + await expect(resolveWipTargetForTask(storeWith(undefined), "FN-1")).resolves.toBe("in-progress"); + }); + + it("falls back to the legacy id when the lookup throws", async () => { + await expect(resolveWipTargetForTask(throwingStore, "FN-1")).resolves.toBe("in-progress"); + }); +}); diff --git a/packages/core/src/duplicate-guard.ts b/packages/core/src/duplicate-guard.ts index 4e4214072b..b00bf927b2 100644 --- a/packages/core/src/duplicate-guard.ts +++ b/packages/core/src/duplicate-guard.ts @@ -1,6 +1,7 @@ import type { Task } from "./types.js"; import type { TaskStore } from "./store.js"; import { computeContentFingerprint } from "./duplicate-detection.js"; +import { resolveArchiveTargetForTask } from "./workflow-lifecycle-traits.js"; /* FNXC:TaskCreationDeduplication 2026-07-26-06:45: @@ -203,7 +204,39 @@ export async function reconcileDeterministicDuplicate( deterministicDuplicateOf: olderSibling.id, }, }); - await store.moveTask(args.createdTask.id, "archived"); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-19:45 (#2808 review — coderabbit): + COMPENSATED, not merely documented. + + The previous note here described this hazard and shipped it: the row is stamped + `deterministicDuplicateOf` BEFORE the move, and `moveTask` rejects a destination the workflow does + not declare. A rejection therefore left a task marked as an archived duplicate while still sitting + in an active lane — visible on the board, counted as live, and permanently mislabelled. Describing + a defect is not resolving it. + + The stamp is rolled back and the original error rethrown, so a failed archive leaves the task + exactly as it was found. Compensation rather than reordering because the stamp is deliberately + written first — a `task:moved` subscriber reading `deterministicDuplicateOf` would see a different + row if the move came first, and this fix should not quietly change that ordering. + + The rollback is best-effort: if it also fails, the original move error still surfaces, because + that is the one that explains what went wrong. + */ + try { + await store.moveTask(args.createdTask.id, await resolveArchiveTargetForTask(store, args.createdTask.id)); + } catch (moveError) { + try { + await store.updateTask(args.createdTask.id, { + sourceMetadataPatch: { deterministicDuplicateOf: null }, + }); + } catch (rollbackError) { + args.logger?.warn("Failed to roll back the deterministic-duplicate stamp after a rejected archive move", { + taskId: args.createdTask.id, + error: rollbackError instanceof Error ? rollbackError.message : String(rollbackError), + }); + } + throw moveError; + } try { await store.recordActivity({ diff --git a/packages/core/src/duplicate-intake.ts b/packages/core/src/duplicate-intake.ts index 63d687de63..b83dc60ca2 100644 --- a/packages/core/src/duplicate-intake.ts +++ b/packages/core/src/duplicate-intake.ts @@ -2,6 +2,7 @@ import { isTerminalColumnRole, type ColumnRoleTraitFlags } from "./column-roles. import { computeContentFingerprint, findDuplicateMatches, tokenize } from "./duplicate-detection.js"; import type { ColumnId } from "./types.js"; import type { TaskStore } from "./store.js"; +import { resolveArchiveTargetForTask } from "./workflow-lifecycle-traits.js"; export interface SameAgentDuplicateInput { title?: string | null; @@ -324,7 +325,7 @@ export async function archiveAsSameAgentDuplicate( details: "Auto-archived as same-agent duplicate during intake", metadata: { siblingTaskIds: siblingIds, scores }, }); - await store.moveTask(taskId, "archived"); + await store.moveTask(taskId, await resolveArchiveTargetForTask(store, taskId)); } /** @@ -421,3 +422,4 @@ export async function flagTriageDuplicate( await store.updateTask(taskId, { sourceMetadataPatch }); return sourceMetadataPatch; } + diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index e94395687d..97d3e54aa2 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -467,7 +467,7 @@ export { createWorkflowEventBus, getWorkflowEventBus, emitWorkflowLifecycleEvent export type { WorkflowEventBus, WorkflowEventSubscriber, WorkflowEventSubscription } from "./workflow-events.js"; export { findWorkflowEventShapeViolations, isIdsOnlyWorkflowEvent, MAX_ID_VALUE_LENGTH, IMPLEMENTATION_EXITS } from "./types/workflow-events.js"; export type { WorkflowLifecycleEvent, WorkflowLifecycleEventType, WorkflowLifecycleEventBase, TaskTransitionedEvent, NodeEnteredEvent, NodeCompletedEvent, RunSuspendedEvent, RunResumedEvent, WorkflowEventShapeViolation, ImplementationExit } from "./types/workflow-events.js"; -export { columnsWithFlag, columnHasFlag, resolveReboundTarget, resolveCompleteColumn, resolveMergeOrchestrationColumn, resolveLifecycleColumns, resolveTaskLifecycleColumns, resolveTerminalColumns, resolveReviewColumns, declaresAnyLifecycleTrait } from "./workflow-lifecycle-traits.js"; +export { columnHasFlag, columnsWithFlag, declaresAnyLifecycleTrait, resolveArchiveTargetForTask, resolveCompleteColumn, resolveLifecycleColumns, resolveMergeOrchestrationColumn, resolveReboundTarget, resolveReboundTargetForTask, resolveReviewColumns, resolveTaskLifecycleColumns, resolveTerminalColumns, resolveWipTargetForTask } from "./workflow-lifecycle-traits.js"; export type { LifecycleColumns } from "./workflow-lifecycle-traits.js"; export { resolveReviewLevelSteps, applyReviewLevelPreset } from "./review-level-preset.js"; export { diff --git a/packages/core/src/workflow-lifecycle-traits.ts b/packages/core/src/workflow-lifecycle-traits.ts index 1d19c2c57e..2298f3c282 100644 --- a/packages/core/src/workflow-lifecycle-traits.ts +++ b/packages/core/src/workflow-lifecycle-traits.ts @@ -388,3 +388,63 @@ export async function resolveTaskLifecycleColumns( return undefined; } } + +/* +FNXC:WorkflowResolvedColumns 2026-07-30-20:50 (census-invisible moveTask destinations): +MOVE-TARGET resolvers, kept beside `resolveTaskLifecycleColumns` because they answer the same question +for the other half of a conversion. + +The lifecycle-column census is an AST scan for COMPARISONS, so a `moveTask` DESTINATION — a call +argument — is invisible to it. 51 such destinations exist in production; 22 deliberately pass +`recoveryRehome: true` (the #1411 legacy safe-landing escape, which must not be converted), and the rest +are rejected outright on a board that does not declare the target now that U12 hoisted the +`workflowHasColumn` check out of its dead flag-gated branch. See +`docs/solutions/architecture-patterns/hardcoded-movetask-destinations-are-census-invisible.md`. + +Both fall back to the legacy id: `resolveWorkflowIrForTask` degrades to the BUILT-IN IR rather than +throwing, so a board whose workflow cannot be read behaves exactly as before. + +ONE definition each, rather than a copy per call site — four sites already needed the rebound target and +they must not drift apart. +*/ +export async function resolveReboundTargetForTask(store: WorkflowIrResolverStore, taskId: string): Promise { + try { + const ir = await resolveWorkflowIrForTask(store, taskId); + if (ir) { + const target = resolveReboundTarget(ir); + if (target) return target; + } + } catch { /* degraded: legacy id */ } + return "todo"; +} + +/** + * The WIP lane this task's workflow declares, or the legacy id. See above. + * + * FIRST `countsTowardWip` column, deliberately: this answers "where does a card go when it re-enters + * execution?", which is a single destination, not a membership test. Callers asking "is this card in + * WIP?" want `columnsWithFlag(ir, "countsTowardWip")` instead — a board may declare several. + */ +export async function resolveWipTargetForTask(store: WorkflowIrResolverStore, taskId: string): Promise { + try { + const ir = await resolveWorkflowIrForTask(store, taskId); + if (ir) { + const wip = columnsWithFlag(ir, "countsTowardWip"); + if (wip.length > 0) return wip[0]; + } + } catch { /* degraded: legacy id */ } + return "in-progress"; +} + +/** The archive lane this task's workflow declares, or the legacy id. See above. */ +export async function resolveArchiveTargetForTask(store: WorkflowIrResolverStore, taskId: string): Promise { + try { + const ir = await resolveWorkflowIrForTask(store, taskId); + if (ir) { + const archived = columnsWithFlag(ir, "archived"); + if (archived.length > 0) return archived[0]; + } + } catch { /* degraded: legacy id */ } + return "archived"; +} + diff --git a/packages/engine/src/auto-recovery-handlers/contamination.ts b/packages/engine/src/auto-recovery-handlers/contamination.ts index 53d4bcbb7c..46d01da72f 100644 --- a/packages/engine/src/auto-recovery-handlers/contamination.ts +++ b/packages/engine/src/auto-recovery-handlers/contamination.ts @@ -1,4 +1,5 @@ import type { TaskStore } from "@fusion/core"; +import { resolveReboundTargetForTask } from "@fusion/core"; import { classifyForeignOnlyContamination } from "../branch-conflicts.js"; import type { AutoRecoveryContext, AutoRecoveryDecision, AutoRecoveryFailure, AutoRecoveryHandlers } from "../auto-recovery.js"; import { createLogger, type Logger } from "../logger.js"; @@ -81,7 +82,12 @@ export class ContaminationAutoRecoveryHandler implements Pick[2]); + /* FNXC:WorkflowResolvedColumns 2026-07-30-22:20: census-invisible moveTask DESTINATION — a call argument, not a comparison. */ + await store.moveTask(task.id, await resolveReboundTargetForTask(store, task.id), { preserveProgress: true, moveSource: "engine" } as Parameters[2]); // FN-7551: the attempt just dispatched — record it as attemptCount + 1 // (decision.attemptCount is the count BEFORE this dispatch). await this.emitOverseerInterventionSafe(() => @@ -4491,7 +4494,8 @@ export class ProjectEngine { error: null, verificationFailureCount: nextBounces, }); - await store.moveTask(taskId, "in-progress"); + /* FNXC:WorkflowResolvedColumns 2026-07-30-21:40: census-invisible moveTask DESTINATION — a call argument, not a comparison. */ + await store.moveTask(taskId, await resolveWipTargetForTask(store, taskId)); await store.logEntry( taskId, `Deterministic ${failedKind} verification failed (${nextBounces}/${cap}) — moved back to in-progress with status=merging-fix for remediation`, @@ -4607,7 +4611,8 @@ export class ProjectEngine { error: null, mergeConflictBounceCount: nextBounces, }); - await store.moveTask(taskId, "in-progress"); + /* FNXC:WorkflowResolvedColumns 2026-07-30-21:40: census-invisible moveTask DESTINATION — a call argument, not a comparison. */ + await store.moveTask(taskId, await resolveWipTargetForTask(store, taskId)); await store.logEntry( taskId, `Auto-merge conflicts unresolved (${maxAutoMergeRetriesOnErr}/${maxAutoMergeRetriesOnErr}) — bounced to in-progress for re-rebase (bounce ${nextBounces}/${bounceCap})`, diff --git a/packages/engine/src/recovery/foreign-only-contamination.ts b/packages/engine/src/recovery/foreign-only-contamination.ts index 4203694484..f72c061ba6 100644 --- a/packages/engine/src/recovery/foreign-only-contamination.ts +++ b/packages/engine/src/recovery/foreign-only-contamination.ts @@ -2,6 +2,7 @@ import { exec } from "node:child_process"; import { existsSync } from "node:fs"; import { promisify } from "node:util"; import type { Task, TaskStore } from "@fusion/core"; +import { resolveReboundTargetForTask } from "@fusion/core"; import { activeSessionRegistry } from "../active-session-registry.js"; import { classifyForeignOnlyContamination, @@ -72,7 +73,12 @@ export async function recoverForeignOnlyContamination( taskId: task.id, }); - await deps.taskStore.moveTask(task.id, "todo", { + /* FNXC:WorkflowResolvedColumns 2026-07-30-19:55 (#2808 review — coderabbit): census-invisible moveTask + DESTINATION — a call argument, not a comparison, so the census never scored it. This requeue is not a + #1411 `recoveryRehome` escape, so an undeclared destination is REJECTED and the recovery never completes: + that is what the hardcoded `todo` used to cause on any board without that column. The destination now + comes from the task's own workflow, and the legacy id remains only as the unresolvable fallback. */ + await deps.taskStore.moveTask(task.id, await resolveReboundTargetForTask(deps.taskStore, task.id), { moveSource: "engine", preserveResumeState: true, preserveProgress: true, @@ -105,7 +111,12 @@ export async function recoverForeignOnlyContamination( await execAsync("git worktree prune", { cwd: deps.repoDir, timeout: GIT_TIMEOUT_MS, maxBuffer: GIT_MAX_BUFFER }).catch(() => undefined); await execAsync(`git branch -D ${quote(task.branch)}`, { cwd: deps.repoDir, timeout: GIT_TIMEOUT_MS, maxBuffer: GIT_MAX_BUFFER }).catch(() => undefined); - await deps.taskStore.moveTask(task.id, "todo", { + /* FNXC:WorkflowResolvedColumns 2026-07-30-19:55 (#2808 review — coderabbit): census-invisible moveTask + DESTINATION — a call argument, not a comparison, so the census never scored it. This requeue is not a + #1411 `recoveryRehome` escape, so an undeclared destination is REJECTED and the recovery never completes: + that is what the hardcoded `todo` used to cause on any board without that column. The destination now + comes from the task's own workflow, and the legacy id remains only as the unresolvable fallback. */ + await deps.taskStore.moveTask(task.id, await resolveReboundTargetForTask(deps.taskStore, task.id), { moveSource: "engine", preserveResumeState: true, preserveProgress: true, diff --git a/packages/engine/src/restart-recovery-coordinator.ts b/packages/engine/src/restart-recovery-coordinator.ts index 918e88999b..ca9fb9c375 100644 --- a/packages/engine/src/restart-recovery-coordinator.ts +++ b/packages/engine/src/restart-recovery-coordinator.ts @@ -1,4 +1,5 @@ import type { Task, TaskStore } from "@fusion/core"; +import { resolveReboundTargetForTask } from "@fusion/core"; import type { TaskExecutor } from "./executor.js"; import { createLogger } from "./logger.js"; import { setImmediate as setImmediateCb } from "node:timers"; @@ -201,6 +202,7 @@ export class RestartRecoveryCoordinator { task.id, "Restart recovery: interrupted run had no step progress and no fn_task_done — requeued to todo for safe retry", ); - await this.store.moveTask(task.id, "todo"); + /* FNXC:WorkflowResolvedColumns 2026-07-30-20:50: census-invisible moveTask DESTINATION — a call argument, not a comparison. This requeue is not a #1411 `recoveryRehome` escape, so on a board that does not declare `todo` the move is REJECTED and the recovery it belongs to never completes. */ + await this.store.moveTask(task.id, await resolveReboundTargetForTask(this.store, task.id)); } } diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index 803eb6fd22..bc972b81df 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -13,7 +13,6 @@ "packages/dashboard/app/components/TaskDetailModal.tsx": 4, "packages/engine/src/replan-target.ts": 4, "packages/core/src/async-mission-store-queries.ts": 3, - "packages/core/src/task-store/async-merge-coordination.ts": 3, "packages/core/src/task-store/task-artifacts-ops.ts": 3, "packages/dashboard/app/components/DockTaskList.tsx": 3, "packages/dashboard/app/components/TaskCard.tsx": 3, @@ -22,20 +21,14 @@ "packages/dashboard/app/hooks/useTaskDiffStats.ts": 3, "packages/dashboard/app/utils/taskActivity.ts": 3, "packages/dashboard/app/utils/worktreeGrouping.ts": 3, - "packages/dashboard/src/chat.ts": 3, "packages/dashboard/src/routes/register-task-workflow-routes.ts": 3, "packages/engine/src/planner-overseer.ts": 3, "packages/core/src/agent-store.ts": 2, - "packages/core/src/async-mission-store.ts": 2, "packages/core/src/node-override-guard.ts": 2, - "packages/core/src/task-move-disposer.ts": 2, - "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, "packages/core/src/task-store/task-id-integrity.ts": 2, "packages/dashboard/app/components/Board.tsx": 2, "packages/dashboard/app/components/DocumentsView.tsx": 2, @@ -43,7 +36,6 @@ "packages/dashboard/app/components/WorkflowResultsTab.tsx": 2, "packages/dashboard/app/utils/taskRevert.ts": 2, "packages/dashboard/src/github-tracking-state.ts": 2, - "packages/dashboard/src/server.ts": 2, "packages/engine/src/auto-merge-finalization.ts": 2, "packages/core/src/eval-automation.ts": 1, "packages/core/src/eval-signal-collector.ts": 1, @@ -51,6 +43,7 @@ "packages/core/src/mission-store.ts": 1, "packages/core/src/plugin-store.ts": 1, "packages/core/src/stalled-review-detector.ts": 1, + "packages/core/src/task-store/comments-ops.ts": 1, "packages/core/src/task-store/lifecycle-ops.ts": 1, "packages/core/src/task-store/merge-queue-ops-2.ts": 1, "packages/core/src/task-store/merge-queue-ops.ts": 1, @@ -68,19 +61,7 @@ "packages/dashboard/app/utils/quickAddStart.ts": 1, "packages/dashboard/app/utils/stalePausedReviewCopy.ts": 1, "packages/dashboard/app/utils/taskStuck.ts": 1, - "packages/dashboard/src/github-issue-comment.ts": 1, - "packages/dashboard/src/github-tracking-comments.ts": 1, - "packages/dashboard/src/gitlab-issue-comment.ts": 1, - "packages/dashboard/src/gitlab-source-issue-reconciler.ts": 1, - "packages/dashboard/src/gitlab-tracking-comments.ts": 1, - "packages/dashboard/src/knowledge-index-refresh.ts": 1, - "packages/dashboard/src/planning-board-tools.ts": 1, - "packages/dashboard/src/research-routes.ts": 1, - "packages/dashboard/src/routes/register-agent-core-routes.ts": 1, - "packages/dashboard/src/routes/register-chat-routes.ts": 1, - "packages/dashboard/src/task-planner-chat-context.ts": 1, "packages/dashboard/src/task-planner-chat-metrics.ts": 1, - "packages/dashboard/src/test/mockCoreEngine.ts": 1, "packages/engine/src/backlog-pressure-reporter.ts": 1, "packages/engine/src/ephemeral-worker-manager.ts": 1, "packages/engine/src/merger.ts": 1, @@ -115,6 +96,9 @@ "packages/core/src/store.ts\u0000done": 1, "packages/core/src/store.ts\u0000in-progress": 1, "packages/core/src/store.ts\u0000todo": 1, + "packages/core/src/task-move-disposer.ts\u0000in-progress": 1, + "packages/core/src/task-move-disposer.ts\u0000todo": 1, + "packages/core/src/task-store/archive-lifecycle-2.ts\u0000archived": 1, "packages/core/src/task-store/branch-and-pr-entities.ts\u0000archived": 1, "packages/core/src/task-store/task-store-helpers.ts\u0000in-progress": 1, "packages/core/src/task-store/task-store-helpers.ts\u0000todo": 1, @@ -132,12 +116,17 @@ "packages/dashboard/app/components/TaskContextMenu.tsx\u0000triage": 1, "packages/dashboard/app/components/TaskDetailModal.tsx\u0000todo": 1, "packages/dashboard/app/utils/columnRoles.ts\u0000todo": 1, + "packages/dashboard/src/github-tracking-comments.ts\u0000done": 1, "packages/dashboard/src/github-tracking-state.ts\u0000archived": 1, "packages/dashboard/src/github-tracking-state.ts\u0000done": 1, + "packages/dashboard/src/gitlab-tracking-comments.ts\u0000in-progress": 1, "packages/dashboard/src/reliability-metrics.ts\u0000done": 1, "packages/dashboard/src/reliability-metrics.ts\u0000in-progress": 1, "packages/dashboard/src/routes/register-task-workflow-routes.ts\u0000todo": 1, "packages/dashboard/src/routes/register-task-workflow-routes.ts\u0000triage": 1, + "packages/dashboard/src/server.ts\u0000archived": 1, + "packages/dashboard/src/task-planner-chat-context.ts\u0000done": 1, + "packages/dashboard/src/test/mockCoreEngine.ts\u0000in-review": 1, "packages/engine/src/agent-heartbeat.ts\u0000archived": 1, "packages/engine/src/agent-heartbeat.ts\u0000done": 1, "packages/engine/src/cli-agent/task-session.ts\u0000done": 1,