diff --git a/internal/agent/image_gen_gate_test.go b/internal/agent/image_gen_gate_test.go index de0c0938..9c20f9c7 100644 --- a/internal/agent/image_gen_gate_test.go +++ b/internal/agent/image_gen_gate_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/nextlevelbuilder/goclaw/internal/providers" + "github.com/nextlevelbuilder/goclaw/internal/tools" ) // imageCapableProvider is a stub provider that also implements CapabilitiesAware @@ -115,3 +116,61 @@ func TestImageGenGate_FinalIteration_AllToolsStripped(t *testing.T) { t.Errorf("final iteration must strip all tools; got %d: %v", len(defs), defs) } } + +type filteringExecutor struct { + stubExecutor + defs []providers.ToolDefinition +} + +func (e *filteringExecutor) ProviderDefs() []providers.ToolDefinition { + return e.defs +} + +func (e *filteringExecutor) Get(name string) (tools.Tool, bool) { + return nil, false +} + +func TestImageGenGate_FilteringNoPanic(t *testing.T) { + prov := &imageCapableProvider{imageGen: true} + + exec := &filteringExecutor{ + defs: []providers.ToolDefinition{ + { + Type: "function", + Function: &providers.ToolFunctionSchema{ + Name: "read_file", + }, + }, + { + Type: "image_generation", + Function: nil, + }, + }, + } + + l := &Loop{ + provider: prov, + allowImageGeneration: true, + tools: exec, + orchMode: "spawn", // Triggers orchModeDenyTools + disabledTools: map[string]bool{"read_file": true}, // Triggers disabled tools filter + agentType: "open", // Triggers bootstrap filter + skillEvolve: false, // Triggers skill evolve filter + } + + req := &RunRequest{ + ChannelType: "telegram", // Triggers channel filtering + } + + // This should run successfully without panic. + defs, allowed, _ := l.buildFilteredTools(req, true, 1, 10, nil, nil) + + if allowed != nil { + if _, exists := allowed[""]; exists { + t.Error("allowedTools map should not contain empty key for native tools") + } + } + + _ = defs +} + diff --git a/internal/agent/loop_history_toolnames.go b/internal/agent/loop_history_toolnames.go index 10121aa7..f9cdd3f3 100644 --- a/internal/agent/loop_history_toolnames.go +++ b/internal/agent/loop_history_toolnames.go @@ -31,9 +31,11 @@ func (l *Loop) filteredToolNames() []string { return l.tools.List() } defs := l.toolPolicy.FilterTools(l.tools, l.id, l.provider.Name(), l.agentToolPolicy, nil, false, false) - names := make([]string, len(defs)) - for i, d := range defs { - names[i] = d.Function.Name + names := make([]string, 0, len(defs)) + for _, d := range defs { + if d.Function != nil { + names = append(names, d.Function.Name) + } } return names } diff --git a/internal/agent/loop_pipeline_callbacks.go b/internal/agent/loop_pipeline_callbacks.go index c331d1ce..9c36b755 100644 --- a/internal/agent/loop_pipeline_callbacks.go +++ b/internal/agent/loop_pipeline_callbacks.go @@ -239,7 +239,7 @@ func (l *Loop) makeBuildFilteredTools(req *RunRequest) func(state *pipeline.RunS } mcpDefs := 0 for _, td := range toolDefs { - if strings.HasPrefix(strings.TrimSpace(td.Function.Name), "mcp_") { + if td.Function != nil && strings.HasPrefix(strings.TrimSpace(td.Function.Name), "mcp_") { mcpDefs++ } } @@ -508,6 +508,9 @@ func (l *Loop) makeCallLLM(req *RunRequest, emitRun func(AgentEvent)) func(ctx c func shouldRetryTaskMCP(chatReq providers.ChatRequest) bool { hasTaskMCPTool := false for _, td := range chatReq.Tools { + if td.Function == nil { + continue + } name := strings.TrimSpace(td.Function.Name) if strings.HasPrefix(name, "mcp_bx24__") && (strings.Contains(name, "search") || strings.Contains(name, "execute")) { hasTaskMCPTool = true diff --git a/internal/agent/loop_tool_filter.go b/internal/agent/loop_tool_filter.go index 58d1b666..abbb8c99 100644 --- a/internal/agent/loop_tool_filter.go +++ b/internal/agent/loop_tool_filter.go @@ -39,7 +39,9 @@ func (l *Loop) buildFilteredTools(req *RunRequest, hadBootstrap bool, iteration, toolDefs = l.toolPolicy.FilterTools(registry, l.id, l.provider.Name(), l.agentToolPolicy, req.ToolAllow, false, false) allowedTools = make(map[string]bool, len(toolDefs)) for _, td := range toolDefs { - allowedTools[td.Function.Name] = true + if td.Function != nil { + allowedTools[td.Function.Name] = true + } } } else { // No policy → all tools allowed. ProviderDefs() omits per-user MCP tools (not in @@ -67,10 +69,12 @@ func (l *Loop) buildFilteredTools(req *RunRequest, hadBootstrap bool, iteration, if orchDeny := orchModeDenyTools(l.orchMode); len(orchDeny) > 0 { filtered := toolDefs[:0:0] for _, td := range toolDefs { - if !orchDeny[td.Function.Name] { + if td.Function == nil || !orchDeny[td.Function.Name] { filtered = append(filtered, td) } else { - delete(allowedTools, td.Function.Name) + if allowedTools != nil { + delete(allowedTools, td.Function.Name) + } } } toolDefs = filtered @@ -80,10 +84,12 @@ func (l *Loop) buildFilteredTools(req *RunRequest, hadBootstrap bool, iteration, if len(l.disabledTools) > 0 { filtered := toolDefs[:0] for _, td := range toolDefs { - if !l.disabledTools[td.Function.Name] { + if td.Function == nil || !l.disabledTools[td.Function.Name] { filtered = append(filtered, td) } else { - delete(allowedTools, td.Function.Name) + if allowedTools != nil { + delete(allowedTools, td.Function.Name) + } } } toolDefs = filtered @@ -94,7 +100,7 @@ func (l *Loop) buildFilteredTools(req *RunRequest, hadBootstrap bool, iteration, if hadBootstrap && l.agentType != store.AgentTypePredefined { var bootstrapDefs []providers.ToolDefinition for _, td := range toolDefs { - if bootstrapToolAllowlist[td.Function.Name] { + if td.Function != nil && bootstrapToolAllowlist[td.Function.Name] { bootstrapDefs = append(bootstrapDefs, td) } } @@ -106,7 +112,7 @@ func (l *Loop) buildFilteredTools(req *RunRequest, hadBootstrap bool, iteration, if !l.skillEvolve { filtered := toolDefs[:0:0] for _, td := range toolDefs { - if td.Function.Name != "skill_manage" { + if td.Function == nil || td.Function.Name != "skill_manage" { filtered = append(filtered, td) } } @@ -117,10 +123,12 @@ func (l *Loop) buildFilteredTools(req *RunRequest, hadBootstrap bool, iteration, if req.ChannelType != "" { filtered := toolDefs[:0:0] for _, td := range toolDefs { - if tool, ok := l.tools.Get(td.Function.Name); ok { - if ca, ok := tool.(tools.ChannelAware); ok { - if !slices.Contains(ca.RequiredChannelTypes(), req.ChannelType) { - continue + if td.Function != nil { + if tool, ok := l.tools.Get(td.Function.Name); ok { + if ca, ok := tool.(tools.ChannelAware); ok { + if !slices.Contains(ca.RequiredChannelTypes(), req.ChannelType) { + continue + } } } } diff --git a/internal/pipeline/stages_test.go b/internal/pipeline/stages_test.go index 75b90e1c..4360b038 100644 --- a/internal/pipeline/stages_test.go +++ b/internal/pipeline/stages_test.go @@ -3183,3 +3183,51 @@ func TestPruneStage_CacheTtlGate_MarkTouchedOnlyOnMutation(t *testing.T) { t.Error("MarkCacheTouched should NOT be called when prune returns no mutation") } } + +func TestThinkStage_AllowedTools_MixedFunctionAndNativeTools_NoPanic(t *testing.T) { + t.Parallel() + deps := &PipelineDeps{ + Config: PipelineConfig{MaxIterations: 10, MaxTokens: 1000}, + BuildFilteredTools: func(state *RunState) ([]providers.ToolDefinition, error) { + return []providers.ToolDefinition{ + { + Type: "function", + Function: &providers.ToolFunctionSchema{ + Name: "read_file", + }, + }, + { + Type: "image_generation", + Function: nil, + }, + }, nil + }, + CallLLM: func(_ context.Context, _ *RunState, _ providers.ChatRequest) (*providers.ChatResponse, error) { + return &providers.ChatResponse{ + Content: "ok", + FinishReason: "stop", + }, nil + }, + } + stage := NewThinkStage(deps) + state := defaultState() + + err := stage.Execute(context.Background(), state) + if err != nil { + t.Fatalf("Execute() error: %v", err) + } + + if state.Tool.AllowedTools == nil { + t.Fatal("AllowedTools should not be nil") + } + if !state.Tool.AllowedTools["read_file"] { + t.Error("expected read_file in AllowedTools") + } + if state.Tool.AllowedTools[""] { + t.Error("AllowedTools should not contain empty key for native tools") + } + if len(state.Tool.AllowedTools) != 1 { + t.Errorf("AllowedTools len = %d, want 1", len(state.Tool.AllowedTools)) + } +} + diff --git a/internal/pipeline/think_stage.go b/internal/pipeline/think_stage.go index 7530ac5c..addefbb5 100644 --- a/internal/pipeline/think_stage.go +++ b/internal/pipeline/think_stage.go @@ -44,7 +44,9 @@ func (s *ThinkStage) Execute(ctx context.Context, state *RunState) error { } allowed := make(map[string]bool, len(toolDefs)) for _, td := range toolDefs { - allowed[td.Function.Name] = true + if td.Function != nil { + allowed[td.Function.Name] = true + } } state.Tool.AllowedTools = allowed } else { diff --git a/internal/providers/adapter_anthropic_test.go b/internal/providers/adapter_anthropic_test.go index 88d4769c..901b3d5f 100644 --- a/internal/providers/adapter_anthropic_test.go +++ b/internal/providers/adapter_anthropic_test.go @@ -324,3 +324,51 @@ func assertHeader(t *testing.T, h http.Header, key, want string) { t.Errorf("header %q = %q, want %q", key, got, want) } } + +func TestAnthropicAdapterToRequest_NativeToolIgnored(t *testing.T) { + adapter, _ := NewAnthropicAdapter(ProviderConfig{APIKey: "sk-test"}) + + req := ChatRequest{ + Messages: []Message{{Role: "user", Content: "Draw a cat and search"}}, + Tools: []ToolDefinition{ + { + Type: "function", + Function: &ToolFunctionSchema{ + Name: "web_search", + Description: "Search the web", + Parameters: map[string]any{"type": "object"}, + }, + }, + { + Type: "image_generation", + Function: nil, + }, + }, + } + + data, _, err := adapter.ToRequest(req) + if err != nil { + t.Fatal(err) + } + + var body map[string]any + if err := json.Unmarshal(data, &body); err != nil { + t.Fatal(err) + } + + tools, ok := body["tools"].([]any) + if !ok { + t.Fatal("expected tools array in request body") + } + + // Should only serialize function tool, ignoring native tool + if len(tools) != 1 { + t.Fatalf("expected 1 tool in Anthropic request, got %d", len(tools)) + } + + tool := tools[0].(map[string]any) + if tool["name"] != "web_search" { + t.Errorf("expected tool name 'web_search', got %v", tool["name"]) + } +} + diff --git a/internal/providers/anthropic_request.go b/internal/providers/anthropic_request.go index a27bfb0c..237ff301 100644 --- a/internal/providers/anthropic_request.go +++ b/internal/providers/anthropic_request.go @@ -195,6 +195,9 @@ func (p *AnthropicProvider) buildRequestBody(model string, req ChatRequest, stre if len(req.Tools) > 0 { var tools []map[string]any for _, t := range req.Tools { + if t.Type != "function" || t.Function == nil { + continue + } cleanedParams := CleanSchemaForProvider("anthropic", t.Function.Parameters) tool := map[string]any{ "name": t.Function.Name, diff --git a/internal/providers/codex_build.go b/internal/providers/codex_build.go index 802f3ecc..4d9d4304 100644 --- a/internal/providers/codex_build.go +++ b/internal/providers/codex_build.go @@ -122,7 +122,7 @@ func (p *CodexProvider) buildRequestBody(req ChatRequest, stream bool) map[strin "output_format": "png", "partial_images": 1, }) - } else { + } else if t.Function != nil { // Function tool path (default). Works with both value-type and pointer Function // fields — we only access t.Function when type is not "image_generation". tools = append(tools, map[string]any{ diff --git a/internal/providers/codex_test.go b/internal/providers/codex_test.go index b59a5ad9..415b2b92 100644 --- a/internal/providers/codex_test.go +++ b/internal/providers/codex_test.go @@ -1083,3 +1083,43 @@ func TestCodexProviderBuildRequestBodyWithImages(t *testing.T) { t.Errorf("content[1] type = %v, want input_text", content[1]["type"]) } } + +func TestCodexBuildRequestBody_NilFunction_HandlesGracefully(t *testing.T) { + p := NewCodexProvider("test", &staticTokenSource{token: "test"}, "", "gpt-4o") + + req := ChatRequest{ + Messages: []Message{{Role: "user", Content: "Draw and search"}}, + Tools: []ToolDefinition{ + { + Type: "some_unsupported_native_tool", + Function: nil, + }, + { + Type: "function", + Function: &ToolFunctionSchema{ + Name: "web_search", + Description: "Search the web", + Parameters: map[string]any{"type": "object"}, + }, + }, + }, + } + + body := p.buildRequestBody(req, false) + + tools, ok := body["tools"].([]map[string]any) + if !ok { + t.Fatalf("tools is not []map[string]any: %T", body["tools"]) + } + + // Should only serialize the function tool and ignore the native tool with nil Function + if len(tools) != 1 { + t.Fatalf("expected 1 tool in Codex request, got %d", len(tools)) + } + + tool := tools[0] + if tool["type"] != "function" || tool["name"] != "web_search" { + t.Errorf("expected function tool 'web_search', got: %v", tool) + } +} +