diff --git a/.ralph/prd.json b/.ralph/prd.json index e6451a1..94f3f95 100644 --- a/.ralph/prd.json +++ b/.ralph/prd.json @@ -151,7 +151,7 @@ "go test ./pkg/agent -run TestRecordLastChannel passes" ], "priority": 10, - "passes": false, + "passes": true, "notes": "" }, { diff --git a/.ralph/progress.txt b/.ralph/progress.txt index 44297b4..d23571b 100644 --- a/.ralph/progress.txt +++ b/.ralph/progress.txt @@ -6,7 +6,7 @@ Tool 返回值结构化重构 - 将 Tool 接口返回值从 (string, error) 改 ## Progress -### Completed (8/21) +### Completed (9/21) - US-001: Add ToolResult struct and helper functions - US-002: Modify Tool interface to return *ToolResult @@ -16,6 +16,7 @@ Tool 返回值结构化重构 - 将 Tool 接口返回值从 (string, error) 改 - US-007: Heartbeat async task execution support - US-008: Inject callback into async tools in AgentLoop - US-009: State save atomicity - SetLastChannel +- US-010: Update RecordLastChannel to use atomic save ### In Progress @@ -32,7 +33,7 @@ Tool 返回值结构化重构 - 将 Tool 接口返回值从 (string, error) 改 | US-007 | Heartbeat async task execution support | Completed | | | US-008 | Inject callback into async tools in AgentLoop | Completed | | | US-009 | State save atomicity - SetLastChannel | Completed | | -| US-010 | Update RecordLastChannel to use atomic save | Pending | | +| US-010 | Update RecordLastChannel to use atomic save | Completed | | | US-011 | Refactor MessageTool to use ToolResult | Completed | | | US-012 | Refactor ShellTool to use ToolResult | Completed | | | US-013 | Refactor FilesystemTool to use ToolResult | Completed | | @@ -175,4 +176,24 @@ Tool 返回值结构化重构 - 将 Tool 接口返回值从 (string, error) 改 - **Gotchas encountered:** 临时文件必须与目标文件在同一文件系统中,否则 `os.Rename` 会失败。 - **Useful context:** 这个模式将在 US-010 中用于 `RecordLastChannel`,确保状态更新的原子性。 +--- + +## [2026-02-12] - US-010 +- What was implemented: + - 在 AgentLoop 中添加 `state *state.Manager` 字段 + - 在 `NewAgentLoop` 中初始化 `stateManager := state.NewManager(workspace)` + - 实现 `RecordLastChannel(channel string)` 方法,调用 `al.state.SetLastChannel(al.workspace, channel)` + - 实现 `RecordLastChatID(chatID string)` 方法,调用 `al.state.SetLastChatID(al.workspace, chatID)` + - 添加完整的测试:`TestRecordLastChannel`、`TestRecordLastChatID`、`TestNewAgentLoop_StateInitialized` + - 验证状态持久化:创建新的 AgentLoop 实例后状态仍然保留 + +- Files changed: + - `pkg/agent/loop.go` + - `pkg/agent/loop_test.go` (新增) + +- **Learnings for future iterations:** + - **Patterns discovered:** 状态管理应该集中在一个专门的包中(如 `pkg/state`),而不是分散在各个模块中。 + - **Gotchas encountered:** 测试中的 mock Provider 需要实现完整的 `LLMProvider` 接口,包括 `GetDefaultModel()` 方法。 + - **Useful context:** `RecordLastChannel` 方法现在可以用于跟踪用户最后一次活跃的频道,这对于跨会话的上下文恢复很有用。 + --- \ No newline at end of file diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 5030a1b..b6ba43a 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -22,6 +22,7 @@ import ( "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" ) @@ -34,6 +35,7 @@ type AgentLoop struct { contextWindow int // Maximum context window size in tokens maxIterations int sessions *session.SessionManager + state *state.Manager contextBuilder *ContextBuilder tools *tools.ToolRegistry running atomic.Bool @@ -88,6 +90,9 @@ func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers 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) @@ -100,6 +105,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{}, @@ -145,6 +151,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(al.workspace, 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(al.workspace, chatID) +} + func (al *AgentLoop) ProcessDirect(ctx context.Context, content, sessionKey string) (string, error) { return al.ProcessDirectWithChannel(ctx, content, sessionKey, "cli", "direct") } diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go new file mode 100644 index 0000000..da82615 --- /dev/null +++ b/pkg/agent/loop_test.go @@ -0,0 +1,153 @@ +package agent + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/sipeed/picoclaw/pkg/bus" + "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/providers" +) + +// 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 the 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 the 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") + } +}