fix: prevent draft ghost bubbles and typing/reaction state leaks

Three related fixes for the draft message overwrite issue:

1. preSend: explicitly dismiss draft (SendDraft with empty text) before
   sending the permanent message. Previously relied on sendMessage to
   auto-replace the draft, which fails when a user message arrives in
   between, leaving a ghost bubble.

2. RecordTypingStop: call old stop() before storing the new entry.
   Without this, preSend for message A consumes message B's typing stop,
   killing B's indicator prematurely.

3. RecordReactionUndo: call old undo() before storing the new entry.
   Same issue as typing - prevents the previous cycle's preSend from
   consuming the next message's reaction undo.

Also clears statusEditTimes on draft dismiss to prevent stale throttle
state from affecting the next processing cycle.

https://claude.ai/code/session_01GeNd28Z7MtXyt2ejP4x2GW
This commit is contained in:
Claude 2026-03-03 11:03:31 +00:00
parent 51c5e6132e
commit 3eaf808582
2 changed files with 130 additions and 3 deletions

View file

@ -117,15 +117,35 @@ func (m *Manager) RecordPlaceholder(channel, chatID, placeholderID string) {
// RecordTypingStop registers a typing stop function for later invocation. // RecordTypingStop registers a typing stop function for later invocation.
// Implements PlaceholderRecorder. // Implements PlaceholderRecorder.
//
// If a previous typing indicator exists for the same chat, it is stopped
// immediately before the new one is recorded. This prevents the next
// preSend (which finalises the *previous* processing cycle) from
// consuming the *new* message's typing entry.
func (m *Manager) RecordTypingStop(channel, chatID string, stop func()) { func (m *Manager) RecordTypingStop(channel, chatID string, stop func()) {
key := channel + ":" + chatID key := channel + ":" + chatID
if v, loaded := m.typingStops.Load(key); loaded {
if entry, ok := v.(typingEntry); ok {
entry.stop() // idempotent
}
}
m.typingStops.Store(key, typingEntry{stop: stop, createdAt: time.Now()}) m.typingStops.Store(key, typingEntry{stop: stop, createdAt: time.Now()})
} }
// RecordReactionUndo registers a reaction undo function for later invocation. // RecordReactionUndo registers a reaction undo function for later invocation.
// Implements PlaceholderRecorder. // Implements PlaceholderRecorder.
//
// If a previous reaction exists for the same chat, it is undone immediately
// before the new one is recorded. Same rationale as RecordTypingStop: the
// old entry belongs to the previous processing cycle and must not leak into
// the next preSend call.
func (m *Manager) RecordReactionUndo(channel, chatID string, undo func()) { func (m *Manager) RecordReactionUndo(channel, chatID string, undo func()) {
key := channel + ":" + chatID key := channel + ":" + chatID
if v, loaded := m.reactionUndos.Load(key); loaded {
if entry, ok := v.(reactionEntry); ok {
entry.undo() // idempotent
}
}
m.reactionUndos.Store(key, reactionEntry{undo: undo, createdAt: time.Now()}) m.reactionUndos.Store(key, reactionEntry{undo: undo, createdAt: time.Now()})
} }
@ -149,12 +169,17 @@ 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).
// If the status was draft-based (draftID != 0), just clear the entry — // For draft-based entries, explicitly dismiss the draft bubble before
// the final sendMessage will automatically replace the draft bubble. // sending the permanent message. Without this, a user message sent
// between the last draft update and sendMessage may prevent the
// 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 {
// Draft-based: sendMessage replaces the draft, no edit needed if drafter, ok := ch.(DraftSender); ok {
_ = drafter.SendDraft(ctx, msg.ChatID, entry.draftID, "")
}
m.statusEditTimes.Delete(key)
} 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 {

View file

@ -1433,3 +1433,105 @@ func TestGenerateDraftID_Stable(t *testing.T) {
t.Fatalf("expected different draft IDs for different keys, both got %d", id1) t.Fatalf("expected different draft IDs for different keys, both got %d", id1)
} }
} }
// 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()
var dismissCalled bool
var dismissContent string
ch := &mockDraftSender{
mockChannel: mockChannel{
sendFn: func(_ context.Context, _ bus.OutboundMessage) error { return nil },
},
draftFn: func(_ context.Context, _ string, _ int, content string) error {
dismissCalled = true
dismissContent = content
return nil
},
editFn: func(_ context.Context, _, _, _ string) error { return nil },
sendWithID: func(_ context.Context, _, _ string) (string, error) { return "", nil },
}
// Store a draft-based status entry (simulates active streaming)
m.statusMsgIDs.Store("test:123", statusMsgEntry{draftID: 42, createdAt: time.Now()})
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 false for draft-based status")
}
if !dismissCalled {
t.Fatal("expected preSend to call SendDraft to dismiss the draft")
}
if dismissContent != "" {
t.Fatalf("expected empty dismiss content, got %q", dismissContent)
}
}
// TestRecordTypingStop_CleansUpOldEntry verifies that recording a new
// typing stop function calls the previous stop first.
func TestRecordTypingStop_CleansUpOldEntry(t *testing.T) {
m := newTestManager()
var oldStopped atomic.Bool
m.RecordTypingStop("tg", "42", func() { oldStopped.Store(true) })
// Record a new one — old stop should fire
m.RecordTypingStop("tg", "42", func() {})
if !oldStopped.Load() {
t.Fatal("expected old typing stop to be called when new entry is recorded")
}
}
// TestRecordReactionUndo_CleansUpOldEntry verifies that recording a new
// reaction undo function calls the previous undo first.
func TestRecordReactionUndo_CleansUpOldEntry(t *testing.T) {
m := newTestManager()
var oldUndone atomic.Bool
m.RecordReactionUndo("tg", "42", func() { oldUndone.Store(true) })
// Record a new one — old undo should fire
m.RecordReactionUndo("tg", "42", func() {})
if !oldUndone.Load() {
t.Fatal("expected old reaction undo to be called when new entry is recorded")
}
}
// TestPreSend_DraftDismiss_ClearsEditTimes verifies that dismissing a draft
// 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()
ch := &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 },
}
key := "test:123"
m.statusMsgIDs.Store(key, statusMsgEntry{draftID: 42, createdAt: time.Now()})
m.statusEditTimes.Store(key, time.Now())
msg := bus.OutboundMessage{Channel: "test", ChatID: "123", Content: "final"}
m.preSend(context.Background(), "test", msg, ch)
if _, loaded := m.statusEditTimes.Load(key); loaded {
t.Fatal("expected statusEditTimes to be cleared after draft dismiss")
}
}