diff --git a/packages/dashboard/src/__tests__/plan-approval-intake-column.test.ts b/packages/dashboard/src/__tests__/plan-approval-intake-column.test.ts new file mode 100644 index 0000000000..813063c3fe --- /dev/null +++ b/packages/dashboard/src/__tests__/plan-approval-intake-column.test.ts @@ -0,0 +1,129 @@ +// @vitest-environment node +/* +FNXC:WorkflowResolvedColumns 2026-07-29-00:00 (U12 — P0, post-#2515): +Plan approve/reject must resolve the workflow's INTAKE column, not the id `triage`. + +THE STALL THIS PINS. #2515 removed `triage` from the default lineage: there is now one +pre-implementation column, id `todo`, displayed as "Planning". The routes guarded with +`if (task.column !== "triage") throw badRequest(...)`, so after that merge the condition +was TRUE for every default-workflow card and BOTH routes rejected all of them. A card +parked `awaiting-approval` could be neither approved nor rejected — stuck, with no +operator action able to release it, and nothing crashing to reveal it. + +That is the inverse of the usual drift: the guard did not stop firing, it started firing +on everything. + +REVERT CHECK: restore either `task.column !== "triage"` literal and the matching case +fails with 400 instead of succeeding, because these cards are in `todo`. +*/ +import { describe, it, expect, vi } from "vitest"; +import express from "express"; +import { mkdtempSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import type { TaskStore, TaskDetail } from "@fusion/core"; +import { createApiRoutes } from "../routes.js"; +import { request as performRequest } from "../test-request.js"; + +/** The post-#2515 default lineage: ONE pre-implementation column, id `todo`. */ +const MERGED_CODING_IR = { + version: "v2", + name: "builtin-stepwise-coding", + columns: [ + { id: "todo", name: "Planning", traits: [{ trait: "intake" }, { trait: "hold" }] }, + { id: "in-progress", name: "In progress", traits: [{ trait: "wip" }] }, + { id: "done", name: "Done", traits: [{ trait: "complete" }] }, + ], + nodes: [{ id: "start", kind: "start", column: "todo" }, { id: "end", kind: "end", column: "done" }], + edges: [{ from: "start", to: "end" }], +}; + +/** A card parked awaiting approval on the merged planning column. */ +const PLANNING_TASK: TaskDetail = { + id: "FN-200", + title: "awaiting approval", + description: "", + column: "todo", + status: "awaiting-approval", + sourceType: "task_refine", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + prompt: "# Plan", +} as unknown as TaskDetail; + +function createMockStore(overrides: Partial = {}): TaskStore { + return { + getSettings: vi.fn().mockResolvedValue({}), + getRootDir: vi.fn().mockReturnValue(mkdtempSync(join(tmpdir(), "kb-plan-approval-"))), + getTask: vi.fn().mockResolvedValue(PLANNING_TASK), + updateTask: vi.fn().mockResolvedValue(PLANNING_TASK), + moveTask: vi.fn().mockResolvedValue(PLANNING_TASK), + logEntry: vi.fn().mockResolvedValue(undefined), + // Resolve the merged workflow so the routes see its real intake column. + getTaskWorkflowSelectionAsync: vi.fn().mockResolvedValue({ workflowId: "builtin:stepwise-coding" }), + getWorkflowDefinition: vi.fn().mockResolvedValue({ id: "builtin:stepwise-coding", name: "Coding", ir: MERGED_CODING_IR }), + listWorkflowDefinitions: vi.fn().mockResolvedValue([]), + on: vi.fn(), + off: vi.fn(), + getProjectScopedPluginMcpServers: vi.fn().mockResolvedValue([]), + ...overrides, + } as unknown as TaskStore; +} + +function createApp(store: TaskStore) { + const app = express(); + app.use(express.json()); + app.use("/api", createApiRoutes(store)); + return app; +} + +describe("plan approval on the merged planning column (post-#2515)", () => { + it("does NOT reject approve-plan for a card in the merged intake column", async () => { + const res = await performRequest(createApp(createMockStore()), "POST", "/api/tasks/FN-200/approve-plan"); + /* + Assert the SUCCESS status, not merely "not 400" (PR #2571 review — greptile). A + not-400 assertion also passes on a 404 or a 500, so it would keep this case green + while the route was broken in a different way — a guard that reports success without + checking, which is the class this whole audit exists to remove. + */ + expect(res.status).toBe(200); + }); + + it("does NOT reject reject-plan for a card in the merged intake column", async () => { + const res = await performRequest(createApp(createMockStore()), "POST", "/api/tasks/FN-200/reject-plan"); + expect(res.status).toBe(200); + }); + + /* + The two `task_refine` routes share the same converted guard, so they share the same + failure mode (PR #2571 review — greptile): before the fix they rejected every card on a + lineage without `triage`, which is how a stranded refinement became unrecoverable from + the UI. Covering only approve/reject would have left that guard unprotected. + */ + it("does NOT reject the stranded-refinement read for a merged-lineage card", async () => { + // The read path also consults the stranded-refinement list; stub it so a 500 from + // missing store surface cannot masquerade as the guard passing. + const store = createMockStore({ listStrandedRefinements: vi.fn().mockResolvedValue([]) }); + const res = await performRequest(createApp(store), "GET", "/api/tasks/FN-200/stranded-refinement"); + expect(res.status).toBe(200); + }); + + it("does NOT reject expedite-refinement for a merged-lineage card", async () => { + const store = createMockStore({ listStrandedRefinements: vi.fn().mockResolvedValue([]) }); + const res = await performRequest(createApp(store), "POST", "/api/tasks/FN-200/expedite-refinement"); + expect(res.status).toBe(200); + }); + + it("still rejects a card that is NOT in its workflow's intake column", async () => { + // The guard must narrow, not disappear: an in-progress card is still not approvable. + const store = createMockStore({ + getTask: vi.fn().mockResolvedValue({ ...PLANNING_TASK, column: "in-progress" }), + }); + const res = await performRequest(createApp(store), "POST", "/api/tasks/FN-200/approve-plan"); + expect(res.status).toBe(400); + }); +}); diff --git a/packages/dashboard/src/routes/register-task-workflow-routes.ts b/packages/dashboard/src/routes/register-task-workflow-routes.ts index 0eb629e0c2..bddd220038 100644 --- a/packages/dashboard/src/routes/register-task-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-task-workflow-routes.ts @@ -3699,9 +3699,21 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork const { store: scopedStore } = await getProjectContext(req); const task = await scopedStore.getTask(req.params.id); - // Verify task is in triage column with awaiting-approval status - if (task.column !== "triage") { - throw badRequest("Task must be in 'triage' column to approve plan"); + /* + 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 + 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") { + throw badRequest(`Task must be in the '${approveIntakeColumn}' column to approve plan`); } if (task.status !== "awaiting-approval") { throw badRequest("Task must have status 'awaiting-approval' to approve plan"); @@ -3760,9 +3772,11 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork const { store: scopedStore } = await getProjectContext(req); const task = await scopedStore.getTask(req.params.id); - // Verify task is in triage column with awaiting-approval status - if (task.column !== "triage") { - throw badRequest("Task must be in 'triage' column to reject plan"); + // 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") { + throw badRequest(`Task must be in the '${rejectIntakeColumn}' column to reject plan`); } if (task.status !== "awaiting-approval") { throw badRequest("Task must have status 'awaiting-approval' to reject plan"); @@ -3818,8 +3832,11 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (task.sourceType !== "task_refine") { throw badRequest("Task must have sourceType 'task_refine'"); } - if (task.column !== "triage") { - throw badRequest("Task must be in 'triage' column"); + // 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") { + throw badRequest(`Task must be in the '${refineIntakeColumn}' column`); } const stranded = await scopedStore.listStrandedRefinements(); @@ -3869,8 +3886,11 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork if (task.sourceType !== "task_refine") { throw badRequest("Task must have sourceType 'task_refine'"); } - if (task.column !== "triage") { - throw badRequest("Task must be in 'triage' column"); + // 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") { + throw badRequest(`Task must be in the '${refineIntakeColumn}' column`); } if (task.paused) { throw badRequest("Paused refinements cannot be expedited");