diff --git a/.changeset/spawned-agent-capacity.md b/.changeset/spawned-agent-capacity.md new file mode 100644 index 0000000000..e813b8e170 --- /dev/null +++ b/.changeset/spawned-agent-capacity.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": minor +--- + +summary: Spawned child agents now count against Max Concurrent Tasks instead of a hidden spawn budget. +category: breaking +dev: Deletes `maxSpawnedAgentsPerParent` (5) and `maxSpawnedAgentsGlobal` (20). `fn_spawn_agent` now gates on the project agent count via `computeTopLevelConcurrencyClaimedFromStore` plus live children. Children were previously counted by neither capacity gate despite each getting its own git worktree, so a fan-out could add up to 20 worktrees while the scheduler believed the project was at its limit. The old per-parent budget also measured cumulative spawns over a task's life rather than concurrent ones, because the per-parent set was cleared only when the parent task ended. diff --git a/packages/core/src/settings-schema.ts b/packages/core/src/settings-schema.ts index 95fd230ed8..58fe52c47e 100644 --- a/packages/core/src/settings-schema.ts +++ b/packages/core/src/settings-schema.ts @@ -660,8 +660,6 @@ export const DEFAULT_PROJECT_SETTINGS = { maxTotalRetriesBeforeFail: 25, preserveProgressOnStuckRequeue: true, // maxPostReviewFixes MOVED to workflow settings (U4). - maxSpawnedAgentsPerParent: 5, - maxSpawnedAgentsGlobal: 20, // Run maintenance (including WAL checkpointing) every 5 minutes by default. maintenanceIntervalMs: 300_000, autoArchiveDoneTasksEnabled: true, diff --git a/packages/core/src/types/settings-scope.ts b/packages/core/src/types/settings-scope.ts index 2be3afb3e8..fa7f48199b 100644 --- a/packages/core/src/types/settings-scope.ts +++ b/packages/core/src/types/settings-scope.ts @@ -1908,14 +1908,6 @@ export interface ProjectSettings { * steps, and sends the task back through the normal todo → in-progress flow. Set * to 0 to disable. Default: 3. */ maxPostReviewFixes?: number; - /** Maximum number of child agents a single parent agent can spawn. - * Limits the fan-out per executor task to prevent resource exhaustion. - * Default: 5. */ - maxSpawnedAgentsPerParent?: number; - /** Maximum total spawned agents across all parent agents in a single executor instance. - * Provides a global safety cap regardless of how many parent agents are running. - * Default: 20. */ - maxSpawnedAgentsGlobal?: number; /** Interval in milliseconds for periodic maintenance (worktree pruning, WAL checkpoint, * orphan cleanup). 0 disables. Default: 900000 (15 min). */ maintenanceIntervalMs?: number; diff --git a/packages/dashboard/app/components/settings/sections/__tests__/settings-default-descriptions.test.tsx b/packages/dashboard/app/components/settings/sections/__tests__/settings-default-descriptions.test.tsx index de05a10be8..2359367ec9 100644 --- a/packages/dashboard/app/components/settings/sections/__tests__/settings-default-descriptions.test.tsx +++ b/packages/dashboard/app/components/settings/sections/__tests__/settings-default-descriptions.test.tsx @@ -581,8 +581,6 @@ const NOT_SURFACED_ALLOWLIST: Record = { prerebaseAutoEnabled: "internal pre-rebase tuning constant, no UI field", prerebaseHotFiles: "internal pre-rebase tuning constant, no UI field", prerebaseDivergenceThreshold: "internal pre-rebase tuning constant, no UI field", - maxSpawnedAgentsPerParent: "internal spawn-limit constant, no UI field", - maxSpawnedAgentsGlobal: "internal spawn-limit constant, no UI field", // FNXC:Round10 2026-07-13: FN-7907/FN-7908 added chat default model/agent/session settings. // These are configured via the chat New Session defaults picker, not plain description fields. chatNewSessionMode: "chat new-session default mode, configured via the chat defaults picker, not a plain description field", diff --git a/packages/engine/src/__tests__/spawn-agent-capacity.test.ts b/packages/engine/src/__tests__/spawn-agent-capacity.test.ts new file mode 100644 index 0000000000..24ed4e4a8c --- /dev/null +++ b/packages/engine/src/__tests__/spawn-agent-capacity.test.ts @@ -0,0 +1,165 @@ +import { describe, expect, it, vi } from "vitest"; +import { TaskExecutor } from "../executor.js"; + +/* +FNXC:CapacityModel 2026-07-29-14:10 (two numbers — spawned agents count): +`fn_spawn_agent` now gates on the PROJECT agent count instead of two private +budgets (`maxSpawnedAgentsPerParent` 5, `maxSpawnedAgentsGlobal` 20, both deleted). + +Why this is a hole being closed and not only knobs being removed: a spawned child +is an agent AND gets its own git worktree, but was counted by NEITHER capacity +gate. A fan-out could put up to 20 extra worktrees on disk while the scheduler +believed the project was at its configured limit — the operator's two numbers were +simply wrong about what was running. + +The old caps also measured the wrong thing. `totalSpawnedCount` is decremented when +a child is cleaned up, but the per-parent set was only cleared when the PARENT task +ended, so `maxSpawnedAgentsPerParent` throttled cumulative spawns over a task's +life rather than concurrent ones — a long task could exhaust its budget with five +children that had all long since finished. +*/ + +function executorWithSpawnState(opts: { + claimedTasks: unknown[]; + liveChildren: number; + maxConcurrent: number; +}): { executor: TaskExecutor; listTasks: ReturnType } { + const listTasks = vi.fn(async () => opts.claimedTasks); + const executor = Object.create(TaskExecutor.prototype) as TaskExecutor; + const priv = executor as unknown as Record; + priv.store = { + listTasks, + getTask: vi.fn(async (id: string) => ({ id, column: "in-progress" })), + getTaskWorkflowSelectionAsync: vi.fn(async () => undefined), + getWorkflowDefinition: vi.fn(async () => undefined), + }; + priv.options = { agentStore: { createAgent: vi.fn() } }; + priv.spawnedAgents = new Map>(); + priv.totalSpawnedCount = opts.liveChildren; + return { executor, listTasks }; +} + +/** Drive the tool's capacity branch without the agent-creation tail. */ +async function trySpawn(executor: TaskExecutor, taskId: string, maxConcurrent: number) { + const tool = (executor as unknown as { + createSpawnAgentTool(taskId: string, worktreePath: string, settings: unknown): { + execute(id: string, params: unknown): Promise<{ content: Array<{ text: string }>; details: { state: string } }>; + }; + }).createSpawnAgentTool(taskId, "/tmp/wt", { maxConcurrent }); + return tool.execute("call-1", { name: "child", role: "engineer", task: "do a thing" }); +} + +describe("fn_spawn_agent capacity", () => { + /* + Reverting to the private budgets makes this pass at 1/1: the old per-parent cap + was 5 and the old global cap 20, so a project already at its agent limit could + still spawn — which is the hole. + */ + it("refuses to spawn when the project's agent count is already consumed", async () => { + const { executor } = executorWithSpawnState({ + claimedTasks: [{ id: "FN-1", column: "in-progress" }], + liveChildren: 0, + maxConcurrent: 1, + }); + + const result = await trySpawn(executor, "FN-1", 1); + + expect(result.details.state).toBe("error"); + expect(result.content[0]?.text).toContain("Agent capacity reached"); + expect(result.content[0]?.text).toContain("1/1"); + }); + + /* + Live children count toward the SAME number, not a separate budget. Without this + term a parent at the project limit could still fan out, which is exactly what the + private global cap allowed. + */ + it("counts live spawned children toward the project agent count", async () => { + const { executor } = executorWithSpawnState({ + claimedTasks: [{ id: "FN-1", column: "in-progress" }], + liveChildren: 1, + maxConcurrent: 2, + }); + + const result = await trySpawn(executor, "FN-1", 2); + + expect(result.details.state).toBe("error"); + // 1 claimed task + 1 live child == the cap of 2. + expect(result.content[0]?.text).toContain("2/2"); + expect(result.content[0]?.text).toContain("1 spawned child agent(s)"); + }); + + it("permits a spawn while the project has agent headroom", async () => { + const { executor } = executorWithSpawnState({ + claimedTasks: [{ id: "FN-1", column: "in-progress" }], + liveChildren: 0, + maxConcurrent: 4, + }); + + const result = await trySpawn(executor, "FN-1", 4); + + // Past the capacity branch: it proceeds into agent creation rather than + // returning the capacity refusal. + expect(result.content[0]?.text ?? "").not.toContain("Agent capacity reached"); + }); + + /* + The message names the knob an operator can actually change. The deleted caps + pointed at settings that no longer exist, which is worse than no message: it sends + someone hunting for a control that is not there. + */ + it("names Max Concurrent Tasks in the refusal, not a deleted spawn cap", async () => { + const { executor } = executorWithSpawnState({ + claimedTasks: [{ id: "FN-1", column: "in-progress" }], + liveChildren: 0, + maxConcurrent: 1, + }); + + const result = await trySpawn(executor, "FN-1", 1); + + expect(result.content[0]?.text).toContain("Max Concurrent Tasks"); + expect(result.content[0]?.text).not.toMatch(/spawn limit/i); + }); + /* + FNXC:CapacityModel 2026-07-29-19:20 (PR #2579 review — greptile P1, TOCTOU): + Two parents with ONE slot left must not both spawn. + + The check read capacity, then several awaits followed (createAgent, createWorktree, + updateAgentState) before the count was incremented — so both calls passed and both + spawned, producing more agents and more worktrees than Max Concurrent Tasks + permits. That is the very hole this change set out to close, reintroduced by the + fix for it. + */ + it("reserves the slot before awaiting, so two concurrent spawns cannot both pass", async () => { + const { executor } = executorWithSpawnState({ + claimedTasks: [{ id: "FN-1", column: "in-progress" }], + liveChildren: 0, + maxConcurrent: 2, // 1 claimed task + 1 free slot + }); + + // Both callers race the same free slot without awaiting between them. + const [first, second] = await Promise.all([ + trySpawn(executor, "FN-1", 2), + trySpawn(executor, "FN-1", 2), + ]); + + const refused = [first, second].filter((r) => r.content[0]?.text?.includes("Agent capacity reached")); + expect(refused, "exactly one of two racing spawns must be refused").toHaveLength(1); + }); + + it("returns the reserved slot when the spawn fails", async () => { + const { executor } = executorWithSpawnState({ + claimedTasks: [{ id: "FN-1", column: "in-progress" }], + liveChildren: 0, + maxConcurrent: 4, + }); + (executor as unknown as { options: { agentStore: { createAgent: unknown } } }).options.agentStore.createAgent = + vi.fn(async () => { throw new Error("agent store unavailable"); }); + + const result = await trySpawn(executor, "FN-1", 4); + expect(result.content[0]?.text).toContain("Failed to spawn agent"); + + // A failed spawn must not permanently consume capacity. + expect((executor as unknown as { totalSpawnedCount: number }).totalSpawnedCount).toBe(0); + }); +}); \ No newline at end of file diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 359800ee28..3a62df97dc 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -121,6 +121,7 @@ import type { SandboxBackend } from "./sandbox/types.js"; import { ModelRegistry, SessionManager, type ToolDefinition, type AgentSession } from "@earendil-works/pi-coding-agent"; import { PRIORITY_EXECUTE, + computeTopLevelConcurrencyClaimedFromStore, dropPreHeldExecutorSlot, takePreHeldExecutorSlot, type AgentSemaphore, @@ -20989,26 +20990,60 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB }; } - // Read spawn limits from settings - const maxPerParent = settings.maxSpawnedAgentsPerParent ?? 5; - const maxGlobal = settings.maxSpawnedAgentsGlobal ?? 20; + /* + FNXC:CapacityModel 2026-07-29-14:10 (two numbers — spawned agents count): + `maxSpawnedAgentsPerParent` (5) and `maxSpawnedAgentsGlobal` (20) are DELETED. + They were a THIRD and FOURTH limiter with their own private budgets, invisible + to the two the operator configures — and they measured the wrong thing: a + child that finished still counted against `totalSpawnedCount` until its parent + task ended, so the cap throttled cumulative spawns rather than concurrent ones. - // Check per-parent limit - const currentPerParent = this.spawnedAgents.get(taskId)?.size ?? 0; - if (currentPerParent >= maxPerParent) { + A spawned child IS an agent and runs in its own git worktree (branched from the + parent's), so it consumes both configured dimensions. It now checks the SAME + project agent count every other lane checks, via the shared live-claim helper — + one number, one answer, no private budget that can disagree with the board. + + This closes a real hole rather than only deleting knobs: children were counted + by NEITHER capacity gate, so a fan-out could put up to 20 extra worktrees on + disk while the scheduler believed the project was at its limit. + */ + const spawnClaimed = await computeTopLevelConcurrencyClaimedFromStore({ + store: this.store, + tasks: await this.store.listTasks({ slim: true, includeArchived: false }), + }); + const spawnCap = settings.maxConcurrent ?? 2; + const liveChildren = this.totalSpawnedCount; + if (spawnClaimed + liveChildren >= spawnCap) { return { - content: [{ type: "text" as const, text: `Per-parent spawn limit reached (${currentPerParent}/${maxPerParent}). Wait for children to finish or reduce parallelism.` }], + content: [{ + type: "text" as const, + text: `Agent capacity reached (${spawnClaimed + liveChildren}/${spawnCap} running, including ${liveChildren} spawned child agent(s)). Wait for work to finish, or raise Max Concurrent Tasks.`, + }], details: { agentId: "", state: "error" }, }; } + /* + FNXC:CapacityModel 2026-07-29-19:20 (PR #2579 review — greptile P1, TOCTOU): + RESERVE THE SLOT SYNCHRONOUSLY, before the first await. - // Check global limit - if (this.totalSpawnedCount >= maxGlobal) { - return { - content: [{ type: "text" as const, text: `Global spawn limit reached (${this.totalSpawnedCount}/${maxGlobal}). Cannot spawn more agents.` }], - details: { agentId: "", state: "error" }, - }; - } + The check above reads capacity, then several awaits follow (createAgent, + createWorktree, updateAgentState) before `totalSpawnedCount` was incremented. + Two parents calling fn_spawn_agent with one slot left both passed the check + and both spawned — more agents and more worktrees than Max Concurrent Tasks + permits, which is the very hole this change set out to close. + + JS is single-threaded, so incrementing here — with NO await between the read + and the increment — makes check-and-reserve atomic against every other spawn + call. The reservation is rolled back on any failure below, and the success + path no longer double-counts. + */ + this.totalSpawnedCount++; + let spawnReservationHeld = true; + const releaseSpawnReservation = () => { + if (!spawnReservationHeld) return; + spawnReservationHeld = false; + this.totalSpawnedCount = Math.max(0, this.totalSpawnedCount - 1); + }; try { // Create agent in AgentStore with reportsTo = parent task ID @@ -21122,7 +21157,9 @@ Child agent: ${agent.id} (${name})`; this.spawnedAgents.set(taskId, new Set()); } this.spawnedAgents.get(taskId)!.add(agent.id); - this.totalSpawnedCount++; + // The slot was already reserved before the awaits above; converting the + // reservation into the live count is a no-op rather than a second increment. + spawnReservationHeld = false; // Run child asynchronously (don't await — parent continues working) this.runSpawnedChild(agent.id, childSession, taskPrompt).catch((err: unknown) => { @@ -21143,6 +21180,10 @@ Child agent: ${agent.id} (${name})`; details: result, }; } catch (err: unknown) { + // FNXC:CapacityModel 2026-07-29-19:20: a failed spawn must return the slot + // it reserved, or a project permanently loses capacity to a spawn that + // never happened. + releaseSpawnReservation(); const errorMessage = err instanceof Error ? err.message : String(err); return { content: [{ type: "text" as const, text: `Failed to spawn agent: ${errorMessage}` }],