feat(bitrix24): scope Open Channel mention gate to group-style connectors only [B24:2794] (#1295)

Before: every Open Channel message required a bot @mention before the agent
picked it up — including external-customer turns on 1-to-1 connectors
(Facebook Messenger, Zalo OA, …) where the customer has no syntactic way
to produce a mention. The gate, designed for group-style connectors
(Zalo personal pools multiple customers into one Open Channel chat),
was silently dropping every legitimate customer message on every other
connector.

This change scopes the gate by connector:

- Internal staff (IS_CONNECTOR=N) — STILL gate. Operators can @-mention
  and we don't want the bot jumping into operator-to-operator chatter
  in the same session.

- External customer (IS_CONNECTOR=Y) on a whitelisted connector
  (default: synity_zalo_personal) — STILL gate. Multiple customers
  share the chat, so untagged turns would otherwise spam them.

- External customer on any other connector (Facebook, Zalo OA, the long
  tail of 1-to-1 connectors, legacy payloads without CHAT_ENTITY_ID) —
  do NOT gate. Every message is addressed to the bot by construction.

The whitelist is overridable via BITRIX24_REQUIRE_MENTION_CONNECTORS
(comma-separated connector codes — the leading token of CHAT_ENTITY_ID).
Default keeps synity_zalo_personal gated so a fresh deployment is safe.
Adding a future group-style connector (e.g. a Zalo group flavour) is an
env edit + restart, not a code change.

- mention_gate.go (new):
    connectorCodeFromEntityID() parses "synity_zalo_personal|20|grp|960"
        into "synity_zalo_personal".
    requireMentionConnectorSet() reads the env once (sync.Once cache)
        and falls back to the hardcoded default {synity_zalo_personal}.
    shouldRequireMentionForOpenline() is the policy matrix.
- mention_gate_test.go (new): pins the parser, the env-override path,
    and every row of the policy matrix.
- handle.go: replaces the unconditional Open Channel mention gate with
    a call to shouldRequireMentionForOpenline(). Adds chat_entity_id to
    the drop log so operators can see which connector tripped the gate.
- handle_test.go:
    Update existing "ConnectorWithoutMentionDropped" test to set
    ChatEntityID to the whitelisted connector — that's what the gate now
    keys on.
    Add new "OneToOneConnectorWithoutMentionForwarded" test pinning the
    new behavior: facebook (not whitelisted) forwards every customer
    message without a mention.

Surface parity: backend-only. No API contract / web UI / CLI change.
The env var follows the same operational model as the other BITRIX24_*
vars in docker-compose.yml; documenting it there is a no-op until the
default needs overriding.

Co-authored-by: DangTinh311 <dangtinh31193@gmail.com>

internal/channels/bitrix24/handle.go

@@ -145,13 +145,22 @@ func (c *Channel) handleMessage(ctx context.Context, evt *Event) {
 	// without an explicit mention, traffic is dropped so the bot doesn't
 	// spam the customer or interfere with operator handling.
 	if isOpenChannel {
-		if !c.isMentionedParams(&evt.Params) {
-			slog.Info("bitrix24 message: dropped OL message without mention",
-				"from_user_id", evt.Params.FromUserID,
-				"from_connector", evt.Params.FromIsConnector,
-				"dialog_id", evt.Params.DialogID,
-				"message_id", evt.Params.MessageID)
-			return
+		// Mention gate is now scoped per-connector: see shouldRequireMentionForOpenline
+		// for the policy (internal staff always gate; external customer gates only
+		// for group-style connectors like Zalo personal; 1-to-1 connectors reply
+		// to every customer message). Connector whitelist is overridable via
+		// BITRIX24_REQUIRE_MENTION_CONNECTORS env so adding e.g. a future Zalo group
+		// connector doesn't need a code change.
+		if shouldRequireMentionForOpenline(evt.Params.FromIsConnector, evt.Params.ChatEntityID) {
+			if !c.isMentionedParams(&evt.Params) {
+				slog.Info("bitrix24 message: dropped OL message without mention",
+					"from_user_id", evt.Params.FromUserID,
+					"from_connector", evt.Params.FromIsConnector,
+					"chat_entity_id", evt.Params.ChatEntityID,
+					"dialog_id", evt.Params.DialogID,
+					"message_id", evt.Params.MessageID)
+				return
+			}
 		}
 	}
 	if isGroup {

internal/channels/bitrix24/handle_test.go

@@ -654,10 +654,13 @@ func TestHandleMessage_Blocked_DoesNotCollectContact(t *testing.T) {
 // --- Open Channel (MESSAGE_TYPE="L") gating ---------------------------------
 
 // TestHandleMessage_OpenChannel_ConnectorWithoutMentionDropped covers the
-// customer side of the Open Channel gate without an explicit mention: a
-// Zalo/FB customer (IS_CONNECTOR=Y) sending into the session WITHOUT
-// @-mentioning the bot must NOT trigger the agent. Humans handle plain
-// customer traffic; the bot only steps in when explicitly called.
+// customer side of the Open Channel gate on a GROUP-style connector
+// (Zalo personal) without an explicit mention: customers chatting amongst
+// themselves in the same Open Channel chat must NOT trigger the agent.
+// The CHAT_ENTITY_ID prefix is what tells us "this connector is
+// group-style and therefore needs the gate" — connectors that don't
+// match the whitelist are handled by sibling tests (no gate, every
+// message forwards).
 func TestHandleMessage_OpenChannel_ConnectorWithoutMentionDropped(t *testing.T) {
 	ch, mb := newHandleTestChannel(t, 1058, false)
 	defer resetWebhookRouterForTest()
@@ -670,12 +673,42 @@ func TestHandleMessage_OpenChannel_ConnectorWithoutMentionDropped(t *testing.T)
 			MessageID:       "m-ol-customer",
 			MessageType:     "L",
 			ChatEntityType:  "LINES",
+			ChatEntityID:    "synity_zalo_personal|20|grp|960",
 			Message:         "Em hỏi giá sản phẩm A",
 			FromIsConnector: true,
 		},
 	})
 	if _, ok := drainOne(mb, 100*time.Millisecond); ok {
-		t.Error("Open Channel connector messages without mention must be dropped")
+		t.Error("Open Channel connector (whitelisted group-style) messages without mention must be dropped")
+	}
+}
+
+// TestHandleMessage_OpenChannel_OneToOneConnectorWithoutMentionForwarded
+// covers the inverse case: a 1-to-1 connector (e.g. Facebook Messenger,
+// Zalo OA) where each customer has their own Open Channel chat. Every
+// customer message is addressed to the bot by construction; gating
+// would silently drop legitimate customer turns. CHAT_ENTITY_ID prefix
+// "facebook" is not in the require-mention whitelist, so the message
+// MUST forward without a mention.
+func TestHandleMessage_OpenChannel_OneToOneConnectorWithoutMentionForwarded(t *testing.T) {
+	ch, mb := newHandleTestChannel(t, 1058, false)
+	defer resetWebhookRouterForTest()
+
+	ch.DispatchEvent(context.Background(), &Event{
+		Type: EventMessageAdd,
+		Params: EventParams{
+			FromUserID:      "1074",
+			DialogID:        "chat5208",
+			MessageID:       "m-fb-customer",
+			MessageType:     "L",
+			ChatEntityType:  "LINES",
+			ChatEntityID:    "facebook|34|36305652112415023|1074",
+			Message:         "hello, do you have product X in stock?",
+			FromIsConnector: true,
+		},
+	})
+	if _, ok := drainOne(mb, 200*time.Millisecond); !ok {
+		t.Error("1-to-1 connector (facebook) customer messages must forward without requiring mention")
 	}
 }
 

internal/channels/bitrix24/mention_gate.go

@@ -0,0 +1,112 @@
+package bitrix24
+
+import (
+	"os"
+	"strings"
+	"sync"
+)
+
+// defaultRequireMentionConnectors is the built-in set of Open Channel
+// connectors that require a bot @mention before reply. Group-style
+// connectors — where multiple external customers share a single Open
+// Channel chat — MUST gate, otherwise the bot would spam every customer
+// turn (including customer-to-customer chatter that doesn't address the
+// bot at all). 1-to-1 connectors (Facebook Messenger, Zalo OA, …) do NOT
+// need gating: every customer message is addressed to the bot by
+// construction, so requiring a mention there would drop legitimate
+// traffic that customers have no syntactic way to produce.
+//
+// "synity_zalo_personal" is currently the only known group-style Openline
+// connector in this deployment — Zalo personal pools several customers
+// into one Bitrix Open Channel chat. Operators can extend or override
+// the set via the BITRIX24_REQUIRE_MENTION_CONNECTORS env var (see
+// requireMentionConnectorSet below).
+var defaultRequireMentionConnectors = []string{"synity_zalo_personal"}
+
+var (
+	requireMentionOnce sync.Once
+	requireMentionSet  map[string]bool
+)
+
+// requireMentionConnectorSet returns the connector codes (the leading
+// token of CHAT_ENTITY_ID — e.g. "synity_zalo_personal" out of
+// "synity_zalo_personal|20|grp|960") that require a bot @mention.
+//
+// Resolution: env BITRIX24_REQUIRE_MENTION_CONNECTORS overrides the
+// hardcoded default. The env value is a comma-separated list of codes;
+// empty entries and whitespace are tolerated. An empty env value falls
+// back to the default set — set the var to a single comma or a sentinel
+// like "none" if you genuinely want to disable gating entirely.
+//
+// The set is parsed once and cached for the process lifetime. Operators
+// who edit the env need to restart the container (same operational
+// model as the other BITRIX24_* env vars in docker-compose.yml).
+func requireMentionConnectorSet() map[string]bool {
+	requireMentionOnce.Do(func() {
+		raw := strings.TrimSpace(os.Getenv("BITRIX24_REQUIRE_MENTION_CONNECTORS"))
+		set := map[string]bool{}
+		if raw == "" {
+			for _, code := range defaultRequireMentionConnectors {
+				set[code] = true
+			}
+		} else {
+			for _, code := range strings.Split(raw, ",") {
+				if code = strings.TrimSpace(code); code != "" {
+					set[code] = true
+				}
+			}
+		}
+		requireMentionSet = set
+	})
+	return requireMentionSet
+}
+
+// connectorCodeFromEntityID extracts the leading token of CHAT_ENTITY_ID.
+// Open Channel payloads ship CHAT_ENTITY_ID as a pipe-separated string
+// like "synity_zalo_personal|20|grp|960"; the first segment names the
+// connector ("synity_zalo_personal" here). Returns "" when the field is
+// empty or missing — callers treat empty as "unknown connector, do not
+// gate" so legacy Openline events without an entity id default to the
+// non-gated path.
+func connectorCodeFromEntityID(entityID string) string {
+	entityID = strings.TrimSpace(entityID)
+	if entityID == "" {
+		return ""
+	}
+	if i := strings.IndexByte(entityID, '|'); i >= 0 {
+		return entityID[:i]
+	}
+	return entityID
+}
+
+// shouldRequireMentionForOpenline decides whether an Open Channel inbound
+// message must @-mention the bot before the agent picks it up.
+//
+// Policy (matches reviewed business intent):
+//
+//   - Internal staff in an Openline chat (IS_CONNECTOR=N) → require mention.
+//     Staff use the operator UI and CAN @-mention; without the gate the bot
+//     would jump into every operator-to-operator side conversation in the
+//     same Open Channel session.
+//
+//   - External customer (IS_CONNECTOR=Y) on a group-style connector listed
+//     in requireMentionConnectorSet → require mention. Customers cannot
+//     produce a mention from outside (no syntax for it in Zalo), so this is
+//     effectively "drop traffic that the upstream connector didn't tag as
+//     bot-addressed".
+//
+//   - External customer on any other connector (FB Messenger, Zalo OA, the
+//     long tail of 1-to-1 connectors, or any payload without a recognised
+//     CHAT_ENTITY_ID) → do NOT require mention. Every message is addressed
+//     to the bot by construction; gating would silently drop legitimate
+//     customer turns.
+func shouldRequireMentionForOpenline(fromConnector bool, chatEntityID string) bool {
+	if !fromConnector {
+		return true
+	}
+	code := connectorCodeFromEntityID(chatEntityID)
+	if code == "" {
+		return false
+	}
+	return requireMentionConnectorSet()[code]
+}

internal/channels/bitrix24/mention_gate_test.go

@@ -0,0 +1,133 @@
+package bitrix24
+
+import (
+	"sync"
+	"testing"
+)
+
+// resetRequireMentionCache lets each test observe the env override path —
+// the package-level sync.Once otherwise locks in whatever the FIRST caller
+// in the process saw. Tests that touch BITRIX24_REQUIRE_MENTION_CONNECTORS
+// MUST call this before reading the set, and again at cleanup so a later
+// test doesn't inherit a polluted cache.
+func resetRequireMentionCache() {
+	requireMentionOnce = sync.Once{}
+	requireMentionSet = nil
+}
+
+func TestConnectorCodeFromEntityID(t *testing.T) {
+	cases := []struct {
+		in   string
+		want string
+	}{
+		// Real-shape payload — what the Bitrix Open Channel webhook actually
+		// ships for the Synity Zalo personal connector.
+		{"synity_zalo_personal|20|grp|960", "synity_zalo_personal"},
+		// Hypothetical future Facebook Open Channel payload — verifies the
+		// helper doesn't require any specific connector name to work.
+		{"facebook|34|36305652112415023|1074", "facebook"},
+		// Token with no pipe at all — return the whole string so the caller
+		// can still look it up in the whitelist.
+		{"plain_token", "plain_token"},
+		// Surrounding whitespace from sloppy upstream serialization gets
+		// trimmed before the split — otherwise we'd never match.
+		{"  spaced|x|y  ", "spaced"},
+		// Empty / pipe-leading cases collapse to "" so the gate falls into
+		// the "unknown connector → don't gate" branch.
+		{"", ""},
+		{"|leading|pipe", ""},
+	}
+	for _, tc := range cases {
+		t.Run(tc.in, func(t *testing.T) {
+			if got := connectorCodeFromEntityID(tc.in); got != tc.want {
+				t.Errorf("connectorCodeFromEntityID(%q) = %q; want %q", tc.in, got, tc.want)
+			}
+		})
+	}
+}
+
+// TestRequireMentionConnectorSet_DefaultWhenEnvUnset pins the safety
+// default: when operators haven't configured BITRIX24_REQUIRE_MENTION_CONNECTORS
+// the gate still trips for the one connector we currently know is
+// group-style (Zalo personal). Without this default a fresh deployment
+// would silently reply to every Zalo customer turn.
+func TestRequireMentionConnectorSet_DefaultWhenEnvUnset(t *testing.T) {
+	resetRequireMentionCache()
+	defer resetRequireMentionCache()
+	t.Setenv("BITRIX24_REQUIRE_MENTION_CONNECTORS", "")
+
+	set := requireMentionConnectorSet()
+	if !set["synity_zalo_personal"] {
+		t.Error("default set must include synity_zalo_personal")
+	}
+	if set["facebook"] {
+		t.Error("default set must not include facebook (1-to-1 connector)")
+	}
+}
+
+// TestRequireMentionConnectorSet_EnvOverride is the whole point of the
+// env-driven design — operators can add a future group-style connector
+// without redeploying the binary. Once "zalo_group" gets shipped this
+// is the runtime knob that turns the gate on for it.
+func TestRequireMentionConnectorSet_EnvOverride(t *testing.T) {
+	resetRequireMentionCache()
+	defer resetRequireMentionCache()
+	t.Setenv("BITRIX24_REQUIRE_MENTION_CONNECTORS", "synity_zalo_personal, zalo_group ,  ")
+
+	set := requireMentionConnectorSet()
+	if !set["synity_zalo_personal"] {
+		t.Error("override must keep synity_zalo_personal")
+	}
+	if !set["zalo_group"] {
+		t.Error("override must add zalo_group")
+	}
+	// Whitespace + trailing empty entry from the comma split must NOT
+	// register a phantom "" key — that would gate every payload with an
+	// empty CHAT_ENTITY_ID, which we explicitly want to leave un-gated.
+	if set[""] {
+		t.Error("override must not register an empty-string key")
+	}
+}
+
+// TestShouldRequireMentionForOpenline is the policy matrix in one place.
+// Each row corresponds to a decided business case from the spec review,
+// so a regression on any row is a regression on a deliberate decision.
+func TestShouldRequireMentionForOpenline(t *testing.T) {
+	resetRequireMentionCache()
+	defer resetRequireMentionCache()
+	t.Setenv("BITRIX24_REQUIRE_MENTION_CONNECTORS", "")
+
+	cases := []struct {
+		name          string
+		fromConnector bool
+		chatEntityID  string
+		want          bool
+	}{
+		// Staff (IS_CONNECTOR=N) ALWAYS gate — they can @-mention and we
+		// don't want the bot jumping into operator-to-operator chatter.
+		{"staff in zalo personal still gates", false, "synity_zalo_personal|20|grp|960", true},
+		{"staff in facebook still gates", false, "facebook|34|x|960", true},
+		{"staff without entity id still gates", false, "", true},
+		// Customer (IS_CONNECTOR=Y) on the whitelisted connector → gate.
+		// Multiple customers share one Zalo personal Open Channel chat, so
+		// every untagged turn would otherwise spam them.
+		{"customer on zalo personal gates", true, "synity_zalo_personal|20|grp|960", true},
+		// Customer on a 1-to-1 connector → DO NOT gate. Every message is
+		// addressed to the bot; gating would drop legitimate traffic.
+		{"customer on facebook does not gate", true, "facebook|34|x|1074", false},
+		{"customer on zalo_oa does not gate", true, "zalo_oa|1|x|1", false},
+		// Customer with empty CHAT_ENTITY_ID → DO NOT gate. We can't
+		// distinguish the source, and the safe default for the long tail
+		// of legacy payloads is "let it through" rather than "silently
+		// drop and look broken".
+		{"customer without entity id does not gate", true, "", false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := shouldRequireMentionForOpenline(tc.fromConnector, tc.chatEntityID); got != tc.want {
+				t.Errorf("shouldRequireMentionForOpenline(connector=%v, entity=%q) = %v; want %v",
+					tc.fromConnector, tc.chatEntityID, got, tc.want)
+			}
+		})
+	}
+}