diff --git a/packages/cli/src/__tests__/cli-active-count-lanes.test.ts b/packages/cli/src/__tests__/cli-active-count-lanes.test.ts new file mode 100644 index 0000000000..dcad77d8d1 --- /dev/null +++ b/packages/cli/src/__tests__/cli-active-count-lanes.test.ts @@ -0,0 +1,183 @@ +/* +FNXC:WorkflowLifecycleColumns 2026-08-02-09:20 (fleet: the CLI surface on a renamed board): + +THE INVARIANT: `active=N` counts the board's own wip and review lanes. + +The same four-line aggregation appears FOUR times in `dashboard.ts` — the TUI stats refresh, the serve +summary, the status line, and the agent-stats pass — each comparing the default lineage's two ids. On a +renamed board every one reported `active=0` while the board was plainly busy. + +WHY THIS IS WORSE THAN AN INTERNAL INERT GUARD: this number is the operator's first read of a project. A +recovery path that silently stops firing is invisible until something breaks; a stats line that says zero is +read, believed, and acted on — "nothing is running, so I can restart the engine". + +The four copies are now one helper, which is the other half of the fix: four independent copies of a +lifecycle decision is how they drift, and these four were identical by accident rather than by construction. +*/ +import { describe, expect, it, vi } from "vitest"; +import type { TaskStore, WorkflowIr } from "@fusion/core"; + +import { countActiveTasks } from "../commands/dashboard.js"; + +const RENAMED_IR = { + version: "v2", id: "wf-renamed", name: "renamed", + nodes: [{ id: "start", kind: "start", column: "backlog" }], + edges: [], + columns: [ + { id: "backlog", name: "Backlog", traits: [{ trait: "intake" }] }, + { id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "signoff", name: "Sign-off", traits: [{ trait: "merge" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], +} as unknown as WorkflowIr; + +function storeFor(ir: WorkflowIr | undefined) { + const selection = { workflowId: "wf-renamed", stepIds: [] as string[] }; + const getWorkflowDefinition = vi.fn(async () => (ir ? { ir } : undefined)); + const getTaskWorkflowSelectionAsync = vi.fn(async () => (ir ? selection : undefined)); + return { + store: { + getTaskWorkflowSelection: () => (ir ? selection : undefined), + getTaskWorkflowSelectionAsync, + getWorkflowDefinition, + } as unknown as TaskStore, + getWorkflowDefinition, + getTaskWorkflowSelectionAsync, + }; +} + +describe("the CLI's active-task count resolves the board's lanes", () => { + it("counts a renamed board's wip and review cards", async () => { + // Pre-fix: neither `building` nor `signoff` matched, so this returned 0 for a busy board. + const { store } = storeFor(RENAMED_IR); + + const active = await countActiveTasks(store, [ + { id: "FN-1", column: "building" }, + { id: "FN-2", column: "signoff" }, + { id: "FN-3", column: "backlog" }, + { id: "FN-4", column: "shipped" }, + ]); + + expect(active).toBe(2); + }); + + it("counts nothing when no card is in either lane", async () => { + // The paired negative: the count must not degrade into "every card is active". + const { store } = storeFor(RENAMED_IR); + + expect(await countActiveTasks(store, [ + { id: "FN-5", column: "backlog" }, + { id: "FN-6", column: "shipped" }, + ])).toBe(0); + }); + + it("PINS the per-task selection read, so the cost is visible rather than hidden", async () => { + /* + FNXC:WorkflowLifecycleColumns 2026-08-02-13:40 (PR #2728 review — greptile P2, and it is a fair catch): + The shared IR cache avoids repeated workflow-DEFINITION reads, not the per-task SELECTION read — a board + can mix workflows, so "which workflow governs this card" has to be asked per card. My first version + asserted only the definition count, which made the aggregation look cheaper than it is. + + THE TRADE-OFF, stated rather than hidden: the previous implementation did ZERO reads and was wrong on + every renamed board (`active=0` on a busy board). N selection reads per stats refresh is the price of a + correct answer with today's resolver. The durable fix is a bulk selection read or a list projection that + carries column flags — the dashboard already avoids this entirely by reading board flags it has in hand + (`enrichRunningAgentTaskShapeFromFlags`), which is the shape a `listTasks` projection should copy. + + Pinned as an EXACT count so a future bulk read shows up here as a deliberate change rather than drifting. + */ + const { store, getTaskWorkflowSelectionAsync } = storeFor(RENAMED_IR); + const tasks = Array.from({ length: 12 }, (_, i) => ({ id: `FN-${i}`, column: "building" })); + + await countActiveTasks(store, tasks); + + expect(getTaskWorkflowSelectionAsync).toHaveBeenCalledTimes(12); + }); + + it("resolves one IR per WORKFLOW, not per task", async () => { + /* + The cost of converting a per-list aggregation is the reason to assert this: a 500-card board must not + become 500 workflow reads. The shared cache is what makes that true, and only a call count can see it — + the returned number is identical either way. + */ + const { store, getWorkflowDefinition } = storeFor(RENAMED_IR); + const tasks = Array.from({ length: 25 }, (_, i) => ({ id: `FN-${i}`, column: "building" })); + + expect(await countActiveTasks(store, tasks)).toBe(25); + expect(getWorkflowDefinition).toHaveBeenCalledTimes(1); + }); + + it("behaves identically on the DEFAULT board", async () => { + // No workflow selection: falls back to the legacy pair. Passes either way by design. + const { store } = storeFor(undefined); + + expect(await countActiveTasks(store, [ + { id: "FN-7", column: "in-progress" }, + { id: "FN-8", column: "in-review" }, + { id: "FN-9", column: "todo" }, + ])).toBe(2); + }); +}); + +/* +FNXC:WorkflowLifecycleColumns 2026-08-02-13:00 (PR #2728 review — greptile P1 x2): + +THE INVARIANT: the shared missing-worktree classifier answers with the CALLER'S review lane. + +`isInReviewMissingWorktreeSessionStartFailure` is used by three surfaces — the dashboard retry route, the CLI +retry command, and `fn_task_retry` (the MCP tool agents call). All three resolve the review lane before +calling it, and the classifier compared the literal anyway. So every surface recognised a renamed review lane +and then delegated to a predicate that did not: the card was refused for the ONE reason the delegate exists to +allow, and no message anywhere mentions columns. + +Testing the classifier directly rather than through three route harnesses: it is a pure function and the +defect lives in it. The three call sites passing their sets is asserted structurally below, because a call +site that accepts the parameter and does not pass it is the failure mode a unit test cannot see. +*/ +describe("the shared missing-worktree classifier takes the caller's review lane", () => { + const FAILURE = "Refusing to start coding agent in missing worktree: /gone"; + + it("recognises a renamed review lane when the caller supplies it", async () => { + const { isInReviewMissingWorktreeSessionStartFailure } = await import("@fusion/engine"); + const task = { id: "FN-1", column: "signoff", error: FAILURE } as never; + + // Pre-fix: `signoff` !== "in-review", so the retry bypass this classifier exists for never applied. + expect(isInReviewMissingWorktreeSessionStartFailure(task, new Set(["signoff"]))).toBe(true); + }); + + it("still refuses a column outside the supplied set", async () => { + const { isInReviewMissingWorktreeSessionStartFailure } = await import("@fusion/engine"); + const task = { id: "FN-2", column: "building", error: FAILURE } as never; + + expect(isInReviewMissingWorktreeSessionStartFailure(task, new Set(["signoff"]))).toBe(false); + }); + + it("keeps the legacy literal when no set is supplied", async () => { + // Backwards compatibility is the reason the parameter is optional: existing callers must not change. + const { isInReviewMissingWorktreeSessionStartFailure } = await import("@fusion/engine"); + + expect(isInReviewMissingWorktreeSessionStartFailure({ id: "FN-3", column: "in-review", error: FAILURE } as never)).toBe(true); + expect(isInReviewMissingWorktreeSessionStartFailure({ id: "FN-4", column: "signoff", error: FAILURE } as never)).toBe(false); + }); + + it("is called WITH a resolved set at all three surfaces", async () => { + /* + A call site that accepts the parameter and forgets to pass it is exactly the half-conversion this thread + was about, and no unit test on the classifier can see it. Structural, and it names the file so a fourth + surface has to be added here deliberately. + */ + const { readFile } = await import("node:fs/promises"); + const surfaces = [ + new URL("../extension.ts", import.meta.url), + new URL("../commands/task.ts", import.meta.url), + new URL("../../../dashboard/src/routes/register-task-workflow-routes.ts", import.meta.url), + ]; + + for (const surface of surfaces) { + const code = (await readFile(surface, "utf8")).replace(/\/\*[\s\S]*?\*\//g, ""); + const call = code.match(/isInReviewMissingWorktreeSessionStartFailure\(([^)]*)\)/); + expect(call, `${surface.pathname} does not call the classifier`).toBeTruthy(); + expect(call?.[1], `${surface.pathname} calls it without a resolved review set`).toContain(","); + } + }); +}); diff --git a/packages/cli/src/commands/dashboard.ts b/packages/cli/src/commands/dashboard.ts index 36f58f3c9f..7aaf897cd6 100644 --- a/packages/cli/src/commands/dashboard.ts +++ b/packages/cli/src/commands/dashboard.ts @@ -34,7 +34,37 @@ import { FUSION_NON_RETRYABLE_EXIT_CODE, isPostgresUniqueError, ProjectPartitionRekeyError, + resolveTaskLifecycleColumns, + type WorkflowIr, } from "@fusion/core"; + +/* +FNXC:WorkflowLifecycleColumns 2026-08-02-08:50 (fleet: CLI dashboard/serve stats): +"ACTIVE" IS THE BOARD'S WIP AND REVIEW LANES, counted once for a whole task list. + +The same aggregation appears FOUR times in this file (the TUI stats refresh, the serve summary, the status +line and the agent-stats pass), each comparing the default lineage's two ids. On a renamed board every one of +them reported `active=0` while the board was plainly busy — and this number is the operator's first read of a +new project, so it is the surface most likely to be believed. + +ONE resolution per WORKFLOW, not per task: the shared IR cache means a 500-card board costs one read per +distinct workflow. Extracted rather than inlined four times, because four copies of a lifecycle decision is +how copies drift — these four were identical by accident, not by construction. +*/ +export async function countActiveTasks( + store: unknown, + tasks: Array<{ id: string; column: string }>, +): Promise { + const irCache = new Map(); + let active = 0; + for (const task of tasks) { + const lifecycle = await resolveTaskLifecycleColumns(store as never, task.id, irCache); + if (task.column === (lifecycle?.wip ?? "in-progress") || task.column === (lifecycle?.review ?? "in-review")) { + active += 1; + } + } + return active; +} import { createServer, refreshAllCustomProviderModels, @@ -805,9 +835,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: for (const task of tasks) { counts.set(task.column, (counts.get(task.column) ?? 0) + 1); } - const active = tasks.filter((task) => - task.column === "in-progress" || task.column === "in-review" - ).length; + const active = await countActiveTasks(store, tasks); const agents = await agentStore.listAgents(); const agentStats = { idle: 0, active: 0, running: 0, error: 0 }; for (const agent of agents) { @@ -1136,9 +1164,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: for (const task of tasks) { counts.set(task.column, (counts.get(task.column) ?? 0) + 1); } - const active = tasks.filter((task) => - task.column === "in-progress" || task.column === "in-review" - ).length; + const active = await countActiveTasks(taskStore, tasks); const agents = await agentStore.listAgents(); const agentStats = { idle: 0, active: 0, running: 0, error: 0 }; for (const agent of agents) { @@ -1326,9 +1352,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: for (const task of tasks) { counts.set(task.column, (counts.get(task.column) ?? 0) + 1); } - const active = tasks.filter((task) => - task.column === "in-progress" || task.column === "in-review" - ).length; + const active = await countActiveTasks(store, tasks); taskSummary = `tasks=${tasks.length} active=${active} columns=${Array.from(counts.entries()) .map(([column, count]) => `${column}:${count}`) .join(",")}`; @@ -2917,9 +2941,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?: for (const task of tasks) { counts.set(task.column, (counts.get(task.column) ?? 0) + 1); } - const active = tasks.filter((task) => - task.column === "in-progress" || task.column === "in-review" - ).length; + const active = await countActiveTasks(store, tasks); const agents = await agentStore.listAgents(); const agentStats = { idle: 0, active: 0, running: 0, error: 0 }; for (const agent of agents) { diff --git a/packages/cli/src/commands/task.ts b/packages/cli/src/commands/task.ts index 1df973c98c..e01dbb130b 100644 --- a/packages/cli/src/commands/task.ts +++ b/packages/cli/src/commands/task.ts @@ -1,4 +1,4 @@ -import { TaskStore, COLUMNS, COLUMN_LABELS, CentralCore, buildAutoPauseClearPatch, buildManualRetryResetPatch, extractIntentSignature, findNearDuplicates, getTaskDuplicateLineage, isWorkspaceTask, reconcileDeterministicDuplicate, resolveTaskGithubTracking, runDeterministicDuplicateGuard, type Settings, type Column, type ColumnId, type StepStatus, type AgentLogType, type AgentLogEntry, type IntentSignature, type NearDuplicateCandidate, type NearDuplicateMatch, type TaskDependencyMutation } from "@fusion/core"; +import { TaskStore, COLUMNS, COLUMN_LABELS, columnsWithFlag, resolveTaskLifecycleColumns, resolveWorkflowIrForTask, CentralCore, buildAutoPauseClearPatch, buildManualRetryResetPatch, extractIntentSignature, findNearDuplicates, getTaskDuplicateLineage, isWorkspaceTask, reconcileDeterministicDuplicate, resolveTaskGithubTracking, runDeterministicDuplicateGuard, type Settings, type Column, type ColumnId, type StepStatus, type AgentLogType, type AgentLogEntry, type IntentSignature, type NearDuplicateCandidate, type NearDuplicateMatch, type TaskDependencyMutation } from "@fusion/core"; import { isInReviewMissingWorktreeSessionStartFailure, runAiMerge, landWorkspaceTask, installBaselineArchiveWorktreeDisposer } from "@fusion/engine"; import { createInterface } from "node:readline/promises"; import type { PlanningQuestion, PlanningSummary } from "@fusion/core"; @@ -58,7 +58,11 @@ async function formatTaskDuplicateLineage(task: Awaited { try { const linked = await store.getTask(id); - return linked.column === "archived" ? `${id} (archived)` : id; + /* FNXC:WorkflowLifecycleColumns 2026-08-02-08:10 (fleet: CLI surface): the board's archived column. + With the literal, a renamed board's archived duplicates printed with no `(archived)` marker, so the + operator could not tell a live duplicate from a filed one in the lineage line. */ + const linkedLifecycle = await resolveTaskLifecycleColumns(store, id); + return linked.column === (linkedLifecycle?.archived ?? "archived") ? `${id} (archived)` : id; } catch { return id; } @@ -335,8 +339,22 @@ async function runCliNearDuplicateCheck(args: { } if (!args.bypass) { const cutoff = Date.now() - 7 * 24 * 60 * 60 * 1000; - const taskCandidates = (await args.store.listTasks({ slim: false, includeArchived: false })) - .filter((task) => task.column !== "done") + /* + FNXC:WorkflowLifecycleColumns 2026-08-02-08:15 (fleet: CLI surface): + Duplicate detection compares against UNFINISHED work, and "finished" is the board's complete column. + With the literal, a renamed board kept every completed card in the candidate set: the guard then + reported a new task as a duplicate of work that had already landed, which is the opposite of useful. + Resolved per candidate through one shared cache, because candidates can span workflows. + */ + const candidateIrCache = new Map(); + const allCandidates = await args.store.listTasks({ slim: false, includeArchived: false }); + const completeByTaskId = new Map(); + for (const task of allCandidates) { + const lifecycle = await resolveTaskLifecycleColumns(args.store, task.id, candidateIrCache as never); + completeByTaskId.set(task.id, lifecycle?.complete ?? "done"); + } + const taskCandidates = allCandidates + .filter((task) => task.column !== completeByTaskId.get(task.id)) .filter((task) => { const createdAtMs = Date.parse(task.createdAt); return Number.isFinite(createdAtMs) && createdAtMs >= cutoff; @@ -612,6 +630,10 @@ export async function runTaskList(projectName?: string) { ALL. That is the R8/U10 surface change (no surface derives its column set from the legacy enum) and a far bigger fix than this glyph. */ + /* DELIBERATE-LITERAL: `col` comes from the legacy `COLUMNS` enum this loop iterates, so the literal + matches its own receiver by construction. The real defect is named in the comment above — a card in a + workflow-renamed column is not rendered at all — and converting this glyph would hide that behind a + trait lookup while the loop still cannot see the card. Retires with the loop. */ const dot = col === "done" || col === "archived" ? "○" : "●"; console.log(` ${dot} ${label} (${colTasks.length})`); @@ -918,7 +940,11 @@ export async function runTaskSetNode(id: string, nodeNameOrId: string, projectNa await withBoardWrite(projectName, { id, action: "set node override" }, async (context) => { const task = await context.store.getTask(id); - if (task.column === "in-progress") { + /* FNXC:WorkflowLifecycleColumns 2026-08-02-08:20 (fleet: CLI surface): the board's wip lane. With the + literal these guards never fired on a renamed board, so `fn task set-node` / `clear-node` rewrote the + node override of a card that was actively executing — the guard exists because that races the run. */ + const nodeGuardLifecycle = await resolveTaskLifecycleColumns(context.store, id); + if (task.column === (nodeGuardLifecycle?.wip ?? "in-progress")) { console.error(`Cannot change node override: task ${id} is in progress`); await closeBoardContextAndExit(context, 1); return; @@ -944,7 +970,11 @@ export async function runTaskClearNode(id: string, projectName?: string) { await withBoardWrite(projectName, { id, action: "clear node override" }, async (context) => { const task = await context.store.getTask(id); - if (task.column === "in-progress") { + /* FNXC:WorkflowLifecycleColumns 2026-08-02-08:20 (fleet: CLI surface): the board's wip lane. With the + literal these guards never fired on a renamed board, so `fn task set-node` / `clear-node` rewrote the + node override of a card that was actively executing — the guard exists because that races the run. */ + const nodeGuardLifecycle = await resolveTaskLifecycleColumns(context.store, id); + if (task.column === (nodeGuardLifecycle?.wip ?? "in-progress")) { console.error(`Cannot change node override: task ${id} is in progress`); await closeBoardContextAndExit(context, 1); return; @@ -1305,8 +1335,45 @@ export async function runTaskRetry(id: string, projectName?: string) { throw new Error(`Task ${id} not found`); } + /* + FNXC:WorkflowLifecycleColumns 2026-08-02-08:30 (fleet: CLI surface — the retry gate exists TWICE): + This is the CLI's copy of the dashboard route's retry classifier, and #2713 converted only the route. So + after that PR `POST /tasks/:id/retry` accepted a renamed board's stalled review card while + `fn task retry` still refused it with "not in a retryable state" — the same operator action answering + differently depending on which surface they used. + Worth stating as a rule: when a gate is duplicated across surfaces, converting one of them creates a + DISAGREEMENT that is harder to diagnose than the original inert guard. Grep for the classifier by name + before claiming a lane is converted. + */ + /* + FNXC:WorkflowLifecycleColumns 2026-07-31-06:10 (PR #2728 review — greptile, both lane findings): + THE REVIEW LANE IS A SET HERE TOO. + + `resolveTaskLifecycleColumns(...).review` is a single id from ONE flag (`mergeOrchestration`), so + this gate refused a stalled card in a `humanReview`-only lane, and refused a card in a SECOND merge + lane that the dashboard's retry route accepts. Same operator action, different answer per surface — + which is precisely the disagreement this PR was opened to remove, reappearing one level down. + + The union matches `resolveReviewColumnsForTask` in the dashboard routes and the notifier's copy. + That is now FOUR inline copies of one definition; #2730 adds `resolveReviewColumns` to core so they + can converge. Not imported here yet because #2730 is unmerged and stacking on an open PR is what + stranded #2568 four deep. + */ + const retryIr = await resolveWorkflowIrForTask(context.store, id).catch(() => undefined); + const retryReviewColumns = new Set( + retryIr === undefined + ? ["in-review"] + : (() => { + const lanes = [ + ...columnsWithFlag(retryIr, "mergeOrchestration"), + ...columnsWithFlag(retryIr, "mergeBlocker"), + ...columnsWithFlag(retryIr, "humanReview"), + ]; + return lanes.length > 0 ? lanes : ["in-review"]; + })(), + ); const isInReviewStatusNone = - task.column === "in-review" && (task.status === null || task.status === undefined); + retryReviewColumns.has(task.column) && (task.status === null || task.status === undefined); const hasIncompleteSteps = task.steps.some( (s: { status: string }) => s.status === "pending" || s.status === "in-progress", ); @@ -1317,7 +1384,7 @@ export async function runTaskRetry(id: string, projectName?: string) { const isInReviewExecutionStall = isInReviewStatusNone && isExecutionFailureInReview; const isInReviewMergeRetryStall = isInReviewStatusNone && (task.mergeRetries ?? 0) > 0; const isInReviewRetry = - task.column === "in-review" && + retryReviewColumns.has(task.column) && (task.status === "failed" || task.status === "stuck-killed" || isInReviewExecutionStall || @@ -1326,7 +1393,7 @@ export async function runTaskRetry(id: string, projectName?: string) { FNXC:MissingWorktreeRetry 2026-07-10-18:28: Upstream #1992 requires operator retry to recover an in-review task whose session start refused a missing/incomplete/unregistered worktree even when the row is stuck in an invalid merge-active status. This signature-only bypass clears stale session metadata instead of requiring a valid `merging` transition. */ - const isMissingWorktreeSessionRetry = isInReviewMissingWorktreeSessionStartFailure(task); + const isMissingWorktreeSessionRetry = isInReviewMissingWorktreeSessionStartFailure(task, retryReviewColumns.has(task.column)); // Validate task is in a retryable state if (task.status !== 'failed' && task.status !== 'stuck-killed' && !isInReviewRetry && !isMissingWorktreeSessionRetry) { diff --git a/packages/cli/src/extension.ts b/packages/cli/src/extension.ts index c6053172b3..c9d2034e3e 100644 --- a/packages/cli/src/extension.ts +++ b/packages/cli/src/extension.ts @@ -35,6 +35,9 @@ import { resolveTaskGithubTracking, formatCurrentTaskLine, type SecretScope, + resolveTaskLifecycleColumns, + resolveWorkflowIrForTask, + columnsWithFlag, } from "@fusion/core"; import { getGhErrorMessage, @@ -870,7 +873,10 @@ async function formatDuplicateLineageLine(task: Task, store: TaskStore): Promise const labels = await Promise.all(lineage.map(async (id) => { try { const linked = await store.getTask(id); - return linked.column === "archived" ? `${id} (archived)` : id; + /* FNXC:WorkflowLifecycleColumns 2026-08-02-12:45 (fleet): the board's archived column — the same marker + as the CLI command's copy of this helper, converted there in this PR. */ + const linkedLifecycle = await resolveTaskLifecycleColumns(store, id); + return linked.column === (linkedLifecycle?.archived ?? "archived") ? `${id} (archived)` : id; } catch { return id; } @@ -885,6 +891,11 @@ export function formatTaskLine(t: Task): string { const source = getTaskSourceLabel(t); const sourceSuffix = source ? ` [via: ${source}]` : ""; const deps = t.dependencies.length ? ` [deps: ${t.dependencies.join(", ")}]` : ""; + /* DELIBERATE-LITERAL: `formatTaskLine` is a SYNCHRONOUS formatter taking only a Task — no store, no IR, and + it is called from list rendering where a per-row async resolution would be a read per line. The literal + only decides whether to print "(paused)", so a renamed board's mislabel is cosmetic. Converting it means + threading resolved flags in from every caller, which belongs with the board-render conversion that owns + the same problem (see the glyph note in commands/task.ts). */ const isTerminalColumn = t.column === "done" || t.column === "archived"; const paused = t.paused && !isTerminalColumn ? " (paused)" : ""; return `${t.id} ${label}${sourceSuffix}${deps}${paused}`; @@ -1865,8 +1876,30 @@ export default function kbExtension(pi: ExtensionAPI) { }; } + /* + FNXC:WorkflowLifecycleColumns 2026-08-02-12:40 (PR #2728 review — the retry gate exists THREE times): + This is `fn_task_retry`, the MCP tool AGENTS call — the third copy of the same classifier, after the + dashboard route (#2713) and the CLI command (this PR). Converting two of three is worse than converting + none: the operator retries from the board and it works, the agent retries the same card and is told it + is not retryable, and nothing in either message mentions columns. + + Same SET semantics as the other two surfaces (mergeBlocker / humanReview can sit on different columns, + #2713), and the same note applies: three copies of one predicate is the argument for a set-returning + resolver in core, which is a follow-up rather than a rider on this PR. + */ + const retryIr = await resolveWorkflowIrForTask(store, params.id).catch(() => undefined); + const retryReviewColumns = new Set( + retryIr + ? [ + ...columnsWithFlag(retryIr, "mergeBlocker"), + ...columnsWithFlag(retryIr, "humanReview"), + ...columnsWithFlag(retryIr, "mergeOrchestration").slice(0, 1), + ] + : [], + ); + if (retryReviewColumns.size === 0) retryReviewColumns.add("in-review"); const isInReviewStatusNone = - task.column === "in-review" && (task.status === null || task.status === undefined); + retryReviewColumns.has(task.column) && (task.status === null || task.status === undefined); const hasIncompleteSteps = task.steps.some( (s: { status: string }) => s.status === "pending" || s.status === "in-progress", ); @@ -1877,7 +1910,7 @@ export default function kbExtension(pi: ExtensionAPI) { const isInReviewExecutionStall = isInReviewStatusNone && isExecutionFailureInReview; const isInReviewMergeRetryStall = isInReviewStatusNone && (task.mergeRetries ?? 0) > 0; const isInReviewRetry = - task.column === "in-review" && + retryReviewColumns.has(task.column) && (task.status === "failed" || task.status === "stuck-killed" || isInReviewExecutionStall || @@ -1886,7 +1919,7 @@ export default function kbExtension(pi: ExtensionAPI) { FNXC:MissingWorktreeRetry 2026-07-10-18:30: Upstream #1992 requires fn_task_retry to recover an in-review unusable-worktree session-start failure even when status remains merge-active. Keep this status bypass constrained to the centrally classified missing/incomplete/unregistered worktree signature. */ - const isMissingWorktreeSessionRetry = isInReviewMissingWorktreeSessionStartFailure(task); + const isMissingWorktreeSessionRetry = isInReviewMissingWorktreeSessionStartFailure(task, retryReviewColumns.has(task.column)); // Validate task is in a retryable state if (task.status !== 'failed' && task.status !== 'stuck-killed' && !isInReviewRetry && !isMissingWorktreeSessionRetry) { diff --git a/packages/dashboard/src/routes/register-task-workflow-routes.ts b/packages/dashboard/src/routes/register-task-workflow-routes.ts index 76d88512b7..07073f5480 100644 --- a/packages/dashboard/src/routes/register-task-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-task-workflow-routes.ts @@ -2876,7 +2876,10 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork FNXC:MissingWorktreeRetry 2026-07-10-18:32: Dashboard retry must support the upstream #1992 signature where the task is stranded in a merge-active status but the durable failure is an unusable worktree session-start assertion. Only that classifier bypasses the merge-active status gate. */ - const isMissingWorktreeSessionRetry = isInReviewMissingWorktreeSessionStartFailure(task); + /* FNXC:WorkflowLifecycleColumns 2026-08-02-12:15 (PR #2728 review): the classifier now takes the set + this route already resolved, instead of falling back to its own literal — the gate above and this + delegate must agree about which columns are review. */ + const isMissingWorktreeSessionRetry = isInReviewMissingWorktreeSessionStartFailure(task, retryReviewColumns.has(task.column)); if (task.status !== "failed" && task.status !== "stuck-killed" && !retrySpecification && !strandedSpecificationRetry && !isInReviewRetry && !isMissingWorktreeSessionRetry) { throw badRequest(`Task is not in a retryable state (current status: ${task.status || 'none'})`); } diff --git a/packages/engine/src/__tests__/restart-recovery-coordinator.test.ts b/packages/engine/src/__tests__/restart-recovery-coordinator.test.ts index d297dd7c71..ce30ff630f 100644 --- a/packages/engine/src/__tests__/restart-recovery-coordinator.test.ts +++ b/packages/engine/src/__tests__/restart-recovery-coordinator.test.ts @@ -3,6 +3,7 @@ import type { TaskStore, Task } from "@fusion/core"; import { RestartRecoveryCoordinator, extractMissingWorktreePathFromSessionStartFailure, + isInReviewMissingWorktreeSessionStartFailure, isMissingWorktreeSessionStartFailure, isMergeActiveMissingWorktreeSessionStartFailure, isRecoverableMissingWorktreeReviewFailure, @@ -26,6 +27,57 @@ function createTask(overrides: Partial): Task { } as Task; } +/* +FNXC:MissingWorktreeRetry 2026-07-31-06:10 (PR #2728 review — greptile): +The classifier hardcoded `in-review`, so on a renamed board a card stranded by an unusable-worktree +session start was not recognised as retryable — while every guard AROUND it had already been +converted. A disagreement between neighbouring checks is harder to diagnose than the original inert +literal, because each one individually looks right. + +`reviewColumns` is optional and defaults to the legacy id, so the three existing call sites are +unchanged until each passes its own resolved set. +*/ +describe("isInReviewMissingWorktreeSessionStartFailure", () => { + /* + FNXC:MissingWorktreeRetry 2026-07-30-10:05 (PR #2728, aligned to #2736's signature): + The second parameter is the caller's already-RESOLVED answer, not a lane set. Both PRs widened this + function and each typechecked on its own branch; whichever merged second would have overwritten the + other's signature and broken its call site without git flagging a conflict. This file now matches + #2736 exactly, so the second merge is a no-op here. + + Recording why these cases were rewritten rather than left: they were written against the SET form, + so after the switch `["signoff"]` was simply a truthy value and two of them passed for the wrong + reason. Engine tsconfig excludes `src/__tests__`, so tsc could not see the mismatch — only reading + them could. + */ + const stranded = (column: string): Task => ({ + id: "FN-1", + column, + error: "Refusing to start coding agent in missing worktree: /repo/.worktrees/FN-1", + } as unknown as Task); + + it("recognises a stranded card when the caller resolved the lane as review", () => { + expect(isInReviewMissingWorktreeSessionStartFailure(stranded("signoff"), true)).toBe(true); + }); + + it("refuses when the caller resolved the lane as NOT review, even on the legacy id", () => { + /* The resolved answer wins over the literal — otherwise a board that renamed `in-review` to + something else, and kept `in-review` as an ordinary column, would retry cards sitting there. */ + expect(isInReviewMissingWorktreeSessionStartFailure(stranded("in-review"), false)).toBe(false); + }); + + it("keeps the legacy id when the caller supplies nothing", () => { + expect(isInReviewMissingWorktreeSessionStartFailure(stranded("in-review"))).toBe(true); + expect(isInReviewMissingWorktreeSessionStartFailure(stranded("signoff"))).toBe(false); + }); + + it("still requires the worktree failure, so resolving the lane did not widen the classifier", () => { + const healthy = { id: "FN-2", column: "signoff", error: "something else entirely" } as unknown as Task; + + expect(isInReviewMissingWorktreeSessionStartFailure(healthy, true)).toBe(false); + }); +}); + describe("RestartRecoveryCoordinator", () => { it("classifies missing-worktree session-start failures across all assertValidWorktreeSession variants", () => { expect(isMissingWorktreeSessionStartFailure("Refusing to start coding agent in missing worktree: /tmp/wt")).toBe(true); diff --git a/packages/engine/src/restart-recovery-coordinator.ts b/packages/engine/src/restart-recovery-coordinator.ts index 085f7be8f3..081cbb7ca4 100644 --- a/packages/engine/src/restart-recovery-coordinator.ts +++ b/packages/engine/src/restart-recovery-coordinator.ts @@ -93,8 +93,25 @@ export function isMergeActiveMissingWorktreeSessionStartFailure(task: Task): boo && isMissingWorktreeSessionStartFailure(task.error); } -export function isInReviewMissingWorktreeSessionStartFailure(task: Task): boolean { - return task.column === "in-review" +/** + * FNXC:WorkflowLifecycleColumns 2026-07-31-01:15 (PR #2736 review — greptile P1): + * `isReviewColumn` is an optional RESOLVED answer; omitted, it is exactly today's behaviour. + * + * This predicate selects the SPECIALIZED retry that clears `worktree`/`branch`/`sessionFile`. Its + * caller in `commands/task.ts` resolves the review lane from the task's workflow, so on a renamed + * board the two classifiers disagreed: the generic in-review retry fired while this one did not, and + * the generic branch leaves the stale session metadata in place — so the next execution hit the very + * same missing-worktree failure. A retry that reports success and changes nothing. + * + * Optional rather than required because the other caller (`extension.ts`) still asks BOTH questions + * with the literal. It is internally consistent that way, so a default preserves its meaning exactly + * while the converted caller passes the resolved answer. + */ +export function isInReviewMissingWorktreeSessionStartFailure( + task: Task, + isReviewColumn?: boolean, +): boolean { + return (isReviewColumn ?? task.column === "in-review") && isMissingWorktreeSessionStartFailure(task.error); } diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index fe7bdf513b..ad3bb97ad1 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -10,8 +10,6 @@ "packages/dashboard/src/github-tracking-comments.ts": 9, "packages/dashboard/src/github-tracking-reconciler.ts": 9, "packages/engine/src/notification/notification-service.ts": 9, - "packages/cli/src/commands/dashboard.ts": 8, - "packages/cli/src/commands/task.ts": 8, "packages/core/src/default-workflow-hooks.ts": 7, "packages/dashboard/app/components/Column.tsx": 7, "packages/core/src/live-agent-count.ts": 6, @@ -21,7 +19,6 @@ "packages/dashboard/app/components/ListView.tsx": 6, "packages/dashboard/src/reliability-metrics.ts": 6, "packages/engine/src/runtimes/in-process-runtime.ts": 6, - "packages/cli/src/extension.ts": 5, "packages/core/src/task-store/merge-queue-ops-2.ts": 5, "packages/dashboard/app/components/TaskDetailModal.tsx": 5, "packages/dashboard/app/hooks/useTaskDiffStats.ts": 5, @@ -142,6 +139,10 @@ "deliberateByFile": { "packages/dashboard/app/components/TaskCard.tsx\u0000triage": 2, "packages/dashboard/app/components/TaskDetailModal.tsx\u0000triage": 2, + "packages/cli/src/commands/task.ts\u0000archived": 1, + "packages/cli/src/commands/task.ts\u0000done": 1, + "packages/cli/src/extension.ts\u0000archived": 1, + "packages/cli/src/extension.ts\u0000done": 1, "packages/dashboard/app/components/command-center/MissionControlPanel.tsx\u0000done": 1, "packages/dashboard/app/components/command-center/MissionControlPanel.tsx\u0000in-review": 1, "packages/dashboard/app/components/command-center/MissionControlPanel.tsx\u0000todo": 1,