From ec6da7a530ce5da48df422b38be697afad36192b Mon Sep 17 00:00:00 2001
From: Achton Smidt Winther <achton@reload.dk>
Date: Tue, 24 Feb 2026 21:39:49 +0100
Subject: [PATCH] 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>
---
 pkg/tools/spawn.go      |  5 +--
 pkg/tools/spawn_test.go | 79 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 2 deletions(-)
 create mode 100644 pkg/tools/spawn_test.go

diff --git a/pkg/tools/spawn.go b/pkg/tools/spawn.go
index 73d385cb0..8b166b41f 100644
--- a/pkg/tools/spawn.go
+++ b/pkg/tools/spawn.go
@@ -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)
diff --git a/pkg/tools/spawn_test.go b/pkg/tools/spawn_test.go
new file mode 100644
index 000000000..0646c82a9
--- /dev/null
+++ b/pkg/tools/spawn_test.go
@@ -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)
+	}
+}