diff --git a/.changeset/u8-triage-column-audit.md b/.changeset/u8-triage-column-audit.md new file mode 100644 index 0000000000..144601210a --- /dev/null +++ b/.changeset/u8-triage-column-audit.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix a card parked in a non-existent column after a dependency abort, and restore usage-limit pausing while planning. +category: fix +dev: P0 audit after the Planning-column merge removed `triage` from the default lineage. `handleDepAbortCleanup` wrote the literal `triage` (undeclared column; only a restart-time reconcile could rescue the card) and now uses `resolveReboundColumnFor`. `UsageLimitPauser.taskUsesProvider` identified the planning lane by `column === "triage"`, which stopped matching for default cards, so the provider fan-out skipped them; it now matches any pre-implementation column. diff --git a/packages/engine/src/__tests__/executor-triage-column-audit.test.ts b/packages/engine/src/__tests__/executor-triage-column-audit.test.ts new file mode 100644 index 0000000000..37417facac --- /dev/null +++ b/packages/engine/src/__tests__/executor-triage-column-audit.test.ts @@ -0,0 +1,164 @@ +/* +FNXC:WorkflowLifecycleColumns 2026-07-29-15:30 (P0 audit after the Planning-column merge): + +The default coding lineage no longer declares a `triage` column — it has ONE pre-implementation +column, id `todo`. `triage` remains a legal column id (legacy coding, the Task enum), so nothing +throws; every `column === "triage"` comparison simply stops matching for default-workflow cards. +Silent non-firing guards are the failure class this program has now found seven times. + +These pin the two sites in the executor's ownership that genuinely misbehaved. Both were observed +FAILING against the pre-fix code. +*/ +import { describe, expect, it, vi } from "vitest"; +import type { Task, TaskDetail, WorkflowIr } from "@fusion/core"; +import "./executor-test-helpers.js"; +import { TaskExecutor } from "../executor.js"; +import { createMockStore, resetExecutorMocks } from "./executor-test-helpers.js"; +import { UsageLimitPauser } from "../usage-limit-detector.js"; + +const WF = "custom:planning-only"; + +/** The post-merge default shape: ONE pre-implementation column, no `triage`. */ +function planningOnlyIr(): WorkflowIr { + return { + version: "v2", + id: WF, + nodes: [], + edges: [], + columns: [ + { id: "todo", label: "Planning", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] }, + { id: "in-progress", label: "In progress", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "in-review", label: "In review", traits: [{ trait: "mergeOrchestration" }] }, + { id: "done", label: "Done", traits: [{ trait: "complete" }] }, + ], + } as unknown as WorkflowIr; +} + +describe("dependency-abort cleanup requeues to a DECLARED column", () => { + it("uses the workflow's own planner column, never the literal triage", async () => { + resetExecutorMocks(); + const store = createMockStore(); + const selection = { workflowId: WF, stepIds: [] }; + store.getTask.mockResolvedValue({ id: "FN-DEP", column: "in-progress", branch: null } as TaskDetail); + store.getTaskWorkflowSelection = vi.fn(() => selection); + store.getTaskWorkflowSelectionAsync = vi.fn(async () => selection); + store.getWorkflowDefinition = vi.fn(async () => ({ id: WF, ir: planningOnlyIr() })); + const executor = new TaskExecutor(store, "/tmp/test"); + + await (executor as any).handleDepAbortCleanup("FN-DEP", "/tmp/test/wt"); + + /* + The failure this pins: the card was parked in a column its workflow does not declare, where + nothing routes it onward and only a restart-time reconcile could rescue it. + */ + expect(store.moveTask).toHaveBeenCalledWith("FN-DEP", "todo"); + expect(store.moveTask).not.toHaveBeenCalledWith("FN-DEP", "triage"); + }); +}); + +describe("usage-limit fan-out still recognises the planning lane", () => { + function detectorHarness(tasks: Task[], ir: WorkflowIr = planningOnlyIr()) { + const store = createMockStore(); + const selection = { workflowId: WF, stepIds: [] }; + store.getSettings.mockResolvedValue({ maxConcurrent: 2, maxWorktrees: 4, pollIntervalMs: 15_000 }); + store.listTasks = vi.fn().mockResolvedValue(tasks); + store.getTask.mockResolvedValue(tasks[0]); + store.pauseTask = vi.fn().mockResolvedValue(undefined); + store.getTaskWorkflowSelection = vi.fn(() => selection); + store.getTaskWorkflowSelectionAsync = vi.fn(async () => selection); + store.getWorkflowDefinition = vi.fn(async () => ({ id: WF, ir })); + return { store, detector: new UsageLimitPauser(store as never) }; + } + + /** A workflow with a SECOND processing lane — not a planning column, despite not being wip. */ + function twoLaneIr(): WorkflowIr { + return { + version: "v2", + id: WF, + nodes: [], + edges: [], + columns: [ + { id: "todo", label: "Planning", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] }, + { id: "in-progress", label: "Build", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "qa", label: "QA", traits: [] }, + { id: "done", label: "Done", traits: [{ trait: "complete" }] }, + ], + } as unknown as WorkflowIr; + } + + const planning = (id: string, column: string): Task => + ({ id, column, paused: false, planningModelProvider: "anthropic", planningModelId: "m" }) as Task; + + it("pauses a peer card sitting in the merged Planning column (id `todo`)", async () => { + /* + Before the fix the lane resolved to no providers for a `todo` card, so the peer kept running + against the rate-limited provider. Nothing failed — the containment just stopped happening. + */ + const peer = planning("FN-PEER", "todo"); + const { store, detector } = detectorHarness([planning("FN-TRIGGER", "todo"), peer]); + + await detector.onUsageLimitHit("triage", "FN-TRIGGER", "429 usage limit reached", "anthropic"); + + expect(store.pauseTask).toHaveBeenCalledWith("FN-PEER", true, undefined, expect.anything()); + }); + + it("still pauses a peer in a workflow that DOES declare triage (legacy coding)", async () => { + const peer = planning("FN-PEER", "triage"); + const legacyIr = { version: "v2", id: WF, nodes: [], edges: [], columns: [ + { id: "triage", label: "Triage", traits: [{ trait: "intake" }] }, + { id: "todo", label: "Todo", traits: [{ trait: "hold", config: { release: "capacity" } }] }, + { id: "in-progress", label: "In progress", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "done", label: "Done", traits: [{ trait: "complete" }] }, + ] } as unknown as WorkflowIr; + const { store, detector } = detectorHarness([planning("FN-TRIGGER", "triage"), peer], legacyIr); + + await detector.onUsageLimitHit("triage", "FN-TRIGGER", "429 usage limit reached", "anthropic"); + + expect(store.pauseTask).toHaveBeenCalledWith("FN-PEER", true, undefined, expect.anything()); + }); + + it("does NOT treat a custom non-terminal column as a planning lane", async () => { + /* + FNXC PR #2572 review (greptile): the previous predicate was "not in-progress and not + in-review", which reads ANY custom column — a second processing lane, a manual hold, a + bespoke review stage — as pre-implementation. A planning-provider limit would then pause + cards nowhere near planning. `qa` carries no lifecycle trait, so it is not the planning lane. + */ + const peer = planning("FN-PEER", "qa"); + const { store, detector } = detectorHarness([planning("FN-TRIGGER", "todo"), peer], twoLaneIr()); + + await detector.onUsageLimitHit("triage", "FN-TRIGGER", "429 usage limit reached", "anthropic"); + + expect(store.pauseTask).not.toHaveBeenCalledWith("FN-PEER", true, undefined, expect.anything()); + }); + + it("does NOT treat a MID-PIPELINE hold column as a planning lane", async () => { + /* + FNXC PR #2572 review (greptile, 2nd): `hold` is not a synonym for planning. A workflow may + carry the trait on a manual/timed/dependency wait that sits AFTER implementation; a card + parked there is not queued for planning and must not be paused by a planning-provider limit. + */ + const midPipelineHoldIr = { version: "v2", id: WF, nodes: [], edges: [], columns: [ + { id: "todo", label: "Planning", traits: [{ trait: "intake" }] }, + { id: "in-progress", label: "Build", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }, + { id: "awaiting-signoff", label: "Awaiting sign-off", traits: [{ trait: "hold", config: { release: "manual" } }] }, + { id: "done", label: "Done", traits: [{ trait: "complete" }] }, + ] } as unknown as WorkflowIr; + const peer = planning("FN-PEER", "awaiting-signoff"); + const { store, detector } = detectorHarness([planning("FN-TRIGGER", "todo"), peer], midPipelineHoldIr); + + await detector.onUsageLimitHit("triage", "FN-TRIGGER", "429 usage limit reached", "anthropic"); + + expect(store.pauseTask).not.toHaveBeenCalledWith("FN-PEER", true, undefined, expect.anything()); + }); + + it("does NOT sweep a card that is mid-implementation into the planning lane", async () => { + /* The guard must stay narrow: an in-progress card is the executor lane's business. */ + const peer = planning("FN-PEER", "in-progress"); + const { store, detector } = detectorHarness([planning("FN-TRIGGER", "todo"), peer]); + + await detector.onUsageLimitHit("triage", "FN-TRIGGER", "429 usage limit reached", "anthropic"); + + expect(store.pauseTask).not.toHaveBeenCalledWith("FN-PEER", true, undefined, expect.anything()); + }); +}); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 967c44e4c4..09f2bef8a4 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -16424,8 +16424,21 @@ export class TaskExecutor { // Update task: clear worktree and status, move to triage await this.store.updateTask(taskId, { worktree: null, status: null }); - await this.store.moveTask(taskId, "triage"); - await this.store.logEntry(taskId, "Execution stopped — work discarded, moved to triage for re-planning"); + /* + FNXC:WorkflowLifecycleColumns 2026-07-29-15:10 (P0 audit after the Planning-column merge): + This wrote the LITERAL `triage`. The default coding lineage no longer declares that column — + it has one pre-implementation column, id `todo` — so a card that gained a dependency + mid-execution had its work discarded and was then parked in a column its own workflow does + not define. Nothing in the graph routes a card out of an undeclared column, and the only + rescue is `reconcileUndeclaredTaskColumns` on the NEXT ENGINE START, so between the abort and + a restart the card is stalled with no automatic recovery. It does not throw, which is why it + would have surfaced as a user report rather than a red test. + + Resolve the rebound target from the task's own workflow (hold -> intake -> first declared + column), the same helper the other ~16 executor rebounds already use. + */ + await this.store.moveTask(taskId, await resolveReboundColumnFor(this.store, taskId)); + await this.store.logEntry(taskId, "Execution stopped — work discarded, requeued for re-planning"); } /** diff --git a/packages/engine/src/usage-limit-detector.ts b/packages/engine/src/usage-limit-detector.ts index 0f886e5a85..ac34379430 100644 --- a/packages/engine/src/usage-limit-detector.ts +++ b/packages/engine/src/usage-limit-detector.ts @@ -11,6 +11,7 @@ */ import type { Task, TaskStore } from "@fusion/core"; +import { resolveTaskLifecycleColumns, type WorkflowIr } from "@fusion/core"; import { resolveExecutorSessionModel, resolveMergerSessionModel, @@ -121,9 +122,27 @@ export class UsageLimitPauser { provider: string, settings: Awaited>, agentType: string, + preImplementationColumns?: ReadonlySet, ): boolean { + /* + FNXC:WorkflowLifecycleColumns 2026-07-29-15:20 (P0 audit after the Planning-column merge): + The planning lane was identified by the LITERAL `triage`. The default coding lineage no + longer declares that column, so this comparison stopped matching for every default-workflow + card — silently. Nothing throws; the lane simply resolves to no providers, so when a provider + hits a usage limit during a PLANNING session the fan-out that pauses other tasks on that same + provider skips every default card, and they keep hammering the rate-limited provider. The + triggering task is still paused by the explicit fallback below, so no card is stranded — what + is lost is the blast-radius containment. + + A planning session runs while the card is PRE-IMPLEMENTATION. The caller has already excluded + `done`/`archived`, so that is exactly "not the implementation column and not the review + column" — which matches `todo`, `triage`, `ideas`, and a renamed planner alike. The two + literals that remain here name the wip and review lanes and belong to the executor/scheduler + vocabulary conversion, not to this fix. + */ + const isPreImplementation = preImplementationColumns?.has(task.column) === true; const providersByActiveLane = agentType === "triage" - ? (task.column === "triage" ? [ + ? (isPreImplementation ? [ resolvePlanningSessionModel(task.planningModelProvider, task.planningModelId, settings).provider, resolveValidatorSessionModel(task.validatorModelProvider, task.validatorModelId, settings).provider, ] : []) @@ -170,12 +189,48 @@ export class UsageLimitPauser { // FNXC:ArchitectureHotPath 2026-07-22-17:20: slim payload — this scan only reads column/pause/model-provider scalars, never heavy detail fields. this.store.listTasks({ slim: true }), ]); + /* + FNXC:WorkflowLifecycleColumns 2026-07-29-20:50 (P0 audit, PR #2572 review — greptile): + The planning lane is resolved PER TASK from its own workflow, not inferred by excluding two + literals. "Not `in-progress` and not `in-review`" reads any custom non-terminal column — a + second processing lane, a manual hold, a bespoke review stage — as pre-implementation, so a + planning-provider limit would pause cards that are nowhere near planning. Trait-derived + intake/hold is the only answer that holds for a workflow this code has never seen. + + One IR read per WORKFLOW, not per task: the cache is caller-owned (the U1 contract) and shared + across the whole fan-out, so a 400-card board spanning three workflows reads three IRs. A task + whose workflow cannot be resolved yields an empty set and is skipped rather than guessed into + the lane — conservative, because the cost of a wrong include is pausing work that was fine. + */ + const irCache = new Map(); + const preImplementationByTask = new Map>(); + if (agentType === "triage") { + await Promise.all(tasks.map(async (task) => { + const columns = await resolveTaskLifecycleColumns(this.store, task.id, irCache).catch(() => undefined); + /* + FNXC:WorkflowLifecycleColumns 2026-07-29-22:10 (PR #2572 review — greptile, 2nd): + INTAKE ONLY. `hold` is not a synonym for "planning": a workflow may carry a hold trait on + a MID-PIPELINE wait — manual release, timed, dependency, external event — and a card + parked there is downstream of implementation, not queued for planning. Including hold + would pause it on a planning-provider limit, which is the same over-classification as the + literal-exclusion predicate this replaced, just further along. + + The planning session is the one that runs on an intake card, so intake is the lane. When a + workflow's hold column IS its pre-implementation queue it is normally the same column as + intake (the merged Planning lane declares both traits) and is covered by that; where they + differ, the hold column is a wait and is deliberately excluded. + */ + const lanes = new Set(); + if (columns?.intake) lanes.add(columns.intake); + preImplementationByTask.set(task.id, lanes); + })); + } const affectedTasks = tasks.filter((task) => task.column !== "done" && task.column !== "archived" && task.paused !== true && providerId !== "unknown" - && this.taskUsesProvider(task, providerId, settings, agentType)); + && this.taskUsesProvider(task, providerId, settings, agentType, preImplementationByTask.get(task.id))); // Always include the task that produced the 429 even if its actual provider // came from a runtime fallback not represented in persisted task settings.