From 844a4eefc7ae1c26bb12490495c736d5bf3b8550 Mon Sep 17 00:00:00 2001
From: SakoroYou <165740095+Sakurapainting@users.noreply.github.com>
Date: Thu, 19 Mar 2026 21:11:36 +0800
Subject: [PATCH] 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
---
 pkg/agent/instance.go      | 20 ++++++++++++--------
 pkg/agent/instance_test.go | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go
index 1c3635322..d2a4f81a4 100644
--- a/pkg/agent/instance.go
+++ b/pkg/agent/instance.go
@@ -3,13 +3,13 @@ package agent
 import (
 	"context"
 	"fmt"
-	"log"
 	"os"
 	"path/filepath"
 	"regexp"
 	"strings"
 
 	"github.com/sipeed/picoclaw/pkg/config"
+	"github.com/sipeed/picoclaw/pkg/logger"
 	"github.com/sipeed/picoclaw/pkg/media"
 	"github.com/sipeed/picoclaw/pkg/memory"
 	"github.com/sipeed/picoclaw/pkg/providers"
@@ -85,9 +85,11 @@ func NewAgentInstance(
 	if cfg.Tools.IsToolEnabled("exec") {
 		execTool, err := tools.NewExecToolWithConfig(workspace, restrict, cfg, allowReadPaths)
 		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") {
@@ -210,8 +212,8 @@ func NewAgentInstance(
 			})
 			lightCandidates = resolved
 		} else {
-			log.Printf("routing: light_model %q not found in model_list — routing disabled for agent %q",
-				rc.LightModel, agentID)
+			logger.WarnCF("agent", "Routing light model not found; routing disabled",
+				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 {
 	store, err := memory.NewJSONLStore(dir)
 	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)
 	}
 
@@ -328,11 +331,12 @@ func initSessionStore(dir string) session.SessionStore {
 		// Migration failure means the store could not write data.
 		// Fall back to SessionManager to avoid a split state where
 		// 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()
 		return session.NewSessionManager(dir)
 	} 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)
diff --git a/pkg/agent/instance_test.go b/pkg/agent/instance_test.go
index 5a13c8f1b..b3318ad1f 100644
--- a/pkg/agent/instance_test.go
+++ b/pkg/agent/instance_test.go
@@ -246,3 +246,37 @@ func TestNewAgentInstance_AllowsMediaTempDirForReadListAndExec(t *testing.T) {
 		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")
+	}
+}