Fix and enhance as comment from best friend

config/config.example.json

@@ -3,7 +3,7 @@
     "defaults": {
       "workspace": "~/.picoclaw/workspace",
       "restrict_to_workspace": true,
-      "model": "",
+      "model": "glm-4.7",
       "max_tokens": 8192,
       "temperature": 0.7,
       "max_tool_iterations": 20

pkg/config/config.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"sync"
 
 	"github.com/caarlos0/env/v11"
@@ -304,7 +305,7 @@ type CronToolsConfig struct {
 
 type ExecConfig struct {
 	EnableDenyPatterns bool     `json:"enable_deny_patterns" env:"PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS"`
-	CustomDenyPatterns []string `json:"custom_deny_patterns,omitempty"`
+	CustomDenyPatterns []string `json:"custom_deny_patterns,omitempty" env:"PICOCLAW_TOOLS_EXEC_CUSTOM_DENY_PATTERNS"`
 }
 
 type ToolsConfig struct {
@@ -411,12 +412,12 @@ func DefaultConfig() *Config {
 		Tools: ToolsConfig{
 			Web: WebToolsConfig{
 				Search: WebSearchConfig{
-					Provider:   "ollama",
+					Provider:   "",
 					APIKey:     "",
-					Endpoint:   "https://ollama.com/api/web_search",
+					Endpoint:   "",
-					RestType:   "POST",
+					RestType:   "",
-					QueryParam: "query",
+					QueryParam: "",
-					MaxResults: 5,
+					MaxResults: 0,
 				},
 			},
 			Cron: CronToolsConfig{
@@ -457,6 +458,10 @@ func LoadConfig(path string) (*Config, error) {
 		return nil, err
 	}
 
+	if strings.TrimSpace(cfg.Tools.Web.Search.Provider) == "" {
+		return nil, fmt.Errorf("Please check new config for web search as config example")
+	}
+
 	return cfg, nil
 }
 

pkg/config/config_test.go

@@ -284,19 +284,24 @@ func TestDefaultConfig_Channels(t *testing.T) {
 	}
 }
 
-// TestDefaultConfig_WebTools verifies web tools config
+// TestDefaultConfig_WebTools_EmptyDefaults verifies web search defaults are intentionally empty.
-func TestDefaultConfig_WebTools(t *testing.T) {
+func TestDefaultConfig_WebTools_EmptyDefaults(t *testing.T) {
 	cfg := DefaultConfig()
 
-	// Verify web tools defaults
+	if cfg.Tools.Web.Search.MaxResults != 0 {
-	if cfg.Tools.Web.Search.MaxResults != 5 {
+		t.Error("Expected Search MaxResults 0, got ", cfg.Tools.Web.Search.MaxResults)
-		t.Error("Expected Search MaxResults 5, got ", cfg.Tools.Web.Search.MaxResults)
 	}
-	if cfg.Tools.Web.Search.Provider == "" {
+	if cfg.Tools.Web.Search.Provider != "" {
-		t.Error("Search provider should not be empty by default")
+		t.Error("Search provider should be empty by default")
 	}
-	if cfg.Tools.Web.Search.QueryParam == "" {
+	if cfg.Tools.Web.Search.Endpoint != "" {
-		t.Error("Search query param should not be empty by default")
+		t.Error("Search endpoint should be empty by default")
+	}
+	if cfg.Tools.Web.Search.RestType != "" {
+		t.Error("Search rest_type should be empty by default")
+	}
+	if cfg.Tools.Web.Search.QueryParam != "" {
+		t.Error("Search query param should be empty by default")
 	}
 }
 
@@ -364,7 +369,7 @@ func TestDefaultConfig_OpenAIWebSearchEnabled(t *testing.T) {
 func TestLoadConfig_OpenAIWebSearchDefaultsTrueWhenUnset(t *testing.T) {
 	dir := t.TempDir()
 	configPath := filepath.Join(dir, "config.json")
-	if err := os.WriteFile(configPath, []byte(`{"providers":{"openai":{"api_base":""}}}`), 0o600); err != nil {
+	if err := os.WriteFile(configPath, []byte(`{"providers":{"openai":{"api_base":""}},"tools":{"web":{"search":{"provider":"brave"}}}}`), 0o600); err != nil {
 		t.Fatalf("WriteFile() error: %v", err)
 	}
 
@@ -380,7 +385,7 @@ func TestLoadConfig_OpenAIWebSearchDefaultsTrueWhenUnset(t *testing.T) {
 func TestLoadConfig_OpenAIWebSearchCanBeDisabled(t *testing.T) {
 	dir := t.TempDir()
 	configPath := filepath.Join(dir, "config.json")
-	if err := os.WriteFile(configPath, []byte(`{"providers":{"openai":{"web_search":false}}}`), 0o600); err != nil {
+	if err := os.WriteFile(configPath, []byte(`{"providers":{"openai":{"web_search":false}},"tools":{"web":{"search":{"provider":"brave"}}}}`), 0o600); err != nil {
 		t.Fatalf("WriteFile() error: %v", err)
 	}
 
@@ -392,3 +397,19 @@ func TestLoadConfig_OpenAIWebSearchCanBeDisabled(t *testing.T) {
 		t.Fatal("OpenAI codex web search should be false when disabled in config file")
 	}
 }
+
+func TestLoadConfig_WebSearchProviderRequired(t *testing.T) {
+	dir := t.TempDir()
+	configPath := filepath.Join(dir, "config.json")
+	if err := os.WriteFile(configPath, []byte(`{"tools":{"web":{"search":{"provider":""}}}}`), 0o600); err != nil {
+		t.Fatalf("WriteFile() error: %v", err)
+	}
+
+	_, err := LoadConfig(configPath)
+	if err == nil {
+		t.Fatal("expected error when web search provider is not set")
+	}
+	if err.Error() != "Please check new config for web search as config example" {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}

pkg/migrate/migrate_test.go

@@ -649,6 +649,13 @@ func TestRunFullMigration(t *testing.T) {
 				"apiKey": "sk-or-migrate-test",
 			},
 		},
+		"tools": map[string]interface{}{
+			"web": map[string]interface{}{
+				"search": map[string]interface{}{
+					"provider": "brave",
+				},
+			},
+		},
 		"channels": map[string]interface{}{
 			"telegram": map[string]interface{}{
 				"enabled": true,

pkg/providers/tool_call_extract_test.go

@@ -0,0 +1,41 @@
+package providers
+
+import "testing"
+
+func TestExtractToolCallsFromText_WebAPIFormat(t *testing.T) {
+	text := `I'll search for USD/EUR.
+/WebAPI
+{"name": "web_search", "arguments": {"query": "current USD to EUR exchange rate"}}
+</tool_call>`
+
+	calls := extractToolCallsFromText(text)
+	if len(calls) != 1 {
+		t.Fatalf("len(calls) = %d, want 1", len(calls))
+	}
+	if calls[0].Name != "web_search" {
+		t.Fatalf("calls[0].Name = %q, want %q", calls[0].Name, "web_search")
+	}
+	if calls[0].Arguments["query"] != "current USD to EUR exchange rate" {
+		t.Fatalf("query arg mismatch: %+v", calls[0].Arguments)
+	}
+
+	stripped := stripToolCallsFromText(text)
+	if stripped == text {
+		t.Fatalf("expected stripped text to remove webapi tool call block")
+	}
+}
+
+func TestExtractToolCallsFromText_JSONWrapperFormat(t *testing.T) {
+	text := `before {"tool_calls":[{"id":"call_1","type":"function","function":{"name":"web_search","arguments":"{\"query\":\"btc price\"}"}}]} after`
+
+	calls := extractToolCallsFromText(text)
+	if len(calls) != 1 {
+		t.Fatalf("len(calls) = %d, want 1", len(calls))
+	}
+	if calls[0].Name != "web_search" {
+		t.Fatalf("calls[0].Name = %q, want %q", calls[0].Name, "web_search")
+	}
+	if calls[0].Arguments["query"] != "btc price" {
+		t.Fatalf("query arg mismatch: %+v", calls[0].Arguments)
+	}
+}

pkg/tools/web.go

@@ -191,7 +191,10 @@ func (p *OllamaSearchProvider) Search(ctx context.Context, query string, count i
 		"count":      count,
 	}
 
-	bodyJSON, _ := json.Marshal(requestBody)
+	bodyJSON, err := json.Marshal(requestBody)
+	if err != nil {
+		return "", fmt.Errorf("failed to encode Ollama web search request: %w", err)
+	}
 
 	req, err := http.NewRequestWithContext(ctx, "POST", p.baseURL, strings.NewReader(string(bodyJSON)))
 	if err != nil {
@@ -337,8 +340,8 @@ func NewWebSearchTool(opts ...WebSearchToolOptions) *WebSearchTool {
 			},
 		}
 	}
-	// Priority order: Brave > Ollama > DuckDuckGo
+	// Priority order: Brave > Ollama > Perplexity > DuckDuckGo
-	priorityOrder := []string{"brave", "ollama", "duckduckgo"}
+	priorityOrder := []string{"brave", "ollama", "perplexity", "duckduckgo"}
 	optMap := make(map[string]WebSearchToolOptions)
 
 	// Build map of enabled providers
@@ -350,6 +353,7 @@ func NewWebSearchTool(opts ...WebSearchToolOptions) *WebSearchTool {
 
 	var selectedOpt *WebSearchToolOptions
 	var provider SearchProvider
+	var providerErrors []string
 
 	// Try providers in priority order
 	for _, providerName := range priorityOrder {
@@ -361,12 +365,26 @@ func NewWebSearchTool(opts ...WebSearchToolOptions) *WebSearchTool {
 		selectedOpt = &opt
 		var err error
 		provider, err = createProvider(&opt)
+		if err != nil {
+			providerErrors = append(providerErrors, fmt.Sprintf("%s: %v", providerName, err))
+			logger.WarnCF("tool", "Web search provider initialization failed",
+				map[string]interface{}{
+					"provider": providerName,
+					"error":    err.Error(),
+				})
+		}
 		if err == nil && provider != nil {
 			break
 		}
 	}
 
 	if provider == nil {
+		if len(providerErrors) > 0 {
+			logger.ErrorCF("tool", "No usable web search provider after trying configured providers",
+				map[string]interface{}{
+					"errors": strings.Join(providerErrors, "; "),
+				})
+		}
 		return nil
 	}
 
@@ -375,6 +393,12 @@ func NewWebSearchTool(opts ...WebSearchToolOptions) *WebSearchTool {
 		if ddgOpt, exists := optMap["duckduckgo"]; exists {
 			if p, err := createProvider(&ddgOpt); err == nil && p != nil {
 				fallbackProvider = p
+			} else if err != nil {
+				logger.WarnCF("tool", "Fallback web search provider initialization failed",
+					map[string]interface{}{
+						"provider": "duckduckgo",
+						"error":    err.Error(),
+					})
 			}
 		}
 	}
@@ -536,6 +560,38 @@ func (t *WebSearchTool) Execute(ctx context.Context, args map[string]interface{}
 				ForUser: strings.Join(summary, "\n"),
 			}
 		}
+
+		preview := result
+		if len(preview) > 512 {
+			preview = preview[:512] + "..."
+		}
+
+		var raw map[string]interface{}
+		if err := json.Unmarshal([]byte(result), &parsed); err != nil {
+			logger.WarnCF("tool", "Failed to parse Ollama web search response",
+				map[string]interface{}{
+					"provider": "ollama",
+					"query":    query,
+					"error":    err.Error(),
+					"preview":  preview,
+				})
+		} else if len(parsed.Results) == 0 {
+			logger.WarnCF("tool", "Ollama web search response has empty results",
+				map[string]interface{}{
+					"provider": "ollama",
+					"query":    query,
+					"preview":  preview,
+				})
+		} else if err := json.Unmarshal([]byte(result), &raw); err == nil {
+			if _, hasResults := raw["results"]; !hasResults {
+				logger.WarnCF("tool", "Ollama web search response missing expected 'results' field",
+					map[string]interface{}{
+						"provider": "ollama",
+						"query":    query,
+						"preview":  preview,
+					})
+			}
+		}
 	}
 
 	// Default: return as-is

pkg/tools/web_test.go

@@ -3,12 +3,29 @@ package tools
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
 )
 
+type failingSearchProvider struct {
+	err error
+}
+
+func (p *failingSearchProvider) Search(ctx context.Context, query string, count int) (string, error) {
+	return "", p.err
+}
+
+type staticSearchProvider struct {
+	result string
+}
+
+func (p *staticSearchProvider) Search(ctx context.Context, query string, count int) (string, error) {
+	return p.result, nil
+}
+
 // TestWebTool_WebFetch_Success verifies successful URL fetching
 func TestWebTool_WebFetch_Success(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -181,10 +198,24 @@ func TestWebTool_WebSearch_NoApiKey(t *testing.T) {
 		t.Errorf("Expected nil tool when Brave API key is empty")
 	}
 
-	// No providers should return nil
+	// Perplexity without API key should return nil
-	tool = NewWebSearchTool()
+	tool = NewWebSearchTool(WebSearchToolOptions{Provider: "perplexity", APIKey: ""})
 	if tool != nil {
-		t.Errorf("Expected nil tool when no providers are provided")
+		t.Errorf("Expected nil tool when Perplexity API key is empty")
+	}
+
+	// No providers should default to DuckDuckGo
+	tool = NewWebSearchTool()
+	if tool == nil {
+		t.Errorf("Expected non-nil tool when no providers are provided (DuckDuckGo default)")
+	}
+}
+
+// TestWebTool_WebSearch_PerplexityProvider verifies Perplexity provider can be selected
+func TestWebTool_WebSearch_PerplexityProvider(t *testing.T) {
+	tool := NewWebSearchTool(WebSearchToolOptions{Provider: "perplexity", APIKey: "test-key", MaxResults: 3})
+	if tool == nil {
+		t.Fatal("Expected non-nil tool for Perplexity provider")
 	}
 }
 
@@ -202,6 +233,101 @@ func TestWebTool_WebSearch_MissingQuery(t *testing.T) {
 	}
 }
 
+// TestWebTool_WebSearch_FallbackOnPrimaryError verifies fallback to duckduckgo when primary provider fails.
+func TestWebTool_WebSearch_FallbackOnPrimaryError(t *testing.T) {
+	tool := &WebSearchTool{
+		provider:   &failingSearchProvider{err: fmt.Errorf("Ollama API error: {\"error\": \"unauthorized\"}")},
+		fallback:   &staticSearchProvider{result: "Results for: test query (via DuckDuckGo)\n1. Example\n   https://example.com"},
+		maxResults: 5,
+	}
+
+	ctx := context.Background()
+	result := tool.Execute(ctx, map[string]interface{}{"query": "test query"})
+
+	if result.IsError {
+		t.Fatalf("Expected successful fallback result, got error: %s", result.ForLLM)
+	}
+
+	if !strings.Contains(result.ForUser, "DuckDuckGo") {
+		t.Errorf("Expected ForUser to contain fallback provider result, got: %s", result.ForUser)
+	}
+
+	if !strings.Contains(result.ForLLM, "Primary search provider failed") {
+		t.Errorf("Expected ForLLM to mention primary provider failure, got: %s", result.ForLLM)
+	}
+}
+
+func TestWebTool_WebSearch_OllamaSummaryFormatting(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPost {
+			t.Fatalf("method = %s, want %s", r.Method, http.MethodPost)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		w.Write([]byte(`{
+			"results": [
+				{"title":"Result One","url":"https://example.com/1","content":"Snippet one"},
+				{"title":"Result Two","url":"https://example.com/2","content":"Snippet two"}
+			]
+		}`))
+	}))
+	defer server.Close()
+
+	tool := NewWebSearchTool(WebSearchToolOptions{
+		Provider:   "ollama",
+		BaseURL:    server.URL,
+		Param:      "query",
+		MaxResults: 5,
+	})
+	if tool == nil {
+		t.Fatal("Expected non-nil tool for Ollama provider")
+	}
+
+	result := tool.Execute(context.Background(), map[string]interface{}{"query": "golang"})
+	if result.IsError {
+		t.Fatalf("Expected successful result, got error: %s", result.ForLLM)
+	}
+
+	if !strings.Contains(result.ForUser, "Web search summary for: golang") {
+		t.Errorf("Expected summary header in ForUser, got: %s", result.ForUser)
+	}
+	if !strings.Contains(result.ForUser, "1. Result One") || !strings.Contains(result.ForUser, "https://example.com/1") {
+		t.Errorf("Expected first formatted result in ForUser, got: %s", result.ForUser)
+	}
+	if !strings.Contains(result.ForUser, "Snippet one") {
+		t.Errorf("Expected snippet content in ForUser, got: %s", result.ForUser)
+	}
+	if !strings.Contains(result.ForLLM, `"results"`) {
+		t.Errorf("Expected raw JSON in ForLLM, got: %s", result.ForLLM)
+	}
+}
+
+func TestWebTool_WebSearch_OllamaNon2xxReturnsError(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusUnauthorized)
+		w.Write([]byte(`{"error":"unauthorized"}`))
+	}))
+	defer server.Close()
+
+	tool := NewWebSearchTool(WebSearchToolOptions{
+		Provider:   "ollama",
+		BaseURL:    server.URL,
+		Param:      "query",
+		MaxResults: 5,
+	})
+	if tool == nil {
+		t.Fatal("Expected non-nil tool for Ollama provider")
+	}
+
+	result := tool.Execute(context.Background(), map[string]interface{}{"query": "golang"})
+	if !result.IsError {
+		t.Fatalf("Expected error result on non-2xx response, got: %s", result.ForLLM)
+	}
+	if !strings.Contains(result.ForLLM, "Ollama API error") {
+		t.Errorf("Expected Ollama API error in ForLLM, got: %s", result.ForLLM)
+	}
+}
+
 // TestWebTool_WebFetch_HTMLExtraction verifies HTML text extraction
 func TestWebTool_WebFetch_HTMLExtraction(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {