From 234fb474afdb5890e6bb73bb2ba4db7929b00d73 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 24 Aug 2026 06:39:29 -0700 Subject: [PATCH] Fix spawn_agent to thread director package turn budget spawn_agent resolved a director's turn budget as Infinity because resolveDirectorDispatch never surfaced the package's nudge.maxTurns and agent-fleet.ts never passed it to resolveSubAgentMaxTurns, unlike task(). Also removes the false "hard cap 4 workers" prompt claim (no such cap exists), corrects the task-watchdog exemption comment (no-progress/thrash detectors were removed), and deletes the unread profile.maxTurns config knob. --- CHANGELOG.md | 10 +++++++ docs/IMPLEMENTATION.md | 5 ++-- src/agent/directors/skywalker/package.ts | 2 +- src/agent/prompts.ts | 2 +- src/config.test.ts | 3 +- src/config/index.ts | 2 -- src/config/profiles.ts | 2 -- src/profiles.test.ts | 23 +++++--------- src/subagent/agent-fleet.ts | 8 ++++- src/subagent/spawn-budget.test.ts | 38 ++++++++++++++++++++++++ src/tui/tool-execution-watchdog.ts | 4 +-- 11 files changed, 69 insertions(+), 30 deletions(-) create mode 100644 src/subagent/spawn-budget.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d7e6e5918..de77e013e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,16 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename ### Agent +- `spawn_agent` now threads the resolved director package's `nudge.maxTurns` + budget the same way `task()` does, closing a parity gap where a director + dispatched via `spawn_agent` resolved to an unbounded turn budget instead of + its configured finite one. Removed the false "hard cap 4 workers" claim from + director prompt text (no such cap exists anywhere in the fleet code). The + unused `maxTurns` field on project/named profile files + (`.corbits/profile.json`, `~/.corbits/profiles/.json`) has been + removed since nothing read it — a silently-ignored knob is worse than no + knob. + - `evaluateSubAgentStop` now always requires the final assistant text; the omitted-text branch that unconditionally completed a tool-less turn is removed, so every call path gets the `incomplete-report` nudge and salvage diff --git a/docs/IMPLEMENTATION.md b/docs/IMPLEMENTATION.md index 43d9b0f78..4be525468 100644 --- a/docs/IMPLEMENTATION.md +++ b/docs/IMPLEMENTATION.md @@ -286,7 +286,7 @@ OpenAI-compatible `baseURL` values are normalized during provider resolution. A ### Profiles (`src/config/profiles.ts`) -Profiles supply per-project or named-profile overrides for `model`, `maxTurns`, and `systemPromptExtensions` (the only allowed keys; any other key is rejected on load). +Profiles supply per-project or named-profile overrides for `model` and `systemPromptExtensions` (the only allowed keys; any other key is rejected on load). - Project profile: `.corbits/profile.json` in the repo root — committed, credential-free. - Named profiles: `~/.corbits/profiles/.json` — user-level overrides, inherited via the `profile` key or the `--profile` flag. @@ -295,12 +295,11 @@ Profiles supply per-project or named-profile overrides for `model`, `maxTurns`, { "profile": "work", "model": "claude-opus-4-8", - "maxTurns": 50, "systemPromptExtensions": ["no-destructive-migrations"] } ``` -`resolveProfile` merges a named profile with the project profile, with **project profile field values overriding the named profile's**. The resolved `model` / `maxTurns` feed into provider resolution and the director; `systemPromptExtensions` are appended to the system prompt. Workflow profile metadata is deprecated because workflows are started only by explicit slash commands. CLI flags (`--model`, `--profile`) still win over profile values during config resolution. +`resolveProfile` merges a named profile with the project profile, with **project profile field values overriding the named profile's**. The resolved `model` feeds into provider resolution and the director; `systemPromptExtensions` are appended to the system prompt. Workflow profile metadata is deprecated because workflows are started only by explicit slash commands. CLI flags (`--model`, `--profile`) still win over profile values during config resolution. ### Provider Configuration diff --git a/src/agent/directors/skywalker/package.ts b/src/agent/directors/skywalker/package.ts index 5bfa987dd..ff14fc759 100644 --- a/src/agent/directors/skywalker/package.ts +++ b/src/agent/directors/skywalker/package.ts @@ -97,7 +97,7 @@ Before responding, classify: Tiny / single-file / one-route / clear bounded edit: write_file/edit_file/delete_file on this session. Do not spawn. -Substantial / multi-file / parallel lanes / long-running: spawn build (hard cap 4). Keep long-blocking jobs off the parent so Enter can steer. +Substantial / multi-file / parallel lanes / long-running: spawn build. Keep long-blocking jobs off the parent so Enter can steer. Docs/design (PRODUCT.md, ARCHITECTURE.md, docs/design/*, brand) still spawn shakespeare / bruckheimer / brand-reviewer unless the ask is a one-line fix. diff --git a/src/agent/prompts.ts b/src/agent/prompts.ts index 46be70d76..95e8a490c 100644 --- a/src/agent/prompts.ts +++ b/src/agent/prompts.ts @@ -64,7 +64,7 @@ export function buildHarnessFacts( "- Change files with write_file/edit_file and remove files with delete_file; shell file-writes and deletions are blocked.", ] : [ - "- Change files with write_file/edit_file and remove files with delete_file for tiny/single-file/one-route bounded edits. Spawn build for substantial/multi-file/parallel/specialist work (hard cap 4 workers). Docs/design still spawn shakespeare/bruckheimer/brand-reviewer except one-line fixes.", + "- Change files with write_file/edit_file and remove files with delete_file for tiny/single-file/one-route bounded edits. Spawn build for substantial/multi-file/parallel/specialist work. Docs/design still spawn shakespeare/bruckheimer/brand-reviewer except one-line fixes.", "- Shell file-writes and deletions are blocked; never use echo/heredoc/sed/rm as a substitute for product tools. Path tools are the DIY surface.", ]), "- Use the provided tools for file reads/searches instead of shelling out as a substitute.", diff --git a/src/config.test.ts b/src/config.test.ts index 799dabde0..3210832f6 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -768,12 +768,11 @@ describe("loadConfig", () => { await mkdir(join(cwd, ".corbits"), { recursive: true }); await writeFile( join(cwd, ".corbits", "profile.json"), - JSON.stringify({ model: "profile-model", maxTurns: 25, systemPromptExtensions: ["ext1"] }), + JSON.stringify({ model: "profile-model", systemPromptExtensions: ["ext1"] }), ); const config = await loadConfig(["--cwd", cwd, "task"], { globalSettingsPath: globalPath }); assertConfigured(config); expect(config.model).toBe("profile-model"); - expect(config.maxTurns).toBe(25); expect(config.systemPromptExtensions).toEqual(["ext1"]); } finally { await rm(cwd, { recursive: true, force: true }); diff --git a/src/config/index.ts b/src/config/index.ts index 843ba7797..dcb5f831e 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -323,7 +323,6 @@ export interface Config { providers: ProviderCatalogEntry[]; profile?: string; systemPromptExtensions?: string[]; - maxTurns?: number; // Per-call inactivity timeout in ms (default 120_000 in the harness). Tune // higher for reasoning models with long silent-thinking stretches. inactivityTimeoutMs?: number; @@ -768,7 +767,6 @@ export async function loadConfig( ...(profile.systemPromptExtensions !== undefined ? { systemPromptExtensions: profile.systemPromptExtensions } : {}), - ...(profile.maxTurns !== undefined ? { maxTurns: profile.maxTurns } : {}), ...(profile.inactivityTimeoutMs !== undefined ? { inactivityTimeoutMs: profile.inactivityTimeoutMs } : {}), diff --git a/src/config/profiles.ts b/src/config/profiles.ts index f9ca998f0..26b40d4c1 100644 --- a/src/config/profiles.ts +++ b/src/config/profiles.ts @@ -7,7 +7,6 @@ import { SETTINGS_DIR_NAME } from "../branding.js"; const ProfileSchema = type({ "profile?": "string", "model?": "string", - "maxTurns?": "number.integer >= 1", "systemPromptExtensions?": "string[]", "workflow?": "string", // Per-call inactivity timeout in milliseconds. If the provider yields no @@ -87,7 +86,6 @@ export async function resolveProfile(cwd: string, profileName?: string): Promise const merged: ProfileConfig = { ...namedProfile }; if (projectProfile !== null && projectProfile !== undefined) { if (projectProfile.model !== undefined) merged.model = projectProfile.model; - if (projectProfile.maxTurns !== undefined) merged.maxTurns = projectProfile.maxTurns; if (projectProfile.systemPromptExtensions !== undefined) { merged.systemPromptExtensions = projectProfile.systemPromptExtensions; } diff --git a/src/profiles.test.ts b/src/profiles.test.ts index ee19bff01..48804476c 100644 --- a/src/profiles.test.ts +++ b/src/profiles.test.ts @@ -30,9 +30,9 @@ test("loadProfile parses valid profile", async () => { const dir = makeTmp(); await mkdir(dir, { recursive: true }); const path = join(dir, "profile.json"); - await writeFile(path, JSON.stringify({ model: "claude-opus-4-8", maxTurns: 50 })); + await writeFile(path, JSON.stringify({ model: "claude-opus-4-8" })); const result = await loadProfile(path); - expect(result).toEqual({ model: "claude-opus-4-8", maxTurns: 50 }); + expect(result).toEqual({ model: "claude-opus-4-8" }); }); test("loadProfile parses systemPromptExtensions", async () => { @@ -52,14 +52,6 @@ test("loadProfile rejects unknown keys", async () => { await expect(loadProfile(path)).rejects.toThrow(/unknownKey must be removed/); }); -test("loadProfile rejects invalid maxTurns", async () => { - const dir = makeTmp(); - await mkdir(dir, { recursive: true }); - const path = join(dir, "profile.json"); - await writeFile(path, JSON.stringify({ maxTurns: -1 })); - await expect(loadProfile(path)).rejects.toThrow(/maxTurns/); -}); - test("loadProfile rejects non-array systemPromptExtensions", async () => { const dir = makeTmp(); await mkdir(dir, { recursive: true }); @@ -89,11 +81,11 @@ test("resolveProfile applies project profile fields", async () => { await mkdir(dir, { recursive: true }); await writeFile( join(dir, "profile.json"), - JSON.stringify({ model: "claude-sonnet", maxTurns: 30 }), + JSON.stringify({ model: "claude-sonnet", systemPromptExtensions: ["ext1"] }), ); const result = await resolveProfile(cwd); expect(result.model).toBe("claude-sonnet"); - expect(result.maxTurns).toBe(30); + expect(result.systemPromptExtensions).toEqual(["ext1"]); }); test("resolveProfile surfaces profile name when set", async () => { @@ -112,7 +104,7 @@ test("resolveProfile: project profile fields override named profile fields", asy await mkdir(namedDir, { recursive: true }); await writeFile( join(namedDir, "work.json"), - JSON.stringify({ model: "base-model", maxTurns: 10 }), + JSON.stringify({ model: "base-model", systemPromptExtensions: ["ext1"] }), ); const localDir = join(cwd, ".corbits"); await mkdir(localDir, { recursive: true }); @@ -128,8 +120,7 @@ test("resolveProfile: project profile fields override named profile fields", asy const namedProfile = await loadProfile(join(namedDir, "work.json")); const merged = { ...namedProfile }; if (projectProfile?.model !== undefined) merged.model = projectProfile.model; - if (projectProfile?.maxTurns !== undefined) merged.maxTurns = projectProfile.maxTurns; expect(merged.model).toBe("override-model"); - // maxTurns not in project profile so named profile value survives - expect(merged.maxTurns).toBe(10); + // systemPromptExtensions not in project profile so named profile value survives + expect(merged.systemPromptExtensions).toEqual(["ext1"]); }); diff --git a/src/subagent/agent-fleet.ts b/src/subagent/agent-fleet.ts index 7a5d18b41..62ec962e8 100644 --- a/src/subagent/agent-fleet.ts +++ b/src/subagent/agent-fleet.ts @@ -283,7 +283,7 @@ function fleetResult(callId: string, content: string): ToolResult { } /** Resolve agent=/intent= to a closed director. Mirrors task()'s director-only branch. */ -function resolveDirectorDispatch( +export function resolveDirectorDispatch( agentId: string | undefined, intent: TaskIntent | undefined, ): @@ -293,6 +293,7 @@ function resolveDirectorDispatch( systemPromptRole: string; capabilities: ReturnType; roleDefault: ReturnType; + profileMaxTurns: number | undefined; } | { ok: false; error: string } { if (agentId !== undefined && agentId.length > 0) { @@ -311,6 +312,7 @@ function resolveDirectorDispatch( systemPromptRole: formatDirectorSystemPrompt(pkg), capabilities: packageToCapabilities(pkg), roleDefault: defaultEffortForDirector(pkg), + profileMaxTurns: pkg.nudge?.maxTurns, }; } if (intent !== undefined) { @@ -323,6 +325,7 @@ function resolveDirectorDispatch( systemPromptRole: formatDirectorSystemPrompt(pkg), capabilities: packageToCapabilities(pkg), roleDefault: defaultEffortForDirector(pkg), + profileMaxTurns: pkg.nudge?.maxTurns, }; } return { @@ -383,6 +386,9 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool { const resolvedMaxTurns = resolveSubAgentMaxTurns({ ...(settings !== undefined ? { settings } : {}), ...(taskMaxTurns !== undefined ? { taskMaxTurns } : {}), + ...(resolved.profileMaxTurns !== undefined + ? { profileMaxTurns: resolved.profileMaxTurns } + : {}), }); let provider: SubAgentProvider = resolveDep(deps.provider); diff --git a/src/subagent/spawn-budget.test.ts b/src/subagent/spawn-budget.test.ts new file mode 100644 index 000000000..9b8d52683 --- /dev/null +++ b/src/subagent/spawn-budget.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, test } from "bun:test"; +import { resolveSubAgentMaxTurns } from "../config/settings.js"; +import { resolveDirector } from "../agent/directors/registry.js"; +import { resolveDirectorDispatch } from "./agent-fleet.js"; + +describe("spawn_agent vs task() turn budget parity", () => { + for (const id of ["intern", "explore", "build", "critique", "greybeard"]) { + test(`${id}: spawn_agent and task() resolve the same finite budget`, () => { + const resolved = resolveDirector({ agentId: id }); + expect(resolved.ok).toBe(true); + const pkgMax = resolved.ok ? resolved.package.nudge?.maxTurns : undefined; + expect(Number.isFinite(pkgMax)).toBe(true); + + // task() path: passes profileMaxTurns (task-tool.ts:595) + const viaTask = resolveSubAgentMaxTurns({ profileMaxTurns: pkgMax as number }); + expect(viaTask).toBe(pkgMax as number); + + // spawn_agent path: resolveDirectorDispatch now surfaces the same + // package budget, and agent-fleet.ts threads it through. + const dispatch = resolveDirectorDispatch(id, undefined); + expect(dispatch.ok).toBe(true); + const viaSpawn = resolveSubAgentMaxTurns({ + ...(dispatch.ok && dispatch.profileMaxTurns !== undefined + ? { profileMaxTurns: dispatch.profileMaxTurns } + : {}), + }); + expect(viaSpawn).toBe(pkgMax as number); + expect(viaSpawn).toBe(viaTask); + }); + } + + test("non-director dispatch with no explicit maxTurns remains unbounded", () => { + // No agent/intent resolved to a director means no package budget exists; + // this is intentional and must not gain a default cap. + const viaSpawn = resolveSubAgentMaxTurns({}); + expect(viaSpawn).toBe(Infinity); + }); +}); diff --git a/src/tui/tool-execution-watchdog.ts b/src/tui/tool-execution-watchdog.ts index 4cfb6ebdd..8cc786ee1 100644 --- a/src/tui/tool-execution-watchdog.ts +++ b/src/tui/tool-execution-watchdog.ts @@ -71,8 +71,8 @@ export const MAX_TOOL_APPROVAL_PAUSE_MS = 1_800_000; * to MAX_TOOL_EXECUTION_TIMEOUT_MS or tools.maxTimeoutMs. * * The task tool is exempt: it runs an entire sub-agent that carries its own - * bounds (maxTurns, no-progress, thrash, opt-in deadline), so the generic - * per-tool budget would abort healthy long-running workers mid-run. + * bounds (maxTurns, opt-in deadline), so the generic per-tool budget would + * abort healthy long-running workers mid-run. * * mcp__* tool calls are the opposite of exempt: they arm unconditionally (see * resolveMcpToolTimeoutMs) even when no Settings are configured, because an