diff --git a/cmd/picoclaw/internal/onboard/helpers.go b/cmd/picoclaw/internal/onboard/helpers.go index 721d74552..2294cfa4c 100644 --- a/cmd/picoclaw/internal/onboard/helpers.go +++ b/cmd/picoclaw/internal/onboard/helpers.go @@ -146,6 +146,11 @@ func createWorkspaceTemplates(workspace string) { } func copyEmbeddedToTarget(targetDir string) error { + legacyWorkspaceFiles := map[string]struct{}{ + "AGENTS.md": {}, + "IDENTITY.md": {}, + } + // Ensure target directory exists if err := os.MkdirAll(targetDir, 0o755); err != nil { return fmt.Errorf("Failed to create target directory: %w", err) @@ -172,6 +177,9 @@ func copyEmbeddedToTarget(targetDir string) error { if err != nil { return fmt.Errorf("Failed to get relative path for %s: %v\n", path, err) } + if _, skip := legacyWorkspaceFiles[filepath.Base(new_path)]; skip { + return nil + } // Build target file path targetPath := filepath.Join(targetDir, new_path) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 53c245568..5bb597187 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -645,7 +645,7 @@ func (al *AgentLoop) drainBusToSteering(ctx context.Context, activeScope, active if handled, response := al.tryHandlePriorityCommand(ctx, msg); handled { if response != "" { - al.PublishResponseIfNeeded(ctx, msg.Channel, msg.ChatID, response) + al.publishResponse(ctx, msg.Channel, msg.ChatID, response) } continue } @@ -676,20 +676,32 @@ func (al *AgentLoop) Stop() { al.running.Store(false) } +func (al *AgentLoop) resetMessageToolRound(agent *AgentInstance) { + if agent == nil { + return + } + + if tool, ok := agent.Tools.Get("message"); ok { + if resetter, ok := tool.(interface{ ResetSentInRound() }); ok { + resetter.ResetSentInRound() + } + } +} + func (al *AgentLoop) PublishResponseIfNeeded(ctx context.Context, channel, chatID, response string) { if response == "" { return } alreadySentToSameChat := false - defaultAgent := al.GetRegistry().GetDefaultAgent() - if defaultAgent != nil { - if tool, ok := defaultAgent.Tools.Get("message"); ok { - if mt, ok := tool.(*tools.MessageTool); ok { - alreadySentToSameChat = mt.HasSentTo(channel, chatID) - } + al.GetRegistry().ForEachTool("message", func(tool tools.Tool) { + if alreadySentToSameChat { + return } - } + if mt, ok := tool.(*tools.MessageTool); ok && mt.HasSentTo(channel, chatID) { + alreadySentToSameChat = true + } + }) if alreadySentToSameChat { logger.DebugCF( @@ -700,6 +712,14 @@ func (al *AgentLoop) PublishResponseIfNeeded(ctx context.Context, channel, chatI return } + al.publishResponse(ctx, channel, chatID, response) +} + +func (al *AgentLoop) publishResponse(ctx context.Context, channel, chatID, response string) { + if response == "" { + return + } + al.bus.PublishOutbound(ctx, bus.OutboundMessage{ Channel: channel, ChatID: chatID, @@ -1364,20 +1384,35 @@ func (al *AgentLoop) askSideQuestion( } var channel, chatID, senderID, senderDisplayName string + var media []string var activeSkills []string + var history []providers.Message + var summary string if opts != nil { channel = opts.Channel chatID = opts.ChatID senderID = opts.SenderID senderDisplayName = opts.SenderDisplayName + media = append([]string(nil), opts.Media...) activeSkills = activeSkillNames(agent, *opts) + + if !opts.NoHistory { + if resp, err := al.contextManager.Assemble(ctx, &AssembleRequest{ + SessionKey: opts.SessionKey, + Budget: agent.ContextWindow, + MaxTokens: agent.MaxTokens, + }); err == nil && resp != nil { + history = resp.History + summary = resp.Summary + } + } } messages := agent.ContextBuilder.BuildMessages( - nil, - "", + history, + summary, question, - nil, + media, channel, chatID, senderID, @@ -1476,11 +1511,7 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) } // Reset message-tool state for this round so we don't skip publishing due to a previous round. - if tool, ok := agent.Tools.Get("message"); ok { - if resetter, ok := tool.(interface{ ResetSentInRound() }); ok { - resetter.ResetSentInRound() - } - } + al.resetMessageToolRound(agent) // Resolve session key from route, while preserving explicit agent-scoped keys. scopeKey := resolveScopeKey(route, msg.SessionKey) diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go index 7616aa6f6..4d7c0212e 100644 --- a/pkg/agent/loop_test.go +++ b/pkg/agent/loop_test.go @@ -9,6 +9,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "reflect" "slices" "strings" "testing" @@ -244,6 +245,10 @@ func TestProcessMessage_BtwCommandRunsWithoutPersistingHistory(t *testing.T) { msgBus := bus.NewMessageBus() provider := &recordingProvider{} al := NewAgentLoop(cfg, msgBus, provider) + defaultAgent := al.GetRegistry().GetDefaultAgent() + if defaultAgent == nil { + t.Fatal("expected default agent") + } msg := bus.InboundMessage{ Channel: "telegram", @@ -251,6 +256,17 @@ func TestProcessMessage_BtwCommandRunsWithoutPersistingHistory(t *testing.T) { ChatID: "chat-1", Content: "/btw explain side effects", } + route, _, err := al.resolveMessageRoute(msg) + if err != nil { + t.Fatalf("resolveMessageRoute() error = %v", err) + } + sessionKey := resolveScopeKey(route, msg.SessionKey) + initialHistory := []providers.Message{ + {Role: "user", Content: "We decided to avoid global state."}, + {Role: "assistant", Content: "Right, keep it request-scoped."}, + } + defaultAgent.Sessions.SetHistory(sessionKey, initialHistory) + defaultAgent.Sessions.SetSummary(sessionKey, "The team decided to keep state request-scoped.") response, err := al.processMessage(context.Background(), msg) if err != nil { @@ -262,20 +278,88 @@ func TestProcessMessage_BtwCommandRunsWithoutPersistingHistory(t *testing.T) { if len(provider.lastMessages) == 0 { t.Fatal("provider did not receive any messages") } + if len(provider.lastMessages) != 4 { + t.Fatalf("provider messages len = %d, want 4 (system + history + user)", len(provider.lastMessages)) + } + if !strings.Contains(provider.lastMessages[0].Content, "The team decided to keep state request-scoped.") { + t.Fatalf("system prompt missing session summary: %q", provider.lastMessages[0].Content) + } + if provider.lastMessages[1].Content != initialHistory[0].Content || + provider.lastMessages[2].Content != initialHistory[1].Content { + t.Fatalf("provider history = %+v, want seeded session history", provider.lastMessages[1:3]) + } lastMessage := provider.lastMessages[len(provider.lastMessages)-1] if lastMessage.Role != "user" || lastMessage.Content != "explain side effects" { t.Fatalf("last provider message = %+v, want stripped /btw question", lastMessage) } - route, _, err := al.resolveMessageRoute(msg) - if err != nil { - t.Fatalf("resolveMessageRoute() error = %v", err) - } - sessionKey := resolveScopeKey(route, msg.SessionKey) history := al.GetRegistry().GetDefaultAgent().Sessions.GetHistory(sessionKey) - if len(history) != 0 { - t.Fatalf("session history len = %d, want 0 for /btw", len(history)) + if !reflect.DeepEqual(history, initialHistory) { + t.Fatalf("session history = %#v, want %#v", history, initialHistory) + } +} + +func TestPublishResponseIfNeeded_SkipsWhenNonDefaultAgentAlreadySentToChat(t *testing.T) { + tmpDir := t.TempDir() + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: tmpDir, + ModelName: "test-model", + MaxTokens: 4096, + MaxToolIterations: 10, + }, + List: []config.AgentConfig{ + {ID: "main", Default: true}, + {ID: "support"}, + }, + }, + Tools: config.ToolsConfig{ + Message: config.ToolConfig{Enabled: true}, + }, + } + + msgBus := bus.NewMessageBus() + al := NewAgentLoop(cfg, msgBus, &recordingProvider{}) + + supportAgent, ok := al.GetRegistry().GetAgent("support") + if !ok || supportAgent == nil { + t.Fatal("expected support agent") + } + + tool, ok := supportAgent.Tools.Get("message") + if !ok { + t.Fatal("expected message tool on support agent") + } + mt, ok := tool.(*tools.MessageTool) + if !ok { + t.Fatal("expected message tool type") + } + + if result := mt.Execute(context.Background(), map[string]any{ + "channel": "telegram", + "chat_id": "chat-1", + "content": "interim reply", + }); result == nil || result.IsError { + t.Fatalf("message tool setup result = %+v, want successful send", result) + } + + select { + case outbound := <-msgBus.OutboundChan(): + if outbound.Content != "interim reply" { + t.Fatalf("expected setup outbound %q, got %#v", "interim reply", outbound) + } + default: + t.Fatal("expected interim outbound from message tool setup") + } + + al.PublishResponseIfNeeded(context.Background(), "telegram", "chat-1", "final reply") + + select { + case outbound := <-msgBus.OutboundChan(): + t.Fatalf("expected final reply to be suppressed, got outbound %#v", outbound) + default: } } diff --git a/pkg/agent/steering.go b/pkg/agent/steering.go index ad6613e8c..863badd03 100644 --- a/pkg/agent/steering.go +++ b/pkg/agent/steering.go @@ -346,11 +346,7 @@ func (al *AgentLoop) Continue(ctx context.Context, sessionKey, channel, chatID s return "", fmt.Errorf("no agent available for session %q", sessionKey) } - if tool, ok := agent.Tools.Get("message"); ok { - if resetter, ok := tool.(interface{ ResetSentInRound() }); ok { - resetter.ResetSentInRound() - } - } + al.resetMessageToolRound(agent) return al.continueWithSteeringMessages(ctx, agent, sessionKey, channel, chatID, steeringMsgs) } diff --git a/pkg/agent/steering_test.go b/pkg/agent/steering_test.go index fd4364efc..4ca36eef3 100644 --- a/pkg/agent/steering_test.go +++ b/pkg/agent/steering_test.go @@ -1081,6 +1081,29 @@ func TestAgentLoop_Steering_BtwCommandBypassesQueuedTurn(t *testing.T) { t.Fatal("timeout waiting for first LLM call to start") } + messageTool, ok := al.GetRegistry().GetDefaultAgent().Tools.Get("message") + var mt *tools.MessageTool + if !ok { + mt = tools.NewMessageTool() + al.RegisterTool(mt) + } else { + var typeOK bool + mt, typeOK = messageTool.(*tools.MessageTool) + if !typeOK { + t.Fatal("expected message tool type") + } + } + mt.SetSendCallback(func(channel, chatID, content, replyToMessageID string) error { + return nil + }) + if result := mt.Execute(context.Background(), map[string]any{ + "channel": "test", + "chat_id": "chat1", + "content": "already sent from busy turn", + }); result == nil || result.IsError { + t.Fatalf("message tool setup result = %+v, want successful send", result) + } + if err := msgBus.PublishInbound(pubCtx, btw); err != nil { t.Fatalf("publish /btw inbound: %v", err) } @@ -1103,11 +1126,15 @@ func TestAgentLoop_Steering_BtwCommandBypassesQueuedTurn(t *testing.T) { select { case outbound := <-msgBus.OutboundChan(): - if outbound.Content != "long turn finished" { - t.Fatalf("expected original turn response after release, got %q", outbound.Content) - } + t.Fatalf("expected busy turn final response to stay suppressed, got %q", outbound.Content) case <-time.After(2 * time.Second): - t.Fatal("timeout waiting for original turn response") + } + + provider.mu.Lock() + callCount := provider.calls + provider.mu.Unlock() + if callCount != 2 { + t.Fatalf("provider call count = %d, want 2", callCount) } cancelRun()