From e573178e31847b64ed4cf9d29d86b9cba8b07c98 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 9 Aug 2026 18:21:55 -0700 Subject: [PATCH] fix(agents): never let a built-in workflow role agent be unroutable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The board stopped moving. Work items churned held -> running -> held at ~3.5/sec across every task, pinning a core and writing ~19k workflowWorkItem audit rows/hour while nothing executed. Hold reason: workflow-principal-role-pool-exhausted:executor. provisionBuiltinWorkflowRoleAgents seeded the four permanent owners (triage, executor, reviewer, merger) with runtimeConfig.enabled=false, while the router's available() treats enabled===false as unavailable. The only permanent principals for every built-in role were unroutable BY CONSTRUCTION — shipped that way, so any instance without operator-created role agents deadlocks at its first workflow node. Nothing self-recovers: a pool only changes by operator action. Routability of these four is an invariant, not a setting. Unlike an operator's agent, disabling one does not opt an agent out — it removes the only thing that can run that stage, and there is no fallback. - seed built-ins enabled; converge existing rows on provisioning - enforceBuiltinWorkflowRoleRoutability coerces enabled back at the durable writeAgent seam, so no REST/UI/plugin/restore path can reintroduce the deadlock. Other runtimeConfig keys are preserved; operator-owned agents keep their off switch - share the static routability predicate (isWorkflowPrincipalEligible) between provisioning and the router so the two cannot drift apart again Also fix the spin itself: a principal hold had no cooldown, so the scheduler re-dispatched instantly and the run re-entered only to re-fence and re-park. It now records a backoff ladder (15s -> 5m) checked before graph entry, and logs once per distinct reason instead of every pass — the same self-recovering shape as holdForSessionContention. The hold never increments `attempt`, so no existing guard could ever fire. Co-Authored-By: Claude Opus 5 --- ...in-workflow-role-agents-always-routable.md | 7 ++ .../builtin-workflow-role-routability.test.ts | 83 +++++++++++++++++++ packages/core/src/agents/agent-role-policy.ts | 48 +++++++++++ packages/core/src/agents/agent-store.ts | 26 +++++- packages/core/src/index.ts | 3 + .../src/agents/workflow-agent-router.ts | 12 ++- packages/engine/src/executor.ts | 81 ++++++++++++++++-- 7 files changed, 246 insertions(+), 14 deletions(-) create mode 100644 .changeset/builtin-workflow-role-agents-always-routable.md create mode 100644 packages/core/src/agents/__tests__/builtin-workflow-role-routability.test.ts diff --git a/.changeset/builtin-workflow-role-agents-always-routable.md b/.changeset/builtin-workflow-role-agents-always-routable.md new file mode 100644 index 0000000000..b16c87135e --- /dev/null +++ b/.changeset/builtin-workflow-role-agents-always-routable.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix a deadlock where built-in workflow agents were unroutable, leaving every task stuck and spinning. +category: fix +dev: `provisionBuiltinWorkflowRoleAgents` seeded the four permanent owners with `runtimeConfig.enabled: false` while the router's `available()` rejects `enabled === false`, so no built-in role could ever be routed. Built-ins are now seeded enabled, existing rows converge on provisioning, and `enforceBuiltinWorkflowRoleRoutability` coerces them back at the durable `writeAgent` seam so no API/UI/plugin path can disable them. The static routability predicate (`isWorkflowPrincipalEligible`) is shared by provisioning and the router so they cannot drift. Separately, a workflow-principal hold now uses a backoff ladder (`PRINCIPAL_HOLD_BACKOFF_MS`, 15s→5m) checked before graph entry, instead of re-dispatching immediately — the old path spun ~3.5×/sec writing ~19k audit rows/hour with nothing executing. diff --git a/packages/core/src/agents/__tests__/builtin-workflow-role-routability.test.ts b/packages/core/src/agents/__tests__/builtin-workflow-role-routability.test.ts new file mode 100644 index 0000000000..6dcacc4dcb --- /dev/null +++ b/packages/core/src/agents/__tests__/builtin-workflow-role-routability.test.ts @@ -0,0 +1,83 @@ +/* +FNXC:WorkflowAgentRouting 2026-08-10-01:15 (a built-in workflow owner is never unroutable — regression): + +Reported symptom: every task stopped moving. Work items churned `held → running → held` at ~3.5×/second across +the whole board, pinning a CPU core and writing ~19k `workflowWorkItem` audit rows/hour while ZERO work +executed. The hold reason was `workflow-principal-role-pool-exhausted:executor`. + +Root cause: `provisionBuiltinWorkflowRoleAgents` seeded the four permanent workflow owners (triage, executor, +reviewer, merger) with `runtimeConfig: { enabled: false }`, while the router's `available()` treats +`enabled === false` as unavailable. The only permanent principals for every built-in role were therefore +unroutable BY CONSTRUCTION — a defect every fresh instance ships with, not a local misconfiguration. Nothing +recovers on its own, because the pool can only change through operator action. + +Invariant under test: a built-in workflow role owner is always routable. It is coerced back to routable at the +durable write seam, so no caller — REST, dashboard toggle, plugin, provisioning, config restore — can put the +system into the deadlocked state; and the static routability predicate is SHARED with the router so +"what provisioning produces" and "what routing accepts" cannot drift apart again. +*/ +import { describe, expect, it } from "vitest"; +import { + enforceBuiltinWorkflowRoleRoutability, + isBuiltinWorkflowRoleAgent, + isWorkflowPrincipalEligible, +} from "../agent-role-policy.js"; + +const builtIn = (runtimeConfig?: Record) => ({ + id: "agent-builtin", + metadata: { builtInWorkflowRole: true, workflowRole: "executor" }, + runtimeConfig, +}); + +const operatorOwned = (runtimeConfig?: Record) => ({ + id: "agent-operator", + metadata: {}, + runtimeConfig, +}); + +describe("built-in workflow role routability invariant", () => { + it("coerces a disabled built-in owner back to routable", () => { + const result = enforceBuiltinWorkflowRoleRoutability(builtIn({ enabled: false })); + expect(result.runtimeConfig).toMatchObject({ enabled: true }); + expect(isWorkflowPrincipalEligible(result)).toBe(true); + }); + + it("preserves every other runtimeConfig key while coercing", () => { + // The operator's heartbeat cadence and claim policy are theirs; only routability is non-negotiable. + const result = enforceBuiltinWorkflowRoleRoutability( + builtIn({ enabled: false, heartbeatIntervalMs: 3_600_000, autoClaimRelevantTasks: true }), + ); + expect(result.runtimeConfig).toEqual({ + enabled: true, + heartbeatIntervalMs: 3_600_000, + autoClaimRelevantTasks: true, + }); + }); + + it("leaves an operator-owned agent free to be disabled", () => { + // The invariant protects the engine's own principals, not every agent — operators keep their off switch. + const result = enforceBuiltinWorkflowRoleRoutability(operatorOwned({ enabled: false })); + expect(result.runtimeConfig).toMatchObject({ enabled: false }); + expect(isWorkflowPrincipalEligible(result)).toBe(false); + expect(isBuiltinWorkflowRoleAgent(result)).toBe(false); + }); + + it("is a no-op for an already-routable built-in owner (same reference, no churn)", () => { + const agent = builtIn({ enabled: true }); + expect(enforceBuiltinWorkflowRoleRoutability(agent)).toBe(agent); + const unset = builtIn(); + expect(enforceBuiltinWorkflowRoleRoutability(unset)).toBe(unset); + }); + + /* + The predicate the router consults. `enabled === false` was the exact bit that made the pool look exhausted; + paused/errored agents must stay excluded so the coercion never resurrects a genuinely broken principal. + */ + it("still excludes paused and errored agents from principal routing", () => { + expect(isWorkflowPrincipalEligible({ runtimeConfig: { enabled: true }, state: "paused" })).toBe(false); + expect(isWorkflowPrincipalEligible({ runtimeConfig: { enabled: true }, state: "error" })).toBe(false); + expect(isWorkflowPrincipalEligible({ runtimeConfig: { enabled: true }, state: "active" })).toBe(true); + // An unset runtimeConfig is routable: only an explicit `false` opts an agent out. + expect(isWorkflowPrincipalEligible({ state: "active" })).toBe(true); + }); +}); diff --git a/packages/core/src/agents/agent-role-policy.ts b/packages/core/src/agents/agent-role-policy.ts index f0c516fbc1..ff4a97d460 100644 --- a/packages/core/src/agents/agent-role-policy.ts +++ b/packages/core/src/agents/agent-role-policy.ts @@ -79,6 +79,54 @@ export function canAgentReceiveImplementationTasks(agent: RoleTaggedAgent): bool return getAgentAssignmentPolicy(agent) !== "none"; } +/** + * FNXC:WorkflowAgentRouting 2026-08-10-01:15: + * The STATIC half of workflow-principal routability, shared so provisioning and the router cannot drift. + * A disabled runtime, a paused/errored agent, or a transient per-task worker can never own a workflow stage. + * The router adds the dynamic half (session capacity); this predicate is the part provisioning must satisfy + * for an instance to be able to route a role at all. + * + * Extracted after every built-in workflow owner shipped `runtimeConfig: { enabled: false }` while the router + * treated `enabled === false` as unavailable — so the only permanent principals for triage/executor/reviewer/ + * merger were unroutable by construction, and any instance without operator-created role agents held at its + * first workflow node. + */ +/** True for the four provenance-marked permanent owners that route built-in workflow stages. */ +export function isBuiltinWorkflowRoleAgent(agent: { metadata?: Record | null }): boolean { + return agent.metadata?.builtInWorkflowRole === true; +} + +/** + * FNXC:WorkflowAgentRouting 2026-08-10-01:15: + * HARD INVARIANT: a built-in workflow role owner is never unroutable. + * + * These four are the engine's own principals for triage/executor/reviewer/merger. Unlike an operator's agent, + * disabling one does not "opt an agent out" — it removes the only thing that can run that workflow stage, and + * the engine has no fallback: every task holds at its first node of that role and the hold re-dispatches + * forever. That is not a configuration an operator can meaningfully choose, so `runtimeConfig.enabled` is + * COERCED back to true for them at the write seam rather than validated and rejected: the write still + * succeeds, every other runtimeConfig key the caller sent is preserved, and the system cannot be put into the + * deadlocked state by an API call, a UI toggle, a plugin, or a stale record. + * + * To take a built-in owner out of rotation, add your own agent with that role and route to it — that path + * leaves the role routable, which is the property this protects. + */ +export function enforceBuiltinWorkflowRoleRoutability | null; + runtimeConfig?: Record | null; +}>(agent: T): T { + if (!isBuiltinWorkflowRoleAgent(agent)) return agent; + if (agent.runtimeConfig?.enabled !== false) return agent; + return { ...agent, runtimeConfig: { ...agent.runtimeConfig, enabled: true } }; +} + +export function isWorkflowPrincipalEligible( + agent: Pick & { state?: string; id?: string }, +): boolean { + if (agent.runtimeConfig?.enabled === false) return false; + return agent.state !== "paused" && agent.state !== "error"; +} + export function isImplementationTask(task: Pick): boolean { return IMPLEMENTATION_TASK_COLUMNS.has(task.column); } diff --git a/packages/core/src/agents/agent-store.ts b/packages/core/src/agents/agent-store.ts index f3ce4ac7bf..04eda27402 100644 --- a/packages/core/src/agents/agent-store.ts +++ b/packages/core/src/agents/agent-store.ts @@ -110,6 +110,7 @@ import { BUILTIN_WORKFLOW_ROLE_AGENT_DEFAULT_LIST, type BuiltinWorkflowRole, } from "./workflow-role-agent-defaults.js"; +import { enforceBuiltinWorkflowRoleRoutability } from "./agent-role-policy.js"; const agentStoreLog = createLogger("agent-store"); @@ -2058,7 +2059,16 @@ export class AgentStore extends EventEmitter { roles: [definition.role], title: definition.title, metadata: { builtInWorkflowRole: true, workflowRole: definition.role }, - runtimeConfig: { enabled: false }, + /* + FNXC:WorkflowAgentRouting 2026-08-10-01:15: + These four ARE the permanent principals that route built-in workflow stages, and the router treats + `runtimeConfig.enabled === false` as unavailable. Provisioning them disabled therefore created a + system that could not route ANY built-in role: every task held at its first workflow node with + `workflow-principal-role-pool-exhausted:`, and the held item was re-claimed with no backoff, + burning CPU and ~19k audit rows/hour while nothing executed. Seed them ENABLED so a fresh instance + can route out of the box; an operator can still disable one deliberately afterwards. + */ + runtimeConfig: { enabled: true }, instructionsText: definition.instructionsText, soul: definition.soul, bundleConfig: { ...BUILTIN_WORKFLOW_AGENT_BUNDLE_CONFIG, files: [...BUILTIN_WORKFLOW_AGENT_BUNDLE_CONFIG.files] }, @@ -2070,6 +2080,11 @@ export class AgentStore extends EventEmitter { || (agent.bundleConfig !== undefined && !canonicalOrPartialBundle) || (Boolean(agent.instructionsText?.trim()) && agent.instructionsText !== definition.instructionsText); const updates: Partial = {}; + // FNXC:WorkflowAgentRouting 2026-08-10-01:15: converge every existing built-in owner to routable. + // See enforceBuiltinWorkflowRoleRoutability — routability is an invariant for these four, not a setting. + if (agent.runtimeConfig?.enabled === false) { + updates.runtimeConfig = { ...agent.runtimeConfig, enabled: true }; + } if (!customInstructions && !agent.instructionsText?.trim()) updates.instructionsText = definition.instructionsText; if (!agent.soul?.trim()) updates.soul = definition.soul; // FNXC:WorkflowAgentIdentities 2026-08-08-06:38: Do not rewrite complete canonical @@ -3183,9 +3198,16 @@ export class AgentStore extends EventEmitter { } private async writeAgent(agent: Agent, executor?: QueryHandle): Promise { + /* + FNXC:WorkflowAgentRouting 2026-08-10-01:15: + The single durable write seam for agents, and therefore the only place the built-in-owner routability + invariant cannot be bypassed. Enforcing here — rather than in the REST handler — covers the API, the + dashboard toggle, plugins, provisioning, config-revision restores, and any future caller at once. + */ + const routable = enforceBuiltinWorkflowRoleRoutability(agent); // FNXC:SqliteFinalRemoval 2026-06-25-23:40: // Backend mode: delegate to async Drizzle writeAgent helper. - await writeAgentAsync(executor ?? this.asyncLayer!.db, agent, this.asyncLayer!.projectId); + await writeAgentAsync(executor ?? this.asyncLayer!.db, routable, this.asyncLayer!.projectId); return; } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 73b1f9eaa3..0c8098727c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -796,6 +796,9 @@ export { getAgentAssignmentPolicy, isAgentAutoAssignable, canAgentReceiveImplementationTasks, + isWorkflowPrincipalEligible, + isBuiltinWorkflowRoleAgent, + enforceBuiltinWorkflowRoleRoutability, evaluateImplementationTaskBind, assertImplementationTaskBindAllowed, AgentTaskRoutingPolicyError, diff --git a/packages/engine/src/agents/workflow-agent-router.ts b/packages/engine/src/agents/workflow-agent-router.ts index a3ec1ca2ab..dfbc7ffc13 100644 --- a/packages/engine/src/agents/workflow-agent-router.ts +++ b/packages/engine/src/agents/workflow-agent-router.ts @@ -2,6 +2,7 @@ import { canAgentReceiveImplementationTasks, classifyWorkflowAgentNode, isEphemeralAgent, + isWorkflowPrincipalEligible, resolveColumnAgentBinding, type Agent, type TaskDetail, @@ -133,13 +134,10 @@ function available(agent: Agent | undefined, activeSessions: ReadonlyMap undefined); } - await this.store.logEntry(task.id, `Workflow stage held — ${principalHoldReason}`).catch(() => undefined); /* * FNXC:WorkflowAgentRouting 2026-08-07-23:50: * The task must end this run with EXACTLY ONE active continuation, and the hold @@ -7892,6 +7927,9 @@ export class TaskExecutor { } return; } + // FNXC:WorkflowAgentRouting 2026-08-10-01:15: this run cleared the principal fence, so any prior hold + // is resolved — drop the ladder so a later hold starts from the short delay rather than a stale one. + this.clearPrincipalHoldBackoff(task.id); /* Direct graph node fences are terminalized only after the interpreter * returns, preserving their historical principal through all handler and * tool-gate calls while ensuring completed work cannot render as active. @@ -11487,6 +11525,27 @@ export class TaskExecutor { */ private sessionContentionHoldAttempts = new Map(); + /* + FNXC:WorkflowAgentRouting 2026-08-10-01:15: + Per-task cooldown for a workflow-principal hold, so an unroutable role pool is a cheap wait instead of a + dispatch hot loop. IN-MEMORY on purpose, matching the session-contention hold: it needs no schema change, + and a restart clearing it is correct — a restart is exactly when agent configuration may have changed. + */ + private principalHoldBackoff = new Map(); + + /** True while a principal hold is still cooling down, so dispatch should not re-enter the graph. */ + private isPrincipalHoldCoolingDown(taskId: string): boolean { + const hold = this.principalHoldBackoff.get(taskId); + if (!hold) return false; + if (Date.now() >= hold.until) return false; + return true; + } + + /** Clear the cooldown once the task dispatches for any other reason. */ + private clearPrincipalHoldBackoff(taskId: string): void { + this.principalHoldBackoff.delete(taskId); + } + private clearSessionContentionHold(taskId: string): void { this.sessionContentionHoldAttempts.delete(taskId); } @@ -14016,6 +14075,18 @@ export class TaskExecutor { both pass the graphRouting.has gate, both enter executeWorkflowGraph, and one park status=failed while the other still owned work (FN-8471 overseer thrash). */ + /* + FNXC:WorkflowAgentRouting 2026-08-10-01:15: + Honor an active principal-hold cooldown BEFORE the graph is entered. Without this the hold is recorded and + then immediately re-tested by the next dispatch, which is the hot loop itself: re-entering only to re-fence + and re-park costs a graph run, two work-item writes, and two audit rows per pass for a condition that can + only change when an operator enables or adds an agent. Skipping here is what makes the hold a real wait. + */ + if (this.isPrincipalHoldCoolingDown(task.id)) { + executorLog.debug(`execute() called for ${task.id} while a workflow-principal hold is cooling down — deferring`); + if (dropPreHeldExecutorSlot(task.id)) this.options.semaphore?.release(); + return; + } if (this.graphRouting.has(task.id)) { // Duplicate dispatch while the graph runner owns this task — drop it, // mirroring the executingTaskLock duplicate-invocation behavior.