Merge pull request #4 from dj-oyu/fix/orchestration-spawn-adoption

feat(orch): orchestration spawn adoption and sandbox improvements
This commit is contained in:
dj-oyu 2026-03-01 03:59:03 +09:00 committed by GitHub
commit 7fffdb0ba0
22 changed files with 1218 additions and 184 deletions

View file

@ -19,28 +19,50 @@ import (
const orchestrationGuidance = `## Orchestration const orchestrationGuidance = `## Orchestration
You are the conductor, not the performer. Prefer delegation over doing everything inline. You are the conductor, not the performer. **Your primary job is to delegate, not to implement.**
Use **spawn** (non-blocking) when: ### spawn (non-blocking) DEFAULT choice
- Tasks can run in parallel or in the background Returns immediately. Use for any task that can run independently.
- Multiple independent tasks can run simultaneously spawn each one Call the spawn tool with JSON arguments like this:
- You don't need the result to decide the next step
- The operation is long-running (builds, fetches, analysis, file processing)
Use **subagent** (blocking) when: Tool: spawn
- You need the result before you can continue Arguments: {"task": "Examine pkg/auth/ and report middleware pattern", "preset": "scout", "label": "auth-scout"}
- Correctness of the next step depends on the outcome
Do inline only when: Tool: spawn
- It's a single fast tool call (read a file, quick search) Arguments: {"task": "Implement rate limiter in pkg/ratelimit/ with tests", "preset": "coder", "label": "rate-limiter"}
- Delegation overhead clearly outweighs the benefit
Default bias: if a task involves more than 2-3 tool calls or can run independently, delegate it. ### subagent (blocking) only when you need the answer NOW
When you spawn, immediately plan what comes next blocking means you've stopped thinking. Blocks until the subagent finishes. Use only when you cannot proceed without the result.
Fork aggressively: explore multiple directions simultaneously. Does not take a preset it runs with default tools.
Tool: subagent
Arguments: {"task": "Read pkg/config/config.go and list all SubagentsConfig fields", "label": "config-check"}
### When to use which
- spawn: parallel tasks, independent work, implementation, long analysis, >2 tool calls
- subagent: you need the result before your next decision
- inline: single quick tool call where delegation overhead is wasteful
### Presets (for spawn only)
| preset | role | can write | can exec |
|--------|------|-----------|----------|
| scout | explore, investigate | no | no |
| analyst | analyze, run tests | no | go test/vet, git |
| coder | implement + verify | yes (sandbox) | test/lint/fmt |
| worker | build + install | yes (sandbox) | build/package mgr |
| coordinator | orchestrate others | yes (sandbox) | general + spawn |
### Parallel spawning
Spawn multiple independent tasks at once do NOT wait between them:
Tool: spawn
Arguments: {"task": "Analyze error handling patterns in pkg/providers/", "preset": "analyst", "label": "error-patterns"}
Tool: spawn
Arguments: {"task": "List all HTTP endpoints in pkg/miniapp/", "preset": "scout", "label": "endpoints"}
After spawning, record the assignment in ## Orchestration > Delegated in MEMORY.md. After spawning, record the assignment in ## Orchestration > Delegated in MEMORY.md.
When results come back, synthesize and decide the next fork.` When results come back, synthesize findings and decide the next fork.`
type ContextBuilder struct { type ContextBuilder struct {
workspace string workspace string
@ -123,9 +145,23 @@ func (cb *ContextBuilder) getIdentity() string {
` `
} }
// Conditional identity and plan executing rule for orchestration mode
identity := "a helpful AI assistant"
executingRule := `Work through the current Phase's steps.
Mark each "- [x]" via edit_file. The system will auto-advance phases.`
if cb.orchestrationEnabled {
identity = "a conductor AI agent that orchestrates subagents"
executingRule = `Delegate the current Phase's steps to subagents using spawn.
For each step: spawn a subagent with the appropriate preset (scout for investigation,
coder for implementation, analyst for review). Spawn multiple independent steps in parallel.
When a subagent completes, mark "- [x]" via edit_file and record findings in
## Orchestration > Findings in MEMORY.md.
Only do a step inline if it's a single quick tool call (e.g., reading one file).`
}
return fmt.Sprintf(prompt+`# picoclaw 🦞 return fmt.Sprintf(prompt+`# picoclaw 🦞
You are picoclaw, a helpful AI assistant. You are picoclaw, %s.
## Workspace ## Workspace
Your workspace is at: %s Your workspace is at: %s
@ -148,8 +184,7 @@ Your workspace is at: %s
After each answer, use edit_file to save findings to ## Context in memory/MEMORY.md. After each answer, use edit_file to save findings to ## Context in memory/MEMORY.md.
When you have enough information, add ## Phase sections with "- [ ]" checkbox steps, and ## Commands section below the header. Then change > Status: to "review". When you have enough information, add ## Phase sections with "- [ ]" checkbox steps, and ## Commands section below the header. Then change > Status: to "review".
- If Status is "review": The plan is awaiting user approval. Do NOT change Status yourself. - If Status is "review": The plan is awaiting user approval. Do NOT change Status yourself.
- If Status is "executing": Work through the current Phase's steps. - If Status is "executing": %s
Mark each "- [x]" via edit_file. The system will auto-advance phases.
- Plan format (header is written by the system do NOT delete it): - Plan format (header is written by the system do NOT delete it):
# Active Plan # Active Plan
> Task: <description> > Task: <description>
@ -176,7 +211,7 @@ Your workspace is at: %s
- For architecture/flow, use arrow text: CLI Pipeline Adapters - For architecture/flow, use arrow text: CLI Pipeline Adapters
5. **Context summaries** - Conversation summaries provided as context are approximate references only. They may be incomplete or outdated. Always defer to explicit user instructions over summary content.`, 5. **Context summaries** - Conversation summaries provided as context are approximate references only. They may be incomplete or outdated. Always defer to explicit user instructions over summary content.`,
workspacePath, workspacePath, workspacePath, workspacePath, toolsSection) identity, workspacePath, workspacePath, workspacePath, workspacePath, toolsSection, executingRule)
} }
func (cb *ContextBuilder) buildToolsSection() string { func (cb *ContextBuilder) buildToolsSection() string {

View file

@ -115,6 +115,7 @@ type processOptions struct {
NoHistory bool // If true, don't load session history (for heartbeat) NoHistory bool // If true, don't load session history (for heartbeat)
TaskID string // Unique task ID for background task status tracking TaskID string // Unique task ID for background task status tracking
Background bool // If true, this is a background task (cron/heartbeat) — enables live task notifications Background bool // If true, this is a background task (cron/heartbeat) — enables live task notifications
SystemMessage bool // If true, this is a system message (subagent result) — skip placeholder and plan nudge
} }
const defaultResponse = "I've completed processing but have no response to give. Increase `max_tool_iterations` in config.json." const defaultResponse = "I've completed processing but have no response to give. Increase `max_tool_iterations` in config.json."
@ -302,6 +303,9 @@ func registerSharedTools(
return registry.CanSpawnSubagent(currentAgentID, targetAgentID) return registry.CanSpawnSubagent(currentAgentID, targetAgentID)
}) })
agent.Tools.Register(spawnTool) agent.Tools.Register(spawnTool)
// Register blocking subagent tool alongside spawn
subagentTool := tools.NewSubagentTool(subagentManager)
agent.Tools.Register(subagentTool)
} }
// Update context builder with the complete tools registry // Update context builder with the complete tools registry
@ -796,24 +800,92 @@ func (al *AgentLoop) processSystemMessage(ctx context.Context, msg bus.InboundMe
return "", nil return "", nil
} }
// Use default agent for system messages // Inject subagent result into session history without running a full LLM loop.
// The conductor will see the result on its next turn. This avoids:
// - Flooding the chat with a response for every subagent completion
// - Consuming the Telegram "Thinking..." placeholder
// - Wasting LLM tokens on processing each result individually
agent := al.registry.GetDefaultAgent() agent := al.registry.GetDefaultAgent()
if agent == nil { if agent == nil {
return "", fmt.Errorf("no default agent for system message") return "", fmt.Errorf("no default agent for system message")
} }
// Use the origin session for context
sessionKey := routing.BuildAgentMainSessionKey(agent.ID) sessionKey := routing.BuildAgentMainSessionKey(agent.ID)
historyMsg := fmt.Sprintf("[System: %s] %s", msg.SenderID, msg.Content)
agent.Sessions.AddMessage(sessionKey, "user", historyMsg)
agent.Sessions.MarkDirty(sessionKey)
return al.runAgentLoop(ctx, agent, processOptions{ // Send a brief notification (SkipPlaceholder to avoid corrupting status messages)
SessionKey: sessionKey, label := msg.SenderID
if idx := strings.LastIndex(label, ":"); idx >= 0 {
label = label[idx+1:]
}
notification := formatSubagentCompletion(label, msg.Metadata)
_ = al.bus.PublishOutbound(ctx, bus.OutboundMessage{
Channel: originChannel, Channel: originChannel,
ChatID: originChatID, ChatID: originChatID,
UserMessage: fmt.Sprintf("[System: %s] %s", msg.SenderID, msg.Content), Content: notification,
DefaultResponse: "Background task completed.", SkipPlaceholder: true,
EnableSummary: false,
SendResponse: true,
}) })
logger.InfoCF("agent", "Subagent result injected into session history",
map[string]any{
"sender_id": msg.SenderID,
"session_key": sessionKey,
"content_len": len(content),
})
return "", nil
}
// formatSubagentCompletion builds the user-facing notification for a completed subagent.
// If metadata contains duration_ms and tool_calls it produces e.g.:
//
// "📋 scout-1 completed (3.2s, 5 tool calls)."
//
// Without metadata it falls back to the plain "📋 scout-1 completed." format.
func formatSubagentCompletion(label string, metadata map[string]string) string {
if len(metadata) == 0 {
return fmt.Sprintf("📋 %s completed.", label)
}
durationMs, _ := strconv.ParseInt(metadata["duration_ms"], 10, 64)
toolCalls, _ := strconv.Atoi(metadata["tool_calls"])
if durationMs <= 0 && toolCalls <= 0 {
return fmt.Sprintf("📋 %s completed.", label)
}
parts := make([]string, 0, 2)
if durationMs > 0 {
parts = append(parts, formatDurationMs(durationMs))
}
if toolCalls > 0 {
if toolCalls == 1 {
parts = append(parts, "1 tool call")
} else {
parts = append(parts, fmt.Sprintf("%d tool calls", toolCalls))
}
}
return fmt.Sprintf("📋 %s completed (%s).", label, strings.Join(parts, ", "))
}
// formatDurationMs converts milliseconds to a human-readable duration string.
// Examples: 800 → "0.8s", 1200 → "1.2s", 65000 → "1m5s", 3661000 → "61m1s".
func formatDurationMs(ms int64) string {
if ms < 1000 {
return fmt.Sprintf("%dms", ms)
}
totalSec := ms / 1000
if totalSec < 60 {
tenths := (ms % 1000) / 100
return fmt.Sprintf("%d.%ds", totalSec, tenths)
}
mins := totalSec / 60
sec := totalSec % 60
if sec == 0 {
return fmt.Sprintf("%dm", mins)
}
return fmt.Sprintf("%dm%ds", mins, sec)
} }
// acquireSessionLock gets or creates a per-session semaphore and acquires it. // acquireSessionLock gets or creates a per-session semaphore and acquires it.
@ -1066,16 +1138,49 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
al.promptDirty.Store(false) al.promptDirty.Store(false)
} }
// 5. Run LLM iteration loop // 5. Run LLM iteration loop (with automatic phase transitions)
finalContent, iteration, err := al.runLLMIteration(ctx, agent, messages, opts, task, preStatus) var finalContent string
var iteration int
const maxPhaseTransitions = 10
for phaseLoop := 0; ; phaseLoop++ {
// On phase transition: rebuild system prompt with new phase context + nudge
if phaseLoop > 0 {
messages = agent.ContextBuilder.BuildMessages(
agent.Sessions.GetHistory(opts.SessionKey),
agent.Sessions.GetSummary(opts.SessionKey),
"", nil, opts.Channel, opts.ChatID,
)
messages = append(messages, providers.Message{
Role: "user",
Content: fmt.Sprintf(
"[System] Phase %d is now active. Continue working on the next steps.",
agent.ContextBuilder.GetCurrentPhase(),
),
})
if len(messages) > 0 {
al.lastSystemPrompt.Store(messages[0].Content)
}
}
curPlanStatus := preStatus
if phaseLoop > 0 {
curPlanStatus = agent.ContextBuilder.GetPlanStatus()
}
var err error
finalContent, iteration, err = al.runLLMIteration(ctx, agent, messages, opts, task, curPlanStatus)
if err != nil { if err != nil {
return "", err return "", err
} }
// 5a. Auto-advance plan phases after LLM iteration // 5a. Auto-advance plan phases after LLM iteration
postStatus := agent.ContextBuilder.GetPlanStatus() postStatus := agent.ContextBuilder.GetPlanStatus()
if agent.ContextBuilder.HasActivePlan() && if !agent.ContextBuilder.HasActivePlan() ||
(postStatus == "executing" || postStatus == "review" || postStatus == "completed") { !(postStatus == "executing" || postStatus == "review" || postStatus == "completed") {
break
}
// Intercept: if AI changed status to executing or review without user approval // Intercept: if AI changed status to executing or review without user approval
// (from interviewing or review), validate and hold at "review". // (from interviewing or review), validate and hold at "review".
if preStatus == "interviewing" || (preStatus == "review" && postStatus == "executing") { if preStatus == "interviewing" || (preStatus == "review" && postStatus == "executing") {
@ -1083,11 +1188,11 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
_ = agent.ContextBuilder.SetPlanStatus("interviewing") _ = agent.ContextBuilder.SetPlanStatus("interviewing")
logger.WarnCF("agent", "Reverted plan to interviewing: "+err.Error(), logger.WarnCF("agent", "Reverted plan to interviewing: "+err.Error(),
map[string]any{"agent_id": agent.ID}) map[string]any{"agent_id": agent.ID})
// Inject rejection into session history so LLM sees it next iteration
rejectionMsg := "[System] Plan rejected: " + err.Error() + ". Fix and try again." rejectionMsg := "[System] Plan rejected: " + err.Error() + ". Fix and try again."
agent.Sessions.AddMessage(opts.SessionKey, "user", rejectionMsg) agent.Sessions.AddMessage(opts.SessionKey, "user", rejectionMsg)
} else { } else {
_ = agent.ContextBuilder.SetPlanStatus("review") _ = agent.ContextBuilder.SetPlanStatus("review")
al.reporter().ReportStateChange(opts.SessionKey, orch.AgentStatePlanReview, "")
if !constants.IsInternalChannel(opts.Channel) { if !constants.IsInternalChannel(opts.Channel) {
planDisplay := agent.ContextBuilder.FormatPlanDisplay() planDisplay := agent.ContextBuilder.FormatPlanDisplay()
_ = al.bus.PublishOutbound(ctx, bus.OutboundMessage{ _ = al.bus.PublishOutbound(ctx, bus.OutboundMessage{
@ -1098,17 +1203,22 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
}) })
} }
} }
} else if postStatus == "executing" && agent.ContextBuilder.GetTotalPhases() == 0 { break
// Safeguard: executing but no phases (shouldn't happen, but be safe). }
if postStatus == "executing" && agent.ContextBuilder.GetTotalPhases() == 0 {
_ = agent.ContextBuilder.SetPlanStatus("interviewing") _ = agent.ContextBuilder.SetPlanStatus("interviewing")
logger.WarnCF("agent", "Reverted plan to interviewing: no phases defined", logger.WarnCF("agent", "Reverted plan to interviewing: no phases defined",
map[string]any{"agent_id": agent.ID}) map[string]any{"agent_id": agent.ID})
} else if agent.ContextBuilder.IsPlanComplete() { break
// Mark plan as completed (keep memory for review; user can /plan clear) }
if agent.ContextBuilder.IsPlanComplete() {
total := agent.ContextBuilder.GetTotalPhases() total := agent.ContextBuilder.GetTotalPhases()
_ = agent.ContextBuilder.SetCurrentPhase(total) _ = agent.ContextBuilder.SetCurrentPhase(total)
if preStatus != "completed" { if preStatus != "completed" {
_ = agent.ContextBuilder.SetPlanStatus("completed") _ = agent.ContextBuilder.SetPlanStatus("completed")
al.reporter().ReportStateChange(opts.SessionKey, orch.AgentStatePlanCompleted, "")
// Deactivate worktree on plan completion // Deactivate worktree on plan completion
commitMsg := "plan: " + agent.ContextBuilder.Memory().GetPlanTaskName() commitMsg := "plan: " + agent.ContextBuilder.Memory().GetPlanTaskName()
@ -1128,7 +1238,15 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
}) })
} }
} }
} else if agent.ContextBuilder.IsCurrentPhaseComplete() { break
}
if agent.ContextBuilder.IsCurrentPhaseComplete() {
if phaseLoop >= maxPhaseTransitions {
logger.WarnCF("agent", "Max phase transitions reached, stopping",
map[string]any{"agent_id": agent.ID, "transitions": phaseLoop})
break
}
prev := agent.ContextBuilder.GetCurrentPhase() prev := agent.ContextBuilder.GetCurrentPhase()
_ = agent.ContextBuilder.AdvancePhase() _ = agent.ContextBuilder.AdvancePhase()
next := agent.ContextBuilder.GetCurrentPhase() next := agent.ContextBuilder.GetCurrentPhase()
@ -1140,7 +1258,11 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
SkipPlaceholder: true, SkipPlaceholder: true,
}) })
} }
al.notifyStateChange()
continue
} }
break
} }
al.notifyStateChange() al.notifyStateChange()
@ -1179,6 +1301,7 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
Channel: opts.Channel, Channel: opts.Channel,
ChatID: opts.ChatID, ChatID: opts.ChatID,
Content: finalContent, Content: finalContent,
SkipPlaceholder: opts.SystemMessage, // suppress Telegram "Thinking..." for system messages
}) })
} }
@ -1259,6 +1382,24 @@ func buildPlanReminder(planStatus string) (providers.Message, bool) {
return providers.Message{Role: "user", Content: content}, true return providers.Message{Role: "user", Content: content}, true
} }
// buildOrchReminder returns a reminder to use spawn/subagent during plan execution.
// Fires on first iteration and every 3rd iteration to reinforce delegation behavior.
func buildOrchReminder(iteration int) (providers.Message, bool) {
if iteration != 1 && iteration%3 != 0 {
return providers.Message{}, false
}
content := `[System] ORCHESTRATION mode active. You MUST delegate plan steps to subagents.
Use spawn (non-blocking, returns immediately) or subagent (blocking, waits for result).
Do NOT implement steps inline unless they are a single trivial tool call.
To delegate, call the tool with JSON arguments:
Tool: spawn Arguments: {"task": "...", "preset": "scout", "label": "..."}
Tool: subagent Arguments: {"task": "...", "label": "..."}
Spawn multiple independent steps in parallel for maximum throughput.`
return providers.Message{Role: "user", Content: content}, true
}
// cdPrefixPattern matches "cd /some/path && " at the start of a shell command. // cdPrefixPattern matches "cd /some/path && " at the start of a shell command.
// Group 1 captures the target directory path. // Group 1 captures the target directory path.
var cdPrefixPattern = regexp.MustCompile(`^cd\s+(\S+)\s*&&\s*`) var cdPrefixPattern = regexp.MustCompile(`^cd\s+(\S+)\s*&&\s*`)
@ -2055,7 +2196,7 @@ func (al *AgentLoop) runLLMIteration(
} }
// Report waiting state to canvas before each LLM call. // Report waiting state to canvas before each LLM call.
al.reporter().ReportStateChange(opts.SessionKey, "waiting", "") al.reporter().ReportStateChange(opts.SessionKey, orch.AgentStateWaiting, "")
// Retry loop for context/token errors // Retry loop for context/token errors
maxRetries := 2 maxRetries := 2
@ -2447,7 +2588,7 @@ func (al *AgentLoop) runLLMIteration(
} }
// Report toolcall state to canvas. // Report toolcall state to canvas.
al.reporter().ReportStateChange(opts.SessionKey, "toolcall", tc.Name) al.reporter().ReportStateChange(opts.SessionKey, orch.AgentStateToolCall, tc.Name)
toolStart := time.Now() toolStart := time.Now()
toolCtx := ctx toolCtx := ctx
@ -2594,6 +2735,18 @@ func (al *AgentLoop) runLLMIteration(
} }
} }
// Inject orchestration nudge during plan execution to encourage spawn usage.
if planSnapshot == "executing" && agent.Subagents != nil && agent.Subagents.Enabled {
if reminder, ok := buildOrchReminder(iteration); ok {
messages = append(messages, reminder)
logger.DebugCF("agent", "Injected orchestration nudge",
map[string]any{
"agent_id": agent.ID,
"iteration": iteration,
})
}
}
// Refresh system prompt: tool execution may have changed workDir, // Refresh system prompt: tool execution may have changed workDir,
// memory, plan status, etc. Update messages[0] so the next LLM // memory, plan status, etc. Update messages[0] so the next LLM
// call sees the current state. // call sees the current state.
@ -3335,6 +3488,7 @@ func (al *AgentLoop) handlePlanCommand(args []string, sessionKey string) (string
if err := agent.ContextBuilder.SetPlanStatus("executing"); err != nil { if err := agent.ContextBuilder.SetPlanStatus("executing"); err != nil {
return fmt.Sprintf("Error: %v", err), true return fmt.Sprintf("Error: %v", err), true
} }
al.reporter().ReportStateChange(sessionKey, orch.AgentStatePlanExecuting, "")
al.planStartPending = true al.planStartPending = true
clearHistory := len(args) > 1 && args[1] == "clear" clearHistory := len(args) > 1 && args[1] == "clear"
al.planClearHistory = clearHistory al.planClearHistory = clearHistory

View file

@ -2991,3 +2991,91 @@ func TestHandleReasoning(t *testing.T) {
} }
}) })
} }
func TestFormatDurationMs(t *testing.T) {
tests := []struct {
ms int64
want string
}{
{0, "0ms"},
{500, "500ms"},
{999, "999ms"},
{1000, "1.0s"},
{1200, "1.2s"},
{3500, "3.5s"},
{59900, "59.9s"},
{60000, "1m"},
{61000, "1m1s"},
{65000, "1m5s"},
{120000, "2m"},
{3661000, "61m1s"},
}
for _, tt := range tests {
t.Run(fmt.Sprintf("%dms", tt.ms), func(t *testing.T) {
got := formatDurationMs(tt.ms)
if got != tt.want {
t.Errorf("formatDurationMs(%d) = %q, want %q", tt.ms, got, tt.want)
}
})
}
}
func TestFormatSubagentCompletion(t *testing.T) {
tests := []struct {
name string
label string
metadata map[string]string
want string
}{
{
"no metadata",
"scout-1",
nil,
"📋 scout-1 completed.",
},
{
"empty metadata",
"scout-1",
map[string]string{},
"📋 scout-1 completed.",
},
{
"duration and tool calls",
"scout-1",
map[string]string{"duration_ms": "3200", "tool_calls": "5"},
"📋 scout-1 completed (3.2s, 5 tool calls).",
},
{
"single tool call",
"coder-1",
map[string]string{"duration_ms": "1200", "tool_calls": "1"},
"📋 coder-1 completed (1.2s, 1 tool call).",
},
{
"duration only",
"scout-2",
map[string]string{"duration_ms": "65000", "tool_calls": "0"},
"📋 scout-2 completed (1m5s).",
},
{
"tool calls only",
"scout-3",
map[string]string{"duration_ms": "0", "tool_calls": "10"},
"📋 scout-3 completed (10 tool calls).",
},
{
"zero everything",
"scout-4",
map[string]string{"duration_ms": "0", "tool_calls": "0"},
"📋 scout-4 completed.",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := formatSubagentCompletion(tt.label, tt.metadata)
if got != tt.want {
t.Errorf("formatSubagentCompletion(%q, %v) = %q, want %q", tt.label, tt.metadata, got, tt.want)
}
})
}
}

View file

@ -630,6 +630,14 @@ func (ms *MemoryStore) getPlanContextFrom(content string) string {
sb.WriteString("\n") sb.WriteString("\n")
} }
// Orchestration section: conductor's delegation tracking (Delegated/Findings/Decisions)
orchContent := ms.extractSection(content, "Orchestration")
if orchContent != "" {
sb.WriteString("### Orchestration\n")
sb.WriteString(orchContent)
sb.WriteString("\n")
}
return sb.String() return sb.String()
} }

View file

@ -884,11 +884,16 @@
<div class="orch-badge-label">CNDR</div> <div class="orch-badge-label">CNDR</div>
<div class="orch-badge-dot"></div> <div class="orch-badge-dot"></div>
</div> </div>
<div class="orch-badge alive" id="orch-badge-secretary"> <div class="orch-badge" id="orch-badge-secretary">
<div class="orch-badge-emoji">👩‍💼</div> <div class="orch-badge-emoji">👩‍💼</div>
<div class="orch-badge-label">SEC</div> <div class="orch-badge-label">SEC</div>
<div class="orch-badge-dot"></div> <div class="orch-badge-dot"></div>
</div> </div>
<div class="orch-badge alive" id="orch-badge-heartbeat">
<div class="orch-badge-emoji">🕊️</div>
<div class="orch-badge-label">HB</div>
<div class="orch-badge-dot"></div>
</div>
</div> </div>
<div class="orch-canvas-wrap"> <div class="orch-canvas-wrap">
<canvas id="orch-canvas" width="320" height="320"></canvas> <canvas id="orch-canvas" width="320" height="320"></canvas>
@ -1810,7 +1815,7 @@ var _orchLastTs = null;
var _orchBOB = [0, -1, -2, -1]; var _orchBOB = [0, -1, -2, -1];
var _orchFRAME_MS = {idle:450, waiting:650, toolcall:90, talking:280, entering:220, exiting:220}; var _orchFRAME_MS = {idle:450, waiting:650, toolcall:90, talking:280, entering:220, exiting:220};
var _orchWALK = 55; var _orchWALK = 55;
var _orchConductor, _orchSecretary, _orchSubagents, _orchSlots, _orchFreeSlots; var _orchConductor, _orchSecretary, _orchHeartbeat, _orchSubagents, _orchSlots, _orchFreeSlots;
function _orchMakeChar(id, emoji, home) { function _orchMakeChar(id, emoji, home) {
return {id:id, emoji:emoji, x:home.x, y:home.y, home:home, target:null, state:'idle', return {id:id, emoji:emoji, x:home.x, y:home.y, home:home, target:null, state:'idle',
@ -1819,7 +1824,10 @@ function _orchMakeChar(id, emoji, home) {
function _orchInitChars() { function _orchInitChars() {
_orchConductor = _orchMakeChar('conductor', '👑', MAP_POSITIONS.conductor); _orchConductor = _orchMakeChar('conductor', '👑', MAP_POSITIONS.conductor);
_orchSecretary = _orchMakeChar('secretary', '👩‍💼', MAP_POSITIONS.secretary); _orchSecretary = _orchMakeChar('secretary', '👩‍💼', MAP_POSITIONS.secretary);
_orchConductor.alive = true; _orchSecretary.alive = true; _orchHeartbeat = _orchMakeChar('heartbeat', '🕊️', MAP_POSITIONS.heartbeat || {x:230,y:58});
_orchConductor.alive = true; _orchSecretary.alive = false; _orchHeartbeat.alive = true;
_orchConductor.statusText = null;
_orchHeartbeat.facing = 1; _orchHeartbeat.flipTimer = 0;
var ps = [{id:'s0',emoji:'🔍'},{id:'s1',emoji:'📊'},{id:'s2',emoji:'💻'}, var ps = [{id:'s0',emoji:'🔍'},{id:'s1',emoji:'📊'},{id:'s2',emoji:'💻'},
{id:'s3',emoji:'🔧'},{id:'s4',emoji:'🎯'}]; {id:'s3',emoji:'🔧'},{id:'s4',emoji:'🎯'}];
_orchSubagents = ps.map(function(p,i){ _orchSubagents = ps.map(function(p,i){
@ -1828,7 +1836,7 @@ function _orchInitChars() {
}); });
_orchSlots = {}; _orchFreeSlots = _orchSubagents.slice(); _orchSlots = {}; _orchFreeSlots = _orchSubagents.slice();
} }
function _orchAllChars() { return [_orchConductor, _orchSecretary].concat(_orchSubagents); } function _orchAllChars() { return [_orchConductor, _orchSecretary, _orchHeartbeat].concat(_orchSubagents); }
function _orchSyncBadge(id, state, alive) { function _orchSyncBadge(id, state, alive) {
var el = document.getElementById('orch-badge-' + id); if (!el) return; var el = document.getElementById('orch-badge-' + id); if (!el) return;
@ -1838,12 +1846,30 @@ function _orchSyncBadge(id, state, alive) {
+ (state==='toolcall'? ' toolcall' : '') + (state==='toolcall'? ' toolcall' : '')
+ (state==='waiting' ? ' waiting' : ''); + (state==='waiting' ? ' waiting' : '');
} }
function _orchSetState(c, state) { c.state=state; _orchSyncBadge(c.id, state, c.alive); } function _orchSetState(c, state, tool) {
c.state=state; _orchSyncBadge(c.id, state, c.alive);
if (c === _orchConductor) {
if (state==='waiting') c.statusText='🤔';
else if (state==='toolcall') c.statusText='⌨';
else if (state==='user_waiting') c.statusText='⏳';
else if (state==='plan_interviewing') c.statusText='📋';
else if (state==='plan_review') c.statusText='🔍';
else if (state==='plan_executing') c.statusText='▶️';
else if (state==='plan_completed') c.statusText='✅';
else c.statusText=null;
// Secretary appears only during plan mode.
var inPlan = state.indexOf('plan_')===0;
if (_orchSecretary.alive !== inPlan) {
_orchSecretary.alive = inPlan;
_orchSyncBadge('secretary', _orchSecretary.state, _orchSecretary.alive);
}
}
}
function _orchMoveTo(c, pos, cb) { c.target=pos; c._onArrive=cb||null; } function _orchMoveTo(c, pos, cb) { c.target=pos; c._onArrive=cb||null; }
function _orchSay(c, text, ttl) { c.bubble={text:text, ttl:ttl||2200}; } function _orchSay(c, text, ttl) { c.bubble={text:text, ttl:ttl||2200}; }
function _orchCharForId(id) { function _orchCharForId(id) {
if (id === 'heartbeat') return _orchSecretary; if (id === 'heartbeat') return _orchHeartbeat;
if (_orchSlots[id]) return _orchSlots[id]; if (_orchSlots[id]) return _orchSlots[id];
return _orchConductor; return _orchConductor;
} }
@ -1865,7 +1891,16 @@ function _orchGC(id) {
_orchSetState(c,'exiting'); _orchSetState(c,'exiting');
_orchMoveTo(c, MAP_POSITIONS.door, function(){ c.alive=false; _orchSetState(c,'idle'); }); _orchMoveTo(c, MAP_POSITIONS.door, function(){ c.alive=false; _orchSetState(c,'idle'); });
} else { } else {
var ch=_orchCharForId(id); ch.alive=false; _orchSetState(ch,'idle'); var ch=_orchCharForId(id);
if (ch === _orchHeartbeat) {
// Heartbeat pigeon is permanent — keep alive, just return to idle.
_orchSetState(ch,'idle');
} else if (ch === _orchConductor) {
// Conductor is permanent — keep alive, show ⏳ waiting for user.
_orchSetState(ch,'user_waiting');
} else {
ch.alive=false; _orchSetState(ch,'idle');
}
} }
} }
function _orchConverse(fromId, toId, text) { function _orchConverse(fromId, toId, text) {
@ -1885,17 +1920,45 @@ function _orchConverse(fromId, toId, text) {
function _orchUpdate(dt) { function _orchUpdate(dt) {
_orchAllChars().forEach(function(c){ _orchAllChars().forEach(function(c){
if (!c.alive && c.state!=='entering') return; if (!c.alive && c.state!=='entering') return;
// Frame animation (bob): pigeon uses state-specific timing instead of shared table.
if (c === _orchHeartbeat) {
if (c.state === 'idle') {
c.frame = 0; // pin still — no bob when inactive
} else {
c.frameTimer += dt;
var pDur = c.state==='toolcall' ? 130 : 380;
if (c.frameTimer >= pDur) { c.frame=(c.frame+1)%4; c.frameTimer-=pDur; }
}
} else {
c.frameTimer+=dt; c.frameTimer+=dt;
var dur=_orchFRAME_MS[c.state]||450; var dur=_orchFRAME_MS[c.state]||450;
if (c.frameTimer>=dur){ c.frame=(c.frame+1)%4; c.frameTimer-=dur; } if (c.frameTimer>=dur){ c.frame=(c.frame+1)%4; c.frameTimer-=dur; }
}
if (c.target){ if (c.target){
var dx=c.target.x-c.x, dy=c.target.y-c.y, dist=Math.sqrt(dx*dx+dy*dy); var dx=c.target.x-c.x, dy=c.target.y-c.y, dist=Math.sqrt(dx*dx+dy*dy);
if (dist>1.5){ var spd=_orchWALK*dt/1000; c.x+=dx/dist*spd; c.y+=dy/dist*spd; } if (dist>1.5){ var spd=_orchWALK*dt/1000; c.x+=dx/dist*spd; c.y+=dy/dist*spd; }
else { c.x=c.target.x; c.y=c.target.y; c.target=null; if(c._onArrive){c._onArrive();c._onArrive=null;} } else { c.x=c.target.x; c.y=c.target.y; c.target=null; if(c._onArrive){c._onArrive();c._onArrive=null;} }
} }
if (c.bubble){ c.bubble.ttl-=dt; if(c.bubble.ttl<=0) c.bubble=null; } if (c.bubble){ c.bubble.ttl-=dt; if(c.bubble.ttl<=0) c.bubble=null; }
// Heartbeat pigeon: direction flip rate reflects activity level.
if (c === _orchHeartbeat) {
if (c.target) {
var pdx = c.target.x - c.x;
if (Math.abs(pdx) > 1) c.facing = pdx > 0 ? 1 : -1;
} else {
var flipRate = c.state==='toolcall' ? 280 : c.state==='waiting' ? 600 : 2800;
c.flipTimer += dt;
if (c.flipTimer >= flipRate) { c.flipTimer -= flipRate; c.facing = -c.facing; }
}
}
}); });
} }
function _orchDrawStatus(c) {
if (!c.statusText) return;
var yOff=_orchBOB[c.frame], cx=Math.floor(c.x), cy=Math.floor(c.y+yOff)-20;
orchCtx.font='11px serif'; orchCtx.textAlign='center'; orchCtx.textBaseline='middle';
orchCtx.fillText(c.statusText, cx, cy);
}
function _orchDrawBubble(c) { function _orchDrawBubble(c) {
if (!c.bubble) return; if (!c.bubble) return;
var yOff=_orchBOB[c.frame], bx=c.x, by=c.y+yOff-18; var yOff=_orchBOB[c.frame], bx=c.x, by=c.y+yOff-18;
@ -1917,12 +1980,26 @@ function _orchDrawChar(c) {
} else if (c.state==='waiting'){ } else if (c.state==='waiting'){
orchCtx.fillStyle='rgba(96,165,250,0.25)'; orchCtx.beginPath(); orchCtx.fillStyle='rgba(96,165,250,0.25)'; orchCtx.beginPath();
orchCtx.arc(cx,cy,11,0,Math.PI*2); orchCtx.fill(); orchCtx.arc(cx,cy,11,0,Math.PI*2); orchCtx.fill();
} else if (c.state==='user_waiting' || c.state==='plan_review'){
orchCtx.fillStyle='rgba(167,139,250,0.18)'; orchCtx.beginPath();
orchCtx.arc(cx,cy,10,0,Math.PI*2); orchCtx.fill();
} else if (c.state==='plan_executing'){
orchCtx.fillStyle='rgba(74,222,128,0.18)'; orchCtx.beginPath();
orchCtx.arc(cx,cy,10,0,Math.PI*2); orchCtx.fill();
} }
orchCtx.font='18px serif'; orchCtx.textAlign='center'; orchCtx.textBaseline='middle'; orchCtx.font='18px serif'; orchCtx.textAlign='center'; orchCtx.textBaseline='middle';
if (c.facing === -1) {
orchCtx.save();
orchCtx.translate(cx, cy); orchCtx.scale(-1, 1);
orchCtx.fillText(c.emoji, 0, 0);
orchCtx.restore();
} else {
orchCtx.fillText(c.emoji, cx, cy); orchCtx.fillText(c.emoji, cx, cy);
}
orchCtx.font='6px Silkscreen,monospace'; orchCtx.textAlign='center'; orchCtx.textBaseline='top'; orchCtx.font='6px Silkscreen,monospace'; orchCtx.textAlign='center'; orchCtx.textBaseline='top';
orchCtx.fillStyle=c.state==='talking'?'#facc15':'#3a4a7a'; orchCtx.fillStyle=c.state==='talking'?'#facc15':'#3a4a7a';
orchCtx.fillText(c.id.toUpperCase(), cx, cy+11); orchCtx.fillText(c.id.toUpperCase(), cx, cy+11);
_orchDrawStatus(c);
_orchDrawBubble(c); _orchDrawBubble(c);
} }
function _orchRender(ts) { function _orchRender(ts) {
@ -1964,7 +2041,7 @@ function connectOrchWs() {
} else if (msg.type==='event') { } else if (msg.type==='event') {
var ev=msg.event||{}; var ev=msg.event||{};
if (ev.type==='agent_spawn') _orchSpawn(ev.id); if (ev.type==='agent_spawn') _orchSpawn(ev.id);
if (ev.type==='agent_state') { var c=_orchCharForId(ev.id); if(c) _orchSetState(c,ev.state); } if (ev.type==='agent_state') { var c=_orchCharForId(ev.id); if(c) _orchSetState(c,ev.state,ev.tool); }
if (ev.type==='agent_gc') _orchGC(ev.id); if (ev.type==='agent_gc') _orchGC(ev.id);
if (ev.type==='conversation') _orchConverse(ev.from, ev.to, ev.text); if (ev.type==='conversation') _orchConverse(ev.from, ev.to, ev.text);
} }

View file

@ -27,6 +27,7 @@ var MAP_POSITIONS = {
door: { x: 160, y: 314 }, // entry / exit point door: { x: 160, y: 314 }, // entry / exit point
conductor: { x: 160, y: 58 }, conductor: { x: 160, y: 58 },
secretary: { x: 108, y: 58 }, secretary: { x: 108, y: 58 },
heartbeat: { x: 230, y: 58 }, // pigeon messenger — periodic heartbeat agent
meeting: { x: 160, y: 161 }, // neutral zone for conversations meeting: { x: 160, y: 161 }, // neutral zone for conversations
stations: [ stations: [
{ x: 40, y: 106 }, // S0 scout { x: 40, y: 106 }, // S0 scout

View file

@ -89,8 +89,8 @@ func (b *Broadcaster) ReportSpawn(id, label, task string) {
} }
// ReportStateChange implements AgentReporter. // ReportStateChange implements AgentReporter.
func (b *Broadcaster) ReportStateChange(id, state, tool string) { func (b *Broadcaster) ReportStateChange(id string, state AgentState, tool string) {
b.Publish(Event{Type: "agent_state", ID: id, State: state, Tool: tool}) b.Publish(Event{Type: "agent_state", ID: id, State: string(state), Tool: tool})
} }
// ReportConversation implements AgentReporter. // ReportConversation implements AgentReporter.

View file

@ -4,7 +4,7 @@ package orch
// Both Broadcaster (real events) and noopReporter (disabled) implement this. // Both Broadcaster (real events) and noopReporter (disabled) implement this.
type AgentReporter interface { type AgentReporter interface {
ReportSpawn(id, label, task string) ReportSpawn(id, label, task string)
ReportStateChange(id, state, tool string) ReportStateChange(id string, state AgentState, tool string)
ReportConversation(from, to, text string) ReportConversation(from, to, text string)
ReportGC(id, reason string) ReportGC(id, reason string)
} }
@ -12,7 +12,7 @@ type AgentReporter interface {
type noopReporter struct{} type noopReporter struct{}
func (n *noopReporter) ReportSpawn(id, label, task string) {} func (n *noopReporter) ReportSpawn(id, label, task string) {}
func (n *noopReporter) ReportStateChange(id, state, tool string) {} func (n *noopReporter) ReportStateChange(id string, state AgentState, tool string) {}
func (n *noopReporter) ReportConversation(from, to, text string) {} func (n *noopReporter) ReportConversation(from, to, text string) {}
func (n *noopReporter) ReportGC(id, reason string) {} func (n *noopReporter) ReportGC(id, reason string) {}

View file

@ -10,8 +10,8 @@ var _ AgentReporter = (*Broadcaster)(nil)
// orchestration mode. // orchestration mode.
func TestNoop_AllMethods_NoPanic(t *testing.T) { func TestNoop_AllMethods_NoPanic(t *testing.T) {
Noop.ReportSpawn("id", "label", "task") Noop.ReportSpawn("id", "label", "task")
Noop.ReportStateChange("id", "waiting", "") Noop.ReportStateChange("id", AgentStateWaiting, "")
Noop.ReportStateChange("id", "toolcall", "bash") Noop.ReportStateChange("id", AgentStateToolCall, "bash")
Noop.ReportConversation("conductor", "sub-1", "do something") Noop.ReportConversation("conductor", "sub-1", "do something")
Noop.ReportGC("id", "completed") Noop.ReportGC("id", "completed")
} }
@ -49,9 +49,9 @@ func TestBroadcaster_ReportStateChange_MapsToAgentStateEvent(t *testing.T) {
b.ReportSpawn("agent-1", "coder", "implement it") b.ReportSpawn("agent-1", "coder", "implement it")
<-sub.Ch // consume spawn <-sub.Ch // consume spawn
b.ReportStateChange("agent-1", "toolcall", "bash") b.ReportStateChange("agent-1", AgentStateToolCall, "bash")
ev := <-sub.Ch ev := <-sub.Ch
if ev.Type != "agent_state" || ev.State != "toolcall" || ev.Tool != "bash" { if ev.Type != "agent_state" || ev.State != string(AgentStateToolCall) || ev.Tool != "bash" {
t.Fatalf("unexpected event: %+v", ev) t.Fatalf("unexpected event: %+v", ev)
} }
snap := b.Snapshot() snap := b.Snapshot()

31
pkg/orch/state.go Normal file
View file

@ -0,0 +1,31 @@
package orch
// AgentState is a typed string representing the lifecycle state of an agent session.
// Values are sent as-is over the WebSocket event stream to the Mini App canvas.
type AgentState string
const (
// AgentStateIdle is the resting state, set automatically by Broadcaster on spawn.
AgentStateIdle AgentState = "idle"
// AgentStateWaiting means the agent is waiting for an LLM response.
AgentStateWaiting AgentState = "waiting"
// AgentStateToolCall means the agent is executing a tool.
// The tool name is carried in the Tool field of the event.
AgentStateToolCall AgentState = "toolcall"
// AgentStatePlanInterviewing means the conductor is in plan-mode interview phase,
// clarifying goals and constraints with the user.
AgentStatePlanInterviewing AgentState = "plan_interviewing"
// AgentStatePlanReview means the conductor has submitted a plan and is waiting
// for user approval before executing.
AgentStatePlanReview AgentState = "plan_review"
// AgentStatePlanExecuting means the conductor is executing an approved plan.
AgentStatePlanExecuting AgentState = "plan_executing"
// AgentStatePlanCompleted means all plan steps have been completed.
AgentStatePlanCompleted AgentState = "plan_completed"
)

View file

@ -227,8 +227,45 @@ func (r *ToolRegistry) GetRuntimeStatus() string {
return strings.Join(parts, "\n\n") return strings.Join(parts, "\n\n")
} }
// buildParamHint extracts parameter names from a JSON schema and returns
// a hint string like "(task, label?, preset?)". Required params are bare,
// optional params have a trailing "?".
func buildParamHint(schema map[string]any) string {
props, _ := schema["properties"].(map[string]any)
if len(props) == 0 {
return ""
}
reqSlice, _ := schema["required"].([]string)
reqSet := make(map[string]bool, len(reqSlice))
for _, r := range reqSlice {
reqSet[r] = true
}
names := make([]string, 0, len(props))
for name := range props {
names = append(names, name)
}
sort.Strings(names)
parts := make([]string, 0, len(names))
// Required params first, then optional
for _, name := range names {
if reqSet[name] {
parts = append(parts, name)
}
}
for _, name := range names {
if !reqSet[name] {
parts = append(parts, name+"?")
}
}
return "(" + strings.Join(parts, ", ") + ")"
}
// GetSummaries returns human-readable summaries of all registered tools. // GetSummaries returns human-readable summaries of all registered tools.
// Returns a slice of "name - description" strings. // Returns a slice of "- `name`(params) - description" strings.
func (r *ToolRegistry) GetSummaries() []string { func (r *ToolRegistry) GetSummaries() []string {
r.mu.RLock() r.mu.RLock()
defer r.mu.RUnlock() defer r.mu.RUnlock()
@ -237,7 +274,8 @@ func (r *ToolRegistry) GetSummaries() []string {
summaries := make([]string, 0, len(sorted)) summaries := make([]string, 0, len(sorted))
for _, name := range sorted { for _, name := range sorted {
tool := r.tools[name] tool := r.tools[name]
summaries = append(summaries, fmt.Sprintf("- `%s` - %s", tool.Name(), tool.Description())) hint := buildParamHint(tool.Parameters())
summaries = append(summaries, fmt.Sprintf("- `%s`%s - %s", tool.Name(), hint, tool.Description()))
} }
return summaries return summaries
} }

View file

@ -337,6 +337,78 @@ func TestToolRegistry_Count(t *testing.T) {
} }
} }
func TestBuildParamHint(t *testing.T) {
tests := []struct {
name string
schema map[string]any
want string
}{
{
name: "required and optional",
schema: map[string]any{
"type": "object",
"properties": map[string]any{
"task": map[string]any{"type": "string"},
"label": map[string]any{"type": "string"},
},
"required": []string{"task"},
},
want: "(task, label?)",
},
{
name: "all required",
schema: map[string]any{
"type": "object",
"properties": map[string]any{
"command": map[string]any{"type": "string"},
},
"required": []string{"command"},
},
want: "(command)",
},
{
name: "no properties",
schema: map[string]any{
"type": "object",
},
want: "",
},
{
name: "empty schema",
schema: map[string]any{},
want: "",
},
{
name: "nil schema",
schema: nil,
want: "",
},
{
name: "multiple optional sorted",
schema: map[string]any{
"type": "object",
"properties": map[string]any{
"task": map[string]any{"type": "string"},
"preset": map[string]any{"type": "string"},
"label": map[string]any{"type": "string"},
"agent_id": map[string]any{"type": "string"},
},
"required": []string{"task"},
},
want: "(task, agent_id?, label?, preset?)",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := buildParamHint(tt.schema)
if got != tt.want {
t.Errorf("buildParamHint() = %q, want %q", got, tt.want)
}
})
}
}
func TestToolRegistry_GetSummaries(t *testing.T) { func TestToolRegistry_GetSummaries(t *testing.T) {
r := NewToolRegistry() r := NewToolRegistry()
r.Register(newMockTool("read_file", "Reads a file")) r.Register(newMockTool("read_file", "Reads a file"))
@ -353,6 +425,32 @@ func TestToolRegistry_GetSummaries(t *testing.T) {
} }
} }
func TestToolRegistry_GetSummaries_WithParamHint(t *testing.T) {
r := NewToolRegistry()
r.Register(&mockRegistryTool{
name: "spawn",
desc: "Spawn a subagent",
params: map[string]any{
"type": "object",
"properties": map[string]any{
"task": map[string]any{"type": "string"},
"preset": map[string]any{"type": "string"},
},
"required": []string{"task"},
},
result: SilentResult("ok"),
})
summaries := r.GetSummaries()
if len(summaries) != 1 {
t.Fatalf("expected 1 summary, got %d", len(summaries))
}
// Should contain param hint
if !strings.Contains(summaries[0], "(task, preset?)") {
t.Errorf("expected param hint in summary, got %q", summaries[0])
}
}
func TestToolToSchema(t *testing.T) { func TestToolToSchema(t *testing.T) {
tool := newMockTool("demo", "demo tool") tool := newMockTool("demo", "demo tool")
schema := ToolToSchema(tool) schema := ToolToSchema(tool)

View file

@ -22,7 +22,8 @@ func IsValidPreset(p Preset) bool {
// ExecPolicy defines which commands are allowed for execution. // ExecPolicy defines which commands are allowed for execution.
type ExecPolicy struct { type ExecPolicy struct {
AllowPattern string // Prefix-match regex; matched commands are allowed AllowRules []string // Command prefix allowlist (e.g., "go test", "pnpm run test")
LocalNetOnly bool // Restrict curl/wget to localhost and RFC 1918 private addresses
} }
// SandboxConfig describes the sandbox isolation policy for a preset. // SandboxConfig describes the sandbox isolation policy for a preset.
@ -43,13 +44,43 @@ type SubagentEnvironment struct {
ContextFiles []string // Files to provide as context ContextFiles []string // Files to provide as context
} }
// presetExecPatterns maps presets to command allowlist regexes. // presetAllowRules maps presets to command prefix allowlists.
var presetExecPatterns = map[Preset]string{ // Each entry is a command prefix: the first N words of the executed command
PresetScout: ``, // No exec allowed // must match exactly. e.g. "go test" allows "go test ./..." but not "go build".
PresetAnalyst: `^(go\s+(test|vet)|git\s+(log|diff|status)|curl|wget|grep|find)\b`, // A single word like "curl" allows any arguments.
PresetCoder: `^(go\s+(test|vet|fmt)|gofmt|goimports|golangci-lint|prettier|eslint|black|ruff|cargo\s+(test|fmt|clippy)|pnpm\s+(test|run\s+(test|lint|format))|bun\s+(test|run\s+(test|lint|format))|uv\s+run\s+)\b`, // curl/wget are included where exec is allowed; LocalNetOnly in ExecPolicy
PresetWorker: `^(go\s+|pnpm\s+(install|add|run|test|build)|bun\s+(install|add|run|test|build)|uv\s+(run|sync|add|pip\s+install)|pip\s+install|cargo\s+)\b`, // ensures all curl/wget requests are restricted to localhost and RFC 1918 addresses.
PresetCoordinator: `^(go\s+|pnpm\s+|bun\s+|curl|wget)\b`, var presetAllowRules = map[Preset][]string{
PresetScout: nil, // No exec allowed
PresetAnalyst: {
"go test", "go vet",
"git log", "git diff", "git status",
"curl", "wget", "grep", "find",
},
PresetCoder: {
"go test", "go vet", "go fmt",
"gofmt", "goimports", "golangci-lint",
"prettier", "eslint", "black", "ruff",
"cargo test", "cargo fmt", "cargo clippy",
"pnpm test", "pnpm run test", "pnpm run lint", "pnpm run format",
"bun test", "bun run test", "bun run lint", "bun run format",
"uv run",
"curl", "wget",
},
PresetWorker: {
"go",
"pnpm install", "pnpm add", "pnpm run", "pnpm test", "pnpm build",
"bun install", "bun add", "bun run", "bun test", "bun build",
"uv run", "uv sync", "uv add", "uv pip install",
"pip install",
"cargo",
"curl", "wget",
},
PresetCoordinator: {
"go",
"pnpm", "bun",
"curl", "wget",
},
} }
// presetSpawnablePresets maps presets to which presets they can spawn. // presetSpawnablePresets maps presets to which presets they can spawn.
@ -107,11 +138,14 @@ func SandboxConfigForPreset(p Preset, writeRoot string) SandboxConfig {
config.WriteRoot = writeRoot config.WriteRoot = writeRoot
} }
// Set ExecPolicy if exec is allowed and pattern is non-empty // Set ExecPolicy if exec is allowed and rules are defined.
// LocalNetOnly is always true: curl/wget in subagents is for local server
// testing only; external HTTP access goes through the web_fetch tool.
if allowed["exec"] { if allowed["exec"] {
if pattern := presetExecPatterns[p]; pattern != "" { if rules := presetAllowRules[p]; len(rules) > 0 {
config.ExecPolicy = &ExecPolicy{ config.ExecPolicy = &ExecPolicy{
AllowPattern: pattern, AllowRules: rules,
LocalNetOnly: true,
} }
} }
} }

View file

@ -1,7 +1,6 @@
package tools package tools
import ( import (
"regexp"
"testing" "testing"
) )
@ -128,6 +127,8 @@ func TestSandboxConfigForPreset_Coder(t *testing.T) {
} }
if config.ExecPolicy == nil { if config.ExecPolicy == nil {
t.Errorf("ExecPolicy: got nil, want non-nil") t.Errorf("ExecPolicy: got nil, want non-nil")
} else if !config.ExecPolicy.LocalNetOnly {
t.Errorf("ExecPolicy.LocalNetOnly: got false, want true")
} }
if config.SpawnablePresets != nil { if config.SpawnablePresets != nil {
t.Errorf("SpawnablePresets: got non-nil, want nil") t.Errorf("SpawnablePresets: got non-nil, want nil")
@ -146,6 +147,8 @@ func TestSandboxConfigForPreset_Coordinator(t *testing.T) {
} }
if config.ExecPolicy == nil { if config.ExecPolicy == nil {
t.Errorf("ExecPolicy: got nil, want non-nil") t.Errorf("ExecPolicy: got nil, want non-nil")
} else if !config.ExecPolicy.LocalNetOnly {
t.Errorf("ExecPolicy.LocalNetOnly: got false, want true")
} }
if config.SpawnablePresets == nil { if config.SpawnablePresets == nil {
t.Errorf("SpawnablePresets: got nil, want non-nil") t.Errorf("SpawnablePresets: got nil, want non-nil")
@ -163,17 +166,18 @@ func TestSandboxConfigForPreset_Coordinator(t *testing.T) {
} }
} }
// TestPresetExecPatterns_Coder validates coder exec allowlist. // TestPresetAllowRules_Coder validates coder exec allowlist.
func TestPresetExecPatterns_Coder(t *testing.T) { func TestPresetAllowRules_Coder(t *testing.T) {
pattern, ok := presetExecPatterns[PresetCoder] rules := presetAllowRules[PresetCoder]
if !ok || pattern == "" { if len(rules) == 0 {
t.Fatalf("coder pattern missing or empty") t.Fatalf("coder rules missing or empty")
} }
re, err := regexp.Compile(pattern) exec, err := NewExecTool(t.TempDir(), true)
if err != nil { if err != nil {
t.Fatalf("failed to compile pattern: %v", err) t.Fatalf("NewExecTool: %v", err)
} }
exec.SetAllowRules(rules)
tests := []struct { tests := []struct {
cmd string cmd string
@ -181,32 +185,53 @@ func TestPresetExecPatterns_Coder(t *testing.T) {
}{ }{
{"go test ./...", true}, {"go test ./...", true},
{"go vet ./...", true}, {"go vet ./...", true},
{"go fmt ./...", true},
{"gofmt -w file.go", true}, {"gofmt -w file.go", true},
{"golangci-lint run", true}, {"golangci-lint run", true},
{"cargo test", true},
{"cargo fmt", true},
{"cargo clippy", true},
{"pnpm test", true},
{"pnpm run test", true},
{"pnpm run lint", true},
{"pnpm run format", true},
{"bun test", true},
{"bun run test", true},
{"uv run pytest", true},
// curl/wget are in the allowlist; LocalNetOnly enforcement is at runtime
{"curl http://localhost:3000/health", true},
{"wget http://127.0.0.1:8080/status", true},
// blocked
{"go build ./...", false}, {"go build ./...", false},
{"npm install", false}, {"npm install", false},
{"pnpm test", true}, {"pnpm install", false},
{"pnpm run build", false},
{"cargo build", false},
{"pwd", false},
{"ls", false},
} }
for _, tt := range tests { for _, tt := range tests {
gotOK := re.MatchString(tt.cmd) result := exec.guardCommand(tt.cmd, t.TempDir())
gotOK := result == ""
if gotOK != tt.wantOK { if gotOK != tt.wantOK {
t.Errorf("cmd %q: got %v, want %v", tt.cmd, gotOK, tt.wantOK) t.Errorf("cmd %q: got allowed=%v, want %v (guard: %q)", tt.cmd, gotOK, tt.wantOK, result)
} }
} }
} }
// TestPresetExecPatterns_Analyst validates analyst exec allowlist. // TestPresetAllowRules_Analyst validates analyst exec allowlist.
func TestPresetExecPatterns_Analyst(t *testing.T) { func TestPresetAllowRules_Analyst(t *testing.T) {
pattern, ok := presetExecPatterns[PresetAnalyst] rules := presetAllowRules[PresetAnalyst]
if !ok || pattern == "" { if len(rules) == 0 {
t.Fatalf("analyst pattern missing or empty") t.Fatalf("analyst rules missing or empty")
} }
re, err := regexp.Compile(pattern) exec, err := NewExecTool(t.TempDir(), true)
if err != nil { if err != nil {
t.Fatalf("failed to compile pattern: %v", err) t.Fatalf("NewExecTool: %v", err)
} }
exec.SetAllowRules(rules)
tests := []struct { tests := []struct {
cmd string cmd string
@ -216,16 +241,109 @@ func TestPresetExecPatterns_Analyst(t *testing.T) {
{"go vet ./...", true}, {"go vet ./...", true},
{"git log --oneline", true}, {"git log --oneline", true},
{"git diff HEAD", true}, {"git diff HEAD", true},
{"git status", true},
{"grep pattern file", true}, {"grep pattern file", true},
{"find . -name '*.go'", true},
{"curl http://example.com", true}, {"curl http://example.com", true},
// blocked
{"go build ./...", false}, {"go build ./...", false},
{"npm install", false}, {"npm install", false},
{"git push", false},
{"git checkout", false},
{"pwd", false},
{"ls", false},
} }
for _, tt := range tests { for _, tt := range tests {
gotOK := re.MatchString(tt.cmd) result := exec.guardCommand(tt.cmd, t.TempDir())
gotOK := result == ""
if gotOK != tt.wantOK { if gotOK != tt.wantOK {
t.Errorf("cmd %q: got %v, want %v", tt.cmd, gotOK, tt.wantOK) t.Errorf("cmd %q: got allowed=%v, want %v (guard: %q)", tt.cmd, gotOK, tt.wantOK, result)
}
}
}
// TestPresetAllowRules_Worker validates worker exec allowlist.
func TestPresetAllowRules_Worker(t *testing.T) {
rules := presetAllowRules[PresetWorker]
if len(rules) == 0 {
t.Fatalf("worker rules missing or empty")
}
exec, err := NewExecTool(t.TempDir(), true)
if err != nil {
t.Fatalf("NewExecTool: %v", err)
}
exec.SetAllowRules(rules)
tests := []struct {
cmd string
wantOK bool
}{
{"go build ./...", true},
{"go test ./...", true},
{"pnpm install", true},
{"pnpm add lodash", true},
{"pnpm run dev", true},
{"bun install", true},
{"bun build", true},
{"uv run pytest", true},
{"uv sync", true},
{"uv pip install flask", true},
{"pip install flask", true},
{"cargo build", true},
{"cargo test", true},
{"curl http://localhost:8080", true},
// blocked
{"npm install", false},
{"pwd", false},
}
for _, tt := range tests {
result := exec.guardCommand(tt.cmd, t.TempDir())
gotOK := result == ""
if gotOK != tt.wantOK {
t.Errorf("cmd %q: got allowed=%v, want %v (guard: %q)", tt.cmd, gotOK, tt.wantOK, result)
}
}
}
// TestPresetAllowRules_Coordinator validates coordinator exec allowlist.
func TestPresetAllowRules_Coordinator(t *testing.T) {
rules := presetAllowRules[PresetCoordinator]
if len(rules) == 0 {
t.Fatalf("coordinator rules missing or empty")
}
exec, err := NewExecTool(t.TempDir(), true)
if err != nil {
t.Fatalf("NewExecTool: %v", err)
}
exec.SetAllowRules(rules)
tests := []struct {
cmd string
wantOK bool
}{
{"go build ./...", true},
{"go test ./...", true},
{"pnpm install", true},
{"pnpm run dev", true},
{"bun install", true},
{"bun run dev", true},
{"curl http://localhost:8080", true},
{"wget http://127.0.0.1:3000", true},
// blocked
{"npm install", false},
{"cargo build", false},
{"pwd", false},
}
for _, tt := range tests {
result := exec.guardCommand(tt.cmd, t.TempDir())
gotOK := result == ""
if gotOK != tt.wantOK {
t.Errorf("cmd %q: got allowed=%v, want %v (guard: %q)", tt.cmd, gotOK, tt.wantOK, result)
} }
} }
} }

View file

@ -6,6 +6,8 @@ import (
"errors" "errors"
"fmt" "fmt"
"io" "io"
"net"
"net/url"
"os" "os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
@ -120,8 +122,9 @@ type ExecTool struct {
workingDir string workingDir string
timeout time.Duration timeout time.Duration
denyPatterns []*regexp.Regexp denyPatterns []*regexp.Regexp
allowPatterns []*regexp.Regexp allowRules [][]string // pre-split command prefix allowlist
restrictToWorkspace bool restrictToWorkspace bool
localNetOnly bool // restrict curl/wget to localhost + RFC 1918
// Background process management // Background process management
bgMu sync.Mutex bgMu sync.Mutex
@ -214,7 +217,7 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf
workingDir: workingDir, workingDir: workingDir,
timeout: 5 * time.Minute, timeout: 5 * time.Minute,
denyPatterns: denyPatterns, denyPatterns: denyPatterns,
allowPatterns: nil, allowRules: nil,
restrictToWorkspace: restrict, restrictToWorkspace: restrict,
bgProcesses: make(map[string]*bgProcess), bgProcesses: make(map[string]*bgProcess),
bgCtx: bgCtx, bgCtx: bgCtx,
@ -705,28 +708,29 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
} }
} }
if len(t.allowPatterns) > 0 { if len(t.allowRules) > 0 {
allowed := false if !matchAllowRules(lower, t.allowRules) {
for _, pattern := range t.allowPatterns {
if pattern.MatchString(lower) {
allowed = true
break
}
}
if !allowed {
var b strings.Builder var b strings.Builder
b.WriteString("Command blocked: not in allowlist [") b.WriteString("Command blocked: not in allowlist [")
for i, p := range t.allowPatterns { for i, rule := range t.allowRules {
if i > 0 { if i > 0 {
b.WriteByte(',') b.WriteByte(',')
} }
b.WriteString(p.String()) b.WriteString(strings.Join(rule, " "))
} }
b.WriteByte(']') b.WriteByte(']')
return b.String() return b.String()
} }
} }
// Restrict curl/wget to localhost and RFC 1918 private addresses.
// External HTTP access is available via the web_fetch tool.
if t.localNetOnly && isCurlOrWget(cmd) {
if errMsg := checkCurlLocalNet(cmd); errMsg != "" {
return errMsg
}
}
if t.restrictToWorkspace { if t.restrictToWorkspace {
if strings.Contains(cmd, "..\\") || strings.Contains(cmd, "../") { if strings.Contains(cmd, "..\\") || strings.Contains(cmd, "../") {
return "Command blocked by safety guard (path traversal detected)" return "Command blocked by safety guard (path traversal detected)"
@ -766,6 +770,12 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
if isExecutable(p) { if isExecutable(p) {
continue continue
} }
// Allow /dev/* paths (e.g. /dev/null, /dev/urandom).
// Device files are not regular filesystem paths and pose
// no workspace-escape risk.
if strings.HasPrefix(p, "/dev/") {
continue
}
// Agent CLI slash commands: skip non-existent paths // Agent CLI slash commands: skip non-existent paths
// (e.g., "/review" is a command, not a file). // (e.g., "/review" is a command, not a file).
if agentCLI { if agentCLI {
@ -830,16 +840,90 @@ func (t *ExecTool) SetRestrictToWorkspace(restrict bool) {
t.restrictToWorkspace = restrict t.restrictToWorkspace = restrict
} }
func (t *ExecTool) SetAllowPatterns(patterns []string) error { // SetAllowRules sets the command prefix allowlist.
t.allowPatterns = make([]*regexp.Regexp, 0, len(patterns)) // Each rule is a space-separated command prefix (e.g. "go test", "pnpm run lint").
for _, p := range patterns { // A command is allowed if its first N words match any rule's N words exactly.
re, err := regexp.Compile(p) func (t *ExecTool) SetAllowRules(rules []string) {
t.allowRules = make([][]string, 0, len(rules))
for _, r := range rules {
words := strings.Fields(strings.ToLower(r))
if len(words) > 0 {
t.allowRules = append(t.allowRules, words)
}
}
}
// matchAllowRules checks if cmd matches any prefix in the allowlist.
func matchAllowRules(cmd string, rules [][]string) bool {
cmdWords := strings.Fields(cmd)
for _, ruleWords := range rules {
if len(cmdWords) < len(ruleWords) {
continue
}
match := true
for i, rw := range ruleWords {
if cmdWords[i] != rw {
match = false
break
}
}
if match {
return true
}
}
return false
}
func (t *ExecTool) SetLocalNetOnly(v bool) {
t.localNetOnly = v
}
// isCurlOrWget reports whether command is a curl or wget invocation.
func isCurlOrWget(command string) bool {
fields := strings.Fields(command)
if len(fields) == 0 {
return false
}
base := filepath.Base(fields[0])
return base == "curl" || base == "wget"
}
// checkCurlLocalNet validates that all http/https URLs in a curl/wget command
// target localhost or RFC 1918 private addresses.
// Returns an error message string, or empty string if the command is allowed.
func checkCurlLocalNet(command string) string {
for _, token := range strings.Fields(command) {
token = strings.Trim(token, "\"'")
if !strings.HasPrefix(token, "http://") && !strings.HasPrefix(token, "https://") {
continue
}
u, err := url.Parse(token)
if err != nil { if err != nil {
return fmt.Errorf("invalid allow pattern %q: %w", p, err) continue
} }
t.allowPatterns = append(t.allowPatterns, re) host := u.Hostname()
if !isLocalHost(host) {
return fmt.Sprintf(
"Command blocked by safety guard "+
"(curl/wget is restricted to localhost and private network; %q is a public address)",
host,
)
} }
return nil }
return ""
}
// isLocalHost reports whether host is localhost or a loopback/RFC 1918 private IP.
// DNS resolution is intentionally avoided to prevent DNS rebinding attacks.
func isLocalHost(host string) bool {
if strings.EqualFold(host, "localhost") {
return true
}
ip := net.ParseIP(host)
if ip == nil {
return false
}
return ip.IsLoopback() || ip.IsPrivate()
} }
// SetBgMaxLifetimeForTest overrides bgMaxLifetime for testing purposes. // SetBgMaxLifetimeForTest overrides bgMaxLifetime for testing purposes.

View file

@ -573,15 +573,12 @@ func TestGuardCommand_DenyPattern_IncludesPattern(t *testing.T) {
} }
} }
// TestGuardCommand_Allowlist_ShowsPatterns verifies that allowlist violation // TestGuardCommand_Allowlist_ShowsRules verifies that allowlist violation
// messages include all configured patterns. // messages include all configured rules.
func TestGuardCommand_Allowlist_ShowsPatterns(t *testing.T) { func TestGuardCommand_Allowlist_ShowsRules(t *testing.T) {
workspace := t.TempDir() workspace := t.TempDir()
tool, _ := NewExecTool(workspace, true) tool, _ := NewExecTool(workspace, true)
err := tool.SetAllowPatterns([]string{`^go\b`, `^git\b`}) tool.SetAllowRules([]string{"go test", "git"})
if err != nil {
t.Fatalf("SetAllowPatterns failed: %v", err)
}
result := tool.guardCommand("curl http://example.com", workspace) result := tool.guardCommand("curl http://example.com", workspace)
if result == "" { if result == "" {
@ -590,8 +587,8 @@ func TestGuardCommand_Allowlist_ShowsPatterns(t *testing.T) {
if !strings.Contains(result, "not in allowlist") { if !strings.Contains(result, "not in allowlist") {
t.Errorf("expected 'not in allowlist' in message, got: %s", result) t.Errorf("expected 'not in allowlist' in message, got: %s", result)
} }
if !strings.Contains(result, `^go\b`) || !strings.Contains(result, `^git\b`) { if !strings.Contains(result, "go test") || !strings.Contains(result, "git") {
t.Errorf("expected allowlist patterns in message, got: %s", result) t.Errorf("expected allowlist rules in message, got: %s", result)
} }
} }
@ -983,3 +980,97 @@ func TestExecTool_Bg_RingBufferOverflow(t *testing.T) {
t.Errorf("ring buffer exceeded max size: %d > %d", bufLen, bgRingBufSize) t.Errorf("ring buffer exceeded max size: %d > %d", bufLen, bgRingBufSize)
} }
} }
// TestIsLocalHost verifies localhost and RFC 1918 detection using net package.
func TestIsLocalHost(t *testing.T) {
tests := []struct {
host string
want bool
}{
// Loopback / localhost
{"localhost", true},
{"LOCALHOST", true},
{"127.0.0.1", true},
{"127.0.0.2", true},
{"::1", true},
// RFC 1918 private ranges
{"10.0.0.1", true},
{"10.255.255.255", true},
{"172.16.0.1", true},
{"172.31.255.255", true},
{"192.168.0.1", true},
{"192.168.1.100", true},
// Public addresses
{"8.8.8.8", false},
{"1.1.1.1", false},
{"example.com", false},
{"api.github.com", false},
// Edge: non-private but routable private-looking address
{"172.15.255.255", false}, // just below 172.16/12
{"172.32.0.0", false}, // just above 172.31/12
}
for _, tt := range tests {
got := isLocalHost(tt.host)
if got != tt.want {
t.Errorf("isLocalHost(%q) = %v, want %v", tt.host, got, tt.want)
}
}
}
// TestCheckCurlLocalNet verifies URL-level enforcement for curl/wget commands.
func TestCheckCurlLocalNet(t *testing.T) {
tests := []struct {
cmd string
wantErr bool
}{
// Allowed: localhost and private IPs
{"curl http://localhost:3000/health", false},
{"curl -v http://127.0.0.1:8080/api/status", false},
{"wget http://192.168.1.10/file.bin", false},
{"curl -X POST http://10.0.0.5:9000/webhook", false},
// Blocked: public addresses
{"curl http://example.com", true},
{"wget https://releases.github.com/v1.tar.gz", true},
{"curl http://8.8.8.8/data", true},
// Allowed: no http URL (e.g. --help, --version — no network access)
{"curl --help", false},
{"curl --version", false},
{"wget --help", false},
}
for _, tt := range tests {
errMsg := checkCurlLocalNet(tt.cmd)
gotErr := errMsg != ""
if gotErr != tt.wantErr {
t.Errorf("checkCurlLocalNet(%q): gotErr=%v wantErr=%v (msg: %q)",
tt.cmd, gotErr, tt.wantErr, errMsg)
}
}
}
// TestExecTool_LocalNetOnly verifies curl/wget blocking via SetLocalNetOnly.
func TestExecTool_LocalNetOnly(t *testing.T) {
tool, _ := NewExecTool("", false)
tool.SetLocalNetOnly(true)
tests := []struct {
cmd string
wantErr bool
}{
{"curl http://localhost:3000", false},
{"curl http://example.com", true},
{"echo hello", false}, // non-curl not affected
}
ctx := context.Background()
for _, tt := range tests {
result := tool.Execute(ctx, map[string]any{"command": tt.cmd})
if tt.wantErr && !result.IsError {
t.Errorf("cmd %q: expected blocked, but succeeded", tt.cmd)
}
if !tt.wantErr && result.IsError && strings.Contains(result.ForLLM, "safety guard") {
t.Errorf("cmd %q: expected allowed, but safety guard blocked: %s", tt.cmd, result.ForLLM)
}
}
}

View file

@ -32,7 +32,7 @@ func (t *SpawnTool) Name() string {
} }
func (t *SpawnTool) Description() string { func (t *SpawnTool) Description() string {
return "Spawn a subagent to handle a task in the background. Use this for complex or time-consuming tasks that can run independently. The subagent will complete the task and report back when done." return "Spawn a subagent that runs NON-BLOCKING in the background and returns immediately. Prefer this over subagent for any task that can run independently. Use preset to control capabilities (scout, analyst, coder, worker, coordinator)."
} }
func (t *SpawnTool) Parameters() map[string]any { func (t *SpawnTool) Parameters() map[string]any {
@ -73,26 +73,35 @@ func (t *SpawnTool) SetAllowlistChecker(check func(targetAgentID string) bool) {
func (t *SpawnTool) Execute(ctx context.Context, args map[string]any) *ToolResult { func (t *SpawnTool) Execute(ctx context.Context, args map[string]any) *ToolResult {
task, ok := args["task"].(string) task, ok := args["task"].(string)
if !ok || strings.TrimSpace(task) == "" { if !ok || strings.TrimSpace(task) == "" {
return ErrorResult("task is required and must be a non-empty string") return ErrorResult(
`Required parameter "task" (string) is missing. ` +
`Example: {"task": "describe what you need done", "preset": "scout"}`,
)
} }
label, _ := args["label"].(string) label, _ := args["label"].(string)
agentID, _ := args["agent_id"].(string) agentID, _ := args["agent_id"].(string)
preset, _ := args["preset"].(string) preset, _ := args["preset"].(string)
// Check allowlist if targeting a specific agent or preset // Check allowlist if targeting a specific agent ID.
checkTarget := agentID // Presets (scout, analyst, etc.) are NOT agent IDs — they are validated
if checkTarget == "" && preset != "" { // separately by IsValidPreset() in the subagent manager.
checkTarget = preset if agentID != "" && t.allowlistCheck != nil {
} if !t.allowlistCheck(agentID) {
if checkTarget != "" && t.allowlistCheck != nil { return ErrorResult(fmt.Sprintf("agent %q is not in the allowed agents list", agentID))
if !t.allowlistCheck(checkTarget) {
return ErrorResult(fmt.Sprintf("not allowed to spawn agent '%s' or preset '%s'", agentID, preset))
} }
} }
// Validate preset name if provided
if preset != "" && !IsValidPreset(Preset(preset)) {
return ErrorResult(fmt.Sprintf(
"preset %q is not valid. Available presets: scout, analyst, coder, worker, coordinator",
preset,
))
}
if t.manager == nil { if t.manager == nil {
return ErrorResult("Subagent manager not configured") return ErrorResult("spawn tool is not available in this session (orchestration may be disabled)")
} }
// Pass callback to manager for async completion notification // Pass callback to manager for async completion notification

View file

@ -33,8 +33,8 @@ func TestSpawnTool_Execute_EmptyTask(t *testing.T) {
if !result.IsError { if !result.IsError {
t.Error("Expected error for invalid task parameter") t.Error("Expected error for invalid task parameter")
} }
if !strings.Contains(result.ForLLM, "task is required") { if !strings.Contains(result.ForLLM, `"task"`) {
t.Errorf("Error message should mention 'task is required', got: %s", result.ForLLM) t.Errorf("Error message should mention '\"task\"', got: %s", result.ForLLM)
} }
}) })
} }
@ -73,7 +73,7 @@ func TestSpawnTool_Execute_NilManager(t *testing.T) {
if !result.IsError { if !result.IsError {
t.Error("Expected error for nil manager") t.Error("Expected error for nil manager")
} }
if !strings.Contains(result.ForLLM, "Subagent manager not configured") { if !strings.Contains(result.ForLLM, "spawn tool is not available") {
t.Errorf("Error message should mention manager not configured, got: %s", result.ForLLM) t.Errorf("Error message should mention spawn tool not available, got: %s", result.ForLLM)
} }
} }

View file

@ -3,7 +3,9 @@ package tools
import ( import (
"context" "context"
"fmt" "fmt"
"log" "sort"
"strconv"
"strings"
"sync" "sync"
"time" "time"
@ -22,6 +24,10 @@ type SubagentTask struct {
Status string Status string
Result string Result string
Created int64 Created int64
CompletedAt int64 `json:"-"`
Iterations int `json:"-"`
ToolCalls int `json:"-"`
ToolStats map[string]int `json:"-"`
} }
type SubagentManager struct { type SubagentManager struct {
@ -33,7 +39,6 @@ type SubagentManager struct {
workspace string workspace string
tools *ToolRegistry tools *ToolRegistry
webSearchOpts WebSearchToolOptions webSearchOpts WebSearchToolOptions
execTool *ExecTool // Shared exec tool for all presets
maxIterations int maxIterations int
maxTokens int maxTokens int
temperature float64 temperature float64
@ -53,11 +58,6 @@ func NewSubagentManager(
if reporter == nil { if reporter == nil {
reporter = orch.Noop reporter = orch.Noop
} }
// Create a shared exec tool for all presets
execTool, err := NewExecTool(workspace, true)
if err != nil {
log.Printf("subagent: failed to create exec tool: %v (exec disabled for subagents)", err)
}
return &SubagentManager{ return &SubagentManager{
tasks: make(map[string]*SubagentTask), tasks: make(map[string]*SubagentTask),
provider: provider, provider: provider,
@ -66,7 +66,6 @@ func NewSubagentManager(
workspace: workspace, workspace: workspace,
tools: NewToolRegistry(), tools: NewToolRegistry(),
webSearchOpts: webSearchOpts, webSearchOpts: webSearchOpts,
execTool: execTool,
maxIterations: 10, maxIterations: 10,
nextID: 1, nextID: 1,
reporter: reporter, reporter: reporter,
@ -249,14 +248,19 @@ After completing, provide a clear summary of what was done and how it was verifi
} else { } else {
task.Status = "completed" task.Status = "completed"
task.Result = loopResult.Content task.Result = loopResult.Content
task.CompletedAt = time.Now().UnixMilli()
task.Iterations = loopResult.Iterations
task.ToolCalls = loopResult.ToolCalls
task.ToolStats = loopResult.ToolStats
// Notify conductor of the result // Notify conductor of the result
sm.reporter.ReportConversation(task.ID, "conductor", loopResult.Content) sm.reporter.ReportConversation(task.ID, "conductor", loopResult.Content)
sm.reporter.ReportGC(task.ID, "completed") sm.reporter.ReportGC(task.ID, "completed")
result = &ToolResult{ result = &ToolResult{
ForLLM: fmt.Sprintf( ForLLM: fmt.Sprintf(
"Subagent '%s' completed (iterations: %d): %s", "Subagent '%s' completed (iterations: %d, tool calls: %d): %s",
task.Label, task.Label,
loopResult.Iterations, loopResult.Iterations,
loopResult.ToolCalls,
loopResult.Content, loopResult.Content,
), ),
ForUser: loopResult.Content, ForUser: loopResult.Content,
@ -269,6 +273,14 @@ After completing, provide a clear summary of what was done and how it was verifi
// Send announce message back to main agent // Send announce message back to main agent
if sm.bus != nil { if sm.bus != nil {
announceContent := fmt.Sprintf("Task '%s' completed.\n\nResult:\n%s", task.Label, task.Result) announceContent := fmt.Sprintf("Task '%s' completed.\n\nResult:\n%s", task.Label, task.Result)
metadata := map[string]string{
"duration_ms": strconv.FormatInt(task.CompletedAt-task.Created, 10),
"iterations": strconv.Itoa(task.Iterations),
"tool_calls": strconv.Itoa(task.ToolCalls),
}
if len(task.ToolStats) > 0 {
metadata["tool_stats"] = formatToolStats(task.ToolStats)
}
pubCtx, pubCancel := context.WithTimeout(context.Background(), 5*time.Second) pubCtx, pubCancel := context.WithTimeout(context.Background(), 5*time.Second)
defer pubCancel() defer pubCancel()
sm.bus.PublishInbound(pubCtx, bus.InboundMessage{ sm.bus.PublishInbound(pubCtx, bus.InboundMessage{
@ -277,6 +289,7 @@ After completing, provide a clear summary of what was done and how it was verifi
// Format: "original_channel:original_chat_id" for routing back // Format: "original_channel:original_chat_id" for routing back
ChatID: fmt.Sprintf("%s:%s", task.OriginChannel, task.OriginChatID), ChatID: fmt.Sprintf("%s:%s", task.OriginChannel, task.OriginChatID),
Content: announceContent, Content: announceContent,
Metadata: metadata,
}) })
} }
} }
@ -306,12 +319,22 @@ func (sm *SubagentManager) buildPresetRegistry(preset Preset, writeRoot string)
registry.Register(NewAppendFileTool(writeRoot, true)) registry.Register(NewAppendFileTool(writeRoot, true))
} }
// Register exec and bg_monitor if allowed // Register exec and bg_monitor if allowed.
// Each subagent gets its own ExecTool to avoid mutating the shared instance's
// allowRules (which would leak sandbox restrictions to the conductor).
if config.AllowedTools["exec"] { if config.AllowedTools["exec"] {
// Use the shared exec tool but set allow patterns execWorkDir := writeRoot
execTool := sm.execTool if execWorkDir == "" {
execWorkDir = sm.workspace
}
execTool, err := NewExecTool(execWorkDir, true)
if err != nil {
// exec disabled for this subagent; skip registration
return registry
}
if config.ExecPolicy != nil { if config.ExecPolicy != nil {
_ = execTool.SetAllowPatterns([]string{config.ExecPolicy.AllowPattern}) execTool.SetAllowRules(config.ExecPolicy.AllowRules)
execTool.SetLocalNetOnly(config.ExecPolicy.LocalNetOnly)
} }
registry.Register(execTool) registry.Register(execTool)
@ -383,7 +406,7 @@ func (t *SubagentTool) Name() string {
} }
func (t *SubagentTool) Description() string { func (t *SubagentTool) Description() string {
return "Execute a subagent task synchronously and return the result. Use this for delegating specific tasks to an independent agent instance. Returns execution summary to user and full details to LLM." return "Run a task in a subagent and BLOCK until it completes, returning the result directly. Use when you need the answer before deciding your next step. For background/parallel tasks, use spawn instead."
} }
func (t *SubagentTool) Parameters() map[string]any { func (t *SubagentTool) Parameters() map[string]any {
@ -411,13 +434,17 @@ func (t *SubagentTool) SetContext(channel, chatID string) {
func (t *SubagentTool) Execute(ctx context.Context, args map[string]any) *ToolResult { func (t *SubagentTool) Execute(ctx context.Context, args map[string]any) *ToolResult {
task, ok := args["task"].(string) task, ok := args["task"].(string)
if !ok { if !ok {
return ErrorResult("task is required").WithError(fmt.Errorf("task parameter is required")) return ErrorResult(
`Required parameter "task" (string) is missing. ` +
`Example: {"task": "describe what you need done"}`,
).WithError(fmt.Errorf("task parameter is required"))
} }
label, _ := args["label"].(string) label, _ := args["label"].(string)
if t.manager == nil { if t.manager == nil {
return ErrorResult("Subagent manager not configured").WithError(fmt.Errorf("manager is nil")) return ErrorResult("subagent tool is not available in this session (orchestration may be disabled)").
WithError(fmt.Errorf("manager is nil"))
} }
// Build messages for subagent // Build messages for subagent
@ -477,8 +504,8 @@ func (t *SubagentTool) Execute(ctx context.Context, args map[string]any) *ToolRe
if labelStr == "" { if labelStr == "" {
labelStr = "(unnamed)" labelStr = "(unnamed)"
} }
llmContent := fmt.Sprintf("Subagent task completed:\nLabel: %s\nIterations: %d\nResult: %s", llmContent := fmt.Sprintf("Subagent task completed:\nLabel: %s\nIterations: %d\nTool calls: %d\nResult: %s",
labelStr, loopResult.Iterations, loopResult.Content) labelStr, loopResult.Iterations, loopResult.ToolCalls, loopResult.Content)
return &ToolResult{ return &ToolResult{
ForLLM: llmContent, ForLLM: llmContent,
@ -488,3 +515,18 @@ func (t *SubagentTool) Execute(ctx context.Context, args map[string]any) *ToolRe
Async: false, Async: false,
} }
} }
// formatToolStats formats a tool stats map as a compact string: "exec:3,read_file:5".
// Keys are sorted alphabetically for deterministic output.
func formatToolStats(stats map[string]int) string {
keys := make([]string, 0, len(stats))
for k := range stats {
keys = append(keys, k)
}
sort.Strings(keys)
parts := make([]string, 0, len(keys))
for _, k := range keys {
parts = append(parts, k+":"+strconv.Itoa(stats[k]))
}
return strings.Join(parts, ",")
}

View file

@ -4,6 +4,7 @@ import (
"context" "context"
"strings" "strings"
"testing" "testing"
"time"
"github.com/sipeed/picoclaw/pkg/bus" "github.com/sipeed/picoclaw/pkg/bus"
"github.com/sipeed/picoclaw/pkg/orch" "github.com/sipeed/picoclaw/pkg/orch"
@ -93,8 +94,11 @@ func TestSubagentTool_Description(t *testing.T) {
if desc == "" { if desc == "" {
t.Error("Description should not be empty") t.Error("Description should not be empty")
} }
if !strings.Contains(desc, "subagent") { if !strings.Contains(desc, "BLOCK") {
t.Errorf("Description should mention 'subagent', got: %s", desc) t.Errorf("Description should mention 'BLOCK', got: %s", desc)
}
if !strings.Contains(desc, "spawn") {
t.Errorf("Description should contrast with spawn, got: %s", desc)
} }
} }
@ -259,9 +263,12 @@ func TestSubagentTool_Execute_MissingTask(t *testing.T) {
t.Error("Expected error for missing task parameter") t.Error("Expected error for missing task parameter")
} }
// ForLLM should contain error message // ForLLM should contain helpful error with example
if !strings.Contains(result.ForLLM, "task is required") { if !strings.Contains(result.ForLLM, `"task"`) {
t.Errorf("Error message should mention 'task is required', got: %s", result.ForLLM) t.Errorf("Error message should mention '\"task\"', got: %s", result.ForLLM)
}
if !strings.Contains(result.ForLLM, "Example") {
t.Errorf("Error message should include usage example, got: %s", result.ForLLM)
} }
// Err should be set // Err should be set
@ -286,8 +293,8 @@ func TestSubagentTool_Execute_NilManager(t *testing.T) {
t.Error("Expected error for nil manager") t.Error("Expected error for nil manager")
} }
if !strings.Contains(result.ForLLM, "Subagent manager not configured") { if !strings.Contains(result.ForLLM, "not available in this session") {
t.Errorf("Error message should mention manager not configured, got: %s", result.ForLLM) t.Errorf("Error message should mention 'not available in this session', got: %s", result.ForLLM)
} }
} }
@ -349,3 +356,69 @@ func TestSubagentTool_ForUserTruncation(t *testing.T) {
t.Error("ForLLM should contain reference to original task") t.Error("ForLLM should contain reference to original task")
} }
} }
func TestFormatToolStats(t *testing.T) {
tests := []struct {
name string
stats map[string]int
want string
}{
{"empty", map[string]int{}, ""},
{"single", map[string]int{"exec": 3}, "exec:3"},
{
"multiple sorted",
map[string]int{"read_file": 5, "exec": 3, "write_file": 1},
"exec:3,read_file:5,write_file:1",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := formatToolStats(tt.stats)
if got != tt.want {
t.Errorf("formatToolStats(%v) = %q, want %q", tt.stats, got, tt.want)
}
})
}
}
// TestSubagentManager_Spawn_SetsMetadata verifies that the bus message from a
// completed spawn includes execution statistics in Metadata.
func TestSubagentManager_Spawn_SetsMetadata(t *testing.T) {
provider := &MockLLMProvider{}
msgBus := bus.NewMessageBus()
mgr := NewSubagentManager(provider, "test-model", "/tmp/test", msgBus, orch.Noop, WebSearchToolOptions{})
_, err := mgr.Spawn(
context.Background(),
"say hello", "meta-test", "", "cli", "direct", "",
nil,
)
if err != nil {
t.Fatalf("Spawn() error: %v", err)
}
// Consume the inbound message from the bus
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel()
received, ok := msgBus.ConsumeInbound(ctx)
if !ok {
t.Fatal("timed out waiting for bus message")
}
if received.Channel != "system" {
t.Fatalf("expected channel 'system', got %q", received.Channel)
}
if received.Metadata == nil {
t.Fatal("Metadata should not be nil")
}
if received.Metadata["iterations"] != "1" {
t.Errorf("iterations = %q, want %q", received.Metadata["iterations"], "1")
}
if received.Metadata["tool_calls"] != "0" {
t.Errorf("tool_calls = %q, want %q", received.Metadata["tool_calls"], "0")
}
// duration_ms should be a non-negative number
if received.Metadata["duration_ms"] == "" {
t.Error("duration_ms should be present")
}
}

View file

@ -36,6 +36,8 @@ type ToolLoopConfig struct {
type ToolLoopResult struct { type ToolLoopResult struct {
Content string Content string
Iterations int Iterations int
ToolCalls int // total tool call count across all iterations
ToolStats map[string]int // tool name → call count
} }
// RunToolLoop executes the LLM + tool call iteration loop. // RunToolLoop executes the LLM + tool call iteration loop.
@ -52,6 +54,8 @@ func RunToolLoop(
} }
iteration := 0 iteration := 0
totalToolCalls := 0
toolStats := map[string]int{}
var finalContent string var finalContent string
for iteration < config.MaxIterations { for iteration < config.MaxIterations {
@ -75,7 +79,7 @@ func RunToolLoop(
llmOpts = map[string]any{} llmOpts = map[string]any{}
} }
// 3. Call LLM (hook: waiting for response) // 3. Call LLM (hook: waiting for response)
reporter.ReportStateChange(config.AgentID, "waiting", "") reporter.ReportStateChange(config.AgentID, orch.AgentStateWaiting, "")
response, err := config.Provider.Chat(ctx, messages, providerToolDefs, config.Model, llmOpts) response, err := config.Provider.Chat(ctx, messages, providerToolDefs, config.Model, llmOpts)
if err != nil { if err != nil {
logger.ErrorCF("toolloop", "LLM call failed", logger.ErrorCF("toolloop", "LLM call failed",
@ -143,7 +147,9 @@ func RunToolLoop(
"tool": tc.Name, "tool": tc.Name,
"iteration": iteration, "iteration": iteration,
}) })
reporter.ReportStateChange(config.AgentID, "toolcall", tc.Name) reporter.ReportStateChange(config.AgentID, orch.AgentStateToolCall, tc.Name)
totalToolCalls++
toolStats[tc.Name]++
// Execute tool (no async callback for subagents - they run independently) // Execute tool (no async callback for subagents - they run independently)
var toolResult *ToolResult var toolResult *ToolResult
@ -172,5 +178,7 @@ func RunToolLoop(
return &ToolLoopResult{ return &ToolLoopResult{
Content: finalContent, Content: finalContent,
Iterations: iteration, Iterations: iteration,
ToolCalls: totalToolCalls,
ToolStats: toolStats,
}, nil }, nil
} }

View file

@ -17,14 +17,14 @@ type reporterSpy struct {
} }
type spyCall struct { type spyCall struct {
state string state orch.AgentState
tool string tool string
} }
func (r *reporterSpy) ReportSpawn(id, label, task string) {} func (r *reporterSpy) ReportSpawn(id, label, task string) {}
func (r *reporterSpy) ReportConversation(from, to, text string) {} func (r *reporterSpy) ReportConversation(from, to, text string) {}
func (r *reporterSpy) ReportGC(id, reason string) {} func (r *reporterSpy) ReportGC(id, reason string) {}
func (r *reporterSpy) ReportStateChange(id, state, tool string) { func (r *reporterSpy) ReportStateChange(id string, state orch.AgentState, tool string) {
r.mu.Lock() r.mu.Lock()
r.calls = append(r.calls, spyCall{state, tool}) r.calls = append(r.calls, spyCall{state, tool})
r.mu.Unlock() r.mu.Unlock()
@ -117,7 +117,7 @@ func TestToolLoop_Reporter_WaitingBeforeLLM(t *testing.T) {
if len(calls) == 0 { if len(calls) == 0 {
t.Fatal("expected at least one ReportStateChange call") t.Fatal("expected at least one ReportStateChange call")
} }
if calls[0].state != "waiting" { if calls[0].state != orch.AgentStateWaiting {
t.Fatalf("first call must be state=waiting, got %+v", calls[0]) t.Fatalf("first call must be state=waiting, got %+v", calls[0])
} }
} }
@ -152,17 +152,62 @@ func TestToolLoop_Reporter_ToolcallOrderedAfterWaiting(t *testing.T) {
if len(calls) < 3 { if len(calls) < 3 {
t.Fatalf("expected at least 3 calls, got %d: %+v", len(calls), calls) t.Fatalf("expected at least 3 calls, got %d: %+v", len(calls), calls)
} }
if calls[0].state != "waiting" { if calls[0].state != orch.AgentStateWaiting {
t.Fatalf("calls[0] must be waiting, got %+v", calls[0]) t.Fatalf("calls[0] must be waiting, got %+v", calls[0])
} }
if calls[1].state != "toolcall" || calls[1].tool != "echo_tool" { if calls[1].state != orch.AgentStateToolCall || calls[1].tool != "echo_tool" {
t.Fatalf("calls[1] must be toolcall(echo_tool), got %+v", calls[1]) t.Fatalf("calls[1] must be toolcall(echo_tool), got %+v", calls[1])
} }
if calls[2].state != "waiting" { if calls[2].state != orch.AgentStateWaiting {
t.Fatalf("calls[2] must be waiting (2nd LLM iteration), got %+v", calls[2]) t.Fatalf("calls[2] must be waiting (2nd LLM iteration), got %+v", calls[2])
} }
} }
// TestToolLoop_ToolCallStats verifies that ToolLoopResult.ToolCalls and
// ToolStats are populated correctly after a tool call iteration.
func TestToolLoop_ToolCallStats(t *testing.T) {
reg := NewToolRegistry()
reg.Register(&echoTool{})
result, err := RunToolLoop(context.Background(), ToolLoopConfig{
Provider: &sequenceMockProvider{},
Model: "test",
Tools: reg,
MaxIterations: 5,
}, []providers.Message{{Role: "user", Content: "do it"}}, "cli", "direct")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if result.ToolCalls != 1 {
t.Errorf("ToolCalls = %d, want 1", result.ToolCalls)
}
if result.ToolStats["echo_tool"] != 1 {
t.Errorf("ToolStats[echo_tool] = %d, want 1", result.ToolStats["echo_tool"])
}
if result.Iterations != 2 {
t.Errorf("Iterations = %d, want 2", result.Iterations)
}
}
// TestToolLoop_NoToolCalls_ZeroStats verifies that a direct answer (no tool
// calls) produces zero ToolCalls and an empty ToolStats map.
func TestToolLoop_NoToolCalls_ZeroStats(t *testing.T) {
result, err := RunToolLoop(context.Background(), ToolLoopConfig{
Provider: &MockLLMProvider{},
Model: "test",
MaxIterations: 1,
}, []providers.Message{{Role: "user", Content: "hi"}}, "cli", "direct")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if result.ToolCalls != 0 {
t.Errorf("ToolCalls = %d, want 0", result.ToolCalls)
}
if len(result.ToolStats) != 0 {
t.Errorf("ToolStats = %v, want empty", result.ToolStats)
}
}
// TestToolLoop_Reporter_NoopImplementsInterface is a compile-time check that // TestToolLoop_Reporter_NoopImplementsInterface is a compile-time check that
// orch.Noop satisfies the orch.AgentReporter interface accepted by // orch.Noop satisfies the orch.AgentReporter interface accepted by
// ToolLoopConfig.Reporter. If Noop ever stops implementing the interface the // ToolLoopConfig.Reporter. If Noop ever stops implementing the interface the