fix(gateway): namespace telegram topic mode by profile under multiplex - #76487
fix(gateway): namespace telegram topic mode by profile under multiplex#76487crdesign8 wants to merge 4 commits into
Conversation
Issue NousResearch#76423: under multiplex_profiles a shared state.db keyed topic mode and bindings only by Telegram chat_id/thread_id, so private-chat ids collided across bots/profiles. - Add profile_name to telegram_dm_topic_mode and telegram_dm_topic_bindings - Schema v2→v3 rebuild; legacy rows migrate into the "default" namespace - Keyword-only profile_name="default" on SessionDB topic APIs (compat)
Issue NousResearch#76423 follow-up: wire SessionDB profile_name through gateway paths. - Resolve profile from source.profile (never process-global active profile) - Stamp adapter._hermes_profile_name for prune under multiplex - /topic enable/status and binding record/recover/disable/restore paths
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the shared-state.db collision and covering the schema migration plus routed SessionDB calls. The core premise is confirmed on current main: hermes_state.py:7965-7988 keys the topic tables only by Telegram IDs.
Problems
plugins/platforms/telegram/adapter.py:1323derives the prune namespace from a static adapter stamp. That is unsafe forprofile_routes:gateway/platforms/base.py:6638-6641retains the receiving transport adapter even whensource.profileselects a different runtime, andgateway/authz_mixin.py:101-107returns that transport for delivery. A primary adapter can therefore prune the default namespace for a routed non-default turn.- Runner-level topic cooldowns are still keyed only by
chat_id(gateway/run.py:6431-6439,19187-19195), so profiles sharing a Telegram DM ID suppress each other's reminder/setup-hint messages. website/docs/user-guide/messaging/telegram.md:830-852documents the old table keys and unscoped manual cleanup SQL.
Suggested changes
- Carry the routed profile through outbound Telegram metadata and use it for stale-binding pruning; add a primary-adapter/profile-route regression test.
- Namespace the two cooldown maps and
/topic offcleanup by(profile, chat_id). - Update the Telegram persistence and cleanup documentation.
Automated hermes-sweeper review.
Address hermes-sweeper review on NousResearch#76487: - Prefer hermes_profile from send metadata when pruning stale topic bindings so profile_routes cannot delete the transport adapter's namespace instead of the routed runtime's - Namespace lobby/capability cooldowns and /topic off cleanup by (profile, chat_id) - Document profile_name PKs and scoped cleanup SQL in telegram.md - Regression: primary-adapter stamp + routed metadata prune isolation
|
Addressed the sweeper review points:
New commit: |
SummaryOne PR addresses #76423. #76487 fixes the reported shared-state collision by adding a profile dimension to Telegram topic-mode state and propagating the routed profile through database, gateway, pruning, cooldown, test, and documentation paths. Related pull requests
Suggested consolidationKeep #76487 open with a salvage path: retain the profile-scoped schema migration, routed-profile SessionDB calls, metadata-based prune namespace, profile-scoped cooldowns, documentation updates, and regression coverage, then obtain contributor re-review of the changes made in response to the keep_open review. There are no duplicate PRs to close. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I76423(["issue #76423 (open)"])
P76487["PR #76487 (open)"]
P76487 -->|best fix| I76423
class I76423 open
class P76487 open
class P76487 best
class P76487 target
click I76423 "https://github.com/NousResearch/hermes-agent/issues/76423"
click P76487 "https://github.com/NousResearch/hermes-agent/pull/76487"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 58 kB of PR diffs, 8 kB of issue/PR text, 5 kB of discussion (4 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
…profile-dimension # Conflicts: # hermes_state.py
|
This is the second time since I made this PR that it has major conflicts. I solved first one, and all the requirements needed, but this is awaiting for review and workflow approval for couple days. So, I'll not fix those conflicts anymore, because it's waste of time, no one is looking for it or another PRs that was made for non contributer, even if it's a bug solver. Thanks anyway, @teknium1 and @GottZ . |
What does this PR do?
When
multiplex_profiles: true, every profile shares onestate.db. The Telegram topic-mode tables were keyed only by Telegram identifiers:telegram_dm_topic_mode— PK(chat_id)telegram_dm_topic_bindings— PK(chat_id, thread_id)In private chats,
chat_idis the user's Telegram id — identical across bots. Multiple profiles therefore collide on the same rows: last write wins, thread bindings get overwritten, replies fail with "Message thread not found."This PR adds a
profile_namedimension to both tables and threads the routed profile (source.profile) through every gateway access path. Legacy rows migrate into the"default"namespace only (no replication across configured profiles).Split into two commits for review:
fix(state)— schema v3 + SessionDB API (profile_name="default"keyword-only for compat)fix(gateway)— wiresource.profileinto call sites; stamp adapters for prune under multiplexRelated Issue
Fixes #76423
Type of Change
Changes Made
Commit 1 — state layer
hermes_state.pyprofile_name TEXT NOT NULL DEFAULT 'default'on both topic tables(profile_name, chat_id)and(profile_name, chat_id, thread_id)telegram_dm_topic_schema_version→ v3 (table rebuild; legacy →default)profile_name="default"on enable/disable/is/get/list/bind/delete/list_unlinkedtests/gateway/test_telegram_topic_profile_isolation_76423.py(new) — migration + cross-profile isolation + default-kwarg compattests/test_hermes_state.py— expect schema version"3"Commit 2 — gateway wiring
gateway/run.py—_telegram_topic_profile_name(source)fromsource.profile(never process-global active profile); pass through mode/bind/recover/disable/restore/rename; stampadapter._hermes_profile_nameon primary + secondary adaptersgateway/slash_commands.py—/topicenable + binding statusplugins/platforms/telegram/adapter.py— prune uses adapter profile stamptests/gateway/test_telegram_topic_profile_routing_76423.py(new) — routed profile isolation at gateway layerHow to Test
Checkout the branch
Activate the project venv
Focused suite (CI-parity wrapper):
Expected: 31 passed.
Optional manual (multiplex fleet):
multiplex_profiles: truewith two Telegram-using profiles/topicon bot A, open a topic, chatChecklist
Code
fix(scope):,feat(scope):, etc.)I've run— full suite is ~70+ minutes locally; not run end-to-end here. Focused suite for this change passes (see How to Test). CI will run the full matrix on the PR.pytest tests/ -qand all tests passDocumentation & Housekeeping
cli-config.yaml.example(no new config keys)CONTRIBUTING.mdorAGENTS.mdNotes
source.profile or "default". They intentionally do not callget_active_profile_name()while handling an event — that would reintroduce cross-profile mis-attribution on a sharedstate.db.profile_name = 'default'only. We do not copy bindings to every configured profile (collision-contaminated data would be multiplied).Focused verification: