mirror of
https://github.com/sipeed/picoclaw.git
synced 2026-06-12 18:08:54 +00:00
fix: reject empty task in spawn tool (#740)
The spawn tool accepts empty strings as valid task arguments, which causes a subagent to run with no meaningful work. The subagent's completion message is then routed back to the originating channel (e.g. Signal, Discord), where the main agent processes it and may hallucinate an unrelated response that gets sent to users. Validate that the task parameter is non-empty after trimming whitespace. Related: #545 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
committed by
GitHub
parent
fd26fa7459
commit
ec6da7a530
+3
-2
@@ -3,6 +3,7 @@ package tools
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"strings"
|
||||
)
|
||||
|
||||
type SpawnTool struct {
|
||||
@@ -66,8 +67,8 @@ 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 {
|
||||
return ErrorResult("task is required")
|
||||
if !ok || strings.TrimSpace(task) == "" {
|
||||
return ErrorResult("task is required and must be a non-empty string")
|
||||
}
|
||||
|
||||
label, _ := args["label"].(string)
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestSpawnTool_Execute_EmptyTask(t *testing.T) {
|
||||
provider := &MockLLMProvider{}
|
||||
manager := NewSubagentManager(provider, "test-model", "/tmp/test", nil)
|
||||
tool := NewSpawnTool(manager)
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
args map[string]any
|
||||
}{
|
||||
{"empty string", map[string]any{"task": ""}},
|
||||
{"whitespace only", map[string]any{"task": " "}},
|
||||
{"tabs and newlines", map[string]any{"task": "\t\n "}},
|
||||
{"missing task key", map[string]any{"label": "test"}},
|
||||
{"wrong type", map[string]any{"task": 123}},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
result := tool.Execute(ctx, tt.args)
|
||||
if result == nil {
|
||||
t.Fatal("Result should not be nil")
|
||||
}
|
||||
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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSpawnTool_Execute_ValidTask(t *testing.T) {
|
||||
provider := &MockLLMProvider{}
|
||||
manager := NewSubagentManager(provider, "test-model", "/tmp/test", nil)
|
||||
tool := NewSpawnTool(manager)
|
||||
|
||||
ctx := context.Background()
|
||||
args := map[string]any{
|
||||
"task": "Write a haiku about coding",
|
||||
"label": "haiku-task",
|
||||
}
|
||||
|
||||
result := tool.Execute(ctx, args)
|
||||
if result == nil {
|
||||
t.Fatal("Result should not be nil")
|
||||
}
|
||||
if result.IsError {
|
||||
t.Errorf("Expected success for valid task, got error: %s", result.ForLLM)
|
||||
}
|
||||
if !result.Async {
|
||||
t.Error("SpawnTool should return async result")
|
||||
}
|
||||
}
|
||||
|
||||
func TestSpawnTool_Execute_NilManager(t *testing.T) {
|
||||
tool := NewSpawnTool(nil)
|
||||
|
||||
ctx := context.Background()
|
||||
args := map[string]any{"task": "test task"}
|
||||
|
||||
result := tool.Execute(ctx, args)
|
||||
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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user