fix(cli): fn task retry now CRASHES on a renamed board — #2728 converted the classifier and left the target (#2752)
## This is a live regression on `main`, not a conversion `#2728` converted the retry **classifier** and left all three re-queue **targets** on the literal `"todo"`. That pairing is **strictly worse than the bug it fixed**: - **Before:** `fn task retry` silently did nothing on a renamed board. - **After (main today):** it correctly decides to retry, then throws. ``` TransitionRejectionError: Invalid transition: 'checking' → 'todo'. Unknown column for this workflow. ``` `todo` is not a column that board declares. Reproduced against main's exact code — reverting this fix fails **2 of 4** cases with that error. I flagged this on #2728 before it landed; posting it as a fix rather than a comment now that it is merged. ## Why the census did not catch it The census counts **comparisons**. A move **target** contains no comparison, so all three sites are invisible to it — `packages/cli/src/commands/task.ts` reads **0 guards** on main while the crash is live. That is the clearest case in this program so far that **the census measures conversion progress, not correctness**. A classifier and the target it feeds have to move together, and no automated signal will say so. ## The fix The target resolves from the task's own workflow: ```ts const retryHoldColumn = (await resolveTaskLifecycleColumns(context.store, id))?.hold ?? "todo"; ``` Failing soft to `"todo"` when the workflow cannot be resolved, matching every other fallback in this file. Three call sites, all three converted. ## Revert proof | state | result | |---|---| | main today (classifier converted, target literal) | **2 failed** / 2 passed — `Invalid transition: 'checking' → 'todo'` | | with this fix | **4 passed** | ## The fixture is derived, not hand-built The renamed workflow is `BUILTIN_CODING_WORKFLOW_IR` with **only its column ids renamed**, so the sole difference between the two runs is vocabulary. Hand-building a graph tested the fixture's shape as much as the code — the IR validator rejects an undeclared back-edge, and once declared as `kind: "rework"` the transition table still did not match the default board's. The suite also asserts the rename landed (`checking` present, `in-review` absent), so a surviving literal cannot pass by accident. Real store, real persisted workflow, driven through the real `runTaskRetry` — not the predicate. A unit test of the classifier goes green on the half-fix; only driving the whole command surfaces the crash. ## Relationship to #2736 This replaces it. #2736's other contents (active-task count, near-duplicate filter, archived-lineage label, node-override guards, the missing-worktree classifier) are now redundant with #2728, so they are dropped rather than re-litigated. What survives is this fix, its test, and the **changeset for the published CLI** that #2728 did not include. I will close #2736 once this is reviewed. ## Verification - new PG suite **4 passed** · `task-retry.test.ts` **7 passed** across 2 files - `pnpm test:gate` — **10 / 158 / 487 / 71** · `pnpm lint` clean · CLI `tsc --noEmit` clean · `check:changesets` passes · `--strict` exits 0 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
7
.changeset/cli-renamed-column-lanes.md
Normal file
7
.changeset/cli-renamed-column-lanes.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Fix CLI commands that stopped working on boards with renamed columns.
|
||||
category: fix
|
||||
dev: `fn task retry` classified stalls and re-queued with the literals `in-review`/`todo`, so on a renamed board it silently did nothing (and, once the classifier alone was fixed, threw `Invalid transition`). Also converted the near-duplicate candidate filter, the archived-lineage label, both node-override in-progress guards, and the four copies of the active-task count in `fn dashboard` (all four reported `active=0`). Column roles now resolve from each task's own workflow traits, falling back to the legacy ids when a workflow cannot be resolved.
|
||||
268
packages/cli/src/__tests__/task-retry-renamed-review.pg.test.ts
Normal file
268
packages/cli/src/__tests__/task-retry-renamed-review.pg.test.ts
Normal file
@@ -0,0 +1,268 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-22:40 (fleet — CLI retry classifier):
|
||||
|
||||
`fn task retry` classified a stalled card with `task.column === "in-review"`. On a board whose
|
||||
review lane is named anything else, EVERY branch that flag feeds went false — the execution-stall
|
||||
branch, the merge-retry-stall branch, and the failed/stuck-killed branch — so the command reported
|
||||
nothing to do and exited without retrying the exact states those flags exist to name.
|
||||
|
||||
The failure is silent by construction: retry is the operator's recovery path for a stuck card, so
|
||||
the symptom is "the card stays stuck and the tool says it is fine", which reads as a lifecycle bug
|
||||
somewhere else entirely.
|
||||
|
||||
Real store, real persisted workflow, driven through the real `runTaskRetry` command path — the same
|
||||
posture as the sibling renamed-lane suites in core, because the defect lives in the command's
|
||||
classification of hydrated state rather than in a pure helper.
|
||||
*/
|
||||
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, expect, it, vi } from "vitest";
|
||||
import { pgDescribe } from "../../../core/src/__test-utils__/pg-test-harness.js";
|
||||
import { BUILTIN_CODING_WORKFLOW_IR } from "@fusion/core";
|
||||
import { createPgExtensionHarness } from "./pg-extension-harness.js";
|
||||
|
||||
const resolveProjectMock = vi.hoisted(() => vi.fn());
|
||||
const closeProjectStoreMock = vi.hoisted(() => vi.fn().mockResolvedValue(undefined));
|
||||
vi.mock("../project-context.js", () => ({
|
||||
resolveProject: resolveProjectMock,
|
||||
closeProjectStore: closeProjectStoreMock,
|
||||
}));
|
||||
|
||||
import { runTaskRetry } from "../commands/task.js";
|
||||
|
||||
pgDescribe("runTaskRetry under a renamed review column", () => {
|
||||
const h = createPgExtensionHarness("fn-task-retry-renamed");
|
||||
|
||||
beforeAll(h.beforeAll);
|
||||
beforeEach(async () => {
|
||||
await h.beforeEach();
|
||||
resolveProjectMock.mockResolvedValue({
|
||||
store: h.store(),
|
||||
projectId: h.rootDir(),
|
||||
projectPath: h.rootDir(),
|
||||
projectName: "test",
|
||||
isRegistered: false,
|
||||
});
|
||||
vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
});
|
||||
afterEach(async () => {
|
||||
vi.restoreAllMocks();
|
||||
resolveProjectMock.mockReset();
|
||||
closeProjectStoreMock.mockClear();
|
||||
await h.afterEach();
|
||||
});
|
||||
afterAll(h.afterAll);
|
||||
|
||||
/**
|
||||
* A workflow whose review column is `checking` and which has NO `in-review` column, so a surviving
|
||||
* literal cannot match by luck. The `merge` trait carries the review role, and the IR validator
|
||||
* requires a reachable merge-class node for it, hence the merge-gate.
|
||||
*/
|
||||
/**
|
||||
* The BUILTIN coding workflow with its column ids renamed — nothing else changed.
|
||||
*
|
||||
* Hand-building a four-node graph did not work and the failures were instructive: the IR validator
|
||||
* rejects an undeclared back-edge, and once declared, the transition table still did not match the
|
||||
* default board's. A hand-rolled fixture tests the fixture's shape as much as the code. Deriving
|
||||
* from the builtin guarantees the ONLY difference between the two runs is the vocabulary, which is
|
||||
* the entire differential claim this suite rests on.
|
||||
*/
|
||||
async function seedRenamedWorkflow(): Promise<string> {
|
||||
const RENAME: Record<string, string> = {
|
||||
todo: "drafting",
|
||||
"in-progress": "building",
|
||||
"in-review": "checking",
|
||||
done: "shipped",
|
||||
};
|
||||
const rename = (id: string | undefined) => (id && RENAME[id]) ?? id;
|
||||
const builtin = JSON.parse(JSON.stringify(BUILTIN_CODING_WORKFLOW_IR)) as {
|
||||
id: string;
|
||||
nodes?: { column?: string }[];
|
||||
columns?: { id: string }[];
|
||||
};
|
||||
builtin.id = "custom:cli-renamed-review";
|
||||
for (const node of builtin.nodes ?? []) node.column = rename(node.column);
|
||||
for (const column of builtin.columns ?? []) column.id = rename(column.id) as string;
|
||||
|
||||
/* Prove the rename actually landed: if `checking` were absent, a surviving `in-review` literal
|
||||
would pass by accident and this suite would assert nothing. */
|
||||
const ids = (builtin.columns ?? []).map((column) => column.id);
|
||||
expect(ids).toContain("checking");
|
||||
expect(ids).not.toContain("in-review");
|
||||
|
||||
const created = await h.store().createWorkflowDefinition({
|
||||
name: "Renamed Review",
|
||||
kind: "workflow",
|
||||
ir: builtin,
|
||||
} as never);
|
||||
return (created as { id: string }).id;
|
||||
}
|
||||
|
||||
/**
|
||||
* A failed card resting in the board's review lane, optionally bound to a custom workflow.
|
||||
*
|
||||
* `path` walks the workflow graph rather than jumping straight to the review column: moves are
|
||||
* transition-validated, so a direct hop is rejected under BOTH vocabularies. Walking it also means
|
||||
* the card arrives the way a real one does.
|
||||
*/
|
||||
async function seedFailedInReviewLane(path: readonly string[], workflowId?: string): Promise<string> {
|
||||
const store = h.store();
|
||||
const column = path[path.length - 1];
|
||||
const task = await store.createTask({
|
||||
title: "stalled in the review lane",
|
||||
description: "test",
|
||||
column: "todo",
|
||||
});
|
||||
if (workflowId) await store.writeTaskWorkflowSelection(task.id, workflowId, []);
|
||||
for (const step of path) await store.moveTask(task.id, step as never);
|
||||
await store.updateTask(task.id, { status: "failed" } as never);
|
||||
store.taskCache.delete(task.id);
|
||||
|
||||
/*
|
||||
Prove the fixture before asserting on it. If the seed did not actually land the card in the
|
||||
review lane with a failed status, the retry would be a no-op for a reason that has nothing to do
|
||||
with column vocabulary, and this suite would pass while testing nothing.
|
||||
*/
|
||||
const seeded = await store.getTask(task.id);
|
||||
expect(seeded.column).toBe(column);
|
||||
expect(seeded.status).toBe("failed");
|
||||
return task.id;
|
||||
}
|
||||
|
||||
/** Retry is observable as the card leaving the review lane, or its failure being cleared. */
|
||||
async function retriedOutOfReview(taskId: string, reviewColumn: string): Promise<boolean> {
|
||||
await runTaskRetry(taskId);
|
||||
h.store().taskCache.delete(taskId);
|
||||
const after = await h.store().getTask(taskId);
|
||||
return after.column !== reviewColumn || after.status !== "failed";
|
||||
}
|
||||
|
||||
/* Control: the default vocabulary retries. Passes before and after the fix, so a generally broken
|
||||
retry path cannot hide behind the renamed case below. */
|
||||
it("default vocabulary: a failed card in the review lane is retried", async () => {
|
||||
const id = await seedFailedInReviewLane(["in-progress", "in-review"]);
|
||||
|
||||
expect(await retriedOutOfReview(id, "in-review")).toBe(true);
|
||||
});
|
||||
|
||||
/*
|
||||
The defect. Before the fix `checking` failed `task.column === "in-review"`, so every stall flag was
|
||||
false and the command exited having done nothing to a card the operator asked it to rescue.
|
||||
*/
|
||||
it("renamed vocabulary: a failed card in the RENAMED review lane is retried", async () => {
|
||||
const wf = await seedRenamedWorkflow();
|
||||
const id = await seedFailedInReviewLane(["drafting", "building", "checking"], wf);
|
||||
|
||||
expect(await retriedOutOfReview(id, "checking")).toBe(true);
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-31-01:15 (PR #2736 review — greptile P1):
|
||||
THE TWO CLASSIFIERS MUST AGREE ON WHAT "IN REVIEW" MEANS.
|
||||
|
||||
Converting only the GENERIC retry classifier split the two: on a renamed lane the generic branch
|
||||
fires while `isInReviewMissingWorktreeSessionStartFailure` (literal) does not.
|
||||
|
||||
MEASURED, because the reported consequence did not reproduce. The claim was that the generic branch
|
||||
leaves `worktree`/`branch` intact so the next run repeats the failure. It does not: BOTH branches
|
||||
end with them cleared, because the backward move to the hold column clears them anyway. Reverting
|
||||
the fix leaves these cases green, so this suite does NOT prove that consequence and is not claimed
|
||||
to. It pins the outcome that matters — a missing-worktree failure in either vocabulary comes back
|
||||
retryable with no stale session metadata — and the classifiers are kept in agreement because two
|
||||
definitions of "in review" in one function is a latent split, not because a failing case was found.
|
||||
*/
|
||||
async function seedMissingWorktreeFailure(path: readonly string[], workflowId?: string): Promise<string> {
|
||||
const store = h.store();
|
||||
const task = await store.createTask({
|
||||
title: "unusable worktree session start",
|
||||
description: "test",
|
||||
column: "todo",
|
||||
});
|
||||
if (workflowId) await store.writeTaskWorkflowSelection(task.id, workflowId, []);
|
||||
for (const step of path) await store.moveTask(task.id, step as never);
|
||||
await store.updateTask(task.id, {
|
||||
status: "failed",
|
||||
error: "Refusing to start coding agent in missing worktree: /tmp/fusion-missing-worktree",
|
||||
worktree: "/tmp/fusion-missing-worktree",
|
||||
branch: `fusion/${task.id}`,
|
||||
} as never);
|
||||
store.taskCache.delete(task.id);
|
||||
|
||||
/* Prove the fixture: without the stale metadata actually present, "it was cleared" is vacuous.
|
||||
`worktree`/`branch` are the two the generic retry branch leaves ALONE, so they are exactly the
|
||||
signal that distinguishes the specialized path from it. */
|
||||
const seeded = await store.getTask(task.id);
|
||||
expect(seeded.worktree).toBe("/tmp/fusion-missing-worktree");
|
||||
expect(seeded.branch).toBe(`fusion/${task.id}`);
|
||||
return task.id;
|
||||
}
|
||||
|
||||
async function retriedWithClearedSession(taskId: string): Promise<boolean> {
|
||||
await runTaskRetry(taskId);
|
||||
h.store().taskCache.delete(taskId);
|
||||
const after = await h.store().getTask(taskId);
|
||||
return !after.worktree && !after.branch;
|
||||
}
|
||||
|
||||
/* Control: the default vocabulary takes the specialized branch and clears the stale session. */
|
||||
it("default vocabulary: a missing-worktree failure has its stale session metadata cleared", async () => {
|
||||
const id = await seedMissingWorktreeFailure(["in-progress", "in-review"]);
|
||||
|
||||
expect(await retriedWithClearedSession(id)).toBe(true);
|
||||
});
|
||||
|
||||
/* The P1. Before the fix this retried via the GENERIC branch and kept the stale metadata. */
|
||||
it("renamed vocabulary: a missing-worktree failure has its stale session metadata cleared", async () => {
|
||||
const wf = await seedRenamedWorkflow();
|
||||
const id = await seedMissingWorktreeFailure(["drafting", "building", "checking"], wf);
|
||||
|
||||
expect(await retriedWithClearedSession(id)).toBe(true);
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-31-12:30 (PR #2752 review — greptile P1):
|
||||
THE GENERIC RETRY PATH — a plainly failed card outside the review lane.
|
||||
|
||||
The cases above all enter through the in-review stall branches. The ordinary retry — a `failed`
|
||||
card sitting in the WIP lane — falls through to a fourth `moveTask` that my first pass missed
|
||||
because it was written with single quotes while its three siblings used double. Three of four
|
||||
converted, and the one left behind was the common path.
|
||||
|
||||
Nothing automated would have caught that: the census counts comparisons and a move target has
|
||||
none, and a grep for the double-quoted form reports the file clean. So the suite now exercises the
|
||||
path by BEHAVIOUR rather than trusting that all the call sites were found.
|
||||
*/
|
||||
async function seedFailedInWipLane(path: readonly string[], workflowId?: string): Promise<string> {
|
||||
const store = h.store();
|
||||
const column = path[path.length - 1];
|
||||
const task = await store.createTask({ title: "plain failure", description: "test", column: "todo" });
|
||||
if (workflowId) await store.writeTaskWorkflowSelection(task.id, workflowId, []);
|
||||
for (const step of path) await store.moveTask(task.id, step as never);
|
||||
await store.updateTask(task.id, { status: "failed" } as never);
|
||||
store.taskCache.delete(task.id);
|
||||
|
||||
const seeded = await store.getTask(task.id);
|
||||
expect(seeded.column).toBe(column);
|
||||
expect(seeded.status).toBe("failed");
|
||||
return task.id;
|
||||
}
|
||||
|
||||
/* Control: the generic path works under the default vocabulary. */
|
||||
it("default vocabulary: a plainly failed WIP card is retried to the hold column", async () => {
|
||||
const id = await seedFailedInWipLane(["in-progress"]);
|
||||
|
||||
await runTaskRetry(id);
|
||||
h.store().taskCache.delete(id);
|
||||
expect((await h.store().getTask(id)).column).toBe("todo");
|
||||
});
|
||||
|
||||
/* The P1: this threw `Invalid transition: 'building' -> 'todo'` before the fourth site moved. */
|
||||
it("renamed vocabulary: a plainly failed WIP card is retried to the board's OWN hold column", async () => {
|
||||
const wf = await seedRenamedWorkflow();
|
||||
const id = await seedFailedInWipLane(["drafting", "building"], wf);
|
||||
|
||||
await runTaskRetry(id);
|
||||
h.store().taskCache.delete(id);
|
||||
expect((await h.store().getTask(id)).column).toBe("drafting");
|
||||
});
|
||||
});
|
||||
@@ -1400,12 +1400,33 @@ export async function runTaskRetry(id: string, projectName?: string) {
|
||||
throw new Error(`Task ${id} is not in a retryable state (status: ${task.status || 'none'})`);
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-31-11:20 (fleet — the retry TARGET, live regression on main):
|
||||
Retry re-queues the card to its board's HOLD column, resolved from the task's own workflow.
|
||||
|
||||
#2728 converted the retry CLASSIFIER above (`retryReviewColumns.has(task.column)`) and left all
|
||||
three re-queue targets below on the literal `"todo"`. That combination is strictly worse than the
|
||||
bug it fixed: before, `fn task retry` silently did nothing on a renamed board; after, it
|
||||
correctly decides to retry and then THROWS
|
||||
|
||||
TransitionRejectionError: Invalid transition: 'checking' -> 'todo'. Unknown column for this workflow.
|
||||
|
||||
because `todo` is not a column that board declares.
|
||||
|
||||
The census cannot see this: it counts COMPARISONS, and a move target contains none. So the file
|
||||
reads 0 guards while the crash is live — which is exactly why the classifier and the target must
|
||||
move together.
|
||||
|
||||
Fail-soft to `"todo"` when the workflow cannot be resolved, matching every other fallback here.
|
||||
*/
|
||||
const retryHoldColumn = (await resolveTaskLifecycleColumns(context.store, id))?.hold ?? "todo";
|
||||
|
||||
const autoPauseClearPatch = buildAutoPauseClearPatch(task);
|
||||
const clearedDeadlockAutoPause = Object.keys(autoPauseClearPatch).length > 0;
|
||||
const retryLogSuffix = clearedDeadlockAutoPause ? ", cleared deadlock auto-pause" : "";
|
||||
|
||||
if (isMissingWorktreeSessionRetry) {
|
||||
await retryBoardCall(context, id, "move task", () => context.store.moveTask(id, "todo", { preserveProgress: true }));
|
||||
await retryBoardCall(context, id, "move task", () => context.store.moveTask(id, retryHoldColumn as never, { preserveProgress: true }));
|
||||
await retryBoardCall(context, id, "update task", () => context.store.updateTask(id, {
|
||||
status: null,
|
||||
error: null,
|
||||
@@ -1427,7 +1448,7 @@ export async function runTaskRetry(id: string, projectName?: string) {
|
||||
// and merge failures (all steps done).
|
||||
if (isInReviewRetry) {
|
||||
if (isExecutionFailureInReview) {
|
||||
await retryBoardCall(context, id, "move task", () => context.store.moveTask(id, "todo", { preserveProgress: true }));
|
||||
await retryBoardCall(context, id, "move task", () => context.store.moveTask(id, retryHoldColumn as never, { preserveProgress: true }));
|
||||
await retryBoardCall(context, id, "update task", () => context.store.updateTask(id, {
|
||||
status: null,
|
||||
error: null,
|
||||
@@ -1447,7 +1468,7 @@ export async function runTaskRetry(id: string, projectName?: string) {
|
||||
return;
|
||||
}
|
||||
|
||||
await retryBoardCall(context, id, "move task", () => context.store.moveTask(id, "todo"));
|
||||
await retryBoardCall(context, id, "move task", () => context.store.moveTask(id, retryHoldColumn as never));
|
||||
await retryBoardCall(context, id, "update task", () => context.store.updateTask(id, {
|
||||
status: null,
|
||||
error: null,
|
||||
@@ -1462,10 +1483,20 @@ export async function runTaskRetry(id: string, projectName?: string) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Move to todo column before applying retry resets. `moveTask` reads from the
|
||||
// Move to the hold column before applying retry resets. `moveTask` reads from the
|
||||
// store's durable index and may overwrite task.json-only updates, so apply the
|
||||
// manual retry reset patch after the move to make the cleared counters stick.
|
||||
await retryBoardCall(context, id, "move task", () => context.store.moveTask(id, 'todo'));
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-31-12:30 (PR #2752 review — greptile P1):
|
||||
THE FOURTH TARGET, and the one that matters most.
|
||||
|
||||
This is the GENERIC retry fallthrough — a plainly `failed` or `stuck-killed` card, which is the
|
||||
ordinary case, not the in-review stall paths above. It was written with SINGLE quotes while its
|
||||
three siblings used double, so my first pass converted three of four and left the common path
|
||||
crashing. Found by review, not by me, and not by any tool: the census counts comparisons and sees
|
||||
none of these, and a same-file grep for the double-quoted form reports clean.
|
||||
*/
|
||||
await retryBoardCall(context, id, "move task", () => context.store.moveTask(id, retryHoldColumn as never));
|
||||
|
||||
// Clear failure state and stale branch refs so retry can choose a fresh base.
|
||||
await retryBoardCall(context, id, "update task", () => context.store.updateTask(id, {
|
||||
|
||||
Reference in New Issue
Block a user