fix: update draft with final content in preSend instead of dismissing

The draft path in preSend was dismissing drafts with empty text, leaving
ghost bubbles visible in Telegram. Now updates the draft with final content
(matching handleTaskStatusSend pattern), deletes orphan placeholders, and
cleans up placeholder map on messageID edit success.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
dj-oyu 2026-03-19 19:26:35 +09:00
parent d27fe8ec32
commit 594c805f47
4 changed files with 150 additions and 35 deletions

View file

@ -14,3 +14,8 @@ type MessageSenderWithID interface {
type DraftSender interface { type DraftSender interface {
SendDraft(ctx context.Context, chatID string, draftID int, content string) error SendDraft(ctx context.Context, chatID string, draftID int, content string) error
} }
// MessageDeleter — channels that can delete a previously sent message.
type MessageDeleter interface {
DeleteMessage(ctx context.Context, chatID string, messageID string) error
}

View file

@ -165,20 +165,32 @@ func (m *Manager) preSend(ctx context.Context, name string, msg bus.OutboundMess
} }
// 3. Try editing a tracked status message (from streaming preview). // 3. Try editing a tracked status message (from streaming preview).
// For draft-based entries, explicitly dismiss the draft bubble before // For draft-based entries, update the draft with the final content so
// sending the permanent message. Without this, a user message sent // the draft bubble becomes the permanent response. If a placeholder
// between the last draft update and sendMessage may prevent the // also exists, delete it to avoid orphan "Thinking…" messages.
// platform from auto-replacing the draft, leaving a ghost bubble.
if v, loaded := m.statusMsgIDs.LoadAndDelete(key); loaded { if v, loaded := m.statusMsgIDs.LoadAndDelete(key); loaded {
if entry, ok := v.(statusMsgEntry); ok { if entry, ok := v.(statusMsgEntry); ok {
if entry.draftID != 0 { if entry.draftID != 0 {
if drafter, ok := ch.(DraftSender); ok { if drafter, ok := ch.(DraftSender); ok {
_ = drafter.SendDraft(ctx, msg.ChatID, entry.draftID, "") if err := drafter.SendDraft(ctx, msg.ChatID, entry.draftID, msg.Content); err == nil {
m.statusEditTimes.Delete(key)
// Draft displays the final content; delete orphan placeholder.
if v, loaded := m.placeholders.LoadAndDelete(key); loaded {
if phEntry, ok := v.(placeholderEntry); ok && phEntry.id != "" {
if deleter, ok := ch.(MessageDeleter); ok {
_ = deleter.DeleteMessage(ctx, msg.ChatID, phEntry.id)
}
}
}
return true
}
} }
m.statusEditTimes.Delete(key) m.statusEditTimes.Delete(key)
// Draft update failed → fall through to placeholder path.
} else if entry.messageID != "" { } else if entry.messageID != "" {
if editor, ok := ch.(MessageEditor); ok { if editor, ok := ch.(MessageEditor); ok {
if err := editor.EditMessage(ctx, msg.ChatID, entry.messageID, msg.Content); err == nil { if err := editor.EditMessage(ctx, msg.ChatID, entry.messageID, msg.Content); err == nil {
m.placeholders.Delete(key)
return true // edited successfully, skip Send return true // edited successfully, skip Send
} }
} }

View file

@ -783,29 +783,22 @@ func TestGenerateDraftID_Stable(t *testing.T) {
} }
} }
// TestPreSend_DismissesDraftBeforeSend verifies that preSend explicitly // TestPreSend_DraftPath_UpdatesWithFinalContent verifies that preSend updates
// dismisses a draft-based status bubble (via SendDraft with empty text) // the draft bubble with the final content (instead of dismissing with empty text),
// before proceeding to send the permanent message. This prevents ghost // and returns true so no separate Send is performed.
// draft bubbles when a user message arrives between the last draft update func TestPreSend_DraftPath_UpdatesWithFinalContent(t *testing.T) {
// and the final sendMessage.
// TestPreSend_DismissesDraftBeforeSend verifies that preSend explicitly
// dismisses a draft-based status bubble (via SendDraft with empty text)
// before proceeding to send the permanent message. This prevents ghost
// draft bubbles when a user message arrives between the last draft update
// and the final sendMessage.
func TestPreSend_DismissesDraftBeforeSend(t *testing.T) {
m := newTestManager() m := newTestManager()
var dismissCalled bool var draftCalled bool
var dismissContent string var draftContent string
ch := &mockDraftSender{ ch := &mockDraftSender{
mockChannel: mockChannel{ mockChannel: mockChannel{
sendFn: func(_ context.Context, _ bus.OutboundMessage) error { return nil }, sendFn: func(_ context.Context, _ bus.OutboundMessage) error { return nil },
}, },
draftFn: func(_ context.Context, _ string, _ int, content string) error { draftFn: func(_ context.Context, _ string, _ int, content string) error {
dismissCalled = true draftCalled = true
dismissContent = content draftContent = content
return nil return nil
}, },
editFn: func(_ context.Context, _, _, _ string) error { return nil }, editFn: func(_ context.Context, _, _, _ string) error { return nil },
@ -817,14 +810,14 @@ func TestPreSend_DismissesDraftBeforeSend(t *testing.T) {
msg := bus.OutboundMessage{Channel: "test", ChatID: "123", Content: "final response"} msg := bus.OutboundMessage{Channel: "test", ChatID: "123", Content: "final response"}
edited := m.preSend(context.Background(), "test", msg, ch) edited := m.preSend(context.Background(), "test", msg, ch)
if edited { if !edited {
t.Fatal("expected preSend to return false for draft-based status") t.Fatal("expected preSend to return true when draft is updated with final content")
} }
if !dismissCalled { if !draftCalled {
t.Fatal("expected preSend to call SendDraft to dismiss the draft") t.Fatal("expected preSend to call SendDraft with final content")
} }
if dismissContent != "" { if draftContent != "final response" {
t.Fatalf("expected empty dismiss content, got %q", dismissContent) t.Fatalf("expected draft content 'final response', got %q", draftContent)
} }
} }
@ -864,13 +857,10 @@ func TestRecordReactionUndo_CleansUpOldEntry(t *testing.T) {
} }
} }
// TestPreSend_DraftDismiss_ClearsEditTimes verifies that dismissing a draft // TestPreSend_DraftUpdate_ClearsEditTimes verifies that updating a draft
// in preSend also clears the statusEditTimes entry for that key, preventing // with final content in preSend also clears the statusEditTimes entry for
// stale throttle state from affecting the next processing cycle. // that key, preventing stale throttle state from affecting the next cycle.
// TestPreSend_DraftDismiss_ClearsEditTimes verifies that dismissing a draft func TestPreSend_DraftUpdate_ClearsEditTimes(t *testing.T) {
// in preSend also clears the statusEditTimes entry for that key, preventing
// stale throttle state from affecting the next processing cycle.
func TestPreSend_DraftDismiss_ClearsEditTimes(t *testing.T) {
m := newTestManager() m := newTestManager()
ch := &mockDraftSender{ ch := &mockDraftSender{
@ -887,9 +877,94 @@ func TestPreSend_DraftDismiss_ClearsEditTimes(t *testing.T) {
m.statusEditTimes.Store(key, time.Now()) m.statusEditTimes.Store(key, time.Now())
msg := bus.OutboundMessage{Channel: "test", ChatID: "123", Content: "final"} msg := bus.OutboundMessage{Channel: "test", ChatID: "123", Content: "final"}
m.preSend(context.Background(), "test", msg, ch) edited := m.preSend(context.Background(), "test", msg, ch)
if !edited {
t.Fatal("expected preSend to return true when draft updated")
}
if _, loaded := m.statusEditTimes.Load(key); loaded { if _, loaded := m.statusEditTimes.Load(key); loaded {
t.Fatal("expected statusEditTimes to be cleared after draft dismiss") t.Fatal("expected statusEditTimes to be cleared after draft update")
}
}
// mockDraftSenderWithDelete implements DraftSender + MessageDeleter + MessageEditor.
type mockDraftSenderWithDelete struct {
mockDraftSender
deleteFn func(ctx context.Context, chatID, messageID string) error
}
func (m *mockDraftSenderWithDelete) DeleteMessage(ctx context.Context, chatID, messageID string) error {
return m.deleteFn(ctx, chatID, messageID)
}
// TestPreSend_DraftPath_DeletesPlaceholder verifies that when the draft path
// succeeds, any orphan placeholder message is deleted via DeleteMessage.
func TestPreSend_DraftPath_DeletesPlaceholder(t *testing.T) {
m := newTestManager()
var deleteCalled bool
var deletedMsgID string
ch := &mockDraftSenderWithDelete{
mockDraftSender: mockDraftSender{
mockChannel: mockChannel{
sendFn: func(_ context.Context, _ bus.OutboundMessage) error { return nil },
},
draftFn: func(_ context.Context, _ string, _ int, _ string) error { return nil },
editFn: func(_ context.Context, _, _, _ string) error { return nil },
sendWithID: func(_ context.Context, _, _ string) (string, error) { return "", nil },
},
deleteFn: func(_ context.Context, _, messageID string) error {
deleteCalled = true
deletedMsgID = messageID
return nil
},
}
key := "test:123"
m.statusMsgIDs.Store(key, statusMsgEntry{draftID: 42, createdAt: time.Now()})
m.RecordPlaceholder("test", "123", "ph-99")
msg := bus.OutboundMessage{Channel: "test", ChatID: "123", Content: "final"}
edited := m.preSend(context.Background(), "test", msg, ch)
if !edited {
t.Fatal("expected preSend to return true")
}
if !deleteCalled {
t.Fatal("expected DeleteMessage to be called for orphan placeholder")
}
if deletedMsgID != "ph-99" {
t.Fatalf("expected deleted message ID ph-99, got %s", deletedMsgID)
}
if _, loaded := m.placeholders.Load(key); loaded {
t.Fatal("expected placeholder to be removed from map")
}
}
// TestPreSend_MessageIDPath_CleansPlaceholder verifies that when the messageID
// edit path succeeds, the placeholder map entry is also cleaned up.
func TestPreSend_MessageIDPath_CleansPlaceholder(t *testing.T) {
m := newTestManager()
ch := &mockMessageEditor{
mockChannel: mockChannel{
sendFn: func(_ context.Context, _ bus.OutboundMessage) error { return nil },
},
editFn: func(_ context.Context, _, _, _ string) error { return nil },
}
key := "test:123"
m.statusMsgIDs.Store(key, statusMsgEntry{messageID: "status-77", createdAt: time.Now()})
m.RecordPlaceholder("test", "123", "ph-leftover")
msg := bus.OutboundMessage{Channel: "test", ChatID: "123", Content: "final response"}
edited := m.preSend(context.Background(), "test", msg, ch)
if !edited {
t.Fatal("expected preSend to return true (status message edited)")
}
if _, loaded := m.placeholders.Load(key); loaded {
t.Fatal("expected placeholder to be cleaned up after messageID edit")
} }
} }

View file

@ -71,6 +71,29 @@ func (c *TelegramChannel) SendDraft(ctx context.Context, chatID string, draftID
return nil return nil
} }
// DeleteMessage implements channels.MessageDeleter.
// It deletes a previously sent message by its platform message ID.
func (c *TelegramChannel) DeleteMessage(ctx context.Context, chatID string, messageID string) error {
if !c.IsRunning() {
return channels.ErrNotRunning
}
cid, _, err := parseTelegramChatID(chatID)
if err != nil {
return fmt.Errorf("invalid chat ID %s: %w", chatID, channels.ErrSendFailed)
}
var mid int
if _, scanErr := fmt.Sscanf(messageID, "%d", &mid); scanErr != nil {
return fmt.Errorf("invalid message ID %s: %w", messageID, channels.ErrSendFailed)
}
return c.bot.DeleteMessage(ctx, &telego.DeleteMessageParams{
ChatID: telego.ChatID{ID: cid},
MessageID: mid,
})
}
// formatChatID formats a chat ID with optional thread ID as "chatID/threadID". // formatChatID formats a chat ID with optional thread ID as "chatID/threadID".
func formatChatID(chatID int64, threadID int) string { func formatChatID(chatID int64, threadID int) string {
if threadID != 0 { if threadID != 0 {