diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 15294fb99..762856c0f 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -66,16 +66,15 @@ type AgentLoop struct { // processOptions configures how a message is processed type processOptions struct { - SessionKey string // Session identifier for history/context - Channel string // Target channel for tool execution - ChatID string // Target chat ID for tool execution - SenderID string // Originating sender identifier (for logging/audit) - UserMessage string // User message content (may include prefix) - DefaultResponse string // Response when LLM returns empty - EnableSummary bool // Whether to trigger summarization - SendResponse bool // Whether to send response via bus - NoHistory bool // If true, don't load session history (for heartbeat) - Streaming bool // If true, stream token deltas to bus via OnTextDelta + SessionKey string // Session identifier for history/context + Channel string // Target channel for tool execution + ChatID string // Target chat ID for tool execution + SenderID string // Originating sender identifier (for logging/audit) + UserMessage string // User message content (may include prefix) + EnableSummary bool // Whether to trigger summarization + SendResponse bool // Whether to send response via bus + NoHistory bool // If true, don't load session history (for heartbeat) + Streaming bool // If true, stream token deltas to bus via OnTextDelta } // createToolRegistry creates a tool registry with common tools. @@ -242,10 +241,22 @@ func NewAgentLoop(ctx context.Context, cfg *config.Config, msgBus *bus.MessageBu sessionsDir := filepath.Join(workspace, "sessions") sessionsManager := session.NewSessionManager(sessionsDir, session.WithSessionDelegate(memDelegate, "picoclaw")) - // Meta-tools for progressive disclosure (tool_search + tool_call) + // Meta-tools for progressive disclosure (tool_search + tool_call). + // tool_search returns full parameter schemas; discovered tools are + // dynamically promoted to native callables via PrepareStep. toolsRegistry.RegisterMetaTools() - toolsRegistry.MarkGateway("memory") - toolsRegistry.MarkGateway("skill_search") + + // Gateway tools: always visible to the LLM. Keep this set minimal — + // dynamic promotion handles everything else after tool_search. + for _, name := range []string{ + "memory", + "read_file", + "write_file", + "list_dir", + "exec", + } { + toolsRegistry.MarkGateway(name) + } // Wire skills loader into tool_search for unified discovery if ts, ok := toolsRegistry.Get("tool_search"); ok { @@ -437,15 +448,14 @@ func (al *AgentLoop) ProcessDirectStreaming(ctx context.Context, content, sessio } return al.runAgentLoop(ctx, processOptions{ - SessionKey: msg.SessionKey, - Channel: msg.Channel, - ChatID: msg.ChatID, - SenderID: msg.SenderID, - UserMessage: msg.Content, - DefaultResponse: "I've completed processing but have no response to give.", - EnableSummary: true, - SendResponse: false, - Streaming: true, + SessionKey: msg.SessionKey, + Channel: msg.Channel, + ChatID: msg.ChatID, + SenderID: msg.SenderID, + UserMessage: msg.Content, + EnableSummary: true, + SendResponse: false, + Streaming: true, }) } @@ -453,14 +463,13 @@ func (al *AgentLoop) ProcessDirectStreaming(ctx context.Context, content, sessio // Each heartbeat is independent and doesn't accumulate context. func (al *AgentLoop) ProcessHeartbeat(ctx context.Context, content, channel, chatID string) (string, error) { return al.runAgentLoop(ctx, processOptions{ - SessionKey: "heartbeat", - Channel: channel, - ChatID: chatID, - UserMessage: content, - DefaultResponse: "I've completed processing but have no response to give.", - EnableSummary: false, - SendResponse: false, - NoHistory: true, // Don't load session history for heartbeat + SessionKey: "heartbeat", + Channel: channel, + ChatID: chatID, + UserMessage: content, + EnableSummary: false, + SendResponse: false, + NoHistory: true, // Don't load session history for heartbeat }) } @@ -492,13 +501,12 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) // Process as user message return al.runAgentLoop(ctx, processOptions{ - SessionKey: msg.SessionKey, - Channel: msg.Channel, - ChatID: msg.ChatID, - UserMessage: msg.Content, - DefaultResponse: "I've completed processing but have no response to give.", - EnableSummary: true, - SendResponse: false, + SessionKey: msg.SessionKey, + Channel: msg.Channel, + ChatID: msg.ChatID, + UserMessage: msg.Content, + EnableSummary: true, + SendResponse: false, }) } @@ -631,9 +639,56 @@ func (al *AgentLoop) assembleContext(ctx context.Context, opts processOptions) a } adaptedTools := picofantasy.BuildAdaptedTools(al.tools, al.bus, opts.Channel, opts.ChatID, adaptCfg) + // Dynamic tool promotion via PrepareStep: after tool_search discovers tools, + // they become native callables in the next inference step — no tool_call needed. + promotedSet := make(map[string]bool) + for _, at := range adaptedTools { + promotedSet[at.Info().Name] = true + } + registry := al.tools + msgBus := al.bus + channel := opts.Channel + chatID := opts.ChatID + + prepareStep := func(ctx context.Context, psOpts fantasy.PrepareStepFunctionOptions) (context.Context, fantasy.PrepareStepResult, error) { + discovered := registry.DrainDiscovered() + if len(discovered) == 0 { + return ctx, fantasy.PrepareStepResult{}, nil + } + + var newTools []tools.Tool + for _, t := range discovered { + if promotedSet[t.Name()] { + continue + } + newTools = append(newTools, t) + promotedSet[t.Name()] = true + } + + if len(newTools) == 0 { + return ctx, fantasy.PrepareStepResult{}, nil + } + + newAdapted := picofantasy.AdaptTools(newTools, msgBus, channel, chatID, adaptCfg) + expanded := append(adaptedTools, newAdapted...) + adaptedTools = expanded + + logger.InfoCF("agent", "Dynamic tool promotion via PrepareStep", + map[string]interface{}{ + "promoted": len(newTools), + "total_tools": len(expanded), + "names": toolNames(newTools), + }) + + return ctx, fantasy.PrepareStepResult{ + Tools: expanded, + }, nil + } + agentOpts := []fantasy.AgentOption{ fantasy.WithTools(adaptedTools...), fantasy.WithStopConditions(fantasy.StepCountIs(al.maxIterations)), + fantasy.WithPrepareStep(prepareStep), } if systemPrompt != "" { agentOpts = append(agentOpts, fantasy.WithSystemPrompt(systemPrompt)) @@ -668,10 +723,6 @@ func (al *AgentLoop) assembleContext(ctx context.Context, opts processOptions) a // postProcess handles the common finalization after Generate or Stream: // extract final text, save session, summarize, observe, optionally send response. func (al *AgentLoop) postProcess(ctx context.Context, opts processOptions, finalContent string, stepCount int) string { - if finalContent == "" { - finalContent = opts.DefaultResponse - } - al.sessions.Save(opts.SessionKey) if opts.EnableSummary { @@ -700,6 +751,98 @@ func (al *AgentLoop) postProcess(ctx context.Context, opts processOptions, final return finalContent } +// resolveFinalContent normalizes the final assistant response from an agent run. +// Some providers return an empty final response even though an earlier step +// already produced text. In that case, recover the latest non-empty text from +// steps. If no text exists at all, return a deterministic error. +func (al *AgentLoop) resolveFinalContent(finalContent string, steps []fantasy.StepResult) (string, error) { + trimmed := strings.TrimSpace(finalContent) + if trimmed != "" { + return trimmed, nil + } + + for i := len(steps) - 1; i >= 0; i-- { + stepText := strings.TrimSpace(steps[i].Content.Text()) + if stepText != "" { + logger.WarnCF("agent", "Recovered empty final response from prior step text", + map[string]interface{}{ + "step_index": i, + }) + return stepText, nil + } + } + + type candidate struct { + text string + score int + } + candidates := make([]candidate, 0, 8) + for i := len(steps) - 1; i >= 0; i-- { + toolResults := steps[i].Content.ToolResults() + for j := len(toolResults) - 1; j >= 0; j-- { + tr := toolResults[j] + switch out := tr.Result.(type) { + case fantasy.ToolResultOutputContentText: + txt := strings.TrimSpace(out.Text) + if txt != "" { + score := 2 + if tr.ToolName == "tool_search" || strings.Contains(strings.ToLower(txt), "\"kind\":\"tool\"") { + score = 0 + } + if strings.Contains(strings.ToLower(txt), "tool not found") || + strings.Contains(strings.ToLower(txt), "path is required") { + score = -1 + } + candidates = append(candidates, candidate{text: txt, score: score}) + } + case fantasy.ToolResultOutputContentError: + if out.Error != nil { + txt := strings.TrimSpace(out.Error.Error()) + if txt != "" { + candidates = append(candidates, candidate{text: txt, score: -1}) + } + } + case fantasy.ToolResultOutputContentMedia: + txt := strings.TrimSpace(out.Text) + if txt != "" { + candidates = append(candidates, candidate{text: txt, score: 1}) + } + } + if len(candidates) >= 8 { + break + } + } + if len(candidates) >= 8 { + break + } + } + + bestText := "" + bestScore := -1000 + for _, c := range candidates { + if c.score > bestScore { + bestScore = c.score + bestText = c.text + } + } + + if bestText != "" && bestScore > 0 { + logger.WarnCF("agent", "Recovered empty final response from tool results", + map[string]interface{}{ + "candidates": len(candidates), + "score": bestScore, + }) + return bestText, nil + } + + toolCalls := 0 + for _, step := range steps { + toolCalls += len(step.Content.ToolCalls()) + } + + return "", fmt.Errorf("agent produced no final response text (steps=%d, tool_calls=%d)", len(steps), toolCalls) +} + // runAgentLoop is the core message processing logic. // It delegates to assembleContext for shared pre-processing, then branches on // opts.Streaming to either Generate (synchronous) or Stream (real-time deltas). @@ -730,7 +873,15 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, opts processOptions) (str al.auditStep(ctx, step, opts.SessionKey) } - finalContent := result.Response.Content.Text() + finalContent, err := al.resolveFinalContent(result.Response.Content.Text(), result.Steps) + if err != nil { + logger.ErrorCF("agent", "Agent finished without final response text", + map[string]interface{}{ + "error": err.Error(), + "steps": len(result.Steps), + }) + return "", err + } return al.postProcess(ctx, opts, finalContent, len(result.Steps)), nil } @@ -779,7 +930,15 @@ func (al *AgentLoop) runStreaming(ctx context.Context, opts processOptions, ac a return "", fmt.Errorf("agent Stream failed: %w", err) } - finalContent := result.Response.Content.Text() + finalContent, err := al.resolveFinalContent(result.Response.Content.Text(), result.Steps) + if err != nil { + logger.ErrorCF("agent", "Streaming agent finished without final response text", + map[string]interface{}{ + "error": err.Error(), + "steps": len(result.Steps), + }) + return "", err + } return al.postProcess(ctx, opts, finalContent, len(result.Steps)), nil } @@ -1302,3 +1461,11 @@ func (al *AgentLoop) handleCommand(_ context.Context, msg bus.InboundMessage) (s return "", false } + +func toolNames(tt []tools.Tool) []string { + names := make([]string, len(tt)) + for i, t := range tt { + names[i] = t.Name() + } + return names +} diff --git a/pkg/fantasy/adapter.go b/pkg/fantasy/adapter.go index 3e5147b1c..2997cffaa 100644 --- a/pkg/fantasy/adapter.go +++ b/pkg/fantasy/adapter.go @@ -196,6 +196,24 @@ func BuildAdaptedTools(registry *tools.ToolRegistry, msgBus *bus.MessageBus, cha return adapted } +// AdaptTools wraps specific Tool instances as Fantasy AgentTools. +// Used by PrepareStep to promote discovered tools to native callables. +func AdaptTools(picoTools []tools.Tool, msgBus *bus.MessageBus, channel, chatID string, cfg AdaptedToolsConfig) []fantasy.AgentTool { + adapted := make([]fantasy.AgentTool, 0, len(picoTools)) + for _, tool := range picoTools { + adapted = append(adapted, &PicoToolAdapter{ + inner: tool, + bus: msgBus, + channel: channel, + chatID: chatID, + memStore: cfg.MemStore, + agentID: cfg.AgentID, + sessionKey: cfg.SessionKey, + }) + } + return adapted +} + // parseToolArgs deserializes a JSON string into a map. // Handles both JSON objects and empty inputs gracefully. func parseToolArgs(input string) (map[string]interface{}, error) { diff --git a/pkg/tools/registry.go b/pkg/tools/registry.go index d78f5cc0a..6cbf17cb9 100644 --- a/pkg/tools/registry.go +++ b/pkg/tools/registry.go @@ -15,12 +15,18 @@ type ToolRegistry struct { // gatewayTools are always visible to the LLM; all other tools are // discovered via tool_search + tool_call (progressive disclosure). gatewayTools map[string]bool + // discoveredTools tracks tools that tool_search has returned as results + // in the current session. PrepareStep drains this set to dynamically + // promote discovered tools to native callables. + discoveredTools map[string]bool + discoveredMu sync.Mutex } func NewToolRegistry() *ToolRegistry { return &ToolRegistry{ - tools: make(map[string]Tool), - gatewayTools: make(map[string]bool), + tools: make(map[string]Tool), + gatewayTools: make(map[string]bool), + discoveredTools: make(map[string]bool), } } @@ -197,3 +203,60 @@ func (r *ToolRegistry) GetSummaries() []string { } return summaries } + +// MarkDiscovered records that a tool was returned by tool_search. +// Thread-safe; called from tool_search's Execute path. +func (r *ToolRegistry) MarkDiscovered(names ...string) { + r.discoveredMu.Lock() + defer r.discoveredMu.Unlock() + for _, name := range names { + if name == "tool_search" || name == "tool_call" { + continue + } + if r.gatewayTools[name] { + continue + } + r.discoveredTools[name] = true + } +} + +// DrainDiscovered atomically returns and clears the set of tools discovered +// since the last drain. PrepareStep calls this to promote discovered tools +// to native callables for the next inference step. +func (r *ToolRegistry) DrainDiscovered() []Tool { + r.discoveredMu.Lock() + names := make([]string, 0, len(r.discoveredTools)) + for name := range r.discoveredTools { + names = append(names, name) + } + r.discoveredTools = make(map[string]bool) + r.discoveredMu.Unlock() + + r.mu.RLock() + defer r.mu.RUnlock() + promoted := make([]Tool, 0, len(names)) + for _, name := range names { + if tool, ok := r.tools[name]; ok { + promoted = append(promoted, tool) + } + } + return promoted +} + +// IsGateway returns true if the named tool is a gateway tool. +func (r *ToolRegistry) IsGateway(name string) bool { + r.mu.RLock() + defer r.mu.RUnlock() + return r.gatewayTools[name] +} + +// GetSchema returns the full parameter schema for a named tool, or nil if not found. +func (r *ToolRegistry) GetSchema(name string) map[string]interface{} { + r.mu.RLock() + defer r.mu.RUnlock() + tool, ok := r.tools[name] + if !ok { + return nil + } + return tool.Parameters() +} diff --git a/pkg/tools/search.go b/pkg/tools/search.go index ff83f2d6f..7cac93a60 100644 --- a/pkg/tools/search.go +++ b/pkg/tools/search.go @@ -31,7 +31,7 @@ func (t *ToolSearchTool) SetSkillsLoader(sl *skills.SkillsLoader) { func (t *ToolSearchTool) Name() string { return "tool_search" } func (t *ToolSearchTool) Description() string { - return "Search for available tools and skills by keyword. Returns names, descriptions, and kind (tool or skill). Use this to discover capabilities before invoking them with tool_call (tools) or skill_read (skills)." + return "Search for available tools and skills by keyword. Returns names, descriptions, parameter schemas, and kind (tool or skill). Discovered tools become directly callable in your next step — no need to use tool_call. For skills, use skill_read to load full content." } func (t *ToolSearchTool) Parameters() map[string]interface{} { @@ -48,12 +48,14 @@ func (t *ToolSearchTool) Parameters() map[string]interface{} { } type toolSearchResult struct { - Name string `json:"name"` - Description string `json:"description"` - Kind string `json:"kind"` - Score int `json:"score,omitempty"` - Tags []string `json:"tags,omitempty"` - Domain string `json:"domain,omitempty"` + Name string `json:"name"` + Description string `json:"description"` + Kind string `json:"kind"` + Score int `json:"score,omitempty"` + Tags []string `json:"tags,omitempty"` + Domain string `json:"domain,omitempty"` + Parameters map[string]interface{} `json:"parameters,omitempty"` + Required []string `json:"required,omitempty"` } func (t *ToolSearchTool) Execute(_ context.Context, args map[string]interface{}) *ToolResult { @@ -67,7 +69,7 @@ func (t *ToolSearchTool) Execute(_ context.Context, args map[string]interface{}) var results []toolSearchResult - // Search tools + // Search tools — include parameter schemas so the LLM can call them correctly t.registry.mu.RLock() for _, tool := range t.registry.tools { if tool.Name() == "tool_search" || tool.Name() == "tool_call" { @@ -75,11 +77,14 @@ func (t *ToolSearchTool) Execute(_ context.Context, args map[string]interface{}) } score := fuzzyScore(tool.Name(), tool.Description(), queryTerms) if score > 0 { + params, required := extractSchemaFields(tool.Parameters()) results = append(results, toolSearchResult{ Name: tool.Name(), Description: tool.Description(), Kind: "tool", Score: score, + Parameters: params, + Required: required, }) } } @@ -115,10 +120,42 @@ func (t *ToolSearchTool) Execute(_ context.Context, args map[string]interface{}) return &ToolResult{ForLLM: fmt.Sprintf("No tools or skills match query: %q. Try a broader search or use tool_search with no query to list all.", query)} } + // Record discovered tool names so PrepareStep can promote them to native callables. + discoveredNames := make([]string, 0, len(results)) + for _, r := range results { + if r.Kind == "tool" { + discoveredNames = append(discoveredNames, r.Name) + } + } + if len(discoveredNames) > 0 { + t.registry.MarkDiscovered(discoveredNames...) + } + b, _ := jsonv2.Marshal(results) return &ToolResult{ForLLM: string(b)} } +// extractSchemaFields pulls the properties map and required list from a +// tool's full JSON Schema parameters object. +func extractSchemaFields(params map[string]interface{}) (map[string]interface{}, []string) { + props, hasProps := params["properties"].(map[string]interface{}) + if !hasProps { + return nil, nil + } + var required []string + switch r := params["required"].(type) { + case []string: + required = r + case []interface{}: + for _, v := range r { + if s, ok := v.(string); ok { + required = append(required, s) + } + } + } + return props, required +} + func (t *ToolSearchTool) listAll() *ToolResult { var results []toolSearchResult @@ -127,10 +164,13 @@ func (t *ToolSearchTool) listAll() *ToolResult { if tool.Name() == "tool_search" || tool.Name() == "tool_call" { continue } + params, required := extractSchemaFields(tool.Parameters()) results = append(results, toolSearchResult{ Name: tool.Name(), Description: tool.Description(), Kind: "tool", + Parameters: params, + Required: required, }) } t.registry.mu.RUnlock() @@ -151,6 +191,17 @@ func (t *ToolSearchTool) listAll() *ToolResult { return results[i].Name < results[j].Name }) + // Record all tool names for promotion + discoveredNames := make([]string, 0) + for _, r := range results { + if r.Kind == "tool" { + discoveredNames = append(discoveredNames, r.Name) + } + } + if len(discoveredNames) > 0 { + t.registry.MarkDiscovered(discoveredNames...) + } + b, _ := jsonv2.Marshal(results) return &ToolResult{ForLLM: string(b)} } diff --git a/pkg/tools/search_test.go b/pkg/tools/search_test.go index e776699b4..61b750dc3 100644 --- a/pkg/tools/search_test.go +++ b/pkg/tools/search_test.go @@ -397,6 +397,104 @@ func TestToolToSchema_WithoutExamples(t *testing.T) { } } +// --- Discovery tracking tests --- + +func TestToolSearchTool_DiscoveryTracking(t *testing.T) { + r := NewToolRegistry() + r.Register(&stubToolWithSchema{name: "edit_file", desc: "Edit a file"}) + r.Register(&stubToolWithSchema{name: "web_search", desc: "Search the web"}) + + s := NewToolSearchTool(r) + s.Execute(context.Background(), map[string]interface{}{"query": "edit"}) + + discovered := r.DrainDiscovered() + if len(discovered) != 1 { + t.Fatalf("expected 1 discovered tool, got %d", len(discovered)) + } + if discovered[0].Name() != "edit_file" { + t.Errorf("expected edit_file, got %s", discovered[0].Name()) + } + + // Second drain should be empty + discovered2 := r.DrainDiscovered() + if len(discovered2) != 0 { + t.Errorf("expected 0 after drain, got %d", len(discovered2)) + } +} + +func TestToolSearchTool_DiscoverySkipsGateway(t *testing.T) { + r := NewToolRegistry() + r.Register(&stubToolWithSchema{name: "read_file", desc: "Read a file"}) + r.MarkGateway("read_file") + + s := NewToolSearchTool(r) + s.Execute(context.Background(), map[string]interface{}{"query": "read"}) + + discovered := r.DrainDiscovered() + if len(discovered) != 0 { + t.Errorf("gateway tools should not appear in discovered set, got %d", len(discovered)) + } +} + +func TestToolSearchTool_DiscoverySkipsMetaTools(t *testing.T) { + r := NewToolRegistry() + r.RegisterMetaTools() + + s, _ := r.Get("tool_search") + s.Execute(context.Background(), map[string]interface{}{}) + + discovered := r.DrainDiscovered() + for _, d := range discovered { + if d.Name() == "tool_search" || d.Name() == "tool_call" { + t.Errorf("meta-tool %s should not be in discovered set", d.Name()) + } + } +} + +// --- Schema in search results tests --- + +func TestToolSearchTool_ReturnsSchema(t *testing.T) { + r := NewToolRegistry() + r.Register(&stubToolWithSchema{name: "read_file", desc: "Read a file"}) + + s := NewToolSearchTool(r) + result := s.Execute(context.Background(), map[string]interface{}{"query": "read"}) + + var results []toolSearchResult + jsonv2.Unmarshal([]byte(result.ForLLM), &results) + + if len(results) == 0 { + t.Fatal("expected at least one result") + } + if results[0].Parameters == nil { + t.Error("expected parameters in search result") + } + if _, ok := results[0].Parameters["path"]; !ok { + t.Error("expected 'path' in parameters") + } + if len(results[0].Required) == 0 || results[0].Required[0] != "path" { + t.Errorf("expected required=['path'], got %v", results[0].Required) + } +} + +func TestToolSearchTool_ListAll_ReturnsSchema(t *testing.T) { + r := NewToolRegistry() + r.Register(&stubToolWithSchema{name: "read_file", desc: "Read a file"}) + + s := NewToolSearchTool(r) + result := s.Execute(context.Background(), map[string]interface{}{}) + + var results []toolSearchResult + jsonv2.Unmarshal([]byte(result.ForLLM), &results) + + if len(results) == 0 { + t.Fatal("expected at least one result") + } + if results[0].Parameters == nil { + t.Error("expected parameters in listAll result") + } +} + // --- stubTool for testing --- type stubTool struct { name string @@ -410,6 +508,30 @@ func (s *stubTool) Execute(_ context.Context, args map[string]interface{}) *Tool return &ToolResult{ForLLM: "executed " + s.name} } +// stubToolWithSchema returns a realistic JSON Schema parameters object. +type stubToolWithSchema struct { + name string + desc string +} + +func (s *stubToolWithSchema) Name() string { return s.name } +func (s *stubToolWithSchema) Description() string { return s.desc } +func (s *stubToolWithSchema) Parameters() map[string]interface{} { + return map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "path": map[string]interface{}{ + "type": "string", + "description": "Absolute path to the file", + }, + }, + "required": []string{"path"}, + } +} +func (s *stubToolWithSchema) Execute(_ context.Context, args map[string]interface{}) *ToolResult { + return &ToolResult{ForLLM: "executed " + s.name} +} + type stubToolWithExamples struct { stubTool examples []map[string]interface{}