diff --git a/pkg/orch/reporter_test.go b/pkg/orch/reporter_test.go new file mode 100644 index 000000000..120f03c26 --- /dev/null +++ b/pkg/orch/reporter_test.go @@ -0,0 +1,102 @@ +package orch + +import "testing" + +// Compile-time: Broadcaster must satisfy AgentReporter. +var _ AgentReporter = (*Broadcaster)(nil) + +// TestNoop_AllMethods_NoPanic verifies that orch.Noop can be called for all +// four methods without panic. This is the nil-safe baseline for disabled +// orchestration mode. +func TestNoop_AllMethods_NoPanic(t *testing.T) { + Noop.ReportSpawn("id", "label", "task") + Noop.ReportStateChange("id", "waiting", "") + Noop.ReportStateChange("id", "toolcall", "bash") + Noop.ReportConversation("conductor", "sub-1", "do something") + Noop.ReportGC("id", "completed") +} + +// TestBroadcaster_ReportSpawn_MapsToAgentSpawnEvent verifies that ReportSpawn +// publishes an Event with Type="agent_spawn" and the correct ID/Label/Task +// fields, and that the agent appears in the Snapshot immediately. +func TestBroadcaster_ReportSpawn_MapsToAgentSpawnEvent(t *testing.T) { + b := NewBroadcaster() + sub := b.Subscribe() + defer b.Unsubscribe(sub) + + b.ReportSpawn("agent-1", "scout", "find all TODOs") + + ev := <-sub.Ch + if ev.Type != "agent_spawn" { + t.Fatalf("want agent_spawn, got %q", ev.Type) + } + if ev.ID != "agent-1" || ev.Label != "scout" || ev.Task != "find all TODOs" { + t.Fatalf("field mismatch: %+v", ev) + } + snap := b.Snapshot() + if len(snap) != 1 || snap[0].ID != "agent-1" || snap[0].Label != "scout" { + t.Fatalf("snapshot not updated correctly: %v", snap) + } +} + +// TestBroadcaster_ReportStateChange_MapsToAgentStateEvent verifies that +// ReportStateChange publishes agent_state and updates the live snapshot. +func TestBroadcaster_ReportStateChange_MapsToAgentStateEvent(t *testing.T) { + b := NewBroadcaster() + sub := b.Subscribe() + defer b.Unsubscribe(sub) + + b.ReportSpawn("agent-1", "coder", "implement it") + <-sub.Ch // consume spawn + + b.ReportStateChange("agent-1", "toolcall", "bash") + ev := <-sub.Ch + if ev.Type != "agent_state" || ev.State != "toolcall" || ev.Tool != "bash" { + t.Fatalf("unexpected event: %+v", ev) + } + snap := b.Snapshot() + if snap[0].State != "toolcall" || snap[0].Tool != "bash" { + t.Fatalf("snapshot state not updated: %v", snap) + } +} + +// TestBroadcaster_ReportConversation_MapsToConversationEvent verifies that +// ReportConversation publishes a conversation event with correct From/To/Text +// fields and does NOT modify the agent snapshot (conversation is not a state +// change of any agent). +func TestBroadcaster_ReportConversation_MapsToConversationEvent(t *testing.T) { + b := NewBroadcaster() + sub := b.Subscribe() + defer b.Unsubscribe(sub) + + b.ReportConversation("conductor", "sub-1", "please do the task") + + ev := <-sub.Ch + if ev.Type != "conversation" || ev.From != "conductor" || ev.To != "sub-1" || ev.Text != "please do the task" { + t.Fatalf("unexpected event: %+v", ev) + } + if len(b.Snapshot()) != 0 { + t.Fatal("conversation event must not modify agent snapshot") + } +} + +// TestBroadcaster_ReportGC_RemovesAgentFromSnapshot verifies that ReportGC +// publishes agent_gc with the correct Reason and removes the agent from the +// live snapshot so new WS connections no longer see it. +func TestBroadcaster_ReportGC_RemovesAgentFromSnapshot(t *testing.T) { + b := NewBroadcaster() + sub := b.Subscribe() + defer b.Unsubscribe(sub) + + b.ReportSpawn("agent-1", "scout", "task") + <-sub.Ch // consume spawn + + b.ReportGC("agent-1", "completed") + ev := <-sub.Ch + if ev.Type != "agent_gc" || ev.ID != "agent-1" || ev.Reason != "completed" { + t.Fatalf("unexpected event: %+v", ev) + } + if len(b.Snapshot()) != 0 { + t.Fatal("agent must be removed from snapshot after ReportGC") + } +} diff --git a/pkg/tools/subagent_reporter_test.go b/pkg/tools/subagent_reporter_test.go new file mode 100644 index 000000000..5ff68f4e4 --- /dev/null +++ b/pkg/tools/subagent_reporter_test.go @@ -0,0 +1,153 @@ +package tools + +import ( + "context" + "sync/atomic" + "testing" + "time" + + "github.com/sipeed/picoclaw/pkg/orch" +) + +// TestSubagentManager_Spawn_EmitsLifecycleEvents verifies that Spawn() fires +// the correct sequence of orchestration events through a real Broadcaster: +// +// agent_spawn → conversation(conductor→sub) → agent_state(waiting) → +// conversation(sub→conductor) → agent_gc(completed) +// +// It also verifies that the snapshot is empty after ReportGC and that the +// completion callback is invoked. +func TestSubagentManager_Spawn_EmitsLifecycleEvents(t *testing.T) { + b := orch.NewBroadcaster() + sub := b.Subscribe() + defer b.Unsubscribe(sub) + + provider := &MockLLMProvider{} + mgr := NewSubagentManager(provider, "test-model", "/tmp/test", nil, b) + + var callbackCalled int32 + cb := AsyncCallback(func(_ context.Context, _ *ToolResult) { + atomic.StoreInt32(&callbackCalled, 1) + }) + + _, err := mgr.Spawn( + context.Background(), + "say hello", "hello-task", "", "cli", "direct", + cb, + ) + if err != nil { + t.Fatalf("Spawn() error: %v", err) + } + + // Collect events until agent_gc or timeout. + var events []orch.Event + deadline := time.After(3 * time.Second) +loop: + for { + select { + case ev := <-sub.Ch: + events = append(events, ev) + if ev.Type == "agent_gc" { + break loop + } + case <-deadline: + t.Fatalf("timed out waiting for agent_gc; events so far: %+v", events) + } + } + + // 1. First event must be agent_spawn with the correct label. + if len(events) == 0 || events[0].Type != "agent_spawn" { + t.Fatalf("first event must be agent_spawn, got: %+v", events) + } + if events[0].Label != "hello-task" { + t.Errorf("agent_spawn label = %q, want %q", events[0].Label, "hello-task") + } + spawnedID := events[0].ID + + // 2. There must be a conversation from conductor → subagent. + var hasConvToSub bool + for _, ev := range events { + if ev.Type == "conversation" && ev.From == "conductor" && ev.To == spawnedID { + hasConvToSub = true + break + } + } + if !hasConvToSub { + t.Errorf("missing conversation(conductor → %s); events: %+v", spawnedID, events) + } + + // 3. There must be at least one agent_state(waiting) for the subagent. + var hasWaiting bool + for _, ev := range events { + if ev.Type == "agent_state" && ev.ID == spawnedID && ev.State == "waiting" { + hasWaiting = true + break + } + } + if !hasWaiting { + t.Errorf("missing agent_state(waiting) for %s; events: %+v", spawnedID, events) + } + + // 4. Last event must be agent_gc with reason "completed". + last := events[len(events)-1] + if last.Type != "agent_gc" || last.ID != spawnedID || last.Reason != "completed" { + t.Errorf("last event must be agent_gc(completed), got: %+v", last) + } + + // 5. Snapshot must be empty after GC (agent removed from live map). + if snap := b.Snapshot(); len(snap) != 0 { + t.Errorf("snapshot must be empty after agent_gc, got: %v", snap) + } + + // 6. Callback must be called. The callback fires in the same goroutine + // as ReportGC (after the deferred unlock), so we poll briefly. + for i := 0; i < 100; i++ { + if atomic.LoadInt32(&callbackCalled) == 1 { + break + } + time.Sleep(10 * time.Millisecond) + } + if atomic.LoadInt32(&callbackCalled) != 1 { + t.Error("completion callback was not called after agent_gc") + } +} + +// TestSubagentManager_Spawn_SnapshotLiveDuringExecution verifies that the +// Broadcaster snapshot contains the agent between agent_spawn and agent_gc. +// Because Publish() updates the agent map before dispatching to subscribers, +// the snapshot is guaranteed to be non-empty as soon as agent_spawn is +// received on the channel. +func TestSubagentManager_Spawn_SnapshotLiveDuringExecution(t *testing.T) { + b := orch.NewBroadcaster() + sub := b.Subscribe() + defer b.Unsubscribe(sub) + + provider := &MockLLMProvider{} + mgr := NewSubagentManager(provider, "test-model", "/tmp/test", nil, b) + + _, err := mgr.Spawn( + context.Background(), + "any task", "live-test", "", "cli", "direct", + nil, + ) + if err != nil { + t.Fatalf("Spawn() error: %v", err) + } + + // Wait for agent_spawn, then immediately check snapshot. + deadline := time.After(2 * time.Second) + for { + select { + case ev := <-sub.Ch: + if ev.Type == "agent_spawn" { + snap := b.Snapshot() + if len(snap) == 0 { + t.Error("snapshot must contain the spawned agent after agent_spawn event") + } + return // test complete; background goroutine drains safely + } + case <-deadline: + t.Fatal("timed out waiting for agent_spawn event") + } + } +} diff --git a/pkg/tools/toolloop_reporter_test.go b/pkg/tools/toolloop_reporter_test.go new file mode 100644 index 000000000..38284b722 --- /dev/null +++ b/pkg/tools/toolloop_reporter_test.go @@ -0,0 +1,170 @@ +package tools + +import ( + "context" + "sync" + "testing" + + "github.com/sipeed/picoclaw/pkg/orch" + "github.com/sipeed/picoclaw/pkg/providers" +) + +// reporterSpy records every ReportStateChange call in order. +// Spawn/Conversation/GC are not needed for toolloop tests. +type reporterSpy struct { + mu sync.Mutex + calls []spyCall +} + +type spyCall struct { + state string + 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) { + r.mu.Lock() + r.calls = append(r.calls, spyCall{state, tool}) + r.mu.Unlock() +} +func (r *reporterSpy) snapshot() []spyCall { + r.mu.Lock() + defer r.mu.Unlock() + out := make([]spyCall, len(r.calls)) + copy(out, r.calls) + return out +} + +// sequenceMockProvider returns a tool call on the first Chat() call and a +// plain text response on all subsequent calls. Used to exercise the +// waiting → toolcall → waiting event sequence in RunToolLoop. +type sequenceMockProvider struct { + mu sync.Mutex + callCount int +} + +func (m *sequenceMockProvider) Chat( + _ context.Context, + _ []providers.Message, + _ []providers.ToolDefinition, + _ string, + _ map[string]any, +) (*providers.LLMResponse, error) { + m.mu.Lock() + m.callCount++ + n := m.callCount + m.mu.Unlock() + if n == 1 { + return &providers.LLMResponse{ + ToolCalls: []providers.ToolCall{ + {ID: "tc-1", Name: "echo_tool", Arguments: map[string]any{"msg": "hi"}}, + }, + }, nil + } + return &providers.LLMResponse{Content: "done"}, nil +} +func (m *sequenceMockProvider) GetDefaultModel() string { return "test" } +func (m *sequenceMockProvider) SupportsTools() bool { return true } +func (m *sequenceMockProvider) GetContextWindow() int { return 4096 } + +// echoTool is a minimal Tool stub registered as "echo_tool". +type echoTool struct{} + +func (t *echoTool) Name() string { return "echo_tool" } +func (t *echoTool) Description() string { return "echo" } +func (t *echoTool) Parameters() map[string]any { + return map[string]any{"type": "object", "properties": map[string]any{}} +} +func (t *echoTool) Execute(_ context.Context, _ map[string]any) *ToolResult { + return &ToolResult{ForLLM: "echoed"} +} + +// TestToolLoop_NilReporter_FallsBackToNoop ensures that passing nil as +// Reporter does not panic — the loop must substitute orch.Noop internally. +func TestToolLoop_NilReporter_FallsBackToNoop(t *testing.T) { + _, err := RunToolLoop(context.Background(), ToolLoopConfig{ + Provider: &MockLLMProvider{}, + Model: "test", + MaxIterations: 1, + Reporter: nil, // must not panic + }, []providers.Message{{Role: "user", Content: "hi"}}, "cli", "direct") + if err != nil { + t.Fatalf("unexpected error with nil reporter: %v", err) + } +} + +// TestToolLoop_Reporter_WaitingBeforeLLM verifies that ReportStateChange is +// called with state="waiting" before the first LLM call. The mock provider +// returns a direct text answer (no tool calls), so exactly one waiting event +// is expected. +func TestToolLoop_Reporter_WaitingBeforeLLM(t *testing.T) { + rep := &reporterSpy{} + _, err := RunToolLoop(context.Background(), ToolLoopConfig{ + Provider: &MockLLMProvider{}, + Model: "test", + MaxIterations: 1, + Reporter: rep, + AgentID: "sess-1", + }, []providers.Message{{Role: "user", Content: "hi"}}, "cli", "direct") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + calls := rep.snapshot() + if len(calls) == 0 { + t.Fatal("expected at least one ReportStateChange call") + } + if calls[0].state != "waiting" { + t.Fatalf("first call must be state=waiting, got %+v", calls[0]) + } +} + +// TestToolLoop_Reporter_ToolcallOrderedAfterWaiting verifies the canonical +// two-iteration sequence: +// +// waiting (before 1st LLM call) +// toolcall(echo_tool) (before tool execution) +// waiting (before 2nd LLM call) +// +// The sequenceMockProvider returns a tool call on iteration 1 and a text +// response on iteration 2, driving exactly this path. +func TestToolLoop_Reporter_ToolcallOrderedAfterWaiting(t *testing.T) { + rep := &reporterSpy{} + reg := NewToolRegistry() + reg.Register(&echoTool{}) + + _, err := RunToolLoop(context.Background(), ToolLoopConfig{ + Provider: &sequenceMockProvider{}, + Model: "test", + Tools: reg, + MaxIterations: 5, + Reporter: rep, + AgentID: "sess-1", + }, []providers.Message{{Role: "user", Content: "do it"}}, "cli", "direct") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + calls := rep.snapshot() + if len(calls) < 3 { + t.Fatalf("expected at least 3 calls, got %d: %+v", len(calls), calls) + } + if calls[0].state != "waiting" { + t.Fatalf("calls[0] must be waiting, got %+v", calls[0]) + } + if calls[1].state != "toolcall" || calls[1].tool != "echo_tool" { + t.Fatalf("calls[1] must be toolcall(echo_tool), got %+v", calls[1]) + } + if calls[2].state != "waiting" { + t.Fatalf("calls[2] must be waiting (2nd LLM iteration), got %+v", calls[2]) + } +} + +// 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 +// build will fail here before any test runs. +func TestToolLoop_Reporter_NoopImplementsInterface(t *testing.T) { + var _ orch.AgentReporter = orch.Noop +}