From 194781c3e93e3f5f1d680e2ac7811dda6943164a Mon Sep 17 00:00:00 2001 From: Anton Bogdanovich <27antonb@gmail.com> Date: Tue, 5 May 2026 17:13:39 -0700 Subject: [PATCH] feat(agents): add working summary tool feedback --- config/config.example.json | 3 +- docs/operations/debug.md | 41 +++++- pkg/agent/agent_test.go | 39 ++---- pkg/agent/hooks_test.go | 6 +- pkg/agent/pipeline_execute.go | 6 +- pkg/channels/telegram/telegram_test.go | 14 ++ pkg/channels/tool_feedback_animator.go | 63 ++++++++- pkg/channels/tool_feedback_animator_test.go | 56 +++++++- pkg/config/config.go | 11 +- pkg/config/config_test.go | 26 ++++ pkg/utils/tool_feedback.go | 145 ++++++++++++++++++++ pkg/utils/tool_feedback_test.go | 79 ++++++++++- 12 files changed, 443 insertions(+), 46 deletions(-) diff --git a/config/config.example.json b/config/config.example.json index 910c4fbd3..5d0ff3683 100644 --- a/config/config.example.json +++ b/config/config.example.json @@ -16,7 +16,8 @@ "tool_feedback": { "enabled": false, "max_args_length": 300, - "separate_messages": false + "separate_messages": false, + "style": "raw" } } }, diff --git a/docs/operations/debug.md b/docs/operations/debug.md index eacd72380..aeeb38c33 100644 --- a/docs/operations/debug.md +++ b/docs/operations/debug.md @@ -66,7 +66,8 @@ Debug logs are server-side only. If you want the agent to send a visible notific "tool_feedback": { "enabled": true, "max_args_length": 300, - "separate_messages": true + "separate_messages": true, + "style": "raw" } } } @@ -80,6 +81,39 @@ When `enabled` is `true`, every tool call sends a short message to the chat befo {"query": "picoclaw release notes"} ``` +Set `style` to `working_summary` to use a compact, non-argument progress message that can be edited in place as tools run: + +```json +{ + "agents": { + "defaults": { + "tool_feedback": { + "enabled": true, + "separate_messages": false, + "style": "working_summary" + } + } + } +} +``` + +The message starts as: + +```text +Working... +• tool: `read_file` — `README.md` +``` + +When more tools run, editable channels merge recent tool lines into the same progress message: + +```text +Working... +• tool: `read_file` — `README.md` +• tool: `exec` — `test.sh` +• tool: `write_file` — `config.json` +``` + +The `working_summary` style intentionally does not show raw tool arguments, explanations, URLs, command arguments, environment variables, or full paths. It only shows the tool name plus a safe basename for file tools and executable commands where available. This keeps progress visible without exposing secrets in chat history. ### Options @@ -88,14 +122,17 @@ When `enabled` is `true`, every tool call sends a short message to the chat befo | `enabled` | bool | `false` | Send a chat notification for each tool call | | `separate_messages` | bool | `false` | Keep every tool feedback update as a separate chat message instead of reusing a single placeholder/progress message | | `max_args_length` | int | `300` | Maximum characters of the serialised arguments included in the notification | +| `style` | string | `raw` | Feedback format. Use `raw` for the original tool/explanation/argument preview, or `working_summary` for compact progress lines without raw arguments | ### Environment variables -Both fields can also be set via environment variables: +These fields can also be set via environment variables: ```bash PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_ENABLED=true PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_MAX_ARGS_LENGTH=300 +PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_SEPARATE_MESSAGES=false +PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_STYLE=working_summary ``` > **Note:** `tool_feedback` is independent of `--debug` mode. It works in production and does not require the gateway to be started with any special flag. diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go index a75919912..1be0dc988 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -3973,6 +3973,7 @@ func TestProcessHeartbeat_DoesNotPublishToolFeedback(t *testing.T) { ToolFeedback: config.ToolFeedbackConfig{ Enabled: true, MaxArgsLength: 300, + Style: utils.ToolFeedbackStyleWorkingSummary, }, }, }, @@ -4019,6 +4020,7 @@ func TestProcessMessage_PublishesToolFeedbackWhenEnabled(t *testing.T) { ToolFeedback: config.ToolFeedbackConfig{ Enabled: true, MaxArgsLength: 300, + Style: utils.ToolFeedbackStyleWorkingSummary, }, }, }, @@ -4048,7 +4050,6 @@ func TestProcessMessage_PublishesToolFeedbackWhenEnabled(t *testing.T) { select { case outbound := <-msgBus.OutboundChan(): - escapedHeartbeatFile := strings.ReplaceAll(heartbeatFile, `\`, `\\`) if outbound.Channel != "telegram" { t.Fatalf("tool feedback channel = %q, want %q", outbound.Channel, "telegram") } @@ -4058,20 +4059,13 @@ func TestProcessMessage_PublishesToolFeedbackWhenEnabled(t *testing.T) { if outbound.Context.Channel != "telegram" || outbound.Context.ChatID != "chat-1" { t.Fatalf("unexpected tool feedback context: %+v", outbound.Context) } - if !strings.Contains(outbound.Content, "`read_file`") { + if !strings.Contains(outbound.Content, "tool: `read_file`") { t.Fatalf("tool feedback content = %q, want read_file summary", outbound.Content) } - if !strings.Contains(outbound.Content, utils.ToolFeedbackContinuationHint) { - t.Fatalf("tool feedback content = %q, want continuation hint fallback", outbound.Content) - } - if !strings.Contains(outbound.Content, "check tool feedback") { - t.Fatalf("tool feedback content = %q, want current user intent fallback", outbound.Content) - } - if !strings.Contains(outbound.Content, "\"path\":") { - t.Fatalf("tool feedback content = %q, want serialized tool arguments", outbound.Content) - } - if !strings.Contains(outbound.Content, escapedHeartbeatFile) { - t.Fatalf("tool feedback content = %q, want tool argument value", outbound.Content) + if strings.Contains(outbound.Content, utils.ToolFeedbackContinuationHint) || + strings.Contains(outbound.Content, "check tool feedback") || + strings.Contains(outbound.Content, "\"path\":") { + t.Fatalf("tool feedback content = %q, should only include compact tool names", outbound.Content) } if strings.Contains(outbound.Content, "Previous turn explanation") { t.Fatalf("tool feedback content = %q, want no previous assistant fallback", outbound.Content) @@ -4283,6 +4277,7 @@ func TestProcessMessage_DoesNotLeakReasoningContentInToolFeedback(t *testing.T) ToolFeedback: config.ToolFeedbackConfig{ Enabled: true, MaxArgsLength: 300, + Style: utils.ToolFeedbackStyleWorkingSummary, }, }, }, @@ -4313,20 +4308,14 @@ func TestProcessMessage_DoesNotLeakReasoningContentInToolFeedback(t *testing.T) select { case outbound := <-msgBus.OutboundChan(): escapedHeartbeatFile := strings.ReplaceAll(heartbeatFile, `\`, `\\`) - if !strings.Contains(outbound.Content, "`read_file`") { + if !strings.Contains(outbound.Content, "tool: `read_file`") { t.Fatalf("tool feedback content = %q, want read_file summary", outbound.Content) } - if !strings.Contains(outbound.Content, utils.ToolFeedbackContinuationHint) { - t.Fatalf("tool feedback content = %q, want continuation hint fallback", outbound.Content) - } - if !strings.Contains(outbound.Content, "check reasoning fallback") { - t.Fatalf("tool feedback content = %q, want current user intent fallback", outbound.Content) - } - if !strings.Contains(outbound.Content, "\"path\":") { - t.Fatalf("tool feedback content = %q, want serialized tool arguments", outbound.Content) - } - if !strings.Contains(outbound.Content, escapedHeartbeatFile) { - t.Fatalf("tool feedback content = %q, want tool argument value", outbound.Content) + if strings.Contains(outbound.Content, utils.ToolFeedbackContinuationHint) || + strings.Contains(outbound.Content, "check reasoning fallback") || + strings.Contains(outbound.Content, "\"path\":") || + strings.Contains(outbound.Content, escapedHeartbeatFile) { + t.Fatalf("tool feedback content = %q, should only include compact tool names", outbound.Content) } if strings.Contains(outbound.Content, "Read README.md first") { t.Fatalf("tool feedback content = %q, should not leak hidden reasoning", outbound.Content) diff --git a/pkg/agent/hooks_test.go b/pkg/agent/hooks_test.go index 4deef38c7..d0e4809e8 100644 --- a/pkg/agent/hooks_test.go +++ b/pkg/agent/hooks_test.go @@ -17,6 +17,7 @@ import ( "github.com/sipeed/picoclaw/pkg/routing" "github.com/sipeed/picoclaw/pkg/session" "github.com/sipeed/picoclaw/pkg/tools" + "github.com/sipeed/picoclaw/pkg/utils" ) func newHookTestLoop( @@ -809,6 +810,7 @@ func TestAgentLoop_Hooks_ToolFeedbackUsesRewrittenToolName(t *testing.T) { defer cleanup() al.cfg.Agents.Defaults.ToolFeedback.Enabled = true + al.cfg.Agents.Defaults.ToolFeedback.Style = utils.ToolFeedbackStyleWorkingSummary al.RegisterTool(&echoTextTool{}) al.RegisterTool(&echoTextRewrittenTool{}) if err := al.MountHook(NamedHook("tool-rename", &toolRenameHook{})); err != nil { @@ -835,10 +837,10 @@ func TestAgentLoop_Hooks_ToolFeedbackUsesRewrittenToolName(t *testing.T) { select { case outbound := <-msgBus.OutboundChan(): - if !strings.Contains(outbound.Content, "`echo_text_rewritten`") { + if !strings.Contains(outbound.Content, "tool: `echo_text_rewritten`") { t.Fatalf("tool feedback content = %q, want rewritten tool name", outbound.Content) } - if strings.Contains(outbound.Content, "`echo_text`") { + if strings.Contains(outbound.Content, "tool: `echo_text`\n") { t.Fatalf("tool feedback content = %q, want no original tool name", outbound.Content) } case <-time.After(2 * time.Second): diff --git a/pkg/agent/pipeline_execute.go b/pkg/agent/pipeline_execute.go index 0f71c7432..f8edb8954 100644 --- a/pkg/agent/pipeline_execute.go +++ b/pkg/agent/pipeline_execute.go @@ -88,7 +88,8 @@ toolLoop: tc, messages, ) - feedbackMsg := utils.FormatToolFeedbackMessage( + feedbackMsg := utils.FormatToolFeedbackMessageWithStyle( + al.cfg.Agents.Defaults.GetToolFeedbackStyle(), toolName, toolFeedbackExplanation, toolFeedbackArgsPreview(toolArgs, toolFeedbackMaxLen), @@ -369,7 +370,8 @@ toolLoop: tc, messages, ) - feedbackMsg := utils.FormatToolFeedbackMessage( + feedbackMsg := utils.FormatToolFeedbackMessageWithStyle( + al.cfg.Agents.Defaults.GetToolFeedbackStyle(), toolName, toolFeedbackExplanation, toolFeedbackArgsPreview(toolArgs, toolFeedbackMaxLen), diff --git a/pkg/channels/telegram/telegram_test.go b/pkg/channels/telegram/telegram_test.go index 69c76b430..92c714ced 100644 --- a/pkg/channels/telegram/telegram_test.go +++ b/pkg/channels/telegram/telegram_test.go @@ -441,6 +441,20 @@ func TestFitToolFeedbackForTelegram_ReservesAnimationFrame(t *testing.T) { } } +func TestParseContent_WorkingSummaryToolNamesStayCode(t *testing.T) { + content := "Working...\n• tool: `inventorydb__get_location`" + + htmlContent := parseContent(content, false) + if !strings.Contains(htmlContent, "inventorydb__get_location") { + t.Fatalf("parseContent() HTML = %q, want tool name code span", htmlContent) + } + + markdownV2Content := parseContent(content, true) + if !strings.Contains(markdownV2Content, "`inventorydb__get_location`") { + t.Fatalf("parseContent() MarkdownV2 = %q, want tool name code span", markdownV2Content) + } +} + func TestSend_LongMessage_SingleCall(t *testing.T) { // With WithMaxMessageLength(4000), the Manager pre-splits messages before // they reach Send(). A message at exactly 4000 chars should go through diff --git a/pkg/channels/tool_feedback_animator.go b/pkg/channels/tool_feedback_animator.go index b424612bf..1703a622c 100644 --- a/pkg/channels/tool_feedback_animator.go +++ b/pkg/channels/tool_feedback_animator.go @@ -8,6 +8,8 @@ import ( ) const toolFeedbackAnimationInterval = 3 * time.Second +const workingSummaryToolFeedbackAnimationInterval = 10 * time.Second +const maxMergedToolFeedbackLines = 8 const initialToolFeedbackAnimationFrame = "" @@ -122,13 +124,17 @@ func (a *ToolFeedbackAnimator) Update(ctx context.Context, chatID, content strin return "", false, nil } - animatedContent := InitialAnimatedToolFeedbackContent(content) + mergedContent := content + if isWorkingSummaryToolFeedback(baseContent) || isWorkingSummaryToolFeedback(content) { + mergedContent = mergeToolFeedbackContent(baseContent, content) + } + animatedContent := InitialAnimatedToolFeedbackContent(mergedContent) if err := a.editFn(ctx, strings.TrimSpace(chatID), msgID, animatedContent); err != nil { a.Record(chatID, msgID, baseContent) return "", true, err } - a.Record(chatID, msgID, content) + a.Record(chatID, msgID, mergedContent) return msgID, true, nil } @@ -163,7 +169,7 @@ func (a *ToolFeedbackAnimator) detach(chatID string) *toolFeedbackAnimationState func (a *ToolFeedbackAnimator) run(chatID string, entry *toolFeedbackAnimationState) { defer close(entry.done) - ticker := time.NewTicker(toolFeedbackAnimationInterval) + ticker := time.NewTicker(toolFeedbackAnimationIntervalFor(entry.baseContent)) defer ticker.Stop() frameIdx := 1 @@ -227,6 +233,57 @@ func appendToolFeedbackFrame(firstLine, frame string) string { return firstLine + frame } +func mergeToolFeedbackContent(previous, next string) string { + previous = strings.TrimSpace(previous) + next = strings.TrimSpace(next) + if previous == "" { + return next + } + if next == "" { + return previous + } + + lines := make([]string, 0, maxMergedToolFeedbackLines+1) + seen := make(map[string]struct{}) + addLine := func(line string) { + line = strings.TrimSpace(line) + if line == "" || strings.EqualFold(line, "Working...") { + return + } + if _, ok := seen[line]; ok { + return + } + seen[line] = struct{}{} + lines = append(lines, line) + } + + for _, line := range strings.Split(previous, "\n") { + addLine(line) + } + for _, line := range strings.Split(next, "\n") { + addLine(line) + } + if len(lines) > maxMergedToolFeedbackLines { + lines = lines[len(lines)-maxMergedToolFeedbackLines:] + } + if len(lines) == 0 { + return "Working..." + } + return "Working...\n" + strings.Join(lines, "\n") +} + +func isWorkingSummaryToolFeedback(content string) bool { + firstLine, _, _ := strings.Cut(strings.TrimSpace(content), "\n") + return strings.EqualFold(strings.TrimSpace(firstLine), "Working...") +} + +func toolFeedbackAnimationIntervalFor(content string) time.Duration { + if isWorkingSummaryToolFeedback(content) { + return workingSummaryToolFeedbackAnimationInterval + } + return toolFeedbackAnimationInterval +} + func stopToolFeedbackAnimation(entry *toolFeedbackAnimationState) { if entry == nil { return diff --git a/pkg/channels/tool_feedback_animator_test.go b/pkg/channels/tool_feedback_animator_test.go index a23284548..bade38123 100644 --- a/pkg/channels/tool_feedback_animator_test.go +++ b/pkg/channels/tool_feedback_animator_test.go @@ -75,16 +75,17 @@ func TestToolFeedbackAnimator_UpdateStopsTrackingBeforeEdit(t *testing.T) { if messageID != "msg-1" { t.Fatalf("messageID = %q, want msg-1", messageID) } - if content != "🔧 `write_file`\nUpdating config" { + want := "Working...\n• tool: `read_file`\n• tool: `write_file`" + if content != want { t.Fatalf("content = %q, want updated animated content", content) } return nil }) defer animator.StopAll() - animator.Record("chat-1", "msg-1", "🔧 `read_file`\nChecking config") + animator.Record("chat-1", "msg-1", "Working...\n• tool: `read_file`") - msgID, handled, err := animator.Update(context.Background(), "chat-1", "🔧 `write_file`\nUpdating config") + msgID, handled, err := animator.Update(context.Background(), "chat-1", "Working...\n• tool: `write_file`") if err != nil { t.Fatalf("Update() error = %v", err) } @@ -96,6 +97,51 @@ func TestToolFeedbackAnimator_UpdateStopsTrackingBeforeEdit(t *testing.T) { } } +func TestToolFeedbackAnimator_UpdateRawFeedbackReplacesContent(t *testing.T) { + var animator *ToolFeedbackAnimator + animator = NewToolFeedbackAnimator(func(_ context.Context, chatID, messageID, content string) error { + if _, ok := animator.Current(chatID); ok { + t.Fatal("expected tracked tool feedback to be stopped before edit") + } + if messageID != "msg-1" { + t.Fatalf("messageID = %q, want msg-1", messageID) + } + want := "🔧 `write_file`\nWriting config" + if content != want { + t.Fatalf("content = %q, want replacement content", content) + } + return nil + }) + defer animator.StopAll() + + animator.Record("chat-1", "msg-1", "🔧 `read_file`\nReading config") + + msgID, handled, err := animator.Update(context.Background(), "chat-1", "🔧 `write_file`\nWriting config") + if err != nil { + t.Fatalf("Update() error = %v", err) + } + if !handled { + t.Fatal("Update() handled = false, want true") + } + if msgID != "msg-1" { + t.Fatalf("Update() msgID = %q, want msg-1", msgID) + } +} + +func TestToolFeedbackAnimationIntervalForWorkingSummary(t *testing.T) { + got := toolFeedbackAnimationIntervalFor("Working...\n• tool: `read_file`") + if got != workingSummaryToolFeedbackAnimationInterval { + t.Fatalf("toolFeedbackAnimationIntervalFor() = %v, want %v", got, workingSummaryToolFeedbackAnimationInterval) + } +} + +func TestToolFeedbackAnimationIntervalForRawFeedback(t *testing.T) { + got := toolFeedbackAnimationIntervalFor("🔧 `read_file`\nReading config") + if got != toolFeedbackAnimationInterval { + t.Fatalf("toolFeedbackAnimationIntervalFor() = %v, want %v", got, toolFeedbackAnimationInterval) + } +} + func TestToolFeedbackAnimator_UpdateFailureRestoresTracking(t *testing.T) { editErr := errors.New("edit failed") animator := NewToolFeedbackAnimator(func(context.Context, string, string, string) error { @@ -103,9 +149,9 @@ func TestToolFeedbackAnimator_UpdateFailureRestoresTracking(t *testing.T) { }) defer animator.StopAll() - animator.Record("chat-1", "msg-1", "🔧 `read_file`\nChecking config") + animator.Record("chat-1", "msg-1", "Working...\n• tool: `read_file`") - msgID, handled, err := animator.Update(context.Background(), "chat-1", "🔧 `write_file`\nUpdating config") + msgID, handled, err := animator.Update(context.Background(), "chat-1", "Working...\n• tool: `write_file`") if !handled { t.Fatal("Update() handled = false, want true") } diff --git a/pkg/config/config.go b/pkg/config/config.go index acceee4d5..463c77a29 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -249,9 +249,10 @@ type SubTurnConfig struct { } type ToolFeedbackConfig struct { - Enabled bool `json:"enabled" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_ENABLED"` - MaxArgsLength int `json:"max_args_length" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_MAX_ARGS_LENGTH"` - SeparateMessages bool `json:"separate_messages" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_SEPARATE_MESSAGES"` + Enabled bool `json:"enabled" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_ENABLED"` + MaxArgsLength int `json:"max_args_length" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_MAX_ARGS_LENGTH"` + SeparateMessages bool `json:"separate_messages" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_SEPARATE_MESSAGES"` + Style string `json:"style,omitempty" env:"PICOCLAW_AGENTS_DEFAULTS_TOOL_FEEDBACK_STYLE"` } type AgentDefaults struct { @@ -311,6 +312,10 @@ func (d *AgentDefaults) IsToolFeedbackSeparateMessagesEnabled() bool { return d.ToolFeedback.SeparateMessages } +func (d *AgentDefaults) GetToolFeedbackStyle() string { + return strings.TrimSpace(d.ToolFeedback.Style) +} + // GetModelName returns the effective model name for the agent defaults. // It prefers the new "model_name" field but falls back to "model" for backward compatibility. func (d *AgentDefaults) GetModelName() string { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 4f1c5c5e8..b0e952488 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -790,6 +790,9 @@ func TestDefaultConfig_ToolFeedbackDisabled(t *testing.T) { if cfg.Agents.Defaults.ToolFeedback.SeparateMessages { t.Fatal("DefaultConfig().Agents.Defaults.ToolFeedback.SeparateMessages should be false") } + if got := cfg.Agents.Defaults.GetToolFeedbackStyle(); got != "" { + t.Fatalf("DefaultConfig().Agents.Defaults.GetToolFeedbackStyle() = %q, want empty/raw default", got) + } } func TestLoadConfig_ToolFeedbackDefaultsFalseWhenUnset(t *testing.T) { @@ -813,6 +816,29 @@ func TestLoadConfig_ToolFeedbackDefaultsFalseWhenUnset(t *testing.T) { if cfg.Agents.Defaults.ToolFeedback.SeparateMessages { t.Fatal("agents.defaults.tool_feedback.separate_messages should remain false when unset in config file") } + if got := cfg.Agents.Defaults.GetToolFeedbackStyle(); got != "" { + t.Fatalf("agents.defaults.tool_feedback.style = %q, want empty/raw default when unset", got) + } +} + +func TestLoadConfig_ToolFeedbackStyle(t *testing.T) { + dir := t.TempDir() + configPath := filepath.Join(dir, "config.json") + if err := os.WriteFile( + configPath, + []byte(`{"version":1,"agents":{"defaults":{"tool_feedback":{"enabled":true,"style":"working_summary"}}}}`), + 0o600, + ); err != nil { + t.Fatalf("WriteFile() error: %v", err) + } + + cfg, err := LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error: %v", err) + } + if got := cfg.Agents.Defaults.GetToolFeedbackStyle(); got != "working_summary" { + t.Fatalf("agents.defaults.tool_feedback.style = %q, want working_summary", got) + } } func TestLoadConfig_WebPreferNativeDefaultsTrueWhenUnset(t *testing.T) { diff --git a/pkg/utils/tool_feedback.go b/pkg/utils/tool_feedback.go index 1834d7f78..c2597fb91 100644 --- a/pkg/utils/tool_feedback.go +++ b/pkg/utils/tool_feedback.go @@ -4,6 +4,8 @@ import ( "bytes" "encoding/json" "fmt" + "path/filepath" + "regexp" "strings" ) @@ -30,6 +32,8 @@ func FormatArgsJSON(args map[string]any, prettyPrint, disableEscapeHTML bool) st return strings.TrimSpace(buf.String()) } +const ToolFeedbackStyleWorkingSummary = "working_summary" + // FormatToolFeedbackMessage renders a tool feedback message for chat channels. // It keeps the tool name on the first line for animation and can include both // a human explanation and the serialized tool arguments in the body. @@ -57,6 +61,147 @@ func FormatToolFeedbackMessage(toolName, explanation, argsPreview string) string return fmt.Sprintf("\U0001f527 `%s`\n%s", toolName, body) } +// FormatToolFeedbackMessageWithStyle renders alternate tool feedback styles. +// The working_summary style intentionally omits explanations and raw arguments: +// progress UI should not leak internal paths, large JSON blobs, secrets, or +// model-drafted reasoning-like text. +func FormatToolFeedbackMessageWithStyle(style, toolName, explanation, argsPreview string) string { + if strings.EqualFold(strings.TrimSpace(style), ToolFeedbackStyleWorkingSummary) { + return FormatWorkingSummaryToolFeedbackMessage(toolName, argsPreview) + } + return FormatToolFeedbackMessage(toolName, explanation, argsPreview) +} + +func FormatWorkingSummaryToolFeedbackMessage(toolName, argsPreview string) string { + toolName = strings.TrimSpace(toolName) + if toolName == "" { + return "Working..." + } + line := fmt.Sprintf("• tool: `%s`", sanitizeToolFeedbackCodeSpan(toolName)) + if summary := summarizeToolFeedbackArgs(toolName, argsPreview); summary != "" { + line += fmt.Sprintf(" — `%s`", sanitizeToolFeedbackCodeSpan(summary)) + } + return "Working...\n" + line +} + +func sanitizeToolFeedbackCodeSpan(text string) string { + return strings.ReplaceAll(text, "`", "'") +} + +func summarizeToolFeedbackArgs(toolName, argsPreview string) string { + argsPreview = strings.TrimSpace(argsPreview) + if argsPreview == "" { + return "" + } + + var args map[string]any + if err := json.Unmarshal([]byte(argsPreview), &args); err != nil { + return "" + } + + normalizedToolName := strings.ToLower(strings.TrimSpace(toolName)) + if strings.Contains(normalizedToolName, "exec") { + if command := firstStringArg(args, "command"); command != "" { + return truncateToolFeedbackSummary(summarizeExecCommand(command)) + } + return "" + } + if isFileToolFeedbackTool(normalizedToolName) { + if summary := firstStringArg(args, "path", "file_path", "filepath"); summary != "" { + return truncateToolFeedbackSummary(filepath.Base(summary)) + } + } + return "" +} + +func isFileToolFeedbackTool(toolName string) bool { + return strings.Contains(toolName, "read_file") || + strings.Contains(toolName, "write_file") || + strings.Contains(toolName, "edit_file") || + strings.Contains(toolName, "append_file") || + strings.Contains(toolName, "list_dir") +} + +func firstStringArg(args map[string]any, keys ...string) string { + for _, key := range keys { + if value, ok := args[key]; ok { + if s, ok := value.(string); ok { + if normalized := normalizeToolFeedbackSummary(s); normalized != "" { + return normalized + } + } + } + } + return "" +} + +func normalizeToolFeedbackSummary(text string) string { + return redactToolFeedbackSecrets(strings.Join(strings.Fields(text), " ")) +} + +func truncateToolFeedbackSummary(text string) string { + const maxRunes = 96 + runes := []rune(text) + if len(runes) <= maxRunes { + return text + } + return strings.TrimSpace(string(runes[:maxRunes-3])) + "..." +} + +func summarizeExecCommand(command string) string { + fields := strings.Fields(command) + for i := 0; i < len(fields); i++ { + token := strings.Trim(fields[i], `"'`) + if token == "" || isShellAssignment(token) { + continue + } + switch token { + case "env", "command", "time", "timeout", "sudo": + continue + case "bash", "sh", "zsh", "fish", "python", "python3", "node", "deno", "bun", "uv", "uvx", "npx": + for j := i + 1; j < len(fields); j++ { + next := strings.Trim(fields[j], `"'`) + if next == "" || strings.HasPrefix(next, "-") || isShellAssignment(next) { + continue + } + return filepath.Base(next) + } + return token + default: + return filepath.Base(token) + } + } + return "" +} + +func isShellAssignment(token string) bool { + return strings.Contains(token, "=") && !strings.HasPrefix(token, "/") && !strings.HasPrefix(token, ".") +} + +var ( + toolFeedbackSecretValuePatterns = []*regexp.Regexp{ + regexp.MustCompile(`(?i)\bsk-(?:proj|or-v1)?-[A-Za-z0-9_-]{16,}`), + regexp.MustCompile(`\bAIza[A-Za-z0-9_-]{20,}`), + regexp.MustCompile(`\bmat_[A-Za-z0-9_-]{16,}`), + regexp.MustCompile(`\b\d{6,}:[A-Za-z0-9_-]{20,}`), + regexp.MustCompile(`\b(?:ghp|github_pat|glpat|xox[baprs])_[A-Za-z0-9_-]{16,}`), + } + toolFeedbackSecretKVPattern = regexp.MustCompile(`(?i)(\b(?:api[_-]?key|access[_-]?token|auth[_-]?token|token|secret|password|authorization)\b\s*[=:]\s*)("[^"]*"|'[^']*'|[^\s&]+)`) + toolFeedbackSecretFlagPattern = regexp.MustCompile(`(?i)(--(?:api-key|access-token|auth-token|token|secret|password|authorization)(?:=|\s+))("[^"]*"|'[^']*'|[^\s&]+)`) +) + +func redactToolFeedbackSecrets(text string) string { + if text == "" { + return "" + } + for _, pattern := range toolFeedbackSecretValuePatterns { + text = pattern.ReplaceAllString(text, "[redacted]") + } + text = toolFeedbackSecretKVPattern.ReplaceAllString(text, "${1}[redacted]") + text = toolFeedbackSecretFlagPattern.ReplaceAllString(text, "${1}[redacted]") + return text +} + // FitToolFeedbackMessage keeps tool feedback within a single outbound message. // It preserves the first line when possible and truncates the explanation body // instead of letting the message be split into multiple chunks. diff --git a/pkg/utils/tool_feedback_test.go b/pkg/utils/tool_feedback_test.go index da4accce4..b6881a2a0 100644 --- a/pkg/utils/tool_feedback_test.go +++ b/pkg/utils/tool_feedback_test.go @@ -11,7 +11,7 @@ func TestFormatToolFeedbackMessage(t *testing.T) { "I will read README.md first to confirm the current project structure.", "{\n \"path\": \"README.md\"\n}", ) - want := "\U0001f527 `read_file`\nI will read README.md first to confirm the current project structure.\n```json\n{\n \"path\": \"README.md\"\n}\n```" + want := "🔧 `read_file`\nI will read README.md first to confirm the current project structure.\n```json\n{\n \"path\": \"README.md\"\n}\n```" if got != want { t.Fatalf("FormatToolFeedbackMessage() = %q, want %q", got, want) } @@ -19,7 +19,7 @@ func TestFormatToolFeedbackMessage(t *testing.T) { func TestFormatToolFeedbackMessage_EmptyExplanationShowsArgs(t *testing.T) { got := FormatToolFeedbackMessage("read_file", "", "{\n \"path\": \"README.md\"\n}") - want := "\U0001f527 `read_file`\n```json\n{\n \"path\": \"README.md\"\n}\n```" + want := "🔧 `read_file`\n```json\n{\n \"path\": \"README.md\"\n}\n```" if got != want { t.Fatalf("FormatToolFeedbackMessage() = %q, want %q", got, want) } @@ -35,12 +35,85 @@ func TestFormatToolFeedbackMessage_EmptyToolNameOmitsToolLine(t *testing.T) { func TestFormatToolFeedbackMessage_EmptyExplanationAndArgsKeepsOnlyToolLine(t *testing.T) { got := FormatToolFeedbackMessage("read_file", "", "") - want := "\U0001f527 `read_file`" + want := "🔧 `read_file`" if got != want { t.Fatalf("FormatToolFeedbackMessage() = %q, want %q", got, want) } } +func TestFormatToolFeedbackMessageWithStyle_WorkingSummary(t *testing.T) { + got := FormatToolFeedbackMessageWithStyle( + "working_summary", + "read_file", + "I will read README.md first to confirm the current project structure.", + "{\n \"path\": \"README.md\"\n}", + ) + want := "Working...\n• tool: `read_file` — `README.md`" + if got != want { + t.Fatalf("FormatToolFeedbackMessageWithStyle() = %q, want %q", got, want) + } +} + +func TestFormatToolFeedbackMessageWithStyle_WorkingSummaryShowsFileBasenameOnly(t *testing.T) { + got := FormatToolFeedbackMessageWithStyle( + "working_summary", + "write_file", + "", + "{\"path\":\"/home/user/private/config.json\"}", + ) + want := "Working...\n• tool: `write_file` — `config.json`" + if got != want { + t.Fatalf("FormatToolFeedbackMessageWithStyle() = %q, want %q", got, want) + } +} + +func TestFormatToolFeedbackMessageWithStyle_WorkingSummaryShowsExecCommand(t *testing.T) { + got := FormatToolFeedbackMessageWithStyle( + "working_summary", + "exec", + "", + "{\n \"action\": \"run\",\n \"command\": \"scripts/gog_me forms add-question FORM --title Name --type paragraph\",\n \"timeout\": 120\n}", + ) + want := "Working...\n• tool: `exec` — `gog_me`" + if got != want { + t.Fatalf("FormatToolFeedbackMessageWithStyle() = %q, want %q", got, want) + } +} + +func TestFormatToolFeedbackMessageWithStyle_WorkingSummaryShowsExecScriptNameOnly(t *testing.T) { + got := FormatToolFeedbackMessageWithStyle( + "working_summary", + "exec", + "", + "{\"command\":\"OPENAI_API_KEY=sk-proj-abcdefghijklmnopqrstuvwxyz0123456789 bash -lc /home/server/.picoclaw/main/workspace/tmp_add_questions_anya_form_api.sh --api-key sk-or-v1-abcdefghijklmnopqrstuvwxyz0123456789\"}", + ) + want := "Working...\n• tool: `exec` — `tmp_add_questions_anya_form_api.sh`" + if got != want { + t.Fatalf("FormatToolFeedbackMessageWithStyle() = %q, want %q", got, want) + } +} + +func TestFormatToolFeedbackMessageWithStyle_WorkingSummarySanitizesCodeSpan(t *testing.T) { + got := FormatToolFeedbackMessageWithStyle("working_summary", "read_file", "", "{\"path\":\"bad`path\"}") + want := "Working...\n• tool: `read_file` — `bad'path`" + if got != want { + t.Fatalf("FormatToolFeedbackMessageWithStyle() = %q, want %q", got, want) + } +} + +func TestFormatToolFeedbackMessageWithStyle_WorkingSummaryOmitsNonFileToolArgs(t *testing.T) { + got := FormatToolFeedbackMessageWithStyle( + "working_summary", + "web_fetch", + "", + `{"url":"https://example.test/?token=mat_abcdefghijklmnopqrstuvwxyz0123456789"}`, + ) + want := "Working...\n• tool: `web_fetch`" + if got != want { + t.Fatalf("FormatToolFeedbackMessageWithStyle() = %q, want %q", got, want) + } +} + func TestFitToolFeedbackMessage_TruncatesBodyWithinSingleMessage(t *testing.T) { got := FitToolFeedbackMessage( "\U0001f527 `read_file`\nRead README.md first to confirm the current project structure.",