fix(discord): stop typing after successful delivery (#589)

internal/channels/discord/discord.go

@@ -33,8 +33,8 @@ type Channel struct {
 	approvedGroups  sync.Map // chatID → true (in-memory cache for paired groups)
 	groupHistory    *channels.PendingHistory
 	historyLimit    int
-	agentStore      store.AgentStore             // for agent key lookup (nil = writer commands disabled)
-	configPermStore store.ConfigPermissionStore   // for group file writer management (nil = writer commands disabled)
+	agentStore      store.AgentStore            // for agent key lookup (nil = writer commands disabled)
+	configPermStore store.ConfigPermissionStore // for group file writer management (nil = writer commands disabled)
 }
 
 // New creates a new Discord channel from config.
@@ -123,7 +123,7 @@ func (c *Channel) Stop(_ context.Context) error {
 }
 
 // Send delivers an outbound message to a Discord channel.
-func (c *Channel) Send(_ context.Context, msg bus.OutboundMessage) error {
+func (c *Channel) Send(_ context.Context, msg bus.OutboundMessage) (err error) {
 	if !c.IsRunning() {
 		return fmt.Errorf("discord bot not running")
 	}
@@ -151,10 +151,10 @@ func (c *Channel) Send(_ context.Context, msg bus.OutboundMessage) error {
 		return nil
 	}
 
-	// Stop typing indicator controller
-	if ctrl, ok := c.typingCtrls.LoadAndDelete(channelID); ok {
-		ctrl.(*typing.Controller).Stop()
-	}
+	typingCtrl := c.currentTypingCtrl(channelID)
+	defer func() {
+		c.finishTyping(channelID, typingCtrl, err)
+	}()
 
 	content := msg.Content
 
@@ -251,3 +251,46 @@ func lastIndexByte(s string, c byte) int {
 	}
 	return -1
 }
+
+func (c *Channel) currentTypingCtrl(channelID string) *typing.Controller {
+	ctrl, ok := c.typingCtrls.Load(channelID)
+	if !ok {
+		return nil
+	}
+
+	typed, ok := ctrl.(*typing.Controller)
+	if !ok {
+		c.typingCtrls.Delete(channelID)
+		return nil
+	}
+
+	return typed
+}
+
+func (c *Channel) finishTyping(channelID string, expected *typing.Controller, sendErr error) {
+	if expected == nil {
+		return
+	}
+	if sendErr != nil {
+		slog.Warn("discord: outbound send failed; keeping typing indicator active until TTL",
+			"channel_id", channelID, "error", sendErr)
+		return
+	}
+
+	current, ok := c.typingCtrls.Load(channelID)
+	if !ok {
+		return
+	}
+
+	typed, ok := current.(*typing.Controller)
+	if !ok {
+		c.typingCtrls.Delete(channelID)
+		return
+	}
+	if typed != expected {
+		return
+	}
+
+	c.typingCtrls.Delete(channelID)
+	typed.Stop()
+}

internal/channels/discord/discord_test.go

@@ -0,0 +1,130 @@
+package discord
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+
+	"github.com/bwmarrin/discordgo"
+
+	"github.com/nextlevelbuilder/goclaw/internal/bus"
+	"github.com/nextlevelbuilder/goclaw/internal/channels"
+	"github.com/nextlevelbuilder/goclaw/internal/channels/typing"
+)
+
+func TestSendStopsTypingAfterPlaceholderEditSucceeds(t *testing.T) {
+	var stopCalled atomic.Bool
+	var stopBeforeRequest atomic.Bool
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPatch {
+			t.Fatalf("unexpected method: %s", r.Method)
+		}
+		if r.URL.Path != "/channels/channel-1/messages/placeholder-1" {
+			t.Fatalf("unexpected path: %s", r.URL.Path)
+		}
+		if stopCalled.Load() {
+			stopBeforeRequest.Store(true)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"id":"placeholder-1","channel_id":"channel-1","content":"done"}`))
+	}))
+	defer server.Close()
+
+	ch := newTestChannel(t, server)
+
+	ctrl := typing.New(typing.Options{
+		StopFn: func() error {
+			stopCalled.Store(true)
+			return nil
+		},
+	})
+	ch.typingCtrls.Store("channel-1", ctrl)
+	ch.placeholders.Store("inbound-1", "placeholder-1")
+
+	err := ch.Send(context.Background(), bus.OutboundMessage{
+		Channel: "discord",
+		ChatID:  "channel-1",
+		Content: "done",
+		Metadata: map[string]string{
+			"placeholder_key": "inbound-1",
+		},
+	})
+	if err != nil {
+		t.Fatalf("Send() error = %v", err)
+	}
+	if !stopCalled.Load() {
+		t.Fatal("expected typing controller to stop after successful placeholder edit")
+	}
+	if stopBeforeRequest.Load() {
+		t.Fatal("typing controller stopped before placeholder edit request")
+	}
+	if _, ok := ch.typingCtrls.Load("channel-1"); ok {
+		t.Fatal("expected typing controller to be removed after successful delivery")
+	}
+}
+
+func TestSendKeepsTypingActiveWhenDeliveryFails(t *testing.T) {
+	var stopCalled atomic.Bool
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPost {
+			t.Fatalf("unexpected method: %s", r.Method)
+		}
+		if r.URL.Path != "/channels/channel-1/messages" {
+			t.Fatalf("unexpected path: %s", r.URL.Path)
+		}
+		http.Error(w, "boom", http.StatusInternalServerError)
+	}))
+	defer server.Close()
+
+	ch := newTestChannel(t, server)
+
+	ctrl := typing.New(typing.Options{
+		StopFn: func() error {
+			stopCalled.Store(true)
+			return nil
+		},
+	})
+	ch.typingCtrls.Store("channel-1", ctrl)
+
+	err := ch.Send(context.Background(), bus.OutboundMessage{
+		Channel: "discord",
+		ChatID:  "channel-1",
+		Content: "done",
+	})
+	if err == nil {
+		t.Fatal("expected Send() to return an error")
+	}
+	if stopCalled.Load() {
+		t.Fatal("typing controller stopped even though Discord delivery failed")
+	}
+	if stored, ok := ch.typingCtrls.Load("channel-1"); !ok || stored != ctrl {
+		t.Fatal("expected typing controller to remain active after delivery failure")
+	}
+}
+
+func newTestChannel(t *testing.T, server *httptest.Server) *Channel {
+	t.Helper()
+
+	prevEndpointChannels := discordgo.EndpointChannels
+	discordgo.EndpointChannels = server.URL + "/channels/"
+	t.Cleanup(func() {
+		discordgo.EndpointChannels = prevEndpointChannels
+	})
+
+	session, err := discordgo.New("Bot test-token")
+	if err != nil {
+		t.Fatalf("discordgo.New() error = %v", err)
+	}
+	session.Client = server.Client()
+
+	ch := &Channel{
+		BaseChannel: channels.NewBaseChannel(channels.TypeDiscord, nil, nil),
+		session:     session,
+	}
+	ch.SetRunning(true)
+	return ch
+}