From 83045f24f31c460cf942f990d7d6cf6f4c2d30fb Mon Sep 17 00:00:00 2001 From: Sawyer Date: Sun, 20 Sep 2026 22:52:42 -0700 Subject: [PATCH] fix(skywalker): spawn one focused task per worker --- docs/ARCHITECTURE.md | 6 +++--- docs/IMPLEMENTATION.md | 2 +- docs/PRODUCT.md | 2 +- src/agent/directors/skywalker/package.test.ts | 5 +++++ src/agent/directors/skywalker/package.ts | 8 +++++--- src/agent/prompts.test.ts | 4 ++-- src/agent/prompts.ts | 4 ++-- src/prompts.test.ts | 3 +++ src/subagent/agent-fleet.ts | 2 +- 9 files changed, 23 insertions(+), 13 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 97bacb163..a821a01ca 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -230,9 +230,9 @@ Three distinct concepts (do not conflate them): | --------------- | -------------------------------------------------------- | -------------------------------------------------------------------------------------- | | **Agent** | A runtime entity with its own loop, tools, and context | Primary session or a spawned child | | **Task** | A checklist item owned by _one_ agent via `manage_tasks` | Local work plan — not a spawn | -| **Fleet agent** | A short-lived worker for one self-contained job | Spawned with **`spawn_agent`**; mailbox mail arrives as inbound on TUI and nested runs | +| **Fleet agent** | A short-lived worker for one focused, self-contained job | Spawned with **`spawn_agent`**; mailbox mail arrives as inbound on TUI and nested runs | -The **`spawn_agent`** tool starts a fleet agent on a separate inference source (tier/profile resolved from settings) and returns immediately with an `agent_id`. On the TUI primary, Skywalker idles after spawn and occupancy delivers mailbox mail as system inbound when a worker finishes or fails (including while siblings still run). Nested orchestrators collect through mailbox mail the same way. Declared fan-out is unlimited: excess dispatches enqueue rather than fail. `run()` is admitted by `src/subagent/admission.ts` (default burst window of 8 is race-avoidance so a 429 freeze can fire before a herd — not a declared-spawn cap). Occupancy is the whole first `run()`, including a mounted `wait_agents` (exec primary). Nested children of an already-admitted parent bypass **capacity** so a nested orchestrator cannot deadlock while holding a slot; they still wait on a provider 429 pause. Drain is FIFO among currently admissible jobs (a paused provider is skipped, not head-of-line for every provider). Resume and followup inference re-enter the same queue. Queued workers report wait/list status `queued` (live, not failed). Lowering capacity never cancels in-flight work. Retryable provider 429s freeze new admits via the shared retry remapper in `createCorbitsRetryPolicy`; `quota_exhausted` does not freeze. `list_agents` remains mailbox-scoped. The dispatch brief separates durable `context`, actionable `prompt`, and optional `goals` (checklist seeds for the _child's_ own `manage_tasks` list). Implement/review dispatches (and their default directors) fail closed without non-empty `success_criteria`. The child returns a structured report (`Summary` / `Findings` / `Blockers` / `Paths`) plus a tools-used footer. Parent and child never share a `manage_tasks` list. +The **`spawn_agent`** tool starts a fleet agent on a separate inference source (tier/profile resolved from settings) and returns immediately with an `agent_id`. Each spawned worker gets one focused task; fan-out width follows independent lanes (one lane per PR/path/ownership). Keep the dispatch brief contract (`intent`, `success_criteria`, `do_not`, `report_focus`) tight. On the TUI primary, Skywalker idles after spawn and occupancy delivers mailbox mail as system inbound when a worker finishes or fails (including while siblings still run). Nested orchestrators collect through mailbox mail the same way. Declared fan-out is unlimited: excess dispatches enqueue rather than fail. `run()` is admitted by `src/subagent/admission.ts` (default burst window of 8 is race-avoidance so a 429 freeze can fire before a herd — not a declared-spawn cap). Occupancy is the whole first `run()`, including a mounted `wait_agents` (exec primary). Nested children of an already-admitted parent bypass **capacity** so a nested orchestrator cannot deadlock while holding a slot; they still wait on a provider 429 pause. Drain is FIFO among currently admissible jobs (a paused provider is skipped, not head-of-line for every provider). Resume and followup inference re-enter the same queue. Queued workers report wait/list status `queued` (live, not failed). Lowering capacity never cancels in-flight work. Retryable provider 429s freeze new admits via the shared retry remapper in `createCorbitsRetryPolicy`; `quota_exhausted` does not freeze. `list_agents` remains mailbox-scoped. The dispatch brief separates durable `context`, actionable `prompt`, and optional `goals` (checklist seeds for the _child's_ own `manage_tasks` list). Implement/review dispatches (and their default directors) fail closed without non-empty `success_criteria`. The child returns a structured report (`Summary` / `Findings` / `Blockers` / `Paths`) plus a tools-used footer. Parent and child never share a `manage_tasks` list. Workers ask the spawning parent with **`ask_director`** (not the human). That parks a question while the worker stays `running`. On the TUI primary that arrives as an idle-send wake. On a nested orchestrator the question arrives as mailbox mail with an `awaiting_director` status — that is not terminal. Once a parked ask is surfaced (TUI wake, nested mailbox mail, or a successful `list_agents`), further `list_agents` calls fail closed until **`send_input`** answers or the ask is dropped — `list_agents` is not a poll. The parent answers with **`send_input`**, then continues. Escalate to the human with **`ask_operator`** only when the parent cannot resolve it. @@ -340,7 +340,7 @@ The primary session identity is **Skywalker** (`buildChatRole` → `createSkywal - `buildChatRole` — Skywalker primary identity (orchestrate; DIY tiny/bounded product edits; spawn for substantial work). - `buildHarnessFacts` — the non-derivable rules: shell file-writes are blocked, path tools are the DIY surface on primary (spawn builder/docs directors for substantial work), dependency installs and off-limits paths need approval, images are native multimodal input, core tools plus the advertised catalog (including `skill_search`) are resident (MCP and other unadvertised tools load via `tool_search`; use `search_agents` before dispatching specialists), workflows run only from slash-command steps, and session memory lives at `.corbits/MEMORY.md`. -- `buildGuidelines` — be concise, prefer `spawn_agent` (mailbox mail on TUI; exec-primary `wait_agents`) for substantial product work, DIY tiny/bounded edits on the parent, answer questions and diagnose visual/product feedback before editing, work autonomously for explicit coding tasks, use `lsp` for symbol work, and require implementation agents to run the repository-defined typecheck, relevant tests, and every defined full verification command. Agents report exact commands, outcomes, and exit statuses; a repository with no typecheck command produces an explicit Blocker backed by project-configuration evidence rather than an invented command or silent skip. +- `buildGuidelines` — be concise, prefer `spawn_agent` (mailbox mail on TUI; exec-primary `wait_agents`) for substantial product work with one focused task per worker and fan-out by independent lanes, DIY tiny/bounded edits on the parent, answer questions and diagnose visual/product feedback before editing, work autonomously for explicit coding tasks, use `lsp` for symbol work, and require implementation agents to run the repository-defined typecheck, relevant tests, and every defined full verification command. Agents report exact commands, outcomes, and exit statuses; a repository with no typecheck command produces an explicit Blocker backed by project-configuration evidence rather than an invented command or silent skip. - `buildPromptDisciplineBlock` — a shared, prohibition-form section appended exactly once to every built prompt (chat and sub-agent, every provider family). Primary vs worker wording differs for product writes: workers are told to use `read_file`/`edit_file`/`write_file`; Skywalker is told to DIY tiny/bounded edits with those path tools and spawn directors for substantial work. Shared rules: never `cat`/`sed`/heredoc/`echo` for file work, no setting or exporting environment variables (recurring needs belong in project settings), `web_fetch`/`web_search` instead of `curl`/`wget`/hand-rolled queries, one operation per `run_shell` call, turn semantics (a tool-less reply is the final answer, no repeat searches, stop and change approach after three failed attempts, batch independent reads in parallel), and TTY output rules (short bold headers, one-line bullets, backticks for paths/commands, no wide tables). **Provider-conditional residuals.** Per-family additions layer on top of the shared block via the same `ModelFamilyPolicy` mechanism the directors use (`src/subagent/provider-family.ts`, `src/agent/model-family-policy.ts`) — additive lines, never prompt forks. **Grok** leaves get `buildGrokLeafAntiThrashNote` (gated by `shouldApplyGrokAntiThrash` / `applyGrokFinishBias`, withheld from orchestrators): a compact finish-bias reinforcement plus a one-line reminder to route file/web work through the dedicated tools rather than `run_shell`, motivated by observed tool-routing thrash on the same harness. **Kimi** intentionally has no residual yet — `detectModelFamily` already resolves the family so callers can branch on it, but the prompt seam is left unfilled pending eval characterization of Kimi's behavior, mirroring the provisional (permissive-default) policy in `model-family-policy.ts`. diff --git a/docs/IMPLEMENTATION.md b/docs/IMPLEMENTATION.md index 8e65f2b85..2edcdfbc7 100644 --- a/docs/IMPLEMENTATION.md +++ b/docs/IMPLEMENTATION.md @@ -170,7 +170,7 @@ Twenty packages under `src/agent/directors//` register in `DIRECTOR_REGISTRY 6. There is no static write-path declaration on packages or profiles (CL-6952 removed it — no shipped director ever set one). Instead, `agent-fleet.ts` tracks each running dispatch by cwd; a new mutating dispatch that lands on the same cwd as a live mutating peer (`pending_init`/`running`, and not a declared read-only `modelRole` of `explore`/`plan`/`review`/`test`) records at most one `concurrent-lane-overlap` entry per cwd wave in `intervention-log.ts` (class `conflict`). The wave flag clears when no live mutating writer remains for that cwd. Terminal-but-unsettled lanes (for example cancelled with `finishedAt` set while the run promise has not reached `finally`) are pruned from the map and do not warn. This is advisory only — it never blocks the spawn, since cwd overlap does not prove the two lanes touch the same files. 7. Spawn effort: pin > package `modelRole` default (`defaultEffortForDirector`; intern=low; plan/review/orchestrator=high; implement/explore/docs/test=medium) > orchestrator/worker binary > parent inheritance. Optional skills are listed in the identity header for awareness; workers mount `skill_search` + `use_skill` scoped to the dispatch's `optionalSkills` and load bodies on demand (grok/kimi leaves omit `skill_search` and load brief-named skills straight through `use_skill`). Primary mounts `use_skill` for its own skill list. -Intent defaults: `intent=implement` → director `builder`; `explore` → `explorer`; `plan` → `counsel`; `review` → `critic`; general → error. Spawn: skywalker full fleet; all other directors, including greybeard, mount no fleet tools. Live `` injects cwd, platform, arch, runtime, date, and git status on every chat and worker prompt. +Intent defaults: `intent=implement` → director `builder`; `explore` → `explorer`; `plan` → `counsel`; `review` → `critic`; general → error. Spawn: skywalker full fleet; all other directors, including greybeard, mount no fleet tools. Skywalker assigns one focused task per worker; fan-out width follows independent lanes (one lane per PR/path/ownership). Live `` injects cwd, platform, arch, runtime, date, and git status on every chat and worker prompt. ### Auto Mode diff --git a/docs/PRODUCT.md b/docs/PRODUCT.md index 2f730bee8..49e8ec01e 100644 --- a/docs/PRODUCT.md +++ b/docs/PRODUCT.md @@ -166,7 +166,7 @@ The primary session is always **orchestrator** (single-agent mode is gone). Its There is **no catch-all worker**. `spawn_agent` requires `agent=…` or a non-general `intent` (implement/explore/plan/review→critic); bare dispatch and `intent=general` are refused. Named `spawn_agent(agent=…)` selects a director package without requiring a plugin profile, except `skywalker` which is the primary session identity and is refused as a spawned worker. Nested spawn is runtime-enforced: only skywalker (full fleet allowlist) may spawn; all other workers, including greybeard, have no fleet tools. Primary omits an allowlist so plugin profiles remain reachable from the main session. -Corbits Code fans work out to short-lived **fleet agents** — workers with their own loop, tools, and checklist — while the primary session stays focused. +Corbits Code fans work out to short-lived **fleet agents** — workers with their own loop, tools, and checklist — while the primary session stays focused. Each worker gets one focused task; fan-out width follows independent lanes (one lane per PR/path/ownership). - **Agents** are runtime entities (primary session or child). - **Tasks** are checklist items owned by one agent via `manage_tasks`. diff --git a/src/agent/directors/skywalker/package.test.ts b/src/agent/directors/skywalker/package.test.ts index 4c1098385..1312334f7 100644 --- a/src/agent/directors/skywalker/package.test.ts +++ b/src/agent/directors/skywalker/package.test.ts @@ -100,6 +100,9 @@ describe("skywalkerPackage", () => { expect(p).toContain("fan-out"); expect(p).toContain("0–1 worker"); expect(p).toContain("named, non-overlapping lanes"); + expect(p).toContain("one focused task"); + expect(p).toContain("one lane per PR/path/ownership"); + expect(p).toContain("Do not pack a multi-step workflow into one worker"); expect(p).not.toContain("2–4 workers"); expect(p).not.toContain("at most 4"); expect(p).not.toContain("Prefer synthesizing early returns"); @@ -221,6 +224,8 @@ describe("skywalkerPackage", () => { expect(p).toContain("clean-room"); expect(p).toContain("no fork"); expect(p).toContain("required for implement/review"); + expect(p).toContain("keep it tight"); + expect(p).toContain("One job per spawn"); // CL-6953 / CL-6807: single contract statement (spawn graph); the routing // and handoff restatements are gone. expect(p).not.toContain("Runtime requires success_criteria"); diff --git a/src/agent/directors/skywalker/package.ts b/src/agent/directors/skywalker/package.ts index dee45b471..aced09925 100644 --- a/src/agent/directors/skywalker/package.ts +++ b/src/agent/directors/skywalker/package.ts @@ -68,9 +68,10 @@ When the operator (or brief) gives an http(s) URL to read: # Effort scaling (IMPLEMENTATION / ORCHESTRATION) Scale fan-out to the ask: +- Each spawned worker gets **one focused task**. Do not pack a multi-step workflow into one worker. - Simple (answer, one-path lookup, tiny fix): 0–1 worker, few tools; often answer without fleet - Tiny single-file / one-route asks: **DIY on the parent** with write_file/edit_file; skip spawn, skip explorer, skip plan, skip critic. Do not always explorer→plan→implement→critic for simple work — that burns wall clock. -- Multi-lane work: spawn only named, non-overlapping lanes (distinct path/package/ownership). Width follows independent lanes. Do not invent a numeric cap. +- Multi-lane work: spawn only named, non-overlapping lanes (one lane per PR/path/ownership). Width follows independent lanes. Do not invent a numeric cap. # Anti-cascade (stall / dig / diagnose) @@ -115,7 +116,7 @@ Docs/design (PRODUCT.md, ARCHITECTURE.md, docs/design/*, brand) still spawn shak ## If ORCHESTRATION → coordinate -Track with manage_tasks. Parallelize independent lanes via spawn_agent, then idle. After each spawn wave, update the operator and end the turn. +Track with manage_tasks. One focused task per worker. Parallelize independent lanes (one lane per PR/path/ownership) via spawn_agent, then idle. After each spawn wave, update the operator and end the turn. ## If COMMUNICATION → answer directly @@ -137,12 +138,13 @@ Do not reclassify COMMUNICATION as ORCHESTRATION just to justify parallel spawn Skywalker = full closed set. Greybeard = limited spawn only (intern/explorer/critic) — not a second primary. You may spawn: builder, explorer, counsel, intern, critic, greybeard, neckbeard, bruckheimer, gaasbot, draper, emil, rand, shakespeare, testsmith, tester, gauntlet, prober, migrator, warden. -When spawning, pass a typed brief. success_criteria is required for implement/review and their default directors; recommended otherwise: +When spawning, pass a typed brief and keep it tight. success_criteria is required for implement/review and their default directors; recommended otherwise: - intent — explore | implement | plan | review - success_criteria — done-definition the worker must meet - do_not — hard constraints - report_focus — what the parent needs back - agent — specialist id when known (must match a closed director id above) +One job per spawn — do not stuff extra work into the prompt. # Report shape diff --git a/src/agent/prompts.test.ts b/src/agent/prompts.test.ts index a9061fa80..3f311556f 100644 --- a/src/agent/prompts.test.ts +++ b/src/agent/prompts.test.ts @@ -221,8 +221,8 @@ Scope and conventions: - If a required check genuinely cannot run because of a missing runtime or dependency, sandbox restriction, or permissions, record the exact inability under Blockers; never silently skip a required check. Orchestration: -- Break multi-step or parallel work into focused worker dispatches with distinct lenses; prefer \`spawn_agent\` (fire several in one turn when jobs are independent), then reply with who is running and end the turn — workers keep running while you are idle. Mailbox mail arrives as inbound when a worker finishes; read it and do not poll. \`list_agents\` shows the fleet without blocking; after a parked ask is surfaced, answer with \`send_input\` and do not poll \`list_agents\`. -- Pass the typed spawn contract: \`intent\`, \`success_criteria\` (done-when; required for implement/review and their default directors), \`do_not\` (scope fence), and \`report_focus\`. Free-form \`prompt\` without \`success_criteria\` fail-closes for implement/review and their default directors. +- One focused task per spawned worker. Fan-out width follows independent lanes (one lane per PR/path/ownership). Break multi-step or parallel work into those dispatches with distinct lenses; prefer \`spawn_agent\` (fire several in one turn when jobs are independent), then reply with who is running and end the turn — workers keep running while you are idle. Mailbox mail arrives as inbound when a worker finishes; read it and do not poll. \`list_agents\` shows the fleet without blocking; after a parked ask is surfaced, answer with \`send_input\` and do not poll \`list_agents\`. +- Pass the typed spawn contract and keep it tight: \`intent\`, \`success_criteria\` (done-when; required for implement/review and their default directors), \`do_not\` (scope fence), and \`report_focus\`. Free-form \`prompt\` without \`success_criteria\` fail-closes for implement/review and their default directors. - After workers return, classify fail / incomplete-report vs parent-initiated interrupt vs operator-cancel vs clean complete. Fail-path (\`status: failed\` or salvage \`incomplete-report\`): diagnose from the report or error and MAY spawn one successor with a changed brief. Parent-initiated interrupt (\`interrupt_agent\` / \`send_input\` with \`interrupt:true\` unblocks wait with \`stop_reason: interrupted\`): the worker is often still running and often has no report — \`resume_agent\`, or idle for its mailbox mail; do not \`spawn_agent\` a successor against a still-live worker. Successor only if that session is no longer resumable. Operator-cancel (\`stop_reason\` cancelled): wait for the operator; do not auto-retry. Identical brief: refuse. Merge Summary/Findings into a coherent answer for the operator; do not paste raw fleet-agent dumps. - Use manage_tasks for your own coordination checklist; spawning workers is \`spawn_agent\`, not manage_tasks. - If context is compacted automatically, do not stop tasks early due to token fear; persist progress via manage_tasks and worker reports.`); diff --git a/src/agent/prompts.ts b/src/agent/prompts.ts index ce81caeff..460b54bc9 100644 --- a/src/agent/prompts.ts +++ b/src/agent/prompts.ts @@ -207,12 +207,12 @@ const GUIDELINE_SUB_BLOCKS: Record< if (ctx.subAgent) return []; return [ "Orchestration:", - "- Break multi-step or parallel work into focused worker dispatches with distinct lenses; prefer `spawn_agent` (fire several in one turn when jobs are independent), then reply with who is running and end the turn — workers keep running while you are idle. " + + "- One focused task per spawned worker. Fan-out width follows independent lanes (one lane per PR/path/ownership). Break multi-step or parallel work into those dispatches with distinct lenses; prefer `spawn_agent` (fire several in one turn when jobs are independent), then reply with who is running and end the turn — workers keep running while you are idle. " + (ctx.waitAgentsMounted ? "This surface has no mailbox delivery: collect with `wait_agents`; do not poll `list_agents`." : "Mailbox mail arrives as inbound when a worker finishes; read it and do not poll.") + " `list_agents` shows the fleet without blocking; after a parked ask is surfaced, answer with `send_input` and do not poll `list_agents`.", - "- Pass the typed spawn contract: `intent`, `success_criteria` (done-when; required for implement/review and their default directors), `do_not` (scope fence), and `report_focus`. Free-form `prompt` without `success_criteria` fail-closes for implement/review and their default directors.", + "- Pass the typed spawn contract and keep it tight: `intent`, `success_criteria` (done-when; required for implement/review and their default directors), `do_not` (scope fence), and `report_focus`. Free-form `prompt` without `success_criteria` fail-closes for implement/review and their default directors.", "- After workers return, classify fail / incomplete-report vs parent-initiated interrupt vs operator-cancel vs clean complete. Fail-path (`status: failed` or salvage `incomplete-report`): diagnose from the report or error and MAY spawn one successor with a changed brief. Parent-initiated interrupt (`interrupt_agent` / `send_input` with `interrupt:true` unblocks wait with `stop_reason: interrupted`): the worker is often still running and often has no report — `resume_agent`" + (ctx.waitAgentsMounted ? " or re-wait" diff --git a/src/prompts.test.ts b/src/prompts.test.ts index 4d6e3512f..6a4417a39 100644 --- a/src/prompts.test.ts +++ b/src/prompts.test.ts @@ -173,6 +173,9 @@ test("orchestrator guidelines teach the typed task spawn contract", () => { expect(guidelines).toContain("report_focus"); expect(guidelines).toContain("intent"); expect(guidelines).toContain("spawn_agent"); + expect(guidelines).toContain("One focused task per spawned worker"); + expect(guidelines).toContain("one lane per PR/path/ownership"); + expect(guidelines).toContain("keep it tight"); // CL-7678: the default (TUI/nested) surface is unmounted — spawn then idle // on mailbox mail. The wait_agents collect path is exec-primary opt-in. expect(guidelines).toContain("mailbox mail arrives as inbound"); diff --git a/src/subagent/agent-fleet.ts b/src/subagent/agent-fleet.ts index 640b3bd58..50ce0feda 100644 --- a/src/subagent/agent-fleet.ts +++ b/src/subagent/agent-fleet.ts @@ -526,7 +526,7 @@ const SpawnAgentArgs = type({ export const spawnAgentToolDefinition: ToolDefinition = { name: SPAWN_AGENT_TOOL_NAME, description: - "Start a worker agent and return IMMEDIATELY with its agent_id — this never blocks on the worker's completion. Pass agent= a director/profile id returned by search_agents, or intent= (one of explore|implement|review|plan|general). The child starts blank. success_criteria is required for implement/review (and their default directors). Fire several spawn_agent calls in one turn to start workers in parallel, then reply and end the turn — workers keep running while you are idle. Reports arrive as mailbox mail where mailbox delivery is mounted; where wait_agents is mounted (exec primary), collect with it instead. Do not poll. Excess fan-out is queued rather than refused.", + "Start a worker agent and return IMMEDIATELY with its agent_id — this never blocks on the worker's completion. Pass agent= a director/profile id returned by search_agents, or intent= (one of explore|implement|review|plan|general). The child starts blank. One focused task per worker. success_criteria is required for implement/review (and their default directors). Fire several spawn_agent calls in one turn to start independent lanes in parallel, then reply and end the turn — workers keep running while you are idle. Reports arrive as mailbox mail where mailbox delivery is mounted; where wait_agents is mounted (exec primary), collect with it instead. Do not poll. Excess fan-out is queued rather than refused.", inputSchema: { type: "object", properties: {