feat(subagent): proactive specialist delegation + swarm execution fixes - #243
Conversation
Fixes: - Task tool dropped the parent's permission mode, so every delegated sub-agent ran at --auto high regardless of the parent. Forward PermissionMode so a child never exceeds its parent's authority. - Swarm members never executed: swarm_spawn accepts agent types teammate/subagent, but the launcher looked those up as specialist names (only worker/explorer/code-review exist), so every member failed with "specialist not found". Run each member from its swarm definition via an inline manifest on the executor. Enhancements: - Auto-delegation: the orchestrator system prompt now lists the available specialists and when to use each, nudging it to offload read-heavy work to a sub-agent so large tool output stays out of the main context. - @-mention: a leading "@<specialist> <task>" in the TUI composer delegates to that specialist (transcript keeps the verbatim mention); the @ palette suggests specialists on the first token, file references mid-message. - swarm.maxTeamSize config caps concurrent members per team (0 => default 8; negative rejected), honored in user and project config. - Proactive spawning: read-only specialists (explorer/code-review) auto- approve via a new args-aware tool permission (tools.ArgsPermissioner), so the agent delegates exploration off a normal prompt without an approval prompt; write-capable specialists, resume, and unknown names still prompt. The exec gate registers specialist tools at medium/high autonomy so a non-trivial headless run can auto-delegate read-only too.
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR adds specialist delegation across the full stack: a new ChangesSpecialist Delegation, Permission, and TUI Integration
Sequence Diagram(s)sequenceDiagram
rect rgba(70, 130, 180, 0.5)
Note over User,specialistExec: TUI specialist mention → agent delegation → read-only auto-approval
end
participant User
participant autocomplete as TUI autocomplete
participant launchPrompt as TUI launchPrompt
participant agentLoop as agent.Run / executeToolCall
participant TaskTool
participant specialistExec as specialist.Executor
User->>autocomplete: types "`@exp`"...
autocomplete->>autocomplete: leadingSpecialistSuggestions
autocomplete-->>User: overlay shows "`@explorer`"
User->>launchPrompt: submits "`@explorer` summarize the repo"
launchPrompt->>launchPrompt: expandSpecialistMention
launchPrompt->>agentLoop: Task-delegation directive
agentLoop->>agentLoop: effectivePermission(TaskTool, args)
agentLoop->>TaskTool: PermissionForArgs(args)
TaskTool->>specialistExec: IsReadOnlySpecialist("explorer")
specialistExec-->>TaskTool: true
TaskTool-->>agentLoop: PermissionAllow (auto-approved)
agentLoop->>specialistExec: executor.Run(TaskParameters{Manifest: inlineManifest})
specialistExec-->>agentLoop: specialist result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
Vasanthdev2004
left a comment
There was a problem hiding this comment.
APPROVE
Walked through the 21 files (852 +, 16 -) at 755bd7e4. The two fixes are correct, the four enhancements are well-scoped, and the test coverage closes the gaps that motivated the work. Two follow-ups for a later PR; no blockers.
Fixes
1. Task no longer over-grants the child's authority. internal/agent/loop.go:550 now routes through effectivePermission(tool, args) so the args-aware permission is consulted for the Task tool. internal/specialist/task_tool.go:114-122 propagates options.PermissionMode into TaskRunOptions, and internal/specialist/exec.go:specialistAutonomy clamps the child's --auto to the parent's resolved mode (unsafe/"" → high; any other mode → low). The comment block on BuildArgsInput.PermissionMode documents the historical bug clearly: "Dropping this made every Task sub-agent run at --auto high (unsafe) regardless of the parent." Verified.
2. Swarm members actually run. internal/swarm/launcher_specialist.go:30-58 now passes an inline Manifest to executor.Run (specialistManifestForMember) so the member runs from its swarm definition instead of failing a name lookup against the specialist registry. TestSpecialistLauncherRunsUnregisteredSwarmAgent exercises exactly the catch-22 the fix targets: a Load that returns no specialists, a RunChild that records its args, and an assertion that ran == true and the args include the member's inline system prompt. Verified.
Enhancements
3. Auto-delegation nudge in the system prompt. specialistDelegationContext in internal/agent/system_prompt.go:84-117 only renders when options.Specialists is non-empty, so a run with no delegatable specialists reproduces the previous prompt byte-for-byte (the comment is explicit). The nudge is well-scoped: "delegate read-heavy or parallelizable work" + one-line per-specialist WhenToUse description. No new policy, no new permission.
4. @<specialist> <task> TUI mention. expandSpecialistMention in internal/tui/autocomplete.go:401-428 does a case-insensitive EqualFold against the registered SpecialistInfo names, and only fires on a leading @token (the first word of the message) — mid-message @file references are untouched. The agent-facing expansion is "Use the Task tool to delegate this to the <name> specialist, then report its result back to me:\n\n"; the transcript keeps the verbatim @mention. TestSpecialistMention* covers the parsing edge cases. The suggestion overlay in recomputeSuggestions (autocomplete.go:86-100) routes leading @ to specialist completions and still routes mid-message @ to the file picker — the precedence is the right one.
5. swarm.maxTeamSize config. New SwarmConfig.MaxTeamSize (internal/config/types.go:103); Resolve rejects negative values with a clear error (resolver.go:60-62); both mergeConfig and mergeProjectConfig honor the latest non-zero setting. 0 maps to the swarm's built-in default of 8 inside registerSpecialistTools. Documented in the field comment.
6. ArgsPermissioner interface + read-only auto-approval. internal/tools/types.go:100-107 adds the interface; internal/agent/loop.go:effectivePermission is the single integration point (no other call sites needed changes). The fallback to tool.Safety().Permission for tools that don't implement it means the change is opt-in across the whole tool surface — blast radius is exactly Task, and a tool without the interface is byte-identical to before. The read-only check in internal/specialist/exec.go:manifestIsReadOnly requires every resolved tool to be in the hardcoded readOnlySpecialistTools set (read_file, list_directory, grep, glob, update_plan) — fail-closed for anything else, so a custom read-only tool that isn't on the list still gets the prompt. The exec gate in cli/exec.go:172-176 re-resolves Swarm.MaxTeamSize so --list-tools stays offline. The resume and unknown name paths in PermissionForArgs always fall through to PermissionPrompt (tested in TestTaskToolPermissionForArgs).
Test coverage (8 new files, all checked)
effective_permission_test.go— interface presence vs. fallback behaviorspecialist_delegation_test.go— orchestrator prompt section renderingspecialist_summaries_test.go—SpecialistInfoshape from the runtimeswarm_config_test.go—Resolverejects negativemaxTeamSize, honors last non-zero in mergeauto_approve_test.go—IsReadOnlySpecialistfor read-only / write-capable / unknown / resume / malformedpermission_inline_test.go— inline manifestValidateand swarm launcher wiringlauncher_specialist_test.go— the swarm-catch-22 fix end-to-endspecialist_mention_test.go—@<name> <task>parsing + mid-message@file-routing stays intact
Non-blocking follow-ups
-
Default
--auto highfoot-gun whenPermissionModeis unset.specialistAutonomypreserves the historical""→highmapping for the empty case (the comment is honest about this), but the historical behavior is exactly the bug the rest of this PR fixes. Worth either returning an error when a fresh specialist run is dispatched without aPermissionMode, or defaulting tolowand letting an unsafe orchestrator explicitly opt in. As written, a caller that forgets to wiretools.RunOptions.PermissionModesilently re-introduces the privilege escalation. -
@<name>is case-insensitive, but the suggestion overlay's match isn't case-preserving. When a user types@EXPLORERand the autocomplete inserts, the inserted token uses the matched name's case (strings.TrimSpace(spec.Name)) — that's fine. Worth a one-liner in the test for the case-mismatch path to lock the behavior in. -
@<specialist>mid-message is intentionally NOT routed to a specialist. A user typing "ask @explorer to look at this" sees a file picker instead of a specialist mention. That's the right call (avoiding false-positive specialist routing) but worth a sentence inAGENTS.mdor the autocomplete doc comment so users don't try it and be confused.
This is the right shape. The two fixes fix real bugs, the four enhancements are narrowly scoped, and the test surface is complete. Happy to see it land.
|
Verdict: APPROVE. Walked through the 21 files (852 +, 16 -) at Fixes
Enhancements
Test coverage (8 new files, all checked)
Non-blocking follow-ups
This is the right shape. Happy to see it land. |
anandh8x
left a comment
There was a problem hiding this comment.
Low: swarm.maxTeamSize cannot be reset to the default once an earlier config layer sets it nonzero.
internal/config/resolver.go:182 only merges the field when the source value is nonzero, but 0 is the documented use-default value. A later config file cannot intentionally clear an inherited limit back to default. This is non-blocking for the delegation changes, but it is worth tracking if layered config resets are expected.
Verification: diff review plus git diff --check passed for this PR.
specialistAutonomy mapped an empty PermissionMode to "--auto high" (unsafe), silently re-introducing the privilege escalation the rest of this PR fixes: an orchestrator that forgets to wire PermissionMode would get an unsafe child. The Task tool and swarm both now propagate a resolved mode, so "" only occurs for a caller that omitted it -- default that case to "low". Only an explicit unsafe parent now yields --auto high; everything else (incl. unset/unknown) is fail-safe low. Updates the stale "back-compat" comments and the tests that documented the old behavior. Addresses the non-blocking follow-up from PR review.
Resolve the internal/tui/model.go struct-field conflict by keeping BOTH the @specialist suggestion field (suggestionsAreSpecialists, this branch) and main's mouseReleased field (from the merged image-input PR). All other files auto-merged.
Summary
Two fixes and four enhancements to the sub-agent (specialist) + swarm subsystem. An audit found the swarm was exposed but non-functional and the
Tasktool over-granted authority; the enhancements make the orchestrator delegate to specialists proactively and safely.Fixes
Taskdropped the parent's permission mode — every delegated sub-agent ran at--auto highregardless of the parent. Now forwardsPermissionMode, so a child never exceeds its parent's authority.swarm_spawnaccepts agent typesteammate/subagent, but the launcher looked those up as specialist names (onlyworker/explorer/code-reviewexist), so every member failed withspecialist "…" not found. Members now run from their swarm definition via an inline manifest on the executor.Enhancements
@-mention — a leading@<specialist> <task>in the TUI composer delegates to that specialist (the transcript keeps the verbatim mention); the@palette suggests specialists on the first token, while file references mid-message are unchanged.swarm.maxTeamSizeconfig — caps concurrent members per team (0⇒ default 8; negative rejected), honored in user and project config.explorer/code-review) auto-approve via a new opt-in args-aware permission (tools.ArgsPermissioner), so the agent delegates exploration off a normal prompt without an approval prompt; write-capable specialists,resume, and unknown names still prompt. The exec gate registers specialist tools atmedium/highautonomy so a non-trivial headless run can auto-delegate read-only too. Blast radius of the permission change is theTasktool only — every other tool falls back to its static permission unchanged.Tests
8 new test files: permission-mode propagation, inline-manifest execution, the swarm launcher running an unregistered agent type, the delegation prompt section, specialist summaries,
@-mention parsing + palette,swarm.maxTeamSizeresolution, read-only detection + args-awareTaskpermission, and theeffectivePermissionfallback (a tool without the interface is unchanged).Verification
gofmt ·
go vet ./...·go build ./...clean; package suites pass under-race. Functionally verified on a live run: swarm members spawn and return results; read-only specialists auto-spawn at--auto medium(no unsafe flag) while write-capable ones stay gated and don't run.Summary by CodeRabbit
Release Notes
New Features
@specialistsyntax to delegate tasks.Improvements