fix(agent): avoid process exit on exec init failure and add regression test (#1784)
* fix(agent): make exec tool init failure non-fatal * test(agent): add regression test for invalid exec config fallback
This commit is contained in:
parent
9a25fad20a
commit
844a4eefc7
2 changed files with 46 additions and 8 deletions
|
|
@ -3,13 +3,13 @@ package agent
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
"log"
|
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"regexp"
|
"regexp"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"github.com/sipeed/picoclaw/pkg/config"
|
"github.com/sipeed/picoclaw/pkg/config"
|
||||||
|
"github.com/sipeed/picoclaw/pkg/logger"
|
||||||
"github.com/sipeed/picoclaw/pkg/media"
|
"github.com/sipeed/picoclaw/pkg/media"
|
||||||
"github.com/sipeed/picoclaw/pkg/memory"
|
"github.com/sipeed/picoclaw/pkg/memory"
|
||||||
"github.com/sipeed/picoclaw/pkg/providers"
|
"github.com/sipeed/picoclaw/pkg/providers"
|
||||||
|
|
@ -85,9 +85,11 @@ func NewAgentInstance(
|
||||||
if cfg.Tools.IsToolEnabled("exec") {
|
if cfg.Tools.IsToolEnabled("exec") {
|
||||||
execTool, err := tools.NewExecToolWithConfig(workspace, restrict, cfg, allowReadPaths)
|
execTool, err := tools.NewExecToolWithConfig(workspace, restrict, cfg, allowReadPaths)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
log.Fatalf("Critical error: unable to initialize exec tool: %v", err)
|
logger.ErrorCF("agent", "Failed to initialize exec tool; continuing without exec",
|
||||||
|
map[string]any{"error": err.Error()})
|
||||||
|
} else {
|
||||||
|
toolsRegistry.Register(execTool)
|
||||||
}
|
}
|
||||||
toolsRegistry.Register(execTool)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if cfg.Tools.IsToolEnabled("edit_file") {
|
if cfg.Tools.IsToolEnabled("edit_file") {
|
||||||
|
|
@ -210,8 +212,8 @@ func NewAgentInstance(
|
||||||
})
|
})
|
||||||
lightCandidates = resolved
|
lightCandidates = resolved
|
||||||
} else {
|
} else {
|
||||||
log.Printf("routing: light_model %q not found in model_list — routing disabled for agent %q",
|
logger.WarnCF("agent", "Routing light model not found; routing disabled",
|
||||||
rc.LightModel, agentID)
|
map[string]any{"light_model": rc.LightModel, "agent_id": agentID})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -320,7 +322,8 @@ func (a *AgentInstance) Close() error {
|
||||||
func initSessionStore(dir string) session.SessionStore {
|
func initSessionStore(dir string) session.SessionStore {
|
||||||
store, err := memory.NewJSONLStore(dir)
|
store, err := memory.NewJSONLStore(dir)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
log.Printf("memory: init store: %v; using json sessions", err)
|
logger.WarnCF("agent", "Memory JSONL store init failed; falling back to json sessions",
|
||||||
|
map[string]any{"error": err.Error()})
|
||||||
return session.NewSessionManager(dir)
|
return session.NewSessionManager(dir)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -328,11 +331,12 @@ func initSessionStore(dir string) session.SessionStore {
|
||||||
// Migration failure means the store could not write data.
|
// Migration failure means the store could not write data.
|
||||||
// Fall back to SessionManager to avoid a split state where
|
// Fall back to SessionManager to avoid a split state where
|
||||||
// some sessions are in JSONL and others remain in JSON.
|
// some sessions are in JSONL and others remain in JSON.
|
||||||
log.Printf("memory: migration failed: %v; falling back to json sessions", merr)
|
logger.WarnCF("agent", "Memory migration failed; falling back to json sessions",
|
||||||
|
map[string]any{"error": merr.Error()})
|
||||||
store.Close()
|
store.Close()
|
||||||
return session.NewSessionManager(dir)
|
return session.NewSessionManager(dir)
|
||||||
} else if n > 0 {
|
} else if n > 0 {
|
||||||
log.Printf("memory: migrated %d session(s) to jsonl", n)
|
logger.InfoCF("agent", "Memory migrated to JSONL", map[string]any{"sessions_migrated": n})
|
||||||
}
|
}
|
||||||
|
|
||||||
return session.NewJSONLBackend(store)
|
return session.NewJSONLBackend(store)
|
||||||
|
|
|
||||||
|
|
@ -246,3 +246,37 @@ func TestNewAgentInstance_AllowsMediaTempDirForReadListAndExec(t *testing.T) {
|
||||||
t.Fatalf("exec output missing media content: %s", execResult.ForLLM)
|
t.Fatalf("exec output missing media content: %s", execResult.ForLLM)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestNewAgentInstance_InvalidExecConfigDoesNotExit(t *testing.T) {
|
||||||
|
workspace := t.TempDir()
|
||||||
|
|
||||||
|
cfg := &config.Config{
|
||||||
|
Agents: config.AgentsConfig{
|
||||||
|
Defaults: config.AgentDefaults{
|
||||||
|
Workspace: workspace,
|
||||||
|
ModelName: "test-model",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
Tools: config.ToolsConfig{
|
||||||
|
ReadFile: config.ReadFileToolConfig{Enabled: true},
|
||||||
|
Exec: config.ExecConfig{
|
||||||
|
ToolConfig: config.ToolConfig{Enabled: true},
|
||||||
|
EnableDenyPatterns: true,
|
||||||
|
CustomDenyPatterns: []string{"[invalid-regex"},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, &mockProvider{})
|
||||||
|
if agent == nil {
|
||||||
|
t.Fatal("expected agent instance, got nil")
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, ok := agent.Tools.Get("exec"); ok {
|
||||||
|
t.Fatal("exec tool should not be registered when exec config is invalid")
|
||||||
|
}
|
||||||
|
|
||||||
|
if _, ok := agent.Tools.Get("read_file"); !ok {
|
||||||
|
t.Fatal("read_file tool should still be registered")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue