diff --git a/gateway/authz_mixin.py b/gateway/authz_mixin.py index fae4b74a5e11..46417dae3f9a 100644 --- a/gateway/authz_mixin.py +++ b/gateway/authz_mixin.py @@ -94,6 +94,7 @@ def _authorization_adapter( self, platform: Optional[Platform], profile: Optional[str] = None, + account: Optional[str] = None, ): """Resolve the live adapter whose intake policy should gate authorization. @@ -102,11 +103,30 @@ def _authorization_adapter( ``self.adapters``. ``SessionSource.profile`` selects which map to consult. When a stamped profile has its own adapter registry entry, the default profile's same-platform adapter must not be consulted as a fallback. + + Multi-account gateways (#8287) add a second dimension the same way: + named-account adapters live in ``_account_adapters[platform][account]`` + while the default account uses ``self.adapters``. A stamped account + with no registry entry fails closed for the same reason a profile + does — replying out the wrong bot is worse than not replying. """ if not platform: return None profile_name = (profile or "").strip() or None + # Coerce defensively: a MagicMock/SimpleNamespace source auto-creates + # a truthy ``account`` attribute (AGENTS.md pitfall #17), which must + # read as "default account" rather than trip the fail-closed branch. + account_name = account.strip() if isinstance(account, str) else None + account_name = account_name or None + if account_name == "default": + account_name = None if profile_name and profile_name != "default": + if account_name: + # A named account inside a secondary profile is not a + # supported combination yet — fail closed rather than guess a + # bot (#8287). Checked before the active-profile fast path so + # an account under a named active profile also fails closed. + return None active_profile = None active_profile_fn = getattr(self, "_active_profile_name", None) if callable(active_profile_fn): @@ -124,6 +144,9 @@ def _authorization_adapter( # (e.g. its adapter failed to connect) must NOT fall back to the # default profile's adapter — that sends replies out the wrong bot. return None + if account_name: + account_adapters = getattr(self, "_account_adapters", None) or {} + return (account_adapters.get(platform) or {}).get(account_name) adapters = getattr(self, "adapters", None) or {} return adapters.get(platform) @@ -152,6 +175,7 @@ def _adapter_for_source(self, source: Optional[SessionSource]): return self._authorization_adapter( getattr(source, "platform", None), getattr(source, "profile", None), + getattr(source, "account", None), ) def _registered_transport_adapter(self, source: SessionSource): diff --git a/gateway/config.py b/gateway/config.py index a00fa0f9a1ca..e6cd6c392a59 100644 --- a/gateway/config.py +++ b/gateway/config.py @@ -11,6 +11,7 @@ import logging import os import json +import re from pathlib import Path from dataclasses import asdict, dataclass, field, is_dataclass from typing import Dict, List, Optional, Any, Callable @@ -731,6 +732,38 @@ def from_dict(cls, data: Dict[str, Any]) -> "PlatformConfig": if _typing_text is None: _typing_text = extra.get("typing_status_text") + # Multi-account blocks (#8287): ``accounts:`` may arrive top-level + # (``platforms.telegram.accounts`` in YAML) or bridged into extra by + # the shared-key loop. Normalize account names (lowercased) into + # ``extra["accounts"]`` so the adapter registry has a single read + # path. Tokens are secrets and load from ``_BOT_TOKEN_`` + # env vars in ``_apply_env_overrides`` — a ``token`` key inside a YAML + # account block is honored for parity but ``.env`` is the supported + # home for credentials. + _accounts = data.get("accounts") + if _accounts is None: + _accounts = extra.get("accounts") + if isinstance(_accounts, dict): + _norm_accounts: Dict[str, Any] = {} + for _acct_name, _acct_block in _accounts.items(): + _acct_key = str(_acct_name).strip().lower() + if not _acct_key: + continue + # Account names become a session-key namespace suffix + # (``agent:main@``), so the charset is restricted: + # ``:`` would break key splitting, ``@`` the suffix parse. + if not re.fullmatch(r"[a-z0-9][a-z0-9_-]*", _acct_key): + logger.warning( + "Ignoring platform account %r: names must match " + "[a-z0-9][a-z0-9_-]* (they become session-key " + "namespace suffixes)", + _acct_name, + ) + continue + _norm_accounts[_acct_key] = _coerce_dict(_acct_block) + if _norm_accounts: + extra["accounts"] = _norm_accounts + channel_overrides: Dict[str, ChannelOverride] = {} raw_overrides = data.get("channel_overrides") or {} if isinstance(raw_overrides, dict): @@ -1911,6 +1944,40 @@ def _enable_from_env(platform: Platform) -> PlatformConfig: if telegram_token: telegram_config = _enable_from_env(Platform.TELEGRAM) telegram_config.token = telegram_token + + # Multi-account tokens (#8287): ``TELEGRAM_BOT_TOKEN_`` declares + # an additional bot account named ```` (lowercased). The + # unsuffixed ``TELEGRAM_BOT_TOKEN`` remains the default account, so + # single-bot setups are byte-identical to before. Candidate names are + # enumerated from the process env (dotenv loads ``.env`` there) and each + # value is read back through ``getenv`` so profile-scoped secrets win + # when a scope is active. Behavioral per-account settings (allowlists, + # home channels, display names) belong in ``platforms.telegram.accounts`` + # in config.yaml — env vars carry only the credential. + _tg_account_prefix = "TELEGRAM_BOT_TOKEN_" + for _env_name in sorted(os.environ): + if not _env_name.startswith(_tg_account_prefix): + continue + _acct_name = _env_name[len(_tg_account_prefix):].strip().lower() + _acct_token = getenv(_env_name) + if not _acct_name or not _acct_token: + continue + # Same charset rule as the YAML block: names become session-key + # namespace suffixes. + if not re.fullmatch(r"[a-z0-9][a-z0-9_-]*", _acct_name): + logger.warning( + "Ignoring %s: account names must match [a-z0-9][a-z0-9_-]*", + _env_name, + ) + continue + _tg_cfg = _enable_from_env(Platform.TELEGRAM) + _tg_accounts = _tg_cfg.extra.setdefault("accounts", {}) + if not isinstance(_tg_accounts, dict): + _tg_accounts = {} + _tg_cfg.extra["accounts"] = _tg_accounts + _acct_block = _tg_accounts.setdefault(_acct_name, {}) + if isinstance(_acct_block, dict): + _acct_block["token"] = _acct_token # Reply threading mode for Telegram (off/first/all) telegram_reply_mode = getenv("TELEGRAM_REPLY_TO_MODE", "").lower() diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 6665fd2eb6de..46d726258348 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -3005,6 +3005,13 @@ def set_status_text(self, chat_id: str, text: Optional[str]) -> None: def __init__(self, config: PlatformConfig, platform: Platform): self.config = config self.platform = platform + # Bot account this adapter instance serves (#8287). None = the + # platform's default account, which is every single-bot gateway. + # The runner stamps this after construction when it starts named + # account adapters; ``build_source`` copies it onto every inbound + # ``SessionSource.account`` so session keys, busy guards, and + # outbound routing all stay per-account. + self.account_name: Optional[str] = None self._message_handler: Optional[MessageHandler] = None # Optional gateway-supplied fan-out for platform-native emoji # reaction events (see ``set_reaction_handler``). @@ -7095,6 +7102,13 @@ def build_source( parent_chat_id=str(parent_chat_id) if parent_chat_id else None, message_id=str(message_id) if message_id else None, profile=profile, + # Bot account this adapter serves (#8287). This is the single + # inbound-construction site every platform's normal-event path + # flows through, so stamping here rather than in per-platform + # helpers is what actually routes named-bot traffic to its own + # session key and egress adapter. None on default/single-bot + # adapters, which keeps their sources byte-identical. + account=getattr(self, "account_name", None), role_authorized=role_authorized, auto_thread_created=auto_thread_created, auto_thread_initial_name=auto_thread_initial_name, diff --git a/gateway/run.py b/gateway/run.py index e59af635503a..e81d616ca469 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -2459,6 +2459,7 @@ def _platform_has_bot_credential(platform: "Platform", platform_config: "Platfor build_session_key, is_shared_multi_user_session, neutralize_untrusted_inline_text, + split_key_namespace, ) from gateway.delivery import ( DeliveryRouter, @@ -3554,12 +3555,19 @@ def _parse_session_key(session_key: str) -> "dict | None": thread_id, so we leave ``thread_id`` out to avoid mis-routing. """ parts = session_key.split(":") - if len(parts) >= 5 and parts[0] == "agent" and parts[1] == "main": + # Accept the default namespace with or without a multi-account suffix + # (``agent:main`` / ``agent:main@support``, #8287) — the positional + # layout after the namespace slot is identical. Named-profile keys stay + # excluded, as before. + _ns, _account = split_key_namespace(parts[1]) if len(parts) > 1 else ("", None) + if len(parts) >= 5 and parts[0] == "agent" and _ns == "main": result = { "platform": parts[2], "chat_type": parts[3], "chat_id": parts[4], } + if _account: + result["account"] = _account if len(parts) > 5 and parts[3] in {"dm", "thread"}: result["thread_id"] = parts[5] return result @@ -6491,6 +6499,13 @@ def __init__(self, config: Optional[GatewayConfig] = None): # sites are untouched when multiplexing is off (this dict is empty). # Populated by _start_secondary_profile_adapters(). self._profile_adapters: Dict[str, Dict[Platform, BasePlatformAdapter]] = {} + # Multi-account (#8287): adapters for NAMED bot accounts live here, + # keyed by Platform then account name. self.adapters stays the default + # account's map — the same shape as _profile_adapters above, so every + # existing self.adapters[...] site is untouched when no named accounts + # are configured (this dict is empty). Populated by + # _start_account_adapters(). + self._account_adapters: Dict[Platform, Dict[str, BasePlatformAdapter]] = {} self._warn_if_docker_media_delivery_is_risky() _gateway_runner_ref = _weakref.ref(self) @@ -12304,6 +12319,29 @@ async def start(self) -> bool: continue enabled_platform_count += 1 + # Named bot accounts (#8287) join the SAME fan-out as the default + # adapter, so they connect concurrently rather than serially behind + # it, and their success is independent of the default's outcome — a + # bad or absent default token cannot keep healthy named bots + # offline (#67455 review finding). + _pending_connects.extend( + self._prepare_account_adapters(platform, platform_config) + ) + + # Accounts-only configuration (#8287): named tokens with no default + # credential. Skip the doomed token-less default connect — it would + # fail and be reported as a platform outage — while the named + # accounts queued above still connect. + if not _platform_has_bot_credential(platform, platform_config) and ( + platform_config.extra or {} + ).get("accounts"): + logger.info( + "%s has no default-account credential; " + "connecting named accounts only.", + platform.value, + ) + continue + adapter = self._create_adapter(platform, platform_config) if not adapter: # Distinguish between missing builtin deps and missing plugin @@ -12343,10 +12381,20 @@ async def _connect_one_startup(p, p_cfg, adp): """Connect a single platform; never let one block the others (#83791).""" if await self._abort_startup_if_shutdown_requested(adp, p): return (p, adp, p_cfg, "aborted", None) - logger.info("Connecting to %s...", p.value) - self._update_platform_runtime_status( - p.value, platform_state="connecting", error_code=None, error_message=None, - ) + # A named bot account (#8287) shares the platform but not its + # runtime status: that row reports the DEFAULT account's health, so + # an account connect must not flip the whole platform to + # "connecting" (and, on failure, must not mark the platform down + # while the default bot is serving fine). + _acct = getattr(adp, "account_name", None) + _acct = _acct if isinstance(_acct, str) and _acct else None + if _acct: + logger.info("Connecting to %s (account %r)...", p.value, _acct) + else: + logger.info("Connecting to %s...", p.value) + self._update_platform_runtime_status( + p.value, platform_state="connecting", error_code=None, error_message=None, + ) try: ok = await self._connect_initial_adapter_with_timeout(adp, p) except Exception as _exc: # noqa: BLE001 - surfaced below as a retryable error @@ -12424,6 +12472,35 @@ async def _connect_one_startup(p, p_cfg, adp): platform, adapter, platform_config, outcome, exc = _item if outcome == "aborted": continue + # Named bot accounts (#8287) share this fan-out but not the + # platform's shared state: a named account owns no entry in + # self.adapters and must never claim the platform's single + # _failed_platforms retry slot — doing so would respawn the DEFAULT + # adapter from the account's config. isinstance-guard because a + # MagicMock adapter auto-creates a truthy account_name + # (AGENTS.md pitfall #17), which would route a default adapter down + # this branch. + _account = getattr(adapter, "account_name", None) + if isinstance(_account, str) and _account: + if outcome == "ok": + self._account_adapters.setdefault(platform, {})[_account] = adapter + self._sync_voice_mode_state_to_adapter(adapter) + connected_count += 1 + logger.info( + "✓ %s connected (account %r)", platform.value, _account + ) + else: + logger.warning( + "✗ %s account %r failed to connect%s", + platform.value, + _account, + f": {exc}" if exc else "", + ) + # Freed rather than left orphaned; per-account reconnect + # queueing lands with the delivery/reconnect slice, so this + # account is retried at the next gateway start. + await self._safe_adapter_disconnect(adapter, platform) + continue if outcome == "exception": logger.error("\u2717 %s error: %s", platform.value, exc) # Same defensive cleanup path for exceptions -- an adapter that @@ -15221,6 +15298,112 @@ def _adapter_credential_fingerprint(adapter: Any) -> Optional[str]: import hashlib return hashlib.sha256(("hermes-mux:" + token).encode("utf-8")).hexdigest()[:16] + @staticmethod + def _account_platform_config( + platform: Platform, + platform_config: "PlatformConfig", + account_block: Dict[str, Any], + ) -> "PlatformConfig": + """Derive a per-account PlatformConfig from the platform's own (#8287). + + The account adapter is handed an ordinary PlatformConfig — its own + token, its own home_channel, and account-block settings overriding + the platform-level ``extra`` — so adapter internals stay entirely + account-agnostic. The ``accounts`` map itself is stripped from the + derived ``extra``: an account must never be able to spawn accounts. + """ + import dataclasses as _dc + + from gateway.config import HomeChannel as _HomeChannel + + merged_extra = { + key: value + for key, value in (platform_config.extra or {}).items() + if key != "accounts" + } + home_channel = platform_config.home_channel + token = platform_config.token + for key, value in (account_block or {}).items(): + if key == "token": + token = value + elif key == "home_channel" and isinstance(value, dict): + # The platform is implicit inside its own account block. + channel = dict(value) + channel.setdefault("platform", platform.value) + home_channel = _HomeChannel.from_dict(channel) + else: + merged_extra[key] = value + return _dc.replace( + platform_config, + token=token, + home_channel=home_channel, + extra=merged_extra, + ) + + def _prepare_account_adapters( + self, platform: Platform, platform_config: "PlatformConfig" + ) -> list: + """Create and wire one adapter per NAMED bot account (#8287). + + Prepare-only, deliberately: the returned + ``(platform, account_config, adapter)`` triples join the SAME + ``_pending_connects`` fan-out the default adapter uses, so named + accounts connect concurrently with every other platform instead of + serially behind them. Connecting them in a loop here would reintroduce + exactly the head-of-line blocking #83791 removed — N bots each costing + a full connect timeout. + + Each adapter is wired like a default adapter and stamped with its + account name; ``build_source`` copies that stamp onto every inbound + ``SessionSource.account``, which is where the per-account session keys + from the previous slice light up. Registration into + ``_account_adapters[platform][name]`` happens in the aggregation loop, + keyed off ``adapter.account_name``, so shared state is still mutated + single-threaded exactly as before. + """ + accounts = (platform_config.extra or {}).get("accounts") + if not isinstance(accounts, dict) or not accounts: + return [] + prepared = [] + for account_name, account_block in accounts.items(): + block = account_block if isinstance(account_block, dict) else {} + if not block.get("token"): + logger.warning( + "Skipping %s account %r: no token (set %s_BOT_TOKEN_%s)", + platform.value, + account_name, + platform.value.upper(), + account_name.upper(), + ) + continue + account_config = self._account_platform_config( + platform, platform_config, block + ) + adapter = self._create_adapter(platform, account_config) + if not adapter: + logger.warning( + "No adapter available for %s account %r", + platform.value, + account_name, + ) + continue + adapter.account_name = account_name + adapter.set_message_handler(self._primary_message_handler()) + adapter.set_fatal_error_handler(self._handle_adapter_fatal_error) + adapter.set_session_store(self.session_store) + adapter.set_busy_session_handler(self._handle_active_session_busy_message) + _set_reaction = getattr(adapter, "set_reaction_handler", None) + if callable(_set_reaction): + _set_reaction(self._handle_reaction_event) + adapter.set_topic_recovery_fn(self._recover_telegram_topic_thread_id) + adapter.set_authorization_check( + self._make_adapter_auth_check(adapter.platform) + ) + adapter.set_platform_event_handler(self._primary_platform_event_handler()) + adapter._busy_text_mode = self._busy_text_mode + prepared.append((platform, account_config, adapter)) + return prepared + def _create_adapter( self, platform: Platform, diff --git a/gateway/session.py b/gateway/session.py index cd62db5048f7..2560b0123cb1 100644 --- a/gateway/session.py +++ b/gateway/session.py @@ -186,6 +186,14 @@ class SessionSource: # target is not served. Excluded from repr/equality and wire serialization. profile_route_rejected: bool = field(default=False, repr=False, compare=False) + # Bot account this inbound message arrived on (#8287). A gateway can run + # multiple bot accounts on one platform (TELEGRAM_BOT_TOKEN_); + # the receiving adapter stamps its account name here so session keys, + # busy guards, and outbound delivery all route per account. None => the + # platform's default account — byte-identical behavior to a single-bot + # gateway. + account: Optional[str] = None + # Discord auto-thread metadata. Newly auto-created Discord threads start # with a fast placeholder title from the raw message, then the gateway can # rename them after the first agent turn using the generated session title. @@ -279,6 +287,8 @@ def to_dict(self) -> Dict[str, Any]: d["message_id"] = self.message_id if self.profile: d["profile"] = self.profile + if self.account: + d["account"] = self.account if self.auto_thread_created: d["auto_thread_created"] = True if self.auto_thread_initial_name: @@ -306,6 +316,7 @@ def from_dict(cls, data: Dict[str, Any]) -> "SessionSource": parent_chat_id=data.get("parent_chat_id"), message_id=data.get("message_id"), profile=data.get("profile"), + account=data.get("account"), auto_thread_created=bool(data.get("auto_thread_created", False)), auto_thread_initial_name=data.get("auto_thread_initial_name"), prospective_thread_id=data.get("prospective_thread_id"), @@ -1067,7 +1078,9 @@ def is_shared_multi_user_session( return not group_sessions_per_user -def _session_key_namespace(profile: Optional[str]) -> str: +def _session_key_namespace( + profile: Optional[str], account: Optional[str] = None +) -> str: """Return the ``agent:`` namespace prefix for a session key. The historical key format is ``agent:main:::...`` where @@ -1081,10 +1094,36 @@ def _session_key_namespace(profile: Optional[str]) -> str: - named profile ``coder`` → ``agent:coder`` — keeps the same positional layout, just a different namespace, so two profiles serving the same platform/chat never collide. + + Multi-account gateways (#8287) reuse the slot the same way: a non-default + bot account is appended as ``@`` (``agent:main@support``, + ``agent:coder@support``), so the same chat reached through two bots yields + two sessions while every positional parser keeps its layout. ``:`` stays + the only separator, and account names are charset-restricted at config + parse time so ``@`` cannot appear inside a name. Readers that map the + namespace back to a profile must strip the suffix via + :func:`split_key_namespace`. """ if not profile or profile == "default": - return "agent:main" - return f"agent:{profile}" + ns = "agent:main" + else: + ns = f"agent:{profile}" + if account and account != "default": + return f"{ns}@{account}" + return ns + + +def split_key_namespace(namespace: str) -> tuple[str, Optional[str]]: + """Split a session-key namespace component into ``(profile_ns, account)``. + + ``main`` → ``("main", None)``; ``main@support`` → ``("main", "support")``. + The account suffix was introduced for multi-account gateways (#8287); + every reader that compares or maps the namespace (profile resolution, + key parsers) must strip it through here rather than assuming the raw + slot equals a profile name. + """ + base, sep, account = (namespace or "").partition("@") + return base, (account or None) if sep else None def build_session_key( @@ -1125,7 +1164,18 @@ def build_session_key( shared session per chat. - Without identifiers, messages fall back to one session per platform/chat_type. """ - ns = _session_key_namespace(profile) + # Account comes from the SOURCE, not a caller parameter: which bot + # received the message is intrinsic to the event, and reading it here + # guarantees the adapter-level guard and the session store derive the + # same key for the same event (per-key guards diverging is the #64934 + # bug class). isinstance-guard bare test fixtures: a MagicMock/ + # SimpleNamespace source auto-creates a truthy non-string ``account`` + # (AGENTS.md pitfall #17), which would otherwise be interpolated into + # the namespace and corrupt every key those fixtures derive. + account = getattr(source, "account", None) + if not isinstance(account, str): + account = None + ns = _session_key_namespace(profile, account) platform = source.platform.value slack_scope_id = ( str(source.scope_id) @@ -1788,7 +1838,10 @@ def _profile_from_session_key(session_key: Optional[str]) -> Optional[str]: parts = str(session_key).split(":") if len(parts) < 2 or parts[0] != "agent": return None - namespace = parts[1] or "main" + # Strip a multi-account suffix (agent:main@support) — the account is + # not a profile and must not be resolved as one (#8287). + namespace, _account = split_key_namespace(parts[1] or "main") + namespace = namespace or "main" return "default" if namespace == "main" else namespace @staticmethod diff --git a/plugins/platforms/telegram/adapter.py b/plugins/platforms/telegram/adapter.py index 6a6fb549c3ac..493fba278c8c 100644 --- a/plugins/platforms/telegram/adapter.py +++ b/plugins/platforms/telegram/adapter.py @@ -1202,6 +1202,7 @@ def _is_callback_user_authorized( user_id=normalized_user_id, user_name=str(user_name).strip() if user_name else None, thread_id=str(thread_id) if thread_id is not None else None, + account=getattr(self, "account_name", None), ) return bool(auth_fn(source)) except Exception: @@ -1279,6 +1280,7 @@ def _source_from_message_for_auth(self, message: Message): user_id=user_id, user_name=user_name, thread_id=thread_id, + account=getattr(self, "account_name", None), ) def _source_from_reaction_for_auth(self, update): diff --git a/tests/gateway/test_telegram_multi_account_adapters.py b/tests/gateway/test_telegram_multi_account_adapters.py new file mode 100644 index 000000000000..68022ba75047 --- /dev/null +++ b/tests/gateway/test_telegram_multi_account_adapters.py @@ -0,0 +1,462 @@ +"""Per-account adapter registry and inbound stamping — #8287. + +Named bot accounts get their own adapter instances, registered in +``_account_adapters[platform][name]`` (the account-dimension mirror of +``_profile_adapters``), each seeing an ordinary derived ``PlatformConfig``. + +Two contracts are load-bearing here: + +* **Resolution fails closed.** A stamped account with no registry entry must + never fall back to the default bot — replying out the wrong bot is worse + than not replying. +* **The stamp lands on the NORMAL inbound path.** Every platform's ordinary + traffic is built by ``BasePlatformAdapter.build_source()``. Stamping only + the Telegram auth helpers leaves real named-bot messages with + ``account=None``, which routes them to the default session key and the + default egress adapter — i.e. the feature silently no-ops. +""" + +import asyncio +import sys +import types +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + +import pytest + +import gateway.run as gateway_run +from gateway.config import GatewayConfig, HomeChannel, Platform, PlatformConfig +from gateway.platforms.base import BasePlatformAdapter +from gateway.run import GatewayRunner +from gateway.session import SessionSource, build_session_key + + +@pytest.fixture() +def runner(monkeypatch, tmp_path): + fake_dotenv = types.ModuleType("dotenv") + fake_dotenv.load_dotenv = lambda *args, **kwargs: None + monkeypatch.setitem(sys.modules, "dotenv", fake_dotenv) + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + return gateway_run.GatewayRunner(GatewayConfig()) + + +def _telegram_adapter(token="1:x", account=None): + from plugins.platforms.telegram.adapter import TelegramAdapter + + adapter = TelegramAdapter(PlatformConfig(enabled=True, token=token)) + adapter.account_name = account + return adapter + + +# ── Resolution (authz_mixin) ──────────────────────────────────────────────── + + +def test_default_account_resolves_default_adapter(runner): + default_adapter = MagicMock() + runner.adapters = {Platform.TELEGRAM: default_adapter} + assert runner._authorization_adapter(Platform.TELEGRAM) is default_adapter + assert ( + runner._authorization_adapter(Platform.TELEGRAM, account="default") + is default_adapter + ) + + +def test_named_account_resolves_its_own_adapter(runner): + default_adapter, support_adapter = MagicMock(), MagicMock() + runner.adapters = {Platform.TELEGRAM: default_adapter} + runner._account_adapters = {Platform.TELEGRAM: {"support": support_adapter}} + assert ( + runner._authorization_adapter(Platform.TELEGRAM, account="support") + is support_adapter + ) + + +def test_unknown_account_fails_closed_never_default_bot(runner): + """A stamped account whose adapter is missing (failed to connect, + misconfigured) must NOT fall back to the default adapter.""" + runner.adapters = {Platform.TELEGRAM: MagicMock()} + runner._account_adapters = {} + assert runner._authorization_adapter(Platform.TELEGRAM, account="support") is None + + +def test_account_in_secondary_profile_fails_closed(runner): + """Named account + secondary profile is not a supported combination yet, + and is checked before the active-profile fast path.""" + runner._profile_adapters = {"coder": {Platform.TELEGRAM: MagicMock()}} + assert ( + runner._authorization_adapter( + Platform.TELEGRAM, profile="coder", account="support" + ) + is None + ) + + +def test_adapter_for_source_routes_by_account(runner): + default_adapter, support_adapter = MagicMock(), MagicMock() + runner.adapters = {Platform.TELEGRAM: default_adapter} + runner._account_adapters = {Platform.TELEGRAM: {"support": support_adapter}} + + src_default = SessionSource(platform=Platform.TELEGRAM, chat_id="1", chat_type="dm") + src_support = SessionSource( + platform=Platform.TELEGRAM, chat_id="1", chat_type="dm", account="support" + ) + assert runner._adapter_for_source(src_default) is default_adapter + assert runner._adapter_for_source(src_support) is support_adapter + + +def test_bare_fixture_source_reads_as_default_account(runner): + """A SimpleNamespace/MagicMock source auto-creates a truthy non-string + ``account`` (AGENTS.md pitfall #17). That must read as the default + account, not trip the fail-closed branch and silence every reply.""" + default_adapter = MagicMock() + runner.adapters = {Platform.TELEGRAM: default_adapter} + + bare = SimpleNamespace(platform=Platform.TELEGRAM, profile=None) + assert runner._adapter_for_source(bare) is default_adapter + + mock_source = MagicMock() + mock_source.platform = Platform.TELEGRAM + mock_source.profile = None + mock_source._transport_adapter_ref = None + mock_source.delivered_via_upstream_relay = False + assert runner._adapter_for_source(mock_source) is default_adapter + + +def test_mock_account_does_not_corrupt_the_session_key(): + """The same pitfall on the session-key side: a non-string ``account`` + must not be interpolated into the key namespace. Without the guard this + derives ``agent:main@`` — and a different key every run, + because the repr embeds the object id.""" + mock_source = MagicMock() + mock_source.platform = Platform.TELEGRAM + mock_source.chat_id = "777" + mock_source.chat_type = "dm" + mock_source.user_id = "777" + mock_source.thread_id = None + key = build_session_key(mock_source) + assert "MagicMock" not in key + assert key.split(":")[1] == "main" + + +# ── Derived per-account config ────────────────────────────────────────────── + + +def test_account_platform_config_overrides_and_strips_accounts(): + base = PlatformConfig( + enabled=True, + token="123:default", + extra={ + "accounts": {"support": {}}, + "fallback_ips": ["1.2.3.4"], + "allowed_users": [1], + }, + ) + derived = gateway_run.GatewayRunner._account_platform_config( + Platform.TELEGRAM, + base, + { + "token": "456:support", + "allowed_users": [2, 3], + "home_channel": {"chat_id": "-100999"}, + }, + ) + assert derived.token == "456:support" + assert isinstance(derived.home_channel, HomeChannel) + assert derived.home_channel.chat_id == "-100999" + # The platform is implicit inside an account's own home_channel block. + assert derived.home_channel.platform == Platform.TELEGRAM + # Account block overrides platform-level extra; unrelated keys inherit. + assert derived.extra["allowed_users"] == [2, 3] + assert derived.extra["fallback_ips"] == ["1.2.3.4"] + # An account must never be able to spawn accounts. + assert "accounts" not in derived.extra + # The base config is untouched (dataclasses.replace, not mutation). + assert base.token == "123:default" + assert base.extra["allowed_users"] == [1] + + +# ── Lifecycle (_start_account_adapters) ───────────────────────────────────── + + +def _wire_prepare_mocks(runner): + created = [] + + def _fake_create(platform, config): + adapter = MagicMock() + adapter.platform = platform + adapter.config = config + adapter.account_name = None + created.append(adapter) + return adapter + + runner._create_adapter = _fake_create + runner._make_adapter_auth_check = MagicMock(return_value=lambda *a, **kw: True) + runner._recover_telegram_topic_thread_id = lambda _s: None + runner._handle_adapter_fatal_error = AsyncMock() + runner._handle_active_session_busy_message = AsyncMock() + runner._handle_reaction_event = AsyncMock() + runner.session_store = MagicMock() + runner._busy_text_mode = "full" + return created + + +def _accounts_config(token="123:default", **accounts): + return PlatformConfig( + enabled=True, token=token, extra={"accounts": dict(accounts)} + ) + + +def test_prepare_account_adapters_wires_and_stamps(runner): + _wire_prepare_mocks(runner) + cfg = _accounts_config( + support={"token": "456:support"}, sales={"token": "789:sales"} + ) + prepared = runner._prepare_account_adapters(Platform.TELEGRAM, cfg) + + assert [name for name, _, _ in ((a.account_name, c, p) for p, c, a in prepared)] == [ + "support", + "sales", + ] + by_name = {a.account_name: (p, c, a) for p, c, a in prepared} + _, support_cfg, support = by_name["support"] + # Its OWN derived token, not the platform default. + assert support_cfg.token == "456:support" + assert support.config.token == "456:support" + # Wired like a default adapter. + support.set_message_handler.assert_called_once() + support.set_authorization_check.assert_called_once() + support.set_platform_event_handler.assert_called_once() + # Prepare-only: nothing is connected or registered yet — that is what lets + # accounts join the same concurrent fan-out as every other platform. + assert runner._account_adapters == {} + + +def test_tokenless_account_is_skipped(runner): + created = _wire_prepare_mocks(runner) + cfg = _accounts_config(support={"display_name": "no token"}) + assert runner._prepare_account_adapters(Platform.TELEGRAM, cfg) == [] + assert created == [] + + +def test_no_accounts_is_a_noop(runner): + _wire_prepare_mocks(runner) + cfg = PlatformConfig(enabled=True, token="123:default") + assert runner._prepare_account_adapters(Platform.TELEGRAM, cfg) == [] + assert runner._account_adapters == {} + + +# ── Startup fan-out (real GatewayRunner.start) ───────────────────────────── +# +# These drive the real startup path rather than the helper in isolation. That +# matters: the account wiring lives in _connect_platforms' pre-filter and +# aggregation, and the #83791 rewrite made "where it is wired" the part most +# likely to be wrong. + + +class _ScriptedAdapter(BasePlatformAdapter): + """Adapter whose connect() records order and returns a scripted result.""" + + events: list = [] + + def __init__(self, platform, config, label, ok=True, sleep=0.0): + super().__init__(config, platform) + self._label = label + self._ok = ok + self._sleep = sleep + + async def connect(self, *, is_reconnect: bool = False) -> bool: + _ScriptedAdapter.events.append((self._label, "start")) + if self._sleep: + await asyncio.sleep(self._sleep) + _ScriptedAdapter.events.append((self._label, "end")) + if not self._ok: + self._set_fatal_error("bad_token", "unauthorized", retryable=True) + return self._ok + + async def disconnect(self) -> None: + self._mark_disconnected() + + async def send(self, chat_id, content, reply_to=None, metadata=None): + raise NotImplementedError + + async def get_chat_info(self, chat_id): + return {"id": chat_id} + + +def _account_runner(tmp_path, monkeypatch, *, default_token="123:default", **kw): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + _ScriptedAdapter.events = [] + config = GatewayConfig( + platforms={ + Platform.TELEGRAM: PlatformConfig( + enabled=True, + token=default_token, + extra={ + "accounts": { + "support": {"token": "456:support"}, + "sales": {"token": "789:sales"}, + } + }, + ) + }, + sessions_dir=tmp_path / "sessions", + ) + runner = GatewayRunner(config) + monkeypatch.setattr(runner, "_start_secondary_profile_adapters", lambda: 0) + return runner + + +@pytest.mark.asyncio +async def test_named_accounts_connect_concurrently_with_the_default( + tmp_path, monkeypatch +): + """Accounts join the same fan-out (#83791), not a serial tail behind it. + + The default is slow; both accounts are instant. Under a serial start the + default's connect would end before either account's began, so an account + end can never precede the default's. Only overlap puts them first. + """ + runner = _account_runner(tmp_path, monkeypatch) + + def _make(platform, cfg): + if cfg.token == "123:default": + return _ScriptedAdapter(platform, cfg, "default", sleep=0.3) + label = "support" if cfg.token == "456:support" else "sales" + return _ScriptedAdapter(platform, cfg, label) + + monkeypatch.setattr(runner, "_create_adapter", _make) + await runner.start() + + events = _ScriptedAdapter.events + order = [f"{label}:{kind}" for label, kind in events] + assert "support:end" in order and "default:end" in order, order + assert order.index("support:end") < order.index("default:end"), ( + f"accounts did not overlap the default connect (serial tail?): {order}" + ) + assert set(runner._account_adapters[Platform.TELEGRAM]) == {"support", "sales"} + + +@pytest.mark.asyncio +async def test_failing_default_does_not_keep_named_accounts_offline( + tmp_path, monkeypatch +): + """The #67455 review finding: a bad or absent default token must not + block otherwise healthy named bots.""" + runner = _account_runner(tmp_path, monkeypatch) + + def _make(platform, cfg): + if cfg.token == "123:default": + return _ScriptedAdapter(platform, cfg, "default", ok=False) + label = "support" if cfg.token == "456:support" else "sales" + return _ScriptedAdapter(platform, cfg, label) + + monkeypatch.setattr(runner, "_create_adapter", _make) + await runner.start() + + # Default failed and is NOT registered... + assert Platform.TELEGRAM not in runner.adapters + # ...while both named accounts are live. + assert set(runner._account_adapters[Platform.TELEGRAM]) == {"support", "sales"} + + +@pytest.mark.asyncio +async def test_failed_account_never_claims_the_platform_retry_slot( + tmp_path, monkeypatch +): + """A named account owns no entry in self.adapters and must not take the + platform's single _failed_platforms slot — that slot would respawn the + DEFAULT adapter from the account's config.""" + runner = _account_runner(tmp_path, monkeypatch) + + def _make(platform, cfg): + if cfg.token == "456:support": + return _ScriptedAdapter(platform, cfg, "support", ok=False) + label = {"123:default": "default", "789:sales": "sales"}[cfg.token] + return _ScriptedAdapter(platform, cfg, label) + + monkeypatch.setattr(runner, "_create_adapter", _make) + await runner.start() + + # Default is healthy and registered; the failed account did not mark the + # platform for reconnect, and did not evict the default adapter. + assert Platform.TELEGRAM in runner.adapters + assert runner.adapters[Platform.TELEGRAM].config.token == "123:default" + assert Platform.TELEGRAM not in runner._failed_platforms + assert set(runner._account_adapters[Platform.TELEGRAM]) == {"sales"} + + +@pytest.mark.asyncio +async def test_accounts_only_platform_skips_the_tokenless_default_connect( + tmp_path, monkeypatch +): + """Named tokens with no default credential: the doomed token-less default + connect is skipped entirely, but the accounts still come up.""" + runner = _account_runner(tmp_path, monkeypatch, default_token="") + + def _make(platform, cfg): + label = "support" if cfg.token == "456:support" else "sales" + return _ScriptedAdapter(platform, cfg, label) + + monkeypatch.setattr(runner, "_create_adapter", _make) + await runner.start() + + labels = {label for label, _ in _ScriptedAdapter.events} + assert "default" not in labels, "token-less default should never connect" + assert set(runner._account_adapters[Platform.TELEGRAM]) == {"support", "sales"} + assert Platform.TELEGRAM not in runner.adapters + + +# ── Inbound stamping (real TelegramAdapter) ──────────────────────────────── + + +def test_build_source_stamps_account_on_normal_event_path(): + """The regression guard for the #67455 review's first finding. + + Normal traffic does NOT go through the auth helpers — it goes through + ``build_source``. If the stamp is missing here, named-bot messages keep + ``account=None`` and the whole account dimension is inert for real users. + """ + support = _telegram_adapter(account="support") + source = support.build_source(chat_id="777", chat_type="dm", user_id="42") + assert source.account == "support" + + # Default/single-bot adapters stay byte-identical to before. + default = _telegram_adapter(token="1:y") + assert default.build_source(chat_id="777", chat_type="dm", user_id="42").account is None + + +def test_stamped_source_yields_a_per_account_session_key(): + """End-to-end tie-back to the previous slice: the stamp build_source + applies is what makes the same chat two sessions under two bots.""" + support = _telegram_adapter(account="support") + default = _telegram_adapter(token="1:y") + kw = dict(chat_id="777", chat_type="dm", user_id="42") + + support_key = build_session_key(support.build_source(**kw)) + default_key = build_session_key(default.build_source(**kw)) + + assert support_key != default_key + assert support_key.split(":")[1] == "main@support" + assert default_key == "agent:main:telegram:dm:777" + + +def test_telegram_auth_helper_stamps_account(): + """The auth-path source must agree with the normal path, or the + adapter-level guard and the session store derive different keys for the + same event (the #64934 bug class).""" + adapter = _telegram_adapter(account="support") + + message = MagicMock() + message.chat.id = 777 + message.chat.type = "private" + message.chat.title = None + message.from_user.id = 42 + message.from_user.username = "user" + message.from_user.full_name = "User" + message.message_thread_id = None + message.is_topic_message = False + + assert adapter._source_from_message_for_auth(message).account == "support" + assert _telegram_adapter(token="1:y")._source_from_message_for_auth( + message + ).account is None diff --git a/tests/gateway/test_telegram_multi_account_config.py b/tests/gateway/test_telegram_multi_account_config.py new file mode 100644 index 000000000000..94d4fbdca9e7 --- /dev/null +++ b/tests/gateway/test_telegram_multi_account_config.py @@ -0,0 +1,122 @@ +"""Multi-account Telegram configuration parsing — #8287. + +One gateway, N Telegram bot accounts: tokens arrive as +``TELEGRAM_BOT_TOKEN_`` env vars (secrets stay in .env), behavioral +settings as ``platforms.telegram.accounts.`` in config.yaml. The +unsuffixed ``TELEGRAM_BOT_TOKEN`` remains the default account, so single-bot +configurations parse byte-identically to before. +""" + +import pytest + +from gateway.config import Platform, PlatformConfig, load_gateway_config + + +def test_single_bot_config_has_no_accounts_key(monkeypatch, tmp_path): + """Backward compatibility: an unsuffixed token must not grow an + accounts block — existing single-bot setups stay byte-identical.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("TELEGRAM_BOT_TOKEN", "123:default-token") + + config = load_gateway_config() + tg = config.platforms[Platform.TELEGRAM] + assert tg.token == "123:default-token" + assert "accounts" not in tg.extra + + +def test_suffixed_env_tokens_declare_accounts(monkeypatch, tmp_path): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("TELEGRAM_BOT_TOKEN", "123:default-token") + monkeypatch.setenv("TELEGRAM_BOT_TOKEN_SUPPORT", "456:support-token") + monkeypatch.setenv("TELEGRAM_BOT_TOKEN_SALES", "789:sales-token") + + config = load_gateway_config() + tg = config.platforms[Platform.TELEGRAM] + + # Default account untouched. + assert tg.token == "123:default-token" + # Suffix names are lowercased account names carrying only the credential. + accounts = tg.extra["accounts"] + assert accounts["support"]["token"] == "456:support-token" + assert accounts["sales"]["token"] == "789:sales-token" + + +def test_suffixed_token_alone_enables_platform(monkeypatch, tmp_path): + """A gateway configured with only account tokens (no default) still + enables Telegram — the registry decides which accounts to start.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.delenv("TELEGRAM_BOT_TOKEN", raising=False) + monkeypatch.setenv("TELEGRAM_BOT_TOKEN_SUPPORT", "456:support-token") + + config = load_gateway_config() + tg = config.platforms[Platform.TELEGRAM] + assert tg.enabled + assert tg.token is None + assert tg.extra["accounts"]["support"]["token"] == "456:support-token" + + +def test_empty_suffix_or_value_is_ignored(monkeypatch, tmp_path): + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("TELEGRAM_BOT_TOKEN", "123:default-token") + monkeypatch.setenv("TELEGRAM_BOT_TOKEN_", "999:no-name") + monkeypatch.setenv("TELEGRAM_BOT_TOKEN_EMPTY", "") + + config = load_gateway_config() + tg = config.platforms[Platform.TELEGRAM] + assert "accounts" not in tg.extra + + +def test_yaml_accounts_block_parses_and_normalizes_names(): + cfg = PlatformConfig.from_dict({ + "enabled": True, + "accounts": { + "Support": {"display_name": "Support Bot", "allowed_users": [1, 2]}, + " SALES ": {"home_channel": {"chat_id": "-100123"}}, + }, + }) + accounts = cfg.extra["accounts"] + assert set(accounts) == {"support", "sales"} + assert accounts["support"]["display_name"] == "Support Bot" + assert accounts["support"]["allowed_users"] == [1, 2] + + +def test_yaml_accounts_survive_via_extra_bridge(): + """The shared-key loop can bridge accounts into extra — both routes + normalize identically (the gateway_restart_notification pattern).""" + cfg = PlatformConfig.from_dict({ + "enabled": True, + "extra": {"accounts": {"Support": {"display_name": "S"}}}, + }) + assert cfg.extra["accounts"]["support"]["display_name"] == "S" + + +def test_env_token_merges_into_yaml_account_block(monkeypatch, tmp_path): + """config.yaml declares the behavioral block; .env supplies the token. + The two merge on the same account name.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + (tmp_path / "config.yaml").write_text( + "gateway:\n" + " platforms:\n" + " telegram:\n" + " enabled: true\n" + " accounts:\n" + " support:\n" + " display_name: Support Bot\n", + encoding="utf-8", + ) + monkeypatch.setenv("TELEGRAM_BOT_TOKEN_SUPPORT", "456:support-token") + + config = load_gateway_config() + tg = config.platforms[Platform.TELEGRAM] + support = tg.extra["accounts"]["support"] + assert support["token"] == "456:support-token" + assert support.get("display_name") == "Support Bot" + + +def test_accounts_round_trip_through_to_dict(): + cfg = PlatformConfig.from_dict({ + "enabled": True, + "accounts": {"support": {"display_name": "S"}}, + }) + rebuilt = PlatformConfig.from_dict(cfg.to_dict()) + assert rebuilt.extra["accounts"]["support"]["display_name"] == "S" diff --git a/tests/gateway/test_telegram_multi_account_sessions.py b/tests/gateway/test_telegram_multi_account_sessions.py new file mode 100644 index 000000000000..b971465a4c05 --- /dev/null +++ b/tests/gateway/test_telegram_multi_account_sessions.py @@ -0,0 +1,108 @@ +"""Per-account session identity — #8287. + +A gateway hosting multiple bot accounts on one platform must keep their +conversations apart: the same chat reached through two bots is two sessions. +The account rides in the session-key namespace slot (``agent:main@support``) +— the same mechanism profiles use — so every positional parser +(``parts[2] == platform`` etc.) keeps its layout, and single-bot gateways +produce byte-identical keys to before. +""" + +from gateway.config import Platform +from gateway.run import _parse_session_key +from gateway.session import ( + SessionSource, + build_session_key, + split_key_namespace, +) + + +def _source(account=None, **kw): + defaults = dict( + platform=Platform.TELEGRAM, chat_id="777", chat_type="dm", user_id="777" + ) + defaults.update(kw) + return SessionSource(account=account, **defaults) + + +def test_same_chat_two_bots_two_sessions(): + """The #10455-review isolation requirement: identical chat + user via + two different bot accounts must never share a session key.""" + key_default = build_session_key(_source(account=None)) + key_support = build_session_key(_source(account="support")) + key_sales = build_session_key(_source(account="sales")) + assert len({key_default, key_support, key_sales}) == 3 + + +def test_default_account_key_is_byte_identical_to_legacy(): + """Single-bot gateways must keep every key they have ever generated.""" + assert build_session_key(_source(account=None)) == "agent:main:telegram:dm:777" + assert build_session_key(_source(account="default")) == "agent:main:telegram:dm:777" + + +def test_account_key_keeps_positional_layout(): + """The account lives in the namespace slot — platform/chat_type/chat_id + stay at parts[2:5], so positional parsers are unaffected.""" + key = build_session_key(_source(account="support")) + parts = key.split(":") + assert parts[0] == "agent" + assert parts[1] == "main@support" + assert parts[2] == "telegram" + assert parts[3] == "dm" + assert parts[4] == "777" + + +def test_profile_and_account_compose(): + key = build_session_key(_source(account="support"), profile="coder") + assert key.startswith("agent:coder@support:telegram:") + + +def test_group_and_thread_keys_carry_account(): + group_a = build_session_key( + _source(account="support", chat_type="group", chat_id="-100", user_id="9") + ) + group_b = build_session_key( + _source(account=None, chat_type="group", chat_id="-100", user_id="9") + ) + assert group_a != group_b + assert group_a.split(":")[1] == "main@support" + + +def test_source_account_round_trips_serialization(): + src = _source(account="support") + rebuilt = SessionSource.from_dict(src.to_dict()) + assert rebuilt.account == "support" + # Default account stays wire-invisible (no key emitted), like profile. + assert "account" not in _source(account=None).to_dict() + + +def test_split_key_namespace(): + assert split_key_namespace("main") == ("main", None) + assert split_key_namespace("main@support") == ("main", "support") + assert split_key_namespace("coder@support") == ("coder", "support") + assert split_key_namespace("") == ("", None) + + +def test_profile_resolution_ignores_account_suffix(): + from gateway.session import SessionStore + + resolve = SessionStore._profile_from_session_key + assert resolve("agent:main:telegram:dm:1") == "default" + assert resolve("agent:main@support:telegram:dm:1") == "default" + assert resolve("agent:coder@support:telegram:dm:1") == "coder" + + +def test_parse_session_key_accepts_account_namespace(): + parsed = _parse_session_key("agent:main@support:telegram:dm:777:42") + assert parsed == { + "platform": "telegram", + "chat_type": "dm", + "chat_id": "777", + "account": "support", + "thread_id": "42", + } + # Default-namespace behavior unchanged. + legacy = _parse_session_key("agent:main:telegram:dm:777") + assert legacy == {"platform": "telegram", "chat_type": "dm", "chat_id": "777"} + # Named-profile keys stay excluded, as before. + assert _parse_session_key("agent:coder:telegram:dm:777") is None