From bfc37b784e31d531483163444a3bb1da9b9949dd Mon Sep 17 00:00:00 2001 From: lc6464 <64722907+lc6464@users.noreply.github.com> Date: Sun, 26 Apr 2026 19:43:25 +0800 Subject: [PATCH] fix(channels): bypass placeholder edits for thought and tool calls --- pkg/channels/manager.go | 14 ++++++ pkg/channels/manager_test.go | 98 ++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+) diff --git a/pkg/channels/manager.go b/pkg/channels/manager.go index 7974a39e4..d56c4fd9b 100644 --- a/pkg/channels/manager.go +++ b/pkg/channels/manager.go @@ -134,6 +134,14 @@ func outboundMessageIsToolFeedback(msg bus.OutboundMessage) bool { return strings.EqualFold(strings.TrimSpace(msg.Context.Raw["message_kind"]), "tool_feedback") } +func outboundMessageBypassesPlaceholderEdit(msg bus.OutboundMessage) bool { + if len(msg.Context.Raw) == 0 { + return false + } + kind := strings.TrimSpace(msg.Context.Raw["message_kind"]) + return strings.EqualFold(kind, "thought") || strings.EqualFold(kind, "tool_calls") +} + func outboundMediaChannel(msg bus.OutboundMediaMessage) string { return msg.Context.Channel } @@ -332,6 +340,12 @@ func (m *Manager) preSend(ctx context.Context, name string, msg bus.OutboundMess } return nil, false } + if outboundMessageBypassesPlaceholderEdit(msg) { + if deleter, ok := ch.(MessageDeleter); ok { + deleter.DeleteMessage(ctx, chatID, entry.id) // best effort + } + return nil, false + } if editor, ok := ch.(MessageEditor); ok { content := msg.Content trackedContent := msg.Content diff --git a/pkg/channels/manager_test.go b/pkg/channels/manager_test.go index a5d7c2838..8803134db 100644 --- a/pkg/channels/manager_test.go +++ b/pkg/channels/manager_test.go @@ -1131,6 +1131,104 @@ func TestPreSend_ToolFeedbackSeparateMessagesDeletesPlaceholderAndSkipsEdit(t *t } } +func TestPreSend_ThoughtPlaceholderDeleteAndSkipsEdit(t *testing.T) { + m := newTestManager() + + ch := &mockDeletingMessageEditor{ + mockMessageEditor: mockMessageEditor{ + editFn: func(_ context.Context, _, _, _ string) error { + t.Fatal("expected thought message to bypass placeholder edit") + return nil + }, + }, + } + + m.RecordPlaceholder("test", "123", "456") + + msg := testOutboundMessage(bus.OutboundMessage{ + Channel: "test", + ChatID: "123", + Content: "thinking trace", + Context: bus.InboundContext{ + Channel: "test", + ChatID: "123", + Raw: map[string]string{ + "message_kind": "thought", + }, + }, + }) + + msgIDs, handled := m.preSend(context.Background(), "test", msg, ch) + if handled { + t.Fatalf("expected thought message to fall through so the channel can send a structured message, got %v", msgIDs) + } + if ch.deleteCalls != 1 { + t.Fatalf("expected placeholder deletion, got %d delete calls", ch.deleteCalls) + } + if ch.deletedChatID != "123" || ch.deletedMessageID != "456" { + t.Fatalf("unexpected placeholder deletion target: %s/%s", ch.deletedChatID, ch.deletedMessageID) + } + if _, ok := m.placeholders.Load("test:123"); ok { + t.Fatal("expected placeholder to be consumed before structured thought send") + } +} + +func TestSendWithRetry_ToolCallsPlaceholderDeleteAndFallsThroughToSend(t *testing.T) { + m := newTestManager() + + ch := &mockDeletingMessageEditor{ + mockMessageEditor: mockMessageEditor{ + mockChannel: mockChannel{ + sendFn: func(_ context.Context, msg bus.OutboundMessage) error { + if got := msg.Context.Raw["message_kind"]; got != "tool_calls" { + t.Fatalf("expected tool_calls message kind, got %q", got) + } + if msg.Content != "" { + t.Fatalf("expected empty tool_calls content, got %q", msg.Content) + } + return nil + }, + }, + editFn: func(_ context.Context, _, _, _ string) error { + t.Fatal("expected tool_calls message to bypass placeholder edit") + return nil + }, + }, + } + + m.RecordPlaceholder("test", "123", "456") + + w := &channelWorker{ + ch: ch, + limiter: rate.NewLimiter(rate.Inf, 1), + } + + msg := testOutboundMessage(bus.OutboundMessage{ + Channel: "test", + ChatID: "123", + Context: bus.InboundContext{ + Channel: "test", + ChatID: "123", + Raw: map[string]string{ + "message_kind": "tool_calls", + "tool_calls": `[{"tool_feedback_explanation":"Looking up config","function":{"name":"read_file","arguments":"{}"}}]`, + }, + }, + }) + + m.sendWithRetry(context.Background(), "test", w, msg) + + if ch.deleteCalls != 1 { + t.Fatalf("expected placeholder deletion, got %d delete calls", ch.deleteCalls) + } + if ch.deletedChatID != "123" || ch.deletedMessageID != "456" { + t.Fatalf("unexpected placeholder deletion target: %s/%s", ch.deletedChatID, ch.deletedMessageID) + } + if len(ch.sentMessages) != 1 { + t.Fatalf("expected structured tool_calls message to be sent once, got %d", len(ch.sentMessages)) + } +} + func TestPreSend_NonToolFeedbackSeparateMessagesClearsTrackedMessageWithoutDismiss(t *testing.T) { m := newTestManager() m.config = &config.Config{