From 841bd0098a88e534402ca612c275a61ebf247701 Mon Sep 17 00:00:00 2001 From: stevef Date: Sat, 4 Apr 2026 06:28:09 +0200 Subject: [PATCH] chore: address linter issues from PR review --- cmd/picoclaw/internal/skills/command.go | 4 +++- pkg/agent/instance.go | 6 ++++-- pkg/agent/instance_test.go | 1 + pkg/agent/isolation_tools_test.go | 1 + pkg/agent/loop.go | 9 +++++++-- pkg/agent/loop_mcp.go | 2 +- pkg/config/config.go | 2 +- pkg/gateway/gateway.go | 3 ++- pkg/health/server.go | 9 ++------- pkg/tools/edit.go | 6 ++++-- pkg/tools/filesystem.go | 3 ++- pkg/tools/registry.go | 4 +++- pkg/tools/registry_test.go | 2 +- 13 files changed, 32 insertions(+), 20 deletions(-) diff --git a/cmd/picoclaw/internal/skills/command.go b/cmd/picoclaw/internal/skills/command.go index 19caca9ec..b8f660096 100644 --- a/cmd/picoclaw/internal/skills/command.go +++ b/cmd/picoclaw/internal/skills/command.go @@ -43,7 +43,9 @@ func NewSkillsCommand() *cobra.Command { globalDir := filepath.Dir(internal.GetConfigPath()) globalSkillsDir := filepath.Join(globalDir, "skills") builtinSkillsDir := filepath.Join(globalDir, "picoclaw", "skills") - d.skillsLoader = skills.NewSkillsLoader(d.workspace, d.workspace, globalSkillsDir, builtinSkillsDir, nil, false) + d.skillsLoader = skills.NewSkillsLoader( + d.workspace, d.workspace, globalSkillsDir, builtinSkillsDir, nil, false, + ) return nil }, diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index 3da0538b8..8a9463a46 100644 --- a/pkg/agent/instance.go +++ b/pkg/agent/instance.go @@ -82,7 +82,9 @@ func NewAgentInstance( maxReadFileSize := cfg.Tools.ReadFile.MaxReadFileSize switch cfg.Tools.ReadFile.EffectiveMode() { case config.ReadFileModeLines: - toolsRegistry.Register(tools.NewReadFileLinesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths, denyReadPaths)) + toolsRegistry.Register(tools.NewReadFileLinesTool( + workspace, readRestrict, maxReadFileSize, allowReadPaths, denyReadPaths, + )) default: toolsRegistry.Register(tools.NewReadFileBytesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths, denyReadPaths)) } @@ -248,7 +250,7 @@ func NewAgentInstance( // resolveAgentWorkspace determines the workspace directory for an agent. func resolveAgentWorkspace(agentCfg *config.AgentConfig, defaults *config.AgentDefaults, isolationID string) string { - base := "" + var base string if agentCfg != nil && strings.TrimSpace(agentCfg.Workspace) != "" { base = expandHome(strings.TrimSpace(agentCfg.Workspace)) } else if agentCfg == nil || agentCfg.Default || agentCfg.ID == "" || routing.NormalizeAgentID(agentCfg.ID) == "main" { diff --git a/pkg/agent/instance_test.go b/pkg/agent/instance_test.go index 209477a50..513935148 100644 --- a/pkg/agent/instance_test.go +++ b/pkg/agent/instance_test.go @@ -374,6 +374,7 @@ func TestNewAgentInstance_InvalidExecConfigDoesNotExit(t *testing.T) { t.Fatal("read_file tool should still be registered") } } + func TestNewAgentInstance_IsolatedWorkspace(t *testing.T) { tmpDir := t.TempDir() cfg := &config.Config{ diff --git a/pkg/agent/isolation_tools_test.go b/pkg/agent/isolation_tools_test.go index 989cd21d8..f4d11cfc3 100644 --- a/pkg/agent/isolation_tools_test.go +++ b/pkg/agent/isolation_tools_test.go @@ -22,6 +22,7 @@ func (m *isolationMockTool) Description() string { return "mock tool" } func (m *isolationMockTool) Parameters() map[string]any { return map[string]any{"type": "object", "properties": map[string]any{}} } + func (m *isolationMockTool) Execute(ctx context.Context, args map[string]any) *tools.ToolResult { return tools.SilentResult("executed") } diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index fba643fdd..189334f01 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -1184,13 +1184,14 @@ func (al *AgentLoop) GetConfig() *config.Config { return al.cfg } -// SetMediaStore injects a MediaStore for media lifecycle management. +// GetMediaStore returns the currently configured MediaStore. func (al *AgentLoop) GetMediaStore() media.MediaStore { al.mu.RLock() defer al.mu.RUnlock() return al.mediaStore } +// SetMediaStore injects a MediaStore for media lifecycle management. func (al *AgentLoop) SetMediaStore(s media.MediaStore) { al.mediaStore = s @@ -1640,7 +1641,11 @@ func (al *AgentLoop) getOrCreateIsolatedAgent(agentID, channel, isolationID stri agent.Tools.SetMediaStore(al.mediaStore) // Re-register shared tools (web, message, spawn) to this transient agent - registerSharedTools(al, al.cfg, al.bus, &AgentRegistry{agents: map[string]*AgentInstance{agent.ID: agent}}, baseAgent.Provider) + registerSharedTools( + al, al.cfg, al.bus, + &AgentRegistry{agents: map[string]*AgentInstance{agent.ID: agent}}, + baseAgent.Provider, + ) // Cache this agent instance per chat session al.agentCache.Store(cacheKey, agent) diff --git a/pkg/agent/loop_mcp.go b/pkg/agent/loop_mcp.go index 5e7541d33..ea6613103 100644 --- a/pkg/agent/loop_mcp.go +++ b/pkg/agent/loop_mcp.go @@ -56,7 +56,7 @@ func (r *mcpRuntime) getManager() *mcp.Manager { return r.manager } -// ensureMCPInitialized loads MCP servers/tools once so both Run() and direct +// EnsureMCPInitialized loads MCP servers/tools once so both Run() and direct // agent mode share the same initialization path. func (al *AgentLoop) EnsureMCPInitialized(ctx context.Context) error { if !al.cfg.Tools.IsToolEnabled("mcp") { diff --git a/pkg/config/config.go b/pkg/config/config.go index 15f52d62a..442953981 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -842,7 +842,7 @@ type SkillsToolsConfig struct { ToolConfig ` yaml:"-" envPrefix:"PICOCLAW_TOOLS_SKILLS_"` Registries SkillsRegistriesConfig `yaml:",inline,omitempty" json:"registries"` Github SkillsGithubConfig `yaml:"github,omitempty" json:"github"` - MaxConcurrentSearches int `yaml:"-" json:"max_concurrent_searches" env:"PICOCLAW_TOOLS_SKILLS_MAX_CONCURRENT_SEARCHES"` + MaxConcurrentSearches int `yaml:"-" json:"max_concurrent_searches" env:"PICOCLAW_TOOLS_SKILLS_MAX_CONCURRENT_SEARCHES"` SearchCache SearchCacheConfig `yaml:"-" json:"search_cache"` Whitelist FlexibleStringSlice `json:"whitelist,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_SKILLS_WHITELIST"` WhitelistEnabled bool `json:"whitelist_enabled,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_SKILLS_WHITELIST_ENABLED"` diff --git a/pkg/gateway/gateway.go b/pkg/gateway/gateway.go index 9a3f79e2f..bd22568ae 100644 --- a/pkg/gateway/gateway.go +++ b/pkg/gateway/gateway.go @@ -168,7 +168,8 @@ func Run(debug bool, homePath, configPath string, allowEmptyStartup bool) error } defer pid.RemovePidFile(homePath) - fmt.Printf("🔍 Creating startup provider for model: %s (allow empty: %v)\n", cfg.Agents.Defaults.GetModelName(), allowEmptyStartup) + fmt.Printf("🔍 Creating startup provider for model: %s (allow empty: %v)\n", + cfg.Agents.Defaults.GetModelName(), allowEmptyStartup) provider, modelID, err := createStartupProvider(cfg, allowEmptyStartup) if err != nil { fmt.Printf("❌ Error creating provider: %v\n", err) diff --git a/pkg/health/server.go b/pkg/health/server.go index 273dc3ba9..bef7de7b7 100644 --- a/pkg/health/server.go +++ b/pkg/health/server.go @@ -304,14 +304,9 @@ func (s *Server) readyHandler(w http.ResponseWriter, r *http.Request) { }) } -// HandlerMux defines the interface for an HTTP request multiplexer. -type HandlerMux interface { - HandleFunc(pattern string, handler func(http.ResponseWriter, *http.Request)) -} - // RegisterOnMux registers /health, /ready, /reload and /chat handlers onto the // given mux. This allows the health endpoints to be served by a shared HTTP server. -func (s *Server) RegisterOnMux(mux HandlerMux) { +func (s *Server) RegisterOnMux(mux Mux) { mux.HandleFunc("/health", s.healthHandler) mux.HandleFunc("/ready", s.readyHandler) mux.HandleFunc("/reload", s.reloadHandler) @@ -449,7 +444,7 @@ func (s *Server) handlePostChat(w http.ResponseWriter, r *http.Request) { // Start processing in background go func() { // Use a long-running context for the chat call, but don't bind to r.Context() - // which will be cancelled when this request finishes. + // which will be canceled when this request finishes. ctx := context.Background() logger.Debugf("Starting async chat for session %s", sessionID) reply, err := chatFunc(ctx, req.Message, sessionID, chatID) diff --git a/pkg/tools/edit.go b/pkg/tools/edit.go index e84481c94..4a432acf3 100644 --- a/pkg/tools/edit.go +++ b/pkg/tools/edit.go @@ -16,7 +16,8 @@ type EditFileTool struct { } // NewEditFileTool creates a new EditFileTool with optional directory restriction. -func NewEditFileTool(workspace string, restrict bool, allowPaths []*regexp.Regexp, denyPaths ...[]*regexp.Regexp) *EditFileTool { +func NewEditFileTool(workspace string, restrict bool, allowPaths []*regexp.Regexp, + denyPaths ...[]*regexp.Regexp) *EditFileTool { var denyPatterns []*regexp.Regexp if len(denyPaths) > 0 { denyPatterns = denyPaths[0] @@ -79,7 +80,8 @@ type AppendFileTool struct { fs fileSystem } -func NewAppendFileTool(workspace string, restrict bool, allowPaths []*regexp.Regexp, denyPaths ...[]*regexp.Regexp) *AppendFileTool { +func NewAppendFileTool(workspace string, restrict bool, allowPaths []*regexp.Regexp, + denyPaths ...[]*regexp.Regexp) *AppendFileTool { var denyPatterns []*regexp.Regexp if len(denyPaths) > 0 { denyPatterns = denyPaths[0] diff --git a/pkg/tools/filesystem.go b/pkg/tools/filesystem.go index 84e5a6388..4364d49b9 100644 --- a/pkg/tools/filesystem.go +++ b/pkg/tools/filesystem.go @@ -1283,7 +1283,8 @@ func getSafeRelPath(workspace, path string) (string, error) { // validatePathWithConfigs returns the resolved absolute path if it is allowed // by the given workspace, restriction setting, and path whitelist/blacklist. -func validatePathWithConfigs(path, workspace string, restrict bool, allowPatterns, denyPatterns []*regexp.Regexp) (string, error) { +func validatePathWithConfigs(path, workspace string, restrict bool, + allowPatterns, denyPatterns []*regexp.Regexp) (string, error) { cleaned := filepath.Clean(path) var resolved string diff --git a/pkg/tools/registry.go b/pkg/tools/registry.go index b8e9bd3e2..b7d9e8538 100644 --- a/pkg/tools/registry.go +++ b/pkg/tools/registry.go @@ -449,7 +449,9 @@ func (r *ToolRegistry) Filter(whitelist []string, enabled bool) { for _, w := range whitelist { // Match exact (redundant but safe) or prefix with underscore // We also check for "mcp_" prefix specifically to support MCP tool grouping - if strings.HasPrefix(name, "mcp_"+w+"_") || strings.HasPrefix(name, "tool_"+w+"_") || strings.HasPrefix(name, w+"_") { + if strings.HasPrefix(name, "mcp_"+w+"_") || + strings.HasPrefix(name, "tool_"+w+"_") || + strings.HasPrefix(name, w+"_") { allowed = true break } diff --git a/pkg/tools/registry_test.go b/pkg/tools/registry_test.go index 3ca4cee4b..c5f6ed29f 100644 --- a/pkg/tools/registry_test.go +++ b/pkg/tools/registry_test.go @@ -791,7 +791,7 @@ func TestToolRegistry_Filter_SupportsPrefix(t *testing.T) { } if len(expected) > 0 { - var missing []string + missing := make([]string, 0, len(expected)) for m := range expected { missing = append(missing, m) }