diff --git a/.changeset/retry-follows-planning-column.md b/.changeset/retry-follows-planning-column.md new file mode 100644 index 0000000000..bfbf438688 --- /dev/null +++ b/.changeset/retry-follows-planning-column.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix Retry refusing cards parked mid-planning on five built-in workflows. +category: fix +dev: The manual retry route decided between specification retry (needs-replan + delete PROMPT.md) and execution retry via `!workflowHasColumn(ir, "triage")`. Measured across all 12 builtins: none plans in `triage`, but seven declare that column, so quick-fix / review-heavy / compound-engineering / design / legacy-coding refused a planning-status card in their own planning column with 400. New `workflowPlansInColumn(ir, column)` asks the graph where planning happens; a card in a pre-WIP column that is not the planning column now takes the non-destructive execution retry rather than losing its spec or its button. diff --git a/packages/core/src/__tests__/workflow-plans-in-column.test.ts b/packages/core/src/__tests__/workflow-plans-in-column.test.ts new file mode 100644 index 0000000000..41c84291f1 --- /dev/null +++ b/packages/core/src/__tests__/workflow-plans-in-column.test.ts @@ -0,0 +1,223 @@ +import { describe, expect, it } from "vitest"; +import { workflowDeclaresColumnModel, workflowHasColumn, workflowPlansInColumn } from "../workflow-transitions.js"; +import { BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR } from "../builtin-stepwise-final-review-coding-workflow-ir.js"; +import { BUILTIN_CODING_IDEAS_WORKFLOW_IR } from "../builtin-coding-ideas-workflow-ir.js"; +import { BUILTIN_WORKFLOWS } from "../builtin-workflows.js"; +import { parseWorkflowIr } from "../workflow-ir.js"; +import type { WorkflowIr } from "../workflow-ir.js"; + +/** Builtin IRs are authored either as objects or as JSON strings. */ +function irOf(workflow: { ir: unknown }): WorkflowIr { + return (typeof workflow.ir === "string" ? parseWorkflowIr(workflow.ir) : workflow.ir) as WorkflowIr; +} + +/* +FNXC:WorkflowRetry 2026-07-29-20:55 (triage census): +`workflowPlansInColumn` replaces `!workflowHasColumn(ir, "triage")` as the answer to "does this card +sit where its workflow plans?" — the input to the manual Retry route's DESTRUCTIVE specification +branch (stamp needs-replan, delete PROMPT.md). + +The first test is the measurement that condemned the old proxy, taken across all 12 shipped +workflows. It is not merely dead: 7 builtins still declare a `triage` column while NONE plans there, +so for 5 of them the proxy actively DENIED the retry a card in their planning column needed. +*/ +describe("workflowPlansInColumn", () => { + /* + THE MEASUREMENT THAT CONDEMNS THE OLD PROXY, across every shipped workflow rather than the two + that motivated it. Two facts, both asserted below: + + 1. NO builtin places a planning node in `triage` — planning happens in `todo` wherever it + happens at all. So "the workflow has a triage column" says nothing about where it plans. + 2. 7 of 12 builtins still DECLARE a `triage` column. The proxy is therefore not merely dead, + it is inverted for those: `!workflowHasColumn(ir, "triage")` is FALSE, so a card in `todo` + was denied specification retry by workflows that do all their planning in `todo`. + + If a future workflow ever does plan in `triage`, fact 1 goes red here — deliberately, because that + is the one shape that would make the old proxy meaningful again. + */ + it("MEASUREMENT: no builtin plans in `triage`, yet 7 still declare that column", () => { + const declaringTriage = BUILTIN_WORKFLOWS.filter((w) => workflowHasColumn(irOf(w), "triage")); + const planningInTriage = BUILTIN_WORKFLOWS.filter((w) => workflowPlansInColumn(irOf(w), "triage")); + + expect(planningInTriage.map((w) => w.id)).toEqual([]); + expect(declaringTriage.map((w) => w.id)).toEqual([ + "builtin:legacy-coding", + "builtin:quick-fix", + "builtin:review-heavy", + "builtin:compound-engineering", + "builtin:design", + "builtin:pr-workflow", + "builtin:lead-generation", + ]); + }); + + /* + The live consequence of fact 2: these five workflows declare `triage` AND plan in `todo`, so the + old proxy denied specification retry to a planning/needs-replan card sitting in their planning + column — the manual Retry route answered 400 "not in a retryable state" and the operator had no + button. `workflowPlansInColumn` gives every one of them the right answer. + */ + it("gives specification retry to workflows that declare triage but plan in todo", () => { + const declaresTriageAndPlansInTodo = BUILTIN_WORKFLOWS + .filter((w) => workflowHasColumn(irOf(w), "triage") && workflowPlansInColumn(irOf(w), "todo")) + .map((w) => w.id); + + expect(declaresTriageAndPlansInTodo).toEqual([ + "builtin:legacy-coding", + "builtin:quick-fix", + "builtin:review-heavy", + "builtin:compound-engineering", + "builtin:design", + ]); + }); + + it("reports no planning column for workflows that have no plan nodes at all", () => { + // marketing / pr-workflow / lead-generation never plan, so no column of theirs may be + // classified as a planning column — a spec-deleting retry there would be pure damage. + for (const id of ["builtin:marketing", "builtin:pr-workflow", "builtin:lead-generation"]) { + const workflow = BUILTIN_WORKFLOWS.find((w) => w.id === id); + expect(workflow, id).toBeDefined(); + for (const column of ["todo", "triage", "in-progress", "in-review"]) { + expect(workflowPlansInColumn(irOf(workflow!), column), `${id}/${column}`).toBe(false); + } + } + }); + + it("reports the column that actually hosts the plan nodes for both builtins", () => { + for (const ir of [BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR, BUILTIN_CODING_IDEAS_WORKFLOW_IR]) { + expect(workflowPlansInColumn(ir, "todo")).toBe(true); + // Cards parked elsewhere are not "where planning happens", so a retry there must not be + // treated as a re-plan. + expect(workflowPlansInColumn(ir, "in-progress")).toBe(false); + expect(workflowPlansInColumn(ir, "in-review")).toBe(false); + expect(workflowPlansInColumn(ir, "done")).toBe(false); + } + // Ideas' manual intake column is NOT its planning column — planning runs after promotion. + expect(workflowPlansInColumn(BUILTIN_CODING_IDEAS_WORKFLOW_IR, "ideas")).toBe(false); + }); + + /* + THE CASE THE OLD PROXY GOT WRONG. A workflow with no `triage` column that plans in its own intake + column: the old predicate said `!hasColumn("triage")` -> true for a card in `todo`, so a failed + card parked in `todo` was given a specification retry — needs-replan plus PROMPT.md deleted — + even though `todo` hosts no plan node at all. + */ + it("refuses a column that hosts no plan node, even when the workflow has no triage column", () => { + const plansInIntake = { + version: "v2", + id: "custom:plans-in-intake", + name: "Plans In Intake", + columns: [ + { id: "inbox", traits: ["intake"] }, + { id: "todo", traits: ["hold"] }, + { id: "in-progress", traits: ["wip"] }, + ], + nodes: [ + { id: "plan", kind: "prompt", column: "inbox", config: {} }, + { id: "plan-review", kind: "prompt", column: "inbox", config: {} }, + ], + edges: [], + } as unknown as WorkflowIr; + + expect(workflowHasColumn(plansInIntake, "triage")).toBe(false); + // Old proxy: !hasColumn("triage") === true -> spec retry in todo. Wrong. + expect(workflowPlansInColumn(plansInIntake, "todo")).toBe(false); + expect(workflowPlansInColumn(plansInIntake, "inbox")).toBe(true); + }); + + it("keeps legacy behaviour for a workflow that plans in triage", () => { + const legacy = { + version: "v2", + id: "custom:legacy-triage", + name: "Legacy", + columns: [ + { id: "triage", traits: ["intake"] }, + { id: "todo", traits: ["hold"] }, + ], + nodes: [{ id: "plan", kind: "prompt", column: "triage", config: {} }], + edges: [], + } as unknown as WorkflowIr; + + // A card in `todo` under a triage-planning workflow got generic retry before and must still. + expect(workflowPlansInColumn(legacy, "todo")).toBe(false); + expect(workflowPlansInColumn(legacy, "triage")).toBe(true); + }); + + /* + GREPTILE #2621: a custom workflow whose planning node has a bespoke id was reported as having NO + planning column, so Manual Retry preserved a stale specification instead of replanning. Recognise + the SEMANTIC markers the builtins carry — `config.seam` and `config.workflowAction` — so an id + outside the known list is still classified when the workflow reuses a builtin planning seam. + */ + it("recognises a planning node by config.seam, whatever its id", () => { + const ir = { + version: "v2", + id: "custom:seam-marker", + name: "Seam", + columns: [{ id: "specify", traits: [{ trait: "intake" }] }, { id: "todo", traits: [{ trait: "hold" }] }], + nodes: [{ id: "write-the-spec", kind: "prompt", column: "specify", config: { seam: "planning", name: "Spec" } }], + edges: [], + } as unknown as WorkflowIr; + + expect(workflowPlansInColumn(ir, "specify")).toBe(true); + expect(workflowPlansInColumn(ir, "todo")).toBe(false); + }); + + it("recognises a planning node by config.workflowAction, whatever its id", () => { + const ir = { + version: "v2", + id: "custom:action-marker", + name: "Action", + columns: [{ id: "backlog", traits: [{ trait: "intake" }] }], + nodes: [{ id: "redo-it", kind: "prompt", column: "backlog", config: { workflowAction: "plan-replan" } }], + edges: [], + } as unknown as WorkflowIr; + + expect(workflowPlansInColumn(ir, "backlog")).toBe(true); + }); + + it("does not classify a non-planning seam as planning", () => { + // `startsWith("plan")` on workflowAction must not swallow unrelated actions, and an + // implementation/review seam is not a planning seam. + const ir = { + version: "v2", + id: "custom:other-seams", + name: "Other", + columns: [{ id: "todo", traits: [{ trait: "hold" }] }], + nodes: [ + { id: "build", kind: "prompt", column: "todo", config: { seam: "implementation" } }, + { id: "check", kind: "prompt", column: "todo", config: { workflowAction: "code-review" } }, + // greptile #2621: a `plan`-PREFIXED action that is not planning. A startsWith("plan") test + // classified this column as planning, and the caller's planning branch DELETES PROMPT.md — + // so the loose match cost a specification. Must be an exact set. + { id: "run-it", kind: "prompt", column: "todo", config: { workflowAction: "plan-execute" } }, + { id: "fixup", kind: "prompt", column: "todo", config: { workflowAction: "pre-merge-remediation" } }, + ], + edges: [], + } as unknown as WorkflowIr; + + expect(workflowPlansInColumn(ir, "todo")).toBe(false); + }); + + /* + GREPTILE #2621: callers gating a DESTRUCTIVE action must be able to tell "not a planning column" + from "this IR cannot answer the question". A v1 IR is the second, and reading it as the first made + Manual Retry answer 400 for a v1 planning card that the old predicate admitted. + */ + it("distinguishes an unanswerable IR from a negative answer", () => { + expect(workflowDeclaresColumnModel({ version: 1 } as unknown as WorkflowIr)).toBe(false); + expect(workflowDeclaresColumnModel({} as unknown as WorkflowIr)).toBe(false); + // Columns but no nodes: placement is still unanswerable. + expect(workflowDeclaresColumnModel({ + version: "v2", columns: [{ id: "todo", traits: [] }], + } as unknown as WorkflowIr)).toBe(false); + expect(workflowDeclaresColumnModel(BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR)).toBe(true); + expect(workflowDeclaresColumnModel(BUILTIN_CODING_IDEAS_WORKFLOW_IR)).toBe(true); + }); + + it("returns false rather than throwing for a v1 IR with no nodes array", () => { + // Callers use this to gate a destructive branch; an un-nodeed IR must fail CLOSED. + expect(workflowPlansInColumn({ version: 1 } as unknown as WorkflowIr, "todo")).toBe(false); + expect(workflowPlansInColumn({} as unknown as WorkflowIr, "todo")).toBe(false); + }); +}); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index cfa0203d55..705e933fbf 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -438,6 +438,8 @@ export { resolveColumnAdjacency, resolveAllowedColumns, workflowHasColumn, + workflowPlansInColumn, + workflowDeclaresColumnModel, } from "./workflow-transitions.js"; export type { ColumnAdjacency } from "./workflow-transitions.js"; // ── U8: pre-evaluated plugin gate verdicts (KTD-2) ─────────────────────────── diff --git a/packages/core/src/workflow-transitions.ts b/packages/core/src/workflow-transitions.ts index bf21b93c47..bd3743deec 100644 --- a/packages/core/src/workflow-transitions.ts +++ b/packages/core/src/workflow-transitions.ts @@ -212,3 +212,82 @@ export function workflowHasColumn(ir: WorkflowIr, columnId: string): boolean { const v2 = ir as WorkflowIrV2; return Array.isArray(v2.columns) && v2.columns.some((c) => c.id === columnId); } + +/* +FNXC:WorkflowRetry 2026-07-29-20:55 (triage census — the dead `hasColumn("triage")` proxy): +Callers need to know "is this card sitting where its workflow actually plans?" — the manual Retry +route uses it to choose SPECIFICATION retry (status -> needs-replan AND delete PROMPT.md) over +generic execution retry. + +That question was previously asked as `!workflowHasColumn(ir, "triage")`: a workflow with no triage +column was assumed to plan in place in `todo`. MEASURED after U11 merged the two pre-implementation +columns: NO builtin workflow declares a `triage` column any more (`builtin:coding` and +`builtin:coding-ideas` both report false), so that proxy is now a constant `true` and decides +nothing. It happens to yield the right answer for both builtins only because their plan nodes really +are in `todo` — the guard is dead AND accidentally correct, which is worse than wrong, because the +next workflow that plans somewhere else inherits a spec-deleting false positive with no failing test. + +Ask the graph directly instead. The plan family is the authoritative answer to "where does planning +happen", and it is derived per workflow, so a custom board that plans in its own intake column is +handled without a vocabulary list. +*/ +/* +Planning nodes are recognised by SEMANTIC MARKERS first, ids second. + +MEASURED shapes across the 12 builtins: the specification node carries +`config.seam === "planning"` (builtinPromptConfig), the replan node carries +`config.workflowAction === "plan-replan"`, and ids in use are `plan`, `planning` +(builtin:legacy-coding), `plan-review`, `plan-replan`, `plan-review-step`. + +Matching on markers means a custom workflow that reuses the builtin planning seam or action is +classified correctly whatever it names its node — id-only matching reported such a workflow's real +planning column as non-planning. The id list stays as a backstop for hand-authored IRs that set +neither marker. + +KNOWN LIMIT, stated rather than hidden: a fully bespoke planning node — custom id, custom prompt, +no seam and no workflowAction — is still not recognised. That is unknowable without guessing, and the +failure direction is the safe one: the caller falls back to ordinary execution retry, which PRESERVES +the specification instead of deleting it, and the card stays retryable. +*/ +const PLANNING_NODE_IDS = new Set(["plan", "planning", "plan-review", "plan-replan", "plan-review-step"]); +const PLANNING_SEAMS = new Set(["planning", "plan-review"]); +/* +An EXACT set, never a `startsWith("plan")` prefix (greptile #2621). The prefix matched in the +DESTRUCTIVE direction: a custom action such as `plan-execute` would have classified an +implementation column as a planning column, and the caller then stamps `needs-replan` and DELETES +PROMPT.md. MEASURED workflowAction vocabulary in tree: `plan-replan`, `code-review`, +`pre-merge-remediation` — only the first is planning. A new planning action must be added here +deliberately; being unlisted costs a replan, being wrongly listed costs a specification. +*/ +const PLANNING_WORKFLOW_ACTIONS = new Set(["plan-replan"]); + +function isPlanningNode(node: { id?: unknown; config?: unknown }): boolean { + if (typeof node?.id === "string" && PLANNING_NODE_IDS.has(node.id)) return true; + const config = node?.config as { seam?: unknown; workflowAction?: unknown } | undefined; + if (typeof config?.seam === "string" && PLANNING_SEAMS.has(config.seam)) return true; + if (typeof config?.workflowAction === "string" && PLANNING_WORKFLOW_ACTIONS.has(config.workflowAction)) return true; + return false; +} + +/** + * True when the workflow places any planning node in `columnId` — i.e. cards plan in that column. + * + * Returns false for an IR with no node list (a v1 IR). Callers gating a DESTRUCTIVE action on this + * must not read that false as "this column is past planning" — it means "this IR cannot answer the + * question"; use {@link workflowDeclaresColumnModel} to tell the two apart. + */ +export function workflowPlansInColumn(ir: WorkflowIr, columnId: string): boolean { + const v2 = ir as WorkflowIrV2 & { nodes?: Array<{ id?: string; column?: string; config?: unknown }> }; + if (!Array.isArray(v2.nodes)) return false; + return v2.nodes.some((node) => isPlanningNode(node) && node.column === columnId); +} + +/** + * True when the IR describes columns and nodes at all — i.e. a v2 graph whose placement questions + * are answerable. A v1 IR answers `false`, so callers can distinguish "not a planning column" from + * "this workflow has no column model" instead of treating silence as a verdict. + */ +export function workflowDeclaresColumnModel(ir: WorkflowIr): boolean { + const v2 = ir as WorkflowIrV2 & { nodes?: unknown }; + return Array.isArray(v2.columns) && v2.columns.length > 0 && Array.isArray(v2.nodes); +} diff --git a/packages/dashboard/src/__tests__/routes-task-retry-planning-column.test.ts b/packages/dashboard/src/__tests__/routes-task-retry-planning-column.test.ts new file mode 100644 index 0000000000..bef032a2f9 --- /dev/null +++ b/packages/dashboard/src/__tests__/routes-task-retry-planning-column.test.ts @@ -0,0 +1,362 @@ +// @vitest-environment node +/* +FNXC:ManualRetry 2026-07-29-21:05 (triage census — register-task-workflow-routes.ts): + +## Symptom Verification + +Original symptom: `POST /api/tasks/:id/retry` decided between its DESTRUCTIVE specification branch +(stamp `status: "needs-replan"` AND delete PROMPT.md) and ordinary execution retry using +`!workflowHasColumn(ir, "triage")` as a proxy for "this card plans in place". + +MEASURED across all 12 builtins (see packages/core/src/__tests__/workflow-plans-in-column.test.ts): +NOT ONE workflow plans in `triage`, while SEVEN still declare that column. So the proxy answered the +wrong question in both directions, and the damaging direction is DENIAL: + + builtin:quick-fix, review-heavy, compound-engineering, design, legacy-coding + declare `triage` AND run every plan node in `todo`. `!hasColumn("triage")` is FALSE for them, + so a `planning` / `needs-replan` card sitting in their PLANNING column was refused: + 400 - "Task is not in a retryable state (current status: needs-replan)" + The operator had no button at all on a card that was parked mid-planning. + + A workflow that plans somewhere other than `todo` got the mirror-image fault: a card in `todo` + was handed the spec-DELETING branch even though `todo` hosts no plan node. + +Exact reproduction: a `needs-replan` card in `todo` on `builtin:quick-fix`. Retry it. + +Assertion it is gone: that POST returns 200 and takes the specification branch +(`status: "needs-replan"` re-stamped by the retry path) instead of 400. The mirror-image case and +both already-correct builtins are asserted alongside, so neither direction can regress alone. + +## Surface Enumeration + +- All five builtins that declare `triage` but plan in `todo` — previously 400, must now retry. +- Default workflow (`builtin:coding`, plans in `todo`) — must STILL get specification retry. +- Manual-intake plan-in-place workflow (`builtin:coding-ideas`) — must STILL; FN-8587 path. +- Custom workflow planning outside `todo` — must NOT take the spec-deleting branch... +- ...but must STILL be retryable (routed to execution retry), or the fix strands the card instead. +- Legacy workflow planning in `triage`, card in `todo` — must NOT, unchanged from before. +- A wip-column card must gain no retry path it lacked before (the widening is pre-WIP only). +- Every status that reaches the branch: `failed`, `planning`, `needs-replan`, stuckKillCount>0. +*/ +import { describe, expect, it, vi } from "vitest"; +import express from "express"; +import type { Task, TaskStore } from "@fusion/core"; +import { registerTaskWorkflowRoutes } from "../routes/register-task-workflow-routes.js"; +import { request as performRequest } from "../test-request.js"; +import { ApiError, sendErrorResponse } from "../api-error.js"; + +/** A workflow with NO triage column whose planning happens in its own intake column. */ +const PLANS_IN_INBOX = { + version: "v2", + id: "custom:plans-in-inbox", + name: "Plans In Inbox", + columns: [ + { id: "inbox", traits: [{ trait: "intake" }] }, + { id: "todo", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: "in-progress", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "in-review", traits: [{ trait: "review" }] }, + { id: "done", traits: [{ trait: "complete" }] }, + ], + nodes: [ + { id: "plan", kind: "prompt", column: "inbox", config: {} }, + { id: "plan-review", kind: "prompt", column: "inbox", config: {} }, + ], + edges: [], +}; + +/** Legacy shape: planning runs in `triage`, so a card in `todo` is past specification. */ +const PLANS_IN_TRIAGE = { + ...PLANS_IN_INBOX, + id: "custom:plans-in-triage", + columns: [{ id: "triage", traits: [{ trait: "intake" }] }, ...PLANS_IN_INBOX.columns.slice(1)], + nodes: [{ id: "plan", kind: "prompt", column: "triage", config: {} }], +}; + +function mkTask(overrides: Partial = {}): Task { + return { + id: "FN-9001", + title: "retry planning column", + description: "d", + column: "todo", + status: "failed", + dependencies: [], + createdAt: "2026-07-29T09:00:00.000Z", + updatedAt: "2026-07-29T09:10:00.000Z", + size: "M", + subtasks: [], + log: [], + tags: [], + blockedBy: [], + mergeRetries: 0, + steps: [], + source: { sourceType: "api" }, + ...overrides, + } as unknown as Task; +} + +function buildApp(input: { task: Task; workflowId?: string; definition?: unknown }) { + const updateTask = vi.fn(async () => input.task); + const moveTask = vi.fn(async () => input.task); + const store = { + getTask: async () => input.task, + getTaskDetail: async () => input.task, + updateTask, + moveTask, + logEntry: vi.fn(async () => {}), + getSettings: async () => ({}), + getSettingsFast: async () => ({}), + // A path that cannot exist, so the specification branch's PROMPT.md unlink is a + // no-op (`force: true`) rather than touching any real tree. + getRootDir: () => "/tmp/fusion-retry-planning-column-does-not-exist", + listTasks: async () => [input.task], + getTaskWorkflowSelectionAsync: async () => (input.workflowId ? { workflowId: input.workflowId } : null), + getWorkflowDefinition: async () => input.definition, + getWorkflowSettingsProjectId: () => undefined, + listTaskWorkflowStepResults: async () => [], + getTaskWorkflowStepInstances: async () => [], + deleteTaskWorkflowStepInstances: async () => {}, + } as unknown as TaskStore; + + const runtimeLogger = { warn: vi.fn(), error: vi.fn(), log: vi.fn() }; + const router = express.Router(); + registerTaskWorkflowRoutes({ + router, + store, + options: {}, + runtimeLogger: runtimeLogger as never, + planningLogger: runtimeLogger as never, + chatLogger: runtimeLogger as never, + getProjectIdFromRequest: () => undefined, + getScopedStore: async () => store, + getProjectContext: async () => ({ store, engine: undefined as never, projectId: "p-1" }), + prioritizeProjectsForCurrentDirectory: (projects: unknown) => projects, + emitRemoteRouteDiagnostic: () => {}, + emitAuthSyncAuditLog: () => {}, + parseScopeParam: () => undefined, + resolveAutomationStore: () => ({}) as never, + resolveRoutineStore: () => ({}) as never, + resolveRoutineRunner: () => ({}) as never, + registerDispose: () => {}, + dispose: () => {}, + rethrowAsApiError: (error: unknown): never => { + if (error instanceof ApiError) throw error; + throw new ApiError(500, error instanceof Error ? error.message : "Internal server error"); + }, + } as never, { + runtimeLogger, + upload: { single: () => (_req: unknown, _res: unknown, next: () => void) => next() }, + taskDetailActivityLogLimit: 100, + validateOptionalModelField: (value: unknown) => (typeof value === "string" ? value : undefined), + normalizeModelSelectionPair: (provider: string | null, modelId: string | null) => ({ provider: provider ?? null, modelId: modelId ?? null }), + runGitCommand: async () => "", + isGitRepo: async () => true, + resolveIntegrationBranch: async () => "main", + trimTaskDetailActivityLog: (task: unknown) => task, + triggerCommentWakeForAssignedAgent: async () => {}, + resolveSelfHealingManager: () => undefined, + } as never); + + const app = express(); + app.use(express.json()); + app.use("/api", router); + app.use((error: unknown, _req: express.Request, res: express.Response, _next: express.NextFunction) => { + if (error instanceof ApiError) { + sendErrorResponse(res, error.statusCode, error.message, { details: error.details }); + return; + } + sendErrorResponse(res, 500, error instanceof Error ? error.message : "Internal server error"); + }); + return { app, updateTask, moveTask }; +} + +/** The status patch the route wrote — `needs-replan` means the specification branch was taken. */ +function statusPatchOf(updateTask: ReturnType): unknown { + const call = updateTask.mock.calls.find((c) => c[1] && "status" in (c[1] as object)); + return (call?.[1] as { status?: unknown } | undefined)?.status; +} + +async function retry(app: express.Express) { + return performRequest(app, "POST", "/api/tasks/FN-9001/retry", "{}", { "content-type": "application/json" }); +} + +describe("POST /api/tasks/:id/retry — specification retry follows the plan node's column", () => { + /* + THE DEFECT. Old predicate: `!workflowHasColumn(ir, "triage")` -> true (this workflow has no triage + column), so a card in `todo` was given `needs-replan` and had its PROMPT.md deleted, even though + `todo` hosts no plan node. Reverting to that predicate makes this assertion read "needs-replan". + */ + it("does NOT take the spec-deleting branch for a card parked outside the planning column", async () => { + const { app, updateTask } = buildApp({ + task: mkTask(), + workflowId: "custom:plans-in-inbox", + definition: { ir: PLANS_IN_INBOX }, + }); + + const res = await retry(app); + expect(res.status).toBe(200); + expect(statusPatchOf(updateTask)).toBeNull(); + }); + + it("does NOT take it for a legacy workflow that plans in triage (unchanged)", async () => { + const { app, updateTask } = buildApp({ + task: mkTask(), + workflowId: "custom:plans-in-triage", + definition: { ir: PLANS_IN_TRIAGE }, + }); + + const res = await retry(app); + expect(res.status).toBe(200); + expect(statusPatchOf(updateTask)).toBeNull(); + }); + + /* + The other half of the ratchet: narrowing must not remove specification retry from the cards that + legitimately get it. Both builtins plan in `todo`, so a failed `todo` card there IS a planning + failure. Without these, "return false always" would pass the tests above. + */ + /* + THE HEADLINE DEFECT, on real shipped workflows. These five declare a `triage` column and run every + plan node in `todo`, so the old `!workflowHasColumn(ir, "triage")` was FALSE and a card parked + mid-planning got 400 "not in a retryable state" — no button at all. Reverting to that predicate + turns each of these into a 400. + */ + it.each([ + "builtin:quick-fix", + "builtin:review-heavy", + "builtin:compound-engineering", + "builtin:design", + "builtin:legacy-coding", + ])("retries a needs-replan card in the planning column of %s (was 400)", async (workflowId) => { + const { app, updateTask } = buildApp({ task: mkTask({ status: "needs-replan" }), workflowId }); + + const res = await retry(app); + expect(res.status).toBe(200); + expect(statusPatchOf(updateTask)).toBe("needs-replan"); + }); + + it("STILL takes it for the default workflow, whose plan nodes are in todo", async () => { + const { app, updateTask } = buildApp({ task: mkTask() }); // no selection -> builtin:coding + + const res = await retry(app); + expect(res.status).toBe(200); + expect(statusPatchOf(updateTask)).toBe("needs-replan"); + }); + + it("STILL takes it for the Coding (Ideas) plan-in-place workflow (FN-8587 path)", async () => { + const { app, updateTask } = buildApp({ task: mkTask(), workflowId: "builtin:coding-ideas" }); + + const res = await retry(app); + expect(res.status).toBe(200); + expect(statusPatchOf(updateTask)).toBe("needs-replan"); + }); + + /* + Narrowing the destructive branch must not leave a card with NO button. A `planning` card parked + outside its planning column would otherwise answer 400 "not in a retryable state" — a card nothing + can rescue, which is worse than the spec loss this change removes. It must stay retryable and take + the ordinary execution retry (status cleared, spec preserved). + */ + it("keeps a stranded planning-status card retryable, routed to the non-destructive branch", async () => { + const { app, updateTask } = buildApp({ + task: mkTask({ status: "planning" }), + workflowId: "custom:plans-in-inbox", + definition: { ir: PLANS_IN_INBOX }, + }); + + const res = await retry(app); + expect(res.status).toBe(200); // NOT 400 "not in a retryable state" + expect(statusPatchOf(updateTask)).toBeNull(); // and NOT needs-replan + }); + + it("does not hand a retry path to a status that never had one (in-progress)", async () => { + // The widening above is scoped to pre-WIP columns; an in-progress card with a planning + // status must still be refused exactly as before. + const { app } = buildApp({ + task: mkTask({ column: "in-progress", status: "planning" }), + workflowId: "custom:plans-in-inbox", + definition: { ir: PLANS_IN_INBOX }, + }); + + expect((await retry(app)).status).toBe(400); + }); + + /* + GREPTILE #2621: a v1 IR declares no columns and no nodes, so the placement question is + UNANSWERABLE. Reading that as "past planning" made both flags false and this route answered 400 — + a regression against the old predicate, which admitted the card. It must stay retryable and take + the non-destructive branch so an unanswerable question never costs a specification. + */ + it("keeps a v1 column-less workflow retryable instead of 400ing", async () => { + const { app, updateTask } = buildApp({ + task: mkTask({ status: "needs-replan" }), + workflowId: "custom:v1-no-columns", + definition: { ir: { version: 1, id: "custom:v1-no-columns", name: "V1", steps: [] } }, + }); + + const res = await retry(app); + expect(res.status).toBe(200); // was 400 "not in a retryable state" + expect(statusPatchOf(updateTask)).toBeNull(); // non-destructive: spec preserved + }); + + /* + GREPTILE #2621: a custom workflow whose planning node carries the builtin planning SEAM but a + bespoke id must still be recognised, or Manual Retry preserves a stale spec instead of replanning. + */ + it("takes the specification branch for a bespoke planning node id carrying the planning seam", async () => { + const seamIr = { + version: "v2", + id: "custom:seam-planner", + name: "Seam Planner", + columns: [ + { id: "todo", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] }, + { id: "in-progress", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + ], + nodes: [{ id: "write-the-spec", kind: "prompt", column: "todo", config: { seam: "planning" } }], + edges: [], + }; + const { app, updateTask } = buildApp({ + task: mkTask({ status: "needs-replan" }), + workflowId: "custom:seam-planner", + definition: { ir: seamIr }, + }); + + const res = await retry(app); + expect(res.status).toBe(200); + expect(statusPatchOf(updateTask)).toBe("needs-replan"); + }); + + /* + GREPTILE #2621: the v1 branch must stay PRE-WIP scoped like the v2 one. Admitting every column let a + planning-status card in `in-progress`/`in-review` through, and the generic branch then clears + worktree/branch/retry counters and rebounds it — destroying live execution or review state. + */ + it.each(["in-progress", "in-review"])("refuses a v1 planning-status card parked in %s", async (column) => { + const { app } = buildApp({ + task: mkTask({ column, status: "planning" as never, steps: [] as never }), + workflowId: "custom:v1-no-columns", + definition: { ir: { version: 1, id: "custom:v1-no-columns", name: "V1", steps: [] } }, + }); + + expect((await retry(app)).status).toBe(400); + }); + + it("applies to every status that reaches the branch, not just `failed`", async () => { + for (const overrides of [ + { status: "planning" }, + { status: "needs-replan" }, + { status: null, stuckKillCount: 2 }, + ] as Array>) { + const bad = buildApp({ + task: mkTask(overrides), + workflowId: "custom:plans-in-inbox", + definition: { ir: PLANS_IN_INBOX }, + }); + expect(await retry(bad.app).then((r) => r.status)).toBe(200); + expect(statusPatchOf(bad.updateTask)).toBeNull(); + + const good = buildApp({ task: mkTask(overrides) }); + expect(await retry(good.app).then((r) => r.status)).toBe(200); + expect(statusPatchOf(good.updateTask)).toBe("needs-replan"); + } + }); +}); diff --git a/packages/dashboard/src/routes/register-task-workflow-routes.ts b/packages/dashboard/src/routes/register-task-workflow-routes.ts index 1de53375ec..0355a523ab 100644 --- a/packages/dashboard/src/routes/register-task-workflow-routes.ts +++ b/packages/dashboard/src/routes/register-task-workflow-routes.ts @@ -56,6 +56,9 @@ import { parseExplicitDuplicateMarker, resolveWorkflowIrForTask, workflowHasColumn, + workflowPlansInColumn, + workflowDeclaresColumnModel, + resolveLifecycleColumns, columnHasFlag, columnsWithFlag, resolveReboundTarget, @@ -2628,20 +2631,59 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork 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 - cards in "todo", so the manual Retry button — which the cards already show for - needs-replan/planning/failed states — must offer the planning retry there too instead - of 400ing with "not in a retryable state". Gated on the task's OWN workflow declaring - no "triage" column, so default-workflow todo cards (where todo failures are execution - failures) keep the existing generic-retry semantics. + FNXC:ManualRetry 2026-07-30-02:10 (supersedes the 2026-07-13 gate and #2614's intake resolve): + The question this branch must answer is "does this card sit where its workflow PLANS?", because + the yes-branch is DESTRUCTIVE: it stamps needs-replan AND deletes PROMPT.md. + + Two predicates stood in for it and neither answered it. #2614 resolved the INTAKE column, which + is right for the merged lineage but wrong wherever intake and the planning column differ. The + older arm asked `!workflowHasColumn(ir, "triage")`, and MEASURED across all 12 builtins: NOT ONE + plans in `triage`, while SEVEN still declare that column. So for the five that declare `triage` + AND run every plan node in `todo` — quick-fix, review-heavy, compound-engineering, design, + legacy-coding — the predicate was FALSE and a planning/needs-replan card sitting in its own + planning column was refused outright: + 400 "Task is not in a retryable state (current status: needs-replan)" + The operator had no button at all on a card parked mid-planning. Verified still live on main + after #2614: 9 of this file's 14 retry tests fail without the change below. + + The mirror-image fault is destructive rather than obstructive: a workflow that plans anywhere + other than `todo` had a `todo` card's PROMPT.md deleted for a re-plan nobody asked for. + + Ask the graph directly. `workflowPlansInColumn` recognises planning nodes by the semantic markers + the builtins carry (`config.seam`, an exact `workflowAction` set) with node ids as a backstop. */ - if (!retrySpecification && task.column === "todo" && retrySpecificationStatus) { - const workflowIr = await resolveWorkflowIrForTask(scopedStore, task.id); - retrySpecification = !workflowHasColumn(workflowIr, "triage"); + const workflowIr = await resolveWorkflowIrForTask(scopedStore, task.id); + const retrySpecification = retrySpecificationStatus && workflowPlansInColumn(workflowIr, task.column); + /* + Narrowing the DESTRUCTIVE branch must not narrow RETRYABILITY — those were one boolean and are + two concerns. A planning-status card parked outside its planning column would otherwise fail the + gate below and answer "not in a retryable state", leaving the operator NO button: that trades a + card which loses its spec for a card nothing can rescue. Such a card stays retryable and takes + the ordinary, non-destructive execution retry. + + A v1 IR declares neither columns nor nodes, so the placement question is UNANSWERABLE rather than + answered "no"; treating that silence as "past planning" is what produced a 400 for a v1 planning + card. Scoped to pre-WIP columns otherwise, so no in-progress/in-review status gains a retry path + it did not have. + */ + let strandedSpecificationRetry = false; + if (retrySpecificationStatus && !retrySpecification) { + if (!workflowDeclaresColumnModel(workflowIr)) { + /* + FNXC:ManualRetry 2026-07-30-03:10 (greptile #2621): + Still PRE-WIP ONLY. Admitting every column here was a real regression: a v1 workflow with a + planning/needs-replan status on an `in-progress` or `in-review` card would be admitted, and + the generic branch then clears worktree/branch/retry counters and rebounds the card — losing + live execution or review state that was never in question. A v1 IR yields no roles, so the + legacy pre-implementation ids are the only pre-WIP signal available. + */ + strandedSpecificationRetry = task.column === "triage" || task.column === "todo"; + } else { + const lifecycle = resolveLifecycleColumns(workflowIr); + strandedSpecificationRetry = lifecycle !== undefined + && (task.column === lifecycle.intake || task.column === lifecycle.hold); + } } const isInReviewStatusNone = task.column === "in-review" && (task.status === null || task.status === undefined); @@ -2694,7 +2736,7 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork 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); - if (task.status !== "failed" && task.status !== "stuck-killed" && !retrySpecification && !isInReviewRetry && !isMissingWorktreeSessionRetry) { + 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'})`); }