From 6cf48a6b08ad3bd58d677b79a6e9cd4e2790c488 Mon Sep 17 00:00:00 2001 From: SYNITY Date: Thu, 16 Jul 2026 17:25:59 +0700 Subject: [PATCH] feat(bitrix24): send pairing reply with code instead of silent drop [B24:2794] (#1445) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an agent's DM/Group policy is "pairing (require code)", an unpaired Bitrix24 sender got silence: the channel only had a Phase 03 stub (logPairingNeeded) that logged "pairing required (Phase 07 will send pairing reply)" and dropped the message — no code was ever generated or sent, so an admin had nothing to approve. Bitrix24 was the only channel missing this; Discord/Zalo/WhatsApp/Slack/Feishu already send a pairing reply. Replace the stub with sendPairingReply, mirroring the other channels: - RequestPairing() to generate the code (keyed by senderID for DMs, "group:" for groups so it matches CheckGroupPolicy.IsPaired). - Render the shared systemmessages.KeyPairingAccountRequired message (code + how to approve). - Send it back into the chat via the v2 API (imbot.v2.Chat.Message.send), consistent with the welcome / OAuth-invite messages — not the legacy imbot.message.add v1. - Keep the per-sender debounce so a spammy sender can't flood the chat. Tests: send fires on the v2 endpoint with the code, no-op without a pairing service, and the second call within the debounce window is suppressed. Co-authored-by: DangTinh311 --- internal/channels/bitrix24/handle.go | 63 ++++++++--- internal/channels/bitrix24/pairing_test.go | 118 +++++++++++++++++++++ 2 files changed, 165 insertions(+), 16 deletions(-) create mode 100644 internal/channels/bitrix24/pairing_test.go diff --git a/internal/channels/bitrix24/handle.go b/internal/channels/bitrix24/handle.go index 1a6d4ffd..c9d1c591 100644 --- a/internal/channels/bitrix24/handle.go +++ b/internal/channels/bitrix24/handle.go @@ -13,6 +13,7 @@ import ( "github.com/nextlevelbuilder/goclaw/internal/channels" "github.com/nextlevelbuilder/goclaw/internal/channels/media" "github.com/nextlevelbuilder/goclaw/internal/store" + "github.com/nextlevelbuilder/goclaw/internal/systemmessages" ) // mentionMatcher is the compiled-once regex + rendered tag string used to @@ -91,8 +92,8 @@ func (c *Channel) DispatchEvent(ctx context.Context, evt *Event) { // 2. Group-only: require-mention gate + strip the mention from the text. // 3. Skip empty payloads (no text + no media). // 4. Policy: DM vs Group via BaseChannel.CheckDMPolicy / CheckGroupPolicy. -// Pairing policies trigger a pairing-reply stub (Phase 07 will wire up -// the full pairing flow; Phase 03 logs and drops). +// Pairing policies send a pairing reply (code + approve instructions) via +// sendPairingReply; deny drops silently. // 5. Build metadata map with bitrix_* keys so the agent can echo / reply // to the right dialog + message ID later. // 6. Forward to BaseChannel.HandleMessage → publishes bus.InboundMessage. @@ -258,16 +259,17 @@ func (c *Channel) handleMessage(ctx context.Context, evt *Event) { peerKind = "group" } - // Policy gating. BaseChannel reports PolicyNeedsPairing for unpaired DMs - // under the default "pairing" policy. Phase 07 will answer with a real - // pairing reply; Phase 03 logs the intent so the flow is visible while - // the rest of the channel comes online. + // Policy gating. BaseChannel reports PolicyNeedsPairing for unpaired senders + // under the "pairing" policy. Answer with a real pairing reply (code + how to + // approve) via the v2 chat API, matching the other channels (Discord/Zalo/ + // WhatsApp/Slack/Feishu). Group pairings are keyed by "group:" so the + // generated code matches what CheckGroupPolicy.IsPaired later verifies. if peerKind == "direct" { switch c.CheckDMPolicy(ctx, senderID, c.cfg.DMPolicy) { case channels.PolicyDeny: return case channels.PolicyNeedsPairing: - c.logPairingNeeded(senderID, chatID, peerKind) + c.sendPairingReply(ctx, senderID, chatID, peerKind) return } } else { @@ -275,7 +277,7 @@ func (c *Channel) handleMessage(ctx context.Context, evt *Event) { case channels.PolicyDeny: return case channels.PolicyNeedsPairing: - c.logPairingNeeded(senderID, chatID, peerKind) + c.sendPairingReply(ctx, "group:"+chatID, chatID, peerKind) return } } @@ -597,16 +599,45 @@ func (c *Channel) mention() *mentionMatcher { return c.mentionRe } -// logPairingNeeded is the Phase 03 stand-in for a real pairing reply. -// Phase 07 replaces this with a proper pairing message via imbot.message.add; -// until then we log the intent so operators can see unpaired attempts. -func (c *Channel) logPairingNeeded(senderID, chatID, peerKind string) { - if !c.CanSendPairingNotif(senderID, pairingDebounce) { +// sendPairingReply requests a pairing code for an unpaired sender and sends it +// back into the chat via the v2 API (imbot.v2.Chat.Message.send) so an admin can +// approve access. Mirrors the pairing reply the other channels already send +// (Discord/Zalo/WhatsApp/Slack/Feishu) using the shared pairing.account_required +// system message. Debounced per-sender so a spammy sender can't flood the chat. +// +// pairingSenderID is the id the pairing is keyed by — the raw senderID for DMs, +// "group:" for groups — so the generated code matches what CheckDMPolicy +// / CheckGroupPolicy later verify via IsPaired. +func (c *Channel) sendPairingReply(ctx context.Context, pairingSenderID, chatID, peerKind string) { + ps := c.PairingService() + if ps == nil { return } - c.MarkPairingNotifSent(senderID) - slog.Info("bitrix24: pairing required (Phase 07 will send pairing reply)", - "sender_id", senderID, "chat_id", chatID, "peer_kind", peerKind, + if !c.CanSendPairingNotif(pairingSenderID, pairingDebounce) { + return + } + + code, err := ps.RequestPairing(ctx, pairingSenderID, c.Name(), chatID, "default", nil) + if err != nil { + slog.Debug("bitrix24 pairing: request failed", + "sender_id", pairingSenderID, "chat_id", chatID, "err", err) + return + } + + replyText := c.SystemMessage("", systemmessages.KeyPairingAccountRequired, systemmessages.Vars{ + "platform": "Bitrix24", + "sender_id": pairingSenderID, + "code": code, + }) + + if err := c.sendChunkV2Public(ctx, chatID, replyText, 0); err != nil { + slog.Warn("bitrix24 pairing: reply send failed", + "sender_id", pairingSenderID, "chat_id", chatID, "err", err) + return + } + c.MarkPairingNotifSent(pairingSenderID) + slog.Info("bitrix24 pairing: reply sent", + "sender_id", pairingSenderID, "chat_id", chatID, "peer_kind", peerKind, "portal", c.cfg.Portal) } diff --git a/internal/channels/bitrix24/pairing_test.go b/internal/channels/bitrix24/pairing_test.go new file mode 100644 index 00000000..d7a99fbb --- /dev/null +++ b/internal/channels/bitrix24/pairing_test.go @@ -0,0 +1,118 @@ +package bitrix24 + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + + "github.com/nextlevelbuilder/goclaw/internal/store" + "github.com/nextlevelbuilder/goclaw/internal/systemmessages" +) + +// fakeBitrixPairingStore implements store.PairingStore for pairing-reply tests. +type fakeBitrixPairingStore struct { + requested bool + requestedFor string +} + +func (s *fakeBitrixPairingStore) RequestPairing(_ context.Context, senderID, _, _, _ string, _ map[string]string) (string, error) { + s.requested = true + s.requestedFor = senderID + return "code123", nil +} +func (s *fakeBitrixPairingStore) ApprovePairing(context.Context, string, string) (*store.PairedDeviceData, error) { + return nil, nil +} +func (s *fakeBitrixPairingStore) DenyPairing(context.Context, string) error { return nil } +func (s *fakeBitrixPairingStore) RevokePairing(context.Context, string, string) error { return nil } +func (s *fakeBitrixPairingStore) IsPaired(context.Context, string, string) (bool, error) { + return false, nil +} +func (s *fakeBitrixPairingStore) ListPending(context.Context) []store.PairingRequestData { return nil } +func (s *fakeBitrixPairingStore) ListPaired(context.Context) []store.PairedDeviceData { return nil } +func (s *fakeBitrixPairingStore) MigrateGroupChatID(context.Context, string, string, string) error { + return nil +} + +// TestSendPairingReply_SendsCodeViaV2 verifies an unpaired sender gets a pairing +// code sent back through the v2 chat API (imbot.v2.Chat.Message.send), not the +// old v1 imbot.message.add, and not silence. +func TestSendPairingReply_SendsCodeViaV2(t *testing.T) { + var calls int32 + var gotPath, gotMessage string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&calls, 1) + gotPath = r.URL.Path + _ = r.ParseForm() + gotMessage = r.Form.Get("fields[message]") + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"result": map[string]any{"id": 999}}) + })) + defer srv.Close() + + bc := newChannelWithBoundPortal(t, srv) + bc.SetSystemMessages(systemmessages.NewResolver(nil)) + ps := &fakeBitrixPairingStore{} + bc.SetPairingService(ps) + + bc.sendPairingReply(context.Background(), "42", "chat100", "direct") + + if atomic.LoadInt32(&calls) == 0 { + t.Fatal("expected a REST send for the pairing reply, got none (bot stayed silent)") + } + if !strings.HasSuffix(gotPath, "/rest/imbot.v2.Chat.Message.send.json") { + t.Errorf("pairing reply used wrong endpoint %q; want v2 imbot.v2.Chat.Message.send", gotPath) + } + if !ps.requested { + t.Error("expected RequestPairing to be called to generate a code") + } + if !strings.Contains(gotMessage, "code123") { + t.Errorf("pairing message should contain the code; got %q", gotMessage) + } +} + +// TestSendPairingReply_NoService_NoOp: without a pairing service the call is a +// safe no-op (no send). +func TestSendPairingReply_NoService_NoOp(t *testing.T) { + var calls int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&calls, 1) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"result": map[string]any{"id": 1}}) + })) + defer srv.Close() + + bc := newChannelWithBoundPortal(t, srv) // no SetPairingService → PairingService() nil + bc.sendPairingReply(context.Background(), "42", "chat100", "direct") + + if got := atomic.LoadInt32(&calls); got != 0 { + t.Errorf("expected no send without a pairing service, got %d", got) + } +} + +// TestSendPairingReply_Debounced: a second call within the debounce window does +// not fire a second send. +func TestSendPairingReply_Debounced(t *testing.T) { + var calls int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&calls, 1) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"result": map[string]any{"id": 1}}) + })) + defer srv.Close() + + bc := newChannelWithBoundPortal(t, srv) + bc.SetSystemMessages(systemmessages.NewResolver(nil)) + bc.SetPairingService(&fakeBitrixPairingStore{}) + + bc.sendPairingReply(context.Background(), "42", "chat100", "direct") + bc.sendPairingReply(context.Background(), "42", "chat100", "direct") // debounced + + if got := atomic.LoadInt32(&calls); got != 1 { + t.Errorf("expected 1 send (second debounced), got %d", got) + } +}