diff --git a/internal/channels/discord/discord.go b/internal/channels/discord/discord.go index d995cf56..b7098956 100644 --- a/internal/channels/discord/discord.go +++ b/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() +} diff --git a/internal/channels/discord/discord_test.go b/internal/channels/discord/discord_test.go new file mode 100644 index 00000000..7a062ed6 --- /dev/null +++ b/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 +}