U12 drift: register-task-workflow-routes.ts — resolve the intake column (7 -> 1) (#2614)
**File claimed: `packages/dashboard/src/routes/register-task-workflow-routes.ts`.** Per-file lifecycle-column guard count: **7 → 1**, and the 1 is comment prose (line 3758), so this file is done for completion-bar item 1. ## The bug this fixes `retrySpecification` decided "this Retry is a re-plan, not a generic retry" with `task.column === "triage"`. `status: "planning"` is retryable **only** through that flag — it is not in the generic `failed`/`stuck-killed` set. So on any lineage whose intake column is not literally named `triage`, a card visibly sitting in planning got `400 Task is not in a retryable state`. The operator's Retry button did nothing, with no error to explain why. Post-#2515 that includes the **default** workflow: `columnsWithFlag(resolveDefaultWorkflowIr(), "intake")` is `["todo"]` and the default's columns are `[todo, in-progress, in-review, done, archived]` — `triage` is not declared at all. The pre-existing `todo` fallback below it papered over the default case (it fires when the workflow has no `triage`), which is why this did not show up as a total outage; custom and renamed lineages had no such cover. Now: `const retryIntakeColumn = await resolveIntakeColumnForTask(scopedStore, task.id)`. ## Red-green, measured `packages/dashboard/src/__tests__/plan-approval-intake-column.test.ts` — new case, custom lineage with intake `backlog`, card in `backlog` with `status: "planning"`: - with the change: `200` - with `task.column === "triage"` restored: **`AssertionError: expected 400 to be 200`** The fixture uses `planning` deliberately. A `failed` fixture would pass either way through the generic retryable set and prove nothing. ## What is NOT tested, and why not This PR also removes four `&& task.column !== "triage"` disjuncts I added earlier while widening the P0 approve/reject guard. **Those are untestable by construction** and I am not claiming coverage for them: removing an extra acceptance only shrinks what the guard accepts, and no case can feed these routes a `triage` card now that no shipped lineage declares one. I re-widened one guard and confirmed the suite stays green — i.e. nothing depends on the disjunct in either direction. That is the honest result, not a passing test. ## Three fixtures updated, not guards re-widened `stranded-refinements-routes.test.ts` failed with three `expected 400 to be 200` — the same failures that made me widen in the first place. This time I probed instead: `BASE_TASK` had `column: "triage"`, a column the default workflow no longer declares, so a 400 is **correct** and the fixtures were pre-merge artifacts describing a board shape the product stopped shipping. Changed to `column: "todo"` with the resolver output recorded in the file. ## Observation, deliberately not fixed here The `todo` fallback at ~2642 (`retrySpecification = !workflowHasColumn(workflowIr, "triage")`) is now near-dead: for the merged default the first branch already fires. It survives only for a lineage that has a `todo` column, no `triage`, and some *other* intake column — where treating a `todo` card as planning is arguably wrong. Deleting it is a behaviour change with its own blast radius, so it does not ride along in a conversion commit. ## Verification `pnpm lint` clean. `pnpm test:gate` green (10 / 482 / 71). Target suites: `plan-approval-intake-column.test.ts` 8/8, `stranded-refinements-routes.test.ts` 12/12. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
7
.changeset/u12-retry-intake-column.md
Normal file
7
.changeset/u12-retry-intake-column.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Retry now works for planning cards on boards whose first column isn't named "triage".
|
||||
category: fix
|
||||
dev: register-task-workflow-routes.ts resolves the intake column via columnsWithFlag(ir,"intake") instead of comparing task.column to the literal "triage"; 7 lifecycle-column comparisons in the file drop to 1 (comment text).
|
||||
@@ -239,3 +239,63 @@ describe("reset verification uses the resolved rebound column", () => {
|
||||
expect((correctionCall![1] as { column: string }).column).toBe("backlog");
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8 drift conversion):
|
||||
A SPEC RETRY must be recognised in the workflow's intake column, whatever its id.
|
||||
|
||||
`retrySpecification` decided "this is a re-plan, not a generic retry" with
|
||||
`task.column === "triage"`, plus a `todo` fallback that only applied when the workflow
|
||||
had no `triage` column at all. Both are id checks, so on a lineage whose intake column is
|
||||
named anything else the flag stayed false — and because `status: "planning"` is retryable
|
||||
ONLY through that flag, the route answered 400 "not in a retryable state" for a card that
|
||||
was plainly sitting in planning. The operator's Retry button did nothing.
|
||||
|
||||
This is the one conversion in this PR with an observable behaviour change, so it is the
|
||||
one that gets a test. Narrowing the four `&& task.column !== "triage"` disjuncts I had
|
||||
added while widening the P0 guard cannot be tested by construction: removing an extra
|
||||
acceptance only shrinks what is accepted, and no case feeds these routes a `triage` card
|
||||
now that no shipped lineage declares one.
|
||||
|
||||
REVERT CHECK: restore `task.column === "triage"` and this fails with 400, because the
|
||||
card is in `backlog` — measured, not assumed.
|
||||
*/
|
||||
describe("spec retry resolves the workflow's intake column", () => {
|
||||
const CUSTOM_IR = {
|
||||
version: "v2",
|
||||
name: "custom",
|
||||
columns: [
|
||||
{ id: "backlog", name: "Backlog", traits: [{ trait: "intake" }, { trait: "hold" }] },
|
||||
{ id: "building", name: "Building", traits: [{ trait: "wip" }] },
|
||||
{ id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
nodes: [{ id: "start", kind: "start", column: "backlog" }, { id: "end", kind: "end", column: "shipped" }],
|
||||
edges: [{ from: "start", to: "end" }],
|
||||
};
|
||||
|
||||
it("accepts a retry for a planning card in a custom intake column", async () => {
|
||||
/*
|
||||
`status: "planning"` is deliberate: it is NOT in the generic retryable set
|
||||
(`failed` / `stuck-killed`), so the request survives the retryable-state check only if
|
||||
`retrySpecification` resolved true. A `failed` fixture would pass either way and prove
|
||||
nothing.
|
||||
*/
|
||||
const planningTask = {
|
||||
...PLANNING_TASK,
|
||||
id: "FN-400",
|
||||
column: "backlog",
|
||||
status: "planning",
|
||||
} as unknown as TaskDetail;
|
||||
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue(planningTask),
|
||||
updateTask: vi.fn().mockResolvedValue(planningTask),
|
||||
moveTask: vi.fn().mockResolvedValue(planningTask),
|
||||
getTaskWorkflowSelectionAsync: vi.fn().mockResolvedValue({ workflowId: "wf-custom" }),
|
||||
getWorkflowDefinition: vi.fn().mockResolvedValue({ id: "wf-custom", name: "Custom", ir: CUSTOM_IR }),
|
||||
});
|
||||
|
||||
const res = await performRequest(createApp(store), "POST", "/api/tasks/FN-400/retry");
|
||||
expect(res.status).toBe(200);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -43,11 +43,25 @@ async function REQUEST(app: express.Express, method: string, path: string) {
|
||||
return performRequest(app, method, path);
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8 drift conversion):
|
||||
`todo`, not `triage`. #2515 merged the default lineage's pre-implementation columns into
|
||||
one whose id is `todo` (displayed "Planning") and REMOVED `triage` — so the default
|
||||
workflow no longer declares `triage` at all, and these refine routes now correctly reject
|
||||
a card sitting there. The fixture was a pre-merge artifact: it described a board shape the
|
||||
product no longer ships.
|
||||
|
||||
Verified rather than assumed: `columnsWithFlag(resolveDefaultWorkflowIr(), "intake")` is
|
||||
`["todo"]`, and the default's columns are `[todo, in-progress, in-review, done, archived]`.
|
||||
That is why narrowing the routes' intake guards to the resolved column (dropping the
|
||||
legacy `|| === "triage"` disjunct) surfaced these three, and why updating the fixture is
|
||||
the correct resolution rather than re-widening the guard.
|
||||
*/
|
||||
const BASE_TASK: TaskDetail = {
|
||||
id: "FN-100",
|
||||
title: "refine",
|
||||
description: "refine",
|
||||
column: "triage",
|
||||
column: "todo",
|
||||
sourceType: "task_refine",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
@@ -95,7 +109,8 @@ describe("stranded refinement routes", () => {
|
||||
const res = await REQUEST(createApp(store), "POST", "/api/tasks/FN-100/expedite-refinement");
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.expedited).toBe(true);
|
||||
expect(res.body.task.column).toBe("triage");
|
||||
// Echoes the fixture's column, which is now the merged planning column.
|
||||
expect(res.body.task.column).toBe("todo");
|
||||
});
|
||||
|
||||
it("GET /tasks/:id/stranded-refinement returns detail", async () => {
|
||||
|
||||
@@ -2619,7 +2619,17 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
task.status === "planning" ||
|
||||
task.status === "needs-replan" ||
|
||||
(task.stuckKillCount ?? 0) > 0;
|
||||
let retrySpecification = task.column === "triage" && retrySpecificationStatus;
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8 drift conversion):
|
||||
The INTAKE column, resolved from the task's workflow. `=== "triage"` stopped matching
|
||||
for default-workflow cards once the merged lineage dropped that id, so a spec retry on
|
||||
a planning card fell through to the generic-retry path below. That path still catches
|
||||
it for the merged shape (it keys on `todo` when the workflow declares no `triage`), so
|
||||
this was not a stall — but it worked by accident of the two conditions overlapping,
|
||||
not because either was right.
|
||||
*/
|
||||
const retryIntakeColumn = await resolveIntakeColumnForTask(scopedStore, task.id);
|
||||
let retrySpecification = task.column === retryIntakeColumn && retrySpecificationStatus;
|
||||
/*
|
||||
FNXC:ManualRetry 2026-07-13-12:20:
|
||||
Plan-in-place workflows (Coding (Ideas): no "triage" column) keep planning/replanning
|
||||
@@ -3745,16 +3755,24 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — P0, post-#2515):
|
||||
Resolve the workflow's INTAKE column; do not name `triage`. #2515 removed `triage`
|
||||
from the default lineage — the single pre-implementation column is now id `todo`
|
||||
displayed as "Planning" — so `task.column !== "triage"` became TRUE for every
|
||||
displayed as "Planning" — so comparing the card's column against the legacy
|
||||
`triage` id became TRUE for every
|
||||
default-workflow card and this route rejected all of them. A card parked
|
||||
`awaiting-approval` could not be approved OR rejected (same guard below), i.e. it
|
||||
was STUCK with no operator action able to release it. The guard did not stop
|
||||
firing; it started firing on everything.
|
||||
*/
|
||||
const approveIntakeColumn = await resolveIntakeColumnForTask(scopedStore, task.id);
|
||||
// WIDEN, never narrow: accept the resolved intake column OR the legacy id, so this
|
||||
// P0 fix cannot reject a card the route previously allowed.
|
||||
if (task.column !== approveIntakeColumn && task.column !== "triage") {
|
||||
/*
|
||||
The resolved column ONLY — the legacy-`triage` disjunct this comment
|
||||
used to justify is gone (PR #2614 review — greptile: the comment outlived the code).
|
||||
It was a belt-and-braces widening added with the P0 fix, on the theory that a card
|
||||
might still be sitting in `triage`. Nothing shipped declares that column since
|
||||
#2515, so the disjunct only widened what the guard accepts, and re-adding it changed
|
||||
no test in either direction. A guard that accepts a column no workflow declares is
|
||||
not caution, it is an unreachable branch that reads like a requirement.
|
||||
*/
|
||||
if (task.column !== approveIntakeColumn) {
|
||||
throw badRequest(`Task must be in the '${approveIntakeColumn}' column to approve plan`);
|
||||
}
|
||||
if (task.status !== "awaiting-approval") {
|
||||
@@ -3817,7 +3835,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
// Same P0 as approve-plan above: resolve the intake column rather than naming
|
||||
// `triage`, which #2515 removed from the default lineage.
|
||||
const rejectIntakeColumn = await resolveIntakeColumnForTask(scopedStore, task.id);
|
||||
if (task.column !== rejectIntakeColumn && task.column !== "triage") {
|
||||
if (task.column !== rejectIntakeColumn) {
|
||||
throw badRequest(`Task must be in the '${rejectIntakeColumn}' column to reject plan`);
|
||||
}
|
||||
if (task.status !== "awaiting-approval") {
|
||||
@@ -3877,7 +3895,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
// Intake column, resolved from the task's workflow (#2515 removed `triage` from
|
||||
// the default lineage, so the literal rejected every default-workflow card).
|
||||
const refineIntakeColumn = await resolveIntakeColumnForTask(scopedStore, task.id);
|
||||
if (task.column !== refineIntakeColumn && task.column !== "triage") {
|
||||
if (task.column !== refineIntakeColumn) {
|
||||
throw badRequest(`Task must be in the '${refineIntakeColumn}' column`);
|
||||
}
|
||||
|
||||
@@ -3931,7 +3949,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
// Intake column, resolved from the task's workflow (#2515 removed `triage` from
|
||||
// the default lineage, so the literal rejected every default-workflow card).
|
||||
const refineIntakeColumn = await resolveIntakeColumnForTask(scopedStore, task.id);
|
||||
if (task.column !== refineIntakeColumn && task.column !== "triage") {
|
||||
if (task.column !== refineIntakeColumn) {
|
||||
throw badRequest(`Task must be in the '${refineIntakeColumn}' column`);
|
||||
}
|
||||
if (task.paused) {
|
||||
@@ -4643,7 +4661,13 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
|
||||
// If task is already at its workflow's intake column, skip the transition
|
||||
// check and moveTask. Just reset for replanning in place.
|
||||
if (task.column === "triage" || task.column === respecifyTarget) {
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — R8 drift conversion):
|
||||
`respecifyTarget` IS the resolved intake column (`resolveIntakeColumnForTask`), so the
|
||||
`=== "triage"` disjunct only ever fired for a workflow whose intake is literally
|
||||
triage — which that same call already returns. Redundant before the merge, dead after.
|
||||
*/
|
||||
if (task.column === respecifyTarget) {
|
||||
// Log the revision request
|
||||
await scopedStore.logEntry(task.id, "AI spec revision requested", feedback);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user