Merge branch 'fix/harden-tool-call-extraction'
This commit is contained in:
commit
1eca8ae02e
14 changed files with 710 additions and 120 deletions
|
|
@ -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)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -31,6 +31,8 @@ type AgentInstance struct {
|
|||
Subagents *config.SubagentsConfig
|
||||
SkillsFilter []string
|
||||
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)
|
||||
|
|
@ -112,6 +115,8 @@ func NewAgentInstance(
|
|||
Subagents: subagents,
|
||||
SkillsFilter: skillsFilter,
|
||||
Candidates: candidates,
|
||||
SummarizeMessageThreshold: defaults.SummarizeMessageThreshold,
|
||||
SummarizeTokenPercentage: defaults.SummarizeTokenPercentage,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -313,7 +313,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,
|
||||
})
|
||||
|
|
@ -663,7 +663,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 {
|
||||
|
|
@ -680,10 +689,17 @@ 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",
|
||||
|
|
@ -697,6 +713,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
|
||||
}
|
||||
|
||||
|
|
@ -724,9 +765,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() {
|
||||
|
|
|
|||
|
|
@ -177,6 +177,8 @@ type AgentDefaults struct {
|
|||
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"`
|
||||
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 {
|
||||
|
|
@ -478,6 +480,137 @@ type ClawHubRegistryConfig struct {
|
|||
MaxResponseSize int `json:"max_response_size" env:"PICOCLAW_SKILLS_REGISTRIES_CLAWHUB_MAX_RESPONSE_SIZE"`
|
||||
}
|
||||
|
||||
func DefaultConfig() *Config {
|
||||
return &Config{
|
||||
Agents: AgentsConfig{
|
||||
Defaults: AgentDefaults{
|
||||
Workspace: "~/.picoclaw/workspace",
|
||||
RestrictToWorkspace: true,
|
||||
Provider: "",
|
||||
Model: "glm-4.7",
|
||||
MaxTokens: 8192,
|
||||
MaxToolIterations: 20,
|
||||
SummarizeMessageThreshold: 50,
|
||||
SummarizeTokenPercentage: 85,
|
||||
},
|
||||
},
|
||||
Channels: ChannelsConfig{
|
||||
WhatsApp: WhatsAppConfig{
|
||||
Enabled: false,
|
||||
BridgeURL: "ws://localhost:3001",
|
||||
AllowFrom: FlexibleStringSlice{},
|
||||
},
|
||||
Telegram: TelegramConfig{
|
||||
Enabled: false,
|
||||
Token: "",
|
||||
AllowFrom: FlexibleStringSlice{},
|
||||
},
|
||||
Feishu: FeishuConfig{
|
||||
Enabled: false,
|
||||
AppID: "",
|
||||
AppSecret: "",
|
||||
EncryptKey: "",
|
||||
VerificationToken: "",
|
||||
AllowFrom: FlexibleStringSlice{},
|
||||
},
|
||||
Discord: DiscordConfig{
|
||||
Enabled: false,
|
||||
Token: "",
|
||||
AllowFrom: FlexibleStringSlice{},
|
||||
},
|
||||
MaixCam: MaixCamConfig{
|
||||
Enabled: false,
|
||||
Host: "0.0.0.0",
|
||||
Port: 18790,
|
||||
AllowFrom: FlexibleStringSlice{},
|
||||
},
|
||||
QQ: QQConfig{
|
||||
Enabled: false,
|
||||
AppID: "",
|
||||
AppSecret: "",
|
||||
AllowFrom: FlexibleStringSlice{},
|
||||
},
|
||||
DingTalk: DingTalkConfig{
|
||||
Enabled: false,
|
||||
ClientID: "",
|
||||
ClientSecret: "",
|
||||
AllowFrom: FlexibleStringSlice{},
|
||||
},
|
||||
Slack: SlackConfig{
|
||||
Enabled: false,
|
||||
BotToken: "",
|
||||
AppToken: "",
|
||||
AllowFrom: FlexibleStringSlice{},
|
||||
},
|
||||
LINE: LINEConfig{
|
||||
Enabled: false,
|
||||
ChannelSecret: "",
|
||||
ChannelAccessToken: "",
|
||||
WebhookHost: "0.0.0.0",
|
||||
WebhookPort: 18791,
|
||||
WebhookPath: "/webhook/line",
|
||||
AllowFrom: FlexibleStringSlice{},
|
||||
},
|
||||
OneBot: OneBotConfig{
|
||||
Enabled: false,
|
||||
WSUrl: "ws://127.0.0.1:3001",
|
||||
AccessToken: "",
|
||||
ReconnectInterval: 5,
|
||||
GroupTriggerPrefix: []string{},
|
||||
AllowFrom: FlexibleStringSlice{},
|
||||
},
|
||||
},
|
||||
Providers: ProvidersConfig{
|
||||
Anthropic: ProviderConfig{},
|
||||
OpenAI: OpenAIProviderConfig{WebSearch: true},
|
||||
OpenRouter: ProviderConfig{},
|
||||
Groq: ProviderConfig{},
|
||||
Zhipu: ProviderConfig{},
|
||||
VLLM: ProviderConfig{},
|
||||
Gemini: ProviderConfig{},
|
||||
Nvidia: ProviderConfig{},
|
||||
Moonshot: ProviderConfig{},
|
||||
ShengSuanYun: ProviderConfig{},
|
||||
},
|
||||
Gateway: GatewayConfig{
|
||||
Host: "0.0.0.0",
|
||||
Port: 18790,
|
||||
},
|
||||
Tools: ToolsConfig{
|
||||
Web: WebToolsConfig{
|
||||
Brave: BraveConfig{
|
||||
Enabled: false,
|
||||
APIKey: "",
|
||||
MaxResults: 5,
|
||||
},
|
||||
DuckDuckGo: DuckDuckGoConfig{
|
||||
Enabled: true,
|
||||
MaxResults: 5,
|
||||
},
|
||||
Perplexity: PerplexityConfig{
|
||||
Enabled: false,
|
||||
APIKey: "",
|
||||
MaxResults: 5,
|
||||
},
|
||||
},
|
||||
Cron: CronToolsConfig{
|
||||
ExecTimeoutMinutes: 5, // default 5 minutes for LLM operations
|
||||
},
|
||||
Exec: ExecConfig{
|
||||
EnableDenyPatterns: true,
|
||||
},
|
||||
},
|
||||
Heartbeat: HeartbeatConfig{
|
||||
Enabled: true,
|
||||
Interval: 30, // default 30 minutes
|
||||
},
|
||||
Devices: DevicesConfig{
|
||||
Enabled: false,
|
||||
MonitorUSB: true,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func LoadConfig(path string) (*Config, error) {
|
||||
cfg := DefaultConfig()
|
||||
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -969,24 +969,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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -185,10 +185,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:
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -267,3 +277,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 ""
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,22 +5,17 @@ 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
|
||||
}
|
||||
|
||||
end := findMatchingBrace(text, start)
|
||||
if end == start {
|
||||
return nil
|
||||
}
|
||||
|
||||
jsonStr := text[start:end]
|
||||
|
||||
var wrapper struct {
|
||||
ToolCalls []struct {
|
||||
ID string `json:"id"`
|
||||
|
|
@ -32,10 +27,7 @@ func extractToolCallsFromText(text string) []ToolCall {
|
|||
} `json:"tool_calls"`
|
||||
}
|
||||
|
||||
if err := json.Unmarshal([]byte(jsonStr), &wrapper); err != nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
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{}
|
||||
|
|
@ -52,21 +44,73 @@ func extractToolCallsFromText(text string) []ToolCall {
|
|||
},
|
||||
})
|
||||
}
|
||||
|
||||
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 {
|
||||
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
|
||||
}
|
||||
|
||||
var wrapper struct {
|
||||
ToolCalls interface{} `json:"tool_calls"`
|
||||
}
|
||||
|
||||
if err := json.Unmarshal([]byte(jsonStr), &wrapper); err == nil && wrapper.ToolCalls != nil {
|
||||
return strings.TrimSpace(text[:i] + text[end:])
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return text
|
||||
}
|
||||
|
||||
end := findMatchingBrace(text, start)
|
||||
if end == start {
|
||||
return text
|
||||
// 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
|
||||
}
|
||||
|
||||
return strings.TrimSpace(text[:start] + text[end:])
|
||||
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
|
||||
}
|
||||
|
|
|
|||
126
pkg/providers/tool_call_extract_test.go
Normal file
126
pkg/providers/tool_call_extract_test.go
Normal file
|
|
@ -0,0 +1,126 @@
|
|||
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: "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.",
|
||||
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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -136,8 +136,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
|
||||
|
|
|
|||
146
pkg/tools/verify.go
Normal file
146
pkg/tools/verify.go
Normal file
|
|
@ -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 ""
|
||||
}
|
||||
50
pkg/tools/verify_test.go
Normal file
50
pkg/tools/verify_test.go
Normal file
|
|
@ -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")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
|
@ -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
|
||||
- Transparency in every action
|
||||
- Continuous improvement through reflection
|
||||
Loading…
Add table
Reference in a new issue