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:
parent
01222a2851
commit
5a9e9ebaf1
2 changed files with 130 additions and 3 deletions
|
|
@ -117,15 +117,35 @@ func (m *Manager) RecordPlaceholder(channel, chatID, placeholderID string) {
|
|||
|
||||
// RecordTypingStop registers a typing stop function for later invocation.
|
||||
// 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()) {
|
||||
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()})
|
||||
}
|
||||
|
||||
// RecordReactionUndo registers a reaction undo function for later invocation.
|
||||
// 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()) {
|
||||
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()})
|
||||
}
|
||||
|
||||
|
|
@ -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).
|
||||
// If the status was draft-based (draftID != 0), just clear the entry —
|
||||
// the final sendMessage will automatically replace the draft bubble.
|
||||
// For draft-based entries, explicitly dismiss the draft bubble before
|
||||
// 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 entry, ok := v.(statusMsgEntry); ok {
|
||||
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 != "" {
|
||||
if editor, ok := ch.(MessageEditor); ok {
|
||||
if err := editor.EditMessage(ctx, msg.ChatID, entry.messageID, msg.Content); err == nil {
|
||||
|
|
|
|||
|
|
@ -1433,3 +1433,105 @@ func TestGenerateDraftID_Stable(t *testing.T) {
|
|||
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")
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue