feat(agents): add working summary tool feedback
This commit is contained in:
parent
788cda5c7a
commit
194781c3e9
12 changed files with 443 additions and 46 deletions
|
|
@ -16,7 +16,8 @@
|
|||
"tool_feedback": {
|
||||
"enabled": false,
|
||||
"max_args_length": 300,
|
||||
"separate_messages": false
|
||||
"separate_messages": false,
|
||||
"style": "raw"
|
||||
}
|
||||
}
|
||||
},
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
|
|
@ -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, "<code>inventorydb__get_location</code>") {
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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.",
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue