Skip to content

feat(gateway): account-aware adapter registry + inbound stamping (#8287, 3/6) - #87556

Open
Hotragn wants to merge 4 commits into
NousResearch:mainfrom
Hotragn:feat/8287-03-adapter-registry
Open

feat(gateway): account-aware adapter registry + inbound stamping (#8287, 3/6)#87556
Hotragn wants to merge 4 commits into
NousResearch:mainfrom
Hotragn:feat/8287-03-adapter-registry

Conversation

@Hotragn

@Hotragn Hotragn commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Third slice of #67455, split for review. 1/6 is #86497 (config parsing), 2/6 is #86532 (session identity). This slice brings named bot accounts online and stamps their inbound traffic — which is what turns 2/6's per-account session keys from a data structure into observable behaviour.

Stacked on 2/6; the reviewable commits are the last two.

What this does

Registry. _account_adapters[platform][name] mirrors the proven _profile_adapters two-level registry. self.adapters stays the default account's map, so every existing self.adapters[...] site is untouched when no named accounts are configured — the dict is simply empty.

Fail-closed resolution. _authorization_adapter gains the account dimension with the same contract profiles already have: a stamped account with no registry entry resolves to None, never the default bot. Replying out the wrong bot is worse than not replying. A named account inside a secondary profile is not a supported combination yet and also fails closed — checked before the active-profile fast path, so it holds when that profile is the active one too.

Derived per-account config. Each account adapter is handed an ordinary PlatformConfig — its own token, its own home_channel (the platform is implicit inside an account's own block), account-block settings overriding platform extra. Adapter internals stay completely account-agnostic. The accounts map is stripped from the derived extra: an account must never be able to spawn accounts.

The stamp lands on the real inbound path. This is the part worth reviewing closely. The account is stamped in BasePlatformAdapter.build_source() — the single construction site every platform's normal event flows through. Stamping only the Telegram auth helpers would leave ordinary named-bot traffic at account=None, routing it to the default session key and default egress: the feature would silently no-op for real messages while multi-account tests passed. Both Telegram auth-path helpers are stamped as well, so the adapter-level guard and the session store cannot derive different keys for the same event (the #64934 bug class).

Startup rides the parallel fan-out from #83791

Named accounts join the same _pending_connects fan-out the default adapter uses, rather than being started in a loop after it. _prepare_account_adapters() only creates and wires; the (platform, config, adapter) triples then connect concurrently with every other platform. A serial tail here would have reintroduced exactly the head-of-line blocking #83791 removed, at N accounts × one connect timeout each.

Consequences that fall out of sharing that path, each covered by a test that drives the real start():

  • Named accounts no longer depend on the default adapter's outcome, so a bad or absent default token cannot keep healthy named bots offline (review finding below).
  • Registration happens in the single-threaded aggregation loop, keyed off adapter.account_name, so shared state is mutated exactly as before.
  • A named account owns no entry in self.adapters and never claims the platform's single _failed_platforms retry slot — that slot would respawn the default adapter from the account's config.
  • The platform runtime-status row reports the default account's health, so an account connect neither flips it to connecting nor marks the platform down while the default bot is serving fine.
  • An accounts-only platform (named tokens, no default credential) skips the doomed token-less default connect entirely.

Second commit: a latent bug this slice would have activated

build_session_key read source.account with a bare getattr. A MagicMock/SimpleNamespace source auto-creates a truthy non-string attribute for any name (AGENTS.md pitfall #17, already guarded for profile), so such a fixture derived

agent:main@<MagicMock name='mock.account' id='2071...'>:telegram:dm:777

— a corrupted key, and a different key on every run because the repr embeds the object id.

This was latent while nothing stamped the field: before this slice, source.account was only ever set by an explicit test. The build_source() stamp is what puts a real value on that attribute for the first time, so it is hardened in the same series rather than left as a trap. Coerced to str-or-None, matching the guard applied in _authorization_adapter, so both readers of the account dimension agree on what "no account" means.

Review findings from #67455 folded in

Two of the four findings on the umbrella PR belong to this slice and are addressed here:

  • "Normal Telegram events call self.build_source() … ordinary named-bot traffic still gets the default session key and default egress adapter." → stamped in build_source(), with test_build_source_stamps_account_on_normal_event_path as the regression guard for exactly that miss.
  • "Named accounts start only after the default adapter connects. A bad default token blocks otherwise healthy named bots." → they now connect concurrently with it, proven end-to-end by test_failing_default_does_not_keep_named_accounts_offline.

The remaining two (reconnect queueing, account-aware hermes send) belong to slices 4/6 and 5/6.

Deliberately out of scope

Per-account reconnect queueing lands with the delivery/reconnect slice (4/6). Until then a named account that fails to connect at startup is logged, disconnected cleanly (never leaked), and retried at the next gateway start — without affecting the default account or its siblings.

Tests

18 tests in tests/gateway/test_telegram_multi_account_adapters.py.

Unit: resolution (default / named / unknown-fails-closed / account-under-secondary-profile / non-string fixture reads as default), the derived config (overrides, home-channel platform defaulting, accounts stripped, base config not mutated), and prepare-only wiring.

Stamping goes through a real TelegramAdapter on both the build_source path and the auth-helper path, including the end-to-end tie-back that the stamp is what makes one chat two session keys under two bots.

Startup is exercised through the real GatewayRunner.start(), reusing the harness #83791 added — including its event-order (not wall-clock) technique for proving overlap, since time.monotonic() on Windows is too coarse to distinguish serial from parallel:

  • test_named_accounts_connect_concurrently_with_the_default
  • test_failing_default_does_not_keep_named_accounts_offline
  • test_failed_account_never_claims_the_platform_retry_slot
  • test_accounts_only_platform_skips_the_tokenless_default_connect

tests/gateway/test_startup_connect_parallel.py (#83791's own regression tests) stays green.

@Hotragn
Hotragn marked this pull request as draft August 16, 2026 08:43
First slice of the account-aware gateway: configuration surface only,
no runtime behavior change.

- TELEGRAM_BOT_TOKEN_<ACCOUNT> env vars declare additional bot accounts
  (lowercased names); the unsuffixed TELEGRAM_BOT_TOKEN remains the
  default account, so single-bot setups parse byte-identically. Tokens
  are secrets: env/.env is their supported home.
- platforms.telegram.accounts.<name> in config.yaml carries the
  behavioral per-account settings (display names, allowlists, home
  channels) and merges with env tokens on the account name; the block
  arrives top-level or bridged into extra (the same two-route pattern
  as gateway_restart_notification) and round-trips through to_dict.

Registry, session-key, and routing slices follow in this branch per
the acceptance architecture in the NousResearch#10455 review.
Second slice: the same chat reached through two bot accounts is two
sessions.

- SessionSource.account carries which bot received the message (stamped
  by the adapter in the upcoming registry slice); wire-invisible when
  unset, serialized like profile.
- The account rides in the session-key NAMESPACE slot — the same
  mechanism profiles use: agent:main@support / agent:coder@support.
  Positional parsers keep their layout (parts[2] == platform), and
  single-bot gateways produce byte-identical keys (locked by test).
- build_session_key reads the account from the SOURCE, not a caller
  parameter, so the adapter-level guard and the session store derive
  the same key for the same event — per-key guards diverging is the
  NousResearch#64934 bug class, and this keeps that door shut by construction.
- The two namespace readers are account-aware via a shared helper:
  _profile_from_session_key strips the suffix instead of resolving
  'main@support' as a profile name, and _parse_session_key accepts the
  suffixed default namespace (named-profile keys stay excluded).
- Account names are charset-restricted at config parse ([a-z0-9][a-z0-9_-]*)
  so ':' and '@' can never reach a key.

Includes the NousResearch#10455-review isolation test: same chat + user via two
accounts yields distinct keys.
…sResearch#8287)

Third slice: one adapter instance per named bot account, and the inbound
stamp that makes the previous slice's per-account session keys light up.

- _account_adapters[platform][name] mirrors the proven _profile_adapters
  two-level registry: self.adapters stays the default account's map, so
  every existing self.adapters[...] site is untouched when no named
  accounts are configured (the dict is empty).
- _authorization_adapter gains the account dimension with the same
  fail-closed contract as profiles: a stamped account with no registry
  entry resolves to None rather than the default bot — replying out the
  wrong bot is worse than not replying. _adapter_for_source reads
  source.account, so inbound routing follows the stamp automatically.
  A named account under a secondary profile is not a supported
  combination yet and also fails closed, checked before the
  active-profile fast path so it holds for a named ACTIVE profile too.
- Each account adapter sees an ordinary derived PlatformConfig (its own
  token, its own home_channel with the platform implicit, account-block
  settings overriding platform extra) — adapter internals stay
  account-agnostic. The accounts map is stripped from the derived extra:
  an account must never be able to spawn accounts.
- The account is stamped in BasePlatformAdapter.build_source(), the
  single construction site every platform's NORMAL inbound event flows
  through. Stamping only the Telegram auth helpers would leave ordinary
  named-bot traffic at account=None — default session key, default
  egress — i.e. the feature would silently no-op for real messages.
  Both Telegram auth-path helpers are stamped too, so the adapter-level
  guard and the session store cannot derive different keys for one event
  (the NousResearch#64934 bug class).

Startup rides the parallel connect fan-out added by NousResearch#83791 rather than
bolting a serial tail onto it. _prepare_account_adapters() creates and
wires the account adapters, and their (platform, config, adapter) triples
join the same _pending_connects list the default adapter uses, so named
bots connect concurrently with every other platform — connecting them in
a loop would reintroduce exactly the head-of-line blocking that PR
removed, at N accounts x one timeout each. Consequences, all covered by
tests that drive the real start():

- Named accounts no longer depend on the default adapter's outcome, so a
  bad or absent default token cannot keep healthy named bots offline
  (NousResearch#67455 review finding).
- Registration happens in the single-threaded aggregation loop keyed off
  adapter.account_name, so shared state is mutated exactly as before.
- A named account owns no entry in self.adapters and never claims the
  platform's single _failed_platforms retry slot — that slot would
  respawn the DEFAULT adapter from the account's config.
- The platform runtime-status row reports the DEFAULT account's health,
  so an account connect neither flips it to "connecting" nor marks the
  platform down while the default bot is serving fine.
- An accounts-only platform (named tokens, no default credential) skips
  the doomed token-less default connect entirely.

Per-account reconnect queueing, delivery/status/cron consumers, and
setup UX land in the remaining slices; a named account that fails to
connect at startup is disconnected cleanly and retried at the next
gateway start until then.
…ousResearch#8287)

build_session_key read source.account with a bare getattr. A MagicMock or
SimpleNamespace source auto-creates a truthy NON-STRING attribute for any
name (AGENTS.md pitfall NousResearch#17, already guarded for `profile`), so such a
fixture derived

    agent:main@<MagicMock name='mock.account' id='...'>:telegram:dm:777

instead of the default namespace — a corrupted key, and a different key on
every run because the repr embeds the object id.

This was latent while nothing stamped the field: before the registry slice,
source.account was only ever set by an explicit test. The stamp added in
build_source() is what puts a real value on the attribute for the first
time, so harden the read in the same series rather than leave a trap for
the next fixture that flows through.

isinstance-coerce to str-or-None, matching the guard the same commit
applies in _authorization_adapter, so both readers of the account
dimension agree on what "no account" means.
@Hotragn
Hotragn force-pushed the feat/8287-03-adapter-registry branch from 4128aff to 2f99ade Compare August 16, 2026 08:59
@Hotragn
Hotragn marked this pull request as ready for review August 16, 2026 09:00
@alt-glitch alt-glitch added type/feature New feature or request comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins platform/telegram Telegram bot adapter P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 16, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

feat(gateway): account-aware adapter registry + inbound stamping (#8287, 3/6)

  1. Failed account adapters have no retry until next gateway start. _prepare_account_adapters + the aggregation loop disconnect and free a failed named account (gateway/run.py), with per-account reconnect deferred to a future slice. A transient connect failure therefore takes the named bot offline for the whole gateway lifetime. Consider wiring account adapters into the existing reconnect path (or at least one retry via _connect_initial_adapter_with_timeout) before this lands — the deliberate avoidance of _failed_platforms is correct, but the account needs some retry mechanism.
  2. Fail-closed resolution is silent. In gateway/authz_mixin._authorization_adapter, a stamped account with no registry entry returns None with no logging — a misconfigured or failed account silently swallows every inbound message. A warning-level log per fail-closed event would make the failure diagnosable.
  3. Other namespace consumers need the @account suffix stripped. The PR updates _profile_from_session_key and _parse_session_key, but any remaining code that matches session keys by profile-namespace prefix (e.g. agent:main:% / agent:coder:% LIKE filters for profile-scoped session listing) will silently miss agent:main@support:... / agent:coder@support:... rows. Grep for other raw parts[1] / namespace-prefix consumers and route them through split_key_namespace.
  4. Account-block keys override platform-level extra wholesale in _account_platform_config (e.g. an account allowed_users replaces the platform-wide list). A typo'd or unintended account key silently overrides a global setting; consider an explicit allowlist of per-account keys.

Solid, well-tested slice overall (fail-closed contract, concurrent fan-out, byte-identical single-bot keys all covered) — items (1)-(3) are the ones to resolve across the 3/6 series.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/telegram Telegram bot adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants