From 5b596ed2f0aea803fd7afdb4b526e2775c02d8b6 Mon Sep 17 00:00:00 2001 From: lc6464 <64722907+lc6464@users.noreply.github.com> Date: Thu, 9 Apr 2026 22:15:46 +0800 Subject: [PATCH 1/6] fix(chat): keep tool summaries and assistant output together --- pkg/agent/loop.go | 2 +- web/backend/api/session.go | 55 +++++++++++++++++- web/backend/api/session_test.go | 98 +++++++++++++++++++++++++++++---- 3 files changed, 141 insertions(+), 14 deletions(-) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index e089e6d3d..1b53e3dac 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -1409,7 +1409,7 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) Media: msg.Media, DefaultResponse: defaultResponse, EnableSummary: true, - SendResponse: false, + SendResponse: msg.Channel == "pico", } // context-dependent commands check their own Runtime fields and report diff --git a/web/backend/api/session.go b/web/backend/api/session.go index a2e931010..9e712be0c 100644 --- a/web/backend/api/session.go +++ b/web/backend/api/session.go @@ -4,6 +4,7 @@ import ( "bufio" "encoding/json" "errors" + "fmt" "net/http" "os" "path/filepath" @@ -14,6 +15,7 @@ import ( "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/providers" + "github.com/sipeed/picoclaw/pkg/utils" ) // registerSessionRoutes binds session list and detail endpoints to the ServeMux. @@ -72,6 +74,8 @@ const ( // pkg/memory/jsonl.go so oversized lines fail consistently everywhere. maxSessionJSONLLineSize = 10 * 1024 * 1024 maxSessionTitleRunes = 60 + // Keep session reconstruction aligned with tool_feedback max args preview. + sessionToolFeedbackMaxArgsLength = 300 handledToolResponseSummaryText = "Requested output delivered via tool attachment." ) @@ -275,6 +279,11 @@ func visibleSessionMessages(messages []providers.Message) []sessionChatMessage { } case "assistant": + toolSummaryMessages := visibleAssistantToolSummaryMessages(msg.ToolCalls) + if len(toolSummaryMessages) > 0 { + transcript = append(transcript, toolSummaryMessages...) + } + visibleToolMessages := visibleAssistantToolMessages(msg.ToolCalls) if len(visibleToolMessages) > 0 { transcript = append(transcript, visibleToolMessages...) @@ -283,7 +292,7 @@ func visibleSessionMessages(messages []providers.Message) []sessionChatMessage { // Pico web chat can persist both visible `message` tool output and a // later plain assistant reply in the same turn. Hide only the fixed // internal summary that marks handled tool delivery. - if len(visibleToolMessages) > 0 || !sessionMessageVisible(msg) || assistantMessageInternalOnly(msg) { + if !sessionMessageVisible(msg) || assistantMessageInternalOnly(msg) { continue } @@ -302,6 +311,50 @@ func assistantMessageInternalOnly(msg providers.Message) bool { return strings.TrimSpace(msg.Content) == handledToolResponseSummaryText } +func visibleAssistantToolSummaryMessages(toolCalls []providers.ToolCall) []sessionChatMessage { + if len(toolCalls) == 0 { + return nil + } + + messages := make([]sessionChatMessage, 0, len(toolCalls)) + for _, tc := range toolCalls { + name := tc.Name + argsJSON := "" + if tc.Function != nil { + if name == "" { + name = tc.Function.Name + } + argsJSON = tc.Function.Arguments + } + + if strings.TrimSpace(name) == "" { + continue + } + + if strings.TrimSpace(argsJSON) == "" && len(tc.Arguments) > 0 { + if encodedArgs, err := json.Marshal(tc.Arguments); err == nil { + argsJSON = string(encodedArgs) + } + } + + argsPreview := strings.TrimSpace(argsJSON) + if argsPreview == "" { + argsPreview = "{}" + } + + messages = append(messages, sessionChatMessage{ + Role: "assistant", + Content: formatToolCallSummary(name, utils.Truncate(argsPreview, sessionToolFeedbackMaxArgsLength)), + }) + } + + return messages +} + +func formatToolCallSummary(name, argsPreview string) string { + return fmt.Sprintf("\U0001f527 `%s`\n```\n%s\n```", name, argsPreview) +} + func visibleAssistantToolMessages(toolCalls []providers.ToolCall) []sessionChatMessage { if len(toolCalls) == 0 { return nil diff --git a/web/backend/api/session_test.go b/web/backend/api/session_test.go index 9248c11b7..167c17ecf 100644 --- a/web/backend/api/session_test.go +++ b/web/backend/api/session_test.go @@ -273,11 +273,14 @@ func TestHandleGetSession_ReconstructsVisibleMessageToolOutput(t *testing.T) { if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { t.Fatalf("Unmarshal() error = %v", err) } - if len(resp.Messages) != 2 { - t.Fatalf("len(resp.Messages) = %d, want 2", len(resp.Messages)) + if len(resp.Messages) != 3 { + t.Fatalf("len(resp.Messages) = %d, want 3", len(resp.Messages)) } - if resp.Messages[1].Role != "assistant" || resp.Messages[1].Content != "visible tool output" { - t.Fatalf("assistant message = %#v, want visible tool output", resp.Messages[1]) + if !strings.Contains(resp.Messages[1].Content, "`message`") { + t.Fatalf("tool summary message = %#v, want message tool summary", resp.Messages[1]) + } + if resp.Messages[2].Role != "assistant" || resp.Messages[2].Content != "visible tool output" { + t.Fatalf("assistant message = %#v, want visible tool output", resp.Messages[2]) } } @@ -336,14 +339,17 @@ func TestHandleGetSession_PreservesFinalAssistantReplyAfterMessageToolOutput(t * if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { t.Fatalf("Unmarshal() error = %v", err) } - if len(resp.Messages) != 3 { - t.Fatalf("len(resp.Messages) = %d, want 3", len(resp.Messages)) + if len(resp.Messages) != 4 { + t.Fatalf("len(resp.Messages) = %d, want 4", len(resp.Messages)) } - if resp.Messages[1].Role != "assistant" || resp.Messages[1].Content != "visible tool output" { - t.Fatalf("interim assistant message = %#v, want visible tool output", resp.Messages[1]) + if !strings.Contains(resp.Messages[1].Content, "`message`") { + t.Fatalf("tool summary message = %#v, want message tool summary", resp.Messages[1]) } - if resp.Messages[2].Role != "assistant" || resp.Messages[2].Content != "final assistant reply" { - t.Fatalf("final assistant message = %#v, want final assistant reply", resp.Messages[2]) + if resp.Messages[2].Role != "assistant" || resp.Messages[2].Content != "visible tool output" { + t.Fatalf("interim assistant message = %#v, want visible tool output", resp.Messages[2]) + } + if resp.Messages[3].Role != "assistant" || resp.Messages[3].Content != "final assistant reply" { + t.Fatalf("final assistant message = %#v, want final assistant reply", resp.Messages[3]) } } @@ -400,8 +406,76 @@ func TestHandleListSessions_MessageCountUsesVisibleTranscript(t *testing.T) { if len(items) != 1 { t.Fatalf("len(items) = %d, want 1", len(items)) } - if items[0].MessageCount != 2 { - t.Fatalf("items[0].MessageCount = %d, want 2", items[0].MessageCount) + if items[0].MessageCount != 3 { + t.Fatalf("items[0].MessageCount = %d, want 3", items[0].MessageCount) + } +} + +func TestHandleGetSession_PreservesToolSummaryAndAssistantContent(t *testing.T) { + configPath, cleanup := setupOAuthTestEnv(t) + defer cleanup() + + dir := sessionsTestDir(t, configPath) + store, err := memory.NewJSONLStore(dir) + if err != nil { + t.Fatalf("NewJSONLStore() error = %v", err) + } + + sessionKey := picoSessionPrefix + "detail-tool-summary-and-content" + for _, msg := range []providers.Message{ + {Role: "user", Content: "check file"}, + { + Role: "assistant", + Content: "model final reply", + ToolCalls: []providers.ToolCall{ + { + ID: "call_1", + Type: "function", + Function: &providers.FunctionCall{ + Name: "read_file", + Arguments: `{"path":"README.md","start_line":1,"end_line":10}`, + }, + }, + }, + }, + } { + if err := store.AddFullMessage(nil, sessionKey, msg); err != nil { + t.Fatalf("AddFullMessage() error = %v", err) + } + } + + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/api/sessions/detail-tool-summary-and-content", nil) + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + + var resp struct { + Messages []struct { + Role string `json:"role"` + Content string `json:"content"` + } `json:"messages"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("Unmarshal() error = %v", err) + } + if len(resp.Messages) != 3 { + t.Fatalf("len(resp.Messages) = %d, want 3", len(resp.Messages)) + } + if resp.Messages[0].Role != "user" || resp.Messages[0].Content != "check file" { + t.Fatalf("first message = %#v, want user/check file", resp.Messages[0]) + } + if !strings.Contains(resp.Messages[1].Content, "`read_file`") { + t.Fatalf("tool summary message = %#v, want read_file summary", resp.Messages[1]) + } + if resp.Messages[2].Role != "assistant" || resp.Messages[2].Content != "model final reply" { + t.Fatalf("assistant message = %#v, want model final reply", resp.Messages[2]) } } From 2aeed8fb3a5be7f00d32bf8c7a8e71e57e2aebd6 Mon Sep 17 00:00:00 2001 From: lc6464 <64722907+lc6464@users.noreply.github.com> Date: Thu, 9 Apr 2026 22:32:35 +0800 Subject: [PATCH 2/6] fix(pico): stream assistant text between tool calls --- pkg/agent/loop.go | 16 ++++++- pkg/agent/loop_test.go | 98 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 1 deletion(-) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 1b53e3dac..8aa71a168 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -1409,7 +1409,7 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) Media: msg.Media, DefaultResponse: defaultResponse, EnableSummary: true, - SendResponse: msg.Channel == "pico", + SendResponse: false, } // context-dependent commands check their own Runtime fields and report @@ -2253,6 +2253,20 @@ turnLoop: } logger.DebugCF("agent", "LLM response", llmResponseFields) + if al.bus != nil && ts.channel == "pico" { + liveContent := response.Content + if liveContent == "" && len(response.ToolCalls) == 0 && response.ReasoningContent != "" { + liveContent = response.ReasoningContent + } + if strings.TrimSpace(liveContent) != "" { + al.bus.PublishOutbound(turnCtx, bus.OutboundMessage{ + Channel: ts.channel, + ChatID: ts.chatID, + Content: liveContent, + }) + } + } + if len(response.ToolCalls) == 0 || gracefulTerminal { responseContent := response.Content if responseContent == "" && response.ReasoningContent != "" { diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go index 3d04b81cc..7c10e11aa 100644 --- a/pkg/agent/loop_test.go +++ b/pkg/agent/loop_test.go @@ -1069,6 +1069,40 @@ func (m *toolFeedbackProvider) GetDefaultModel() string { return "heartbeat-tool-feedback-model" } +type picoInterleavedContentProvider struct { + calls int +} + +func (m *picoInterleavedContentProvider) Chat( + ctx context.Context, + messages []providers.Message, + tools []providers.ToolDefinition, + model string, + opts map[string]any, +) (*providers.LLMResponse, error) { + m.calls++ + if m.calls == 1 { + return &providers.LLMResponse{ + Content: "intermediate model text", + ToolCalls: []providers.ToolCall{{ + ID: "call_tool_limit_test", + Type: "function", + Name: "tool_limit_test_tool", + Arguments: map[string]any{"value": "x"}, + }}, + }, nil + } + + return &providers.LLMResponse{ + Content: "final model text", + ToolCalls: []providers.ToolCall{}, + }, nil +} + +func (m *picoInterleavedContentProvider) GetDefaultModel() string { + return "pico-interleaved-content-model" +} + type toolLimitOnlyProvider struct{} func (m *toolLimitOnlyProvider) Chat( @@ -2732,6 +2766,70 @@ func TestProcessMessage_PublishesToolFeedbackWhenEnabled(t *testing.T) { } } +func TestProcessMessage_PicoPublishesAssistantContentDuringToolCalls(t *testing.T) { + tmpDir := t.TempDir() + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: tmpDir, + ModelName: "test-model", + MaxTokens: 4096, + MaxToolIterations: 10, + }, + }, + } + + msgBus := bus.NewMessageBus() + provider := &picoInterleavedContentProvider{} + al := NewAgentLoop(cfg, msgBus, provider) + + agent := al.GetRegistry().GetDefaultAgent() + if agent == nil { + t.Fatal("expected default agent") + } + agent.Tools.Register(&toolLimitTestTool{}) + + response, err := al.processMessage(context.Background(), bus.InboundMessage{ + Channel: "pico", + SenderID: "user-1", + ChatID: "session-1", + Content: "run with tools", + }) + if err != nil { + t.Fatalf("processMessage() error = %v", err) + } + if response != "final model text" { + t.Fatalf("processMessage() response = %q, want %q", response, "final model text") + } + + outputs := make([]string, 0, 2) + deadline := time.After(2 * time.Second) + for len(outputs) < 2 { + select { + case outbound := <-msgBus.OutboundChan(): + outputs = append(outputs, outbound.Content) + case <-deadline: + t.Fatalf("timed out waiting for pico outputs, got %v", outputs) + } + } + + if outputs[0] != "intermediate model text" { + t.Fatalf("first outbound content = %q, want %q", outputs[0], "intermediate model text") + } + if outputs[1] != "final model text" { + t.Fatalf("second outbound content = %q, want %q", outputs[1], "final model text") + } + + select { + case outbound := <-msgBus.OutboundChan(): + if outbound.Content == "final model text" { + t.Fatalf("unexpected duplicate final pico output: %+v", outbound) + } + case <-time.After(200 * time.Millisecond): + } +} + func TestResolveMediaRefs_ResolvesToBase64(t *testing.T) { store := media.NewFileMediaStore() dir := t.TempDir() From 9982ee29a88cbb8790ab74958b91e5127b347511 Mon Sep 17 00:00:00 2001 From: lc6464 <64722907+lc6464@users.noreply.github.com> Date: Thu, 9 Apr 2026 22:59:36 +0800 Subject: [PATCH 3/6] fix(pico): avoid duplicate final websocket message --- pkg/agent/loop.go | 10 +++------- pkg/agent/loop_test.go | 30 ++++++++++++++++++++++-------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 8aa71a168..431376be3 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -2253,16 +2253,12 @@ turnLoop: } logger.DebugCF("agent", "LLM response", llmResponseFields) - if al.bus != nil && ts.channel == "pico" { - liveContent := response.Content - if liveContent == "" && len(response.ToolCalls) == 0 && response.ReasoningContent != "" { - liveContent = response.ReasoningContent - } - if strings.TrimSpace(liveContent) != "" { + if al.bus != nil && ts.channel == "pico" && len(response.ToolCalls) > 0 { + if strings.TrimSpace(response.Content) != "" { al.bus.PublishOutbound(turnCtx, bus.OutboundMessage{ Channel: ts.channel, ChatID: ts.chatID, - Content: liveContent, + Content: response.Content, }) } } diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go index 7c10e11aa..371f91d89 100644 --- a/pkg/agent/loop_test.go +++ b/pkg/agent/loop_test.go @@ -2766,7 +2766,7 @@ func TestProcessMessage_PublishesToolFeedbackWhenEnabled(t *testing.T) { } } -func TestProcessMessage_PicoPublishesAssistantContentDuringToolCalls(t *testing.T) { +func TestRun_PicoPublishesAssistantContentDuringToolCallsWithoutFinalDuplicate(t *testing.T) { tmpDir := t.TempDir() cfg := &config.Config{ @@ -2790,17 +2790,21 @@ func TestProcessMessage_PicoPublishesAssistantContentDuringToolCalls(t *testing. } agent.Tools.Register(&toolLimitTestTool{}) - response, err := al.processMessage(context.Background(), bus.InboundMessage{ + runCtx, runCancel := context.WithCancel(context.Background()) + defer runCancel() + + runDone := make(chan error, 1) + go func() { + runDone <- al.Run(runCtx) + }() + + if err := msgBus.PublishInbound(context.Background(), bus.InboundMessage{ Channel: "pico", SenderID: "user-1", ChatID: "session-1", Content: "run with tools", - }) - if err != nil { - t.Fatalf("processMessage() error = %v", err) - } - if response != "final model text" { - t.Fatalf("processMessage() response = %q, want %q", response, "final model text") + }); err != nil { + t.Fatalf("PublishInbound() error = %v", err) } outputs := make([]string, 0, 2) @@ -2821,6 +2825,16 @@ func TestProcessMessage_PicoPublishesAssistantContentDuringToolCalls(t *testing. t.Fatalf("second outbound content = %q, want %q", outputs[1], "final model text") } + runCancel() + select { + case err := <-runDone: + if err != nil { + t.Fatalf("Run() error = %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for Run() to exit") + } + select { case outbound := <-msgBus.OutboundChan(): if outbound.Content == "final model text" { From bd13092831ca87457cbae7dc53f3d9b4d6b22bbd Mon Sep 17 00:00:00 2001 From: lc6464 <64722907+lc6464@users.noreply.github.com> Date: Thu, 9 Apr 2026 23:52:02 +0800 Subject: [PATCH 4/6] fix(review): align tool feedback reconstruction with runtime behavior --- pkg/agent/loop.go | 16 +++++-- pkg/utils/tool_feedback.go | 9 ++++ pkg/utils/tool_feedback_test.go | 11 +++++ web/backend/api/session.go | 51 ++++++++++++++-------- web/backend/api/session_test.go | 77 +++++++++++++++++++++++++++++++++ 5 files changed, 142 insertions(+), 22 deletions(-) create mode 100644 pkg/utils/tool_feedback.go create mode 100644 pkg/utils/tool_feedback_test.go diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 431376be3..89e92aa14 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -2255,11 +2255,21 @@ turnLoop: if al.bus != nil && ts.channel == "pico" && len(response.ToolCalls) > 0 { if strings.TrimSpace(response.Content) != "" { - al.bus.PublishOutbound(turnCtx, bus.OutboundMessage{ + outCtx, outCancel := context.WithTimeout(turnCtx, 3*time.Second) + err := al.bus.PublishOutbound(outCtx, bus.OutboundMessage{ Channel: ts.channel, ChatID: ts.chatID, Content: response.Content, }) + outCancel() + if err != nil { + logger.WarnCF("agent", "Failed to publish pico interim tool-call content", map[string]any{ + "error": err.Error(), + "channel": ts.channel, + "chat_id": ts.chatID, + "iteration": iteration, + }) + } } } @@ -2400,7 +2410,7 @@ turnLoop: string(argsJSON), al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(), ) - feedbackMsg := fmt.Sprintf("\U0001f527 `%s`\n```\n%s\n```", toolName, feedbackPreview) + feedbackMsg := utils.FormatToolFeedbackMessage(toolName, feedbackPreview) fbCtx, fbCancel := context.WithTimeout(turnCtx, 3*time.Second) _ = al.bus.PublishOutbound(fbCtx, bus.OutboundMessage{ Channel: ts.channel, @@ -2682,7 +2692,7 @@ turnLoop: string(argsJSON), al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(), ) - feedbackMsg := fmt.Sprintf("\U0001f527 `%s`\n```\n%s\n```", tc.Name, feedbackPreview) + feedbackMsg := utils.FormatToolFeedbackMessage(tc.Name, feedbackPreview) fbCtx, fbCancel := context.WithTimeout(turnCtx, 3*time.Second) _ = al.bus.PublishOutbound(fbCtx, bus.OutboundMessage{ Channel: ts.channel, diff --git a/pkg/utils/tool_feedback.go b/pkg/utils/tool_feedback.go new file mode 100644 index 000000000..028908617 --- /dev/null +++ b/pkg/utils/tool_feedback.go @@ -0,0 +1,9 @@ +package utils + +import "fmt" + +// FormatToolFeedbackMessage renders the tool name and arguments preview in the +// same markdown shape used by live tool feedback and session reconstruction. +func FormatToolFeedbackMessage(toolName, argsPreview string) string { + return fmt.Sprintf("\U0001f527 `%s`\n```\n%s\n```", toolName, argsPreview) +} \ No newline at end of file diff --git a/pkg/utils/tool_feedback_test.go b/pkg/utils/tool_feedback_test.go new file mode 100644 index 000000000..9f64f66d7 --- /dev/null +++ b/pkg/utils/tool_feedback_test.go @@ -0,0 +1,11 @@ +package utils + +import "testing" + +func TestFormatToolFeedbackMessage(t *testing.T) { + got := FormatToolFeedbackMessage("read_file", "{\"path\":\"README.md\"}") + want := "\U0001f527 `read_file`\n```\n{\"path\":\"README.md\"}\n```" + if got != want { + t.Fatalf("FormatToolFeedbackMessage() = %q, want %q", got, want) + } +} \ No newline at end of file diff --git a/web/backend/api/session.go b/web/backend/api/session.go index 9e712be0c..a368e9b79 100644 --- a/web/backend/api/session.go +++ b/web/backend/api/session.go @@ -4,7 +4,6 @@ import ( "bufio" "encoding/json" "errors" - "fmt" "net/http" "os" "path/filepath" @@ -74,12 +73,15 @@ const ( // pkg/memory/jsonl.go so oversized lines fail consistently everywhere. maxSessionJSONLLineSize = 10 * 1024 * 1024 maxSessionTitleRunes = 60 - // Keep session reconstruction aligned with tool_feedback max args preview. - sessionToolFeedbackMaxArgsLength = 300 handledToolResponseSummaryText = "Requested output delivered via tool attachment." ) +func defaultToolFeedbackMaxArgsLength() int { + defaults := config.AgentDefaults{} + return defaults.GetToolFeedbackMaxArgsLength() +} + // extractPicoSessionID extracts the session UUID from a full session key. // Returns the UUID and true if the key matches the Pico session pattern. func extractPicoSessionID(key string) (string, bool) { @@ -206,7 +208,7 @@ func (h *Handler) readJSONLSession(dir, sessionID string) (sessionFile, error) { }, nil } -func buildSessionListItem(sessionID string, sess sessionFile) sessionListItem { +func buildSessionListItem(sessionID string, sess sessionFile, toolFeedbackMaxArgsLength int) sessionListItem { preview := "" for _, msg := range sess.Messages { if msg.Role == "user" { @@ -223,7 +225,7 @@ func buildSessionListItem(sessionID string, sess sessionFile) sessionListItem { } title := preview - validMessageCount := len(visibleSessionMessages(sess.Messages)) + validMessageCount := len(visibleSessionMessages(sess.Messages, toolFeedbackMaxArgsLength)) return sessionListItem{ ID: sessionID, @@ -264,7 +266,7 @@ func sessionMessagePreview(msg providers.Message) string { return "" } -func visibleSessionMessages(messages []providers.Message) []sessionChatMessage { +func visibleSessionMessages(messages []providers.Message, toolFeedbackMaxArgsLength int) []sessionChatMessage { transcript := make([]sessionChatMessage, 0, len(messages)) for _, msg := range messages { @@ -279,7 +281,7 @@ func visibleSessionMessages(messages []providers.Message) []sessionChatMessage { } case "assistant": - toolSummaryMessages := visibleAssistantToolSummaryMessages(msg.ToolCalls) + toolSummaryMessages := visibleAssistantToolSummaryMessages(msg.ToolCalls, toolFeedbackMaxArgsLength) if len(toolSummaryMessages) > 0 { transcript = append(transcript, toolSummaryMessages...) } @@ -311,10 +313,13 @@ func assistantMessageInternalOnly(msg providers.Message) bool { return strings.TrimSpace(msg.Content) == handledToolResponseSummaryText } -func visibleAssistantToolSummaryMessages(toolCalls []providers.ToolCall) []sessionChatMessage { +func visibleAssistantToolSummaryMessages(toolCalls []providers.ToolCall, toolFeedbackMaxArgsLength int) []sessionChatMessage { if len(toolCalls) == 0 { return nil } + if toolFeedbackMaxArgsLength <= 0 { + toolFeedbackMaxArgsLength = defaultToolFeedbackMaxArgsLength() + } messages := make([]sessionChatMessage, 0, len(toolCalls)) for _, tc := range toolCalls { @@ -344,17 +349,13 @@ func visibleAssistantToolSummaryMessages(toolCalls []providers.ToolCall) []sessi messages = append(messages, sessionChatMessage{ Role: "assistant", - Content: formatToolCallSummary(name, utils.Truncate(argsPreview, sessionToolFeedbackMaxArgsLength)), + Content: utils.FormatToolFeedbackMessage(name, utils.Truncate(argsPreview, toolFeedbackMaxArgsLength)), }) } return messages } -func formatToolCallSummary(name, argsPreview string) string { - return fmt.Sprintf("\U0001f527 `%s`\n```\n%s\n```", name, argsPreview) -} - func visibleAssistantToolMessages(toolCalls []providers.ToolCall) []sessionChatMessage { if len(toolCalls) == 0 { return nil @@ -400,7 +401,19 @@ func (h *Handler) sessionsDir() (string, error) { return "", err } - workspace := cfg.Agents.Defaults.Workspace + return resolveSessionsDir(cfg.Agents.Defaults.Workspace), nil +} + +func (h *Handler) sessionRuntimeSettings() (string, int, error) { + cfg, err := config.LoadConfig(h.configPath) + if err != nil { + return "", 0, err + } + + return resolveSessionsDir(cfg.Agents.Defaults.Workspace), cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(), nil +} + +func resolveSessionsDir(workspace string) string { if workspace == "" { home, _ := os.UserHomeDir() workspace = filepath.Join(home, ".picoclaw", "workspace") @@ -416,14 +429,14 @@ func (h *Handler) sessionsDir() (string, error) { } } - return filepath.Join(workspace, "sessions"), nil + return filepath.Join(workspace, "sessions") } // handleListSessions returns a list of Pico session summaries. // // GET /api/sessions func (h *Handler) handleListSessions(w http.ResponseWriter, r *http.Request) { - dir, err := h.sessionsDir() + dir, toolFeedbackMaxArgsLength, err := h.sessionRuntimeSettings() if err != nil { http.Error(w, "failed to resolve sessions directory", http.StatusInternalServerError) return @@ -507,7 +520,7 @@ func (h *Handler) handleListSessions(w http.ResponseWriter, r *http.Request) { } seen[sessionID] = struct{}{} - items = append(items, buildSessionListItem(sessionID, sess)) + items = append(items, buildSessionListItem(sessionID, sess, toolFeedbackMaxArgsLength)) } // Sort by updated descending (most recent first) @@ -555,7 +568,7 @@ func (h *Handler) handleGetSession(w http.ResponseWriter, r *http.Request) { return } - dir, err := h.sessionsDir() + dir, toolFeedbackMaxArgsLength, err := h.sessionRuntimeSettings() if err != nil { http.Error(w, "failed to resolve sessions directory", http.StatusInternalServerError) return @@ -582,7 +595,7 @@ func (h *Handler) handleGetSession(w http.ResponseWriter, r *http.Request) { } } - messages := visibleSessionMessages(sess.Messages) + messages := visibleSessionMessages(sess.Messages, toolFeedbackMaxArgsLength) w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]any{ diff --git a/web/backend/api/session_test.go b/web/backend/api/session_test.go index 167c17ecf..5d7620362 100644 --- a/web/backend/api/session_test.go +++ b/web/backend/api/session_test.go @@ -13,6 +13,7 @@ import ( "github.com/sipeed/picoclaw/pkg/memory" "github.com/sipeed/picoclaw/pkg/providers" "github.com/sipeed/picoclaw/pkg/session" + "github.com/sipeed/picoclaw/pkg/utils" ) func sessionsTestDir(t *testing.T, configPath string) string { @@ -479,6 +480,82 @@ func TestHandleGetSession_PreservesToolSummaryAndAssistantContent(t *testing.T) } } +func TestHandleGetSession_UsesConfiguredToolFeedbackMaxArgsLength(t *testing.T) { + configPath, cleanup := setupOAuthTestEnv(t) + defer cleanup() + + cfg, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + cfg.Agents.Defaults.ToolFeedback.MaxArgsLength = 20 + err = config.SaveConfig(configPath, cfg) + if err != nil { + t.Fatalf("SaveConfig() error = %v", err) + } + + dir := sessionsTestDir(t, configPath) + store, err := memory.NewJSONLStore(dir) + if err != nil { + t.Fatalf("NewJSONLStore() error = %v", err) + } + + argsJSON := `{"path":"README.md","start_line":1,"end_line":10,"extra":"abcdefghijklmnopqrstuvwxyz"}` + sessionKey := picoSessionPrefix + "detail-tool-summary-max-args" + err = store.AddFullMessage(nil, sessionKey, providers.Message{Role: "user", Content: "check file"}) + if err != nil { + t.Fatalf("AddFullMessage(user) error = %v", err) + } + err = store.AddFullMessage(nil, sessionKey, providers.Message{ + Role: "assistant", + ToolCalls: []providers.ToolCall{{ + ID: "call_1", + Type: "function", + Function: &providers.FunctionCall{ + Name: "read_file", + Arguments: argsJSON, + }, + }}, + }) + if err != nil { + t.Fatalf("AddFullMessage(assistant) error = %v", err) + } + + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/api/sessions/detail-tool-summary-max-args", nil) + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + + var resp struct { + Messages []struct { + Role string `json:"role"` + Content string `json:"content"` + } `json:"messages"` + } + err = json.Unmarshal(rec.Body.Bytes(), &resp) + if err != nil { + t.Fatalf("Unmarshal() error = %v", err) + } + if len(resp.Messages) < 2 { + t.Fatalf("len(resp.Messages) = %d, want at least 2", len(resp.Messages)) + } + + wantPreview := utils.Truncate(argsJSON, 20) + if !strings.Contains(resp.Messages[1].Content, wantPreview) { + t.Fatalf("tool summary = %q, want preview %q", resp.Messages[1].Content, wantPreview) + } + if strings.Contains(resp.Messages[1].Content, argsJSON) { + t.Fatalf("tool summary = %q, expected configured truncation", resp.Messages[1].Content) + } +} + func TestHandleGetSession_IncludesMediaOnlyMessages(t *testing.T) { configPath, cleanup := setupOAuthTestEnv(t) defer cleanup() From 58f634b582cb335a8bb8b6682a7c143210a87e04 Mon Sep 17 00:00:00 2001 From: lc6464 <64722907+lc6464@users.noreply.github.com> Date: Fri, 10 Apr 2026 00:02:20 +0800 Subject: [PATCH 5/6] style(lint): satisfy gci and golines for review fixes --- pkg/utils/tool_feedback.go | 2 +- pkg/utils/tool_feedback_test.go | 2 +- web/backend/api/session.go | 5 ++++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/utils/tool_feedback.go b/pkg/utils/tool_feedback.go index 028908617..a6c8895b8 100644 --- a/pkg/utils/tool_feedback.go +++ b/pkg/utils/tool_feedback.go @@ -6,4 +6,4 @@ import "fmt" // same markdown shape used by live tool feedback and session reconstruction. func FormatToolFeedbackMessage(toolName, argsPreview string) string { return fmt.Sprintf("\U0001f527 `%s`\n```\n%s\n```", toolName, argsPreview) -} \ No newline at end of file +} diff --git a/pkg/utils/tool_feedback_test.go b/pkg/utils/tool_feedback_test.go index 9f64f66d7..d7a55ce6b 100644 --- a/pkg/utils/tool_feedback_test.go +++ b/pkg/utils/tool_feedback_test.go @@ -8,4 +8,4 @@ func TestFormatToolFeedbackMessage(t *testing.T) { if got != want { t.Fatalf("FormatToolFeedbackMessage() = %q, want %q", got, want) } -} \ No newline at end of file +} diff --git a/web/backend/api/session.go b/web/backend/api/session.go index a368e9b79..ae580d9aa 100644 --- a/web/backend/api/session.go +++ b/web/backend/api/session.go @@ -313,7 +313,10 @@ func assistantMessageInternalOnly(msg providers.Message) bool { return strings.TrimSpace(msg.Content) == handledToolResponseSummaryText } -func visibleAssistantToolSummaryMessages(toolCalls []providers.ToolCall, toolFeedbackMaxArgsLength int) []sessionChatMessage { +func visibleAssistantToolSummaryMessages( + toolCalls []providers.ToolCall, + toolFeedbackMaxArgsLength int, +) []sessionChatMessage { if len(toolCalls) == 0 { return nil } From bd88385923306f6d78ed6ba749606c3c3008160f Mon Sep 17 00:00:00 2001 From: lc6464 <64722907+lc6464@users.noreply.github.com> Date: Fri, 10 Apr 2026 00:19:45 +0800 Subject: [PATCH 6/6] fix(agent): gate pico interim publish for internal turns --- pkg/agent/loop.go | 28 +++++++++++++----------- pkg/agent/loop_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 89e92aa14..ac230aa86 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -88,6 +88,7 @@ type processOptions struct { DefaultResponse string // Response when LLM returns empty EnableSummary bool // Whether to trigger summarization SendResponse bool // Whether to send response via bus + AllowInterimPicoPublish bool // Whether pico tool-call interim text can be published when SendResponse is false SuppressToolFeedback bool // Whether to suppress inline tool feedback messages NoHistory bool // If true, don't load session history (for heartbeat) SkipInitialSteeringPoll bool // If true, skip the steering poll at loop start (used by Continue) @@ -1398,18 +1399,19 @@ func (al *AgentLoop) processMessage(ctx context.Context, msg bus.InboundMessage) }) opts := processOptions{ - SessionKey: sessionKey, - Channel: msg.Channel, - ChatID: msg.ChatID, - MessageID: msg.MessageID, - ReplyToMessageID: inboundMetadata(msg, metadataKeyReplyToMessage), - SenderID: msg.SenderID, - SenderDisplayName: msg.Sender.DisplayName, - UserMessage: msg.Content, - Media: msg.Media, - DefaultResponse: defaultResponse, - EnableSummary: true, - SendResponse: false, + SessionKey: sessionKey, + Channel: msg.Channel, + ChatID: msg.ChatID, + MessageID: msg.MessageID, + ReplyToMessageID: inboundMetadata(msg, metadataKeyReplyToMessage), + SenderID: msg.SenderID, + SenderDisplayName: msg.Sender.DisplayName, + UserMessage: msg.Content, + Media: msg.Media, + DefaultResponse: defaultResponse, + EnableSummary: true, + SendResponse: false, + AllowInterimPicoPublish: true, } // context-dependent commands check their own Runtime fields and report @@ -2253,7 +2255,7 @@ turnLoop: } logger.DebugCF("agent", "LLM response", llmResponseFields) - if al.bus != nil && ts.channel == "pico" && len(response.ToolCalls) > 0 { + if al.bus != nil && ts.channel == "pico" && len(response.ToolCalls) > 0 && ts.opts.AllowInterimPicoPublish { if strings.TrimSpace(response.Content) != "" { outCtx, outCancel := context.WithTimeout(turnCtx, 3*time.Second) err := al.bus.PublishOutbound(outCtx, bus.OutboundMessage{ diff --git a/pkg/agent/loop_test.go b/pkg/agent/loop_test.go index 371f91d89..a67c8d040 100644 --- a/pkg/agent/loop_test.go +++ b/pkg/agent/loop_test.go @@ -2844,6 +2844,55 @@ func TestRun_PicoPublishesAssistantContentDuringToolCallsWithoutFinalDuplicate(t } } +func TestRunAgentLoop_PicoSkipsInterimPublishWhenNotAllowed(t *testing.T) { + tmpDir := t.TempDir() + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: tmpDir, + ModelName: "test-model", + MaxTokens: 4096, + MaxToolIterations: 10, + }, + }, + } + + msgBus := bus.NewMessageBus() + provider := &picoInterleavedContentProvider{} + al := NewAgentLoop(cfg, msgBus, provider) + + agent := al.GetRegistry().GetDefaultAgent() + if agent == nil { + t.Fatal("expected default agent") + } + agent.Tools.Register(&toolLimitTestTool{}) + + response, err := al.runAgentLoop(context.Background(), agent, processOptions{ + SessionKey: "agent:main:pico:session-1", + Channel: "pico", + ChatID: "session-1", + UserMessage: "run with tools", + DefaultResponse: defaultResponse, + EnableSummary: false, + SendResponse: false, + AllowInterimPicoPublish: false, + SuppressToolFeedback: true, + }) + if err != nil { + t.Fatalf("runAgentLoop() error = %v", err) + } + if response != "final model text" { + t.Fatalf("runAgentLoop() response = %q, want %q", response, "final model text") + } + + select { + case outbound := <-msgBus.OutboundChan(): + t.Fatalf("unexpected outbound message when interim publish disabled: %+v", outbound) + case <-time.After(200 * time.Millisecond): + } +} + func TestResolveMediaRefs_ResolvesToBase64(t *testing.T) { store := media.NewFileMediaStore() dir := t.TempDir()