Merge branch 'main' of https://github.com/sipeed/picoclaw

pkg/agent/context.go

@@ -170,8 +170,8 @@ func (cb *ContextBuilder) BuildMessages(history []providers.Message, summary str
 	// Log system prompt summary for debugging (debug mode only)
 	logger.DebugCF("agent", "System prompt built",
 		map[string]interface{}{
-			"total_chars": len(systemPrompt),
-			"total_lines": strings.Count(systemPrompt, "\n") + 1,
+			"total_chars":   len(systemPrompt),
+			"total_lines":   strings.Count(systemPrompt, "\n") + 1,
 			"section_count": strings.Count(systemPrompt, "\n\n---\n\n") + 1,
 		})
 
@@ -193,9 +193,9 @@ func (cb *ContextBuilder) BuildMessages(history []providers.Message, summary str
 	// --- INICIO DEL FIX ---
 	//Diegox-17
 	for len(history) > 0 && (history[0].Role == "tool") {
-    	logger.DebugCF("agent", "Removing orphaned tool message from history to prevent LLM error", 
-        	map[string]interface{}{"role": history[0].Role})
-    	history = history[1:]
+		logger.DebugCF("agent", "Removing orphaned tool message from history to prevent LLM error",
+			map[string]interface{}{"role": history[0].Role})
+		history = history[1:]
 	}
 	//Diegox-17
 	// --- FIN DEL FIX ---

pkg/agent/loop.go

@@ -19,9 +19,11 @@ import (
 
 	"github.com/sipeed/picoclaw/pkg/bus"
 	"github.com/sipeed/picoclaw/pkg/config"
+	"github.com/sipeed/picoclaw/pkg/constants"
 	"github.com/sipeed/picoclaw/pkg/logger"
 	"github.com/sipeed/picoclaw/pkg/providers"
 	"github.com/sipeed/picoclaw/pkg/session"
+	"github.com/sipeed/picoclaw/pkg/state"
 	"github.com/sipeed/picoclaw/pkg/tools"
 	"github.com/sipeed/picoclaw/pkg/utils"
 )
@@ -31,13 +33,14 @@ type AgentLoop struct {
 	provider       providers.LLMProvider
 	workspace      string
 	model          string
-	contextWindow  int           // Maximum context window size in tokens
+	contextWindow  int // Maximum context window size in tokens
 	maxIterations  int
 	sessions       *session.SessionManager
+	state          *state.Manager
 	contextBuilder *ContextBuilder
 	tools          *tools.ToolRegistry
 	running        atomic.Bool
-	summarizing    sync.Map      // Tracks which sessions are currently being summarized
+	summarizing    sync.Map // Tracks which sessions are currently being summarized
 }
 
 // processOptions configures how a message is processed
@@ -49,19 +52,23 @@ type processOptions struct {
 	DefaultResponse string // Response when LLM returns empty
 	EnableSummary   bool   // Whether to trigger summarization
 	SendResponse    bool   // Whether to send response via bus
+	NoHistory       bool   // If true, don't load session history (for heartbeat)
 }
 
-func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers.LLMProvider) *AgentLoop {
-	workspace := cfg.WorkspacePath()
-	os.MkdirAll(workspace, 0755)
+// createToolRegistry creates a tool registry with common tools.
+// This is shared between main agent and subagents.
+func createToolRegistry(workspace string, restrict bool, cfg *config.Config, msgBus *bus.MessageBus) *tools.ToolRegistry {
+	registry := tools.NewToolRegistry()
 
-	restrict := cfg.Agents.Defaults.RestrictToWorkspace
+	// File system tools
+	registry.Register(tools.NewReadFileTool(workspace, restrict))
+	registry.Register(tools.NewWriteFileTool(workspace, restrict))
+	registry.Register(tools.NewListDirTool(workspace, restrict))
+	registry.Register(tools.NewEditFileTool(workspace, restrict))
+	registry.Register(tools.NewAppendFileTool(workspace, restrict))
 
-	toolsRegistry := tools.NewToolRegistry()
-	toolsRegistry.Register(tools.NewReadFileTool(workspace, restrict))
-	toolsRegistry.Register(tools.NewWriteFileTool(workspace, restrict))
-	toolsRegistry.Register(tools.NewListDirTool(workspace, restrict))
-	toolsRegistry.Register(tools.NewExecTool(workspace, restrict))
+	// Shell execution
+	registry.Register(tools.NewExecTool(workspace, restrict))
 
 	if searchTool := tools.NewWebSearchTool(tools.WebSearchToolOptions{
 		BraveAPIKey:          cfg.Tools.Web.Brave.APIKey,
@@ -74,7 +81,8 @@ func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers
 	}
 	toolsRegistry.Register(tools.NewWebFetchTool(50000))
 
-	// Register message tool
+	// Message tool - available to both agent and subagent
+	// Subagent uses it to communicate directly with user
 	messageTool := tools.NewMessageTool()
 	messageTool.SetSendCallback(func(channel, chatID, content string) error {
 		msgBus.PublishOutbound(bus.OutboundMessage{
@@ -84,20 +92,39 @@ func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers
 		})
 		return nil
 	})
-	toolsRegistry.Register(messageTool)
+	registry.Register(messageTool)
 
-	// Register spawn tool
-	subagentManager := tools.NewSubagentManager(provider, workspace, msgBus)
+	return registry
+}
+
+func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers.LLMProvider) *AgentLoop {
+	workspace := cfg.WorkspacePath()
+	os.MkdirAll(workspace, 0755)
+
+	restrict := cfg.Agents.Defaults.RestrictToWorkspace
+
+	// Create tool registry for main agent
+	toolsRegistry := createToolRegistry(workspace, restrict, cfg, msgBus)
+
+	// Create subagent manager with its own tool registry
+	subagentManager := tools.NewSubagentManager(provider, cfg.Agents.Defaults.Model, workspace, msgBus)
+	subagentTools := createToolRegistry(workspace, restrict, cfg, msgBus)
+	// Subagent doesn't need spawn/subagent tools to avoid recursion
+	subagentManager.SetTools(subagentTools)
+
+	// Register spawn tool (for main agent)
 	spawnTool := tools.NewSpawnTool(subagentManager)
 	toolsRegistry.Register(spawnTool)
 
-	// Register edit file tool
-	editFileTool := tools.NewEditFileTool(workspace, restrict)
-	toolsRegistry.Register(editFileTool)
-	toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict))
+	// Register subagent tool (synchronous execution)
+	subagentTool := tools.NewSubagentTool(subagentManager)
+	toolsRegistry.Register(subagentTool)
 
 	sessionsManager := session.NewSessionManager(filepath.Join(workspace, "sessions"))
 
+	// Create state manager for atomic state persistence
+	stateManager := state.NewManager(workspace)
+
 	// Create context builder and set tools registry
 	contextBuilder := NewContextBuilder(workspace)
 	contextBuilder.SetToolsRegistry(toolsRegistry)
@@ -110,6 +137,7 @@ func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers
 		contextWindow:  cfg.Agents.Defaults.MaxTokens, // Restore context window for summarization
 		maxIterations:  cfg.Agents.Defaults.MaxToolIterations,
 		sessions:       sessionsManager,
+		state:          stateManager,
 		contextBuilder: contextBuilder,
 		tools:          toolsRegistry,
 		summarizing:    sync.Map{},
@@ -135,11 +163,22 @@ func (al *AgentLoop) Run(ctx context.Context) error {
 			}
 
 			if response != "" {
-				al.bus.PublishOutbound(bus.OutboundMessage{
-					Channel: msg.Channel,
-					ChatID:  msg.ChatID,
-					Content: response,
-				})
+				// Check if the message tool already sent a response during this round.
+				// If so, skip publishing to avoid duplicate messages to the user.
+				alreadySent := false
+				if tool, ok := al.tools.Get("message"); ok {
+					if mt, ok := tool.(*tools.MessageTool); ok {
+						alreadySent = mt.HasSentInRound()
+					}
+				}
+
+				if !alreadySent {
+					al.bus.PublishOutbound(bus.OutboundMessage{
+						Channel: msg.Channel,
+						ChatID:  msg.ChatID,
+						Content: response,
+					})
+				}
 			}
 		}
 	}
@@ -155,6 +194,18 @@ func (al *AgentLoop) RegisterTool(tool tools.Tool) {
 	al.tools.Register(tool)
 }
 
+// RecordLastChannel records the last active channel for this workspace.
+// This uses the atomic state save mechanism to prevent data loss on crash.
+func (al *AgentLoop) RecordLastChannel(channel string) error {
+	return al.state.SetLastChannel(channel)
+}
+
+// RecordLastChatID records the last active chat ID for this workspace.
+// This uses the atomic state save mechanism to prevent data loss on crash.
+func (al *AgentLoop) RecordLastChatID(chatID string) error {
+	return al.state.SetLastChatID(chatID)
+}
+
 func (al *AgentLoop) ProcessDirect(ctx context.Context, content, sessionKey string) (string, error) {
 	return al.ProcessDirectWithChannel(ctx, content, sessionKey, "cli", "direct")
 }
@@ -171,10 +222,30 @@ func (al *AgentLoop) ProcessDirectWithChannel(ctx context.Context, content, sess
 	return al.processMessage(ctx, msg)
 }
 
+// ProcessHeartbeat processes a heartbeat request without session history.
+// Each heartbeat is independent and doesn't accumulate context.
+func (al *AgentLoop) ProcessHeartbeat(ctx context.Context, content, channel, chatID string) (string, error) {
+	return al.runAgentLoop(ctx, processOptions{
+		SessionKey:      "heartbeat",
+		Channel:         channel,
+		ChatID:          chatID,
+		UserMessage:     content,
+		DefaultResponse: "I've completed processing but have no response to give.",
+		EnableSummary:   false,
+		SendResponse:    false,
+		NoHistory:       true, // Don't load session history for heartbeat
+	})
+}
+
 func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) (string, error) {
-	// Add message preview to log
-	preview := utils.Truncate(msg.Content, 80)
-	logger.InfoCF("agent", fmt.Sprintf("Processing message from %s:%s: %s", msg.Channel, msg.SenderID, preview),
+	// Add message preview to log (show full content for error messages)
+	var logContent string
+	if strings.Contains(msg.Content, "Error:") || strings.Contains(msg.Content, "error") {
+		logContent = msg.Content // Full content for errors
+	} else {
+		logContent = utils.Truncate(msg.Content, 80)
+	}
+	logger.InfoCF("agent", fmt.Sprintf("Processing message from %s:%s: %s", msg.Channel, msg.SenderID, logContent),
 		map[string]interface{}{
 			"channel":     msg.Channel,
 			"chat_id":     msg.ChatID,
@@ -211,41 +282,70 @@ func (al *AgentLoop) processSystemMessage(ctx context.Context, msg bus.InboundMe
 			"chat_id":   msg.ChatID,
 		})
 
-	// Parse origin from chat_id (format: "channel:chat_id")
-	var originChannel, originChatID string
+	// Parse origin channel from chat_id (format: "channel:chat_id")
+	var originChannel string
 	if idx := strings.Index(msg.ChatID, ":"); idx > 0 {
 		originChannel = msg.ChatID[:idx]
-		originChatID = msg.ChatID[idx+1:]
 	} else {
 		// Fallback
 		originChannel = "cli"
-		originChatID = msg.ChatID
 	}
 
-	// Use the origin session for context
-	sessionKey := fmt.Sprintf("%s:%s", originChannel, originChatID)
+	// Extract subagent result from message content
+	// Format: "Task 'label' completed.\n\nResult:\n<actual content>"
+	content := msg.Content
+	if idx := strings.Index(content, "Result:\n"); idx >= 0 {
+		content = content[idx+8:] // Extract just the result part
+	}
 
-	// Process as system message with routing back to origin
-	return al.runAgentLoop(ctx, processOptions{
-		SessionKey:      sessionKey,
-		Channel:         originChannel,
-		ChatID:          originChatID,
-		UserMessage:     fmt.Sprintf("[System: %s] %s", msg.SenderID, msg.Content),
-		DefaultResponse: "Background task completed.",
-		EnableSummary:   false,
-		SendResponse:    true, // Send response back to original channel
-	})
+	// Skip internal channels - only log, don't send to user
+	if constants.IsInternalChannel(originChannel) {
+		logger.InfoCF("agent", "Subagent completed (internal channel)",
+			map[string]interface{}{
+				"sender_id":   msg.SenderID,
+				"content_len": len(content),
+				"channel":     originChannel,
+			})
+		return "", nil
+	}
+
+	// Agent acts as dispatcher only - subagent handles user interaction via message tool
+	// Don't forward result here, subagent should use message tool to communicate with user
+	logger.InfoCF("agent", "Subagent completed",
+		map[string]interface{}{
+			"sender_id":   msg.SenderID,
+			"channel":     originChannel,
+			"content_len": len(content),
+		})
+
+	// Agent only logs, does not respond to user
+	return "", nil
 }
 
 // runAgentLoop is the core message processing logic.
 // It handles context building, LLM calls, tool execution, and response handling.
 func (al *AgentLoop) runAgentLoop(ctx context.Context, opts processOptions) (string, error) {
+	// 0. Record last channel for heartbeat notifications (skip internal channels)
+	if opts.Channel != "" && opts.ChatID != "" {
+		// Don't record internal channels (cli, system, subagent)
+		if !constants.IsInternalChannel(opts.Channel) {
+			channelKey := fmt.Sprintf("%s:%s", opts.Channel, opts.ChatID)
+			if err := al.RecordLastChannel(channelKey); err != nil {
+				logger.WarnCF("agent", "Failed to record last channel: %v", map[string]interface{}{"error": err.Error()})
+			}
+		}
+	}
+
 	// 1. Update tool contexts
 	al.updateToolContexts(opts.Channel, opts.ChatID)
 
-	// 2. Build messages
-	history := al.sessions.GetHistory(opts.SessionKey)
-	summary := al.sessions.GetSummary(opts.SessionKey)
+	// 2. Build messages (skip history for heartbeat)
+	var history []providers.Message
+	var summary string
+	if !opts.NoHistory {
+		history = al.sessions.GetHistory(opts.SessionKey)
+		summary = al.sessions.GetSummary(opts.SessionKey)
+	}
 	messages := al.contextBuilder.BuildMessages(
 		history,
 		summary,
@@ -264,6 +364,9 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, opts processOptions) (str
 		return "", err
 	}
 
+	// If last tool had ForUser content and we already sent it, we might not need to send final response
+	// This is controlled by the tool's Silent flag and ForUser content
+
 	// 5. Handle empty response
 	if finalContent == "" {
 		finalContent = opts.DefaultResponse
@@ -315,18 +418,7 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, messages []providers.M
 			})
 
 		// Build tool definitions
-		toolDefs := al.tools.GetDefinitions()
-		providerToolDefs := make([]providers.ToolDefinition, 0, len(toolDefs))
-		for _, td := range toolDefs {
-			providerToolDefs = append(providerToolDefs, providers.ToolDefinition{
-				Type: td["type"].(string),
-				Function: providers.ToolFunctionDefinition{
-					Name:        td["function"].(map[string]interface{})["name"].(string),
-					Description: td["function"].(map[string]interface{})["description"].(string),
-					Parameters:  td["function"].(map[string]interface{})["parameters"].(map[string]interface{}),
-				},
-			})
-		}
+		providerToolDefs := al.tools.ToProviderDefs()
 
 		// Log LLM request details
 		logger.DebugCF("agent", "LLM request",
@@ -382,7 +474,7 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, messages []providers.M
 		logger.InfoCF("agent", "LLM requested tool calls",
 			map[string]interface{}{
 				"tools":     toolNames,
-				"count":     len(toolNames),
+				"count":     len(response.ToolCalls),
 				"iteration": iteration,
 			})
 
@@ -418,14 +510,47 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, messages []providers.M
 					"iteration": iteration,
 				})
 
-			result, err := al.tools.ExecuteWithContext(ctx, tc.Name, tc.Arguments, opts.Channel, opts.ChatID)
-			if err != nil {
-				result = fmt.Sprintf("Error: %v", err)
+			// Create async callback for tools that implement AsyncTool
+			// NOTE: Following openclaw's design, async tools do NOT send results directly to users.
+			// Instead, they notify the agent via PublishInbound, and the agent decides
+			// whether to forward the result to the user (in processSystemMessage).
+			asyncCallback := func(callbackCtx context.Context, result *tools.ToolResult) {
+				// Log the async completion but don't send directly to user
+				// The agent will handle user notification via processSystemMessage
+				if !result.Silent && result.ForUser != "" {
+					logger.InfoCF("agent", "Async tool completed, agent will handle notification",
+						map[string]interface{}{
+							"tool":        tc.Name,
+							"content_len": len(result.ForUser),
+						})
+				}
+			}
+
+			toolResult := al.tools.ExecuteWithContext(ctx, tc.Name, tc.Arguments, opts.Channel, opts.ChatID, asyncCallback)
+
+			// Send ForUser content to user immediately if not Silent
+			if !toolResult.Silent && toolResult.ForUser != "" && opts.SendResponse {
+				al.bus.PublishOutbound(bus.OutboundMessage{
+					Channel: opts.Channel,
+					ChatID:  opts.ChatID,
+					Content: toolResult.ForUser,
+				})
+				logger.DebugCF("agent", "Sent tool result to user",
+					map[string]interface{}{
+						"tool":        tc.Name,
+						"content_len": len(toolResult.ForUser),
+					})
+			}
+
+			// Determine content for LLM based on tool result
+			contentForLLM := toolResult.ForLLM
+			if contentForLLM == "" && toolResult.Err != nil {
+				contentForLLM = toolResult.Err.Error()
 			}
 
 			toolResultMsg := providers.Message{
 				Role:       "tool",
-				Content:    result,
+				Content:    contentForLLM,
 				ToolCallID: tc.ID,
 			}
 			messages = append(messages, toolResultMsg)
@@ -440,13 +565,19 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, messages []providers.M
 
 // updateToolContexts updates the context for tools that need channel/chatID info.
 func (al *AgentLoop) updateToolContexts(channel, chatID string) {
+	// Use ContextualTool interface instead of type assertions
 	if tool, ok := al.tools.Get("message"); ok {
-		if mt, ok := tool.(*tools.MessageTool); ok {
+		if mt, ok := tool.(tools.ContextualTool); ok {
 			mt.SetContext(channel, chatID)
 		}
 	}
 	if tool, ok := al.tools.Get("spawn"); ok {
-		if st, ok := tool.(*tools.SpawnTool); ok {
+		if st, ok := tool.(tools.ContextualTool); ok {
+			st.SetContext(channel, chatID)
+		}
+	}
+	if tool, ok := al.tools.Get("subagent"); ok {
+		if st, ok := tool.(tools.ContextualTool); ok {
 			st.SetContext(channel, chatID)
 		}
 	}

pkg/agent/loop_test.go

@@ -0,0 +1,529 @@
+package agent
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/sipeed/picoclaw/pkg/bus"
+	"github.com/sipeed/picoclaw/pkg/config"
+	"github.com/sipeed/picoclaw/pkg/providers"
+	"github.com/sipeed/picoclaw/pkg/tools"
+)
+
+// mockProvider is a simple mock LLM provider for testing
+type mockProvider struct{}
+
+func (m *mockProvider) Chat(ctx context.Context, messages []providers.Message, tools []providers.ToolDefinition, model string, opts map[string]interface{}) (*providers.LLMResponse, error) {
+	return &providers.LLMResponse{
+		Content:   "Mock response",
+		ToolCalls: []providers.ToolCall{},
+	}, nil
+}
+
+func (m *mockProvider) GetDefaultModel() string {
+	return "mock-model"
+}
+
+func TestRecordLastChannel(t *testing.T) {
+	// Create temp workspace
+	tmpDir, err := os.MkdirTemp("", "agent-test-*")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	// Create test config
+	cfg := &config.Config{
+		Agents: config.AgentsConfig{
+			Defaults: config.AgentDefaults{
+				Workspace:         tmpDir,
+				Model:             "test-model",
+				MaxTokens:         4096,
+				MaxToolIterations: 10,
+			},
+		},
+	}
+
+	// Create agent loop
+	msgBus := bus.NewMessageBus()
+	provider := &mockProvider{}
+	al := NewAgentLoop(cfg, msgBus, provider)
+
+	// Test RecordLastChannel
+	testChannel := "test-channel"
+	err = al.RecordLastChannel(testChannel)
+	if err != nil {
+		t.Fatalf("RecordLastChannel failed: %v", err)
+	}
+
+	// Verify channel was saved
+	lastChannel := al.state.GetLastChannel()
+	if lastChannel != testChannel {
+		t.Errorf("Expected channel '%s', got '%s'", testChannel, lastChannel)
+	}
+
+	// Verify persistence by creating a new agent loop
+	al2 := NewAgentLoop(cfg, msgBus, provider)
+	if al2.state.GetLastChannel() != testChannel {
+		t.Errorf("Expected persistent channel '%s', got '%s'", testChannel, al2.state.GetLastChannel())
+	}
+}
+
+func TestRecordLastChatID(t *testing.T) {
+	// Create temp workspace
+	tmpDir, err := os.MkdirTemp("", "agent-test-*")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	// Create test config
+	cfg := &config.Config{
+		Agents: config.AgentsConfig{
+			Defaults: config.AgentDefaults{
+				Workspace:         tmpDir,
+				Model:             "test-model",
+				MaxTokens:         4096,
+				MaxToolIterations: 10,
+			},
+		},
+	}
+
+	// Create agent loop
+	msgBus := bus.NewMessageBus()
+	provider := &mockProvider{}
+	al := NewAgentLoop(cfg, msgBus, provider)
+
+	// Test RecordLastChatID
+	testChatID := "test-chat-id-123"
+	err = al.RecordLastChatID(testChatID)
+	if err != nil {
+		t.Fatalf("RecordLastChatID failed: %v", err)
+	}
+
+	// Verify chat ID was saved
+	lastChatID := al.state.GetLastChatID()
+	if lastChatID != testChatID {
+		t.Errorf("Expected chat ID '%s', got '%s'", testChatID, lastChatID)
+	}
+
+	// Verify persistence by creating a new agent loop
+	al2 := NewAgentLoop(cfg, msgBus, provider)
+	if al2.state.GetLastChatID() != testChatID {
+		t.Errorf("Expected persistent chat ID '%s', got '%s'", testChatID, al2.state.GetLastChatID())
+	}
+}
+
+func TestNewAgentLoop_StateInitialized(t *testing.T) {
+	// Create temp workspace
+	tmpDir, err := os.MkdirTemp("", "agent-test-*")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	// Create test config
+	cfg := &config.Config{
+		Agents: config.AgentsConfig{
+			Defaults: config.AgentDefaults{
+				Workspace:         tmpDir,
+				Model:             "test-model",
+				MaxTokens:         4096,
+				MaxToolIterations: 10,
+			},
+		},
+	}
+
+	// Create agent loop
+	msgBus := bus.NewMessageBus()
+	provider := &mockProvider{}
+	al := NewAgentLoop(cfg, msgBus, provider)
+
+	// Verify state manager is initialized
+	if al.state == nil {
+		t.Error("Expected state manager to be initialized")
+	}
+
+	// Verify state directory was created
+	stateDir := filepath.Join(tmpDir, "state")
+	if _, err := os.Stat(stateDir); os.IsNotExist(err) {
+		t.Error("Expected state directory to exist")
+	}
+}
+
+// TestToolRegistry_ToolRegistration verifies tools can be registered and retrieved
+func TestToolRegistry_ToolRegistration(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "agent-test-*")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	cfg := &config.Config{
+		Agents: config.AgentsConfig{
+			Defaults: config.AgentDefaults{
+				Workspace:         tmpDir,
+				Model:             "test-model",
+				MaxTokens:         4096,
+				MaxToolIterations: 10,
+			},
+		},
+	}
+
+	msgBus := bus.NewMessageBus()
+	provider := &mockProvider{}
+	al := NewAgentLoop(cfg, msgBus, provider)
+
+	// Register a custom tool
+	customTool := &mockCustomTool{}
+	al.RegisterTool(customTool)
+
+	// Verify tool is registered by checking it doesn't panic on GetStartupInfo
+	// (actual tool retrieval is tested in tools package tests)
+	info := al.GetStartupInfo()
+	toolsInfo := info["tools"].(map[string]interface{})
+	toolsList := toolsInfo["names"].([]string)
+
+	// Check that our custom tool name is in the list
+	found := false
+	for _, name := range toolsList {
+		if name == "mock_custom" {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Error("Expected custom tool to be registered")
+	}
+}
+
+// TestToolContext_Updates verifies tool context is updated with channel/chatID
+func TestToolContext_Updates(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "agent-test-*")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	cfg := &config.Config{
+		Agents: config.AgentsConfig{
+			Defaults: config.AgentDefaults{
+				Workspace:         tmpDir,
+				Model:             "test-model",
+				MaxTokens:         4096,
+				MaxToolIterations: 10,
+			},
+		},
+	}
+
+	msgBus := bus.NewMessageBus()
+	provider := &simpleMockProvider{response: "OK"}
+	_ = NewAgentLoop(cfg, msgBus, provider)
+
+	// Verify that ContextualTool interface is defined and can be implemented
+	// This test validates the interface contract exists
+	ctxTool := &mockContextualTool{}
+
+	// Verify the tool implements the interface correctly
+	var _ tools.ContextualTool = ctxTool
+}
+
+// TestToolRegistry_GetDefinitions verifies tool definitions can be retrieved
+func TestToolRegistry_GetDefinitions(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "agent-test-*")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	cfg := &config.Config{
+		Agents: config.AgentsConfig{
+			Defaults: config.AgentDefaults{
+				Workspace:         tmpDir,
+				Model:             "test-model",
+				MaxTokens:         4096,
+				MaxToolIterations: 10,
+			},
+		},
+	}
+
+	msgBus := bus.NewMessageBus()
+	provider := &mockProvider{}
+	al := NewAgentLoop(cfg, msgBus, provider)
+
+	// Register a test tool and verify it shows up in startup info
+	testTool := &mockCustomTool{}
+	al.RegisterTool(testTool)
+
+	info := al.GetStartupInfo()
+	toolsInfo := info["tools"].(map[string]interface{})
+	toolsList := toolsInfo["names"].([]string)
+
+	// Check that our custom tool name is in the list
+	found := false
+	for _, name := range toolsList {
+		if name == "mock_custom" {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Error("Expected custom tool to be registered")
+	}
+}
+
+// TestAgentLoop_GetStartupInfo verifies startup info contains tools
+func TestAgentLoop_GetStartupInfo(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "agent-test-*")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	cfg := &config.Config{
+		Agents: config.AgentsConfig{
+			Defaults: config.AgentDefaults{
+				Workspace:         tmpDir,
+				Model:             "test-model",
+				MaxTokens:         4096,
+				MaxToolIterations: 10,
+			},
+		},
+	}
+
+	msgBus := bus.NewMessageBus()
+	provider := &mockProvider{}
+	al := NewAgentLoop(cfg, msgBus, provider)
+
+	info := al.GetStartupInfo()
+
+	// Verify tools info exists
+	toolsInfo, ok := info["tools"]
+	if !ok {
+		t.Fatal("Expected 'tools' key in startup info")
+	}
+
+	toolsMap, ok := toolsInfo.(map[string]interface{})
+	if !ok {
+		t.Fatal("Expected 'tools' to be a map")
+	}
+
+	count, ok := toolsMap["count"]
+	if !ok {
+		t.Fatal("Expected 'count' in tools info")
+	}
+
+	// Should have default tools registered
+	if count.(int) == 0 {
+		t.Error("Expected at least some tools to be registered")
+	}
+}
+
+// TestAgentLoop_Stop verifies Stop() sets running to false
+func TestAgentLoop_Stop(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "agent-test-*")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	cfg := &config.Config{
+		Agents: config.AgentsConfig{
+			Defaults: config.AgentDefaults{
+				Workspace:         tmpDir,
+				Model:             "test-model",
+				MaxTokens:         4096,
+				MaxToolIterations: 10,
+			},
+		},
+	}
+
+	msgBus := bus.NewMessageBus()
+	provider := &mockProvider{}
+	al := NewAgentLoop(cfg, msgBus, provider)
+
+	// Note: running is only set to true when Run() is called
+	// We can't test that without starting the event loop
+	// Instead, verify the Stop method can be called safely
+	al.Stop()
+
+	// Verify running is false (initial state or after Stop)
+	if al.running.Load() {
+		t.Error("Expected agent to be stopped (or never started)")
+	}
+}
+
+// Mock implementations for testing
+
+type simpleMockProvider struct {
+	response string
+}
+
+func (m *simpleMockProvider) Chat(ctx context.Context, messages []providers.Message, tools []providers.ToolDefinition, model string, opts map[string]interface{}) (*providers.LLMResponse, error) {
+	return &providers.LLMResponse{
+		Content:   m.response,
+		ToolCalls: []providers.ToolCall{},
+	}, nil
+}
+
+func (m *simpleMockProvider) GetDefaultModel() string {
+	return "mock-model"
+}
+
+// mockCustomTool is a simple mock tool for registration testing
+type mockCustomTool struct{}
+
+func (m *mockCustomTool) Name() string {
+	return "mock_custom"
+}
+
+func (m *mockCustomTool) Description() string {
+	return "Mock custom tool for testing"
+}
+
+func (m *mockCustomTool) Parameters() map[string]interface{} {
+	return map[string]interface{}{
+		"type":       "object",
+		"properties": map[string]interface{}{},
+	}
+}
+
+func (m *mockCustomTool) Execute(ctx context.Context, args map[string]interface{}) *tools.ToolResult {
+	return tools.SilentResult("Custom tool executed")
+}
+
+// mockContextualTool tracks context updates
+type mockContextualTool struct {
+	lastChannel string
+	lastChatID  string
+}
+
+func (m *mockContextualTool) Name() string {
+	return "mock_contextual"
+}
+
+func (m *mockContextualTool) Description() string {
+	return "Mock contextual tool"
+}
+
+func (m *mockContextualTool) Parameters() map[string]interface{} {
+	return map[string]interface{}{
+		"type":       "object",
+		"properties": map[string]interface{}{},
+	}
+}
+
+func (m *mockContextualTool) Execute(ctx context.Context, args map[string]interface{}) *tools.ToolResult {
+	return tools.SilentResult("Contextual tool executed")
+}
+
+func (m *mockContextualTool) SetContext(channel, chatID string) {
+	m.lastChannel = channel
+	m.lastChatID = chatID
+}
+
+// testHelper executes a message and returns the response
+type testHelper struct {
+	al *AgentLoop
+}
+
+func (h testHelper) executeAndGetResponse(tb testing.TB, ctx context.Context, msg bus.InboundMessage) string {
+	// Use a short timeout to avoid hanging
+	timeoutCtx, cancel := context.WithTimeout(ctx, responseTimeout)
+	defer cancel()
+
+	response, err := h.al.processMessage(timeoutCtx, msg)
+	if err != nil {
+		tb.Fatalf("processMessage failed: %v", err)
+	}
+	return response
+}
+
+const responseTimeout = 3 * time.Second
+
+// TestToolResult_SilentToolDoesNotSendUserMessage verifies silent tools don't trigger outbound
+func TestToolResult_SilentToolDoesNotSendUserMessage(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "agent-test-*")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	cfg := &config.Config{
+		Agents: config.AgentsConfig{
+			Defaults: config.AgentDefaults{
+				Workspace:         tmpDir,
+				Model:             "test-model",
+				MaxTokens:         4096,
+				MaxToolIterations: 10,
+			},
+		},
+	}
+
+	msgBus := bus.NewMessageBus()
+	provider := &simpleMockProvider{response: "File operation complete"}
+	al := NewAgentLoop(cfg, msgBus, provider)
+	helper := testHelper{al: al}
+
+	// ReadFileTool returns SilentResult, which should not send user message
+	ctx := context.Background()
+	msg := bus.InboundMessage{
+		Channel:    "test",
+		SenderID:   "user1",
+		ChatID:     "chat1",
+		Content:    "read test.txt",
+		SessionKey: "test-session",
+	}
+
+	response := helper.executeAndGetResponse(t, ctx, msg)
+
+	// Silent tool should return the LLM's response directly
+	if response != "File operation complete" {
+		t.Errorf("Expected 'File operation complete', got: %s", response)
+	}
+}
+
+// TestToolResult_UserFacingToolDoesSendMessage verifies user-facing tools trigger outbound
+func TestToolResult_UserFacingToolDoesSendMessage(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "agent-test-*")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	cfg := &config.Config{
+		Agents: config.AgentsConfig{
+			Defaults: config.AgentDefaults{
+				Workspace:         tmpDir,
+				Model:             "test-model",
+				MaxTokens:         4096,
+				MaxToolIterations: 10,
+			},
+		},
+	}
+
+	msgBus := bus.NewMessageBus()
+	provider := &simpleMockProvider{response: "Command output: hello world"}
+	al := NewAgentLoop(cfg, msgBus, provider)
+	helper := testHelper{al: al}
+
+	// ExecTool returns UserResult, which should send user message
+	ctx := context.Background()
+	msg := bus.InboundMessage{
+		Channel:    "test",
+		SenderID:   "user1",
+		ChatID:     "chat1",
+		Content:    "run hello",
+		SessionKey: "test-session",
+	}
+
+	response := helper.executeAndGetResponse(t, ctx, msg)
+
+	// User-facing tool should include the output in final response
+	if response != "Command output: hello world" {
+		t.Errorf("Expected 'Command output: hello world', got: %s", response)
+	}
+}

pkg/agent/memory.go

@@ -40,8 +40,8 @@ func NewMemoryStore(workspace string) *MemoryStore {
 
 // getTodayFile returns the path to today's daily note file (memory/YYYYMM/YYYYMMDD.md).
 func (ms *MemoryStore) getTodayFile() string {
-	today := time.Now().Format("20060102")      // YYYYMMDD
-	monthDir := today[:6]                       // YYYYMM
+	today := time.Now().Format("20060102") // YYYYMMDD
+	monthDir := today[:6]                  // YYYYMM
 	filePath := filepath.Join(ms.memoryDir, monthDir, today+".md")
 	return filePath
 }
@@ -104,8 +104,8 @@ func (ms *MemoryStore) GetRecentDailyNotes(days int) string {
 
 	for i := 0; i < days; i++ {
 		date := time.Now().AddDate(0, 0, -i)
-		dateStr := date.Format("20060102")      // YYYYMMDD
-		monthDir := dateStr[:6]                 // YYYYMM
+		dateStr := date.Format("20060102") // YYYYMMDD
+		monthDir := dateStr[:6]            // YYYYMM
 		filePath := filepath.Join(ms.memoryDir, monthDir, dateStr+".md")
 
 		if data, err := os.ReadFile(filePath); err == nil {