A rate-limited provider kept getting hammered — the executor and merger lane checks were still literals (#2672)

`taskUsesProvider` resolves a task's **active lane** to decide which
providers it is running on. The **planner** half was converted to traits
— its own note in that function describes this exact failure and says it
was fixed — and the **executor** and **merger** halves were left as
`task.column === "in-progress"` / `=== "in-review"`.

So on a renamed board an actively-executing card resolved **no
providers**, and a provider rate limit never paused it: the engine kept
sending work to the limited provider with that card.

## Measured

Renamed board (`building` = wip, `checking` = review), limit triggered
by a peer:

| lane | before | after |
|---|---|---|
| executor | `["FN-TRIGGER"]` | `["FN-TRIGGER","FN-PEER"]` |
| merger | `["FN-TRIGGER"]` | `["FN-TRIGGER","FN-PEER"]` |

The trigger was paused either way — but only through the
*always-include-the-trigger* fallback, and **that is what made the hole
quiet**: one task always gets paused, so the behaviour looks like it
works.

Resolved from the **same per-workflow IR cache** the planner lane
already uses, so this adds no reads. Both halves fail soft to the legacy
literal, so an unresolvable workflow behaves exactly as before.

## How it was found — the part worth keeping

A scan for *"legacy literal within a few lines of a role-resolved call"*
flagged this file. That is the same heuristic that produced #2670, and
it is now 2 for 3.

The first thing I suspected here was the `done`/`archived` terminal
filter. I wrote that fix, and **its revert stayed green.** That is not a
dead end — chasing *why* it would not go red showed the lane check
already excludes finished cards, so the terminal literal there is
genuinely redundant, and the real defect was one line over in the lane
check itself. **A revert that stays green is information: either the
guard is vacuous, or you are looking at the wrong line.**

I dropped the unprovable change and kept the provable one. The
`done`/`archived` filter is deliberately unchanged, with that reason
recorded in the test.

## Revert proofs, isolated

- wip lane back to the literal → **1 of 55 fails** (the executing peer)
- review lane back to the literal → **1 of 55 fails** (the merger peer)

Three paired cases pass under both, so "pause everything with that
provider" cannot pass for "resolve the lane": a card parked in the
renamed planning lane is **not** paused on an executor limit, a wip card
is **not** paused on a merger limit, and the legacy vocabulary still
pauses peers when no workflow resolves.

## Verification

- 55/55 `usage-limit-detector`; `pnpm test:gate` **71/71**; engine
typecheck clean; `pnpm lint` clean
- census unchanged at 748 — the fail-soft literals remain by design,
which is why the count is not the measure of this 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-30 02:50:30 -07:00
committed by GitHub
parent 89d084e5bd
commit da77e61118
3 changed files with 287 additions and 3 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: A provider rate limit now pauses every task actually running on that provider, including on renamed boards.
category: fix
dev: `taskUsesProvider`'s executor and merger lane checks resolve the workflow's wip/review columns (from the same per-workflow IR cache the planner lane already uses) instead of comparing against `"in-progress"`/`"in-review"`; both fail soft to the legacy literal.

View File

@@ -333,3 +333,217 @@ describe("usage-limit parking and recovery round trip (U11)", () => {
expect(store.pauseTask).not.toHaveBeenCalledWith("FN-106", false); expect(store.pauseTask).not.toHaveBeenCalledWith("FN-106", false);
}); });
}); });
/*
FNXC:WorkflowLifecycleColumns 2026-07-31-22:35:
THE EXECUTOR AND MERGER LANES WERE STILL LITERALS. `taskUsesProvider` resolves a task's active lane to decide
which providers it is running on; the PLANNER lane was converted to traits (the note in that function
describes the exact failure and says it was fixed), and the executor/merger halves were left as
`task.column === "in-progress"` / `=== "in-review"`.
So on a renamed board an actively-executing card resolved NO providers, and a provider rate limit never
paused it — the engine kept hammering the limited provider with that card. Measured before the fix on a
renamed board (`building` = wip), executor limit triggered by a peer: only the TRIGGERING task was paused,
and only because of the always-include-the-trigger fallback. The peer running on the same rate-limited
provider was left going.
HOW THIS WAS FOUND, because the route matters more than the bug: a scan for "legacy literal within a few
lines of a role-resolved call" flagged this file. The FIRST thing I suspected there — the `done`/`archived`
terminal filter — turned out to be a FALSE POSITIVE: its revert stayed green, because the lane check already
excludes finished cards. Chasing why the revert would not go red is what surfaced the real defect one line
over. A revert proof that stays green is information, not a dead end.
*/
describe("a provider rate limit pauses every card actually running on that provider", () => {
const RENAMED_IR = {
version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [],
columns: [
{ id: "queued", name: "Queued", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] },
{ id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
{ id: "checking", name: "Checking", traits: [{ trait: "merge" }, { trait: "merge-blocker" }] },
{ id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] },
],
};
const card = (id: string, column: string) => ({
id, column, dependencies: [], steps: [], currentStep: 0, log: [],
modelProvider: "openai-codex", modelId: "gpt-5",
});
function resolvingStore(tasks: any[], ir: unknown) {
const store = createMockStore(tasks);
const selection = { workflowId: "wf-renamed", stepIds: [] };
store.getTaskWorkflowSelection = () => (ir ? selection : undefined);
store.getTaskWorkflowSelectionAsync = async () => (ir ? selection : undefined);
store.getWorkflowDefinition = async () => (ir ? { id: "wf-renamed", ir } : undefined);
return store;
}
async function pausedIds(store: any, agentType = "executor", trigger = "FN-TRIGGER"): Promise<string[]> {
await new UsageLimitPauser(store).onUsageLimitHit(agentType, trigger, "rate_limit_error: Rate limit exceeded", "openai-codex");
return (store.pauseTask.mock.calls as unknown[][]).map((call) => call[0] as string);
}
it("pauses a PEER executing in the renamed WIP column", async () => {
// Pre-fix: only FN-TRIGGER, via the always-include fallback. FN-PEER kept running on the limited provider.
const store = resolvingStore(
[card("FN-TRIGGER", "building"), card("FN-PEER", "building"), card("FN-SHIPPED", "shipped")],
RENAMED_IR,
);
const paused = await pausedIds(store);
expect(paused).toContain("FN-PEER");
// The terminal lane is still excluded — by the lane check itself, which is why the `done`/`archived`
// filter above it did not need converting.
expect(paused).not.toContain("FN-SHIPPED");
});
it("keeps the LEGACY wip id when a resolvable workflow declares no wip column", async () => {
/*
FNXC:WorkflowLifecycleColumns 2026-07-31-23:55 (PR #2672 review — greptile P1):
THE FALLBACK IS PER ROLE, not per object, and nothing pinned that until now.
A workflow that RESOLVES but declares no `wip` column previously suppressed the
legacy id — because the fallback keyed on whether `activeLanes` existed at all —
so a card in `in-progress` resolved no providers and kept running on the limited
one. A missing ROLE is not the same fact as a missing WORKFLOW; only the second
means "no basis to judge".
Found by mutation: reverting to the per-object form left all 57 existing tests
green, so the fix greptile asked for was unproven.
*/
const NO_WIP_IR = {
version: "v2", id: "wf-nowip", name: "no wip", nodes: [], edges: [],
columns: [
{ id: "queued", name: "Queued", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] },
{ id: "checking", name: "Checking", traits: [{ trait: "merge" }, { trait: "merge-blocker" }] },
{ id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] },
],
};
const store = resolvingStore(
[card("FN-TRIGGER", "in-progress"), card("FN-LEGACY-WIP", "in-progress")],
NO_WIP_IR,
);
const paused = await pausedIds(store);
expect(paused).toContain("FN-LEGACY-WIP");
});
it("pauses a merger-lane peer in the renamed REVIEW column", async () => {
const store = resolvingStore(
[card("FN-TRIGGER", "checking"), card("FN-PEER", "checking"), card("FN-WIP", "building")],
RENAMED_IR,
);
const paused = await pausedIds(store, "merger");
expect(paused).toContain("FN-PEER");
// A wip card is not on the merger lane, so a merger-provider limit must leave it alone.
expect(paused).not.toContain("FN-WIP");
});
it("does NOT pause a card parked in the renamed planning lane on an executor limit", async () => {
// The paired negative: "pause everything with that provider" must not pass for "resolve the lane".
const store = resolvingStore([card("FN-TRIGGER", "building"), card("FN-PARKED", "queued")], RENAMED_IR);
expect(await pausedIds(store)).not.toContain("FN-PARKED");
});
it("keeps the legacy literals when no workflow resolves", async () => {
const store = resolvingStore([card("FN-TRIGGER", "in-progress"), card("FN-PEER", "in-progress")], undefined);
expect(await pausedIds(store)).toContain("FN-PEER");
});
});
/*
FNXC:WorkflowLifecycleColumns 2026-07-31-23:35 (PR #2672 review, greptile P1 + P2):
TWO PROPERTIES MY FIRST VERSION GOT WRONG.
1. THE FALLBACK IS PER ROLE, NOT PER OBJECT. I keyed it on whether `activeLanes` existed at all, so a
workflow that RESOLVES but declares no wip (or no review) column suppressed the legacy id and resolved no
providers — reintroducing the exact bug this change fixes, for the partial-vocabulary case. A missing ROLE
is not the same fact as a missing WORKFLOW; only the second means "no basis".
2. RESOLVE ONLY THE CANDIDATES. Resolving every task before filtering made a rate limit on a 400-card board
pay 400 resolutions to pause a handful, including terminal and paused cards that can never be affected.
*/
describe("lane resolution degrades per role and costs only what it must", () => {
/** Declares a wip lane but NO review lane — the partial vocabulary that broke the first version. */
const NO_REVIEW_IR = {
version: "v2", id: "wf-partial", name: "partial", nodes: [], edges: [],
columns: [
{ id: "queued", name: "Queued", traits: [{ trait: "intake" }, { trait: "hold", config: { release: "capacity" } }] },
{ id: "in-progress", name: "In progress", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
],
};
const card = (id: string, column: string) => ({
id, column, dependencies: [], steps: [], currentStep: 0, log: [],
modelProvider: "openai-codex", modelId: "gpt-5",
});
function storeFor(tasks: any[], ir: unknown, workflowId = "wf-partial") {
const store = createMockStore(tasks);
const selection = { workflowId, stepIds: [] };
const reads = { definition: 0 };
/*
Records WHICH task ids were asked about, not how many reads happened. A read count is
concurrency-dependent here — the shared IR cache fills after an await, so same-workflow tasks race and the
number lands anywhere between 1 and N. Asking "was this task resolved at all?" is exact and race-free,
which is what a filtering assertion actually needs.
*/
const asked: string[] = [];
store.getTaskWorkflowSelectionAsync = async (id: string) => {
asked.push(id);
return selection;
};
store.getTaskWorkflowSelection = (id: string) => {
asked.push(id);
return selection;
};
store.getWorkflowDefinition = async () => {
reads.definition += 1;
return ir ? { id: workflowId, ir } : undefined;
};
(store as any).reads = reads;
(store as any).asked = asked;
return store;
}
it("keeps the legacy REVIEW id when the workflow declares a wip lane but no review lane", async () => {
// Pre-fix: `activeLanes` was truthy, `review` undefined, so an `in-review` card resolved no providers and
// kept running on the rate-limited provider.
const store = storeFor([card("FN-TRIGGER", "in-review"), card("FN-PEER", "in-review")], NO_REVIEW_IR);
await new UsageLimitPauser(store).onUsageLimitHit("merger", "FN-TRIGGER", "rate_limit_error: Rate limit exceeded", "openai-codex");
expect((store.pauseTask.mock.calls as unknown[][]).map((c) => c[0])).toContain("FN-PEER");
});
it("does not resolve a workflow for terminal or paused tasks", async () => {
// Only FN-LIVE is a plausible candidate; the other three can never be affected, so paying a resolution
// for them is cost with no possible outcome.
const store = storeFor(
[
card("FN-LIVE", "in-progress"),
card("FN-DONE", "done"),
card("FN-ARCHIVED", "archived"),
{ ...card("FN-PAUSED", "in-progress"), paused: true },
],
NO_REVIEW_IR,
);
await new UsageLimitPauser(store).onUsageLimitHit("executor", "FN-LIVE", "rate_limit_error: Rate limit exceeded", "openai-codex");
// Exact, not a count: the three tasks that can never be affected are never resolved at all.
const asked = (store as any).asked as string[];
expect(asked).toContain("FN-LIVE");
expect(asked).not.toContain("FN-DONE");
expect(asked).not.toContain("FN-ARCHIVED");
expect(asked).not.toContain("FN-PAUSED");
});
});

View File

@@ -125,6 +125,12 @@ export class UsageLimitPauser {
settings: Awaited<ReturnType<TaskStore["getSettings"]>>, settings: Awaited<ReturnType<TaskStore["getSettings"]>>,
agentType: string, agentType: string,
preImplementationColumns?: ReadonlySet<string>, preImplementationColumns?: ReadonlySet<string>,
/*
FNXC:WorkflowLifecycleColumns 2026-07-31-22:10:
The WIP and REVIEW lanes of this task's own workflow, resolved by the caller from the same per-workflow
IR cache as `preImplementationColumns`. Optional so an unresolvable workflow keeps the legacy literals.
*/
activeLanes?: { wip?: string; review?: string },
): boolean { ): boolean {
/* /*
FNXC:WorkflowLifecycleColumns 2026-07-29-15:20 (P0 audit after the Planning-column merge): FNXC:WorkflowLifecycleColumns 2026-07-29-15:20 (P0 audit after the Planning-column merge):
@@ -149,12 +155,36 @@ export class UsageLimitPauser {
resolveValidatorSessionModel(task.validatorModelProvider, task.validatorModelId, settings).provider, resolveValidatorSessionModel(task.validatorModelProvider, task.validatorModelId, settings).provider,
] : []) ] : [])
: agentType === "executor" : agentType === "executor"
? (task.column === "in-progress" ? [ /*
FNXC:WorkflowLifecycleColumns 2026-07-31-22:15:
THE EXECUTOR AND MERGER LANES ARE RESOLVED TOO. The note above describes exactly this failure for the
PLANNER lane and says it was fixed there — these two were left as literals, so the same silent hole
stayed open one lane over: on a renamed board an actively-executing card in the WIP column resolved NO
providers, so a provider rate limit never paused it and the engine kept hammering the limited provider
with that card.
MEASURED before the fix, renamed board (`building` = wip), executor limit on a peer card: only the
TRIGGERING task was paused, and that only because of the always-include-the-trigger fallback below.
The peer executing on the same rate-limited provider was left running.
FNXC:WorkflowLifecycleColumns 2026-07-31-23:10 (PR #2672 review, greptile P1):
THE FALLBACK IS PER ROLE, not per object. My first version keyed it on whether `activeLanes` existed
at all, so a workflow that RESOLVES but declares no wip (or no review) column suppressed the legacy
id and resolved no providers — reintroducing this very bug for the partial-vocabulary case. A missing
ROLE is not the same fact as a missing WORKFLOW, and only the second one means "no basis".
Falling back per role can only ADD pauses, for a card sitting in a legacy-named column on a board that
declares no such role — which is the safe direction here and the one the note above states: the cost
of a wrong include is pausing work that was fine.
*/
? (((activeLanes?.wip ?? "in-progress") === task.column) ? [
resolveExecutorSessionModel(task.modelProvider, task.modelId, settings).provider, resolveExecutorSessionModel(task.modelProvider, task.modelId, settings).provider,
resolveValidatorSessionModel(task.validatorModelProvider, task.validatorModelId, settings).provider, resolveValidatorSessionModel(task.validatorModelProvider, task.validatorModelId, settings).provider,
] : []) ] : [])
: agentType === "merger" : agentType === "merger"
? (task.column === "in-review" ? [resolveMergerSessionModel(settings, undefined, task).provider] : []) ? (((activeLanes?.review ?? "in-review") === task.column)
? [resolveMergerSessionModel(settings, undefined, task).provider]
: [])
: []; : [];
const resolvedProviders = providersByActiveLane; const resolvedProviders = providersByActiveLane;
return resolvedProviders.some((candidate) => candidate?.trim().toLowerCase() === provider); return resolvedProviders.some((candidate) => candidate?.trim().toLowerCase() === provider);
@@ -206,6 +236,32 @@ export class UsageLimitPauser {
*/ */
const irCache = new Map<string, WorkflowIr>(); const irCache = new Map<string, WorkflowIr>();
const preImplementationByTask = new Map<string, ReadonlySet<string>>(); const preImplementationByTask = new Map<string, ReadonlySet<string>>();
/*
FNXC:WorkflowLifecycleColumns 2026-07-31-22:20:
Resolved for EVERY agent type, because the executor and merger lanes need it and only the
pre-implementation lane is planner-only. Shares `irCache`, so this is still one IR read per WORKFLOW
across the whole fan-out — a task whose workflow cannot be resolved yields `undefined` and the callee
keeps the legacy literal for it.
*/
const activeLanesByTask = new Map<string, { wip?: string; review?: string } | undefined>();
/*
FNXC:WorkflowLifecycleColumns 2026-07-31-23:20 (PR #2672 review, greptile P2):
RESOLVE ONLY THE CANDIDATES. The first version resolved every task on the board before filtering, so a
rate limit on a 400-card board paid 400 resolutions to pause a handful — and terminal or paused cards,
which can never be affected, were resolved too. The cheap predicates run first now, so the resolution
cost tracks the number of plausible tasks rather than the size of the board.
KNOWN AND UNCHANGED: concurrent misses on the same workflow can still both read, because `irCache` fills
after the await. That is a property this path shares with the planner-lane resolution above it, and
serialising to fix it would trade a bounded duplicate read for latency on the pause — which is the
operation an operator is waiting on. Named rather than silently accepted.
*/
const laneCandidates = tasks.filter((task) =>
task.paused !== true && task.column !== "done" && task.column !== "archived");
await Promise.all(laneCandidates.map(async (task) => {
const columns = await resolveTaskLifecycleColumns(this.store, task.id, irCache).catch(() => undefined);
activeLanesByTask.set(task.id, columns ? { wip: columns.wip, review: columns.review } : undefined);
}));
if (agentType === PLANNER_AGENT_ROLE) { if (agentType === PLANNER_AGENT_ROLE) {
await Promise.all(tasks.map(async (task) => { await Promise.all(tasks.map(async (task) => {
const columns = await resolveTaskLifecycleColumns(this.store, task.id, irCache).catch(() => undefined); const columns = await resolveTaskLifecycleColumns(this.store, task.id, irCache).catch(() => undefined);
@@ -232,7 +288,14 @@ export class UsageLimitPauser {
&& task.column !== "archived" && task.column !== "archived"
&& task.paused !== true && task.paused !== true
&& providerId !== "unknown" && providerId !== "unknown"
&& this.taskUsesProvider(task, providerId, settings, agentType, preImplementationByTask.get(task.id))); && this.taskUsesProvider(
task,
providerId,
settings,
agentType,
preImplementationByTask.get(task.id),
activeLanesByTask.get(task.id),
));
// Always include the task that produced the 429 even if its actual provider // Always include the task that produced the 429 even if its actual provider
// came from a runtime fallback not represented in persisted task settings. // came from a runtime fallback not represented in persisted task settings.