P0: two silent triage guards in the executor's ownership — one strands a card with nothing to rescue it (#2572)

P0 audit of the executor's assigned `triage` sites after the
Planning-column merge. **One of them can strand a card**, so leading
with that.

## The stall — `handleDepAbortCleanup`

`executor.ts` moved a dependency-aborted task to the **literal**
`triage`. The default coding lineage no longer declares that column.

A card that gains a dependency mid-execution has its work discarded and
is then parked in a column its own workflow does not define. Nothing in
the graph routes a card out of an undeclared column. The only rescue is
`reconcileUndeclaredTaskColumns`, which runs on the **next engine
start** — so between the abort and a restart the card is stalled with no
automatic recovery. It does not throw, so it would have surfaced as a
user report, not a red test.

Fixed to `resolveReboundColumnFor`, the helper the other ~16 executor
rebounds already use.

## The silent skip — `UsageLimitPauser.taskUsesProvider`

The planning lane was identified by the same literal. For a default card
the lane resolved to **no providers**, so when a provider hit a usage
limit during a *planning* session, the fan-out that pauses peers on that
provider skipped every default-workflow card and they kept hammering the
rate-limited provider.

Not a stall: the triggering task is still paused by the explicit
fallback below the filter. What was lost is blast-radius containment. A
planning session runs while the card is pre-implementation, and 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.

## Full audit table for my assigned sites

| Site | (a) Still fires for a default card? | (b) What silently stops |
(c) Action |
|---|---|---|---|
| `executor.ts:16395` `moveTask(id, "triage")` | **No** — writes an
undeclared column | Card parked where nothing routes it; rescue only at
next engine start | **Fixed** — `resolveReboundColumnFor` |
| `usage-limit-detector.ts:126` `column === "triage"` | **No** |
Usage-limit fan-out skips every default card; peers keep hitting the
limited provider | **Fixed** — pre-implementation predicate |
| `executor.ts:3409` `from === "todo" \|\| from === "triage"` | **Yes**,
via the `todo` arm | — | Unchanged; `triage` arm still live for
legacy-coding |
| `executor.ts:4951` `originColumn === "todo" \|\| === "triage"` |
**Yes**, via the `todo` arm | — | Unchanged |
| `executor.ts:4963` `originColumn === "triage"` double-hop | No, and
correctly so | Nothing — the extra hop exists only for shapes that
declare `triage` | Unchanged; still required by legacy-coding |
| `executor.ts:1110` `Type.Literal("triage")` | n/a | — | **Not a
column** — an agent ROLE in `spawnAgentParams` |

Counts for my ownership: **6 sites audited, 2 defects, 2 fixed, 3
correct as-is, 1 false positive.**

## Red-green

Reverting each fix fails its own test:

```
Tests  2 failed | 2 passed (4)
  × dependency-abort cleanup requeues to a DECLARED column
  × usage-limit fan-out … pauses a peer card sitting in the merged Planning column (id `todo`)
```

The other two are the regression floor and pass both ways by design: a
legacy workflow that **does** declare `triage` still fans out, and an
in-progress card is still **not** swept into the planning lane (the
guard must stay narrow — "any non-wip column" would have been the easy
wrong fix).

## Verification

- New audit suite + graph-boundary + step-session + ownership ledger —
**45 tests green**
- `pnpm test:gate` green (10 / 414 / 71); `pnpm lint` clean; `tsc
--noEmit` clean
- Changeset included (`patch`, `fix`)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-29 19:03:10 -07:00
committed by GitHub
parent ad3dc202f8
commit a68785a41d
4 changed files with 243 additions and 4 deletions

View File

@@ -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.

View File

@@ -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());
});
});

View File

@@ -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");
}
/**

View File

@@ -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<ReturnType<TaskStore["getSettings"]>>,
agentType: string,
preImplementationColumns?: ReadonlySet<string>,
): 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<string, WorkflowIr>();
const preImplementationByTask = new Map<string, ReadonlySet<string>>();
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<string>();
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.