chore: address linter issues from PR review
This commit is contained in:
parent
bf7756466e
commit
841bd0098a
13 changed files with 32 additions and 20 deletions
|
|
@ -43,7 +43,9 @@ func NewSkillsCommand() *cobra.Command {
|
||||||
globalDir := filepath.Dir(internal.GetConfigPath())
|
globalDir := filepath.Dir(internal.GetConfigPath())
|
||||||
globalSkillsDir := filepath.Join(globalDir, "skills")
|
globalSkillsDir := filepath.Join(globalDir, "skills")
|
||||||
builtinSkillsDir := filepath.Join(globalDir, "picoclaw", "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
|
return nil
|
||||||
},
|
},
|
||||||
|
|
|
||||||
|
|
@ -82,7 +82,9 @@ func NewAgentInstance(
|
||||||
maxReadFileSize := cfg.Tools.ReadFile.MaxReadFileSize
|
maxReadFileSize := cfg.Tools.ReadFile.MaxReadFileSize
|
||||||
switch cfg.Tools.ReadFile.EffectiveMode() {
|
switch cfg.Tools.ReadFile.EffectiveMode() {
|
||||||
case config.ReadFileModeLines:
|
case config.ReadFileModeLines:
|
||||||
toolsRegistry.Register(tools.NewReadFileLinesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths, denyReadPaths))
|
toolsRegistry.Register(tools.NewReadFileLinesTool(
|
||||||
|
workspace, readRestrict, maxReadFileSize, allowReadPaths, denyReadPaths,
|
||||||
|
))
|
||||||
default:
|
default:
|
||||||
toolsRegistry.Register(tools.NewReadFileBytesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths, denyReadPaths))
|
toolsRegistry.Register(tools.NewReadFileBytesTool(workspace, readRestrict, maxReadFileSize, allowReadPaths, denyReadPaths))
|
||||||
}
|
}
|
||||||
|
|
@ -248,7 +250,7 @@ func NewAgentInstance(
|
||||||
|
|
||||||
// resolveAgentWorkspace determines the workspace directory for an agent.
|
// resolveAgentWorkspace determines the workspace directory for an agent.
|
||||||
func resolveAgentWorkspace(agentCfg *config.AgentConfig, defaults *config.AgentDefaults, isolationID string) string {
|
func resolveAgentWorkspace(agentCfg *config.AgentConfig, defaults *config.AgentDefaults, isolationID string) string {
|
||||||
base := ""
|
var base string
|
||||||
if agentCfg != nil && strings.TrimSpace(agentCfg.Workspace) != "" {
|
if agentCfg != nil && strings.TrimSpace(agentCfg.Workspace) != "" {
|
||||||
base = expandHome(strings.TrimSpace(agentCfg.Workspace))
|
base = expandHome(strings.TrimSpace(agentCfg.Workspace))
|
||||||
} else if agentCfg == nil || agentCfg.Default || agentCfg.ID == "" || routing.NormalizeAgentID(agentCfg.ID) == "main" {
|
} else if agentCfg == nil || agentCfg.Default || agentCfg.ID == "" || routing.NormalizeAgentID(agentCfg.ID) == "main" {
|
||||||
|
|
|
||||||
|
|
@ -374,6 +374,7 @@ func TestNewAgentInstance_InvalidExecConfigDoesNotExit(t *testing.T) {
|
||||||
t.Fatal("read_file tool should still be registered")
|
t.Fatal("read_file tool should still be registered")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestNewAgentInstance_IsolatedWorkspace(t *testing.T) {
|
func TestNewAgentInstance_IsolatedWorkspace(t *testing.T) {
|
||||||
tmpDir := t.TempDir()
|
tmpDir := t.TempDir()
|
||||||
cfg := &config.Config{
|
cfg := &config.Config{
|
||||||
|
|
|
||||||
|
|
@ -22,6 +22,7 @@ func (m *isolationMockTool) Description() string { return "mock tool" }
|
||||||
func (m *isolationMockTool) Parameters() map[string]any {
|
func (m *isolationMockTool) Parameters() map[string]any {
|
||||||
return map[string]any{"type": "object", "properties": 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 {
|
func (m *isolationMockTool) Execute(ctx context.Context, args map[string]any) *tools.ToolResult {
|
||||||
return tools.SilentResult("executed")
|
return tools.SilentResult("executed")
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1184,13 +1184,14 @@ func (al *AgentLoop) GetConfig() *config.Config {
|
||||||
return al.cfg
|
return al.cfg
|
||||||
}
|
}
|
||||||
|
|
||||||
// SetMediaStore injects a MediaStore for media lifecycle management.
|
// GetMediaStore returns the currently configured MediaStore.
|
||||||
func (al *AgentLoop) GetMediaStore() media.MediaStore {
|
func (al *AgentLoop) GetMediaStore() media.MediaStore {
|
||||||
al.mu.RLock()
|
al.mu.RLock()
|
||||||
defer al.mu.RUnlock()
|
defer al.mu.RUnlock()
|
||||||
return al.mediaStore
|
return al.mediaStore
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SetMediaStore injects a MediaStore for media lifecycle management.
|
||||||
func (al *AgentLoop) SetMediaStore(s media.MediaStore) {
|
func (al *AgentLoop) SetMediaStore(s media.MediaStore) {
|
||||||
al.mediaStore = s
|
al.mediaStore = s
|
||||||
|
|
||||||
|
|
@ -1640,7 +1641,11 @@ func (al *AgentLoop) getOrCreateIsolatedAgent(agentID, channel, isolationID stri
|
||||||
agent.Tools.SetMediaStore(al.mediaStore)
|
agent.Tools.SetMediaStore(al.mediaStore)
|
||||||
|
|
||||||
// Re-register shared tools (web, message, spawn) to this transient agent
|
// 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
|
// Cache this agent instance per chat session
|
||||||
al.agentCache.Store(cacheKey, agent)
|
al.agentCache.Store(cacheKey, agent)
|
||||||
|
|
|
||||||
|
|
@ -56,7 +56,7 @@ func (r *mcpRuntime) getManager() *mcp.Manager {
|
||||||
return r.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.
|
// agent mode share the same initialization path.
|
||||||
func (al *AgentLoop) EnsureMCPInitialized(ctx context.Context) error {
|
func (al *AgentLoop) EnsureMCPInitialized(ctx context.Context) error {
|
||||||
if !al.cfg.Tools.IsToolEnabled("mcp") {
|
if !al.cfg.Tools.IsToolEnabled("mcp") {
|
||||||
|
|
|
||||||
|
|
@ -842,7 +842,7 @@ type SkillsToolsConfig struct {
|
||||||
ToolConfig ` yaml:"-" envPrefix:"PICOCLAW_TOOLS_SKILLS_"`
|
ToolConfig ` yaml:"-" envPrefix:"PICOCLAW_TOOLS_SKILLS_"`
|
||||||
Registries SkillsRegistriesConfig `yaml:",inline,omitempty" json:"registries"`
|
Registries SkillsRegistriesConfig `yaml:",inline,omitempty" json:"registries"`
|
||||||
Github SkillsGithubConfig `yaml:"github,omitempty" json:"github"`
|
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"`
|
SearchCache SearchCacheConfig `yaml:"-" json:"search_cache"`
|
||||||
Whitelist FlexibleStringSlice `json:"whitelist,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_SKILLS_WHITELIST"`
|
Whitelist FlexibleStringSlice `json:"whitelist,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_SKILLS_WHITELIST"`
|
||||||
WhitelistEnabled bool `json:"whitelist_enabled,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_SKILLS_WHITELIST_ENABLED"`
|
WhitelistEnabled bool `json:"whitelist_enabled,omitempty" yaml:"-" env:"PICOCLAW_TOOLS_SKILLS_WHITELIST_ENABLED"`
|
||||||
|
|
|
||||||
|
|
@ -168,7 +168,8 @@ func Run(debug bool, homePath, configPath string, allowEmptyStartup bool) error
|
||||||
}
|
}
|
||||||
defer pid.RemovePidFile(homePath)
|
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)
|
provider, modelID, err := createStartupProvider(cfg, allowEmptyStartup)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
fmt.Printf("❌ Error creating provider: %v\n", err)
|
fmt.Printf("❌ Error creating provider: %v\n", err)
|
||||||
|
|
|
||||||
|
|
@ -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
|
// 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.
|
// 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("/health", s.healthHandler)
|
||||||
mux.HandleFunc("/ready", s.readyHandler)
|
mux.HandleFunc("/ready", s.readyHandler)
|
||||||
mux.HandleFunc("/reload", s.reloadHandler)
|
mux.HandleFunc("/reload", s.reloadHandler)
|
||||||
|
|
@ -449,7 +444,7 @@ func (s *Server) handlePostChat(w http.ResponseWriter, r *http.Request) {
|
||||||
// Start processing in background
|
// Start processing in background
|
||||||
go func() {
|
go func() {
|
||||||
// Use a long-running context for the chat call, but don't bind to r.Context()
|
// 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()
|
ctx := context.Background()
|
||||||
logger.Debugf("Starting async chat for session %s", sessionID)
|
logger.Debugf("Starting async chat for session %s", sessionID)
|
||||||
reply, err := chatFunc(ctx, req.Message, sessionID, chatID)
|
reply, err := chatFunc(ctx, req.Message, sessionID, chatID)
|
||||||
|
|
|
||||||
|
|
@ -16,7 +16,8 @@ type EditFileTool struct {
|
||||||
}
|
}
|
||||||
|
|
||||||
// NewEditFileTool creates a new EditFileTool with optional directory restriction.
|
// 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
|
var denyPatterns []*regexp.Regexp
|
||||||
if len(denyPaths) > 0 {
|
if len(denyPaths) > 0 {
|
||||||
denyPatterns = denyPaths[0]
|
denyPatterns = denyPaths[0]
|
||||||
|
|
@ -79,7 +80,8 @@ type AppendFileTool struct {
|
||||||
fs fileSystem
|
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
|
var denyPatterns []*regexp.Regexp
|
||||||
if len(denyPaths) > 0 {
|
if len(denyPaths) > 0 {
|
||||||
denyPatterns = denyPaths[0]
|
denyPatterns = denyPaths[0]
|
||||||
|
|
|
||||||
|
|
@ -1283,7 +1283,8 @@ func getSafeRelPath(workspace, path string) (string, error) {
|
||||||
|
|
||||||
// validatePathWithConfigs returns the resolved absolute path if it is allowed
|
// validatePathWithConfigs returns the resolved absolute path if it is allowed
|
||||||
// by the given workspace, restriction setting, and path whitelist/blacklist.
|
// 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)
|
cleaned := filepath.Clean(path)
|
||||||
var resolved string
|
var resolved string
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -449,7 +449,9 @@ func (r *ToolRegistry) Filter(whitelist []string, enabled bool) {
|
||||||
for _, w := range whitelist {
|
for _, w := range whitelist {
|
||||||
// Match exact (redundant but safe) or prefix with underscore
|
// Match exact (redundant but safe) or prefix with underscore
|
||||||
// We also check for "mcp_" prefix specifically to support MCP tool grouping
|
// 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
|
allowed = true
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -791,7 +791,7 @@ func TestToolRegistry_Filter_SupportsPrefix(t *testing.T) {
|
||||||
}
|
}
|
||||||
|
|
||||||
if len(expected) > 0 {
|
if len(expected) > 0 {
|
||||||
var missing []string
|
missing := make([]string, 0, len(expected))
|
||||||
for m := range expected {
|
for m := range expected {
|
||||||
missing = append(missing, m)
|
missing = append(missing, m)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue