diff --git a/packages/core/src/__tests__/postgres/merge-queue-renamed-review-lane.pg.test.ts b/packages/core/src/__tests__/postgres/merge-queue-renamed-review-lane.pg.test.ts new file mode 100644 index 0000000000..f973bfa9f8 --- /dev/null +++ b/packages/core/src/__tests__/postgres/merge-queue-renamed-review-lane.pg.test.ts @@ -0,0 +1,120 @@ +/* +FNXC:WorkflowResolvedColumns 2026-07-30-07:55 (batch-core): + +THE MERGE QUEUE MUST FILL AND DRAIN ON A BOARD THAT RENAMED ITS REVIEW LANE. + +`enqueueMergeQueueInTransaction` admitted a card only from the literal `in-review`, and +`dequeueMergeQueueOnColumnExitInTransaction` removed it only when leaving that same literal. Both run +inside the move transaction with a `tx` handle and no store, so neither could resolve the lane itself +— the resolved set now comes from `moves.ts`, which already resolves the workflow for the move. + +WHY THE PAIR IS TESTED TOGETHER AND NOT SEPARATELY. Converting one half alone is undetectable: if +enqueue keeps the literal, nothing is ever queued on a renamed board, so a dequeue test has nothing to +observe and passes vacuously. If dequeue keeps the literal, the queue fills and never drains — a leak +that only shows up on the SECOND move. One test drives the whole cycle so neither half can regress +without failing. + +WHY A LIVE STORE. The bug is which column id the guard compares against, and that id comes from the +task's own persisted workflow. A mock handing back a lifecycle struct would assert my own assumption +about what `resolveReviewColumns` returns — the substitution that has produced vacuous tests all +through this program. This drives real PostgreSQL and asserts on OBSERVED QUEUE ROWS. + +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("merge queue fills and drains on a renamed review lane", () => { + const harness = createSharedPgTaskStoreTestHarness({ prefix: "fusion_mq_renamed_review" }); + + beforeAll(harness.beforeAll); + afterAll(harness.afterAll); + beforeEach(async () => { await harness.beforeEach(); }); + afterEach(async () => { await harness.afterEach(); }); + + /** `signoff` carries the merge trait; this board declares no `in-review` column at all. */ + async function renamedReviewWorkflow(store: ReturnType) { + return store.createWorkflowDefinition({ + name: "renamed-review", + ir: { + version: "v2", + name: "renamed-review", + columns: [ + { id: "inbox", name: "Inbox", traits: [{ trait: "intake" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip" }] }, + { id: "signoff", name: "Signoff", traits: [{ trait: "merge" }] }, + { 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("enqueues on entering the renamed review lane and dequeues on leaving it", async () => { + const store = harness.store(); + const definition = await renamedReviewWorkflow(store); + const task = await store.createTask({ description: "renamed review card", workflowId: definition.id } as never); + + /* Adjacency is derived from the graph, so the card walks the board rather than jumping. */ + await store.moveTask(task.id, "building" as never, { bypassGuards: true } as never); + /* + The merge queue is filled by the COMPLETION HANDOFF, not by a bare move into the review lane — + `moveTask` alone walks the board without enqueuing. Driving the real handoff is what exercises + `enqueueMergeQueueInTransaction`, which is the guard under test. + */ + await store.handoffToReview(task.id, { ownerAgentId: null, evidence: { reason: "test", runId: "r1", agentId: "test" } } as never); + + /* + Half one. With the literal, `taskRow.column !== "in-review"` rejected every card on this board and + the enqueue recorded a `mergeQueue:enqueue-rejected` audit instead of a row — the merge queue was + simply never used, and nothing surfaced that as an error. + */ + expect(await store.getMergeQueuedTaskIdsAsync()).toContain(task.id); + + await store.moveTask(task.id, "building" as never, { bypassGuards: true } as never); + + /* + Half two, and the one that fails independently: with dequeue on the literal, `previousColumn !== + "in-review"` is true for `signoff`, so the helper returns early and the row stays. The queue would + fill and never drain, and the leak only becomes visible on this second move. + */ + expect(await store.getMergeQueuedTaskIdsAsync()).not.toContain(task.id); + }); + + it("does NOT dequeue when moving between two review lanes on the same board", async () => { + /* + The paired negative, and the reason the set is broad rather than `lifecycle.review`. A board may + declare a merge-orchestration lane AND a separate human sign-off lane; a card moving between them + has not left review. A single-id answer would treat the second lane as "outside review" and drop + the card out of the merge queue mid-review. + */ + const store = harness.store(); + const definition = await store.createWorkflowDefinition({ + name: "two-review-lanes", + ir: { + version: "v2", + name: "two-review-lanes", + columns: [ + { id: "inbox", name: "Inbox", traits: [{ trait: "intake" }] }, + { id: "signoff", name: "Signoff", traits: [{ trait: "merge" }] }, + { id: "approval", name: "Approval", traits: [{ trait: "human-review" }] }, + { 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); + const task = await store.createTask({ description: "two-lane card", workflowId: definition.id } as never); + + await store.handoffToReview(task.id, { ownerAgentId: null, evidence: { reason: "test", runId: "r1", agentId: "test" } } as never); + expect(await store.getMergeQueuedTaskIdsAsync()).toContain(task.id); + + await store.moveTask(task.id, "approval" as never, { bypassGuards: true } as never); + + expect(await store.getMergeQueuedTaskIdsAsync()).toContain(task.id); + }); +}); diff --git a/packages/core/src/__tests__/postgres/store-archive-reads.pg.test.ts b/packages/core/src/__tests__/postgres/store-archive-reads.pg.test.ts index 2c28709bba..071ffc1649 100644 --- a/packages/core/src/__tests__/postgres/store-archive-reads.pg.test.ts +++ b/packages/core/src/__tests__/postgres/store-archive-reads.pg.test.ts @@ -22,6 +22,45 @@ pgDescribe("TaskStore archived read parity (PostgreSQL)", () => { afterEach(h.afterEach); afterAll(h.afterAll); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-18:50 (batch-core): + + A CARD SITTING IN THE BOARD'S OWN ARCHIVE LANE IS ALREADY ARCHIVED. + + `archiveTask` refuses a card that is already archived. Keyed on the literal, a board whose archive + lane is named `attic` did not refuse — the card was archived a second time, from a lane the board + itself calls archived. + + THE FIXTURE MATTERS, and my first version of this test was vacuous because of it. Calling + `archiveTask` first does NOT produce a renamed-lane card: the archive path stamps `column: + "archived"` (`archiveEntryToTask`, serialization.ts:353), so the guard only ever sees the literal + and both the literal and resolved forms pass. The card has to be MOVED into `attic` by an ordinary + move for the renamed lane to reach the guard at all. + + The unarchive side is deliberately not covered here: its input always carries the literal by + construction, which is recorded at that guard. + */ + it("refuses to archive a card already sitting in the board's renamed archive lane", async () => { + const store = h.store(); + const definition = await store.createWorkflowDefinition({ + name: "renamed-archive", + ir: { + version: "v2", + name: "renamed-archive", + columns: [ + { id: "building", name: "Building", traits: [{ trait: "wip" }] }, + { id: "attic", name: "Attic", traits: [{ trait: "archived" }] }, + ], + nodes: [{ id: "start", kind: "start", column: "building" }, { id: "end", kind: "end", column: "attic" }], + edges: [{ from: "start", to: "end" }], + }, + } as never); + const task = await store.createTask({ description: "already in the attic", workflowId: definition.id } as never); + await store.moveTask(task.id, "attic" as never, { bypassGuards: true } as never); + + await expect(store.archiveTask(task.id, { cleanup: false } as never)).rejects.toThrow(/already archived/); + }); + it("composes archived snapshots into list, search, and detail reads", async () => { const store = h.store(); const first = await store.createTaskWithReservedId( diff --git a/packages/core/src/__tests__/postgres/store-comments.pg.test.ts b/packages/core/src/__tests__/postgres/store-comments.pg.test.ts index acde5dfced..61d0ca405f 100644 --- a/packages/core/src/__tests__/postgres/store-comments.pg.test.ts +++ b/packages/core/src/__tests__/postgres/store-comments.pg.test.ts @@ -109,6 +109,48 @@ pgTest("TaskStore addComment steering + refinement (PostgreSQL)", () => { afterEach(h.afterEach); afterAll(h.afterAll); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-17:55 (batch-core): + + AUTO-REFINEMENT MUST FIRE FOR A TASK FINISHED IN A RENAMED COMPLETE LANE. + + A user comment on finished work creates a refinement task. Keyed on `task.column === "done"`, a + renamed board never entered that branch — the operator got silence where the feature promises a + follow-up, and nothing was logged because the branch was not reached rather than failing inside it. + + Driven through the real store and a real workflow definition so the assertion is on an OBSERVED + refinement row, not on my own belief about what the resolver returns for a renamed lineage. + */ + it("creates a refinement for a user comment on a task in a RENAMED complete lane", async () => { + const store = h.store(); + const definition = await store.createWorkflowDefinition({ + name: "renamed-complete", + ir: { + version: "v2", + name: "renamed-complete", + columns: [ + { id: "building", name: "Building", traits: [{ trait: "wip" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], + nodes: [{ id: "start", kind: "start", column: "building" }, { id: "end", kind: "end", column: "shipped" }], + edges: [{ from: "start", to: "end" }], + }, + } as never); + const task = await store.createTask({ description: "finished on a renamed board", workflowId: definition.id } as never); + await store.moveTask(task.id, "shipped" as never, { bypassGuards: true } as never); + + const before = (await store.listTasks({ slim: true } as never)).length; + await store.addComment(task.id, "please also handle the empty case", "user" as never); + + /* + The witness is a NEW task existing, not a mock call: `refineTask` creates the follow-up, and + asserting on the row proves the branch ran end to end rather than that a spy was invoked. + */ + const after = await store.listTasks({ slim: true } as never); + expect(after.length).toBe(before + 1); + expect(after.some((t: { description?: string }) => (t.description ?? "").includes("empty case"))).toBe(true); + }); + it("adds a steering comment and persists it", async () => { const store = h.store(); const task = await store.createTask({ description: "steering target" }); diff --git a/packages/core/src/__tests__/symbol-locks.test.ts b/packages/core/src/__tests__/symbol-locks.test.ts index 7bad5853ff..7f60a8915a 100644 --- a/packages/core/src/__tests__/symbol-locks.test.ts +++ b/packages/core/src/__tests__/symbol-locks.test.ts @@ -100,4 +100,41 @@ pgDescribe("TaskStore durable symbol locks", () => { )); expect((await store.reconcileStaleSymbolLocks()).reconciled).toContain("pkg/terminal.ts#a"); }); + + /* + FNXC:WorkflowResolvedColumns 2026-07-30-16:20 (batch-core): + + A FINISHED OWNER ON A RENAMED BOARD MUST RELEASE ITS SYMBOL LOCK. + + The reclaim asked `owner.column === "done" || owner.column === "archived"`. On a board that renames + either lane a finished owner never read as terminal, so its lock was held until expiry — and every + other task needing that symbol waited behind a task that had already completed. The case above + covers the DEFAULT board and cannot see this. + + Driven through the real store and a real workflow definition, so the assertion is on OBSERVED + reclaim rather than on my own belief about what the resolver returns for a renamed lineage. + */ + it("reconciles an owner finished in a RENAMED complete lane", async () => { + const store = h.store(); const layer = h.layer(); const projectId = layer.projectId?.trim() || "__legacy_unscoped__"; + const definition = await store.createWorkflowDefinition({ + name: "renamed-terminal", + ir: { + version: "v2", + name: "renamed-terminal", + columns: [ + { id: "building", name: "Building", traits: [{ trait: "wip" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], + nodes: [{ id: "start", kind: "start", column: "building" }, { id: "end", kind: "end", column: "shipped" }], + edges: [{ from: "start", to: "end" }], + }, + } as never); + const owner = await store.createTask({ description: "renamed terminal owner", workflowId: definition.id } as never); + await store.acquireSymbolLocks(["pkg/renamed.ts#A"], { ownerTaskId: owner.id }, 60_000); + await layer.db.update(schema.project.tasks).set({ column: "shipped" }).where(and( + eq(schema.project.tasks.projectId, projectId), eq(schema.project.tasks.id, owner.id), + )); + + expect((await store.reconcileStaleSymbolLocks()).reconciled).toContain("pkg/renamed.ts#a"); + }); }); diff --git a/packages/core/src/__tests__/task-move-disposer.test.ts b/packages/core/src/__tests__/task-move-disposer.test.ts index 367448f548..1b1a0c604f 100644 --- a/packages/core/src/__tests__/task-move-disposer.test.ts +++ b/packages/core/src/__tests__/task-move-disposer.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from "vitest"; +import "@fusion/core"; // registers the built-in column traits import { __setTaskMoveDisposalTimeoutForTesting, disposeTaskBeforeMove, @@ -34,6 +35,78 @@ describe("task move disposer", () => { expect(moveReady).toBe(true); }); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-15:35 (batch-core): + + THE HARD CANCEL MUST FIRE ON A RENAMED BOARD. + + Keyed on `from === "in-progress" && to === "todo"`, this returned early for every board that renamed + either lane, so the disposer never ran: a user pulling a card out of active execution got a task + that LOOKS parked while its agent kept running. A cancellation contract failing OPEN — the operator + believes the work stopped. + + The resolution is LAZY, taken only when the literals do not already match, because an unconditional + await pushed the disposer past the one-microtask window the test above pins. That is why this case + drives a board whose lanes share NO id with the legacy pair. + */ + it("fires the hard cancel on a RENAMED board", async () => { + const renamedIr = { + version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "hold" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip" }] }, + ], + }; + const selection = { workflowId: "wf-renamed", stepIds: [] as string[] }; + const store = { + getTaskWorkflowSelection: () => selection, + getTaskWorkflowSelectionAsync: async () => selection, + getWorkflowDefinition: async () => ({ id: "wf-renamed", ir: renamedIr }), + } as never; + const disposer = vi.fn().mockResolvedValue(undefined); + registerTaskMoveDisposer(store, disposer); + + await disposeTaskBeforeMove(store, { + task: { id: "FN-RENAMED" } as never, + from: "building", + to: "backlog", + source: "user", + }); + + expect(disposer).toHaveBeenCalledOnce(); + }); + + it("does NOT fire for a renamed move that is not wip -> pre-wip", async () => { + /* + The paired negative: resolving lanes must not turn every user move into a cancel. `backlog` is + the hold lane, so hold -> wip is a start, not a cancel. + */ + const renamedIr = { + version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "hold" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip" }] }, + ], + }; + const selection = { workflowId: "wf-renamed", stepIds: [] as string[] }; + const store = { + getTaskWorkflowSelection: () => selection, + getTaskWorkflowSelectionAsync: async () => selection, + getWorkflowDefinition: async () => ({ id: "wf-renamed", ir: renamedIr }), + } as never; + const disposer = vi.fn().mockResolvedValue(undefined); + registerTaskMoveDisposer(store, disposer); + + await disposeTaskBeforeMove(store, { + task: { id: "FN-START" } as never, + from: "backlog", + to: "building", + source: "user", + }); + + expect(disposer).not.toHaveBeenCalled(); + }); + it("awaits every executor registered to the same store", async () => { const store = {} as never; const first = vi.fn().mockResolvedValue(undefined); diff --git a/packages/core/src/async-mission-store.ts b/packages/core/src/async-mission-store.ts index 65e0937d55..4878a7a0d5 100644 --- a/packages/core/src/async-mission-store.ts +++ b/packages/core/src/async-mission-store.ts @@ -1,4 +1,6 @@ import { createLogger } from "./logger.js"; +import { columnsWithFlag, declaresAnyLifecycleTrait } from "./workflow-lifecycle-traits.js"; +import { resolveWorkflowIrForTask } from "./workflow-ir-resolver.js"; const severityAuditLog = createLogger("core-async-mission-store"); /** @@ -1073,12 +1075,35 @@ export class AsyncMissionStore extends EventEmitter { return updated; } + /* + FNXC:WorkflowResolvedColumns 2026-07-30-12:50 (batch-core): + "Is this linked task ARCHIVED?" for the two mission guards below, resolved from the task's own + workflow. Keyed on the literal, a renamed board answered NO for every archived card: `deleteFeature` + treated an archived task as still live and refused the delete without `force`, and feature bootstrap + accepted an archived task as an active target. + + `taskStore` is optional on this class, and a workflow that expresses no trait at all is a v1 upgrade + rather than a board without an archive lane — both keep the legacy id, which is the behaviour these + guards already had. + */ + private async archivedLanesFor(taskId: string): Promise> { + if (!this.taskStore) return new Set(["archived"]); + try { + const ir = await resolveWorkflowIrForTask(this.taskStore, taskId); + if (!ir || !declaresAnyLifecycleTrait(ir)) return new Set(["archived"]); + const archived = columnsWithFlag(ir, "archived"); + return archived.length > 0 ? new Set(archived) : new Set(["archived"]); + } catch { + return new Set(["archived"]); + } + } + async deleteFeature(id: string, force = false): Promise { const feature = await getFeature(this.db, id); if (!feature) throw new Error(`Feature ${id} not found`); if (feature.taskId) { const linkedTask = await getLiveTaskById(this.db, feature.taskId); - const linkedToLiveTask = linkedTask && linkedTask.column !== "archived"; + const linkedToLiveTask = linkedTask && !(await this.archivedLanesFor(feature.taskId)).has(linkedTask.column); if (linkedToLiveTask && !force) { throw new Error(`Feature ${id} is linked to task ${feature.taskId}; pass force to delete anyway`); } @@ -1262,7 +1287,7 @@ export class AsyncMissionStore extends EventEmitter { sql`${schema.project.tasks.deletedAt} is null`, )); const task = taskRows[0]; - if (!task || task.column === "archived") { + if (!task || (await this.archivedLanesFor(input.taskId)).has(task.column)) { throw new Error(`Cannot bootstrap feature ${input.featureId}: task ${input.taskId} is not active in this project`); } if (task.missionId !== input.missionId || task.sliceId !== input.sliceId) { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 92b2f77646..e94395687d 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 } from "./workflow-lifecycle-traits.js"; +export { columnsWithFlag, columnHasFlag, resolveReboundTarget, resolveCompleteColumn, resolveMergeOrchestrationColumn, resolveLifecycleColumns, resolveTaskLifecycleColumns, resolveTerminalColumns, resolveReviewColumns, declaresAnyLifecycleTrait } 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/task-move-disposer.ts b/packages/core/src/task-move-disposer.ts index 60777b9aac..33ce7a1d84 100644 --- a/packages/core/src/task-move-disposer.ts +++ b/packages/core/src/task-move-disposer.ts @@ -1,5 +1,7 @@ import type { TaskStore } from "./store.js"; import type { ColumnId, Task } from "./types.js"; +import { columnsWithFlag, declaresAnyLifecycleTrait } from "./workflow-lifecycle-traits.js"; +import { resolveWorkflowIrForTask } from "./workflow-ir-resolver.js"; export type TaskMoveSource = "user" | "engine" | "scheduler"; export type TaskMoveDisposer = (task: Task) => Promise; @@ -52,7 +54,60 @@ export function getTaskMoveDisposer(store: TaskStore): TaskMoveDisposer | undefi * board state can never claim the task is idle while its agent still runs. */ export async function disposeTaskBeforeMove(store: TaskStore, input: TaskMoveDisposalInput): Promise { - if (input.source !== "user" || input.from !== "in-progress" || input.to !== "todo") return; + /* + FNXC:WorkflowResolvedColumns 2026-07-30-15:20 (batch-core): + THE HARD CANCEL MUST FIRE ON A RENAMED BOARD. + + Keyed on the literals, this returned early for every board that renamed either lane — so the + disposer never ran, and a user pulling a card out of active execution got a task that LOOKS parked + while its agent is still running. A cancellation contract failing OPEN, which is the worst + direction: the operator believes the work stopped. + + Same defect and same direction as the `moveTaskInternal` hard-cancel guards, which resolved their + target as `hold ?? intake` for exactly this reason. Both halves are membership questions here — the + card LEFT a wip lane and ENTERED a pre-wip one — so both take the full sets rather than one id. + + A workflow expressing no trait at all is a v1 upgrade, not a board without these roles, so it keeps + the legacy pair; likewise an unresolvable workflow. Failing to dispose is the harm, so the fallback + stays exactly as permissive as before. + */ + if (input.source !== "user") return; + /* + RESOLVED ONLY WHEN THE LITERALS DO NOT ALREADY MATCH. + + Two reasons, and the second was found by this module's own test rather than reasoned out. First, + the legacy pair is what a default board uses, so short-circuiting keeps that path free of a + workflow read on every user move. Second, and load-bearing: `disposeTaskBeforeMove` is awaited by + the caller BEFORE the new column is published, and the existing test pins that the disposer starts + within one microtask. Adding an unconditional `await` ahead of it pushed the disposer past that + point — a real change to when cancellation begins on the ordinary path, for no benefit there. + + So the default board behaves exactly as before, and only a board whose lanes do NOT match the + legacy pair pays a resolution — which is precisely the case the literals got wrong. + */ + /* FNXC:WorkflowResolvedColumns 2026-07-30-15:50 DELIBERATE-LITERAL: a fast path, not the guard. + The legacy pair is what a default board uses, so matching it short-circuits the workflow read. + The actual lane decision is the RESOLVED membership test inside this block; these two ids only + decide whether resolution is needed, and answering "no" for them is always correct because they + are exactly the pair the resolved test would have matched anyway. */ + if (input.from !== "in-progress" || input.to !== "todo") { + let wipLanes: ReadonlySet = new Set(); + let preWipLanes: ReadonlySet = new Set(); + try { + const ir = await resolveWorkflowIrForTask(store, input.task.id); + if (ir && declaresAnyLifecycleTrait(ir)) { + wipLanes = new Set(columnsWithFlag(ir, "countsTowardWip")); + preWipLanes = new Set([...columnsWithFlag(ir, "intake"), ...columnsWithFlag(ir, "hold")]); + } + } catch { /* degraded: no resolved lanes, so the legacy pair above is the only match */ } + /* + A user move out of a WIP lane into a pre-WIP one is the hard cancel. Keyed on the literals this + returned early for every renamed board, so the disposer never ran and the operator got a card that + LOOKS parked while its agent is still running — a cancellation contract failing OPEN, the same + direction and the same defect as the `moveTaskInternal` hard-cancel guards. + */ + if (!wipLanes.has(input.from) || !preWipLanes.has(input.to)) return; + } const disposer = getTaskMoveDisposer(store); if (!disposer) return; diff --git a/packages/core/src/task-store/archive-lifecycle-2.ts b/packages/core/src/task-store/archive-lifecycle-2.ts index 9a022a9ee1..4e1a46ec99 100644 --- a/packages/core/src/task-store/archive-lifecycle-2.ts +++ b/packages/core/src/task-store/archive-lifecycle-2.ts @@ -7,6 +7,8 @@ * instance as its first parameter and performs byte-identical work. */ import {TaskStore, storeLog} from "../store.js"; +import { columnsWithFlag, declaresAnyLifecycleTrait } from "../workflow-lifecycle-traits.js"; +import { resolveWorkflowIrForTask } from "../workflow-ir-resolver.js"; import {getFeatureByTaskId as getMissionFeatureByTaskId, unlinkFeatureFromTaskId as unlinkMissionFeatureFromTaskId, recordGeneratedFixOperatorStop} from "../async-mission-store-queries.js"; import {TaskHasLineageChildrenError, TaskNotFoundError, TaskSelfDeleteError} from "./errors.js"; import {mkdir, writeFile} from "node:fs/promises"; @@ -241,6 +243,28 @@ export async function deleteTaskIfBackendImpl( }); } + +/* +FNXC:WorkflowResolvedColumns 2026-07-30-18:20 (batch-core): +The archived lanes for one task, resolved from its own workflow. Shared by the archive and unarchive +guards below so the two cannot disagree about what "archived" means — one refusing a card the other +would accept is the half-converted-pair shape. + +A workflow expressing NO trait on any column is a v1 upgrade (`synthesizeDefaultColumns` emits +`traits: []` everywhere) rather than a board without an archive lane, so it keeps the legacy id — as +does a workflow that cannot be read. +*/ +async function archivedLanesForTask(store: TaskStore, taskId: string): Promise> { + const lanes = new Set(["archived"]); + try { + const ir = await resolveWorkflowIrForTask(store, taskId); + if (ir && declaresAnyLifecycleTrait(ir)) { + for (const id of columnsWithFlag(ir, "archived")) lanes.add(id); + } + } catch { /* degraded: the legacy id */ } + return lanes; +} + export async function archiveTaskBackendImpl(store: TaskStore, id: string, optionsOrCleanup: boolean | { cleanup?: boolean; removeLineageReferences?: boolean },): Promise { const layer = store.asyncLayer!; const cleanup = typeof optionsOrCleanup === "boolean" ? optionsOrCleanup : optionsOrCleanup.cleanup !== false; @@ -251,7 +275,12 @@ export async function archiveTaskBackendImpl(store: TaskStore, id: string, optio if (!task) { throw new Error(`Task ${id} not found`); } - if (task.column === "archived") { + /* + FNXC:WorkflowResolvedColumns 2026-07-30-18:20 (batch-core): + Keyed on the literal, a renamed board let an ALREADY-archived card be archived again — a second + archive pass over a row the archive already owns. + */ + if ((await archivedLanesForTask(store, id)).has(task.column)) { throw new Error(`Cannot archive ${id}: task is already archived`); } @@ -392,6 +421,19 @@ export async function unarchiveTaskImpl(store: TaskStore, id: string): Promise, ): Promise { // Read the task row for the column check + priority. const taskRows = await tx @@ -159,7 +172,7 @@ export async function enqueueMergeQueueInTransaction( if (!taskRow) { throw new MergeQueueTaskNotFoundError(taskId); } - if (taskRow.column !== "in-review") { + if (!(reviewColumns ?? new Set(["in-review"])).has(taskRow.column)) { // Record the rejection inside the transaction so it rolls back with the // caller's write if the caller aborts. await recordRunAuditEventWithinTransaction(tx, { @@ -738,8 +751,16 @@ export async function dequeueMergeQueueOnColumnExitInTransaction( previousColumn: string, nextColumn: string, now: string, + reviewColumns?: ReadonlySet, ): Promise { - if (previousColumn !== "in-review" || nextColumn === "in-review") { + /* + FNXC:WorkflowResolvedColumns 2026-07-30-07:40 (batch-core): + The dequeue half of the pair — see `enqueueMergeQueueInTransaction` for why the set is passed in and + why the two must share it. "Left the review lane" is a MEMBERSHIP question on both sides: a board + may declare a merge lane and a separate sign-off lane, and moving between them is not an exit. + */ + const review = reviewColumns ?? new Set(["in-review"]); + if (!review.has(previousColumn) || review.has(nextColumn)) { return; } diff --git a/packages/core/src/task-store/comments-ops.ts b/packages/core/src/task-store/comments-ops.ts index d95e85d878..97f560699a 100644 --- a/packages/core/src/task-store/comments-ops.ts +++ b/packages/core/src/task-store/comments-ops.ts @@ -1,4 +1,5 @@ import { createLogger } from "../logger.js"; +import { columnsWithFlag, declaresAnyLifecycleTrait } from "../workflow-lifecycle-traits.js"; const severityAuditLog = createLogger("core-comments-ops"); /** @@ -152,7 +153,28 @@ export async function addCommentImpl(store: TaskStore, id: string, text: string, // This remains best-effort: failures are logged for observability but never // fail the comment add operation itself. // Steering comments skip refinement — they are injected into the agent stream instead. - if (task.column === "done" && author === "user" && !options?.skipRefinement) { + /* + FNXC:WorkflowResolvedColumns 2026-07-30-17:40 (batch-core): + Auto-refinement fires for a user comment on a FINISHED task. Keyed on the literal, a renamed board + never created one — an operator commenting on completed work got silence where the feature promises + a follow-up task, with nothing logged because the branch was simply never entered. + + Complete only, not the landed set: the original fired on `done` alone, and widening it to archival + would create refinements from comments on archived work that the literal never touched. + + A workflow expressing no trait at all is a v1 upgrade rather than a board without a complete lane, + so it keeps the legacy id. + */ + const refinementLanes = new Set(["done"]); + if (author === "user" && !options?.skipRefinement) { + try { + const ir = await resolveWorkflowIrForTask(store, id); + if (ir && declaresAnyLifecycleTrait(ir)) { + for (const columnId of columnsWithFlag(ir, "complete")) refinementLanes.add(columnId); + } + } catch { /* degraded: the legacy id */ } + } + if (refinementLanes.has(task.column) && author === "user" && !options?.skipRefinement) { try { await store.refineTask(id, text); } catch (err) { diff --git a/packages/core/src/task-store/moves.ts b/packages/core/src/task-store/moves.ts index 8ecf7443ab..7cceb472e8 100644 --- a/packages/core/src/task-store/moves.ts +++ b/packages/core/src/task-store/moves.ts @@ -27,6 +27,7 @@ import { } from "../workflow-transition-policy.js"; import {type DefaultWorkflowMoveContext, applyDefaultWorkflowMoveEffects, isReopenIntoPlanning} from "../default-workflow-hooks.js"; import {columnsWithFlag, resolveLifecycleColumns, resolveReviewColumns} from "../workflow-lifecycle-traits.js"; +import {resolveWorkflowIrForTask} from "../workflow-ir-resolver.js"; import {makeTransitionRejection, makeTransitionPending} from "../transition-types.js"; import {writeTransitionPendingAsync, clearTransitionPendingAsync} from "./async-transition-pending.js"; import type {WorkflowIr} from "../workflow-ir-types.js"; @@ -314,9 +315,32 @@ export async function handoffToReviewImpl(store: TaskStore, taskId: string, opts ); } + /* + FNXC:WorkflowResolvedColumns 2026-07-30-08:20 (batch-core): + HANDOFF TARGETS THE BOARD'S OWN REVIEW LANE. + + This was the literal `"in-review"`, and the consequence is worse than a guard that fails to + match: `moveTaskInternal` REJECTS a target the workflow does not declare + (`TransitionRejectionError: unknown-column`). So on any board that renamed its review lane, + completion handoff did not silently no-op — it THREW, and every task finishing implementation + failed to reach review at all. + + Found by a merge-queue regression test that tried to drive the real handoff path; the merge-queue + guards below could never have been exercised on a renamed board because nothing could get a card + into review in the first place. + + A SINGLE ID, not the broad set: this is a move TARGET, and a move takes exactly one column. That + is the opposite arity from the enqueue/dequeue membership guards in this same file — same + vocabulary, different question. `lifecycle.review` is the first `mergeOrchestration` lane, which + is the lane a completion handoff belongs in; a `humanReview`-only lane is somewhere a card can BE + in review, not somewhere the engine should PUT it. + */ + const handoffIr = await resolveWorkflowIrForTask(store, taskId).catch(() => undefined); + const handoffTarget = (handoffIr ? resolveLifecycleColumns(handoffIr)?.review : undefined) ?? "in-review"; + return store.moveTaskInternal( taskId, - "in-review", + handoffTarget, { ...opts.moveOptions, skipMergeBlocker: true, @@ -408,6 +432,22 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum the fallback: a move must behave exactly as before when there is no basis to resolve from. */ const moveLifecycle = workflowIr ? resolveLifecycleColumns(workflowIr) : undefined; + /* + FNXC:WorkflowResolvedColumns 2026-07-30-07:45 (batch-core): + The BROAD review set for the merge-queue pair below. Resolved once here, beside `moveLifecycle`, + and handed to both `enqueueMergeQueueInTransaction` and + `dequeueMergeQueueOnColumnExitInTransaction` — those run inside the move transaction with only a + `tx` handle and cannot resolve it themselves. + + Broad rather than `moveLifecycle.review`: enqueue/dequeue ask "is this card in / has it left a + review lane", which is MEMBERSHIP. A board may declare a merge-orchestration lane and a separate + human sign-off lane, and a card moving between them has not left review — the narrow single-id + answer would dequeue it and drop it out of the merge queue mid-review. + + `undefined` when the workflow could not be read, which is what makes the helpers fall back to the + legacy id rather than to an empty set that matches nothing. + */ + const moveReviewColumns = workflowIr ? new Set(resolveReviewColumns(workflowIr)) : undefined; if (task.column === toColumn) { if (internal.fromHandoff && toColumn === "in-review") { @@ -447,7 +487,7 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum await enqueueMergeQueueInTransaction(tx, id, { priority: task.priority, now: internal.now }, { agentId: internal.runContext?.agentId, runId: internal.runContext?.runId, - }); + }, moveReviewColumns); // FNXC:PostgresCutover 2026-07-15-12:00: // Same-column retries must share the outer handoff transaction too, // so workflow work cannot survive a rolled-back queue/audit handoff. @@ -769,7 +809,7 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum if (fromColumn === (moveLifecycle?.review ?? "in-review") && toColumn === (moveLifecycle?.complete ?? "done") && !options?.skipMergeBlocker) { /* - FNXC:WorkflowLifecycleColumns 2026-07-31-00:20 (batch-core feed): + FNXC:WorkflowLifecycleColumns 2026-07-30-00:20 (batch-core feed): Hand the merge blocker the lane this guard JUST resolved. The condition above resolves both columns from the workflow; the call below re-asked with the @@ -1083,7 +1123,7 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum }); // Dequeue from merge queue on column exit (if leaving in-review). - await dequeueMergeQueueOnColumnExitInTransaction(tx, id, fromColumn, toColumn, movedAt); + await dequeueMergeQueueOnColumnExitInTransaction(tx, id, fromColumn, toColumn, movedAt, moveReviewColumns); // FNXC:WorkflowReviewGates 2026-07-26-16:40: see isRecognizedInReviewEntry — a // graph-owned crossing into the review column is a legitimate arrival, not a violation. @@ -1113,7 +1153,7 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum await enqueueMergeQueueInTransaction(tx, id, { priority: task.priority, now: internal.now }, { agentId: internal.runContext?.agentId, runId: internal.runContext?.runId, - }); + }, moveReviewColumns); // FNXC:PostgresCutover 2026-06-27-10:25: // Thread the outer move transaction so cancel + upsert commit // atomically with the handoff (no orphaned merge-gate items on rollback). diff --git a/packages/core/src/task-store/symbol-locks.ts b/packages/core/src/task-store/symbol-locks.ts index 794fa6d641..a70c332f48 100644 --- a/packages/core/src/task-store/symbol-locks.ts +++ b/packages/core/src/task-store/symbol-locks.ts @@ -1,4 +1,7 @@ import { and, eq, gt, inArray, sql } from "drizzle-orm"; +import { columnsWithFlag, declaresAnyLifecycleTrait } from "../workflow-lifecycle-traits.js"; +import { resolveWorkflowIrForTask } from "../workflow-ir-resolver.js"; +import type { WorkflowIr } from "../workflow-ir-types.js"; import * as schema from "../postgres/schema/index.js"; import { projectOwnershipPartition, recordRunAuditEventWithinTransaction } from "../postgres/data-layer.js"; import type { DbTransaction } from "../postgres/data-layer.js"; @@ -209,9 +212,37 @@ export async function reconcileStaleSymbolLocksAsync(store: TaskStore): Promise< const held = await layer.db.select().from(schema.project.symbolLocks).where(and(eq(schema.project.symbolLocks.projectId, projectId), eq(schema.project.symbolLocks.status, "held"))); const stale: Array<{ symbolKey: string; ownerTaskId: string; expiresAt: string }> = []; const skipped: string[] = []; + /* + FNXC:WorkflowResolvedColumns 2026-07-30-16:05 (batch-core): + "Is the lock OWNER finished?" resolved from that owner's own workflow. Keyed on the literal pair, a + renamed board never recognised a finished owner, so its symbol lock was never reclaimed — held until + expiry while every other task needing that symbol waited behind a task that had already completed. + + Resolved per OWNER, because owners can run different workflows, and through a shared IR cache so a + sweep over N locks costs one workflow read per distinct workflow rather than per lock. + + A workflow expressing no trait at all is a v1 upgrade rather than a board without terminal lanes, so + it keeps the legacy pair — as does an unresolvable one. Failing to reclaim is the harm here, and the + fallback stays exactly as permissive as before. + */ + const terminalIrCache = new Map(); + const terminalLanesFor = async (taskId: string): Promise> => { + const lanes = new Set(["done", "archived"]); + try { + const ir = await resolveWorkflowIrForTask(store, taskId, terminalIrCache); + if (ir && declaresAnyLifecycleTrait(ir)) { + for (const id of columnsWithFlag(ir, "complete")) lanes.add(id); + for (const id of columnsWithFlag(ir, "archived")) lanes.add(id); + } + } catch { /* degraded: the legacy pair */ } + return lanes; + }; + for (const lock of held) { const owner = await store.getTask(lock.ownerTaskId, { includeDeleted: true }).catch(() => undefined); - const terminal = !owner || owner.deletedAt != null || owner.column === "done" || owner.column === "archived" || owner.status === "failed"; + const terminal = !owner || owner.deletedAt != null + || (await terminalLanesFor(lock.ownerTaskId)).has(owner.column) + || owner.status === "failed"; if (lock.expiresAt <= nowIso || terminal) { stale.push({ symbolKey: lock.symbolKey, ownerTaskId: lock.ownerTaskId, expiresAt: lock.expiresAt }); } else { diff --git a/packages/core/src/workflow-lifecycle-traits.ts b/packages/core/src/workflow-lifecycle-traits.ts index 75b9f9d354..1d19c2c57e 100644 --- a/packages/core/src/workflow-lifecycle-traits.ts +++ b/packages/core/src/workflow-lifecycle-traits.ts @@ -92,6 +92,32 @@ export function columnsWithFlag(ir: WorkflowIr, flag: keyof TraitFlags): string[ .map((c) => c.id); } +/* +FNXC:WorkflowResolvedColumns 2026-07-30-10:40 (batch-core): +DOES THIS WORKFLOW EXPRESS ANY LIFECYCLE TRAITS AT ALL? + +The distinction this program keeps paying for is "could not read" vs "read, and the answer is none". +There is a THIRD state that looks identical to the second and means the opposite: +`synthesizeDefaultColumns` (workflow-ir.ts:158-159) upgrades a v1 graph by emitting every default +column with `traits: []`. Such a board resolves cleanly and answers EMPTY for every role, while its +`done` and `in-review` columns plainly exist and hold cards. + +A caller that treats an empty role set as a real answer is correct for a v2 board that deliberately +declares no such lane, and wrong for a v1 upgrade — where it silently disables whatever the guard +protected. This predicate separates the two: a workflow that expresses NO trait on ANY column has not +made a statement about its lifecycle, so its callers should keep the legacy vocabulary rather than +conclude the role is absent. + +Cheap by construction: it stops at the first column carrying anything. +*/ +export function declaresAnyLifecycleTrait(ir: WorkflowIr): boolean { + const registry = getTraitRegistry(); + return columnsOf(ir).some((c) => { + const flags = registry.resolveColumnFlags(c); + return Object.values(flags).some((v) => v === true); + }); +} + /** Convenience predicate: does `columnId` carry `flag` in this IR? */ export function columnHasFlag(ir: WorkflowIr, columnId: string, flag: keyof TraitFlags): boolean { const column = columnsOf(ir).find((c) => c.id === columnId); diff --git a/packages/dashboard/src/__tests__/chat-manager.test.ts b/packages/dashboard/src/__tests__/chat-manager.test.ts index 0e77e5d725..ec12037aa9 100644 --- a/packages/dashboard/src/__tests__/chat-manager.test.ts +++ b/packages/dashboard/src/__tests__/chat-manager.test.ts @@ -2148,6 +2148,59 @@ describe("ChatManager.sendMessage", () => { }); }); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-05:25 (batch-core): + + THE REFINEMENT PAIR MUST RESOLVE THE SAME WAY ON BOTH HALVES. + + Two separate guards decide this feature: `createSession` REGISTERS the tool only for a finished + task, and the tool's own execute() REFUSES a source task that is not finished. Both compared + `column === "done"`, so on a renamed board the tool was never offered — and if only the + registration half had been converted, the tool would have been offered and then refused itself. + Half-converted pairs are the recurring failure in this program, so this asserts BOTH halves in one + case: the tool is present, and it actually creates the refinement. + + `shipped` carries `complete`; this board declares no `done` column at all. + */ + it("registers AND accepts the refinement tool for a task finished in a RENAMED complete lane", async () => { + mockChatStore.getSession.mockReturnValue({ id: "chat-001", agentId: "task-planner:FN-SHIPPED", status: "active" }); + const createResolvedSession = vi.fn(async () => ({ + session: { prompt: vi.fn().mockResolvedValue(undefined), dispose: vi.fn(), state: { messages: [] } }, + })); + __setCreateResolvedAgentSession(createResolvedSession as any); + + const renamedIr = { + version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [], + columns: [ + { id: "building", name: "Building", traits: [{ trait: "wip" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], + }; + const selection = { workflowId: "wf-renamed", stepIds: [] }; + const taskStore = { + getTask: vi.fn().mockResolvedValue({ id: "FN-SHIPPED", title: "Finished task", column: "shipped" }), + addSteeringComment: vi.fn(), + refineTask: vi.fn().mockResolvedValue({ id: "FN-REF2", description: "d", column: "triage", createdAt: "2026-07-30T00:00:00.000Z" }), + getSettings: vi.fn().mockResolvedValue({}), + getTaskWorkflowSelection: () => selection, + getTaskWorkflowSelectionAsync: async () => selection, + getWorkflowDefinition: async () => ({ id: "wf-renamed", ir: renamedIr }), + }; + const chatManager = new ChatManager(mockChatStore as any, "/tmp/test", mockAgentStore as any, undefined, undefined, undefined, taskStore as any); + + await chatManager.sendMessage("chat-001", "Follow up on this"); + + const createOptions = createResolvedSession.mock.calls[0]?.[0]; + const refinementTool = createOptions.customTools.find((tool: { name: string }) => tool.name === "fn_task_planner_create_refinement"); + /* Half one: registered. Keyed on the literal, this was undefined on a renamed board. */ + expect(refinementTool).toBeDefined(); + + const result = await refinementTool.execute("call-1", { feedback: "Add export support" }); + /* Half two: accepted. A registration-only fix would return isError here instead. */ + expect(result.isError).toBeUndefined(); + expect(taskStore.refineTask).toHaveBeenCalledWith("FN-SHIPPED", "Add export support"); + }); + it("does not register the refinement tool for live task-planner sessions", async () => { mockChatStore.getSession.mockReturnValue({ id: "chat-001", agentId: "task-planner:FN-LIVE", status: "active" }); const createResolvedSession = vi.fn(async () => ({ diff --git a/packages/dashboard/src/__tests__/github-issue-comment.test.ts b/packages/dashboard/src/__tests__/github-issue-comment.test.ts index 3e260b7d0b..1a948b8f93 100644 --- a/packages/dashboard/src/__tests__/github-issue-comment.test.ts +++ b/packages/dashboard/src/__tests__/github-issue-comment.test.ts @@ -178,6 +178,26 @@ describe("GitHubIssueCommentService", () => { expect(mockCommentOnIssue).not.toHaveBeenCalled(); }); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-04:50 (#2783 review — greptile): + ARCHIVAL IS NOT COMPLETION, AND THE CONVERSION MUST NOT WIDEN THE TRIGGER. + + Resolving this gate by role invited using the LANDED set (complete u archived), which I did in the + first pass. That silently changed behaviour on the DEFAULT board: `to === "done"` never fired on + archival, and the landed set does. A vocabulary conversion has to answer the same question under a + new name, not a bigger one — archival's source-issue behaviour is owned by the tracking commenter, + not this one. + + Pinned on the default board deliberately: this is a widening that no renamed-board fixture would + catch, because it is visible precisely where the legacy names still apply. + */ + it("does NOT comment when a task is archived rather than completed", async () => { + store.emit("task:moved", { task: createTask(), from: "done", to: "archived" }); + await flushAsync(); + + expect(mockCommentOnIssue).not.toHaveBeenCalled(); + }); + it("posts comment when setting enabled and task moved to done (non-self-repo, byte-for-byte unchanged)", async () => { mockCommentOnIssue.mockResolvedValue(undefined); diff --git a/packages/dashboard/src/__tests__/github-tracking-state.test.ts b/packages/dashboard/src/__tests__/github-tracking-state.test.ts index 6091697d08..1a6f6d6583 100644 --- a/packages/dashboard/src/__tests__/github-tracking-state.test.ts +++ b/packages/dashboard/src/__tests__/github-tracking-state.test.ts @@ -170,6 +170,47 @@ describe("GitHubTrackingStateService", () => { expect(mockSetIssueState).toHaveBeenCalledWith("owner", "repo", 42, "closed", "completed"); }); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-10:50 (batch-core — the THIRD state): + + A V1-UPGRADED BOARD STILL COMPLETES THINGS. + + This classifier deliberately treats a RESOLVED but EMPTY complete set as a real answer: a board + that declares no completion lane does not "complete" cards. That is right for a v2 board and wrong + for a v1 upgrade — `synthesizeDefaultColumns` emits every default column with `traits: []`, so the + IR resolves cleanly and every flag set is empty while `done` plainly exists and holds finished + cards. + + The consequence was invisible: `decideIssueAction` returned null for every transition, so tracking + NEVER closed a source issue on a v1 board — and because the source-issue commenter defers to this + service whenever tracking targets the same issue, neither posted. The completion comment vanished + with nothing logged. + + Not caught by the renamed-lane fixtures above, because they all express traits. The distinguishing + property is a workflow that expresses NONE. + */ + it("still closes the issue on a V1-UPGRADED board whose columns carry no traits", async () => { + const v1UpgradedIr = { + version: "v2", + id: "custom:v1", + name: "Legacy", + nodes: [], + edges: [], + columns: ["todo", "in-progress", "in-review", "done", "archived"].map((id) => ({ id, name: id, traits: [] })), + }; + const s = new MockStore(); + Object.assign(s, { + getTaskWorkflowSelection: () => ({ workflowId: "custom:v1", stepIds: [] }), + getWorkflowDefinition: async () => ({ ir: v1UpgradedIr }), + }); + new GitHubTrackingStateService(s as unknown as TaskStore).start(); + + s.emit("task:moved", { task: createTask(), from: "in-progress", to: "done" }); + await flushAsync(); + + expect(mockSetIssueState).toHaveBeenCalledWith("owner", "repo", 42, "closed", "completed"); + }); + it("maps a RENAMED archive lane to not_planned", async () => { const s = renamedStore(); new GitHubTrackingStateService(s as unknown as TaskStore).start(); diff --git a/packages/dashboard/src/__tests__/planning-board-tools.test.ts b/packages/dashboard/src/__tests__/planning-board-tools.test.ts index 7c81e99d44..8ffbc0404f 100644 --- a/packages/dashboard/src/__tests__/planning-board-tools.test.ts +++ b/packages/dashboard/src/__tests__/planning-board-tools.test.ts @@ -73,6 +73,49 @@ describe("createPlanningBoardTools", () => { expect(emptyResult.content[0]?.text).toBe("No active tasks."); }); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-06:25 (batch-core): + THE DUPLICATE CHECK MUST NOT LIST FINISHED WORK AS ACTIVE. + + `fn_task_list` exists so the planner can check for duplicates against work still in flight. Keyed on + `column !== "done"`, a renamed board listed every FINISHED task as active — the planner was told to + avoid duplicating work that was already complete, which is the opposite of the tool's purpose. + + It degrades quietly, which is why it needs a test rather than a bug report: the list is merely wrong, + not empty, so nothing looks broken. The case above pins the default board; this one pins a board + whose complete lane is `shipped` and which declares no `done` at all. + */ + it("excludes a task finished in a RENAMED complete lane", async () => { + const renamedIr = { + version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [], + columns: [ + { id: "building", name: "Building", traits: [{ trait: "wip" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], + }; + const selection = { workflowId: "wf-renamed", stepIds: [] }; + const store = { + listTasks: vi.fn(async () => [ + { id: "FN-1", column: "building", title: "Live", description: "Live description", dependencies: [] }, + { id: "FN-2", column: "shipped", title: "Finished", description: "Finished description", dependencies: [] }, + ]), + getTask: vi.fn(async () => { throw new Error("not found"); }), + getTaskWorkflowSelection: () => selection, + getTaskWorkflowSelectionAsync: async () => selection, + getWorkflowDefinition: async () => ({ id: "wf-renamed", ir: renamedIr }), + } as unknown as TaskStore; + + const result = await createPlanningBoardTools(store) + .find((tool) => tool.name === "fn_task_list")! + .execute("c1", {}); + + /* + Asserting the exact line rather than just "does not contain FN-2": a filter that dropped + everything would also satisfy the negative on its own. + */ + expect(result.content[0]?.text).toBe("FN-1 (building): Live"); + }); + it("fn_task_show returns full details and not-found fallback", async () => { const store = createStoreMock({ getTask: vi.fn(async (id: string) => ({ diff --git a/packages/dashboard/src/__tests__/session-diff-landed-lanes.test.ts b/packages/dashboard/src/__tests__/task-lifecycle-lanes.test.ts similarity index 58% rename from packages/dashboard/src/__tests__/session-diff-landed-lanes.test.ts rename to packages/dashboard/src/__tests__/task-lifecycle-lanes.test.ts index 04f1a4b0b5..00f07ee06d 100644 --- a/packages/dashboard/src/__tests__/session-diff-landed-lanes.test.ts +++ b/packages/dashboard/src/__tests__/task-lifecycle-lanes.test.ts @@ -1,7 +1,7 @@ // @vitest-environment node /* -FNXC:WorkflowResolvedColumns 2026-07-31-00:55 (batch-core): +FNXC:WorkflowResolvedColumns 2026-07-30-00:55 (batch-core): "HAS THIS TASK LANDED?" — THE QUESTION THAT PICKS THE DIFF BOUNDARY. @@ -10,7 +10,8 @@ branch) and a LIVE-BRANCH diff by comparing `task.column === "done"`. On a renam took the live-branch path, so its diff was computed against a branch that had been merged and usually deleted — an empty or misleading diff for exactly the tasks an operator reviews after the fact. -`landedColumnsForTask` holds that decision. Testing the seam rather than the routes is deliberate: +`landedColumnsForTask` holds that decision, and now lives in `task-lifecycle-lanes.ts` — shared with the +GitHub/GitLab source-issue commenters and the GitLab backfill reconciler, which all asked it separately. Testing the seam rather than the routes is deliberate: an HTTP fixture over these route registrars starts background work and hangs (measured while converting `register-git-github.ts`), and mocking git + the GitHub client to get past it is the mock-the-world shell FN-5048 tells us not to add. @@ -22,7 +23,7 @@ resolved set is EMPTY even though the columns exist) must BOTH keep the legacy p */ import { describe, expect, it, vi } from "vitest"; import "@fusion/core"; // registers the built-in column traits so flags resolve -import { landedColumnsForTask } from "../routes/register-session-diff-routes.js"; +import { landedColumnsForTask, completeColumnsForTask } from "../task-lifecycle-lanes.js"; function storeWith(ir: unknown, workflowId = "wf") { const selection = { workflowId, stepIds: [] as string[] }; @@ -74,3 +75,41 @@ describe("landedColumnsForTask", () => { expect([...(await landedColumnsForTask(store, "FN-1"))].sort()).toEqual(["archived", "done"]); }); }); + +/* +FNXC:WorkflowResolvedColumns 2026-07-30-03:35 (batch-core): +The narrower variant exists so a caller whose contract EXCLUDES archived work does not silently widen +to it. The GitLab backfill reconciler is that caller — its own note records that archived tasks live +in archiveDb and are intentionally excluded. Pinning the difference here is what stops the two being +"simplified" into one helper later, which would change that caller's behaviour without touching it. +*/ +describe("completeColumnsForTask is narrower than the landed set", () => { + it("returns the renamed complete lane and EXCLUDES the archived one", async () => { + const complete = await completeColumnsForTask(storeWith(RENAMED_IR), "FN-1"); + + expect([...complete]).toEqual(["shipped"]); + expect(complete.has("attic")).toBe(false); + }); + + it("falls back to `done` for a V1-UPGRADED workflow whose complete trait resolves to EMPTY", async () => { + /* + FNXC:WorkflowResolvedColumns 2026-07-30-08:50 (#2783 review — coderabbit): + The RESOLVED-BUT-UNEXPRESSED branch, which is a different code path from the `catch` below and was + the only one of the two left uncovered. `synthesizeDefaultColumns` emits every default column with + `traits: []`, so the IR resolves fine and `columnsWithFlag(ir, "complete")` returns []. Without + this case the `length > 0 ? ... : legacy` compatibility branch could regress to returning an empty + set — refusing every v1 board — while the suite stayed green on the catch path alone. + */ + expect([...(await completeColumnsForTask(storeWith(V1_UPGRADED_IR), "FN-1"))]).toEqual(["done"]); + }); + + it("falls back to `done` alone, not the legacy pair, when the workflow cannot be resolved", async () => { + const store = { + getTaskWorkflowSelectionAsync: async () => { throw new Error("unreadable"); }, + getTaskWorkflowSelection: () => { throw new Error("unreadable"); }, + getWorkflowDefinition: vi.fn(), + } as never; + + expect([...(await completeColumnsForTask(store, "FN-1"))]).toEqual(["done"]); + }); +}); diff --git a/packages/dashboard/src/chat.ts b/packages/dashboard/src/chat.ts index 9976b20f0f..c629e9719b 100644 --- a/packages/dashboard/src/chat.ts +++ b/packages/dashboard/src/chat.ts @@ -29,6 +29,7 @@ import type { TaskStore, PermanentAgentGatingContext, } from "@fusion/core"; +import { completeColumnsForTask, wipColumnsForTask } from "./task-lifecycle-lanes.js"; import type { AgentActionGateContext, SkillSelectionContext } from "@fusion/engine"; import { ApprovalRequestStore, @@ -515,7 +516,13 @@ function createTaskVerificationTools(taskStore: TaskStore, actionGateContext?: A const profile = typeof raw.profile === "string" ? raw.profile : "verify:fast"; if (!taskId || !profiles.has(profile)) return { content: [{ type: "text" as const, text: "ERROR: task_id and an allowlisted profile are required; raw commands are not accepted." }], isError: true, details: {} }; const task = await taskStore.getTask(taskId); - if (!task || task.column !== "in-progress" || !task.worktree || !existsSync(task.worktree)) return { content: [{ type: "text" as const, text: "ERROR: verification requires an in-progress task with a live executor worktree." }], isError: true, details: {} }; + /* + FNXC:WorkflowResolvedColumns 2026-07-30-05:05 (batch-core): + Verification requires a card that is actively being worked, resolved from its own workflow. + Keyed on the literal, chat-driven verification refused every task on a renamed board with + "requires an in-progress task" — naming a column that board does not have. + */ + if (!task || !(await wipColumnsForTask(taskStore, taskId)).has(task.column) || !task.worktree || !existsSync(task.worktree)) return { content: [{ type: "text" as const, text: "ERROR: verification requires an in-progress task with a live executor worktree." }], isError: true, details: {} }; const settings = await taskStore.getSettings(); const command = profile === "verify:fast" ? "pnpm verify:fast" : typeof settings.testCommand === "string" ? settings.testCommand : ""; if (!command) return { content: [{ type: "text" as const, text: "ERROR: the selected verification profile is not configured." }], isError: true, details: {} }; @@ -647,7 +654,14 @@ function createTaskPlannerRefinementTool(taskStore: TaskStore, taskId: string) { } try { const sourceTask = await taskStore.getTask(taskId); - if (sourceTask.column !== "done") { + /* + FNXC:WorkflowResolvedColumns 2026-07-30-05:05 (batch-core): + Refinement is for FINISHED work — complete only, not the landed set: an archived task is off + the board and is not a refinement source. Paired with the tool-registration guard in + `createSession`; if only one of the two resolved, the tool would either be offered and then + refuse, or be withheld from tasks it would have accepted. Both move together. + */ + if (!(await completeColumnsForTask(taskStore, taskId)).has(sourceTask.column)) { return { content: [{ type: "text" as const, text: `ERROR: Current task ${taskId} is ${sourceTask.column}; use planner steering for live tasks instead of creating a refinement.` }], details: { sourceTaskId: taskId, column: sourceTask.column }, @@ -2623,7 +2637,15 @@ export class ChatManager { FNXC:TaskDetailPlannerChat 2026-07-01-21:44: Done-task planner Chat uses a separate task-scoped refinement tool rather than Activity steering. The tool is registered only for synthetic task-planner sessions whose server-loaded current task is done, accepts only feedback text, and calls TaskStore.refineTask with the bound source id so models cannot route refinements to arbitrary tasks/projects/workflows. */ - const taskPlannerRefinementTools = this.taskStore && taskPlannerChatTaskId && taskPlannerTaskColumn === "done" + /* + FNXC:WorkflowResolvedColumns 2026-07-30-05:05 (batch-core): + The registration half of the refinement pair — see the guard inside the tool itself. Resolved + the same way so a renamed board offers the tool exactly where the tool would accept it. + */ + const taskPlannerTaskIsComplete = this.taskStore && taskPlannerChatTaskId + ? (await completeColumnsForTask(this.taskStore, taskPlannerChatTaskId)).has(taskPlannerTaskColumn) + : false; + const taskPlannerRefinementTools = this.taskStore && taskPlannerChatTaskId && taskPlannerTaskIsComplete ? [createTaskPlannerRefinementTool(this.taskStore, taskPlannerChatTaskId)] : []; diff --git a/packages/dashboard/src/github-issue-comment.ts b/packages/dashboard/src/github-issue-comment.ts index c1b73cc409..53f9be90cb 100644 --- a/packages/dashboard/src/github-issue-comment.ts +++ b/packages/dashboard/src/github-issue-comment.ts @@ -1,5 +1,6 @@ import type { TaskStore } from "@fusion/core"; import { GitHubClient } from "./github.js"; +import { completeColumnsForTask } from "./task-lifecycle-lanes.js"; import { getCliPackageVersion } from "./cli-package-version.js"; import { FUSION_SELF_REPO, @@ -115,7 +116,20 @@ export class GitHubIssueCommentService { } private async handleTaskMoved(event: TaskMovedEvent): Promise { - if (event.to !== "done") { + /* + FNXC:WorkflowResolvedColumns 2026-07-30-04:40 (batch-core, corrected after #2783 review): + "Did this task COMPLETE?" — resolved from the task's own workflow. Keyed on `done`, a board that + renamed its complete lane never commented on or closed its GitHub source issues at all: the + listener returned before reading a single setting, so the feature looked disabled rather than + broken. + + COMPLETE ONLY, not the landed set. My first pass used `hasTaskLanded` (complete u archived), which + WIDENED the trigger: on a default board archiving a task would have posted a source-issue comment + that the literal `to === "done"` never posted. A vocabulary conversion must answer the same + question under a new name, not a bigger one — and archival is a separate lifecycle event whose + source-issue behaviour is owned by the tracking commenter, not this one. + */ + if (!(await completeColumnsForTask(this.store, event.task.id)).has(event.to)) { return; } diff --git a/packages/dashboard/src/github-tracking-comments.ts b/packages/dashboard/src/github-tracking-comments.ts index 55faf8af5e..9f9a0b14b1 100644 --- a/packages/dashboard/src/github-tracking-comments.ts +++ b/packages/dashboard/src/github-tracking-comments.ts @@ -162,6 +162,19 @@ export function formatTrackingComment( linkContext?: TrackingLinkContext, options?: { currentVersion?: string | (() => string) }, ): string { + /* + FNXC:WorkflowResolvedColumns 2026-07-30-06:10 DELIBERATE-LITERAL: a transition KIND, not a board column. + + `transition` is the closed union `"in-progress" | "done"` declared in this function's own signature. + It names WHICH COMMENT TEMPLATE to render; the caller decides that from the task's resolved lanes and + passes the kind down. Resolving it against a workflow would be a category error — there is no task + column in scope here at all. + + The census matches on the spelling, so this reads as an unconverted lifecycle guard. It is the same + bare-variable false-positive class as the reports plugin's `ReportStatus`: the AST cannot tell a + foreign enum from a column id because the receiver name carries no type. Marked rather than left + counted, so it is not re-dispatched for conversion indefinitely. + */ if (transition === "done") { const currentVersion = options?.currentVersion; let comment = buildDoneComment(task, linkContext, { includeCommitSubject: true, includeFilesLine: true, currentVersion }); @@ -233,7 +246,7 @@ export class GitHubTrackingCommentService { FNXC:WorkflowResolvedColumns 2026-07-30-23:55 (fleet: github-tracking-comments.ts): Resolved ONCE here — after the tracking-enabled gate — so a move on an UNTRACKED task pays nothing. - FNXC:WorkflowResolvedColumns 2026-07-31-00:40 (PR #2715 review — greptile): + FNXC:WorkflowResolvedColumns 2026-07-30-00:40 (PR #2715 review — greptile): THE TRACKING GATE NOW RUNS FIRST, AND THE COLUMN TEST IS RESOLVED. An earlier version kept a literal `to !== "in-progress" && to !== "done"` early return ABOVE the @@ -303,7 +316,7 @@ export class GitHubTrackingCommentService { return; } /* - FNXC:WorkflowResolvedColumns 2026-07-31-00:40 (PR #2715 review — greptile): + FNXC:WorkflowResolvedColumns 2026-07-30-00:40 (PR #2715 review — greptile): `formatTrackingComment`'s second parameter is a TRANSITION KIND, not a column id — it chooses which comment to build. Passing `event.to` only type-checked because the literal early return had narrowed it to the two legacy ids, so the id and the kind coincided on the default board. They do diff --git a/packages/dashboard/src/github-tracking-state.ts b/packages/dashboard/src/github-tracking-state.ts index e0fa378aa9..70ba9aa532 100644 --- a/packages/dashboard/src/github-tracking-state.ts +++ b/packages/dashboard/src/github-tracking-state.ts @@ -2,7 +2,7 @@ import { createLogger } from "@fusion/core"; const severityAuditLog = createLogger("dashboard-github-tracking-state"); import type { GithubIssueAction, GlobalSettings, ProjectSettings, Task, TaskStore } from "@fusion/core"; -import { columnsWithFlag, resolveWorkflowIrForTask } from "@fusion/core"; +import { columnsWithFlag, declaresAnyLifecycleTrait, resolveWorkflowIrForTask } from "@fusion/core"; import { GitHubClient } from "./github.js"; import { resolveGithubTrackingAuth } from "./github-auth.js"; @@ -218,9 +218,29 @@ export class GitHubTrackingStateService { somewhere is not "completing" it on a board with no completion lane — so the empty set is used as-is rather than falling back to `done`. */ + /* + FNXC:WorkflowResolvedColumns 2026-07-30-10:45 (batch-core — the THIRD state): + The note above draws the right line between "could not read" and "read, and there is no complete + lane". There is a third case that looks exactly like the second and means the opposite, and this + classifier was silently on the wrong side of it. + + `synthesizeDefaultColumns` upgrades a v1 graph by emitting every default column with `traits: []`. + Such a board resolves cleanly, so `ir !== undefined`, and every flag set comes back EMPTY — while + its `done` column plainly exists and holds finished cards. Treating that as "this board does not + complete anything" made `decideIssueAction` return null for every transition, so on a v1-upgraded + board GitHub tracking NEVER closed a source issue. And because the source-issue commenter defers to + this service whenever tracking targets the same issue, neither of them posted: the completion + comment disappeared entirely, with nothing logged as an error. + + `declaresAnyLifecycleTrait` separates the two. A workflow that expresses no trait on ANY column has + not made a statement about its lifecycle and keeps the legacy vocabulary; a v2 board that declares + traits elsewhere but no complete lane still gets the empty set used as-is, which is the behaviour + the note above argues for and which remains correct. + */ const ir = await resolveWorkflowIrForTask(store, event.task.id).catch(() => undefined); - const completeLanes = ir === undefined ? undefined : columnsWithFlag(ir, "complete"); - const archivedLanes = ir === undefined ? undefined : columnsWithFlag(ir, "archived"); + const traitsExpressed = ir !== undefined && declaresAnyLifecycleTrait(ir); + const completeLanes = ir === undefined || !traitsExpressed ? undefined : columnsWithFlag(ir, "complete"); + const archivedLanes = ir === undefined || !traitsExpressed ? undefined : columnsWithFlag(ir, "archived"); const decision = decideIssueAction(event.from, event.to, (columnId) => ({ complete: completeLanes === undefined ? columnId === "done" : completeLanes.includes(columnId), archived: archivedLanes === undefined ? columnId === "archived" : archivedLanes.includes(columnId), diff --git a/packages/dashboard/src/gitlab-issue-comment.ts b/packages/dashboard/src/gitlab-issue-comment.ts index 43fed7b41d..bbd5dfac8e 100644 --- a/packages/dashboard/src/gitlab-issue-comment.ts +++ b/packages/dashboard/src/gitlab-issue-comment.ts @@ -1,5 +1,6 @@ import type { ProjectSettings, Task, TaskStore } from "@fusion/core"; import { resolveGitLabClient, resolveGitLabTarget, resolveGitLabTargetFromItem, safeLogGitLabEntry } from "./gitlab-lifecycle.js"; +import { completeColumnsForTask } from "./task-lifecycle-lanes.js"; import { getCliPackageVersion } from "./cli-package-version.js"; import { formatReleaseVersionLines } from "./fusion-release-version.js"; @@ -71,7 +72,17 @@ export class GitLabIssueCommentService { } private async handleTaskMoved(event: TaskMovedEvent): Promise { - if (event.to !== "done" || event.task.sourceIssue?.provider !== "gitlab") return; + /* + FNXC:WorkflowResolvedColumns 2026-07-30-04:40 (batch-core, corrected after #2783 review): + The GitLab twin of the GitHub commenter's completion check — same question, same literal, same + silent no-op on a renamed board. Provider is tested first because it is free; the lane resolution + is not. + + COMPLETE ONLY, matching its twin: the landed set would widen the trigger to archival, which the + original `to === "done"` never fired on. + */ + if (event.task.sourceIssue?.provider !== "gitlab") return; + if (!(await completeColumnsForTask(this.store, event.task.id)).has(event.to)) return; const settings = await this.store.getSettings() as Pick; if (settings.gitlabCommentOnDone !== true) return; diff --git a/packages/dashboard/src/gitlab-source-issue-reconciler.ts b/packages/dashboard/src/gitlab-source-issue-reconciler.ts index 44940e96d3..ae57473fcc 100644 --- a/packages/dashboard/src/gitlab-source-issue-reconciler.ts +++ b/packages/dashboard/src/gitlab-source-issue-reconciler.ts @@ -1,18 +1,36 @@ -import type { Task, TaskStore } from "@fusion/core"; +import type { Task, TaskStore, WorkflowIr } from "@fusion/core"; +import { completeColumnsForTask } from "./task-lifecycle-lanes.js"; import { resolveGitLabClient, resolveGitLabTarget, safeLogGitLabEntry } from "./gitlab-lifecycle.js"; export const GITLAB_RECONCILE_SCAN_LIMIT = 200; type BackfillResult = { scanned: number; filled: number; skipped: number; errors: number; hasMore: boolean }; -function hasDoneColumn(task: Pick): boolean { - return task.column === "done"; +/* +FNXC:WorkflowResolvedColumns 2026-07-30-03:25 (batch-core): +Candidacy is now resolved from each task's OWN workflow. Keyed on `done`, this backfill found nothing +on a renamed board and reported a clean scan — `scanned: N, filled: 0` reads as "nothing to do", so +the failure was indistinguishable from success. + +Two-stage on purpose: the CHEAP provider and closedAt tests run first and reject almost everything, +so the workflow read only happens for tasks that could actually be candidates. It also shares one IR +cache across the scan, making it one read per distinct workflow rather than per task. + +`completeColumnsForTask`, not the landed set: this backfill's own note records that archived tasks +live in archiveDb and are intentionally excluded, so it must not widen to the archived role. +*/ +function isGitLabSourceCandidate(task: Task): boolean { + return task.sourceIssue?.provider === "gitlab" && !task.sourceIssue.closedAt; } -function isGitLabBackfillCandidate(task: Task): boolean { - return hasDoneColumn(task) - && task.sourceIssue?.provider === "gitlab" - && !task.sourceIssue.closedAt; +async function filterGitLabBackfillCandidates(store: TaskStore, tasks: readonly Task[]): Promise { + const irCache = new Map(); + const candidates: Task[] = []; + for (const task of tasks) { + if (!isGitLabSourceCandidate(task)) continue; + if ((await completeColumnsForTask(store, task.id, irCache)).has(task.column)) candidates.push(task); + } + return candidates; } function normalizeProviderTimestamp(value: string | undefined): string | undefined { @@ -37,7 +55,7 @@ export class GitLabSourceIssueReconciler { const offset = Math.max(0, options?.offset ?? 0); const limit = Math.max(0, options?.limit ?? GITLAB_RECONCILE_SCAN_LIMIT); const listedTasks = await store.listTasks({ slim: false, includeArchived: false } as Parameters[0]); - const matchingTasks = (Array.isArray(listedTasks) ? listedTasks : []).filter(isGitLabBackfillCandidate); + const matchingTasks = await filterGitLabBackfillCandidates(store, Array.isArray(listedTasks) ? listedTasks : []); const tasks = matchingTasks.slice(offset, offset + limit); const hasMore = offset + limit < matchingTasks.length; diff --git a/packages/dashboard/src/gitlab-tracking-comments.ts b/packages/dashboard/src/gitlab-tracking-comments.ts index 718273dc6a..ab54452c0e 100644 --- a/packages/dashboard/src/gitlab-tracking-comments.ts +++ b/packages/dashboard/src/gitlab-tracking-comments.ts @@ -31,12 +31,25 @@ export function formatGitLabTrackingComment( options?: { repository?: string; currentVersion?: string | (() => string) }, ): string { /* - FNXC:WorkflowResolvedColumns 2026-07-31-09:10 (fleet phase — FLAGGED AND LEFT COUNTED, same as its GitHub twin): + FNXC:WorkflowResolvedColumns 2026-07-30-09:10 (fleet phase — FLAGGED AND LEFT COUNTED, same as its GitHub twin): A pure formatter. `transition` is this function's OWN `"in-progress" | "done"` parameter — a discriminant the caller chose, not a column id read off a task — and there is no store or task id in scope to resolve from. `github-tracking-comments.ts:165` is the same site with the same decision, so both halves of the pair now leave exactly one literal, in the same place, for the same reason. */ + /* + FNXC:WorkflowResolvedColumns 2026-07-30-06:10 DELIBERATE-LITERAL: a transition KIND, not a board column. + + `transition` is the closed union `"in-progress" | "done"` declared in this function's own signature. + It names WHICH COMMENT TEMPLATE to render; the caller decides that from the task's resolved lanes and + passes the kind down. Resolving it against a workflow would be a category error — there is no task + column in scope here at all. + + The census matches on the spelling, so this reads as an unconverted lifecycle guard. It is the same + bare-variable false-positive class as the reports plugin's `ReportStatus`: the AST cannot tell a + foreign enum from a column id because the receiver name carries no type. Marked rather than left + counted, so it is not re-dispatched for conversion indefinitely. + */ if (transition === "in-progress") { const prefix = `Fusion task: ${task.id}\n\n`; const stem = "🚧 In progress — work has started on “"; @@ -82,7 +95,7 @@ export class GitLabTrackingCommentService { private async handleTaskMoved(event: TaskMovedEvent): Promise { /* - FNXC:WorkflowResolvedColumns 2026-07-31-09:10 (fleet phase — the GitLab half of the pair): + FNXC:WorkflowResolvedColumns 2026-07-30-09:10 (fleet phase — the GitLab half of the pair): IDENTICAL shape and ordering to `github-tracking-comments.ts`'s `handleTaskMoved`, on purpose. That file's note explains the reordering: the lane test needs a resolved workflow, and resolving on EVERY move to discover that most moves are not notable is the cost worth avoiding — so the cheap tracked-item @@ -108,7 +121,7 @@ export class GitLabTrackingCommentService { return; } /* - FNXC:WorkflowResolvedColumns 2026-07-31-09:20 (fleet phase — a narrowing the old literal was doing for free): + FNXC:WorkflowResolvedColumns 2026-07-30-09:20 (fleet phase — a narrowing the old literal was doing for free): `transition` is derived EXPLICITLY rather than passed as `event.to`. The removed early return (`event.to !== "in-progress" && event.to !== "done"`) was not only a guard — it also NARROWED `event.to` to the formatter's `"in-progress" | "done"` parameter type. Comparing against resolved diff --git a/packages/dashboard/src/knowledge-index-refresh.ts b/packages/dashboard/src/knowledge-index-refresh.ts index 5bc000e1d6..be77472465 100644 --- a/packages/dashboard/src/knowledge-index-refresh.ts +++ b/packages/dashboard/src/knowledge-index-refresh.ts @@ -1,4 +1,5 @@ import type { TaskStore } from "@fusion/core"; +import { completeColumnsForTask } from "./task-lifecycle-lanes.js"; import { refreshKnowledgeForTask } from "./knowledge-index.js"; /** @@ -61,7 +62,16 @@ export class KnowledgeIndexRefreshService { } private async handleTaskMoved(store: TaskStore, event: TaskMovedEvent): Promise { - if (event.to !== "done") return; + /* + FNXC:WorkflowResolvedColumns 2026-07-30-06:05 (batch-core): + Knowledge is re-indexed when a task COMPLETES. Keyed on the literal, a board that renamed its + complete lane never refreshed the index for any task, so the knowledge index silently drifted + further from reality with every finished card and nothing reported it. + + Complete only, not the landed set: archival is a separate event and the original literal never + fired on it. Widening a role set during a rename is a behaviour change, which this is not. + */ + if (!(await completeColumnsForTask(store, event.task.id)).has(event.to)) return; await refreshKnowledgeForTask(store, event.task.id); } } diff --git a/packages/dashboard/src/planning-board-tools.ts b/packages/dashboard/src/planning-board-tools.ts index 6da3c6a18a..90cf6daf5d 100644 --- a/packages/dashboard/src/planning-board-tools.ts +++ b/packages/dashboard/src/planning-board-tools.ts @@ -1,5 +1,7 @@ import * as fusionCore from "@fusion/core"; import { MAX_TASK_LIST_TEXT_CHARS, type TaskStore } from "@fusion/core"; +import { completeColumnsForTask } from "./task-lifecycle-lanes.js"; +import type { WorkflowIr } from "@fusion/core"; import type { ToolDefinition } from "@earendil-works/pi-coding-agent"; type TaskListClamp = (lines: string[], opts?: { maxChars?: number }) => string; @@ -53,7 +55,21 @@ export function createPlanningBoardTools(store: TaskStore): ToolDefinition[] { parameters: { type: "object", properties: {}, additionalProperties: false }, execute: async () => { const tasks = await store.listTasks({ slim: true, includeArchived: false }); - const active = tasks.filter((t) => t.column !== "done"); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-06:05 (batch-core): + "Not finished yet" for the planner's duplicate check. Keyed on the literal, a renamed board + listed every FINISHED task as active, so the planner was told to check for duplicates against + work that was already done — the opposite of the tool's purpose, and it degrades quietly + because the list is merely wrong rather than empty. + + Complete only (the query already passes `includeArchived: false`), and one shared IR cache + across the scan so this costs one workflow read per distinct workflow rather than per task. + */ + const activeIrCache = new Map(); + const active: typeof tasks = []; + for (const t of tasks) { + if (!(await completeColumnsForTask(store, t.id, activeIrCache)).has(t.column)) active.push(t); + } if (active.length === 0) { return { content: [{ type: "text" as const, text: "No active tasks." }], diff --git a/packages/dashboard/src/research-routes.ts b/packages/dashboard/src/research-routes.ts index 8e0201e74e..d780322b81 100644 --- a/packages/dashboard/src/research-routes.ts +++ b/packages/dashboard/src/research-routes.ts @@ -1,4 +1,5 @@ import { Router } from "express"; +import { archivedColumnsForTask } from "./task-lifecycle-lanes.js"; import type { NextFunction, Request, Response } from "express"; import { AsyncLocalStorage } from "node:async_hooks"; import type { TaskStore, ResearchRun, TaskCreateInput } from "@fusion/core"; @@ -419,7 +420,12 @@ export function createResearchRouter(store: TaskStore, options?: ServerOptions): const task = await scopedStore.getTask(req.params.taskId); if (!task) throw notFound(`Task not found: ${req.params.taskId}`); - if (task.column === "archived") throw new ApiError(409, "Cannot enrich archived task"); + /* + FNXC:WorkflowResolvedColumns 2026-07-30-06:50 (batch-core): + Archived tasks are read-only for research enrichment. Keyed on the literal, a renamed board let + an ARCHIVED card be enriched — writes landing on a row the archive treats as immutable. + */ + if ((await archivedColumnsForTask(scopedStore, task.id)).has(task.column)) throw new ApiError(409, "Cannot enrich archived task"); let documentKey: string; try { diff --git a/packages/dashboard/src/routes/register-agent-core-routes.ts b/packages/dashboard/src/routes/register-agent-core-routes.ts index 7ca5dfc82b..3d8520d063 100644 --- a/packages/dashboard/src/routes/register-agent-core-routes.ts +++ b/packages/dashboard/src/routes/register-agent-core-routes.ts @@ -1,4 +1,6 @@ import { mkdir, readdir, readFile, rm, writeFile } from "node:fs/promises"; +import { preWipColumnsForTask } from "../task-lifecycle-lanes.js"; +import type { WorkflowIr } from "@fusion/core"; import path from "node:path"; import type { Request, Response } from "express"; import type { Agent, AgentCapability, AgentUpdateInput, TaskStore, AgentPermissionPolicyRules, AgentPermissionPolicyDisposition, AgentPermissionPolicyToolRules } from "@fusion/core"; @@ -395,7 +397,19 @@ export function registerAgentCoreRoutes(ctx: ApiRoutesContext, deps: AgentCoreRo const total = completedRuns + failedRuns; const successRate = total > 0 ? completedRuns / total : 0; const tasks = await scopedStore.listTasks({ slim: true, includeArchived: false }); - const todoTaskCount = tasks.filter((task) => task.column === "todo").length; + /* + FNXC:WorkflowResolvedColumns 2026-07-30-06:50 (batch-core): + "How many cards are queued" for the agent stats panel — the pre-WIP roles (intake and hold), + resolved per task. Keyed on the literal, a renamed board reported 0 queued forever, so the panel + showed an idle backlog while cards were waiting. + + One shared IR cache across the board scan: one workflow read per distinct workflow, not per task. + */ + const queuedIrCache = new Map(); + let todoTaskCount = 0; + for (const task of tasks) { + if ((await preWipColumnsForTask(scopedStore, task.id, queuedIrCache)).has(task.column)) todoTaskCount += 1; + } res.json({ activeCount, assignedTaskCount, diff --git a/packages/dashboard/src/routes/register-chat-routes.ts b/packages/dashboard/src/routes/register-chat-routes.ts index d170fea77e..0b3c0cb0eb 100644 --- a/packages/dashboard/src/routes/register-chat-routes.ts +++ b/packages/dashboard/src/routes/register-chat-routes.ts @@ -1,4 +1,5 @@ import { randomUUID } from "node:crypto"; +import { archivedColumnsForTask } from "../task-lifecycle-lanes.js"; import { createReadStream } from "node:fs"; import { mkdir, rm, writeFile } from "node:fs/promises"; import { basename, join, resolve } from "node:path"; @@ -221,7 +222,12 @@ export function registerChatRoutes(ctx: ApiRoutesContext, deps: ChatRouteDeps): return; } - if (task.column === "archived") { + /* + FNXC:WorkflowResolvedColumns 2026-07-30-06:50 (batch-core): + Planner chat is refused for archived tasks. Keyed on the literal, a renamed board started + planner sessions against archived cards, whose rows the archive treats as immutable. + */ + if ((await archivedColumnsForTask(scopedStore, task.id)).has(task.column)) { throw badRequest(`Task ${task.id} is archived; planner chat cannot be started for archived tasks`); } diff --git a/packages/dashboard/src/routes/register-session-diff-routes.ts b/packages/dashboard/src/routes/register-session-diff-routes.ts index 11a4e53d30..6e71c9f2c5 100644 --- a/packages/dashboard/src/routes/register-session-diff-routes.ts +++ b/packages/dashboard/src/routes/register-session-diff-routes.ts @@ -1,10 +1,11 @@ -import { createLogger, resolveWorkflowIrForTask, columnsWithFlag } from "@fusion/core"; +import { createLogger } from "@fusion/core"; +import { landedColumnsForTask } from "../task-lifecycle-lanes.js"; const severityAuditLog = createLogger("dashboard-register-session-diff-routes"); import { access } from "node:fs/promises"; import { join } from "node:path"; import type { Request, Router } from "express"; -import type { RunAuditEvent, RunAuditEventFilter, TaskStore } from "@fusion/core"; +import type { RunAuditEvent, RunAuditEventFilter } from "@fusion/core"; import { isWorkspaceTask } from "@fusion/core"; import { ApiError, notFound, rethrowAsApiError } from "../api-error.js"; // FNXC:TaskLookup404 2026-07-26-11:40: shared task-miss -> 404 mapping seam. @@ -27,29 +28,6 @@ export interface SessionDiffRouteDeps { * (e.g. task.branch was never set) so we don't break tests/legacy tasks. */ -/* -FNXC:WorkflowResolvedColumns 2026-07-31-00:20 (batch-core): -"Has this task LANDED?" for the two diff routes below — the question that decides whether a diff is -taken against the merge commit (finished work, on the integration branch) or against the live branch. - -Keyed on `column === "done"`, a task finished in a renamed complete lane took the LIVE-branch path, so -its diff was computed against a branch that had already been merged and usually deleted — an empty or -misleading diff for exactly the tasks an operator reviews after the fact. - -MEMBERSHIP over `complete` and `archived`: both mean landed for this purpose, and a board may declare -more than one of either. The legacy pair is the fallback when the IR cannot be read AND when it -resolves empty — a v1-upgraded workflow carries `traits: []` on every synthesized column -(workflow-ir.ts:158-159), so empty means UNEXPRESSED here, not "this board has no complete lane". -*/ -export async function landedColumnsForTask(store: TaskStore, taskId: string): Promise> { - try { - const ir = await resolveWorkflowIrForTask(store, taskId); - const landed = [...columnsWithFlag(ir, "complete"), ...columnsWithFlag(ir, "archived")]; - return new Set(landed.length > 0 ? landed : ["done", "archived"]); - } catch { - return new Set(["done", "archived"]); - } -} async function worktreeStillBelongsToTask( worktree: string, diff --git a/packages/dashboard/src/server.ts b/packages/dashboard/src/server.ts index ea3a167fd3..8cd229fb6f 100644 --- a/packages/dashboard/src/server.ts +++ b/packages/dashboard/src/server.ts @@ -2,6 +2,7 @@ import { createLogger } from "@fusion/core"; const severityAuditLog = createLogger("dashboard-server"); import express, { type Router } from "express"; +import { archivedColumnsForTask } from "./task-lifecycle-lanes.js"; import { randomUUID } from "node:crypto"; import { join, dirname } from "node:path"; import { existsSync, readFileSync, statSync } from "node:fs"; @@ -1111,12 +1112,28 @@ export function createServer(store: TaskStore, options?: ServerOptions): ReturnT const chatLayer = requireAsyncLayer(store, "Dashboard ChatStore"); const chatStore = options?.chatStore ?? new ChatStore(chatLayer); store.on("task:moved", (data: { task: Task; from: string; to: string }) => { - if (data.to !== "archived") return; + /* + FNXC:WorkflowResolvedColumns 2026-07-30-04:05 (batch-core): + Planner-chat retention is cut off by ARCHIVAL, resolved from the task's own workflow. Keyed on the + literal, a board that renamed its archived lane never reached the delete, so task-planner chat + sessions were retained forever — the retention cutoff this listener exists to enforce simply never + fired, and nothing surfaced that. + + The handler stays synchronous and the resolution is awaited inside the existing fire-and-forget + chain rather than by making the listener `async`. `task:moved` has synchronous subscribers whose + ordering relative to the emitter is load-bearing elsewhere in this codebase, and this listener + only deletes chat rows — there is no reason to make it the one that introduces a microtask + boundary into that emit. + */ + void (async () => { + const archivedLanes = await archivedColumnsForTask(store, data.task.id).catch(() => undefined); + if (!(archivedLanes ?? new Set(["archived"])).has(data.to)) return; /* FNXC:TaskDetailPlannerChatRetention 2026-06-30-18:45: Task-detail planner chats are retained after done when a user interacted, but task archival is the retention cutoff. Delete exact task-planner sessions on archive so normal chats and other tasks' planner chats remain intact while chat:session:deleted events clear dashboard caches. */ - void chatStore.deleteSessionsForAgentId(`${TASK_PLANNER_CHAT_AGENT_ID_PREFIX}${data.task.id}`); + await chatStore.deleteSessionsForAgentId(`${TASK_PLANNER_CHAT_AGENT_ID_PREFIX}${data.task.id}`); + })(); }); options?.engine?.attachChatStore?.(chatStore); if (typeof options?.engineManager?.getAllEngines === "function") { @@ -2915,6 +2932,30 @@ tasks visible on the live board; archived tasks leave it. This predicate is the eligibility rule used by both the create and update listeners (and mirrored by the startup prime's `includeArchived:false`). Exported for unit coverage of the invariant. */ +/* +FNXC:WorkflowResolvedColumns 2026-07-30-04:20 DELIBERATE-LITERAL: sync predicate behind a sync listener. + +NOT OVERLOOKED. On a renamed board this is genuinely wrong — an archived card stays badge-eligible, so +its snapshot is never evicted and the cache grows for the daemon's lifetime. That is the exact memory +leak this predicate was added to fix (FNXC:BadgeSnapshotEviction above), reappearing under a different +column name. It is real backlog, deliberately left counted rather than marked away. + +WHAT BLOCKS IT, measured rather than assumed. Resolving the archived role is async, and both callers +are SYNCHRONOUS `task:updated` / `task:created` listeners whose next statement is documented as +"Update local cache immediately" — the snapshot is written, compared, and published in the same tick. +Awaiting here introduces a microtask boundary into that path, so a second event for the same task can +interleave between the eligibility check and the cache write and publish a stale snapshot. + +WHY NOT AN OPTIONAL `archivedColumns` PARAMETER. Because nothing could fill it: the callers are the +sync listeners. An optional parameter that only tests supply is the inert-injection shape — the +predicate would read as converted, its test would pass by injecting the value, and production would +keep the literal. #2780 caught exactly that twice in this program. + +WHAT WOULD ACTUALLY UNBLOCK IT: give the badge-snapshot scope a resolved-archived-lane cache populated +when a project's workflow is loaded, so the predicate stays sync and reads a map instead of a literal. +That is a lifecycle change to the snapshot scope, not a rename, so it is stated here rather than +quietly skipped. +*/ export function isBadgeEligibleTask(task: Pick): boolean { return task.column !== "archived"; } diff --git a/packages/dashboard/src/task-lifecycle-lanes.ts b/packages/dashboard/src/task-lifecycle-lanes.ts new file mode 100644 index 0000000000..2f9d5d8334 --- /dev/null +++ b/packages/dashboard/src/task-lifecycle-lanes.ts @@ -0,0 +1,150 @@ +import { columnsWithFlag, resolveWorkflowIrForTask, type WorkflowIr } from "@fusion/core"; + +/* +FNXC:WorkflowResolvedColumns 2026-07-30-08:45 (#2783 review — coderabbit): +The store parameter is the shape `resolveWorkflowIrForTask` ACTUALLY needs, not `Pick`. + +The first version took `getTask` — which none of these helpers call — and cast it through `unknown` to +reach the resolver. That cast was a type lie in the load-bearing direction: it let a caller pass a +partial store with no workflow readers, where every call would throw into the catch and silently +return the legacy answer forever. Typed properly, a store that cannot resolve workflows is a compile +error at the call site instead of a silent permanent fallback at runtime. +*/ +type LaneResolverStore = Parameters[0]; + +/* +FNXC:WorkflowResolvedColumns 2026-07-30-03:10 (batch-core): + +ONE ANSWER TO "HAS THIS TASK LANDED?", SHARED BY EVERY DASHBOARD SURFACE THAT ASKS IT. + +Several places asked it independently and all compared against the literal `done`, so on a renamed +board they silently stopped firing — source issues were never commented on or closed, and finished +tasks diffed against a branch that had already been merged. + +WHICH HELPER EACH CALLER TAKES IS NOT UNIFORM, and the split is deliberate: + - `landedColumnsForTask` (complete u archived) — the session-diff boundary, where an archived task + has equally landed and its diff must come from the merge commit. + - `completeColumnsForTask` (complete only) — the GitHub and GitLab source-issue commenters, the + GitLab backfill reconciler, and the knowledge refresh. Their originals fired on `done` alone; + widening them to archival would post comments and close issues the literal never touched. #2783's + review caught exactly that regression in my first pass. + +Five copies of one question is how the halves drift apart (FN-6115 -> FN-6118 -> FN-6123 is the +motivating incident: the same affordance fixed three times because it lived in two components). So +this is the single home, and the callers do nothing but ask it. + +MEMBERSHIP over `complete` and `archived`. Both mean landed for these purposes — an archived task is +not un-finished — and a board may declare more than one column carrying either role, so +`columnsWithFlag(...)[0]` would silently ignore the second. + +EMPTY MEANS UNEXPRESSED, NOT ABSENT. `synthesizeDefaultColumns` (workflow-ir.ts:158-159) upgrades a v1 +graph by emitting every default column with `traits: []`, so a v1-upgraded workflow resolves to an +EMPTY set while its `done` column plainly exists and holds finished cards. Reading empty as "this +board has no complete lane" would stop these surfaces firing on every pre-v2 project — a worse +regression than the one being fixed, and invisible to any v2 fixture. Empty therefore takes the same +legacy fallback as a workflow that cannot be read at all. +*/ +const LEGACY_LANDED_COLUMNS: readonly string[] = ["done", "archived"]; + +export async function landedColumnsForTask( + store: LaneResolverStore, + taskId: string, + irCache?: Map, +): Promise> { + try { + const ir = await resolveWorkflowIrForTask(store, taskId, irCache); + const landed = [...columnsWithFlag(ir, "complete"), ...columnsWithFlag(ir, "archived")]; + return new Set(landed.length > 0 ? landed : LEGACY_LANDED_COLUMNS); + } catch { + return new Set(LEGACY_LANDED_COLUMNS); + } +} + + +/* +FNXC:WorkflowResolvedColumns 2026-07-30-03:25 (batch-core): +COMPLETE ONLY — deliberately narrower than `landedColumnsForTask`, for callers whose contract +excludes archived work rather than merely never encountering it. + +The GitLab backfill reconciler is the case: its own FNXC note records that archived tasks live in +`archiveDb` and that the active-task backfill intentionally excludes them, so it must not widen to +the archived role just because the shared helper offers it. Today it lists with +`includeArchived: false` and would see no archived rows either way — but that is an incidental +property of the query, not the contract, and folding the two together would quietly couple them. +*/ +export async function completeColumnsForTask( + store: LaneResolverStore, + taskId: string, + irCache?: Map, +): Promise> { + try { + const ir = await resolveWorkflowIrForTask(store, taskId, irCache); + const complete = columnsWithFlag(ir, "complete"); + return new Set(complete.length > 0 ? complete : ["done"]); + } catch { + return new Set(["done"]); + } +} + +/* +FNXC:WorkflowResolvedColumns 2026-07-30-04:05 (batch-core): +ARCHIVED ONLY. Separate from `completeColumnsForTask` because archival is a distinct lifecycle event +with its own consumers: retention cutoffs and live-board eligibility ask "is this card OFF the board", +which a complete-but-not-archived card is not. + +The two roles resolve independently and have failed independently before, so they get independent +helpers rather than one flag argument — a caller that wants both asks `landedColumnsForTask`. +*/ +export async function archivedColumnsForTask( + store: LaneResolverStore, + taskId: string, + irCache?: Map, +): Promise> { + try { + const ir = await resolveWorkflowIrForTask(store, taskId, irCache); + const archived = columnsWithFlag(ir, "archived"); + return new Set(archived.length > 0 ? archived : ["archived"]); + } catch { + return new Set(["archived"]); + } +} + +/* +FNXC:WorkflowResolvedColumns 2026-07-30-05:05 (batch-core): +WIP lanes — "is this card actively being worked?". Uses `countsTowardWip`, which is the trait the +concurrency limit is keyed on, so this answers the same question the scheduler does rather than a +parallel one. +*/ +export async function wipColumnsForTask( + store: LaneResolverStore, + taskId: string, + irCache?: Map, +): Promise> { + try { + const ir = await resolveWorkflowIrForTask(store, taskId, irCache); + const wip = columnsWithFlag(ir, "countsTowardWip"); + return new Set(wip.length > 0 ? wip : ["in-progress"]); + } catch { + return new Set(["in-progress"]); + } +} + +/* +FNXC:WorkflowResolvedColumns 2026-07-30-06:50 (batch-core): +PRE-WIP lanes — intake and hold together, the columns a card sits in before work starts. Kept as one +helper because every caller so far asks "is this queued", not "is it specifically intake": splitting +them would push that distinction onto callers that do not have it. +*/ +export async function preWipColumnsForTask( + store: LaneResolverStore, + taskId: string, + irCache?: Map, +): Promise> { + try { + const ir = await resolveWorkflowIrForTask(store, taskId, irCache); + const preWip = [...columnsWithFlag(ir, "intake"), ...columnsWithFlag(ir, "hold")]; + return new Set(preWip.length > 0 ? preWip : ["todo"]); + } catch { + return new Set(["todo"]); + } +} diff --git a/packages/dashboard/src/task-planner-chat-context.ts b/packages/dashboard/src/task-planner-chat-context.ts index 56cb2ff6a4..ea5d755df5 100644 --- a/packages/dashboard/src/task-planner-chat-context.ts +++ b/packages/dashboard/src/task-planner-chat-context.ts @@ -116,6 +116,14 @@ function formatProgress(task: UnknownRecord): string | undefined { return `step ${currentStep + 1} of ${steps.length}`; } + /* + FNXC:WorkflowResolvedColumns 2026-07-30-07:00 DELIBERATE-LITERAL: a STEP status, not a board column. + + `"done"` here is a `StepStatus` read off a workflow STEP (`readString(step, "status")`), counting + completed steps for the "N/M steps done" label. Steps and columns share the spelling and nothing + else — there is no task column in scope. The census matches on the string, so it reads as an + unconverted lifecycle guard; resolving it against a workflow would be a category error. + */ const done = steps.filter((step) => isRecord(step) && readString(step, "status") === "done").length; if (steps.length > 0) { return `${done}/${steps.length} steps done`; diff --git a/packages/dashboard/src/test/mockCoreEngine.ts b/packages/dashboard/src/test/mockCoreEngine.ts index f2f4032f35..4c8502d794 100644 --- a/packages/dashboard/src/test/mockCoreEngine.ts +++ b/packages/dashboard/src/test/mockCoreEngine.ts @@ -70,8 +70,19 @@ export function createEngineMock(overrides: AnyModule = {}): AnyModule { FNXC:MissingWorktreeRetry 2026-07-10-18:45: Dashboard route tests mock @fusion/engine wholesale; the retry route must still exercise the upstream #1992 classifier so merge-active unusable-worktree failures are admitted while unrelated merging rows remain rejected. */ - isInReviewMissingWorktreeSessionStartFailure: vi.fn((task: { column?: string; error?: unknown }) => ( - task.column === "in-review" + /* + FNXC:WorkflowResolvedColumns 2026-07-30-07:00 DELIBERATE-LITERAL: a test double mirroring production's own fallback. + + Production's `isInReviewMissingWorktreeSessionStartFailure` is `(isReviewColumn ?? task.column === + "in-review") && ...` — the literal IS its documented degraded path for a caller that has not + resolved the lane. A double must reproduce that, not improve on it. + + FIDELITY FIX while marking it: this ignored the second parameter entirely, so a route test that + passed a resolved `isReviewColumn` got the literal answer anyway and would have reported a pass + for a renamed board the real classifier handles. Now threaded exactly as production does. + */ + isInReviewMissingWorktreeSessionStartFailure: vi.fn((task: { column?: string; error?: unknown }, isReviewColumn?: boolean) => ( + (isReviewColumn ?? task.column === "in-review") && typeof task.error === "string" && (task.error.includes("Refusing to start coding agent in missing worktree:") || task.error.includes("Refusing to start coding agent in incomplete worktree:")