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

feat(orch): orchestration spawn adoption and sandbox improvements
2026-03-01 03:59:03 +09:00 · 2026-03-01 03:59:03 +09:00 · 0a82e1a7ac
commit 0a82e1a7ac
parent 78dc0b79fc 2f3efb4c8f
23 changed files with 1255 additions and 184 deletions
--- a/.claude/settings.local.json
+++ b/.claude/settings.local.json
@ -0,0 +1,37 @@
+{
+  "permissions": {
+    "allow": [
+      "Bash(grep:*)",
+      "Bash(find:*)",
+      "Bash(go build:*)",
+      "Bash(go test:*)",
+      "Bash(git add:*)",
+      "Bash(git commit:*)",
+      "Bash(git push:*)",
+      "Bash(gh issue view:*)",
+      "Bash(cd /d/vscode/ai/picoclaw && go vet ./pkg/agent/ 2>&1)",
+      "Bash(cd:*)",
+      "Bash(cp:*)",
+      "Bash(ls:*)",
+      "Bash(head:*)",
+      "Bash(wc:*)",
+      "WebFetch(domain:docs.astral.sh)",
+      "WebFetch(domain:github.com)",
+      "Bash(cd /d/vscode/ai/picoclaw && go vet ./pkg/agent/... ./pkg/channels/... ./pkg/bus/... 2>&1)",
+      "Bash(go list:*)",
+      "Bash(go mod:*)",
+      "Bash(go env:*)",
+      "Bash(cd /d/vscode/ai/picoclaw && go vet ./pkg/miniapp/... ./pkg/logger/... 2>&1)",
+      "Bash(cd /d/vscode/ai/picoclaw && go vet ./pkg/logger/... ./pkg/miniapp/... 2>&1)",
+      "Bash(cd /d/vscode/ai/picoclaw && go vet ./pkg/agent/... 2>&1)",
+      "WebSearch",
+      "Bash(gh api:*)",
+      "Bash(gh run:*)",
+      "Bash(gh pr:*)",
+      "Bash(gofmt:*)"
+    ]
+  },
+  "remote": {
+    "defaultEnvironmentId": "env_011CUpDfW35pH2YVfqef4sHE"
+  }
+}
--- a/pkg/agent/context.go
+++ b/pkg/agent/context.go
@ -19,28 +19,50 @@ import (

 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:
- Tasks can run in parallel or in the background
- Multiple independent tasks can run simultaneously — spawn each one
- You don't need the result to decide the next step
- The operation is long-running (builds, fetches, analysis, file processing)
+### spawn (non-blocking) — DEFAULT choice
+Returns immediately. Use for any task that can run independently.
+Call the spawn tool with JSON arguments like this:

-Use **subagent** (blocking) when:
- You need the result before you can continue
- Correctness of the next step depends on the outcome
+Tool: spawn
+Arguments: {"task": "Examine pkg/auth/ and report middleware pattern", "preset": "scout", "label": "auth-scout"}

-Do inline only when:
- It's a single fast tool call (read a file, quick search)
- Delegation overhead clearly outweighs the benefit
+Tool: spawn
+Arguments: {"task": "Implement rate limiter in pkg/ratelimit/ with tests", "preset": "coder", "label": "rate-limiter"}

-Default bias: if a task involves more than 2-3 tool calls or can run independently, delegate it.
-When you spawn, immediately plan what comes next — blocking means you've stopped thinking.
-Fork aggressively: explore multiple directions simultaneously.
+### subagent (blocking) — only when you need the answer NOW
+Blocks until the subagent finishes. Use only when you cannot proceed without the result.
+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.
-When results come back, synthesize and decide the next fork.`
+When results come back, synthesize findings and decide the next fork.`

 type ContextBuilder struct {
 	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 🦞

-You are picoclaw, a helpful AI assistant.
+You are picoclaw, %s.

 ## Workspace
 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.
     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 "executing": Work through the current Phase's steps.
-     Mark each "- [x]" via edit_file. The system will auto-advance phases.
+   - If Status is "executing": %s
   - Plan format (header is written by the system — do NOT delete it):
     # Active Plan
     > Task: <description>
@ -176,7 +211,7 @@ Your workspace is at: %s
   - 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.`,
-		workspacePath, workspacePath, workspacePath, workspacePath, toolsSection)
+		identity, workspacePath, workspacePath, workspacePath, workspacePath, toolsSection, executingRule)
 }

 func (cb *ContextBuilder) buildToolsSection() string {
--- a/pkg/agent/loop.go
+++ b/pkg/agent/loop.go
@ -115,6 +115,7 @@ type processOptions struct {
 	NoHistory       bool   // If true, don't load session history (for heartbeat)
 	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
+	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."
@ -302,6 +303,9 @@ func registerSharedTools(
 				return registry.CanSpawnSubagent(currentAgentID, targetAgentID)
 			})
 			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
@ -796,24 +800,92 @@ func (al *AgentLoop) processSystemMessage(ctx context.Context, msg bus.InboundMe
 		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()
 	if agent == nil {
 		return "", fmt.Errorf("no default agent for system message")
 	}

-	// Use the origin session for context
 	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{
-		SessionKey:      sessionKey,
+	// Send a brief notification (SkipPlaceholder to avoid corrupting status messages)
+	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,
 		ChatID:          originChatID,
-		UserMessage:     fmt.Sprintf("[System: %s] %s", msg.SenderID, msg.Content),
-		DefaultResponse: "Background task completed.",
-		EnableSummary:   false,
-		SendResponse:    true,
+		Content:         notification,
+		SkipPlaceholder: 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.
@ -1066,16 +1138,49 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
 		al.promptDirty.Store(false)
 	}

-	// 5. Run LLM iteration loop
-	finalContent, iteration, err := al.runLLMIteration(ctx, agent, messages, opts, task, preStatus)
-	if err != nil {
-		return "", err
-	}
+	// 5. Run LLM iteration loop (with automatic phase transitions)
+	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 {
+			return "", err
+		}
+
+		// 5a. Auto-advance plan phases after LLM iteration
+		postStatus := agent.ContextBuilder.GetPlanStatus()
+		if !agent.ContextBuilder.HasActivePlan() ||
+			!(postStatus == "executing" || postStatus == "review" || postStatus == "completed") {
+			break
+		}

-	// 5a. Auto-advance plan phases after LLM iteration
-	postStatus := agent.ContextBuilder.GetPlanStatus()
-	if agent.ContextBuilder.HasActivePlan() &&
-		(postStatus == "executing" || postStatus == "review" || postStatus == "completed") {
 		// Intercept: if AI changed status to executing or review without user approval
 		// (from interviewing or review), validate and hold at "review".
 		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")
 				logger.WarnCF("agent", "Reverted plan to interviewing: "+err.Error(),
 					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."
 				agent.Sessions.AddMessage(opts.SessionKey, "user", rejectionMsg)
 			} else {
 				_ = agent.ContextBuilder.SetPlanStatus("review")
+				al.reporter().ReportStateChange(opts.SessionKey, orch.AgentStatePlanReview, "")
 				if !constants.IsInternalChannel(opts.Channel) {
 					planDisplay := agent.ContextBuilder.FormatPlanDisplay()
 					_ = 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 {
-			// Safeguard: executing but no phases (shouldn't happen, but be safe).
+			break
+		}
+
+		if postStatus == "executing" && agent.ContextBuilder.GetTotalPhases() == 0 {
 			_ = agent.ContextBuilder.SetPlanStatus("interviewing")
 			logger.WarnCF("agent", "Reverted plan to interviewing: no phases defined",
 				map[string]any{"agent_id": agent.ID})
-		} else if agent.ContextBuilder.IsPlanComplete() {
-			// Mark plan as completed (keep memory for review; user can /plan clear)
+			break
+		}
+
+		if agent.ContextBuilder.IsPlanComplete() {
 			total := agent.ContextBuilder.GetTotalPhases()
 			_ = agent.ContextBuilder.SetCurrentPhase(total)
 			if preStatus != "completed" {
 				_ = agent.ContextBuilder.SetPlanStatus("completed")
+				al.reporter().ReportStateChange(opts.SessionKey, orch.AgentStatePlanCompleted, "")

 				// Deactivate worktree on plan completion
 				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()
 			_ = agent.ContextBuilder.AdvancePhase()
 			next := agent.ContextBuilder.GetCurrentPhase()
@ -1140,7 +1258,11 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
 					SkipPlaceholder: true,
 				})
 			}
+			al.notifyStateChange()
+			continue
 		}
+
+		break
 	}

 	al.notifyStateChange()
@ -1176,9 +1298,10 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
 	// 8. Optional: send response via bus
 	if opts.SendResponse {
 		_ = al.bus.PublishOutbound(ctx, bus.OutboundMessage{
-			Channel: opts.Channel,
-			ChatID:  opts.ChatID,
-			Content: finalContent,
+			Channel:         opts.Channel,
+			ChatID:          opts.ChatID,
+			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
 }

+// 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.
 // Group 1 captures the target directory path.
 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.
-		al.reporter().ReportStateChange(opts.SessionKey, "waiting", "")
+		al.reporter().ReportStateChange(opts.SessionKey, orch.AgentStateWaiting, "")

 		// Retry loop for context/token errors
 		maxRetries := 2
@ -2447,7 +2588,7 @@ func (al *AgentLoop) runLLMIteration(
 			}

 			// 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()
 			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,
 		// memory, plan status, etc.  Update messages[0] so the next LLM
 		// 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 {
 			return fmt.Sprintf("Error: %v", err), true
 		}
+		al.reporter().ReportStateChange(sessionKey, orch.AgentStatePlanExecuting, "")
 		al.planStartPending = true
 		clearHistory := len(args) > 1 && args[1] == "clear"
 		al.planClearHistory = clearHistory
--- a/pkg/agent/loop_test.go
+++ b/pkg/agent/loop_test.go
@ -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)
+			}
+		})
+	}
+}
--- a/pkg/agent/memory.go
+++ b/pkg/agent/memory.go
@ -630,6 +630,14 @@ func (ms *MemoryStore) getPlanContextFrom(content string) string {
 		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()
 }

--- a/pkg/miniapp/static/index.html
+++ b/pkg/miniapp/static/index.html
@ -884,11 +884,16 @@
        <div class="orch-badge-label">CNDR</div>
        <div class="orch-badge-dot"></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-label">SEC</div>
        <div class="orch-badge-dot"></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 class="orch-canvas-wrap">
      <canvas id="orch-canvas" width="320" height="320"></canvas>
@ -1810,7 +1815,7 @@ var _orchLastTs = null;
 var _orchBOB      = [0, -1, -2, -1];
 var _orchFRAME_MS = {idle:450, waiting:650, toolcall:90, talking:280, entering:220, exiting:220};
 var _orchWALK     = 55;
-var _orchConductor, _orchSecretary, _orchSubagents, _orchSlots, _orchFreeSlots;
+var _orchConductor, _orchSecretary, _orchHeartbeat, _orchSubagents, _orchSlots, _orchFreeSlots;

 function _orchMakeChar(id, emoji, home) {
  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() {
  _orchConductor = _orchMakeChar('conductor', '👑',  MAP_POSITIONS.conductor);
  _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:'💻'},
            {id:'s3',emoji:'🔧'},{id:'s4',emoji:'🎯'}];
  _orchSubagents = ps.map(function(p,i){
@ -1828,7 +1836,7 @@ function _orchInitChars() {
  });
  _orchSlots = {}; _orchFreeSlots = _orchSubagents.slice();
 }
-function _orchAllChars() { return [_orchConductor, _orchSecretary].concat(_orchSubagents); }
+function _orchAllChars() { return [_orchConductor, _orchSecretary, _orchHeartbeat].concat(_orchSubagents); }

 function _orchSyncBadge(id, state, alive) {
  var el = document.getElementById('orch-badge-' + id); if (!el) return;
@ -1838,12 +1846,30 @@ function _orchSyncBadge(id, state, alive) {
    + (state==='toolcall'? ' toolcall' : '')
    + (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 _orchSay(c, text, ttl)  { c.bubble={text:text, ttl:ttl||2200}; }

 function _orchCharForId(id) {
-  if (id === 'heartbeat') return _orchSecretary;
+  if (id === 'heartbeat') return _orchHeartbeat;
  if (_orchSlots[id])     return _orchSlots[id];
  return _orchConductor;
 }
@ -1865,7 +1891,16 @@ function _orchGC(id) {
    _orchSetState(c,'exiting');
    _orchMoveTo(c, MAP_POSITIONS.door, function(){ c.alive=false; _orchSetState(c,'idle'); });
  } 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) {
@ -1885,17 +1920,45 @@ function _orchConverse(fromId, toId, text) {
 function _orchUpdate(dt) {
  _orchAllChars().forEach(function(c){
    if (!c.alive && c.state!=='entering') return;
-    c.frameTimer+=dt;
-    var dur=_orchFRAME_MS[c.state]||450;
-    if (c.frameTimer>=dur){ c.frame=(c.frame+1)%4; c.frameTimer-=dur; }
+    // 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;
+      var dur=_orchFRAME_MS[c.state]||450;
+      if (c.frameTimer>=dur){ c.frame=(c.frame+1)%4; c.frameTimer-=dur; }
+    }
    if (c.target){
      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; }
      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; }
+    // 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) {
  if (!c.bubble) return;
  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'){
    orchCtx.fillStyle='rgba(96,165,250,0.25)'; orchCtx.beginPath();
    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.fillText(c.emoji, cx, cy);
+  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.font='6px Silkscreen,monospace'; orchCtx.textAlign='center'; orchCtx.textBaseline='top';
  orchCtx.fillStyle=c.state==='talking'?'#facc15':'#3a4a7a';
  orchCtx.fillText(c.id.toUpperCase(), cx, cy+11);
+  _orchDrawStatus(c);
  _orchDrawBubble(c);
 }
 function _orchRender(ts) {
@ -1964,7 +2041,7 @@ function connectOrchWs() {
    } else if (msg.type==='event') {
      var ev=msg.event||{};
      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==='conversation') _orchConverse(ev.from, ev.to, ev.text);
    }
--- a/pkg/miniapp/static/map.js
+++ b/pkg/miniapp/static/map.js
@ -27,6 +27,7 @@ var MAP_POSITIONS = {
  door:      { x: 160, y: 314 },  // entry / exit point
  conductor: { x: 160, 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
  stations: [
    { x:  40, y: 106 },  // S0  scout
--- a/pkg/orch/broadcaster.go
+++ b/pkg/orch/broadcaster.go
@ -89,8 +89,8 @@ func (b *Broadcaster) ReportSpawn(id, label, task string) {
 }

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

 // ReportConversation implements AgentReporter.
--- a/pkg/orch/reporter.go
+++ b/pkg/orch/reporter.go
@ -4,17 +4,17 @@ package orch
 // Both Broadcaster (real events) and noopReporter (disabled) implement this.
 type AgentReporter interface {
 	ReportSpawn(id, label, task string)
-	ReportStateChange(id, state, tool string)
+	ReportStateChange(id string, state AgentState, tool string)
 	ReportConversation(from, to, text string)
 	ReportGC(id, reason string)
 }

 type noopReporter struct{}

-func (n *noopReporter) ReportSpawn(id, label, task string)       {}
-func (n *noopReporter) ReportStateChange(id, state, tool string) {}
-func (n *noopReporter) ReportConversation(from, to, text string) {}
-func (n *noopReporter) ReportGC(id, reason string)               {}
+func (n *noopReporter) ReportSpawn(id, label, task string)                         {}
+func (n *noopReporter) ReportStateChange(id string, state AgentState, tool string) {}
+func (n *noopReporter) ReportConversation(from, to, text string)                   {}
+func (n *noopReporter) ReportGC(id, reason string)                                 {}

 // Noop is the AgentReporter to use when orchestration is disabled.
 // Allows nil-free code in callers.
--- a/pkg/orch/reporter_test.go
+++ b/pkg/orch/reporter_test.go
@ -10,8 +10,8 @@ var _ AgentReporter = (*Broadcaster)(nil)
 // orchestration mode.
 func TestNoop_AllMethods_NoPanic(t *testing.T) {
 	Noop.ReportSpawn("id", "label", "task")
-	Noop.ReportStateChange("id", "waiting", "")
-	Noop.ReportStateChange("id", "toolcall", "bash")
+	Noop.ReportStateChange("id", AgentStateWaiting, "")
+	Noop.ReportStateChange("id", AgentStateToolCall, "bash")
 	Noop.ReportConversation("conductor", "sub-1", "do something")
 	Noop.ReportGC("id", "completed")
 }
@ -49,9 +49,9 @@ func TestBroadcaster_ReportStateChange_MapsToAgentStateEvent(t *testing.T) {
 	b.ReportSpawn("agent-1", "coder", "implement it")
 	<-sub.Ch // consume spawn

-	b.ReportStateChange("agent-1", "toolcall", "bash")
+	b.ReportStateChange("agent-1", AgentStateToolCall, "bash")
 	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)
 	}
 	snap := b.Snapshot()
--- a/pkg/orch/state.go
+++ b/pkg/orch/state.go
@ -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"
+)
--- a/pkg/tools/registry.go
+++ b/pkg/tools/registry.go
@ -227,8 +227,45 @@ func (r *ToolRegistry) GetRuntimeStatus() string {
 	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.
-// Returns a slice of "name - description" strings.
+// Returns a slice of "- `name`(params) - description" strings.
 func (r *ToolRegistry) GetSummaries() []string {
 	r.mu.RLock()
 	defer r.mu.RUnlock()
@ -237,7 +274,8 @@ func (r *ToolRegistry) GetSummaries() []string {
 	summaries := make([]string, 0, len(sorted))
 	for _, name := range sorted {
 		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
 }
--- a/pkg/tools/registry_test.go
+++ b/pkg/tools/registry_test.go
@ -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) {
 	r := NewToolRegistry()
 	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) {
 	tool := newMockTool("demo", "demo tool")
 	schema := ToolToSchema(tool)
--- a/pkg/tools/sandbox.go
+++ b/pkg/tools/sandbox.go
@ -22,7 +22,8 @@ func IsValidPreset(p Preset) bool {

 // ExecPolicy defines which commands are allowed for execution.
 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.
@ -43,13 +44,43 @@ type SubagentEnvironment struct {
 	ContextFiles []string // Files to provide as context
 }

-// presetExecPatterns maps presets to command allowlist regexes.
-var presetExecPatterns = map[Preset]string{
-	PresetScout:       ``, // No exec allowed
-	PresetAnalyst:     `^(go\s+(test|vet)|git\s+(log|diff|status)|curl|wget|grep|find)\b`,
-	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`,
-	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`,
-	PresetCoordinator: `^(go\s+|pnpm\s+|bun\s+|curl|wget)\b`,
+// presetAllowRules maps presets to command prefix allowlists.
+// Each entry is a command prefix: the first N words of the executed command
+// must match exactly. e.g. "go test" allows "go test ./..." but not "go build".
+// A single word like "curl" allows any arguments.
+// curl/wget are included where exec is allowed; LocalNetOnly in ExecPolicy
+// ensures all curl/wget requests are restricted to localhost and RFC 1918 addresses.
+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.
@ -107,11 +138,14 @@ func SandboxConfigForPreset(p Preset, writeRoot string) SandboxConfig {
 		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 pattern := presetExecPatterns[p]; pattern != "" {
+		if rules := presetAllowRules[p]; len(rules) > 0 {
 			config.ExecPolicy = &ExecPolicy{
-				AllowPattern: pattern,
+				AllowRules:   rules,
+				LocalNetOnly: true,
 			}
 		}
 	}
--- a/pkg/tools/sandbox_test.go
+++ b/pkg/tools/sandbox_test.go
@ -1,7 +1,6 @@
 package tools

 import (
-	"regexp"
 	"testing"
 )

@ -128,6 +127,8 @@ func TestSandboxConfigForPreset_Coder(t *testing.T) {
 	}
 	if config.ExecPolicy == 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 {
 		t.Errorf("SpawnablePresets: got non-nil, want nil")
@ -146,6 +147,8 @@ func TestSandboxConfigForPreset_Coordinator(t *testing.T) {
 	}
 	if config.ExecPolicy == 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 {
 		t.Errorf("SpawnablePresets: got nil, want non-nil")
@ -163,17 +166,18 @@ func TestSandboxConfigForPreset_Coordinator(t *testing.T) {
 	}
 }

-// TestPresetExecPatterns_Coder validates coder exec allowlist.
-func TestPresetExecPatterns_Coder(t *testing.T) {
-	pattern, ok := presetExecPatterns[PresetCoder]
-	if !ok || pattern == "" {
-		t.Fatalf("coder pattern missing or empty")
+// TestPresetAllowRules_Coder validates coder exec allowlist.
+func TestPresetAllowRules_Coder(t *testing.T) {
+	rules := presetAllowRules[PresetCoder]
+	if len(rules) == 0 {
+		t.Fatalf("coder rules missing or empty")
 	}

-	re, err := regexp.Compile(pattern)
+	exec, err := NewExecTool(t.TempDir(), true)
 	if err != nil {
-		t.Fatalf("failed to compile pattern: %v", err)
+		t.Fatalf("NewExecTool: %v", err)
 	}
+	exec.SetAllowRules(rules)

 	tests := []struct {
 		cmd    string
@ -181,32 +185,53 @@ func TestPresetExecPatterns_Coder(t *testing.T) {
 	}{
 		{"go test ./...", true},
 		{"go vet ./...", true},
+		{"go fmt ./...", true},
 		{"gofmt -w file.go", 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},
 		{"npm install", false},
-		{"pnpm test", true},
+		{"pnpm install", false},
+		{"pnpm run build", false},
+		{"cargo build", false},
+		{"pwd", false},
+		{"ls", false},
 	}

 	for _, tt := range tests {
-		gotOK := re.MatchString(tt.cmd)
+		result := exec.guardCommand(tt.cmd, t.TempDir())
+		gotOK := result == ""
 		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.
-func TestPresetExecPatterns_Analyst(t *testing.T) {
-	pattern, ok := presetExecPatterns[PresetAnalyst]
-	if !ok || pattern == "" {
-		t.Fatalf("analyst pattern missing or empty")
+// TestPresetAllowRules_Analyst validates analyst exec allowlist.
+func TestPresetAllowRules_Analyst(t *testing.T) {
+	rules := presetAllowRules[PresetAnalyst]
+	if len(rules) == 0 {
+		t.Fatalf("analyst rules missing or empty")
 	}

-	re, err := regexp.Compile(pattern)
+	exec, err := NewExecTool(t.TempDir(), true)
 	if err != nil {
-		t.Fatalf("failed to compile pattern: %v", err)
+		t.Fatalf("NewExecTool: %v", err)
 	}
+	exec.SetAllowRules(rules)

 	tests := []struct {
 		cmd    string
@ -216,16 +241,109 @@ func TestPresetExecPatterns_Analyst(t *testing.T) {
 		{"go vet ./...", true},
 		{"git log --oneline", true},
 		{"git diff HEAD", true},
+		{"git status", true},
 		{"grep pattern file", true},
+		{"find . -name '*.go'", true},
 		{"curl http://example.com", true},
+		// blocked
 		{"go build ./...", false},
 		{"npm install", false},
+		{"git push", false},
+		{"git checkout", false},
+		{"pwd", false},
+		{"ls", false},
 	}

 	for _, tt := range tests {
-		gotOK := re.MatchString(tt.cmd)
+		result := exec.guardCommand(tt.cmd, t.TempDir())
+		gotOK := result == ""
 		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)
 		}
 	}
 }
--- a/pkg/tools/shell.go
+++ b/pkg/tools/shell.go
@ -6,6 +6,8 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net"
+	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
@ -120,8 +122,9 @@ type ExecTool struct {
 	workingDir          string
 	timeout             time.Duration
 	denyPatterns        []*regexp.Regexp
-	allowPatterns       []*regexp.Regexp
+	allowRules          [][]string // pre-split command prefix allowlist
 	restrictToWorkspace bool
+	localNetOnly        bool // restrict curl/wget to localhost + RFC 1918

 	// Background process management
 	bgMu        sync.Mutex
@ -214,7 +217,7 @@ func NewExecToolWithConfig(workingDir string, restrict bool, config *config.Conf
 		workingDir:          workingDir,
 		timeout:             5 * time.Minute,
 		denyPatterns:        denyPatterns,
-		allowPatterns:       nil,
+		allowRules:          nil,
 		restrictToWorkspace: restrict,
 		bgProcesses:         make(map[string]*bgProcess),
 		bgCtx:               bgCtx,
@ -705,28 +708,29 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
 		}
 	}

-	if len(t.allowPatterns) > 0 {
-		allowed := false
-		for _, pattern := range t.allowPatterns {
-			if pattern.MatchString(lower) {
-				allowed = true
-				break
-			}
-		}
-		if !allowed {
+	if len(t.allowRules) > 0 {
+		if !matchAllowRules(lower, t.allowRules) {
 			var b strings.Builder
 			b.WriteString("Command blocked: not in allowlist [")
-			for i, p := range t.allowPatterns {
+			for i, rule := range t.allowRules {
 				if i > 0 {
 					b.WriteByte(',')
 				}
-				b.WriteString(p.String())
+				b.WriteString(strings.Join(rule, " "))
 			}
 			b.WriteByte(']')
 			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 strings.Contains(cmd, "..\\") || strings.Contains(cmd, "../") {
 			return "Command blocked by safety guard (path traversal detected)"
@ -766,6 +770,12 @@ func (t *ExecTool) guardCommand(command, cwd string) string {
 				if isExecutable(p) {
 					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
 				// (e.g., "/review" is a command, not a file).
 				if agentCLI {
@ -830,16 +840,90 @@ func (t *ExecTool) SetRestrictToWorkspace(restrict bool) {
 	t.restrictToWorkspace = restrict
 }

-func (t *ExecTool) SetAllowPatterns(patterns []string) error {
-	t.allowPatterns = make([]*regexp.Regexp, 0, len(patterns))
-	for _, p := range patterns {
-		re, err := regexp.Compile(p)
-		if err != nil {
-			return fmt.Errorf("invalid allow pattern %q: %w", p, err)
+// SetAllowRules sets the command prefix allowlist.
+// Each rule is a space-separated command prefix (e.g. "go test", "pnpm run lint").
+// A command is allowed if its first N words match any rule's N words exactly.
+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)
 		}
-		t.allowPatterns = append(t.allowPatterns, re)
 	}
-	return nil
+}
+
+// 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 {
+			continue
+		}
+		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 ""
+}
+
+// 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.
--- a/pkg/tools/shell_test.go
+++ b/pkg/tools/shell_test.go
@ -573,15 +573,12 @@ func TestGuardCommand_DenyPattern_IncludesPattern(t *testing.T) {
 	}
 }

-// TestGuardCommand_Allowlist_ShowsPatterns verifies that allowlist violation
-// messages include all configured patterns.
-func TestGuardCommand_Allowlist_ShowsPatterns(t *testing.T) {
+// TestGuardCommand_Allowlist_ShowsRules verifies that allowlist violation
+// messages include all configured rules.
+func TestGuardCommand_Allowlist_ShowsRules(t *testing.T) {
 	workspace := t.TempDir()
 	tool, _ := NewExecTool(workspace, true)
-	err := tool.SetAllowPatterns([]string{`^go\b`, `^git\b`})
-	if err != nil {
-		t.Fatalf("SetAllowPatterns failed: %v", err)
-	}
+	tool.SetAllowRules([]string{"go test", "git"})

 	result := tool.guardCommand("curl http://example.com", workspace)
 	if result == "" {
@ -590,8 +587,8 @@ func TestGuardCommand_Allowlist_ShowsPatterns(t *testing.T) {
 	if !strings.Contains(result, "not in allowlist") {
 		t.Errorf("expected 'not in allowlist' in message, got: %s", result)
 	}
-	if !strings.Contains(result, `^go\b`) || !strings.Contains(result, `^git\b`) {
-		t.Errorf("expected allowlist patterns in message, got: %s", result)
+	if !strings.Contains(result, "go test") || !strings.Contains(result, "git") {
+		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)
 	}
 }
+
+// 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)
+		}
+	}
+}
--- a/pkg/tools/spawn.go
+++ b/pkg/tools/spawn.go
@ -32,7 +32,7 @@ func (t *SpawnTool) Name() 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 {
@ -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 {
 	task, ok := args["task"].(string)
 	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)
 	agentID, _ := args["agent_id"].(string)
 	preset, _ := args["preset"].(string)

-	// Check allowlist if targeting a specific agent or preset
-	checkTarget := agentID
-	if checkTarget == "" && preset != "" {
-		checkTarget = preset
-	}
-	if checkTarget != "" && t.allowlistCheck != nil {
-		if !t.allowlistCheck(checkTarget) {
-			return ErrorResult(fmt.Sprintf("not allowed to spawn agent '%s' or preset '%s'", agentID, preset))
+	// Check allowlist if targeting a specific agent ID.
+	// Presets (scout, analyst, etc.) are NOT agent IDs — they are validated
+	// separately by IsValidPreset() in the subagent manager.
+	if agentID != "" && t.allowlistCheck != nil {
+		if !t.allowlistCheck(agentID) {
+			return ErrorResult(fmt.Sprintf("agent %q is not in the allowed agents list", agentID))
 		}
 	}

+	// 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 {
-		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
--- a/pkg/tools/spawn_test.go
+++ b/pkg/tools/spawn_test.go
@ -33,8 +33,8 @@ func TestSpawnTool_Execute_EmptyTask(t *testing.T) {
 			if !result.IsError {
 				t.Error("Expected error for invalid task parameter")
 			}
-			if !strings.Contains(result.ForLLM, "task is required") {
-				t.Errorf("Error message should mention 'task is required', got: %s", result.ForLLM)
+			if !strings.Contains(result.ForLLM, `"task"`) {
+				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 {
 		t.Error("Expected error for nil manager")
 	}
-	if !strings.Contains(result.ForLLM, "Subagent manager not configured") {
-		t.Errorf("Error message should mention manager not configured, got: %s", result.ForLLM)
+	if !strings.Contains(result.ForLLM, "spawn tool is not available") {
+		t.Errorf("Error message should mention spawn tool not available, got: %s", result.ForLLM)
 	}
 }
--- a/pkg/tools/subagent.go
+++ b/pkg/tools/subagent.go
@ -3,7 +3,9 @@ package tools
 import (
 	"context"
 	"fmt"
-	"log"
+	"sort"
+	"strconv"
+	"strings"
 	"sync"
 	"time"

@ -22,6 +24,10 @@ type SubagentTask struct {
 	Status        string
 	Result        string
 	Created       int64
+	CompletedAt   int64          `json:"-"`
+	Iterations    int            `json:"-"`
+	ToolCalls     int            `json:"-"`
+	ToolStats     map[string]int `json:"-"`
 }

 type SubagentManager struct {
@ -33,7 +39,6 @@ type SubagentManager struct {
 	workspace      string
 	tools          *ToolRegistry
 	webSearchOpts  WebSearchToolOptions
-	execTool       *ExecTool // Shared exec tool for all presets
 	maxIterations  int
 	maxTokens      int
 	temperature    float64
@ -53,11 +58,6 @@ func NewSubagentManager(
 	if reporter == nil {
 		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{
 		tasks:         make(map[string]*SubagentTask),
 		provider:      provider,
@ -66,7 +66,6 @@ func NewSubagentManager(
 		workspace:     workspace,
 		tools:         NewToolRegistry(),
 		webSearchOpts: webSearchOpts,
-		execTool:      execTool,
 		maxIterations: 10,
 		nextID:        1,
 		reporter:      reporter,
@ -249,14 +248,19 @@ After completing, provide a clear summary of what was done and how it was verifi
 	} else {
 		task.Status = "completed"
 		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
 		sm.reporter.ReportConversation(task.ID, "conductor", loopResult.Content)
 		sm.reporter.ReportGC(task.ID, "completed")
 		result = &ToolResult{
 			ForLLM: fmt.Sprintf(
-				"Subagent '%s' completed (iterations: %d): %s",
+				"Subagent '%s' completed (iterations: %d, tool calls: %d): %s",
 				task.Label,
 				loopResult.Iterations,
+				loopResult.ToolCalls,
 				loopResult.Content,
 			),
 			ForUser: loopResult.Content,
@ -269,14 +273,23 @@ After completing, provide a clear summary of what was done and how it was verifi
 	// Send announce message back to main agent
 	if sm.bus != nil {
 		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)
 		defer pubCancel()
 		sm.bus.PublishInbound(pubCtx, bus.InboundMessage{
 			Channel:  "system",
 			SenderID: fmt.Sprintf("subagent:%s", task.ID),
 			// Format: "original_channel:original_chat_id" for routing back
-			ChatID:  fmt.Sprintf("%s:%s", task.OriginChannel, task.OriginChatID),
-			Content: announceContent,
+			ChatID:   fmt.Sprintf("%s:%s", task.OriginChannel, task.OriginChatID),
+			Content:  announceContent,
+			Metadata: metadata,
 		})
 	}
 }
@ -306,12 +319,22 @@ func (sm *SubagentManager) buildPresetRegistry(preset Preset, writeRoot string)
 		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"] {
-		// Use the shared exec tool but set allow patterns
-		execTool := sm.execTool
+		execWorkDir := writeRoot
+		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 {
-			_ = execTool.SetAllowPatterns([]string{config.ExecPolicy.AllowPattern})
+			execTool.SetAllowRules(config.ExecPolicy.AllowRules)
+			execTool.SetLocalNetOnly(config.ExecPolicy.LocalNetOnly)
 		}
 		registry.Register(execTool)

@ -383,7 +406,7 @@ func (t *SubagentTool) Name() 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 {
@ -411,13 +434,17 @@ func (t *SubagentTool) SetContext(channel, chatID string) {
 func (t *SubagentTool) Execute(ctx context.Context, args map[string]any) *ToolResult {
 	task, ok := args["task"].(string)
 	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)

 	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
@ -477,8 +504,8 @@ func (t *SubagentTool) Execute(ctx context.Context, args map[string]any) *ToolRe
 	if labelStr == "" {
 		labelStr = "(unnamed)"
 	}
-	llmContent := fmt.Sprintf("Subagent task completed:\nLabel: %s\nIterations: %d\nResult: %s",
-		labelStr, loopResult.Iterations, loopResult.Content)
+	llmContent := fmt.Sprintf("Subagent task completed:\nLabel: %s\nIterations: %d\nTool calls: %d\nResult: %s",
+		labelStr, loopResult.Iterations, loopResult.ToolCalls, loopResult.Content)

 	return &ToolResult{
 		ForLLM:  llmContent,
@ -488,3 +515,18 @@ func (t *SubagentTool) Execute(ctx context.Context, args map[string]any) *ToolRe
 		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, ",")
+}
--- a/pkg/tools/subagent_tool_test.go
+++ b/pkg/tools/subagent_tool_test.go
@ -4,6 +4,7 @@ import (
 	"context"
 	"strings"
 	"testing"
+	"time"

 	"github.com/sipeed/picoclaw/pkg/bus"
 	"github.com/sipeed/picoclaw/pkg/orch"
@ -93,8 +94,11 @@ func TestSubagentTool_Description(t *testing.T) {
 	if desc == "" {
 		t.Error("Description should not be empty")
 	}
-	if !strings.Contains(desc, "subagent") {
-		t.Errorf("Description should mention 'subagent', got: %s", desc)
+	if !strings.Contains(desc, "BLOCK") {
+		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")
 	}

-	// ForLLM should contain error message
-	if !strings.Contains(result.ForLLM, "task is required") {
-		t.Errorf("Error message should mention 'task is required', got: %s", result.ForLLM)
+	// ForLLM should contain helpful error with example
+	if !strings.Contains(result.ForLLM, `"task"`) {
+		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
@ -286,8 +293,8 @@ func TestSubagentTool_Execute_NilManager(t *testing.T) {
 		t.Error("Expected error for nil manager")
 	}

-	if !strings.Contains(result.ForLLM, "Subagent manager not configured") {
-		t.Errorf("Error message should mention manager not configured, got: %s", result.ForLLM)
+	if !strings.Contains(result.ForLLM, "not available in this session") {
+		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")
 	}
 }
+
+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")
+	}
+}
--- a/pkg/tools/toolloop.go
+++ b/pkg/tools/toolloop.go
@ -36,6 +36,8 @@ type ToolLoopConfig struct {
 type ToolLoopResult struct {
 	Content    string
 	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.
@ -52,6 +54,8 @@ func RunToolLoop(
 	}

 	iteration := 0
+	totalToolCalls := 0
+	toolStats := map[string]int{}
 	var finalContent string

 	for iteration < config.MaxIterations {
@ -75,7 +79,7 @@ func RunToolLoop(
 			llmOpts = map[string]any{}
 		}
 		// 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)
 		if err != nil {
 			logger.ErrorCF("toolloop", "LLM call failed",
@ -143,7 +147,9 @@ func RunToolLoop(
 					"tool":      tc.Name,
 					"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)
 			var toolResult *ToolResult
@ -172,5 +178,7 @@ func RunToolLoop(
 	return &ToolLoopResult{
 		Content:    finalContent,
 		Iterations: iteration,
+		ToolCalls:  totalToolCalls,
+		ToolStats:  toolStats,
 	}, nil
 }
--- a/pkg/tools/toolloop_reporter_test.go
+++ b/pkg/tools/toolloop_reporter_test.go
@ -17,14 +17,14 @@ type reporterSpy struct {
 }

 type spyCall struct {
-	state string
+	state orch.AgentState
 	tool  string
 }

 func (r *reporterSpy) ReportSpawn(id, label, task string)       {}
 func (r *reporterSpy) ReportConversation(from, to, text 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.calls = append(r.calls, spyCall{state, tool})
 	r.mu.Unlock()
@ -117,7 +117,7 @@ func TestToolLoop_Reporter_WaitingBeforeLLM(t *testing.T) {
 	if len(calls) == 0 {
 		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])
 	}
 }
@ -152,17 +152,62 @@ func TestToolLoop_Reporter_ToolcallOrderedAfterWaiting(t *testing.T) {
 	if len(calls) < 3 {
 		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])
 	}
-	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])
 	}
-	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])
 	}
 }

+// 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
 // orch.Noop satisfies the orch.AgentReporter interface accepted by
 // ToolLoopConfig.Reporter. If Noop ever stops implementing the interface the