From e588b8539f8a1c67f665f788c2bfbd15a14d7474 Mon Sep 17 00:00:00 2001 From: dj-oyu <68707227+dj-oyu@users.noreply.github.com> Date: Wed, 25 Feb 2026 15:44:12 +0900 Subject: [PATCH] fix(orch): improve spawn/subagent tool calling adoption for less capable models Replace pseudo-syntax examples with JSON Tool:/Arguments: format in system prompt and orchestration reminders. Add parameter hints to tool summaries, improve error messages with usage examples, and strengthen delegation nudges. Co-Authored-By: Claude Opus 4.6 --- pkg/agent/context.go | 50 +++++++++-------- pkg/agent/loop.go | 12 +++- pkg/tools/registry.go | 42 +++++++++++++- pkg/tools/registry_test.go | 98 +++++++++++++++++++++++++++++++++ pkg/tools/spawn.go | 8 +-- pkg/tools/subagent.go | 8 ++- pkg/tools/subagent_tool_test.go | 20 ++++--- 7 files changed, 195 insertions(+), 43 deletions(-) diff --git a/pkg/agent/context.go b/pkg/agent/context.go index 72cb0ae35..469a0bfdc 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -21,21 +21,29 @@ const orchestrationGuidance = `## Orchestration You are the conductor, not the performer. **Your primary job is to delegate, not to implement.** -### spawn (non-blocking) — default choice -- Tasks that can run in parallel or in the background -- Multiple independent tasks — spawn each one simultaneously -- Long-running operations (implementation, test suites, analysis) -- Any work that involves more than 2-3 tool calls +### spawn (non-blocking) — DEFAULT choice +Returns immediately. Use for any task that can run independently. +Call the spawn tool with JSON arguments like this: -### subagent (blocking) — when you need the answer now -- You need the result before deciding the next step -- Correctness of your next action depends on the outcome +Tool: spawn +Arguments: {"task": "Examine pkg/auth/ and report middleware pattern", "preset": "scout", "label": "auth-scout"} -### Inline — only for trivial operations -- A single fast tool call (read one file, quick search) -- Overhead of delegation clearly outweighs the benefit +Tool: spawn +Arguments: {"task": "Implement rate limiter in pkg/ratelimit/ with tests", "preset": "coder", "label": "rate-limiter"} -### Presets +### 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 | @@ -44,20 +52,14 @@ You are the conductor, not the performer. **Your primary job is to delegate, not | worker | build + install | yes (sandbox) | build/package mgr | | coordinator | orchestrate others | yes (sandbox) | general + spawn | -### Examples +### Parallel spawning +Spawn multiple independent tasks at once — do NOT wait between them: -Investigate code structure: - spawn(task: "Examine pkg/auth/ and report middleware pattern and entry points", preset: "scout", label: "auth-scout") +Tool: spawn +Arguments: {"task": "Analyze error handling patterns in pkg/providers/", "preset": "analyst", "label": "error-patterns"} -Implement a feature: - spawn(task: "Implement rate limiter in pkg/ratelimit/ with tests. Run go test to verify.", preset: "coder", label: "rate-limiter") - -Get a blocking answer: - subagent(task: "Read pkg/config/config.go and list all SubagentsConfig fields", label: "config-check") - -Parallel exploration: - spawn(task: "Analyze error handling patterns in pkg/providers/", preset: "analyst", label: "error-patterns") - spawn(task: "List all HTTP endpoints in pkg/miniapp/", preset: "scout", label: "endpoints") +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 findings and decide the next fork.` diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index ecaa35ecb..bcb302c62 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -1270,9 +1270,15 @@ func buildOrchReminder(iteration int) (providers.Message, bool) { if iteration != 1 && iteration%3 != 0 { return providers.Message{}, false } - content := "[System] ORCHESTRATION mode active. Delegate plan steps to subagents using spawn (async) or subagent (blocking). " + - "Do NOT implement steps inline unless they are trivial single-tool-call tasks. " + - "Spawn multiple independent steps in parallel for maximum throughput." + 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 } diff --git a/pkg/tools/registry.go b/pkg/tools/registry.go index 111f3c361..6fa7b0dea 100644 --- 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 } diff --git a/pkg/tools/registry_test.go b/pkg/tools/registry_test.go index 788a4935a..099b2baeb 100644 --- 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) diff --git a/pkg/tools/spawn.go b/pkg/tools/spawn.go index 7ef9ca489..fe6f53c35 100644 --- 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,7 +73,7 @@ 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) @@ -87,12 +87,12 @@ func (t *SpawnTool) Execute(ctx context.Context, args map[string]any) *ToolResul } if checkTarget != "" && t.allowlistCheck != nil { if !t.allowlistCheck(checkTarget) { - return ErrorResult(fmt.Sprintf("not allowed to spawn agent '%s' or preset '%s'", agentID, preset)) + return ErrorResult(fmt.Sprintf("preset %q is not allowed. 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 diff --git a/pkg/tools/subagent.go b/pkg/tools/subagent.go index bfe8436f8..41542791c 100644 --- a/pkg/tools/subagent.go +++ b/pkg/tools/subagent.go @@ -384,7 +384,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 { @@ -412,13 +412,15 @@ 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 diff --git a/pkg/tools/subagent_tool_test.go b/pkg/tools/subagent_tool_test.go index 6291724dc..43d5a8fc7 100644 --- a/pkg/tools/subagent_tool_test.go +++ b/pkg/tools/subagent_tool_test.go @@ -93,8 +93,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 +262,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 +292,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) } }