From add20a445416788f71118aa37ee0c8d3ad2b8fad Mon Sep 17 00:00:00 2001 From: KoheiYamashita Date: Tue, 17 Feb 2026 22:22:43 +0900 Subject: [PATCH] feat: add rate limiting, memory/skill tools, and state management - Add rate limiter for tool calls and requests (pkg/agent/ratelimit.go) - Add persistent memory tool (write/read long-term and daily notes) - Add skill tool for listing and reading skills - Add atomic state management (pkg/agent/state/) - Refactor skills loader to use dataDir instead of workspace - Remove skill installer (search/install from registry) - Add rate_limits config section - Update heartbeat service to use state manager for last channel - Refactor provider creation and model name handling Co-Authored-By: Claude Opus 4.6 --- cmd/picoclaw/main.go | 108 ++++++--------------- config/config.example.json | 12 ++- pkg/agent/context.go | 35 ++++--- pkg/agent/loop.go | 47 ++++++++- pkg/agent/loop_test.go | 17 ++++ pkg/agent/memory.go | 10 +- pkg/agent/ratelimit.go | 84 ++++++++++++++++ pkg/agent/state/state.json | 5 + pkg/config/config.go | 42 +++++--- pkg/config/config_test.go | 32 +++++- pkg/heartbeat/service.go | 12 ++- pkg/heartbeat/service_test.go | 16 +-- pkg/providers/http_provider.go | 29 +++--- pkg/skills/installer.go | 171 --------------------------------- pkg/skills/loader.go | 32 +++--- pkg/tools/memory.go | 95 ++++++++++++++++++ pkg/tools/memory_test.go | 162 +++++++++++++++++++++++++++++++ pkg/tools/skills.go | 77 +++++++++++++++ pkg/tools/skills_test.go | 163 +++++++++++++++++++++++++++++++ 19 files changed, 819 insertions(+), 330 deletions(-) create mode 100644 pkg/agent/ratelimit.go create mode 100644 pkg/agent/state/state.json delete mode 100644 pkg/skills/installer.go create mode 100644 pkg/tools/memory.go create mode 100644 pkg/tools/memory_test.go create mode 100644 pkg/tools/skills.go create mode 100644 pkg/tools/skills_test.go diff --git a/cmd/picoclaw/main.go b/cmd/picoclaw/main.go index 10b53948b..a482c600d 100644 --- a/cmd/picoclaw/main.go +++ b/cmd/picoclaw/main.go @@ -156,31 +156,26 @@ func main() { os.Exit(1) } - workspace := cfg.WorkspacePath() - installer := skills.NewSkillInstaller(workspace) + dataDir := cfg.DataPath() // 获取全局配置目录和内置 skills 目录 globalDir := filepath.Dir(getConfigPath()) globalSkillsDir := filepath.Join(globalDir, "skills") builtinSkillsDir := filepath.Join(globalDir, "picoclaw", "skills") - skillsLoader := skills.NewSkillsLoader(workspace, globalSkillsDir, builtinSkillsDir) + skillsLoader := skills.NewSkillsLoader(dataDir, globalSkillsDir, builtinSkillsDir) switch subcommand { case "list": skillsListCmd(skillsLoader) - case "install": - skillsInstallCmd(installer) case "remove", "uninstall": if len(os.Args) < 4 { fmt.Println("Usage: picoclaw skills remove ") return } - skillsRemoveCmd(installer, os.Args[3]) + skillsRemoveCmd(dataDir, os.Args[3]) case "install-builtin": - skillsInstallBuiltinCmd(workspace) + skillsInstallBuiltinCmd(dataDir) case "list-builtin": skillsListBuiltinCmd() - case "search": - skillsSearchCmd(installer) case "show": if len(os.Args) < 4 { fmt.Println("Usage: picoclaw skills show ") @@ -237,7 +232,9 @@ func onboard() { } workspace := cfg.WorkspacePath() - createWorkspaceTemplates(workspace) + dataDir := cfg.DataPath() + os.MkdirAll(workspace, 0755) + createWorkspaceTemplates(dataDir) fmt.Printf("%s picoclaw is ready!\n", logo) fmt.Println("\nNext steps:") @@ -562,10 +559,11 @@ func gatewayCmd() { }) // Setup cron tool and service - cronService := setupCronTool(agentLoop, msgBus, cfg.WorkspacePath(), cfg.Agents.Defaults.RestrictToWorkspace) + cronService := setupCronTool(agentLoop, msgBus, cfg.WorkspacePath(), cfg.DataPath(), cfg.Agents.Defaults.RestrictToWorkspace) heartbeatService := heartbeat.NewHeartbeatService( cfg.WorkspacePath(), + cfg.DataPath(), cfg.Heartbeat.Interval, cfg.Heartbeat.Enabled, ) @@ -647,7 +645,7 @@ func gatewayCmd() { } fmt.Println("✓ Heartbeat service started") - stateManager := state.NewManager(cfg.WorkspacePath()) + stateManager := state.NewManager(cfg.DataPath()) deviceService := devices.NewService(devices.Config{ Enabled: cfg.Devices.Enabled, MonitorUSB: cfg.Devices.MonitorUSB, @@ -987,13 +985,13 @@ func getConfigPath() string { return filepath.Join(home, ".picoclaw", "config.json") } -func setupCronTool(agentLoop *agent.AgentLoop, msgBus *bus.MessageBus, workspace string, restrict bool) *cron.CronService { - cronStorePath := filepath.Join(workspace, "cron", "jobs.json") +func setupCronTool(agentLoop *agent.AgentLoop, msgBus *bus.MessageBus, workspace string, dataDir string, restrict bool) *cron.CronService { + cronStorePath := filepath.Join(dataDir, "cron", "jobs.json") // Create cron service cronService := cron.NewCronService(cronStorePath, nil) - // Create and register CronTool + // Create and register CronTool (workspace is for ExecTool sandboxing) cronTool := tools.NewCronTool(cronService, agentLoop, msgBus, workspace, restrict) agentLoop.RegisterTool(cronTool) @@ -1025,7 +1023,7 @@ func cronCmd() { return } - cronStorePath := filepath.Join(cfg.WorkspacePath(), "cron", "jobs.json") + cronStorePath := filepath.Join(cfg.DataPath(), "cron", "jobs.json") switch subcommand { case "list": @@ -1227,16 +1225,16 @@ func cronEnableCmd(storePath string, disable bool) { func skillsHelp() { fmt.Println("\nSkills commands:") fmt.Println(" list List installed skills") - fmt.Println(" install Install skill from GitHub") - fmt.Println(" install-builtin Install all builtin skills to workspace") - fmt.Println(" list-builtin List available builtin skills") + fmt.Println(" install-builtin Install all builtin skills to workspace") + fmt.Println(" list-builtin List available builtin skills") fmt.Println(" remove Remove installed skill") - fmt.Println(" search Search available skills") fmt.Println(" show Show skill details") fmt.Println() + fmt.Println("To install custom skills, place SKILL.md files manually in:") + fmt.Println(" ~/.picoclaw/data/skills//SKILL.md") + fmt.Println() fmt.Println("Examples:") fmt.Println(" picoclaw skills list") - fmt.Println(" picoclaw skills install sipeed/picoclaw-skills/weather") fmt.Println(" picoclaw skills install-builtin") fmt.Println(" picoclaw skills list-builtin") fmt.Println(" picoclaw skills remove weather") @@ -1260,31 +1258,16 @@ func skillsListCmd(loader *skills.SkillsLoader) { } } -func skillsInstallCmd(installer *skills.SkillInstaller) { - if len(os.Args) < 4 { - fmt.Println("Usage: picoclaw skills install ") - fmt.Println("Example: picoclaw skills install sipeed/picoclaw-skills/weather") - return - } +func skillsRemoveCmd(dataDir string, skillName string) { + skillDir := filepath.Join(dataDir, "skills", skillName) - repo := os.Args[3] - fmt.Printf("Installing skill from %s...\n", repo) - - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - if err := installer.InstallFromGitHub(ctx, repo); err != nil { - fmt.Printf("✗ Failed to install skill: %v\n", err) + if _, err := os.Stat(skillDir); os.IsNotExist(err) { + fmt.Printf("✗ Skill '%s' not found\n", skillName) os.Exit(1) } - fmt.Printf("✓ Skill '%s' installed successfully!\n", filepath.Base(repo)) -} - -func skillsRemoveCmd(installer *skills.SkillInstaller, skillName string) { fmt.Printf("Removing skill '%s'...\n", skillName) - - if err := installer.Uninstall(skillName); err != nil { + if err := os.RemoveAll(skillDir); err != nil { fmt.Printf("✗ Failed to remove skill: %v\n", err) os.Exit(1) } @@ -1292,9 +1275,9 @@ func skillsRemoveCmd(installer *skills.SkillInstaller, skillName string) { fmt.Printf("✓ Skill '%s' removed successfully!\n", skillName) } -func skillsInstallBuiltinCmd(workspace string) { +func skillsInstallBuiltinCmd(dataDir string) { builtinSkillsDir := "./picoclaw/skills" - workspaceSkillsDir := filepath.Join(workspace, "skills") + skillsDir := filepath.Join(dataDir, "skills") fmt.Printf("Copying builtin skills to workspace...\n") @@ -1307,19 +1290,19 @@ func skillsInstallBuiltinCmd(workspace string) { for _, skillName := range skillsToInstall { builtinPath := filepath.Join(builtinSkillsDir, skillName) - workspacePath := filepath.Join(workspaceSkillsDir, skillName) + destPath := filepath.Join(skillsDir, skillName) if _, err := os.Stat(builtinPath); err != nil { fmt.Printf("⊘ Builtin skill '%s' not found: %v\n", skillName, err) continue } - if err := os.MkdirAll(workspacePath, 0755); err != nil { + if err := os.MkdirAll(destPath, 0755); err != nil { fmt.Printf("✗ Failed to create directory for %s: %v\n", skillName, err) continue } - if err := copyDirectory(builtinPath, workspacePath); err != nil { + if err := copyDirectory(builtinPath, destPath); err != nil { fmt.Printf("✗ Failed to copy %s: %v\n", skillName, err) } } @@ -1380,39 +1363,6 @@ func skillsListBuiltinCmd() { } } -func skillsSearchCmd(installer *skills.SkillInstaller) { - fmt.Println("Searching for available skills...") - - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - availableSkills, err := installer.ListAvailableSkills(ctx) - if err != nil { - fmt.Printf("✗ Failed to fetch skills list: %v\n", err) - return - } - - if len(availableSkills) == 0 { - fmt.Println("No skills available.") - return - } - - fmt.Printf("\nAvailable Skills (%d):\n", len(availableSkills)) - fmt.Println("--------------------") - for _, skill := range availableSkills { - fmt.Printf(" 📦 %s\n", skill.Name) - fmt.Printf(" %s\n", skill.Description) - fmt.Printf(" Repo: %s\n", skill.Repository) - if skill.Author != "" { - fmt.Printf(" Author: %s\n", skill.Author) - } - if len(skill.Tags) > 0 { - fmt.Printf(" Tags: %v\n", skill.Tags) - } - fmt.Println() - } -} - func skillsShowCmd(loader *skills.SkillsLoader, skillName string) { content, ok := loader.LoadSkill(skillName) if !ok { diff --git a/config/config.example.json b/config/config.example.json index 660c580cc..4153d44bc 100644 --- a/config/config.example.json +++ b/config/config.example.json @@ -23,8 +23,8 @@ }, "maixcam": { "enabled": false, - "host": "0.0.0.0", - "port": 18790, + "host": "127.0.0.1", + "port": 18792, "allow_from": [] }, "whatsapp": { @@ -56,7 +56,7 @@ "enabled": false, "channel_secret": "YOUR_LINE_CHANNEL_SECRET", "channel_access_token": "YOUR_LINE_CHANNEL_ACCESS_TOKEN", - "webhook_host": "0.0.0.0", + "webhook_host": "127.0.0.1", "webhook_port": 18791, "webhook_path": "/webhook/line", "allow_from": [] @@ -139,7 +139,11 @@ "monitor_usb": true }, "gateway": { - "host": "0.0.0.0", + "host": "127.0.0.1", "port": 18790 + }, + "rate_limits": { + "max_tool_calls_per_minute": 60, + "max_requests_per_minute": 30 } } diff --git a/pkg/agent/context.go b/pkg/agent/context.go index cf5ce2913..4e72a08af 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -16,6 +16,7 @@ import ( type ContextBuilder struct { workspace string + dataDir string skillsLoader *skills.SkillsLoader memory *MemoryStore tools *tools.ToolRegistry // Direct reference to tool registry @@ -29,7 +30,7 @@ func getGlobalConfigDir() string { return filepath.Join(home, ".picoclaw") } -func NewContextBuilder(workspace string) *ContextBuilder { +func NewContextBuilder(workspace string, dataDir string) *ContextBuilder { // builtin skills: skills directory in current project // Use the skills/ directory under the current working directory wd, _ := os.Getwd() @@ -38,11 +39,22 @@ func NewContextBuilder(workspace string) *ContextBuilder { return &ContextBuilder{ workspace: workspace, - skillsLoader: skills.NewSkillsLoader(workspace, globalSkillsDir, builtinSkillsDir), - memory: NewMemoryStore(workspace), + dataDir: dataDir, + skillsLoader: skills.NewSkillsLoader(dataDir, globalSkillsDir, builtinSkillsDir), + memory: NewMemoryStore(dataDir), } } +// GetMemory returns the memory store for tool registration. +func (cb *ContextBuilder) GetMemory() *MemoryStore { + return cb.memory +} + +// GetSkillsLoader returns the skills loader for tool registration. +func (cb *ContextBuilder) GetSkillsLoader() *skills.SkillsLoader { + return cb.skillsLoader +} + // SetToolsRegistry sets the tools registry for dynamic tool summary generation. func (cb *ContextBuilder) SetToolsRegistry(registry *tools.ToolRegistry) { cb.tools = registry @@ -68,9 +80,6 @@ You are picoclaw, a helpful AI assistant. ## Workspace Your workspace is at: %s -- Memory: %s/memory/MEMORY.md -- Daily Notes: %s/memory/YYYYMM/YYYYMMDD.md -- Skills: %s/skills/{skill-name}/SKILL.md %s @@ -80,8 +89,12 @@ Your workspace is at: %s 2. **Be helpful and accurate** - When using tools, briefly explain what you're doing. -3. **Memory** - When remembering something, write to %s/memory/MEMORY.md`, - now, runtime, workspacePath, workspacePath, workspacePath, workspacePath, toolsSection, workspacePath) +3. **Memory** - Use the memory tool to store and retrieve information. + - write_long_term: Save important, date-independent facts (user preferences, project info, permanent notes) + - append_daily: Record today's events and memos (diary-like daily entries) + - read_long_term: Read long-term memory + - read_daily: Read today's daily notes`, + now, runtime, workspacePath, toolsSection) } func (cb *ContextBuilder) buildToolsSection() string { @@ -118,12 +131,12 @@ func (cb *ContextBuilder) BuildSystemPrompt() string { parts = append(parts, bootstrapContent) } - // Skills - show summary, AI can read full content with read_file tool + // Skills - show summary, AI can read full content with skill_read tool skillsSummary := cb.skillsLoader.BuildSkillsSummary() if skillsSummary != "" { parts = append(parts, fmt.Sprintf(`# Skills -The following skills extend your capabilities. To use a skill, read its SKILL.md file using the read_file tool. +The following skills extend your capabilities. To use a skill, call the skill_read tool with the skill name. %s`, skillsSummary)) } @@ -148,7 +161,7 @@ func (cb *ContextBuilder) LoadBootstrapFiles() string { var result string for _, filename := range bootstrapFiles { - filePath := filepath.Join(cb.workspace, filename) + filePath := filepath.Join(cb.dataDir, filename) if data, err := os.ReadFile(filePath); err == nil { result += fmt.Sprintf("## %s\n\n%s\n\n", filename, string(data)) } diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index cac2de7a8..208eb4a02 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -44,6 +44,7 @@ type AgentLoop struct { running atomic.Bool summarizing sync.Map // Tracks which sessions are currently being summarized channelManager *channels.Manager + rateLimiter *rateLimiter } // processOptions configures how a message is processed @@ -112,7 +113,9 @@ func createToolRegistry(workspace string, restrict bool, cfg *config.Config, msg func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers.LLMProvider) *AgentLoop { workspace := cfg.WorkspacePath() + dataDir := cfg.DataPath() os.MkdirAll(workspace, 0755) + os.MkdirAll(dataDir, 0755) restrict := cfg.Agents.Defaults.RestrictToWorkspace @@ -133,15 +136,25 @@ func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers subagentTool := tools.NewSubagentTool(subagentManager) toolsRegistry.Register(subagentTool) - sessionsManager := session.NewSessionManager(filepath.Join(workspace, "sessions")) + // Use dataDir for sessions and state (outside workspace for security) + sessionsManager := session.NewSessionManager(filepath.Join(dataDir, "sessions")) // Create state manager for atomic state persistence - stateManager := state.NewManager(workspace) + stateManager := state.NewManager(dataDir) // Create context builder and set tools registry - contextBuilder := NewContextBuilder(workspace) + contextBuilder := NewContextBuilder(workspace, dataDir) contextBuilder.SetToolsRegistry(toolsRegistry) + // Register memory and skill tools (controlled access to dataDir) + memoryTool := tools.NewMemoryTool(contextBuilder.GetMemory()) + toolsRegistry.Register(memoryTool) + subagentTools.Register(memoryTool) + + skillTool := tools.NewSkillTool(contextBuilder.GetSkillsLoader()) + toolsRegistry.Register(skillTool) + subagentTools.Register(skillTool) + return &AgentLoop{ bus: msgBus, provider: provider, @@ -154,6 +167,7 @@ func NewAgentLoop(cfg *config.Config, msgBus *bus.MessageBus, provider providers contextBuilder: contextBuilder, tools: toolsRegistry, summarizing: sync.Map{}, + rateLimiter: newRateLimiter(cfg.RateLimits.MaxToolCallsPerMinute, cfg.RateLimits.MaxRequestsPerMinute), } } @@ -275,6 +289,16 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) return al.processSystemMessage(ctx, msg) } + // Check request rate limit + if err := al.rateLimiter.checkRequest(); err != nil { + logger.WarnCF("agent", "Request rate limited", + map[string]interface{}{ + "channel": msg.Channel, + "sender_id": msg.SenderID, + }) + return fmt.Sprintf("Rate limited: %v. Please try again later.", err), nil + } + // Check for commands if response, handled := al.handleCommand(ctx, msg); handled { return response, nil @@ -643,6 +667,23 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, messages []providers.M // Execute tool calls for _, tc := range response.ToolCalls { + // Check tool call rate limit + if err := al.rateLimiter.checkToolCall(); err != nil { + logger.WarnCF("agent", "Tool call rate limited", + map[string]interface{}{ + "tool": tc.Name, + "iteration": iteration, + }) + toolResultMsg := providers.Message{ + Role: "tool", + Content: fmt.Sprintf("Rate limited: %v", err), + ToolCallID: tc.ID, + } + messages = append(messages, toolResultMsg) + al.sessions.AddFullMessage(opts.SessionKey, toolResultMsg) + continue + } + // Log tool call with arguments preview argsJSON, _ := json.Marshal(tc.Arguments) argsPreview := utils.Truncate(string(argsJSON), 200) diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go index e9b26f5ee..d51cdf95a 100644 --- a/pkg/agent/loop_test.go +++ b/pkg/agent/loop_test.go @@ -41,6 +41,7 @@ func TestRecordLastChannel(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -86,6 +87,7 @@ func TestRecordLastChatID(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -131,6 +133,7 @@ func TestNewAgentLoop_StateInitialized(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -167,6 +170,7 @@ func TestToolRegistry_ToolRegistration(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -213,6 +217,7 @@ func TestToolContext_Updates(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -244,6 +249,7 @@ func TestToolRegistry_GetDefinitions(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -288,6 +294,7 @@ func TestAgentLoop_GetStartupInfo(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -335,6 +342,7 @@ func TestCreateToolRegistry_ExecDisabled(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -374,6 +382,7 @@ func TestCreateToolRegistry_ExecEnabled(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -418,6 +427,7 @@ func TestCreateToolRegistry_I2CDisabled(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -455,6 +465,7 @@ func TestCreateToolRegistry_I2CEnabled(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -497,6 +508,7 @@ func TestCreateToolRegistry_SPIDisabled(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -534,6 +546,7 @@ func TestCreateToolRegistry_SPIEnabled(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -576,6 +589,7 @@ func TestAgentLoop_Stop(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -698,6 +712,7 @@ func TestToolResult_SilentToolDoesNotSendUserMessage(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -740,6 +755,7 @@ func TestToolResult_UserFacingToolDoesSendMessage(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, @@ -805,6 +821,7 @@ func TestAgentLoop_ContextExhaustionRetry(t *testing.T) { Agents: config.AgentsConfig{ Defaults: config.AgentDefaults{ Workspace: tmpDir, + DataDir: tmpDir, Model: "test-model", MaxTokens: 4096, MaxToolIterations: 10, diff --git a/pkg/agent/memory.go b/pkg/agent/memory.go index 3f6896f91..ea3650d11 100644 --- a/pkg/agent/memory.go +++ b/pkg/agent/memory.go @@ -17,22 +17,22 @@ import ( // - Long-term memory: memory/MEMORY.md // - Daily notes: memory/YYYYMM/YYYYMMDD.md type MemoryStore struct { - workspace string + dataDir string memoryDir string memoryFile string } -// NewMemoryStore creates a new MemoryStore with the given workspace path. +// NewMemoryStore creates a new MemoryStore with the given data directory path. // It ensures the memory directory exists. -func NewMemoryStore(workspace string) *MemoryStore { - memoryDir := filepath.Join(workspace, "memory") +func NewMemoryStore(dataDir string) *MemoryStore { + memoryDir := filepath.Join(dataDir, "memory") memoryFile := filepath.Join(memoryDir, "MEMORY.md") // Ensure memory directory exists os.MkdirAll(memoryDir, 0755) return &MemoryStore{ - workspace: workspace, + dataDir: dataDir, memoryDir: memoryDir, memoryFile: memoryFile, } diff --git a/pkg/agent/ratelimit.go b/pkg/agent/ratelimit.go new file mode 100644 index 000000000..940babc6d --- /dev/null +++ b/pkg/agent/ratelimit.go @@ -0,0 +1,84 @@ +package agent + +import ( + "fmt" + "sync" + "time" +) + +// rateLimiter provides simple sliding-window rate limiting for tool calls and requests. +type rateLimiter struct { + maxToolCallsPerMinute int + maxRequestsPerMinute int + + mu sync.Mutex + toolCallTimes []time.Time + requestTimes []time.Time +} + +func newRateLimiter(maxToolCalls, maxRequests int) *rateLimiter { + return &rateLimiter{ + maxToolCallsPerMinute: maxToolCalls, + maxRequestsPerMinute: maxRequests, + } +} + +// checkToolCall checks if a tool call is allowed under the rate limit. +// Returns nil if allowed, error if rate limited. +func (rl *rateLimiter) checkToolCall() error { + if rl.maxToolCallsPerMinute <= 0 { + return nil + } + + rl.mu.Lock() + defer rl.mu.Unlock() + + now := time.Now() + cutoff := now.Add(-time.Minute) + + // Remove expired entries + rl.toolCallTimes = pruneOld(rl.toolCallTimes, cutoff) + + if len(rl.toolCallTimes) >= rl.maxToolCallsPerMinute { + return fmt.Errorf("tool call limit exceeded (%d/min)", rl.maxToolCallsPerMinute) + } + + rl.toolCallTimes = append(rl.toolCallTimes, now) + return nil +} + +// checkRequest checks if a request is allowed under the rate limit. +// Returns nil if allowed, error if rate limited. +func (rl *rateLimiter) checkRequest() error { + if rl.maxRequestsPerMinute <= 0 { + return nil + } + + rl.mu.Lock() + defer rl.mu.Unlock() + + now := time.Now() + cutoff := now.Add(-time.Minute) + + // Remove expired entries + rl.requestTimes = pruneOld(rl.requestTimes, cutoff) + + if len(rl.requestTimes) >= rl.maxRequestsPerMinute { + return fmt.Errorf("request limit exceeded (%d/min)", rl.maxRequestsPerMinute) + } + + rl.requestTimes = append(rl.requestTimes, now) + return nil +} + +// pruneOld removes timestamps older than cutoff. +func pruneOld(times []time.Time, cutoff time.Time) []time.Time { + i := 0 + for i < len(times) && times[i].Before(cutoff) { + i++ + } + if i == 0 { + return times + } + return times[i:] +} diff --git a/pkg/agent/state/state.json b/pkg/agent/state/state.json new file mode 100644 index 000000000..bdf10f0e7 --- /dev/null +++ b/pkg/agent/state/state.json @@ -0,0 +1,5 @@ +{ + "last_channel": "test:test-chat", + "last_chat_id": "test-chat-id-123", + "timestamp": "2026-02-17T10:56:02.805709569Z" +} \ No newline at end of file diff --git a/pkg/config/config.go b/pkg/config/config.go index 4f7e0028e..2c6378555 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -44,14 +44,15 @@ func (f *FlexibleStringSlice) UnmarshalJSON(data []byte) error { } type Config struct { - Agents AgentsConfig `json:"agents"` - Channels ChannelsConfig `json:"channels"` - Providers ProvidersConfig `json:"providers"` - Gateway GatewayConfig `json:"gateway"` - Tools ToolsConfig `json:"tools"` - Heartbeat HeartbeatConfig `json:"heartbeat"` - Devices DevicesConfig `json:"devices"` - mu sync.RWMutex + Agents AgentsConfig `json:"agents"` + Channels ChannelsConfig `json:"channels"` + Providers ProvidersConfig `json:"providers"` + Gateway GatewayConfig `json:"gateway"` + Tools ToolsConfig `json:"tools"` + Heartbeat HeartbeatConfig `json:"heartbeat"` + Devices DevicesConfig `json:"devices"` + RateLimits RateLimitsConfig `json:"rate_limits"` + mu sync.RWMutex } type AgentsConfig struct { @@ -60,6 +61,7 @@ type AgentsConfig struct { type AgentDefaults struct { Workspace string `json:"workspace" env:"PICOCLAW_AGENTS_DEFAULTS_WORKSPACE"` + DataDir string `json:"data_dir" env:"PICOCLAW_AGENTS_DEFAULTS_DATA_DIR"` RestrictToWorkspace bool `json:"restrict_to_workspace" env:"PICOCLAW_AGENTS_DEFAULTS_RESTRICT_TO_WORKSPACE"` Provider string `json:"provider" env:"PICOCLAW_AGENTS_DEFAULTS_PROVIDER"` Model string `json:"model" env:"PICOCLAW_AGENTS_DEFAULTS_MODEL"` @@ -166,6 +168,11 @@ type DevicesConfig struct { MonitorUSB bool `json:"monitor_usb" env:"PICOCLAW_DEVICES_MONITOR_USB"` } +type RateLimitsConfig struct { + MaxToolCallsPerMinute int `json:"max_tool_calls_per_minute" env:"PICOCLAW_RATE_LIMITS_MAX_TOOL_CALLS_PER_MINUTE"` // 0 = unlimited + MaxRequestsPerMinute int `json:"max_requests_per_minute" env:"PICOCLAW_RATE_LIMITS_MAX_REQUESTS_PER_MINUTE"` // 0 = unlimited +} + type ProvidersConfig struct { Anthropic ProviderConfig `json:"anthropic"` OpenAI ProviderConfig `json:"openai"` @@ -235,6 +242,7 @@ func DefaultConfig() *Config { Agents: AgentsConfig{ Defaults: AgentDefaults{ Workspace: "~/.picoclaw/workspace", + DataDir: "~/.picoclaw/data", RestrictToWorkspace: true, Provider: "", Model: "glm-4.7", @@ -269,8 +277,8 @@ func DefaultConfig() *Config { }, MaixCam: MaixCamConfig{ Enabled: false, - Host: "0.0.0.0", - Port: 18790, + Host: "127.0.0.1", + Port: 18792, AllowFrom: FlexibleStringSlice{}, }, QQ: QQConfig{ @@ -295,7 +303,7 @@ func DefaultConfig() *Config { Enabled: false, ChannelSecret: "", ChannelAccessToken: "", - WebhookHost: "0.0.0.0", + WebhookHost: "127.0.0.1", WebhookPort: 18791, WebhookPath: "/webhook/line", AllowFrom: FlexibleStringSlice{}, @@ -322,7 +330,7 @@ func DefaultConfig() *Config { ShengSuanYun: ProviderConfig{}, }, Gateway: GatewayConfig{ - Host: "0.0.0.0", + Host: "127.0.0.1", Port: 18790, }, Tools: ToolsConfig{ @@ -355,6 +363,10 @@ func DefaultConfig() *Config { Enabled: false, MonitorUSB: true, }, + RateLimits: RateLimitsConfig{ + MaxToolCallsPerMinute: 60, + MaxRequestsPerMinute: 30, + }, } } @@ -403,6 +415,12 @@ func (c *Config) WorkspacePath() string { return expandHome(c.Agents.Defaults.Workspace) } +func (c *Config) DataPath() string { + c.mu.RLock() + defer c.mu.RUnlock() + return expandHome(c.Agents.Defaults.DataDir) +} + func (c *Config) GetAPIKey() string { c.mu.RLock() defer c.mu.RUnlock() diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index b7df307a8..dd8b0a310 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -67,7 +67,7 @@ func TestDefaultConfig_Temperature(t *testing.T) { func TestDefaultConfig_Gateway(t *testing.T) { cfg := DefaultConfig() - if cfg.Gateway.Host != "0.0.0.0" { + if cfg.Gateway.Host != "127.0.0.1" { t.Error("Gateway host should have default value") } if cfg.Gateway.Port == 0 { @@ -198,6 +198,34 @@ func TestSaveConfig_FilePermissions(t *testing.T) { } } +// TestDefaultConfig_DataDir verifies data dir default value +func TestDefaultConfig_DataDir(t *testing.T) { + cfg := DefaultConfig() + + if cfg.Agents.Defaults.DataDir == "" { + t.Error("DataDir should not be empty") + } + if cfg.Agents.Defaults.DataDir != "~/.picoclaw/data" { + t.Errorf("DataDir should be '~/.picoclaw/data', got '%s'", cfg.Agents.Defaults.DataDir) + } +} + +// TestConfig_DataPath verifies DataPath expands home directory +func TestConfig_DataPath(t *testing.T) { + cfg := DefaultConfig() + + path := cfg.DataPath() + if path == "" { + t.Error("DataPath should not be empty") + } + if path == "~/.picoclaw/data" { + t.Error("DataPath should expand ~ to home directory") + } + if path[0] == '~' { + t.Error("DataPath should not start with ~") + } +} + // TestConfig_Complete verifies all config fields are set func TestConfig_Complete(t *testing.T) { cfg := DefaultConfig() @@ -218,7 +246,7 @@ func TestConfig_Complete(t *testing.T) { if cfg.Agents.Defaults.MaxToolIterations == 0 { t.Error("MaxToolIterations should not be zero") } - if cfg.Gateway.Host != "0.0.0.0" { + if cfg.Gateway.Host != "127.0.0.1" { t.Error("Gateway host should have default value") } if cfg.Gateway.Port == 0 { diff --git a/pkg/heartbeat/service.go b/pkg/heartbeat/service.go index dfdaef58b..0cfc50684 100644 --- a/pkg/heartbeat/service.go +++ b/pkg/heartbeat/service.go @@ -34,6 +34,7 @@ type HeartbeatHandler func(prompt, channel, chatID string) *tools.ToolResult // HeartbeatService manages periodic heartbeat checks type HeartbeatService struct { workspace string + dataDir string bus *bus.MessageBus state *state.Manager handler HeartbeatHandler @@ -44,7 +45,7 @@ type HeartbeatService struct { } // NewHeartbeatService creates a new heartbeat service -func NewHeartbeatService(workspace string, intervalMinutes int, enabled bool) *HeartbeatService { +func NewHeartbeatService(workspace string, dataDir string, intervalMinutes int, enabled bool) *HeartbeatService { // Apply minimum interval if intervalMinutes < minIntervalMinutes && intervalMinutes != 0 { intervalMinutes = minIntervalMinutes @@ -56,9 +57,10 @@ func NewHeartbeatService(workspace string, intervalMinutes int, enabled bool) *H return &HeartbeatService{ workspace: workspace, + dataDir: dataDir, interval: time.Duration(intervalMinutes) * time.Minute, enabled: enabled, - state: state.NewManager(workspace), + state: state.NewManager(dataDir), } } @@ -217,7 +219,7 @@ func (hs *HeartbeatService) executeHeartbeat() { // buildPrompt builds the heartbeat prompt from HEARTBEAT.md func (hs *HeartbeatService) buildPrompt() string { - heartbeatPath := filepath.Join(hs.workspace, "HEARTBEAT.md") + heartbeatPath := filepath.Join(hs.dataDir, "HEARTBEAT.md") data, err := os.ReadFile(heartbeatPath) if err != nil { @@ -249,7 +251,7 @@ If there is nothing that requires attention, respond ONLY with: HEARTBEAT_OK // createDefaultHeartbeatTemplate creates the default HEARTBEAT.md file func (hs *HeartbeatService) createDefaultHeartbeatTemplate() { - heartbeatPath := filepath.Join(hs.workspace, "HEARTBEAT.md") + heartbeatPath := filepath.Join(hs.dataDir, "HEARTBEAT.md") defaultContent := `# Heartbeat Check List @@ -353,7 +355,7 @@ func (hs *HeartbeatService) logError(format string, args ...any) { // log writes a message to the heartbeat log file func (hs *HeartbeatService) log(level, format string, args ...any) { - logFile := filepath.Join(hs.workspace, "heartbeat.log") + logFile := filepath.Join(hs.dataDir, "heartbeat.log") f, err := os.OpenFile(logFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) if err != nil { return diff --git a/pkg/heartbeat/service_test.go b/pkg/heartbeat/service_test.go index a2b59e350..9dce0c292 100644 --- a/pkg/heartbeat/service_test.go +++ b/pkg/heartbeat/service_test.go @@ -16,7 +16,7 @@ func TestExecuteHeartbeat_Async(t *testing.T) { } defer os.RemoveAll(tmpDir) - hs := NewHeartbeatService(tmpDir, 30, true) + hs := NewHeartbeatService(tmpDir, tmpDir,30, true) hs.stopChan = make(chan struct{}) // Enable for testing asyncCalled := false @@ -54,7 +54,7 @@ func TestExecuteHeartbeat_Error(t *testing.T) { } defer os.RemoveAll(tmpDir) - hs := NewHeartbeatService(tmpDir, 30, true) + hs := NewHeartbeatService(tmpDir, tmpDir,30, true) hs.stopChan = make(chan struct{}) // Enable for testing hs.SetHandler(func(prompt, channel, chatID string) *tools.ToolResult { @@ -92,7 +92,7 @@ func TestExecuteHeartbeat_Silent(t *testing.T) { } defer os.RemoveAll(tmpDir) - hs := NewHeartbeatService(tmpDir, 30, true) + hs := NewHeartbeatService(tmpDir, tmpDir,30, true) hs.stopChan = make(chan struct{}) // Enable for testing hs.SetHandler(func(prompt, channel, chatID string) *tools.ToolResult { @@ -130,7 +130,7 @@ func TestHeartbeatService_StartStop(t *testing.T) { } defer os.RemoveAll(tmpDir) - hs := NewHeartbeatService(tmpDir, 1, true) + hs := NewHeartbeatService(tmpDir, tmpDir,1, true) err = hs.Start() if err != nil { @@ -149,7 +149,7 @@ func TestHeartbeatService_Disabled(t *testing.T) { } defer os.RemoveAll(tmpDir) - hs := NewHeartbeatService(tmpDir, 1, false) + hs := NewHeartbeatService(tmpDir, tmpDir,1, false) if hs.enabled != false { t.Error("Expected service to be disabled") @@ -166,7 +166,7 @@ func TestExecuteHeartbeat_NilResult(t *testing.T) { } defer os.RemoveAll(tmpDir) - hs := NewHeartbeatService(tmpDir, 30, true) + hs := NewHeartbeatService(tmpDir, tmpDir,30, true) hs.stopChan = make(chan struct{}) // Enable for testing hs.SetHandler(func(prompt, channel, chatID string) *tools.ToolResult { @@ -188,7 +188,7 @@ func TestLogPath(t *testing.T) { } defer os.RemoveAll(tmpDir) - hs := NewHeartbeatService(tmpDir, 30, true) + hs := NewHeartbeatService(tmpDir, tmpDir,30, true) // Write a log entry hs.log("INFO", "Test log entry") @@ -208,7 +208,7 @@ func TestHeartbeatFilePath(t *testing.T) { } defer os.RemoveAll(tmpDir) - hs := NewHeartbeatService(tmpDir, 30, true) + hs := NewHeartbeatService(tmpDir, tmpDir,30, true) // Trigger default template creation hs.buildPrompt() diff --git a/pkg/providers/http_provider.go b/pkg/providers/http_provider.go index 4cf2c6db2..aba3fe2b8 100644 --- a/pkg/providers/http_provider.go +++ b/pkg/providers/http_provider.go @@ -22,12 +22,13 @@ import ( ) type HTTPProvider struct { - apiKey string - apiBase string - httpClient *http.Client + apiKey string + apiBase string + providerName string + httpClient *http.Client } -func NewHTTPProvider(apiKey, apiBase, proxy string) *HTTPProvider { +func NewHTTPProvider(apiKey, apiBase, proxy, providerName string) *HTTPProvider { client := &http.Client{ Timeout: 120 * time.Second, } @@ -42,9 +43,10 @@ func NewHTTPProvider(apiKey, apiBase, proxy string) *HTTPProvider { } return &HTTPProvider{ - apiKey: apiKey, - apiBase: strings.TrimRight(apiBase, "/"), - httpClient: client, + apiKey: apiKey, + apiBase: strings.TrimRight(apiBase, "/"), + providerName: providerName, + httpClient: client, } } @@ -72,8 +74,7 @@ func (p *HTTPProvider) Chat(ctx context.Context, messages []Message, tools []Too } if maxTokens, ok := options["max_tokens"].(int); ok { - lowerModel := strings.ToLower(model) - if strings.Contains(lowerModel, "glm") || strings.Contains(lowerModel, "o1") { + if p.providerName == "openai" || p.providerName == "zhipu" || p.providerName == "glm" { requestBody["max_completion_tokens"] = maxTokens } else { requestBody["max_tokens"] = maxTokens @@ -82,9 +83,9 @@ func (p *HTTPProvider) Chat(ctx context.Context, messages []Message, tools []Too if temperature, ok := options["temperature"].(float64); ok { lowerModel := strings.ToLower(model) - // Kimi k2 models only support temperature=1 - if strings.Contains(lowerModel, "kimi") && strings.Contains(lowerModel, "k2") { - requestBody["temperature"] = 1.0 + // OpenAI reasoning models (o1/o3/o4, gpt-5.x) and Kimi k2 only support temperature=1 + if p.providerName == "openai" || (strings.Contains(lowerModel, "kimi") && strings.Contains(lowerModel, "k2")) { + // Don't send temperature; let the API use its default } else { requestBody["temperature"] = temperature } @@ -374,6 +375,7 @@ func CreateProvider(cfg *config.Config) (LLMProvider, error) { apiKey = cfg.Providers.OpenAI.APIKey apiBase = cfg.Providers.OpenAI.APIBase proxy = cfg.Providers.OpenAI.Proxy + providerName = "openai" if apiBase == "" { apiBase = "https://api.openai.com/v1" } @@ -390,6 +392,7 @@ func CreateProvider(cfg *config.Config) (LLMProvider, error) { apiKey = cfg.Providers.Zhipu.APIKey apiBase = cfg.Providers.Zhipu.APIBase proxy = cfg.Providers.Zhipu.Proxy + providerName = "zhipu" if apiBase == "" { apiBase = "https://open.bigmodel.cn/api/paas/v4" } @@ -446,5 +449,5 @@ func CreateProvider(cfg *config.Config) (LLMProvider, error) { return nil, fmt.Errorf("no API base configured for provider (model: %s)", model) } - return NewHTTPProvider(apiKey, apiBase, proxy), nil + return NewHTTPProvider(apiKey, apiBase, proxy, providerName), nil } diff --git a/pkg/skills/installer.go b/pkg/skills/installer.go deleted file mode 100644 index a3263c525..000000000 --- a/pkg/skills/installer.go +++ /dev/null @@ -1,171 +0,0 @@ -package skills - -import ( - "context" - "encoding/json" - "fmt" - "io" - "net/http" - "os" - "path/filepath" - "strings" - "time" -) - -type SkillInstaller struct { - workspace string -} - -type AvailableSkill struct { - Name string `json:"name"` - Repository string `json:"repository"` - Description string `json:"description"` - Author string `json:"author"` - Tags []string `json:"tags"` -} - -type BuiltinSkill struct { - Name string `json:"name"` - Path string `json:"path"` - Enabled bool `json:"enabled"` -} - -func NewSkillInstaller(workspace string) *SkillInstaller { - return &SkillInstaller{ - workspace: workspace, - } -} - -func (si *SkillInstaller) InstallFromGitHub(ctx context.Context, repo string) error { - skillDir := filepath.Join(si.workspace, "skills", filepath.Base(repo)) - - if _, err := os.Stat(skillDir); err == nil { - return fmt.Errorf("skill '%s' already exists", filepath.Base(repo)) - } - - url := fmt.Sprintf("https://raw.githubusercontent.com/%s/main/SKILL.md", repo) - - client := &http.Client{Timeout: 15 * time.Second} - req, err := http.NewRequestWithContext(ctx, "GET", url, nil) - if err != nil { - return fmt.Errorf("failed to create request: %w", err) - } - - resp, err := client.Do(req) - if err != nil { - return fmt.Errorf("failed to fetch skill: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != 200 { - return fmt.Errorf("failed to fetch skill: HTTP %d", resp.StatusCode) - } - - body, err := io.ReadAll(resp.Body) - if err != nil { - return fmt.Errorf("failed to read response: %w", err) - } - - if err := os.MkdirAll(skillDir, 0755); err != nil { - return fmt.Errorf("failed to create skill directory: %w", err) - } - - skillPath := filepath.Join(skillDir, "SKILL.md") - if err := os.WriteFile(skillPath, body, 0644); err != nil { - return fmt.Errorf("failed to write skill file: %w", err) - } - - return nil -} - -func (si *SkillInstaller) Uninstall(skillName string) error { - skillDir := filepath.Join(si.workspace, "skills", skillName) - - if _, err := os.Stat(skillDir); os.IsNotExist(err) { - return fmt.Errorf("skill '%s' not found", skillName) - } - - if err := os.RemoveAll(skillDir); err != nil { - return fmt.Errorf("failed to remove skill: %w", err) - } - - return nil -} - -func (si *SkillInstaller) ListAvailableSkills(ctx context.Context) ([]AvailableSkill, error) { - url := "https://raw.githubusercontent.com/sipeed/picoclaw-skills/main/skills.json" - - client := &http.Client{Timeout: 15 * time.Second} - req, err := http.NewRequestWithContext(ctx, "GET", url, nil) - if err != nil { - return nil, fmt.Errorf("failed to create request: %w", err) - } - - resp, err := client.Do(req) - if err != nil { - return nil, fmt.Errorf("failed to fetch skills list: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != 200 { - return nil, fmt.Errorf("failed to fetch skills list: HTTP %d", resp.StatusCode) - } - - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response: %w", err) - } - - var skills []AvailableSkill - if err := json.Unmarshal(body, &skills); err != nil { - return nil, fmt.Errorf("failed to parse skills list: %w", err) - } - - return skills, nil -} - -func (si *SkillInstaller) ListBuiltinSkills() []BuiltinSkill { - builtinSkillsDir := filepath.Join(filepath.Dir(si.workspace), "picoclaw", "skills") - - entries, err := os.ReadDir(builtinSkillsDir) - if err != nil { - return nil - } - - var skills []BuiltinSkill - for _, entry := range entries { - if entry.IsDir() { - _ = entry - skillName := entry.Name() - skillFile := filepath.Join(builtinSkillsDir, skillName, "SKILL.md") - - data, err := os.ReadFile(skillFile) - description := "" - if err == nil { - content := string(data) - if idx := strings.Index(content, "\n"); idx > 0 { - firstLine := content[:idx] - if strings.Contains(firstLine, "description:") { - descLine := strings.Index(content[idx:], "\n") - if descLine > 0 { - description = strings.TrimSpace(content[idx+descLine : idx+descLine]) - } - } - } - } - - // skill := BuiltinSkill{ - // Name: skillName, - // Path: description, - // Enabled: true, - // } - - status := "✓" - fmt.Printf(" %s %s\n", status, entry.Name()) - if description != "" { - fmt.Printf(" %s\n", description) - } - } - } - return skills -} diff --git a/pkg/skills/loader.go b/pkg/skills/loader.go index 0c63ae067..6615e93c4 100644 --- a/pkg/skills/loader.go +++ b/pkg/skills/loader.go @@ -52,29 +52,29 @@ func (info SkillInfo) validate() error { } type SkillsLoader struct { - workspace string - workspaceSkills string // workspace skills (项目级别) - globalSkills string // 全局 skills (~/.picoclaw/skills) - builtinSkills string // 内置 skills + dataDir string + dataSkills string // data dir skills + globalSkills string // global skills (~/.picoclaw/skills) + builtinSkills string // builtin skills } -func NewSkillsLoader(workspace string, globalSkills string, builtinSkills string) *SkillsLoader { +func NewSkillsLoader(dataDir string, globalSkills string, builtinSkills string) *SkillsLoader { return &SkillsLoader{ - workspace: workspace, - workspaceSkills: filepath.Join(workspace, "skills"), - globalSkills: globalSkills, // ~/.picoclaw/skills - builtinSkills: builtinSkills, + dataDir: dataDir, + dataSkills: filepath.Join(dataDir, "skills"), + globalSkills: globalSkills, // ~/.picoclaw/skills + builtinSkills: builtinSkills, } } func (sl *SkillsLoader) ListSkills() []SkillInfo { skills := make([]SkillInfo, 0) - if sl.workspaceSkills != "" { - if dirs, err := os.ReadDir(sl.workspaceSkills); err == nil { + if sl.dataSkills != "" { + if dirs, err := os.ReadDir(sl.dataSkills); err == nil { for _, dir := range dirs { if dir.IsDir() { - skillFile := filepath.Join(sl.workspaceSkills, dir.Name(), "SKILL.md") + skillFile := filepath.Join(sl.dataSkills, dir.Name(), "SKILL.md") if _, err := os.Stat(skillFile); err == nil { info := SkillInfo{ Name: dir.Name(), @@ -87,7 +87,7 @@ func (sl *SkillsLoader) ListSkills() []SkillInfo { info.Name = metadata.Name } if err := info.validate(); err != nil { - slog.Warn("invalid skill from workspace", "name", info.Name, "error", err) + slog.Warn("invalid skill from data dir", "name", info.Name, "error", err) continue } skills = append(skills, info) @@ -181,8 +181,8 @@ func (sl *SkillsLoader) ListSkills() []SkillInfo { func (sl *SkillsLoader) LoadSkill(name string) (string, bool) { // 1. 优先从 workspace skills 加载（项目级别） - if sl.workspaceSkills != "" { - skillFile := filepath.Join(sl.workspaceSkills, name, "SKILL.md") + if sl.dataSkills != "" { + skillFile := filepath.Join(sl.dataSkills, name, "SKILL.md") if content, err := os.ReadFile(skillFile); err == nil { return sl.stripFrontmatter(string(content)), true } @@ -234,12 +234,10 @@ func (sl *SkillsLoader) BuildSkillsSummary() string { for _, s := range allSkills { escapedName := escapeXML(s.Name) escapedDesc := escapeXML(s.Description) - escapedPath := escapeXML(s.Path) lines = append(lines, fmt.Sprintf(" ")) lines = append(lines, fmt.Sprintf(" %s", escapedName)) lines = append(lines, fmt.Sprintf(" %s", escapedDesc)) - lines = append(lines, fmt.Sprintf(" %s", escapedPath)) lines = append(lines, fmt.Sprintf(" %s", s.Source)) lines = append(lines, " ") } diff --git a/pkg/tools/memory.go b/pkg/tools/memory.go new file mode 100644 index 000000000..a216432f9 --- /dev/null +++ b/pkg/tools/memory.go @@ -0,0 +1,95 @@ +package tools + +import ( + "context" + "fmt" +) + +// MemoryWriter is the interface for memory operations. +// Implemented by agent.MemoryStore. +type MemoryWriter interface { + WriteLongTerm(content string) error + AppendToday(content string) error + ReadLongTerm() string + ReadToday() string +} + +type MemoryTool struct { + writer MemoryWriter +} + +func NewMemoryTool(writer MemoryWriter) *MemoryTool { + return &MemoryTool{writer: writer} +} + +func (t *MemoryTool) Name() string { + return "memory" +} + +func (t *MemoryTool) Description() string { + return "Store and retrieve persistent memory. Actions: write_long_term, append_daily, read_long_term, read_daily" +} + +func (t *MemoryTool) Parameters() map[string]interface{} { + return map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "action": map[string]interface{}{ + "type": "string", + "description": "The memory action to perform", + "enum": []string{"write_long_term", "append_daily", "read_long_term", "read_daily"}, + }, + "content": map[string]interface{}{ + "type": "string", + "description": "Content to write (required for write_long_term and append_daily)", + }, + }, + "required": []string{"action"}, + } +} + +func (t *MemoryTool) Execute(ctx context.Context, args map[string]interface{}) *ToolResult { + action, ok := args["action"].(string) + if !ok { + return ErrorResult("action is required") + } + + switch action { + case "write_long_term": + content, ok := args["content"].(string) + if !ok || content == "" { + return ErrorResult("content is required for write_long_term") + } + if err := t.writer.WriteLongTerm(content); err != nil { + return ErrorResult(fmt.Sprintf("failed to write long-term memory: %v", err)) + } + return SilentResult("Long-term memory updated successfully") + + case "append_daily": + content, ok := args["content"].(string) + if !ok || content == "" { + return ErrorResult("content is required for append_daily") + } + if err := t.writer.AppendToday(content); err != nil { + return ErrorResult(fmt.Sprintf("failed to append daily note: %v", err)) + } + return SilentResult("Daily note appended successfully") + + case "read_long_term": + content := t.writer.ReadLongTerm() + if content == "" { + return SilentResult("No long-term memory found") + } + return SilentResult(content) + + case "read_daily": + content := t.writer.ReadToday() + if content == "" { + return SilentResult("No daily notes for today") + } + return SilentResult(content) + + default: + return ErrorResult(fmt.Sprintf("unknown action: %s", action)) + } +} diff --git a/pkg/tools/memory_test.go b/pkg/tools/memory_test.go new file mode 100644 index 000000000..be929ff51 --- /dev/null +++ b/pkg/tools/memory_test.go @@ -0,0 +1,162 @@ +package tools + +import ( + "context" + "testing" +) + +type mockMemoryWriter struct { + longTerm string + daily string +} + +func (m *mockMemoryWriter) WriteLongTerm(content string) error { + m.longTerm = content + return nil +} + +func (m *mockMemoryWriter) AppendToday(content string) error { + m.daily += content + return nil +} + +func (m *mockMemoryWriter) ReadLongTerm() string { + return m.longTerm +} + +func (m *mockMemoryWriter) ReadToday() string { + return m.daily +} + +func TestMemoryTool_WriteLongTerm(t *testing.T) { + w := &mockMemoryWriter{} + tool := NewMemoryTool(w) + + result := tool.Execute(context.Background(), map[string]interface{}{ + "action": "write_long_term", + "content": "User prefers dark mode", + }) + + if result.IsError { + t.Fatalf("unexpected error: %s", result.ForLLM) + } + if w.longTerm != "User prefers dark mode" { + t.Errorf("expected 'User prefers dark mode', got '%s'", w.longTerm) + } +} + +func TestMemoryTool_WriteLongTerm_MissingContent(t *testing.T) { + w := &mockMemoryWriter{} + tool := NewMemoryTool(w) + + result := tool.Execute(context.Background(), map[string]interface{}{ + "action": "write_long_term", + }) + + if !result.IsError { + t.Error("expected error for missing content") + } +} + +func TestMemoryTool_AppendDaily(t *testing.T) { + w := &mockMemoryWriter{} + tool := NewMemoryTool(w) + + tool.Execute(context.Background(), map[string]interface{}{ + "action": "append_daily", + "content": "Met with team.", + }) + tool.Execute(context.Background(), map[string]interface{}{ + "action": "append_daily", + "content": " Discussed roadmap.", + }) + + if w.daily != "Met with team. Discussed roadmap." { + t.Errorf("expected appended content, got '%s'", w.daily) + } +} + +func TestMemoryTool_ReadLongTerm(t *testing.T) { + w := &mockMemoryWriter{longTerm: "stored facts"} + tool := NewMemoryTool(w) + + result := tool.Execute(context.Background(), map[string]interface{}{ + "action": "read_long_term", + }) + + if result.IsError { + t.Fatalf("unexpected error: %s", result.ForLLM) + } + if result.ForLLM != "stored facts" { + t.Errorf("expected 'stored facts', got '%s'", result.ForLLM) + } +} + +func TestMemoryTool_ReadLongTerm_Empty(t *testing.T) { + w := &mockMemoryWriter{} + tool := NewMemoryTool(w) + + result := tool.Execute(context.Background(), map[string]interface{}{ + "action": "read_long_term", + }) + + if result.IsError { + t.Error("should not error on empty memory") + } + if result.ForLLM != "No long-term memory found" { + t.Errorf("expected empty message, got '%s'", result.ForLLM) + } +} + +func TestMemoryTool_ReadDaily(t *testing.T) { + w := &mockMemoryWriter{daily: "today's notes"} + tool := NewMemoryTool(w) + + result := tool.Execute(context.Background(), map[string]interface{}{ + "action": "read_daily", + }) + + if result.ForLLM != "today's notes" { + t.Errorf("expected 'today's notes', got '%s'", result.ForLLM) + } +} + +func TestMemoryTool_UnknownAction(t *testing.T) { + w := &mockMemoryWriter{} + tool := NewMemoryTool(w) + + result := tool.Execute(context.Background(), map[string]interface{}{ + "action": "delete_all", + }) + + if !result.IsError { + t.Error("expected error for unknown action") + } +} + +func TestMemoryTool_MissingAction(t *testing.T) { + w := &mockMemoryWriter{} + tool := NewMemoryTool(w) + + result := tool.Execute(context.Background(), map[string]interface{}{}) + + if !result.IsError { + t.Error("expected error for missing action") + } +} + +func TestMemoryTool_NameAndDescription(t *testing.T) { + w := &mockMemoryWriter{} + tool := NewMemoryTool(w) + + if tool.Name() != "memory" { + t.Errorf("expected name 'memory', got '%s'", tool.Name()) + } + if tool.Description() == "" { + t.Error("description should not be empty") + } + params := tool.Parameters() + if params == nil { + t.Error("parameters should not be nil") + } +} diff --git a/pkg/tools/skills.go b/pkg/tools/skills.go new file mode 100644 index 000000000..3d797c862 --- /dev/null +++ b/pkg/tools/skills.go @@ -0,0 +1,77 @@ +package tools + +import ( + "context" + "fmt" + "strings" + + "github.com/sipeed/picoclaw/pkg/skills" +) + +type SkillTool struct { + loader *skills.SkillsLoader +} + +func NewSkillTool(loader *skills.SkillsLoader) *SkillTool { + return &SkillTool{loader: loader} +} + +func (t *SkillTool) Name() string { + return "skill" +} + +func (t *SkillTool) Description() string { + return "Read skill instructions. Actions: skill_list (list available skills), skill_read (read a skill's content)" +} + +func (t *SkillTool) Parameters() map[string]interface{} { + return map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "action": map[string]interface{}{ + "type": "string", + "description": "The skill action to perform", + "enum": []string{"skill_list", "skill_read"}, + }, + "name": map[string]interface{}{ + "type": "string", + "description": "Skill name (required for skill_read)", + }, + }, + "required": []string{"action"}, + } +} + +func (t *SkillTool) Execute(ctx context.Context, args map[string]interface{}) *ToolResult { + action, ok := args["action"].(string) + if !ok { + return ErrorResult("action is required") + } + + switch action { + case "skill_list": + allSkills := t.loader.ListSkills() + if len(allSkills) == 0 { + return SilentResult("No skills available") + } + var sb strings.Builder + for _, s := range allSkills { + sb.WriteString(fmt.Sprintf("- %s (%s): %s\n", s.Name, s.Source, s.Description)) + } + return SilentResult(sb.String()) + + case "skill_read": + name, ok := args["name"].(string) + if !ok || name == "" { + return ErrorResult("name is required for skill_read") + } + content, found := t.loader.LoadSkill(name) + if !found { + return ErrorResult(fmt.Sprintf("skill %q not found", name)) + } + return SilentResult(content) + + default: + return ErrorResult(fmt.Sprintf("unknown action: %s", action)) + } +} diff --git a/pkg/tools/skills_test.go b/pkg/tools/skills_test.go new file mode 100644 index 000000000..506b4b003 --- /dev/null +++ b/pkg/tools/skills_test.go @@ -0,0 +1,163 @@ +package tools + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/sipeed/picoclaw/pkg/skills" +) + +func setupSkillFixture(t *testing.T) (string, *skills.SkillsLoader) { + t.Helper() + tmpDir, err := os.MkdirTemp("", "skill-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + + // Create a test skill + skillDir := filepath.Join(tmpDir, "skills", "test-skill") + os.MkdirAll(skillDir, 0755) + os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(`--- +name: test-skill +description: "A test skill for unit testing" +--- + +# Test Skill + +This is the skill content. +`), 0644) + + loader := skills.NewSkillsLoader(tmpDir, "", "") + return tmpDir, loader +} + +func TestSkillTool_SkillList(t *testing.T) { + tmpDir, loader := setupSkillFixture(t) + defer os.RemoveAll(tmpDir) + + tool := NewSkillTool(loader) + result := tool.Execute(context.Background(), map[string]interface{}{ + "action": "skill_list", + }) + + if result.IsError { + t.Fatalf("unexpected error: %s", result.ForLLM) + } + if result.ForLLM == "No skills available" { + t.Error("expected skills to be listed") + } +} + +func TestSkillTool_SkillList_Empty(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "skill-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + loader := skills.NewSkillsLoader(tmpDir, "", "") + tool := NewSkillTool(loader) + + result := tool.Execute(context.Background(), map[string]interface{}{ + "action": "skill_list", + }) + + if result.IsError { + t.Error("empty list should not be an error") + } + if result.ForLLM != "No skills available" { + t.Errorf("expected 'No skills available', got '%s'", result.ForLLM) + } +} + +func TestSkillTool_SkillRead(t *testing.T) { + tmpDir, loader := setupSkillFixture(t) + defer os.RemoveAll(tmpDir) + + tool := NewSkillTool(loader) + result := tool.Execute(context.Background(), map[string]interface{}{ + "action": "skill_read", + "name": "test-skill", + }) + + if result.IsError { + t.Fatalf("unexpected error: %s", result.ForLLM) + } + if result.ForLLM == "" { + t.Error("expected skill content") + } +} + +func TestSkillTool_SkillRead_NotFound(t *testing.T) { + tmpDir, loader := setupSkillFixture(t) + defer os.RemoveAll(tmpDir) + + tool := NewSkillTool(loader) + result := tool.Execute(context.Background(), map[string]interface{}{ + "action": "skill_read", + "name": "nonexistent", + }) + + if !result.IsError { + t.Error("expected error for nonexistent skill") + } +} + +func TestSkillTool_SkillRead_MissingName(t *testing.T) { + tmpDir, loader := setupSkillFixture(t) + defer os.RemoveAll(tmpDir) + + tool := NewSkillTool(loader) + result := tool.Execute(context.Background(), map[string]interface{}{ + "action": "skill_read", + }) + + if !result.IsError { + t.Error("expected error for missing name") + } +} + +func TestSkillTool_UnknownAction(t *testing.T) { + tmpDir, loader := setupSkillFixture(t) + defer os.RemoveAll(tmpDir) + + tool := NewSkillTool(loader) + result := tool.Execute(context.Background(), map[string]interface{}{ + "action": "skill_delete", + }) + + if !result.IsError { + t.Error("expected error for unknown action") + } +} + +func TestSkillTool_MissingAction(t *testing.T) { + tmpDir, loader := setupSkillFixture(t) + defer os.RemoveAll(tmpDir) + + tool := NewSkillTool(loader) + result := tool.Execute(context.Background(), map[string]interface{}{}) + + if !result.IsError { + t.Error("expected error for missing action") + } +} + +func TestSkillTool_NameAndDescription(t *testing.T) { + tmpDir, loader := setupSkillFixture(t) + defer os.RemoveAll(tmpDir) + + tool := NewSkillTool(loader) + + if tool.Name() != "skill" { + t.Errorf("expected name 'skill', got '%s'", tool.Name()) + } + if tool.Description() == "" { + t.Error("description should not be empty") + } + if tool.Parameters() == nil { + t.Error("parameters should not be nil") + } +}