From 885ee1c8eab3570721ad7fc9a1fc8475306ef3a8 Mon Sep 17 00:00:00 2001 From: Rahul Chand Date: Fri, 20 Feb 2026 01:20:04 +0530 Subject: [PATCH 1/3] fix: harden tool call extraction scanner to handle braces in strings --- pkg/providers/claude_cli_provider.go | 16 ---- pkg/providers/tool_call_extract.go | 39 +++++++++ pkg/providers/tool_call_extract_test.go | 110 ++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 16 deletions(-) create mode 100644 pkg/providers/tool_call_extract_test.go diff --git a/pkg/providers/claude_cli_provider.go b/pkg/providers/claude_cli_provider.go index 58ba3647d..e93cf2c2c 100644 --- a/pkg/providers/claude_cli_provider.go +++ b/pkg/providers/claude_cli_provider.go @@ -181,22 +181,6 @@ func (p *ClaudeCliProvider) stripToolCallsJSON(text string) string { return stripToolCallsFromText(text) } -// findMatchingBrace finds the index after the closing brace matching the opening brace at pos. -func findMatchingBrace(text string, pos int) int { - depth := 0 - for i := pos; i < len(text); i++ { - if text[i] == '{' { - depth++ - } else if text[i] == '}' { - depth-- - if depth == 0 { - return i + 1 - } - } - } - return pos -} - // claudeCliJSONResponse represents the JSON output from the claude CLI. // Matches the real claude CLI v2.x output format. type claudeCliJSONResponse struct { diff --git a/pkg/providers/tool_call_extract.go b/pkg/providers/tool_call_extract.go index 97a219283..298876224 100644 --- a/pkg/providers/tool_call_extract.go +++ b/pkg/providers/tool_call_extract.go @@ -70,3 +70,42 @@ func stripToolCallsFromText(text string) string { return strings.TrimSpace(text[:start] + text[end:]) } + +// findMatchingBrace finds the index after the closing brace matching the opening brace at pos. +// It accounts for braces inside strings and escaped characters. +func findMatchingBrace(text string, pos int) int { + depth := 0 + inString := false + escaped := false + + for i := pos; i < len(text); i++ { + char := text[i] + + if escaped { + escaped = false + continue + } + + if char == '\\' { + escaped = true + continue + } + + if char == '"' { + inString = !inString + continue + } + + if !inString { + if char == '{' { + depth++ + } else if char == '}' { + depth-- + if depth == 0 { + return i + 1 + } + } + } + } + return pos +} diff --git a/pkg/providers/tool_call_extract_test.go b/pkg/providers/tool_call_extract_test.go new file mode 100644 index 000000000..92f973226 --- /dev/null +++ b/pkg/providers/tool_call_extract_test.go @@ -0,0 +1,110 @@ +package providers + +import ( + "reflect" + "testing" +) + +func TestExtractToolCallsFromText(t *testing.T) { + tests := []struct { + name string + text string + want []ToolCall + }{ + { + name: "Basic tool call", + text: `Here is the tool call: {"tool_calls":[{"id":"call_1","type":"function","function":{"name":"search","arguments":"{\"query\":\"hello\"}"}}]} and some more text.`, + want: []ToolCall{ + { + ID: "call_1", + Type: "function", + Name: "search", + Arguments: map[string]interface{}{ + "query": "hello", + }, + Function: &FunctionCall{ + Name: "search", + Arguments: `{"query":"hello"}`, + }, + }, + }, + }, + { + name: "Brace in string", + text: `Tool call with brace in string: {"tool_calls":[{"id":"call_2","type":"function","function":{"name":"msg","arguments":"{\"text\":\"Hello { world }\"}"}}]} post-text.`, + want: []ToolCall{ + { + ID: "call_2", + Type: "function", + Name: "msg", + Arguments: map[string]interface{}{ + "text": "Hello { world }", + }, + Function: &FunctionCall{ + Name: "msg", + Arguments: `{"text":"Hello { world }"}`, + }, + }, + }, + }, + { + name: "Escaped quote and brace in arguments", + text: `Complex: {"tool_calls":[{"id":"call_3","type":"function","function":{"name":"exec","arguments":"{\"cmd\":\"echo \\\"}\\\"\"}"}}]}`, + want: []ToolCall{ + { + ID: "call_3", + Type: "function", + Name: "exec", + Arguments: map[string]interface{}{ + "cmd": `echo "}"`, + }, + Function: &FunctionCall{ + Name: "exec", + Arguments: `{"cmd":"echo \"}\""}`, + }, + }, + }, + }, + { + name: "No tool calls", + text: "Just some normal text here.", + want: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := extractToolCallsFromText(tt.text) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("extractToolCallsFromText() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestStripToolCallsFromText(t *testing.T) { + tests := []struct { + name string + text string + want string + }{ + { + name: "Basic strip", + text: "Prefix text. {\"tool_calls\":[]} Suffix text.", + want: "Prefix text. Suffix text.", + }, + { + name: "No tool calls to strip", + text: "Normal text.", + want: "Normal text.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := stripToolCallsFromText(tt.text); got != tt.want { + t.Errorf("stripToolCallsFromText() = %v, want %v", got, tt.want) + } + }) + } +} From 2c980c33a87b8cabbdb59b13db67edfd46bf7f14 Mon Sep 17 00:00:00 2001 From: Rahul Chand Date: Fri, 20 Feb 2026 02:14:40 +0530 Subject: [PATCH 2/3] feat: Enhance tool call extraction to robustly parse JSON blocks from text, including multiple occurrences. --- pkg/providers/claude_cli_provider_test.go | 21 ---- pkg/providers/tool_call_extract.go | 117 +++++++++++----------- pkg/providers/tool_call_extract_test.go | 16 +++ 3 files changed, 77 insertions(+), 77 deletions(-) diff --git a/pkg/providers/claude_cli_provider_test.go b/pkg/providers/claude_cli_provider_test.go index 063530deb..ba50ebde3 100644 --- a/pkg/providers/claude_cli_provider_test.go +++ b/pkg/providers/claude_cli_provider_test.go @@ -958,24 +958,3 @@ func TestStripToolCallsJSON_OnlyToolCalls(t *testing.T) { // --- findMatchingBrace tests --- -func TestFindMatchingBrace(t *testing.T) { - tests := []struct { - text string - pos int - want int - }{ - {`{"a":1}`, 0, 7}, - {`{"a":{"b":2}}`, 0, 13}, - {`text {"a":1} more`, 5, 12}, - {`{unclosed`, 0, 0}, // no match returns pos - {`{}`, 0, 2}, // empty object - {`{{{}}}`, 0, 6}, // deeply nested - {`{"a":"b{c}d"}`, 0, 13}, // braces in strings (simplified matcher) - } - for _, tt := range tests { - got := findMatchingBrace(tt.text, tt.pos) - if got != tt.want { - t.Errorf("findMatchingBrace(%q, %d) = %d, want %d", tt.text, tt.pos, got, tt.want) - } - } -} diff --git a/pkg/providers/tool_call_extract.go b/pkg/providers/tool_call_extract.go index 298876224..86e7aebd9 100644 --- a/pkg/providers/tool_call_extract.go +++ b/pkg/providers/tool_call_extract.go @@ -5,70 +5,75 @@ import ( "strings" ) -// extractToolCallsFromText parses tool call JSON from response text. -// Both ClaudeCliProvider and CodexCliProvider use this to extract -// tool calls that the model outputs in its response text. func extractToolCallsFromText(text string) []ToolCall { - start := strings.Index(text, `{"tool_calls"`) - if start == -1 { - return nil + for i := 0; i < len(text); i++ { + if text[i] == '{' { + end := findMatchingBrace(text, i) + if end > i { + jsonStr := text[i:end] + // Quick check to avoid expensive parsing if it doesn't mention tool_calls + if !strings.Contains(jsonStr, "tool_calls") { + continue + } + + var wrapper struct { + ToolCalls []struct { + ID string `json:"id"` + Type string `json:"type"` + Function struct { + Name string `json:"name"` + Arguments string `json:"arguments"` + } `json:"function"` + } `json:"tool_calls"` + } + + if err := json.Unmarshal([]byte(jsonStr), &wrapper); err == nil && len(wrapper.ToolCalls) > 0 { + var result []ToolCall + for _, tc := range wrapper.ToolCalls { + var args map[string]interface{} + json.Unmarshal([]byte(tc.Function.Arguments), &args) + + result = append(result, ToolCall{ + ID: tc.ID, + Type: tc.Type, + Name: tc.Function.Name, + Arguments: args, + Function: &FunctionCall{ + Name: tc.Function.Name, + Arguments: tc.Function.Arguments, + }, + }) + } + return result + } + } + } } - - end := findMatchingBrace(text, start) - if end == start { - return nil - } - - jsonStr := text[start:end] - - var wrapper struct { - ToolCalls []struct { - ID string `json:"id"` - Type string `json:"type"` - Function struct { - Name string `json:"name"` - Arguments string `json:"arguments"` - } `json:"function"` - } `json:"tool_calls"` - } - - if err := json.Unmarshal([]byte(jsonStr), &wrapper); err != nil { - return nil - } - - var result []ToolCall - for _, tc := range wrapper.ToolCalls { - var args map[string]interface{} - json.Unmarshal([]byte(tc.Function.Arguments), &args) - - result = append(result, ToolCall{ - ID: tc.ID, - Type: tc.Type, - Name: tc.Function.Name, - Arguments: args, - Function: &FunctionCall{ - Name: tc.Function.Name, - Arguments: tc.Function.Arguments, - }, - }) - } - - return result + return nil } // stripToolCallsFromText removes tool call JSON from response text. func stripToolCallsFromText(text string) string { - start := strings.Index(text, `{"tool_calls"`) - if start == -1 { - return text - } + for i := 0; i < len(text); i++ { + if text[i] == '{' { + end := findMatchingBrace(text, i) + if end > i { + jsonStr := text[i:end] + if !strings.Contains(jsonStr, "tool_calls") { + continue + } - end := findMatchingBrace(text, start) - if end == start { - return text - } + var wrapper struct { + ToolCalls interface{} `json:"tool_calls"` + } - return strings.TrimSpace(text[:start] + text[end:]) + if err := json.Unmarshal([]byte(jsonStr), &wrapper); err == nil && wrapper.ToolCalls != nil { + return strings.TrimSpace(text[:i] + text[end:]) + } + } + } + } + return text } // findMatchingBrace finds the index after the closing brace matching the opening brace at pos. diff --git a/pkg/providers/tool_call_extract_test.go b/pkg/providers/tool_call_extract_test.go index 92f973226..fdd5b957f 100644 --- a/pkg/providers/tool_call_extract_test.go +++ b/pkg/providers/tool_call_extract_test.go @@ -65,6 +65,22 @@ func TestExtractToolCallsFromText(t *testing.T) { }, }, }, + { + name: "Multiple JSON blocks", + text: `Some config: {"debug": true}. Then the tool call: {"tool_calls":[{"id":"c1","type":"function","function":{"name":"search","arguments":"{}"}}]}.`, + want: []ToolCall{ + { + ID: "c1", + Type: "function", + Name: "search", + Arguments: map[string]interface{}{}, + Function: &FunctionCall{ + Name: "search", + Arguments: "{}", + }, + }, + }, + }, { name: "No tool calls", text: "Just some normal text here.", From c473304373b105f7f3967a5087bde3385c2ffe24 Mon Sep 17 00:00:00 2001 From: Rahul Chand Date: Fri, 20 Feb 2026 16:29:43 +0530 Subject: [PATCH 3/3] feat: Introduce `VerifyTool` and 'Plan-Act-Verify' guidance, enhance tool call argument parsing, and refine agent summarization logic. --- pkg/agent/context.go | 11 +- pkg/agent/instance.go | 13 ++- pkg/agent/loop.go | 66 +++++++++-- pkg/config/config.go | 10 +- pkg/providers/openai_compat/provider.go | 57 ++++++++- pkg/tools/shell.go | 14 ++- pkg/tools/toolloop.go | 9 +- pkg/tools/verify.go | 146 ++++++++++++++++++++++++ pkg/tools/verify_test.go | 50 ++++++++ workspace/SOUL.md | 10 +- 10 files changed, 357 insertions(+), 29 deletions(-) create mode 100644 pkg/tools/verify.go create mode 100644 pkg/tools/verify_test.go diff --git a/pkg/agent/context.go b/pkg/agent/context.go index cf5ce2913..ee41da128 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -76,11 +76,16 @@ Your workspace is at: %s ## Important Rules -1. **ALWAYS use tools** - When you need to perform an action (schedule reminders, send messages, execute commands, etc.), you MUST call the appropriate tool. Do NOT just say you'll do it or pretend to do it. +1. **Plan-Act-Verify** - Use a structured approach for all tasks: + - **Plan**: Briefly state what you intend to do before using any tool. + - **Act**: Execute the tool call. + - **Verify**: After seeing the result, explicitly evaluate if the goal was achieved before moving to the next step. -2. **Be helpful and accurate** - When using tools, briefly explain what you're doing. +2. **ALWAYS use tools** - When you need to perform an action (schedule reminders, send messages, execute commands, etc.), you MUST call the appropriate tool. Do NOT just say you'll do it or pretend to do it. -3. **Memory** - When remembering something, write to %s/memory/MEMORY.md`, +3. **Be helpful and accurate** - When using tools, briefly explain what you're doing. + +4. **Memory** - When remembering something, write to %s/memory/MEMORY.md`, now, runtime, workspacePath, workspacePath, workspacePath, workspacePath, toolsSection, workspacePath) } diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index 37b253685..2d534a264 100644 --- a/pkg/agent/instance.go +++ b/pkg/agent/instance.go @@ -30,7 +30,9 @@ type AgentInstance struct { Tools *tools.ToolRegistry Subagents *config.SubagentsConfig SkillsFilter []string - Candidates []providers.FallbackCandidate + Candidates []providers.FallbackCandidate + SummarizeMessageThreshold int + SummarizeTokenPercentage int } // NewAgentInstance creates an agent instance from config. @@ -54,6 +56,7 @@ func NewAgentInstance( toolsRegistry.Register(tools.NewExecToolWithConfig(workspace, restrict, cfg)) toolsRegistry.Register(tools.NewEditFileTool(workspace, restrict)) toolsRegistry.Register(tools.NewAppendFileTool(workspace, restrict)) + toolsRegistry.Register(tools.NewVerifyTool(workspace, restrict)) sessionsDir := filepath.Join(workspace, "sessions") sessionsManager := session.NewSessionManager(sessionsDir) @@ -109,9 +112,11 @@ func NewAgentInstance( Sessions: sessionsManager, ContextBuilder: contextBuilder, Tools: toolsRegistry, - Subagents: subagents, - SkillsFilter: skillsFilter, - Candidates: candidates, + Subagents: subagents, + SkillsFilter: skillsFilter, + Candidates: candidates, + SummarizeMessageThreshold: defaults.SummarizeMessageThreshold, + SummarizeTokenPercentage: defaults.SummarizeTokenPercentage, } } diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 0f1b26c5c..40e685217 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -303,7 +303,7 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) Channel: msg.Channel, ChatID: msg.ChatID, UserMessage: msg.Content, - DefaultResponse: "I've completed processing but have no response to give.", + DefaultResponse: "Task completed, but no final summary was generated.", EnableSummary: true, SendResponse: false, }) @@ -638,7 +638,16 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, agent *AgentInstance, } } - toolResult := agent.Tools.ExecuteWithContext(ctx, tc.Name, tc.Arguments, opts.Channel, opts.ChatID, asyncCallback) + // Check if tool call arguments were malformed (fallback to "raw") + var contentForLLM string + var toolResult *tools.ToolResult + if rawArgs, ok := tc.Arguments["raw"].(string); ok && len(tc.Arguments) == 1 { + errorMsg := fmt.Sprintf("Malformed tool call: The arguments were not valid JSON. Received raw string: %q. Please retry with a valid JSON object matching the tool's schema.", rawArgs) + contentForLLM = fmt.Sprintf("Error: %s\n\nReflection: Your previous tool call failed due to syntax errors. Ensure you are providing a valid JSON object for the arguments, without any trailing tokens or text outside the braces.", errorMsg) + goto addMessage + } + + toolResult = agent.Tools.ExecuteWithContext(ctx, tc.Name, tc.Arguments, opts.Channel, opts.ChatID, asyncCallback) // Send ForUser content to user immediately if not Silent if !toolResult.Silent && toolResult.ForUser != "" && opts.SendResponse { @@ -655,11 +664,18 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, agent *AgentInstance, } // Determine content for LLM based on tool result - contentForLLM := toolResult.ForLLM - if contentForLLM == "" && toolResult.Err != nil { - contentForLLM = toolResult.Err.Error() + contentForLLM = toolResult.ForLLM + if toolResult.Err != nil { + errorMsg := toolResult.Err.Error() + if contentForLLM == "" { + contentForLLM = fmt.Sprintf("Error: %s\n\nReflection: The previous tool call failed. Analyze why it failed and adjust your plan if necessary. If you need to retry with different parameters, do so now.", errorMsg) + } else { + contentForLLM = fmt.Sprintf("%s\n\nError: %s\n\nReflection: The tool execution encountered an issue. Review the output and error above, then decide on the next steps.", contentForLLM, errorMsg) + } } + addMessage: + toolResultMsg := providers.Message{ Role: "tool", Content: contentForLLM, @@ -672,6 +688,31 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, agent *AgentInstance, } } + // Final Summary Nudge: If we finished but have no content to show the user, + // and we actually did some work (iteration > 1), ask for a summary. + if finalContent == "" && iteration > 1 { + logger.InfoCF("agent", "Empty response detected after tool calls, nudging for summary", + map[string]interface{}{"agent_id": agent.ID, "session_key": opts.SessionKey}) + + nudgeMsg := providers.Message{ + Role: "user", + Content: "You have completed the tool calls. Please provide a concise summary of what you did and the final result for the user.", + } + // Don't append to persistent messages, just for this final call + nudgeMessages := append(messages, nudgeMsg) + + // Call LLM one last time without tools + summaryResp, err := agent.Provider.Chat(ctx, nudgeMessages, nil, agent.Model, map[string]interface{}{ + "max_tokens": agent.MaxTokens * 2, // Allow a bit more for summary + "temperature": 0.5, + }) + if err == nil && summaryResp.Content != "" { + finalContent = summaryResp.Content + // Save the nudge response to session so it's in history + agent.Sessions.AddMessage(opts.SessionKey, "assistant", finalContent) + } + } + return finalContent, iteration, nil } @@ -699,9 +740,20 @@ func (al *AgentLoop) updateToolContexts(agent *AgentInstance, channel, chatID st func (al *AgentLoop) maybeSummarize(agent *AgentInstance, sessionKey, channel, chatID string) { newHistory := agent.Sessions.GetHistory(sessionKey) tokenEstimate := al.estimateTokens(newHistory) - threshold := agent.ContextWindow * 75 / 100 - if len(newHistory) > 20 || tokenEstimate > threshold { + // Use configurable thresholds with defaults + tokenPercent := agent.SummarizeTokenPercentage + if tokenPercent == 0 { + tokenPercent = 75 + } + msgThreshold := agent.SummarizeMessageThreshold + if msgThreshold == 0 { + msgThreshold = 20 + } + + threshold := agent.ContextWindow * tokenPercent / 100 + + if len(newHistory) > msgThreshold || tokenEstimate > threshold { summarizeKey := agent.ID + ":" + sessionKey if _, loading := al.summarizing.LoadOrStore(summarizeKey, true); !loading { go func() { diff --git a/pkg/config/config.go b/pkg/config/config.go index 3bdb6f030..b6cd14fad 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -148,7 +148,9 @@ type AgentDefaults struct { ImageModelFallbacks []string `json:"image_model_fallbacks,omitempty"` MaxTokens int `json:"max_tokens" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_TOKENS"` Temperature *float64 `json:"temperature,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_TEMPERATURE"` - MaxToolIterations int `json:"max_tool_iterations" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_TOOL_ITERATIONS"` + MaxToolIterations int `json:"max_tool_iterations" env:"PICOCLAW_AGENTS_DEFAULTS_MAX_TOOL_ITERATIONS"` + SummarizeMessageThreshold int `json:"summarize_message_threshold" env:"PICOCLAW_AGENTS_DEFAULTS_SUMMARIZE_MESSAGE_THRESHOLD"` + SummarizeTokenPercentage int `json:"summarize_token_percentage" env:"PICOCLAW_AGENTS_DEFAULTS_SUMMARIZE_TOKEN_PERCENTAGE"` } type ChannelsConfig struct { @@ -329,8 +331,10 @@ func DefaultConfig() *Config { RestrictToWorkspace: true, Provider: "", Model: "glm-4.7", - MaxTokens: 8192, - MaxToolIterations: 20, + MaxTokens: 8192, + MaxToolIterations: 20, + SummarizeMessageThreshold: 50, + SummarizeTokenPercentage: 85, }, }, Channels: ChannelsConfig{ diff --git a/pkg/providers/openai_compat/provider.go b/pkg/providers/openai_compat/provider.go index 73fac3435..eb5d983a9 100644 --- a/pkg/providers/openai_compat/provider.go +++ b/pkg/providers/openai_compat/provider.go @@ -160,10 +160,20 @@ func parseResponse(body []byte) (*LLMResponse, error) { if tc.Function != nil { name = tc.Function.Name if tc.Function.Arguments != "" { - if err := json.Unmarshal([]byte(tc.Function.Arguments), &arguments); err != nil { + argData := []byte(tc.Function.Arguments) + if err := json.Unmarshal(argData, &arguments); err != nil { + // Attempt to extract the first valid JSON object if it contains junk (e.g. <|call|>) + extracted := extractJSON(tc.Function.Arguments) + if extracted != "" { + if err2 := json.Unmarshal([]byte(extracted), &arguments); err2 == nil { + goto decoded + } + } + log.Printf("openai_compat: failed to decode tool call arguments for %q: %v", name, err) arguments["raw"] = tc.Function.Arguments } + decoded: } } @@ -230,3 +240,48 @@ func asFloat(v interface{}) (float64, bool) { return 0, false } } + +// extractJSON finds the first valid JSON object in a string. +// This is useful when LLMs append junk tokens like <|call|> after the JSON. +func extractJSON(s string) string { + start := strings.Index(s, "{") + if start == -1 { + return "" + } + + depth := 0 + inString := false + escaped := false + + for i := start; i < len(s); i++ { + char := s[i] + + if escaped { + escaped = false + continue + } + + if char == '\\' { + escaped = true + continue + } + + if char == '"' { + inString = !inString + continue + } + + if !inString { + if char == '{' { + depth++ + } else if char == '}' { + depth-- + if depth == 0 { + return s[start : i+1] + } + } + } + } + + return "" +} diff --git a/pkg/tools/shell.go b/pkg/tools/shell.go index d9430672f..7925a2307 100644 --- a/pkg/tools/shell.go +++ b/pkg/tools/shell.go @@ -256,11 +256,17 @@ func (t *ExecTool) guardCommand(command, cwd string) string { return "" } - pathPattern := regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`) - matches := pathPattern.FindAllString(cmd, -1) + // Refined regex to find potential absolute paths while avoiding URLs. + // It matches strings starting with / or [A-Z]:\ that are preceded by space, quote, or start of line. + pathPattern := regexp.MustCompile(`(^|[\s"'])(/[^\s"']+|[A-Za-z]:\\[^"'\s]+)`) + matches := pathPattern.FindAllStringSubmatch(cmd, -1) - for _, raw := range matches { - p, err := filepath.Abs(raw) + for _, match := range matches { + if len(match) < 3 { + continue + } + rawPath := match[2] + p, err := filepath.Abs(rawPath) if err != nil { continue } diff --git a/pkg/tools/toolloop.go b/pkg/tools/toolloop.go index e893217d3..f53eaaf39 100644 --- a/pkg/tools/toolloop.go +++ b/pkg/tools/toolloop.go @@ -130,8 +130,13 @@ func RunToolLoop(ctx context.Context, config ToolLoopConfig, messages []provider // Determine content for LLM contentForLLM := toolResult.ForLLM - if contentForLLM == "" && toolResult.Err != nil { - contentForLLM = toolResult.Err.Error() + if toolResult.Err != nil { + errorMsg := toolResult.Err.Error() + if contentForLLM == "" { + contentForLLM = fmt.Sprintf("Error: %s\n\nReflection: The tool execution failed. Analyze the cause, adjust your approach, and try again if necessary.", errorMsg) + } else { + contentForLLM = fmt.Sprintf("%s\n\nError: %s\n\nReflection: The tool call encountered an issue. Review the output and error, then decide on the next best step.", contentForLLM, errorMsg) + } } // Add tool result message diff --git a/pkg/tools/verify.go b/pkg/tools/verify.go new file mode 100644 index 000000000..ac138f8d1 --- /dev/null +++ b/pkg/tools/verify.go @@ -0,0 +1,146 @@ +package tools + +import ( + "bytes" + "context" + "fmt" + "os/exec" + "path/filepath" + "regexp" + "runtime" + "strings" + "time" +) + +type VerifyTool struct { + workspace string + restrict bool + denyPatterns []*regexp.Regexp +} + +func NewVerifyTool(workspace string, restrict bool) *VerifyTool { + return &VerifyTool{ + workspace: workspace, + restrict: restrict, + denyPatterns: defaultDenyPatterns, // Reusing from shell.go (they are in the same package) + } +} + +func (t *VerifyTool) Name() string { + return "verify" +} + +func (t *VerifyTool) Description() string { + return "Verify the results of your work by running a check command (e.g., 'go test', 'build'). Use this to ensure your changes didn't break anything." +} + +func (t *VerifyTool) Parameters() map[string]interface{} { + return map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "command": map[string]interface{}{ + "type": "string", + "description": "The verification command to run", + }, + "label": map[string]interface{}{ + "type": "string", + "description": "A short label for the verification step (e.g., 'Run unit tests')", + }, + }, + "required": []string{"command"}, + } +} + +func (t *VerifyTool) Execute(ctx context.Context, args map[string]interface{}) *ToolResult { + command, ok := args["command"].(string) + if !ok { + return ErrorResult("command is required") + } + + label, _ := args["label"].(string) + if label == "" { + label = "Verification" + } + + // Safety check (reusing logic from shell.go) + if guardError := t.guardCommand(command, t.workspace); guardError != "" { + return ErrorResult(guardError) + } + + ctx, cancel := context.WithTimeout(ctx, 2*time.Minute) + defer cancel() + + var cmd *exec.Cmd + if runtime.GOOS == "windows" { + cmd = exec.CommandContext(ctx, "powershell", "-NoProfile", "-NonInteractive", "-Command", command) + } else { + cmd = exec.CommandContext(ctx, "sh", "-c", command) + } + cmd.Dir = t.workspace + + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + err := cmd.Run() + outputStr := stdout.String() + if stderr.Len() > 0 { + outputStr += "\nSTDERR:\n" + stderr.String() + } + + if err != nil { + return &ToolResult{ + Err: fmt.Errorf("%s failed: %w", label, err), + ForLLM: fmt.Sprintf("%s FAILED\n\nOutput:\n%s", label, outputStr), + ForUser: fmt.Sprintf("❌ %s failed.\n```\n%s\n```", label, outputStr), + } + } + + return &ToolResult{ + ForLLM: fmt.Sprintf("%s PASSED\n\nOutput:\n%s", label, outputStr), + ForUser: fmt.Sprintf("✅ %s passed successfully.", label), + } +} + +func (t *VerifyTool) guardCommand(command, cwd string) string { + cmdText := strings.TrimSpace(command) + lower := strings.ToLower(cmdText) + + for _, pattern := range t.denyPatterns { + if pattern.MatchString(lower) { + return "Command blocked by safety guard (dangerous pattern detected)" + } + } + + if t.restrict { + if strings.Contains(cmdText, "..\\") || strings.Contains(cmdText, "../") { + return "Command blocked by safety guard (path traversal detected)" + } + + cwdPath, err := filepath.Abs(cwd) + if err != nil { + return "" + } + + pathPattern := regexp.MustCompile(`[A-Za-z]:\\[^\\\"']+|/[^\s\"']+`) + matches := pathPattern.FindAllString(cmdText, -1) + + for _, raw := range matches { + p, err := filepath.Abs(raw) + if err != nil { + continue + } + + rel, err := filepath.Rel(cwdPath, p) + if err != nil { + continue + } + + if strings.HasPrefix(rel, "..") { + return "Command blocked by safety guard (path outside working dir)" + } + } + } + + return "" +} diff --git a/pkg/tools/verify_test.go b/pkg/tools/verify_test.go new file mode 100644 index 000000000..75509e65c --- /dev/null +++ b/pkg/tools/verify_test.go @@ -0,0 +1,50 @@ +package tools + +import ( + "context" + "testing" +) + +func TestVerifyTool(t *testing.T) { + // Create a temp workspace or just use current dir for simple tests + tool := NewVerifyTool(".", false) + + t.Run("SuccessCommand", func(t *testing.T) { + ctx := context.Background() + args := map[string]interface{}{ + "command": "echo 'ok'", + "label": "Check OK", + } + + result := tool.Execute(ctx, args) + if result.IsError { + t.Errorf("Expected success, got error: %v", result.Err) + } + if result.Err != nil { + t.Errorf("Expected nil Err, got: %v", result.Err) + } + }) + + t.Run("FailureCommand", func(t *testing.T) { + ctx := context.Background() + args := map[string]interface{}{ + "command": "exit 1", + "label": "Fail Check", + } + + result := tool.Execute(ctx, args) + if result.Err == nil { + t.Error("Expected error for failing command, got nil") + } + }) + + t.Run("MissingCommand", func(t *testing.T) { + ctx := context.Background() + args := map[string]interface{}{} + + result := tool.Execute(ctx, args) + if !result.IsError { + t.Error("Expected error for missing command") + } + }) +} diff --git a/workspace/SOUL.md b/workspace/SOUL.md index 0be8834f5..a2c6c02ca 100644 --- a/workspace/SOUL.md +++ b/workspace/SOUL.md @@ -5,13 +5,13 @@ I am picoclaw, a lightweight AI assistant powered by AI. ## Personality - Helpful and friendly -- Concise and to the point +- Concise but thorough - Curious and eager to learn -- Honest and transparent +- Honest, transparent, and self-correcting ## Values -- Accuracy over speed +- Accuracy and verification over speed - User privacy and safety -- Transparency in actions -- Continuous improvement \ No newline at end of file +- Transparency in every action +- Continuous improvement through reflection \ No newline at end of file