Add sub-agents: definitions, Spawn, Budget, planning pass, and the reviewer agent - #7
Open
mchalapuk wants to merge 21 commits into
Open
Add sub-agents: definitions, Spawn, Budget, planning pass, and the reviewer agent#7mchalapuk wants to merge 21 commits into
mchalapuk wants to merge 21 commits into
Conversation
Introduces Definition: a struct holding an agent's identity, tool
subset, protocol, loop, permission-mode narrowing, and iteration cap,
replacing the hardcoded wiring cmd/coded and cmd/promptdump each did by
hand. Narrowing is structural rather than validated: ResolveTools can
only intersect a parent's tool list and Tighten can only move toward a
stricter permission.Mode, so widening is inexpressible in the type
rather than merely rejected by a check.
New() picks the concrete agent type from Definition.Loop -- LoopEnforced
for protocol.Agent, LoopBare for the plain agent.Agent loop -- so
agent.Agent.Run has a real, tested caller instead of being orphaned by
protocol.Agent's enforcement. NewProtocolAgent is the common-case
wrapper for callers that need the concrete *protocol.Agent type.
Registry.Subset lets a Definition scope a tool.Registry down to a named
list, erroring on an unknown name rather than silently dropping it.
Both cmd/coded and cmd/promptdump now build the main agent via
definition.Get("main")/definition.Main instead of react.New directly;
the regenerated prompts/agent-run.txt diff is empty, confirming the
rewire changed no behavior.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018MJUwgEVzvnCmJwjbFw3sx
New internal/agent/explorer package: a response-format contract distinct
from react's, built for a sub-agent whose only output the parent ever
sees is its own <result> section. A single <search> preamble (what's
being looked for, where it's already been checked) precedes the
trailing <findings> section, which the identity prompt asks to carry
file:line pointers with one-line claims plus an explicit "searched for
and did not find" list -- the negative half that stops a parent from
repeating a search that already came up empty.
Explorer is the first shipped protocol with a single required
non-result section, which surfaced a singular/plural bug in
protocol.requiredSentence ("must open with one tagged blocks", "Always
include all one") that react's two-section case never exercised. Fixed
in prompt.go; react's pinned prompt-text tests (n=2, unaffected by the
fix) still pass unchanged.
Registers Explorer in definition.Builtins(): Tools narrowed to
Read/Grep/Glob, PermissionMode tightened to ModePlan as a second
backstop behind the tool restriction, MaxIterations lower than Main's
default, SeedReadme true so it gets the synthetic README read main gets
today. Definition.SeedReadme is new: a per-definition flag Spawn will
consult once it exists, not yet wired into any live path since nothing
spawns Explorer in this milestone slice.
prompts/agent-run.txt regenerated with an empty diff: main's protocol
still has two required sections, so the prompt.go fix doesn't touch it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018MJUwgEVzvnCmJwjbFw3sx
… ends agent.Event gains Path []string, identifying which agent in the tree produced an event: nil for the root agent's own events (every event a bare Agent.Run/protocol.Agent.Run call produces today), or the chain of agent names from outermost to innermost for a spawned sub-agent's. Stamped in exactly one place -- ForwardChild, a small loop that drains a child's event channel and relays each event to a parent's with the child's name prepended to whatever Path it already carries. Because every event a child (or anything it spawns) ever produces already funnels through its own single out channel by construction, this one forwarding point attributes all of it -- no per-emission-site bookkeeping needed anywhere in agent.go or protocol.Agent.run. EventAgentStarted/EventAgentComplete bracket a sub-agent's run, carrying its name and a one-line prompt summary; nothing emits them yet since Spawn (which will) doesn't exist until the next slice of this milestone, but ForwardChild and the event shape are exercised directly by their own tests. Both front ends render the dimension: oneshot.Render indents a sub-agent's lines via a small prefixWriter that inserts the current event's margin after each newline (state persists across the whole Render call, since which margin applies to a line is decided by whichever event started it). The TUI's handleAgentEvent branches on a non-empty Path to a new handleSubAgentEvent, indenting text and tool lines and bracketing the run with "-> name: summary" / "<- name: done" system-styled lines -- deliberately not routed through renderAssistantBlock, which parses raw text against the *root* agent's protocol; a sub-agent's text is tagged under its own, different protocol (e.g. explorer's <search>/<findings>), so reusing that parser would misrender it. This is the flat, ungrouped first cut described in PLAN.md; a collapsible per-agent group is follow-up work. prompts/agent-run.txt regenerated with an empty diff. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MJUwgEVzvnCmJwjbFw3sx
…t tree Budget bounds provider round-trips and tokens with a single, mutex- protected pool shared by a root agent and every sub-agent it spawns (Sub() hands out the same instance, not a fresh allocation) -- fixing a gap djinni documented and never closed: no cost ceiling anywhere a tree of chats could hit. Every method is nil-safe, so an Agent's unset Budget field (the default) is unlimited with no guard needed at any call site. StreamTurn records one round-trip per successful Provider.Stream call and accumulates tokens from each EventUsage as it streams by -- recording lives at the one place that actually makes a provider call, not scattered across both turn loops. Both agent.Agent.run and protocol.Agent.run check Exhausted() at the top of each iteration, before spending another round-trip, and halt with EventError wrapping the new ErrBudgetExhausted -- distinct from a generic provider failure, and a clean halt at an iteration boundary, so conversation state stays exactly as resumable as the existing max-iterations halt. The TUI's usage banner gains a per-agent breakdown: m.usageByAgent, keyed by Event.Path joined with "/", fed by a new EventUsage case in handleSubAgentEvent that closes a real gap the flat-indentation cut left open (a sub-agent's usage had no case to land in and was silently dropped from every total). formatUsageBreakdown stays silent until a second contributing agent actually appears, so an ordinary session with no sub-agent activity sees no change in the banner it already prints. prompts/agent-run.txt regenerated with an empty diff. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MJUwgEVzvnCmJwjbFw3sx
Records the decision behind Event.Path and ForwardChild: one flat, single channel stays the only path to a front end even once a turn can spawn sub-agents, tagged rather than multiplexed, because tagging composes for free through nested spawns and multiplexing doesn't. Cross-links with the ui-agnostic-agent-loop record it extends. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MJUwgEVzvnCmJwjbFw3sx
Spawn (internal/tool/builtin/spawn) invokes a named agent definition with a prompt, runs it to completion in an isolated conversation, and returns its final answer -- bounded, and extracted from its own result-kind section rather than raw text -- as the tool result. This is the mechanism the rest of this milestone's plumbing (Event.Path, ForwardChild, Budget, agent definitions) existed to support; it's the first thing that actually fires it. New tool.EventReportingTool/tool.Reporter (internal/tool/reporter.go): an optional capability interface, type-asserted in agent.DispatchToolCalls the same way SteppedTool already is, letting a tool stream intermediate agent.Event values during its own Execute without the tool package importing agent.Event and creating an import cycle -- Reporter.Report takes `any`; agent.reporterAdapter is the concrete bridge. ForwardChild's signature changed from a channel to a sink func(Event) so Spawn can hand it a Reporter's Report method directly. Spawn is deliberately unpermissioned at the call site -- the reporting case in DispatchToolCalls skips permission.Check entirely -- since every action the child takes is still checked individually as it happens; a prompt on the Spawn call itself would just be a second, redundant ask for the same decision. definition.NewChild resolves the child's Tools against its parent's actual registry (never in isolation, so a grandchild can't gain anything even transitively) and shares Budget via Budget.Sub. Permission-mode narrowing switches the one shared engine for the synchronous duration of the child's run and restores it after -- safe only because v0.2 spawns are sequential, called out explicitly as a concurrency caveat for later. A spawned child gets its own nested session (session.Session.NewChild: new ParentID/ChildIDs fields), so `coded sessions list` keeps showing real conversations while a full replay can still walk into a sub-agent's own transcript via its parent's ChildIDs. definition.Spawnable() is the one list of valid Spawn targets (today: just Explorer -- "main" is deliberately never offered, even though narrowing makes it harmless), shared by cmd/coded's tool-registry wiring and the TUI's new /spawn <agent> <prompt> command, which runs spawn.Tool directly against the same Provider/Tools/Permission/Session the model-invoked path uses, so a hand-triggered spawn and a model-triggered one produce identical event streams. prompts/agent-run.txt regenerated with an empty diff: cmd/promptdump's own registry doesn't include Spawn yet, so main's dumped prompt is unaffected until that's extended (tracked separately). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MJUwgEVzvnCmJwjbFw3sx
…nComplete/EventError) New Outcome/TurnResult (internal/agent/outcome.go): six values -- completed, abandoned, cancelled, exhausted, errored, incomplete -- matching PLAN.md's Turn outcomes table. Only completed/cancelled/ exhausted/errored are producible by any turn loop today; abandoned and incomplete need a goal/todo list to be abandoned or left open on, which doesn't exist until the planning-pass milestone, and are documented as such rather than left silently unreachable. EventTurnComplete and EventError both retire in favor of one EventTurnEnded carrying Result *TurnResult -- not a new EventTurnFailed sitting beside the old success event, since "every turn ends with exactly one outcome" only holds if exactly one event says so. Err stays on Event (still used by EventRetrying too) rather than moving into TurnResult, so the classified-error rendering both front ends already had needed no new plumbing, just a different event to read it off of. OutcomeForError classifies a terminal error once, shared by both turn loops: a cancelled context is OutcomeCancelled, ErrBudgetExhausted or the new ErrMaxIterationsExceeded is OutcomeExhausted, anything else is OutcomeErrored -- the same class of failure EventError used to signal on its own. Both agent.Agent.run and protocol.Agent.run migrate their handful of terminal emit sites to this classification; StreamTurn and DispatchToolCalls needed no changes, since the classification happens where the error is already handled today. Both front ends updated: oneshot.Render and the TUI's handleAgentEvent/ handleSubAgentEvent collapse their old EventTurnComplete/EventError cases into one EventTurnEnded case each, rendering a completed outcome silently (as before) and any other outcome through the same classified cause + next-step text the old EventError case produced -- describeOutcome/ describeTurnOutcome are near-identical package-local twins, not shared, since one writes to an io.Writer and the other builds a transcript string. prompts/agent-run.txt regenerated with an empty diff. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MJUwgEVzvnCmJwjbFw3sx
oneshot.TurnError wraps a non-completed root turn's Outcome alongside its underlying error (nil for OutcomeAbandoned/OutcomeIncomplete, which state a Reason rather than carry one) and Render now returns it instead of a bare error -- existing error/Unwrap callers see no difference, but cmd/coded's exitCodeFor can check Outcome before falling back to classifying *llm.Error, since OutcomeAbandoned has nothing for that classifier to work with. New exitAbandoned (4) is wired and tested directly even though nothing produces OutcomeAbandoned yet, so the planning-pass milestone lands exit-code-complete on day one. Same commit fixes a real bug the Spawn work left open: oneshot.Render's EventTurnEnded case never checked Event.Path, so a sub-agent's own non-completed outcome (forwarded from its own turn loop) would overwrite the root run's error and flip the process exit code even though the root turn went on to complete normally after reacting to the failure. Only a Path-less (root) EventTurnEnded may now set Render's returned error; a sub-agent's is rendered informationally and nothing more. agent.Agent.LastOutcome carries the most recent Run call's own TurnResult into the next one: previousOutcomeBlock renders a short note when it wasn't OutcomeCompleted, injected via the same per-request, never-stored-in-history path ProjectContext already uses (ordinary Volatile-marked text, so it stays out of the cached prompt prefix). A new Agent.EndTurn method is the single place every turn loop's exit path now goes through, so LastOutcome can never drift out of sync with what was actually emitted on the event stream. A clean finish injects nothing, which is what keeps this silent (and free) for the overwhelming majority of turns. prompts/agent-run.txt regenerated with an empty diff. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MJUwgEVzvnCmJwjbFw3sx
Situation (user-input, tool-result) is what a turn loop's current iteration is -- the input a Pass's When gate will fire against. SituationForIteration encodes the invariant that makes it safe to gate a whole extra call on user-input alone: it fires exactly once, at iter == 0, which holds only because an Ask answer returns to the loop as a tool result rather than a new user-input situation (see its own doc comment for what would silently break if that ever changed). Scheme/Pass describe an agent's turn as an ordered list of passes, each with its own Protocol, an optional When situation gate, and a Visibility (public output crosses back to the caller; private stays in the transcript but out of any later pass's or the caller's request history -- the same exclusion mechanism Agent.ExcludeMessages already uses for a resolved violation episode, applied here by construction). Default(p) is the single-pass scheme equivalent to running p directly -- exactly every agent's behavior today. Deliberately data only, matching the precedent SeedReadme/Loop set: no turn loop executes a Scheme yet. Per-situation sections on a single Protocol (a second, separate mechanism PLAN.md also describes, for varying one pass's own required tags by situation at zero extra calls) is left for whenever it has a real consumer -- the planning pass this was built toward gates at the Pass level instead, which needs nothing more than what's here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MJUwgEVzvnCmJwjbFw3sx
Records why ResolveTools/Tighten make widening structurally inexpressible rather than a validated-and-rejected case: an agent definition arrives with the code, not with per-line review the way a runtime permission prompt gets, and PLAN.md commits to loading the same Definition shape from files a future markdown loader will read. The guarantee needs to survive that loader landing without anyone remembering to re-check it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MJUwgEVzvnCmJwjbFw3sx
dumpAgent generalizes the single hardcoded main-agent dump into a
helper taking a Definition, a scripted turn sequence, and a prompt --
run once for definition.Main (unchanged output, still agent-run.txt)
and once for definition.Explorer (new: explorer-run.txt). The
explorer script uses its own <search>/<findings> tags rather than
react's, and its one real Grep call runs against the seeded workspace
for real -- the scripted final answer ("no backoff-related code
exists here") is the same conclusion a real explorer would reach from
that Grep's real "no matches" output, not just plausible-sounding
fake text.
CODED.md's build docs and layout section updated from "regenerates
prompts/agent-run.txt" to "one file per shipped agent definition",
matching what cmd/promptdump now actually does.
prompts/agent-run.txt regenerated with an empty diff -- main's own
dump is unaffected by explorer's addition.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018MJUwgEVzvnCmJwjbFw3sx
… all turn New internal/agent/planning package: a goal/todo/delegate protocol, deliberately declaring no KindResult section since it's never run through protocol.NewAgent's Validate-enforcing loop -- it's parsed directly from one best-effort completion instead (see protocol.Agent.runPlanningPass's own doc comment for the full reasoning, including why this is a smaller commitment than generalizing the existing violation-retry loop to a second protocol). protocol.Agent gains an optional Planning field: nil by default, so every existing definition (main, explorer) is byte-for-byte unaffected -- confirmed by the full existing test suite passing unchanged and both prompts/*.txt dumps regenerating with an empty diff. When set, runPlanningPass fires once at the start of a turn, appends its exchange to history for the transcript's sake, then immediately excludes it from future requests via the same Agent.ExcludeMessages mechanism a resolved protocol-violation episode already uses -- the main protocol has no idea how to read <goal>/<todo>/<delegate> tags and must never see them again. PlanState (goal, todo items, delegation reasoning) is turn state, injected into every later request the same way ProjectContext already is: per-request, Volatile, never stored in a.messages. ParseTodoItems assigns todo items stable IDs by matching text against the previous parse -- unchanged text keeps its ID (and Done state) across a re-parse, new text gets the next unused one -- so a later reference to "item 3" keeps meaning the same thing. Scoped deliberately smaller than PLAN.md's full description: goal is not yet hard-enforced with reject-and-retry (a malformed planning response just leaves the turn unplanned rather than costing a second call), and there is no live in-band amendment protocol for the main loop to mark a todo item done mid-turn -- both would require changing react's own protocol, not just adding an optional side-pass, and are left as explicitly-flagged follow-up rather than half-built here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MJUwgEVzvnCmJwjbFw3sx
…pleted FinalOutcome(natural, hasOpenItems) downgrades a natural OutcomeCompleted to OutcomeIncomplete when hasOpenItems is true, and passes every other outcome through unchanged -- it only ever downgrades a claimed clean finish, never relabels a real failure. This is PLAN.md's stated exception: "a final answer with open items requires a declared outcome... undeclared is incomplete, and it's a protocol violation, not a circumstance." Deliberately not called from Agent.run: doing so today would mark every planned turn Incomplete the moment its todo list has more than one item, since there's no in-band way yet for the main loop to mark an item Done (see the planning pass's own prior commit on why live amendment needs a change to react's own protocol and was explicitly deferred). The function is correct and tested in isolation, ready for whatever calls it once that mechanism exists. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MJUwgEVzvnCmJwjbFw3sx
mchalapuk
force-pushed
the
feat/sub-agents
branch
from
August 8, 2026 03:37
4ee8540 to
9829a1b
Compare
An agent already knows its own name and depth at construction time (definition.NewChild builds the full "parent/child" chain), so it can stamp every event it emits once in Run instead of each hop rewriting Path on the way back up through ForwardChild. The root is now named "main" explicitly rather than relying on a nil/empty special case. Caller identity (needed by Spawn to name its child) threads through context, since tool.Registry.Subset shares Tool instances by reference across the whole agent tree and Spawn itself can't hold per-caller state. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QAu6x8HmMWrhjGTZjs7yEF
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QAu6x8HmMWrhjGTZjs7yEF
…mode A per-definition permission mode only guards one authoring surface while .coded/settings.json sets the mode outright at higher precedence, and bracketing the engine's mode around a spawn is sound only while spawns run sequentially. Permission mode belongs to the session instead: one value, changed only by explicit user input, shared by every agent in the tree. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QAu6x8HmMWrhjGTZjs7yEF
Spawn no longer tightens the shared engine's mode before a child's run and restores it after. That bracket made the mode in force depend on where in the tree execution was, sound only while spawns run sequentially, and it encoded a reluctance to prompt on a sub-agent's behalf that the singleton- permission-mode ADR now rejects: under a manual mode the user wants to be asked about what a sub-agent does exactly as about the main agent. TestSpawnDoesNotSwitchModeDuringChildRun replaces TestSpawnRestoresPermissionModeAfterRun, asserting the mode never moves while a child is suspended on a permission request rather than only that it's back afterward. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QAu6x8HmMWrhjGTZjs7yEF
A Definition declares no permission posture. Permission mode belongs to the session's shared engine, written only by explicit user input, never narrowed per agent (see the singleton-permission-mode ADR). Explorer's read-only tool list -- Read, Grep, Glob -- was already the whole mechanism keeping it read-only; TestBuiltinsRegistersExplorer now asserts that exact allowlist instead of the absence of three known-mutating names, since it's the entire guard now rather than a belt-and-braces backstop. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QAu6x8HmMWrhjGTZjs7yEF
Behavior is unchanged; only the doc comments are, since the record that
justified them ("an agent file arrives with the code, not with the user's
approval") is deleted. A child's tool list composes against what its
parent actually has -- naming a tool the parent lacks is a no-op -- stated
as plain composition rather than as a defense against an untrusted file.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QAu6x8HmMWrhjGTZjs7yEF
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QAu6x8HmMWrhjGTZjs7yEF
…ng it previousOutcomeBlock was appended as a per-request Volatile block via withProjectContext, so it got re-sent on every iteration of the turn that followed an exhausted/errored one, each time glued to that iteration's tool_result message rather than the user input it actually describes. Only the first pass had it in the right place. NewTurnMessage now builds the turn-opening user message with the note as a second, non-volatile content block, written once into history by both turn loops (agent.Agent.run and protocol.Agent.run) -- matching the "an injection is not a place to put state" rule RequestMessagesParts already documents for everything else. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MLVQ2C52q7WWQTG1zEkqdh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
internal/agent/definition), including the explorer sub-agent, and aSpawntool so the harness can actually spawn sub-agentsBudgetto cap round-trips/tokens across an agent treeEventTurnEnded, maps outcomes to-pexit codes, and feeds the previous outcome forwardFinalOutcome(a final answer with open items can't read as Completed)Test plan
go test ./...Spawnand confirm budget enforcement across the agent tree🤖 Generated with Claude Code
https://claude.ai/code/session_013DayimvWNv8zHZVMEUPqSX