opencode/picoclaw
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(channels): finalize tool feedback in place

Browse source
This commit is contained in:
lxowalle 2026-04-17 17:18:36 +08:00
parent 9ea8e6167a
commit cea0fa35db
19 changed files with 1073 additions and 154 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

76
pkg/agent/loop.go
View file

@ -874,20 +874,56 @@ func toolFeedbackExplanationFromResponse(
}
explanation := strings.TrimSpace(response.Content)
if explanation == "" {
explanation = strings.TrimSpace(response.Reasoning)
explanation = toolFeedbackExplanationFromToolCalls(response.ToolCalls)
}
if explanation == "" {
explanation = strings.TrimSpace(response.ReasoningContent)
}
if explanation == "" {
explanation = latestUserContent(messages)
if explanation != "" {
explanation = utils.ToolFeedbackContinuationHint + ": " + explanation
}
explanation = toolFeedbackExplanationFromMessages(messages)
}
return utils.Truncate(explanation, maxLen)
}
func toolFeedbackExplanationFromToolCalls(toolCalls []providers.ToolCall) string {
for _, tc := range toolCalls {
if tc.ExtraContent == nil {
continue
}
if explanation := strings.TrimSpace(tc.ExtraContent.ToolFeedbackExplanation); explanation != "" {
return explanation
}
}
return ""
}
func toolFeedbackExplanationForToolCall(
response *providers.LLMResponse,
toolCall providers.ToolCall,
messages []providers.Message,
maxLen int,
) string {
if toolCall.ExtraContent != nil {
if explanation := strings.TrimSpace(toolCall.ExtraContent.ToolFeedbackExplanation); explanation != "" {
return utils.Truncate(explanation, maxLen)
}
}
if response == nil {
return utils.Truncate(toolFeedbackExplanationFromMessages(messages), maxLen)
}
explanation := strings.TrimSpace(response.Content)
if explanation == "" {
explanation = toolFeedbackExplanationFromMessages(messages)
}
return utils.Truncate(explanation, maxLen)
}
func toolFeedbackExplanationFromMessages(messages []providers.Message) string {
explanation := latestUserContent(messages)
if explanation != "" {
return utils.ToolFeedbackContinuationHint + ": " + explanation
}
return ""
}
func shouldPublishToolFeedback(cfg *config.Config, ts *turnState) bool {
if ts == nil || ts.channel == "" || ts.opts.SuppressToolFeedback {
return false
@ -2723,12 +2759,6 @@ turnLoop:
"count": len(normalizedToolCalls),
"iteration": iteration,
})
toolFeedbackExplanation := toolFeedbackExplanationFromResponse(
response,
messages,
al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(),
)
allResponsesHandled := len(normalizedToolCalls) > 0
assistantMsg := providers.Message{
Role: "assistant",
@ -2737,6 +2767,12 @@ turnLoop:
}
for _, tc := range normalizedToolCalls {
argumentsJSON, _ := json.Marshal(tc.Arguments)
toolFeedbackExplanation := toolFeedbackExplanationForToolCall(
response,
tc,
messages,
al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(),
)
extraContent := tc.ExtraContent
if strings.TrimSpace(toolFeedbackExplanation) != "" {
if extraContent == nil {
@ -2822,6 +2858,12 @@ turnLoop:
// Send tool feedback to chat channel if enabled (same as normal tool execution)
if shouldPublishToolFeedback(al.cfg, ts) {
toolFeedbackExplanation := toolFeedbackExplanationForToolCall(
response,
tc,
messages,
al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(),
)
feedbackMsg := utils.FormatToolFeedbackMessage(toolName, toolFeedbackExplanation)
fbCtx, fbCancel := context.WithTimeout(turnCtx, 3*time.Second)
_ = al.bus.PublishOutbound(fbCtx, outboundMessageForTurnWithKind(ts, feedbackMsg, messageKindToolFeedback))
@ -3095,6 +3137,12 @@ turnLoop:
// Send tool feedback to chat channel if enabled (from HEAD)
if shouldPublishToolFeedback(al.cfg, ts) {
toolFeedbackExplanation := toolFeedbackExplanationForToolCall(
response,
tc,
messages,
al.cfg.Agents.Defaults.GetToolFeedbackMaxArgsLength(),
)
feedbackMsg := utils.FormatToolFeedbackMessage(tc.Name, toolFeedbackExplanation)
fbCtx, fbCancel := context.WithTimeout(turnCtx, 3*time.Second)
_ = al.bus.PublishOutbound(fbCtx, outboundMessageForTurnWithKind(ts, feedbackMsg, messageKindToolFeedback))

128
pkg/agent/loop_test.go
View file

@ -1794,6 +1794,43 @@ func (m *toolFeedbackReasoningProvider) GetDefaultModel() string {
return "tool-feedback-reasoning-model"
}
type toolFeedbackExtraContentProvider struct {
filePath string
calls int
}
func (m *toolFeedbackExtraContentProvider) 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{
ToolCalls: []providers.ToolCall{{
ID: "call_explicit_read_file",
Type: "function",
Name: "read_file",
Arguments: map[string]any{"path": m.filePath},
ExtraContent: &providers.ExtraContent{
ToolFeedbackExplanation: "Read README.md first to confirm the current project structure.",
},
}},
}, nil
}
return &providers.LLMResponse{
Content: "DONE",
ToolCalls: []providers.ToolCall{},
}, nil
}
func (m *toolFeedbackExtraContentProvider) GetDefaultModel() string {
return "tool-feedback-extra-content-model"
}
func TestToolFeedbackExplanationFromResponse_UsesCurrentContentFirst(t *testing.T) {
response := &providers.LLMResponse{
Content: "Read README.md first",
@ -1811,10 +1848,15 @@ func TestToolFeedbackExplanationFromResponse_UsesCurrentContentFirst(t *testing.
}
}
func TestToolFeedbackExplanationFromResponse_FallsBackToReasoningContent(t *testing.T) {
func TestToolFeedbackExplanationFromResponse_UsesExplicitToolCallExtraContent(t *testing.T) {
response := &providers.LLMResponse{
Content: "",
ReasoningContent: "current reasoning fallback",
ToolCalls: []providers.ToolCall{{
ID: "call_1",
Name: "read_file",
ExtraContent: &providers.ExtraContent{
ToolFeedbackExplanation: "Read README.md first to confirm the current project structure.",
},
}},
}
messages := []providers.Message{
{Role: "user", Content: "check file"},
@ -1823,15 +1865,73 @@ func TestToolFeedbackExplanationFromResponse_FallsBackToReasoningContent(t *test
}
got := toolFeedbackExplanationFromResponse(response, messages, 300)
if got != "current reasoning fallback" {
t.Fatalf("toolFeedbackExplanationFromResponse() = %q, want reasoning fallback", got)
if got != "Read README.md first to confirm the current project structure." {
t.Fatalf("toolFeedbackExplanationFromResponse() = %q, want explicit tool feedback explanation", got)
}
}
func TestToolFeedbackExplanationFromResponse_UsesLatestUserContentAsLastResort(t *testing.T) {
func TestToolFeedbackExplanationForToolCall_PrefersToolSpecificExtraContent(t *testing.T) {
response := &providers.LLMResponse{
Content: "Shared explanation",
ToolCalls: []providers.ToolCall{
{
ID: "call_1",
Name: "read_file",
ExtraContent: &providers.ExtraContent{
ToolFeedbackExplanation: "Read README.md first.",
},
},
{
ID: "call_2",
Name: "edit_file",
ExtraContent: &providers.ExtraContent{
ToolFeedbackExplanation: "Update config example after reading it.",
},
},
},
}
got1 := toolFeedbackExplanationForToolCall(response, response.ToolCalls[0], nil, 300)
got2 := toolFeedbackExplanationForToolCall(response, response.ToolCalls[1], nil, 300)
if got1 != "Read README.md first." {
t.Fatalf("toolFeedbackExplanationForToolCall() first = %q, want tool-specific explanation", got1)
}
if got2 != "Update config example after reading it." {
t.Fatalf("toolFeedbackExplanationForToolCall() second = %q, want tool-specific explanation", got2)
}
}
func TestToolFeedbackExplanationForToolCall_DoesNotReuseAnotherToolCallExplanation(t *testing.T) {
response := &providers.LLMResponse{
ToolCalls: []providers.ToolCall{
{
ID: "call_1",
Name: "read_file",
},
{
ID: "call_2",
Name: "edit_file",
ExtraContent: &providers.ExtraContent{
ToolFeedbackExplanation: "Update config example after reading it.",
},
},
},
}
messages := []providers.Message{
{Role: "user", Content: "inspect the config and update the example"},
}
got := toolFeedbackExplanationForToolCall(response, response.ToolCalls[0], messages, 300)
want := utils.ToolFeedbackContinuationHint + ": inspect the config and update the example"
if got != want {
t.Fatalf("toolFeedbackExplanationForToolCall() = %q, want %q", got, want)
}
}
func TestToolFeedbackExplanationFromResponse_DoesNotUseReasoningContent(t *testing.T) {
response := &providers.LLMResponse{
Content: "",
ReasoningContent: "",
ReasoningContent: "hidden reasoning should not be shown",
}
messages := []providers.Message{
{Role: "user", Content: "check file"},
@ -3770,7 +3870,7 @@ func TestProcessMessage_PublishesToolFeedbackWhenEnabled(t *testing.T) {
}
}
func TestProcessMessage_PublishesToolFeedbackFromReasoningContent(t *testing.T) {
func TestProcessMessage_DoesNotLeakReasoningContentInToolFeedback(t *testing.T) {
tmpDir := t.TempDir()
heartbeatFile := filepath.Join(tmpDir, "tool-feedback-reasoning.txt")
if err := os.WriteFile(heartbeatFile, []byte("tool feedback task"), 0o644); err != nil {
@ -3819,11 +3919,17 @@ func TestProcessMessage_PublishesToolFeedbackFromReasoningContent(t *testing.T)
if !strings.Contains(outbound.Content, "`read_file`") {
t.Fatalf("tool feedback content = %q, want read_file summary", outbound.Content)
}
if !strings.Contains(outbound.Content, "Read README.md first") {
t.Fatalf("tool feedback content = %q, want reasoning fallback", 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, "Read README.md first") {
t.Fatalf("tool feedback content = %q, should not leak hidden reasoning", outbound.Content)
}
case <-time.After(2 * time.Second):
t.Fatal("expected outbound tool feedback for reasoning fallback")
t.Fatal("expected outbound tool feedback without leaking reasoning")
}
}

126
pkg/channels/discord/discord.go
View file

@ -49,6 +49,8 @@ type DiscordChannel struct {
botUserID string // stored for mention checking
bus *bus.MessageBus
tts tts.TTSProvider
playTTSFn func(context.Context, *discordgo.VoiceConnection, string, uint64)
ttsVoiceFn func(string) (*discordgo.VoiceConnection, bool)
voiceMu sync.RWMutex
voiceSSRC map[string]map[uint32]string // guildID -> ssrc -> userID
@ -95,6 +97,8 @@ func NewDiscordChannel(
bus: bus,
voiceSSRC: make(map[string]map[uint32]string),
}
ch.playTTSFn = ch.playTTS
ch.ttsVoiceFn = ch.voiceConnectionForTTS
ch.progress = channels.NewToolFeedbackAnimator(ch.EditMessage)
return ch, nil
}
@ -180,26 +184,12 @@ func (c *DiscordChannel) Send(ctx context.Context, msg bus.OutboundMessage) ([]s
}
c.ClearToolFeedbackMessage(channelID)
}
} else {
c.DismissToolFeedbackMessage(ctx, channelID)
}
if c.tts != nil && !isToolFeedback {
if ch, err := c.session.State.Channel(channelID); err == nil && ch.GuildID != "" {
if vc, ok := c.session.VoiceConnections[ch.GuildID]; ok && vc != nil {
// Cancel any previous TTS playback
c.ttsMu.Lock()
if c.cancelTTS != nil {
c.cancelTTS()
}
ttsCtx, ttsCancel := context.WithCancel(c.ctx)
c.ttsPlayID++
playID := c.ttsPlayID
c.cancelTTS = ttsCancel
c.ttsMu.Unlock()
go c.playTTS(ttsCtx, vc, msg.Content, playID)
}
trackedMsgID, hasTrackedMsg := c.currentToolFeedbackMessage(channelID)
c.maybeStartTTS(channelID, msg.Content, isToolFeedback)
if !isToolFeedback {
if msgIDs, handled := c.FinalizeToolFeedbackMessage(ctx, msg); handled {
return msgIDs, nil
}
}
@ -213,12 +203,61 @@ func (c *DiscordChannel) Send(ctx context.Context, msg bus.OutboundMessage) ([]s
}
if isToolFeedback {
c.RecordToolFeedbackMessage(channelID, msgID, msg.Content)
} else {
c.ClearToolFeedbackMessage(channelID)
} else if hasTrackedMsg {
c.dismissTrackedToolFeedbackMessage(ctx, channelID, trackedMsgID)
}
return []string{msgID}, nil
}
func (c *DiscordChannel) maybeStartTTS(channelID, content string, isToolFeedback bool) {
if c.tts == nil || isToolFeedback {
return
}
voiceFn := c.ttsVoiceFn
if voiceFn == nil {
voiceFn = c.voiceConnectionForTTS
}
vc, ok := voiceFn(channelID)
if !ok || vc == nil {
return
}
// Cancel any previous TTS playback.
c.ttsMu.Lock()
if c.cancelTTS != nil {
c.cancelTTS()
}
ttsCtx, ttsCancel := context.WithCancel(c.ctx)
c.ttsPlayID++
playID := c.ttsPlayID
c.cancelTTS = ttsCancel
playFn := c.playTTSFn
c.ttsMu.Unlock()
if playFn == nil {
playFn = c.playTTS
}
go playFn(ttsCtx, vc, content, playID)
}
func (c *DiscordChannel) voiceConnectionForTTS(channelID string) (*discordgo.VoiceConnection, bool) {
if c.session == nil || c.session.State == nil {
return nil, false
}
ch, err := c.session.State.Channel(channelID)
if err != nil || ch == nil || ch.GuildID == "" {
return nil, false
}
vc, ok := c.session.VoiceConnections[ch.GuildID]
if !ok || vc == nil {
return nil, false
}
return vc, true
}
// SendMedia implements the channels.MediaSender interface.
func (c *DiscordChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMessage) ([]string, error) {
if !c.IsRunning() {
@ -229,7 +268,7 @@ func (c *DiscordChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMes
if channelID == "" {
return nil, fmt.Errorf("channel ID is empty")
}
c.DismissToolFeedbackMessage(ctx, channelID)
trackedMsgID, hasTrackedMsg := c.currentToolFeedbackMessage(channelID)
store := c.GetMediaStore()
if store == nil {
@ -311,6 +350,9 @@ func (c *DiscordChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMes
if r.err != nil {
return nil, fmt.Errorf("discord send media: %w", channels.ErrTemporary)
}
if hasTrackedMsg {
c.dismissTrackedToolFeedbackMessage(ctx, channelID, trackedMsgID)
}
return []string{r.id}, nil
case <-sendCtx.Done():
// Close all file readers
@ -366,6 +408,13 @@ func (c *DiscordChannel) currentToolFeedbackMessage(chatID string) (string, bool
return c.progress.Current(chatID)
}
func (c *DiscordChannel) takeToolFeedbackMessage(chatID string) (string, string, bool) {
if c.progress == nil {
return "", "", false
}
return c.progress.Take(chatID)
}
func (c *DiscordChannel) RecordToolFeedbackMessage(chatID, messageID, content string) {
if c.progress == nil {
return
@ -385,8 +434,39 @@ func (c *DiscordChannel) DismissToolFeedbackMessage(ctx context.Context, chatID
if !ok {
return
}
c.dismissTrackedToolFeedbackMessage(ctx, chatID, msgID)
}
func (c *DiscordChannel) dismissTrackedToolFeedbackMessage(ctx context.Context, chatID, messageID string) {
if strings.TrimSpace(chatID) == "" || strings.TrimSpace(messageID) == "" {
return
}
c.ClearToolFeedbackMessage(chatID)
_ = c.DeleteMessage(ctx, chatID, msgID)
_ = c.DeleteMessage(ctx, chatID, messageID)
}
func (c *DiscordChannel) finalizeTrackedToolFeedbackMessage(
ctx context.Context,
chatID string,
content string,
editFn func(context.Context, string, string, string) error,
) ([]string, bool) {
msgID, baseContent, ok := c.takeToolFeedbackMessage(chatID)
if !ok || editFn == nil {
return nil, false
}
if err := editFn(ctx, chatID, msgID, content); err != nil {
c.RecordToolFeedbackMessage(chatID, msgID, baseContent)
return nil, false
}
return []string{msgID}, true
}
func (c *DiscordChannel) FinalizeToolFeedbackMessage(ctx context.Context, msg bus.OutboundMessage) ([]string, bool) {
if outboundMessageIsToolFeedback(msg) {
return nil, false
}
return c.finalizeTrackedToolFeedbackMessage(ctx, msg.ChatID, msg.Content, c.EditMessage)
}
func (c *DiscordChannel) sendChunk(ctx context.Context, channelID, content, replyToID string) (string, error) {

132
pkg/channels/discord/discord_test.go
View file

@ -9,12 +9,28 @@ import (
"reflect"
"sync"
"testing"
"time"
"github.com/bwmarrin/discordgo"
"github.com/sipeed/picoclaw/pkg/audio/tts"
"github.com/sipeed/picoclaw/pkg/bus"
"github.com/sipeed/picoclaw/pkg/channels"
)
type stubTTSProvider struct{}
func (stubTTSProvider) Name() string { return "stub-tts" }
func (stubTTSProvider) Synthesize(context.Context, string) (io.ReadCloser, error) {
return io.NopCloser(&noopReader{}), nil
}
type noopReader struct{}
func (*noopReader) Read(p []byte) (int, error) {
return 0, io.EOF
}
func TestApplyDiscordProxy_CustomProxy(t *testing.T) {
session, err := discordgo.New("Bot test-token")
if err != nil {
@ -109,11 +125,9 @@ func TestSend_NonToolFeedbackDeletesTrackedProgressMessage(t *testing.T) {
mu.Unlock()
switch {
case r.Method == http.MethodDelete && r.URL.Path == "/channels/chat-1/messages/prog-1":
w.WriteHeader(http.StatusNoContent)
case r.Method == http.MethodPost && r.URL.Path == "/channels/chat-1/messages":
case r.Method == http.MethodPatch && r.URL.Path == "/channels/chat-1/messages/prog-1":
w.Header().Set("Content-Type", "application/json")
_, _ = io.WriteString(w, `{"id":"final-1"}`)
_, _ = io.WriteString(w, `{"id":"prog-1"}`)
default:
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
}
@ -154,7 +168,7 @@ func TestSend_NonToolFeedbackDeletesTrackedProgressMessage(t *testing.T) {
if err != nil {
t.Fatalf("Send() error = %v", err)
}
if got, want := ids, []string{"final-1"}; !reflect.DeepEqual(got, want) {
if got, want := ids, []string{"prog-1"}; !reflect.DeepEqual(got, want) {
t.Fatalf("Send() ids = %v, want %v", got, want)
}
if _, ok := ch.currentToolFeedbackMessage("chat-1"); ok {
@ -164,10 +178,114 @@ func TestSend_NonToolFeedbackDeletesTrackedProgressMessage(t *testing.T) {
mu.Lock()
defer mu.Unlock()
wantRequests := []string{
"DELETE /channels/chat-1/messages/prog-1",
"POST /channels/chat-1/messages",
"PATCH /channels/chat-1/messages/prog-1",
}
if !reflect.DeepEqual(requests, wantRequests) {
t.Fatalf("requests = %v, want %v", requests, wantRequests)
}
}
func TestFinalizeTrackedToolFeedbackMessage_StopsTrackingBeforeEdit(t *testing.T) {
ch := &DiscordChannel{
progress: channels.NewToolFeedbackAnimator(nil),
}
ch.RecordToolFeedbackMessage("chat-1", "msg-1", "🔧 `read_file`")
msgIDs, handled := ch.finalizeTrackedToolFeedbackMessage(
context.Background(),
"chat-1",
"final reply",
func(_ context.Context, chatID, messageID, content string) error {
if _, ok := ch.currentToolFeedbackMessage(chatID); ok {
t.Fatal("expected tracked tool feedback to be stopped before edit")
}
if chatID != "chat-1" || messageID != "msg-1" || content != "final reply" {
t.Fatalf("unexpected edit args: %s %s %s", chatID, messageID, content)
}
return nil
},
)
if !handled {
t.Fatal("expected finalizeTrackedToolFeedbackMessage to handle tracked message")
}
if got, want := msgIDs, []string{"msg-1"}; !reflect.DeepEqual(got, want) {
t.Fatalf("finalizeTrackedToolFeedbackMessage() ids = %v, want %v", got, want)
}
}
func TestSend_NonToolFeedbackFinalizerStillStartsTTS(t *testing.T) {
var (
mu sync.Mutex
requests []string
)
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
mu.Lock()
requests = append(requests, r.Method+" "+r.URL.Path)
mu.Unlock()
switch {
case r.Method == http.MethodPatch && r.URL.Path == "/channels/chat-1/messages/prog-1":
w.Header().Set("Content-Type", "application/json")
_, _ = io.WriteString(w, `{"id":"prog-1"}`)
default:
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
}
}))
defer server.Close()
origChannels := discordgo.EndpointChannels
discordgo.EndpointChannels = server.URL + "/channels/"
defer func() {
discordgo.EndpointChannels = origChannels
}()
session, err := discordgo.New("Bot test-token")
if err != nil {
t.Fatalf("discordgo.New() error: %v", err)
}
session.Client = server.Client()
ttsStarted := make(chan string, 1)
ch := &DiscordChannel{
BaseChannel: channels.NewBaseChannel("discord", nil, bus.NewMessageBus(), nil),
session: session,
ctx: context.Background(),
typingStop: make(map[string]chan struct{}),
voiceSSRC: make(map[string]map[uint32]string),
tts: tts.TTSProvider(stubTTSProvider{}),
}
ch.ttsVoiceFn = func(string) (*discordgo.VoiceConnection, bool) {
return &discordgo.VoiceConnection{}, true
}
ch.playTTSFn = func(_ context.Context, _ *discordgo.VoiceConnection, text string, _ uint64) {
ttsStarted <- text
}
ch.progress = channels.NewToolFeedbackAnimator(ch.EditMessage)
ch.SetRunning(true)
ch.RecordToolFeedbackMessage("chat-1", "prog-1", "🔧 `read_file`")
ids, err := ch.Send(context.Background(), bus.OutboundMessage{
ChatID: "chat-1",
Content: "final reply",
Context: bus.InboundContext{
Channel: "discord",
ChatID: "chat-1",
},
})
if err != nil {
t.Fatalf("Send() error = %v", err)
}
if got, want := ids, []string{"prog-1"}; !reflect.DeepEqual(got, want) {
t.Fatalf("Send() ids = %v, want %v", got, want)
}
select {
case got := <-ttsStarted:
if got != "final reply" {
t.Fatalf("TTS content = %q, want final reply", got)
}
case <-time.After(2 * time.Second):
t.Fatal("expected TTS to start for finalized tracked tool feedback reply")
}
}

47
pkg/channels/feishu/feishu_64.go
View file

@ -156,6 +156,7 @@ func (c *FeishuChannel) Send(ctx context.Context, msg bus.OutboundMessage) ([]st
}
isToolFeedback := outboundMessageIsToolFeedback(msg)
trackedMsgID, hasTrackedMsg := c.currentToolFeedbackMessage(msg.ChatID)
if isToolFeedback {
animatedContent := channels.InitialAnimatedToolFeedbackContent(msg.Content)
if msgID, ok := c.currentToolFeedbackMessage(msg.ChatID); ok {
@ -169,7 +170,6 @@ func (c *FeishuChannel) Send(ctx context.Context, msg bus.OutboundMessage) ([]st
if msgIDs, handled := c.FinalizeToolFeedbackMessage(ctx, msg); handled {
return msgIDs, nil
}
c.DismissToolFeedbackMessage(ctx, msg.ChatID)
}
// Build interactive card with markdown content
@ -181,15 +181,14 @@ func (c *FeishuChannel) Send(ctx context.Context, msg bus.OutboundMessage) ([]st
if err != nil {
// If card build fails, fall back to plain text
msgID, sendErr := c.sendText(ctx, msg.ChatID, sendContent)
if sendErr == nil && isToolFeedback {
c.RecordToolFeedbackMessage(msg.ChatID, msgID, msg.Content)
}
if !isToolFeedback {
c.ClearToolFeedbackMessage(msg.ChatID)
}
if sendErr != nil {
return nil, sendErr
}
if isToolFeedback {
c.RecordToolFeedbackMessage(msg.ChatID, msgID, msg.Content)
} else if hasTrackedMsg {
c.dismissTrackedToolFeedbackMessage(ctx, msg.ChatID, trackedMsgID)
}
return []string{msgID}, nil
}
@ -198,8 +197,8 @@ func (c *FeishuChannel) Send(ctx context.Context, msg bus.OutboundMessage) ([]st
if err == nil {
if isToolFeedback {
c.RecordToolFeedbackMessage(msg.ChatID, msgID, msg.Content)
} else {
c.ClearToolFeedbackMessage(msg.ChatID)
} else if hasTrackedMsg {
c.dismissTrackedToolFeedbackMessage(ctx, msg.ChatID, trackedMsgID)
}
return []string{msgID}, nil
}
@ -220,8 +219,8 @@ func (c *FeishuChannel) Send(ctx context.Context, msg bus.OutboundMessage) ([]st
if textErr == nil {
if isToolFeedback {
c.RecordToolFeedbackMessage(msg.ChatID, msgID, msg.Content)
} else {
c.ClearToolFeedbackMessage(msg.ChatID)
} else if hasTrackedMsg {
c.dismissTrackedToolFeedbackMessage(ctx, msg.ChatID, trackedMsgID)
}
return []string{msgID}, nil
}
@ -329,6 +328,13 @@ func (c *FeishuChannel) currentToolFeedbackMessage(chatID string) (string, bool)
return c.progress.Current(chatID)
}
func (c *FeishuChannel) takeToolFeedbackMessage(chatID string) (string, string, bool) {
if c.progress == nil {
return "", "", false
}
return c.progress.Take(chatID)
}
func (c *FeishuChannel) RecordToolFeedbackMessage(chatID, messageID, content string) {
if c.progress == nil {
return
@ -348,8 +354,15 @@ func (c *FeishuChannel) DismissToolFeedbackMessage(ctx context.Context, chatID s
if !ok {
return
}
c.dismissTrackedToolFeedbackMessage(ctx, chatID, msgID)
}
func (c *FeishuChannel) dismissTrackedToolFeedbackMessage(ctx context.Context, chatID, messageID string) {
if strings.TrimSpace(chatID) == "" || strings.TrimSpace(messageID) == "" {
return
}
c.ClearToolFeedbackMessage(chatID)
_ = c.DeleteMessage(ctx, chatID, msgID)
_ = c.DeleteMessage(ctx, chatID, messageID)
}
func (c *FeishuChannel) finalizeTrackedToolFeedbackMessage(
@ -358,14 +371,14 @@ func (c *FeishuChannel) finalizeTrackedToolFeedbackMessage(
content string,
editFn func(context.Context, string, string, string) error,
) ([]string, bool) {
msgID, ok := c.currentToolFeedbackMessage(chatID)
msgID, baseContent, ok := c.takeToolFeedbackMessage(chatID)
if !ok || editFn == nil {
return nil, false
}
if err := editFn(ctx, chatID, msgID, content); err != nil {
c.RecordToolFeedbackMessage(chatID, msgID, baseContent)
return nil, false
}
c.ClearToolFeedbackMessage(chatID)
return []string{msgID}, true
}
@ -448,7 +461,7 @@ func (c *FeishuChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMess
if !c.IsRunning() {
return nil, channels.ErrNotRunning
}
c.DismissToolFeedbackMessage(ctx, msg.ChatID)
trackedMsgID, hasTrackedMsg := c.currentToolFeedbackMessage(msg.ChatID)
if msg.ChatID == "" {
return nil, fmt.Errorf("chat ID is empty: %w", channels.ErrSendFailed)
@ -465,6 +478,10 @@ func (c *FeishuChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMess
}
}
if hasTrackedMsg {
c.dismissTrackedToolFeedbackMessage(ctx, msg.ChatID, trackedMsgID)
}
return nil, nil
}

28
pkg/channels/feishu/feishu_64_test.go
View file

@ -312,6 +312,34 @@ func TestFinalizeTrackedToolFeedbackMessage_ClearAfterSuccessfulEdit(t *testing.
}
}
func TestFinalizeTrackedToolFeedbackMessage_StopsTrackingBeforeEdit(t *testing.T) {
ch := &FeishuChannel{
progress: channels.NewToolFeedbackAnimator(nil),
}
ch.RecordToolFeedbackMessage("chat-1", "msg-1", "🔧 `read_file`")
msgIDs, handled := ch.finalizeTrackedToolFeedbackMessage(
context.Background(),
"chat-1",
"final reply",
func(_ context.Context, chatID, messageID, content string) error {
if _, ok := ch.currentToolFeedbackMessage(chatID); ok {
t.Fatal("expected tracked tool feedback to be stopped before edit")
}
if chatID != "chat-1" || messageID != "msg-1" || content != "final reply" {
t.Fatalf("unexpected edit args: %s %s %s", chatID, messageID, content)
}
return nil
},
)
if !handled {
t.Fatal("expected finalizeTrackedToolFeedbackMessage to handle tracked message")
}
if len(msgIDs) != 1 || msgIDs[0] != "msg-1" {
t.Fatalf("unexpected msgIDs: %v", msgIDs)
}
}
func TestFinalizeTrackedToolFeedbackMessage_EditFailureKeepsTrackedMessage(t *testing.T) {
ch := &FeishuChannel{
progress: channels.NewToolFeedbackAnimator(nil),

51
pkg/channels/manager.go
View file

@ -138,6 +138,16 @@ func outboundMediaChatID(msg bus.OutboundMediaMessage) string {
return msg.ChatID
}
func dismissTrackedToolFeedbackMessage(ctx context.Context, ch Channel, chatID string) {
if cleaner, ok := ch.(toolFeedbackMessageCleaner); ok {
cleaner.DismissToolFeedbackMessage(ctx, chatID)
return
}
if tracker, ok := ch.(toolFeedbackMessageTracker); ok {
tracker.ClearToolFeedbackMessage(chatID)
}
}
// RecordPlaceholder registers a placeholder message for later editing.
// Implements PlaceholderRecorder.
func (m *Manager) RecordPlaceholder(channel, chatID, placeholderID string) {
@ -218,17 +228,13 @@ func (m *Manager) preSend(ctx context.Context, name string, msg bus.OutboundMess
}
}
if !outboundMessageIsToolFeedback(msg) {
isToolFeedback := outboundMessageIsToolFeedback(msg)
if !isToolFeedback {
if finalizer, ok := ch.(toolFeedbackMessageFinalizer); ok {
if msgIDs, handled := finalizer.FinalizeToolFeedbackMessage(ctx, msg); handled {
return msgIDs, true
}
}
if cleaner, ok := ch.(toolFeedbackMessageCleaner); ok {
cleaner.DismissToolFeedbackMessage(ctx, chatID)
} else if tracker, ok := ch.(toolFeedbackMessageTracker); ok {
tracker.ClearToolFeedbackMessage(chatID)
}
}
// 3. If a stream already finalized this message, delete the placeholder and skip send
@ -243,6 +249,9 @@ func (m *Manager) preSend(ctx context.Context, name string, msg bus.OutboundMess
}
}
}
if !isToolFeedback {
dismissTrackedToolFeedbackMessage(ctx, ch, chatID)
}
return nil, true
}
@ -251,12 +260,14 @@ func (m *Manager) preSend(ctx context.Context, name string, msg bus.OutboundMess
if entry, ok := v.(placeholderEntry); ok && entry.id != "" {
if editor, ok := ch.(MessageEditor); ok {
content := msg.Content
if outboundMessageIsToolFeedback(msg) {
if isToolFeedback {
content = InitialAnimatedToolFeedbackContent(msg.Content)
}
if err := editor.EditMessage(ctx, chatID, entry.id, content); err == nil {
if tracker, ok := ch.(toolFeedbackMessageTracker); ok && outboundMessageIsToolFeedback(msg) {
if tracker, ok := ch.(toolFeedbackMessageTracker); ok && isToolFeedback {
tracker.RecordToolFeedbackMessage(chatID, entry.id, msg.Content)
} else if !isToolFeedback {
dismissTrackedToolFeedbackMessage(ctx, ch, chatID)
}
return []string{entry.id}, true
}
@ -276,12 +287,6 @@ func (m *Manager) preSendMedia(ctx context.Context, name string, msg bus.Outboun
chatID := outboundMediaChatID(msg)
key := name + ":" + chatID
if cleaner, ok := ch.(toolFeedbackMessageCleaner); ok {
cleaner.DismissToolFeedbackMessage(ctx, chatID)
} else if tracker, ok := ch.(toolFeedbackMessageTracker); ok {
tracker.ClearToolFeedbackMessage(chatID)
}
// 1. Stop typing
if v, loaded := m.typingStops.LoadAndDelete(key); loaded {
if entry, ok := v.(typingEntry); ok {
@ -360,22 +365,27 @@ func (m *Manager) GetStreamer(ctx context.Context, channelName, chatID string) (
// Mark streamActive on Finalize so preSend knows to clean up the placeholder
key := channelName + ":" + chatID
return &finalizeHookStreamer{
Streamer: streamer,
onFinalize: func() { m.streamActive.Store(key, true) },
Streamer: streamer,
onFinalize: func(finalizeCtx context.Context) {
dismissTrackedToolFeedbackMessage(finalizeCtx, ch, chatID)
m.streamActive.Store(key, true)
},
}, true
}
// finalizeHookStreamer wraps a Streamer to run a hook on Finalize.
type finalizeHookStreamer struct {
Streamer
onFinalize func()
onFinalize func(context.Context)
}
func (s *finalizeHookStreamer) Finalize(ctx context.Context, content string) error {
if err := s.Streamer.Finalize(ctx, content); err != nil {
return err
}
s.onFinalize()
if s.onFinalize != nil {
s.onFinalize(ctx)
}
return nil
}
@ -817,8 +827,9 @@ func (m *Manager) runWorker(ctx context.Context, name string, w *channelWorker)
// Collect all message chunks to send
var chunks []string
// Step 1: Try marker-based splitting if enabled
if m.config != nil && m.config.Agents.Defaults.SplitOnMarker {
// Step 1: Try marker-based splitting if enabled.
// Tool feedback must stay a single message, so it skips marker splitting.
if m.config != nil && m.config.Agents.Defaults.SplitOnMarker && !outboundMessageIsToolFeedback(msg) {
if markerChunks := SplitByMarker(msg.Content); len(markerChunks) > 1 {
for _, chunk := range markerChunks {
chunkMsg := msg

160
pkg/channels/manager_test.go
View file

@ -13,6 +13,7 @@ import (
"golang.org/x/time/rate"
"github.com/sipeed/picoclaw/pkg/bus"
"github.com/sipeed/picoclaw/pkg/config"
"github.com/sipeed/picoclaw/pkg/utils"
)
@ -100,6 +101,33 @@ func (m *mockDeletingMediaChannel) DismissToolFeedbackMessage(_ context.Context,
m.dismissedChatID = chatID
}
type mockStreamer struct {
finalizeFn func(context.Context, string) error
}
func (m *mockStreamer) Update(context.Context, string) error { return nil }
func (m *mockStreamer) Finalize(ctx context.Context, content string) error {
if m.finalizeFn != nil {
return m.finalizeFn(ctx, content)
}
return nil
}
func (m *mockStreamer) Cancel(context.Context) {}
type mockStreamingChannel struct {
mockMessageEditor
streamer Streamer
}
func (m *mockStreamingChannel) BeginStream(context.Context, string) (Streamer, error) {
if m.streamer == nil {
return nil, errors.New("missing streamer")
}
return m.streamer, nil
}
// newTestManager creates a minimal Manager suitable for unit tests.
func newTestManager() *Manager {
return &Manager{
@ -835,7 +863,7 @@ func TestPreSend_ToolFeedbackPlaceholderEditRecordsTrackedMessage(t *testing.T)
}
}
func TestPreSend_NonToolFeedbackDismissesTrackedMessage(t *testing.T) {
func TestPreSend_NonToolFeedbackLeavesTrackedMessageForChannelSend(t *testing.T) {
m := newTestManager()
ch := &mockMessageEditor{}
@ -853,8 +881,8 @@ func TestPreSend_NonToolFeedbackDismissesTrackedMessage(t *testing.T) {
if edited {
t.Fatal("expected preSend to fall through when no placeholder exists")
}
if ch.dismissedChatID != "123" {
t.Fatalf("expected tracked tool feedback to be dismissed for chat 123, got %q", ch.dismissedChatID)
if ch.dismissedChatID != "" {
t.Fatalf("expected tracked tool feedback cleanup to be deferred to channel send, got %q", ch.dismissedChatID)
}
}
@ -891,7 +919,7 @@ func TestPreSend_NonToolFeedbackFinalizerHandlesMessage(t *testing.T) {
}
}
func TestPreSendMedia_DismissesTrackedMessage(t *testing.T) {
func TestPreSendMedia_LeavesTrackedMessageForChannelSend(t *testing.T) {
m := newTestManager()
ch := &mockDeletingMediaChannel{}
@ -903,8 +931,8 @@ func TestPreSendMedia_DismissesTrackedMessage(t *testing.T) {
},
}, ch)
if ch.dismissedChatID != "123" {
t.Fatalf("expected tracked tool feedback to be dismissed for media chat 123, got %q", ch.dismissedChatID)
if ch.dismissedChatID != "" {
t.Fatalf("expected tracked tool feedback cleanup to be deferred to channel media send, got %q", ch.dismissedChatID)
}
}
@ -932,6 +960,126 @@ func TestSplitOutboundMessageContent_ToolFeedbackTruncatesInsteadOfSplitting(t *
}
}
func TestGetStreamer_FinalizeDismissesTrackedToolFeedback(t *testing.T) {
m := newTestManager()
ch := &mockStreamingChannel{
mockMessageEditor: mockMessageEditor{},
streamer: &mockStreamer{
finalizeFn: func(_ context.Context, content string) error {
if content != "final reply" {
t.Fatalf("unexpected finalize content: %q", content)
}
return nil
},
},
}
m.channels["test"] = ch
streamer, ok := m.GetStreamer(context.Background(), "test", "123")
if !ok {
t.Fatal("expected streamer to be available")
}
if err := streamer.Finalize(context.Background(), "final reply"); err != nil {
t.Fatalf("Finalize() error = %v", err)
}
if ch.dismissedChatID != "123" {
t.Fatalf("expected tracked tool feedback to be dismissed for chat 123, got %q", ch.dismissedChatID)
}
if _, ok := m.streamActive.Load("test:123"); !ok {
t.Fatal("expected streamActive marker to be recorded after finalize")
}
}
func TestGetStreamer_FinalizeFailureDoesNotDismissTrackedToolFeedback(t *testing.T) {
m := newTestManager()
ch := &mockStreamingChannel{
mockMessageEditor: mockMessageEditor{},
streamer: &mockStreamer{
finalizeFn: func(context.Context, string) error {
return errors.New("finalize failed")
},
},
}
m.channels["test"] = ch
streamer, ok := m.GetStreamer(context.Background(), "test", "123")
if !ok {
t.Fatal("expected streamer to be available")
}
if err := streamer.Finalize(context.Background(), "final reply"); err == nil {
t.Fatal("expected Finalize() to fail")
}
if ch.dismissedChatID != "" {
t.Fatalf("expected no tool feedback dismissal on finalize failure, got %q", ch.dismissedChatID)
}
if _, ok := m.streamActive.Load("test:123"); ok {
t.Fatal("expected no streamActive marker after finalize failure")
}
}
func TestRunWorker_ToolFeedbackSkipsMarkerSplitting(t *testing.T) {
m := newTestManager()
m.config = &config.Config{
Agents: config.AgentsConfig{
Defaults: config.AgentDefaults{
SplitOnMarker: true,
},
},
}
var (
mu sync.Mutex
received []string
)
ch := &mockChannelWithLength{
mockChannel: mockChannel{
sendFn: func(_ context.Context, msg bus.OutboundMessage) error {
mu.Lock()
received = append(received, msg.Content)
mu.Unlock()
return nil
},
},
maxLen: 200,
}
w := &channelWorker{
ch: ch,
queue: make(chan bus.OutboundMessage, 1),
done: make(chan struct{}),
limiter: rate.NewLimiter(rate.Inf, 1),
}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go m.runWorker(ctx, "test", w)
content := "🔧 `read_file`\nRead current config first.<|[SPLIT]|>Then update the example."
w.queue <- testOutboundMessage(bus.OutboundMessage{
Channel: "test",
ChatID: "123",
Content: content,
Context: bus.InboundContext{
Channel: "test",
ChatID: "123",
Raw: map[string]string{
"message_kind": "tool_feedback",
},
},
})
time.Sleep(100 * time.Millisecond)
mu.Lock()
defer mu.Unlock()
if len(received) != 1 {
t.Fatalf("len(received) = %d, want 1", len(received))
}
if received[0] != content {
t.Fatalf("received[0] = %q, want %q", received[0], content)
}
}
func TestPreSend_PlaceholderEditFails_FallsThrough(t *testing.T) {
m := newTestManager()

6
pkg/channels/telegram/command_registration.go
View file

@ -66,6 +66,10 @@ func (c *TelegramChannel) startCommandRegistration(ctx context.Context, defs []c
if register == nil {
register = c.RegisterCommands
}
delayFn := c.commandRegDelayFn
if delayFn == nil {
delayFn = commandRegistrationDelay
}
regCtx, cancel := context.WithCancel(ctx)
c.commandRegCancel = cancel
@ -91,7 +95,7 @@ func (c *TelegramChannel) startCommandRegistration(ctx context.Context, defs []c
return
}
delay := commandRegistrationDelay(attempt)
delay := delayFn(attempt)
logger.WarnCF("telegram", "Telegram command registration failed; will retry", map[string]any{
"error": err.Error(),
"retry_after": delay.String(),

16
pkg/channels/telegram/command_registration_test.go
View file

@ -31,14 +31,12 @@ func TestStartCommandRegistration_DoesNotBlock(t *testing.T) {
}
func TestStartCommandRegistration_RetriesUntilSuccessThenStops(t *testing.T) {
ch := &TelegramChannel{}
ch := &TelegramChannel{
commandRegDelayFn: func(int) time.Duration { return 5 * time.Millisecond },
}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
origBackoff := commandRegistrationBackoff
commandRegistrationBackoff = []time.Duration{5 * time.Millisecond}
defer func() { commandRegistrationBackoff = origBackoff }()
var attempts atomic.Int32
ch.registerFunc = func(context.Context, []commands.Definition) error {
n := attempts.Add(1)
@ -69,12 +67,10 @@ func TestStartCommandRegistration_RetriesUntilSuccessThenStops(t *testing.T) {
}
func TestStartCommandRegistration_StopsAfterCancel(t *testing.T) {
ch := &TelegramChannel{}
ch := &TelegramChannel{
commandRegDelayFn: func(int) time.Duration { return 5 * time.Millisecond },
}
ctx, cancel := context.WithCancel(context.Background())
origBackoff := commandRegistrationBackoff
commandRegistrationBackoff = []time.Duration{5 * time.Millisecond}
defer func() { commandRegistrationBackoff = origBackoff }()
defer cancel()
var attempts atomic.Int32

113
pkg/channels/telegram/telegram.go
View file

@ -54,8 +54,9 @@ type TelegramChannel struct {
tgCfg *config.TelegramSettings
progress *channels.ToolFeedbackAnimator
registerFunc func(context.Context, []commands.Definition) error
commandRegCancel context.CancelFunc
registerFunc func(context.Context, []commands.Definition) error
commandRegDelayFn func(int) time.Duration
commandRegCancel context.CancelFunc
}
func NewTelegramChannel(
@ -198,17 +199,25 @@ func (c *TelegramChannel) Send(ctx context.Context, msg bus.OutboundMessage) ([]
}
isToolFeedback := outboundMessageIsToolFeedback(msg)
toolFeedbackContent := msg.Content
if isToolFeedback {
animatedContent := channels.InitialAnimatedToolFeedbackContent(msg.Content)
toolFeedbackContent = fitToolFeedbackForTelegram(msg.Content, useMarkdownV2, 4096)
}
if isToolFeedback {
animatedContent := channels.InitialAnimatedToolFeedbackContent(toolFeedbackContent)
if msgID, ok := c.currentToolFeedbackMessage(msg.ChatID); ok {
if err := c.EditMessage(ctx, msg.ChatID, msgID, animatedContent); err == nil {
c.RecordToolFeedbackMessage(msg.ChatID, msgID, msg.Content)
c.RecordToolFeedbackMessage(msg.ChatID, msgID, toolFeedbackContent)
return []string{msgID}, nil
}
c.ClearToolFeedbackMessage(msg.ChatID)
}
} else {
c.DismissToolFeedbackMessage(ctx, msg.ChatID)
}
trackedMsgID, hasTrackedMsg := c.currentToolFeedbackMessage(msg.ChatID)
if !isToolFeedback {
if msgIDs, handled := c.FinalizeToolFeedbackMessage(ctx, msg); handled {
return msgIDs, nil
}
}
// The Manager already splits messages to ≤4000 chars (WithMaxMessageLength),
@ -218,7 +227,7 @@ func (c *TelegramChannel) Send(ctx context.Context, msg bus.OutboundMessage) ([]
var messageIDs []string
queue := []string{msg.Content}
if isToolFeedback {
queue = []string{channels.InitialAnimatedToolFeedbackContent(msg.Content)}
queue = []string{channels.InitialAnimatedToolFeedbackContent(toolFeedbackContent)}
}
for len(queue) > 0 {
chunk := queue[0]
@ -227,6 +236,13 @@ func (c *TelegramChannel) Send(ctx context.Context, msg bus.OutboundMessage) ([]
content := parseContent(chunk, useMarkdownV2)
if len([]rune(content)) > 4096 {
if isToolFeedback {
fittedChunk := fitToolFeedbackForTelegram(chunk, useMarkdownV2, 4096)
if fittedChunk != "" && fittedChunk != chunk {
queue = append([]string{fittedChunk}, queue...)
continue
}
}
runeChunk := []rune(chunk)
ratio := float64(len(runeChunk)) / float64(len([]rune(content)))
smallerLen := int(float64(4096) * ratio * 0.95) // 5% safety margin
@ -294,9 +310,9 @@ func (c *TelegramChannel) Send(ctx context.Context, msg bus.OutboundMessage) ([]
}
if isToolFeedback && len(messageIDs) > 0 {
c.RecordToolFeedbackMessage(msg.ChatID, messageIDs[0], msg.Content)
} else if !isToolFeedback {
c.ClearToolFeedbackMessage(msg.ChatID)
c.RecordToolFeedbackMessage(msg.ChatID, messageIDs[0], toolFeedbackContent)
} else if !isToolFeedback && hasTrackedMsg {
c.dismissTrackedToolFeedbackMessage(ctx, msg.ChatID, trackedMsgID)
}
return messageIDs, nil
@ -480,6 +496,13 @@ func (c *TelegramChannel) currentToolFeedbackMessage(chatID string) (string, boo
return c.progress.Current(chatID)
}
func (c *TelegramChannel) takeToolFeedbackMessage(chatID string) (string, string, bool) {
if c.progress == nil {
return "", "", false
}
return c.progress.Take(chatID)
}
func (c *TelegramChannel) RecordToolFeedbackMessage(chatID, messageID, content string) {
if c.progress == nil {
return
@ -499,8 +522,39 @@ func (c *TelegramChannel) DismissToolFeedbackMessage(ctx context.Context, chatID
if !ok {
return
}
c.dismissTrackedToolFeedbackMessage(ctx, chatID, msgID)
}
func (c *TelegramChannel) dismissTrackedToolFeedbackMessage(ctx context.Context, chatID, messageID string) {
if strings.TrimSpace(chatID) == "" || strings.TrimSpace(messageID) == "" {
return
}
c.ClearToolFeedbackMessage(chatID)
_ = c.DeleteMessage(ctx, chatID, msgID)
_ = c.DeleteMessage(ctx, chatID, messageID)
}
func (c *TelegramChannel) finalizeTrackedToolFeedbackMessage(
ctx context.Context,
chatID string,
content string,
editFn func(context.Context, string, string, string) error,
) ([]string, bool) {
msgID, baseContent, ok := c.takeToolFeedbackMessage(chatID)
if !ok || editFn == nil {
return nil, false
}
if err := editFn(ctx, chatID, msgID, content); err != nil {
c.RecordToolFeedbackMessage(chatID, msgID, baseContent)
return nil, false
}
return []string{msgID}, true
}
func (c *TelegramChannel) FinalizeToolFeedbackMessage(ctx context.Context, msg bus.OutboundMessage) ([]string, bool) {
if outboundMessageIsToolFeedback(msg) {
return nil, false
}
return c.finalizeTrackedToolFeedbackMessage(ctx, msg.ChatID, msg.Content, c.EditMessage)
}
// SendPlaceholder implements channels.PlaceholderCapable.
@ -534,7 +588,7 @@ func (c *TelegramChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMe
if !c.IsRunning() {
return nil, channels.ErrNotRunning
}
c.DismissToolFeedbackMessage(ctx, msg.ChatID)
trackedMsgID, hasTrackedMsg := c.currentToolFeedbackMessage(msg.ChatID)
chatID, threadID, err := resolveTelegramOutboundTarget(msg.ChatID, &msg.Context)
if err != nil {
@ -643,6 +697,10 @@ func (c *TelegramChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMe
}
}
if hasTrackedMsg {
c.dismissTrackedToolFeedbackMessage(ctx, msg.ChatID, trackedMsgID)
}
return messageIDs, nil
}
@ -1014,6 +1072,37 @@ func parseContent(text string, useMarkdownV2 bool) string {
return markdownToTelegramHTML(text)
}
func fitToolFeedbackForTelegram(content string, useMarkdownV2 bool, maxParsedLen int) string {
content = strings.TrimSpace(content)
if content == "" || maxParsedLen <= 0 {
return ""
}
if len([]rune(parseContent(content, useMarkdownV2))) <= maxParsedLen {
return content
}
low := 1
high := len([]rune(content))
best := utils.Truncate(content, 1)
for low <= high {
mid := (low + high) / 2
candidate := utils.FitToolFeedbackMessage(content, mid)
if candidate == "" {
high = mid - 1
continue
}
if len([]rune(parseContent(candidate, useMarkdownV2))) <= maxParsedLen {
best = candidate
low = mid + 1
continue
}
high = mid - 1
}
return best
}
// parseTelegramChatID splits "chatID/threadID" into its components.
// Returns threadID=0 when no "/" is present (non-forum messages).
func parseTelegramChatID(chatID string) (int64, int, error) {

2
pkg/channels/telegram/telegram_group_command_filter_test.go
View file

@ -108,7 +108,7 @@ func TestHandleMessage_GroupMentionOnly_BotCommandEntity(t *testing.T) {
t.Fatalf("handleMessage error: %v", err)
}
ctx, cancel := context.WithTimeout(context.Background(), 200*time.Microsecond)
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
defer cancel()
select {
case <-ctx.Done():

64
pkg/channels/telegram/telegram_test.go
View file

@ -282,10 +282,8 @@ func TestSend_NonToolFeedbackDeletesTrackedProgressMessage(t *testing.T) {
caller := &stubCaller{
callFn: func(ctx context.Context, url string, data *ta.RequestData) (*ta.Response, error) {
switch {
case strings.Contains(url, "deleteMessage"):
return successBoolResponse(t), nil
case strings.Contains(url, "sendMessage"):
return successResponseWithMessageID(t, 2), nil
case strings.Contains(url, "editMessageText"):
return successResponseWithMessageID(t, 1), nil
default:
t.Fatalf("unexpected API call: %s", url)
return nil, nil
@ -301,14 +299,64 @@ func TestSend_NonToolFeedbackDeletesTrackedProgressMessage(t *testing.T) {
})
assert.NoError(t, err)
assert.Equal(t, []string{"2"}, ids)
require.Len(t, caller.calls, 2)
assert.Contains(t, caller.calls[0].URL, "deleteMessage")
assert.Contains(t, caller.calls[1].URL, "sendMessage")
assert.Equal(t, []string{"1"}, ids)
require.Len(t, caller.calls, 1)
assert.Contains(t, caller.calls[0].URL, "editMessageText")
_, ok := ch.currentToolFeedbackMessage("12345")
assert.False(t, ok, "tracked tool feedback should be cleared after final reply")
}
func TestFinalizeTrackedToolFeedbackMessage_StopsTrackingBeforeEdit(t *testing.T) {
ch := newTestChannel(t, &stubCaller{
callFn: func(context.Context, string, *ta.RequestData) (*ta.Response, error) {
t.Fatal("unexpected API call")
return nil, nil
},
})
ch.RecordToolFeedbackMessage("12345", "1", "🔧 `read_file`")
msgIDs, handled := ch.finalizeTrackedToolFeedbackMessage(
context.Background(),
"12345",
"final reply",
func(_ context.Context, chatID, messageID, content string) error {
_, ok := ch.currentToolFeedbackMessage(chatID)
assert.False(t, ok, "tracked tool feedback should be stopped before edit")
assert.Equal(t, "12345", chatID)
assert.Equal(t, "1", messageID)
assert.Equal(t, "final reply", content)
return nil
},
)
assert.True(t, handled)
assert.Equal(t, []string{"1"}, msgIDs)
}
func TestSend_ToolFeedbackStaysSingleMessageAfterHTMLExpansion(t *testing.T) {
caller := &stubCaller{
callFn: func(ctx context.Context, url string, data *ta.RequestData) (*ta.Response, error) {
return successResponse(t), nil
},
}
ch := newTestChannel(t, caller)
_, err := ch.Send(context.Background(), bus.OutboundMessage{
ChatID: "12345",
Content: "🔧 `read_file`\n" + strings.Repeat("<", 2000),
Context: bus.InboundContext{
Channel: "telegram",
ChatID: "12345",
Raw: map[string]string{
"message_kind": "tool_feedback",
},
},
})
assert.NoError(t, err)
assert.Len(t, caller.calls, 1, "tool feedback should stay a single Telegram message after HTML escaping")
}
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

31
pkg/channels/tool_feedback_animator.go
View file

@ -82,16 +82,22 @@ func (a *ToolFeedbackAnimator) Clear(chatID string) {
if a == nil || strings.TrimSpace(chatID) == "" {
return
}
var entry *toolFeedbackAnimationState
a.mu.Lock()
if old, ok := a.entries[chatID]; ok {
entry = old
delete(a.entries, chatID)
}
a.mu.Unlock()
entry := a.detach(chatID)
stopToolFeedbackAnimation(entry)
}
func (a *ToolFeedbackAnimator) Take(chatID string) (string, string, bool) {
if a == nil || strings.TrimSpace(chatID) == "" {
return "", "", false
}
entry := a.detach(chatID)
if entry == nil || strings.TrimSpace(entry.messageID) == "" {
return "", "", false
}
stopToolFeedbackAnimation(entry)
return entry.messageID, entry.baseContent, true
}
func (a *ToolFeedbackAnimator) StopAll() {
if a == nil {
return
@ -109,6 +115,17 @@ func (a *ToolFeedbackAnimator) StopAll() {
}
}
func (a *ToolFeedbackAnimator) detach(chatID string) *toolFeedbackAnimationState {
if a == nil || strings.TrimSpace(chatID) == "" {
return nil
}
a.mu.Lock()
defer a.mu.Unlock()
entry := a.entries[chatID]
delete(a.entries, chatID)
return entry
}
func (a *ToolFeedbackAnimator) run(chatID string, entry *toolFeedbackAnimationState) {
defer close(entry.done)

19
pkg/channels/tool_feedback_animator_test.go
View file

@ -42,3 +42,22 @@ func TestToolFeedbackAnimator_RecordCurrentAndClear(t *testing.T) {
t.Fatalf("Current() after Clear = (%q, %v), want (\"\", false)", msgID, ok)
}
}
func TestToolFeedbackAnimator_TakeStopsTrackingAndReturnsState(t *testing.T) {
animator := NewToolFeedbackAnimator(nil)
animator.Record("chat-1", "msg-1", "🔧 `read_file`\nChecking config")
msgID, baseContent, ok := animator.Take("chat-1")
if !ok {
t.Fatal("Take() = not found, want tracked message")
}
if msgID != "msg-1" {
t.Fatalf("Take() msgID = %q, want msg-1", msgID)
}
if baseContent != "🔧 `read_file`\nChecking config" {
t.Fatalf("Take() baseContent = %q", baseContent)
}
if _, ok := animator.Current("chat-1"); ok {
t.Fatal("expected tracked message to be removed after Take()")
}
}

14
pkg/config/defaults.go
View file

@ -462,13 +462,13 @@ func defaultChannels() ChannelsConfig {
"use_markdown_v2": false,
},
},
"feishu": map[string]any{},
"discord": map[string]any{
"placeholder": map[string]any{"enabled": true, "text": []string{"Thinking... 💭"}},
},
"maixcam": map[string]any{
"settings": map[string]any{"host": "0.0.0.0", "port": 18790},
},
"feishu": map[string]any{},
"discord": map[string]any{
"placeholder": map[string]any{"enabled": true, "text": []string{"Thinking... 💭"}},
},
"maixcam": map[string]any{
"settings": map[string]any{"host": "0.0.0.0", "port": 18790},
},
"qq": map[string]any{
"settings": map[string]any{"max_message_length": 2000},
},

82
pkg/providers/common/common.go
View file

@ -70,11 +70,23 @@ func NewHTTPClient(proxy string) *http.Client {
// It mirrors protocoltypes.Message but omits SystemParts, which is an
// internal field that would be unknown to third-party endpoints.
type openaiMessage struct {
Role string `json:"role"`
Content string `json:"content"`
ReasoningContent string `json:"reasoning_content,omitempty"`
ToolCalls []ToolCall `json:"tool_calls,omitempty"`
ToolCallID string `json:"tool_call_id,omitempty"`
Role string `json:"role"`
Content string `json:"content"`
ReasoningContent string `json:"reasoning_content,omitempty"`
ToolCalls []openaiToolCall `json:"tool_calls,omitempty"`
ToolCallID string `json:"tool_call_id,omitempty"`
}
type openaiToolCall struct {
ID string `json:"id"`
Type string `json:"type,omitempty"`
Function *openaiFunctionCall `json:"function,omitempty"`
}
type openaiFunctionCall struct {
Name string `json:"name"`
Arguments string `json:"arguments"`
ThoughtSignature string `json:"thought_signature,omitempty"`
}
// SerializeMessages converts internal Message structs to the OpenAI wire format.
@ -84,12 +96,13 @@ type openaiMessage struct {
func SerializeMessages(messages []Message) []any {
out := make([]any, 0, len(messages))
for _, m := range messages {
toolCalls := serializeToolCalls(m.ToolCalls)
if len(m.Media) == 0 {
out = append(out, openaiMessage{
Role: m.Role,
Content: m.Content,
ReasoningContent: m.ReasoningContent,
ToolCalls: m.ToolCalls,
ToolCalls: toolCalls,
ToolCallID: m.ToolCallID,
})
continue
@ -132,8 +145,8 @@ func SerializeMessages(messages []Message) []any {
if m.ToolCallID != "" {
msg["tool_call_id"] = m.ToolCallID
}
if len(m.ToolCalls) > 0 {
msg["tool_calls"] = m.ToolCalls
if len(toolCalls) > 0 {
msg["tool_calls"] = toolCalls
}
if m.ReasoningContent != "" {
msg["reasoning_content"] = m.ReasoningContent
@ -143,6 +156,44 @@ func SerializeMessages(messages []Message) []any {
return out
}
func serializeToolCalls(toolCalls []ToolCall) []openaiToolCall {
if len(toolCalls) == 0 {
return nil
}
out := make([]openaiToolCall, 0, len(toolCalls))
for _, tc := range toolCalls {
wireCall := openaiToolCall{
ID: tc.ID,
Type: tc.Type,
}
if tc.Function != nil {
wireCall.Function = &openaiFunctionCall{
Name: tc.Function.Name,
Arguments: tc.Function.Arguments,
ThoughtSignature: tc.Function.ThoughtSignature,
}
} else if tc.Name != "" || len(tc.Arguments) > 0 || tc.ThoughtSignature != "" {
argsJSON := "{}"
if len(tc.Arguments) > 0 {
if encoded, err := json.Marshal(tc.Arguments); err == nil {
argsJSON = string(encoded)
}
}
wireCall.Function = &openaiFunctionCall{
Name: tc.Name,
Arguments: argsJSON,
ThoughtSignature: tc.ThoughtSignature,
}
}
out = append(out, wireCall)
}
return out
}
func parseDataAudioURL(mediaURL string) (format, data string, ok bool) {
if !strings.HasPrefix(mediaURL, "data:audio/") {
return "", "", false
@ -185,6 +236,7 @@ func ParseResponse(body io.Reader) (*LLMResponse, error) {
Google *struct {
ThoughtSignature string `json:"thought_signature"`
} `json:"google"`
ToolFeedbackExplanation string `json:"tool_feedback_explanation"`
} `json:"extra_content"`
} `json:"tool_calls"`
} `json:"message"`
@ -228,11 +280,17 @@ func ParseResponse(body io.Reader) (*LLMResponse, error) {
ThoughtSignature: thoughtSignature,
}
if thoughtSignature != "" {
toolCall.ExtraContent = &ExtraContent{
Google: &GoogleExtra{
if tc.ExtraContent != nil {
extraContent := &ExtraContent{
ToolFeedbackExplanation: tc.ExtraContent.ToolFeedbackExplanation,
}
if thoughtSignature != "" {
extraContent.Google = &GoogleExtra{
ThoughtSignature: thoughtSignature,
},
}
}
if extraContent.Google != nil || strings.TrimSpace(extraContent.ToolFeedbackExplanation) != "" {
toolCall.ExtraContent = extraContent
}
}

58
pkg/providers/common/common_test.go
View file

@ -162,6 +162,43 @@ func TestSerializeMessages_StripsSystemParts(t *testing.T) {
}
}
func TestSerializeMessages_StripsInternalToolCallExtraContent(t *testing.T) {
messages := []Message{
{
Role: "assistant",
ToolCalls: []ToolCall{{
ID: "call_1",
Type: "function",
Function: &FunctionCall{
Name: "read_file",
Arguments: `{"path":"README.md"}`,
ThoughtSignature: "sig-1",
},
ExtraContent: &ExtraContent{
Google: &GoogleExtra{
ThoughtSignature: "sig-ignored-here",
},
ToolFeedbackExplanation: "Read README.md first.",
},
}},
},
}
result := SerializeMessages(messages)
data, err := json.Marshal(result)
if err != nil {
t.Fatalf("json.Marshal() error = %v", err)
}
payload := string(data)
if strings.Contains(payload, "extra_content") {
t.Fatalf("serialized payload should not include internal extra_content: %s", payload)
}
if !strings.Contains(payload, "thought_signature") {
t.Fatalf("serialized payload should preserve function thought_signature: %s", payload)
}
}
// --- ParseResponse tests ---
func TestParseResponse_BasicContent(t *testing.T) {
@ -234,6 +271,27 @@ func TestParseResponse_WithReasoningContent(t *testing.T) {
}
}
func TestParseResponse_WithToolFeedbackExplanationExtraContent(t *testing.T) {
body := `{"choices":[{"message":{"content":"","tool_calls":[{"id":"call_1","type":"function","function":{"name":"test_tool","arguments":"{}"},"extra_content":{"tool_feedback_explanation":"Check the current config before editing."}}]},"finish_reason":"tool_calls"}]}`
out, err := ParseResponse(strings.NewReader(body))
if err != nil {
t.Fatalf("ParseResponse() error = %v", err)
}
if len(out.ToolCalls) != 1 {
t.Fatalf("len(ToolCalls) = %d, want 1", len(out.ToolCalls))
}
if out.ToolCalls[0].ExtraContent == nil {
t.Fatal("ExtraContent is nil")
}
if out.ToolCalls[0].ExtraContent.ToolFeedbackExplanation != "Check the current config before editing." {
t.Fatalf(
"ToolFeedbackExplanation = %q, want %q",
out.ToolCalls[0].ExtraContent.ToolFeedbackExplanation,
"Check the current config before editing.",
)
}
}
func TestParseResponse_InvalidJSON(t *testing.T) {
_, err := ParseResponse(strings.NewReader("not json"))
if err == nil {

74
web/backend/api/session_test.go
View file

@ -691,6 +691,80 @@ func TestHandleGetSession_UsesConfiguredToolFeedbackMaxArgsLength(t *testing.T)
}
}
func TestHandleGetSession_DoesNotExposeLegacyToolArgumentsWhenExplanationMissing(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
if err := config.SaveConfig(configPath, cfg); 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-legacy-args"
if err := store.AddFullMessage(nil, sessionKey, providers.Message{Role: "user", Content: "check file"}); err != nil {
t.Fatalf("AddFullMessage(user) error = %v", err)
}
if 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,
},
}},
}); 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-legacy-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"`
}
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 at least 2", len(resp.Messages))
}
if !strings.Contains(resp.Messages[1].Content, "`read_file`") {
t.Fatalf("tool summary = %q, want read_file summary", resp.Messages[1].Content)
}
if resp.Messages[1].Content != "🔧 `read_file`" {
t.Fatalf("tool summary = %q, want tool name only when explanation is missing", resp.Messages[1].Content)
}
if strings.Contains(resp.Messages[1].Content, argsJSON) {
t.Fatalf("tool summary = %q, should not expose legacy args", resp.Messages[1].Content)
}
}
func TestHandleGetSession_IncludesMediaOnlyMessages(t *testing.T) {
configPath, cleanup := setupOAuthTestEnv(t)
defer cleanup()