fix(gateway): per-profile session isolation for shared-WhatsApp multiplex - #69042
fix(gateway): per-profile session isolation for shared-WhatsApp multiplex#69042lsshawn wants to merge 2 commits into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing a real multiplex storage seam. The current gateway still constructs one default-scoped SessionStore (gateway/run.py:3439-3448) and its initial SQLite handle is default-scoped (gateway/session.py:1205-1210).
Problems
- The proposed
_dbproperty does not scope the rest ofSessionStore:_entriesand_loadedare single-store state (gateway/session.py:1172-1174,1254-1257), and the routing mirror still reads/writesself.sessions_dir / "sessions.json"(gateway/session.py:1283-1287,1464-1468). A profile-specific DB selected after another profile has loaded the store would therefore use shared in-memory routing state and the default routing directory. - Physical per-profile
state.dbstorage needs an explicit contract decision first. Current docs specify profile-namespaced keys in a shared store (website/docs/user-guide/multi-profile-gateways.md:193-199), and the linked #66887 discussion identifies that choice as unresolved. - The PR has no tests for secondary-profile persistence, restart recovery, or shared-WhatsApp startup.
Suggested changes
- Select the storage contract, then scope the full storage unit per profile if physical stores are intended; add an E2E temp-home regression covering creation and recovery.
gateway/platforms/base.py:6439-6485already stamps routedsource.profile, so avoid duplicating route resolution unless covering a verified bypass path.
Automated hermes-sweeper review.
| # Initialize SQLite session database | ||
| self._db = None | ||
| # Initialize SQLite session database. | ||
| # [LOCAL PATCH] Under multiplex, self._db must resolve to the CURRENT |
There was a problem hiding this comment.
Making only _db profile-sensitive leaves this SessionStore's single _entries map, _loaded flag, and default sessions_dir shared across profiles. After one profile loads the store, a second profile will not load its own routing rows, while sessions.json remains under the default directory. Please scope the whole storage unit per profile after the shared-vs-physical persistence contract is selected.
|
Good review — you talked me out of two thirds of this PR. Narrowed in 77f969c. Storage contract. You're right that this needed deciding before the code, and on rereading, the docs already decide it: Duplicated route resolution. Also right, and dropped. Small correction on the pointer, in case it matters for future reviews: the citation
What's left is the startup fix only: WhatsApp joins Relay as shared process-level ingress under multiplexing. The bridge is a single authenticated session tied to one phone number, so a secondary profile has no credentials of its own to bring — building an adapter for it only produces a connect/retry loop that stalls startup for every profile queued behind it. Tests, as requested for the shared-WhatsApp startup path: a secondary profile skips WhatsApp while its other platforms still start (the skip doesn't abort the profile), and the skip stays gated on I didn't add the persistence/restart-recovery E2E — with the storage changes gone there's no longer a storage behavior in this PR to regress. That belongs with the contract decision above. 79 tests pass across the multiplex suites; ruff clean. On a full |
…plex Under gateway.multiplex_profiles with a single shared WhatsApp (Baileys) account routed to multiple profiles via gateway.profile_routes, inbound messages were served by the DEFAULT profile's config/skills/memory/session DB even when a profile_route matched — leaking default-profile data into routed replies. Three related gaps: 1. run.py _start_one_profile_adapters: secondary profiles tried to start their OWN WhatsApp adapter (no per-profile creds → retry-loop stalls startup). WhatsApp is a single-shared-connection platform like RELAY, so skip it for secondaries (mirrors the existing RELAY skip). The default profile owns the one connection; profile_routes routes groups. 2. session.py _resolve_profile_for_key: when source.profile was unset, the session-key namespace fell back to the active profile (default) even when a profile_route matched — so routed sessions landed in agent:main and the default state.db. Now consults gateway.profile_routes and stamps source.profile so the key namespace agrees with routing. 3. session.py SessionStore._db + run.py session creation: SessionStore held a single default-home SessionDB; every session's history/search/persistence resolved to default's state.db regardless of profile. Made _db a per-home-cached property (resolves get_hermes_home()/state.db under the active ContextVar scope), and wrapped inbound get_or_create_session in _profile_runtime_scope so the session is physically created in the routed profile's state.db. Net: a WhatsApp group routed to profile X now runs fully as X — its own skills, memory, and state.db — and session_search no longer surfaces another profile's history. Multiplex-off behavior is byte-identical (all new code is gated on multiplex_profiles). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses review feedback on NousResearch#69042 by dropping the two storage changes and keeping only the part that does not depend on an unresolved contract. Reverted (session.py is now byte-identical to base): * The per-home _db property. As reviewed, it scoped only the SQLite handle while _entries, _loaded, and the sessions.json routing mirror stayed single-store — and, more fundamentally, physical per-profile state.db contradicts the documented contract of profile-namespaced keys in a SHARED store (multi-profile-gateways.md, "Session keys are namespaced by profile"). That contract choice is yours to make, so I am not front-running it here; happy to reopen separately if you do want physical per-profile stores, in which case the whole storage unit needs scoping, not just the handle. * The duplicate profile_routes resolution in _resolve_profile_for_key. You were right that this was redundant — build_source already resolves the route and stamps source.profile via _profile_name_for_source, and _resolve_profile_for_key reads that field first. I could not identify a real bypass path, so the duplication is gone. What remains is the startup fix, unchanged in behavior: WhatsApp joins Relay as shared process-level ingress under multiplexing. The bridge is one authenticated session tied to a single phone number, so a secondary profile has no credentials to connect with and building an adapter for it only produces a connect/retry loop that stalls startup for every profile queued behind it. Adds the requested regression coverage: a secondary profile skips WhatsApp while its other platforms still start (the skip does not abort the profile), and the skip stays gated on multiplex_profiles so single-profile installs are untouched. Verified the first test fails without the fix.
77f969c to
579feb4
Compare
|
Rebased onto current Heads-up from the rebase: Net diff is one behavioral line in |
…ofiles _start_one_profile_adapters skipped only Platform.RELAY as shared process-level ingress. WhatsApp is the same shape: the bridge is one authenticated session tied to a single phone number, so a secondary profile has no credential of its own to bring; constructing an adapter for it only produced a connect/retry loop that stalled startup for every profile queued behind it. Treat WhatsApp like Relay -- the active profile owns the connection and route-stamped source.profile fans inbound turns out to secondary profiles. Salvage of #69042 (narrowed by its author to this one behavioral line); test re-expressed on the current secondary-startup fixtures. Co-authored-by: sshawn <28279366+lsshawn@users.noreply.github.com>
…ofiles _start_one_profile_adapters skipped only Platform.RELAY as shared process-level ingress. WhatsApp is the same shape: the bridge is one authenticated session tied to a single phone number, so a secondary profile has no credential of its own to bring; constructing an adapter for it only produced a connect/retry loop that stalled startup for every profile queued behind it. Treat WhatsApp like Relay -- the active profile owns the connection and route-stamped source.profile fans inbound turns out to secondary profiles. Salvage of #69042 (narrowed by its author to this one behavioral line); test re-expressed on the current secondary-startup fixtures. Co-authored-by: sshawn <28279366+lsshawn@users.noreply.github.com>
…ofiles _start_one_profile_adapters skipped only Platform.RELAY as shared process-level ingress. WhatsApp is the same shape: the bridge is one authenticated session tied to a single phone number, so a secondary profile has no credential of its own to bring; constructing an adapter for it only produced a connect/retry loop that stalled startup for every profile queued behind it. Treat WhatsApp like Relay -- the active profile owns the connection and route-stamped source.profile fans inbound turns out to secondary profiles. Salvage of #69042 (narrowed by its author to this one behavioral line); test re-expressed on the current secondary-startup fixtures. Co-authored-by: sshawn <28279366+lsshawn@users.noreply.github.com>
…ofiles _start_one_profile_adapters skipped only Platform.RELAY as shared process-level ingress. WhatsApp is the same shape: the bridge is one authenticated session tied to a single phone number, so a secondary profile has no credential of its own to bring; constructing an adapter for it only produced a connect/retry loop that stalled startup for every profile queued behind it. Treat WhatsApp like Relay -- the active profile owns the connection and route-stamped source.profile fans inbound turns out to secondary profiles. Salvage of #69042 (narrowed by its author to this one behavioral line); test re-expressed on the current secondary-startup fixtures. Co-authored-by: sshawn <28279366+lsshawn@users.noreply.github.com>
…ofiles _start_one_profile_adapters skipped only Platform.RELAY as shared process-level ingress. WhatsApp is the same shape: the bridge is one authenticated session tied to a single phone number, so a secondary profile has no credential of its own to bring; constructing an adapter for it only produced a connect/retry loop that stalled startup for every profile queued behind it. Treat WhatsApp like Relay -- the active profile owns the connection and route-stamped source.profile fans inbound turns out to secondary profiles. Salvage of #69042 (narrowed by its author to this one behavioral line); test re-expressed on the current secondary-startup fixtures. Co-authored-by: sshawn <28279366+lsshawn@users.noreply.github.com>
…ofiles _start_one_profile_adapters skipped only Platform.RELAY as shared process-level ingress. WhatsApp is the same shape: the bridge is one authenticated session tied to a single phone number, so a secondary profile has no credential of its own to bring; constructing an adapter for it only produced a connect/retry loop that stalled startup for every profile queued behind it. Treat WhatsApp like Relay -- the active profile owns the connection and route-stamped source.profile fans inbound turns out to secondary profiles. Salvage of #69042 (narrowed by its author to this one behavioral line); test re-expressed on the current secondary-startup fixtures. Co-authored-by: sshawn <28279366+lsshawn@users.noreply.github.com>
…ofiles _start_one_profile_adapters skipped only Platform.RELAY as shared process-level ingress. WhatsApp is the same shape: the bridge is one authenticated session tied to a single phone number, so a secondary profile has no credential of its own to bring; constructing an adapter for it only produced a connect/retry loop that stalled startup for every profile queued behind it. Treat WhatsApp like Relay -- the active profile owns the connection and route-stamped source.profile fans inbound turns out to secondary profiles. Salvage of #69042 (narrowed by its author to this one behavioral line); test re-expressed on the current secondary-startup fixtures. Co-authored-by: sshawn <28279366+lsshawn@users.noreply.github.com>
|
Thanks @lsshawn for this PR — Merged via #101252 (2e25b47) on current main. #101252 won because it routes all the scoped-secret lookups through the shared Closing this PR as superseded by the merged work. |
…ofiles _start_one_profile_adapters skipped only Platform.RELAY as shared process-level ingress. WhatsApp is the same shape: the bridge is one authenticated session tied to a single phone number, so a secondary profile has no credential of its own to bring; constructing an adapter for it only produced a connect/retry loop that stalled startup for every profile queued behind it. Treat WhatsApp like Relay -- the active profile owns the connection and route-stamped source.profile fans inbound turns out to secondary profiles. Salvage of NousResearch#69042 (narrowed by its author to this one behavioral line); test re-expressed on the current secondary-startup fixtures. Co-authored-by: sshawn <28279366+lsshawn@users.noreply.github.com>
Problem
Running
gateway.multiplex_profileswith a single shared WhatsApp (Baileys) account routed to multiple profiles viagateway.profile_routes, inbound messages were served by the default profile's config/skills/memory/session-DB even when aprofile_routematched — leaking default-profile data (memory,session_searchhistory, filesystem) into routed replies. Three related gaps:1. Secondary profiles tried to start their own WhatsApp adapter
_start_one_profile_adaptersattempted a WhatsApp connection per profile. With one shared account, secondaries have no creds → retry-loop that stalls gateway startup. WhatsApp is a single-shared-connection platform likeRELAY, so we now skip it for secondaries (mirrors the existingRELAYskip). The default profile owns the one connection;profile_routesroutes groups.2. Session-key namespace ignored
profile_routesSessionStore._resolve_profile_for_keyfell back to the active profile (default) whensource.profilewas unset — even when aprofile_routematched — so routed sessions landed inagent:main+ the defaultstate.db. Now consultsgateway.profile_routesand stampssource.profileso the key namespace agrees with routing.3.
SessionStore._db+ inbound session creation were default-boundSessionStoreheld a single default-homeSessionDB; every session's history/search/persistence resolved to default'sstate.dbregardless of profile. Made_dba per-home-cached property (resolvesget_hermes_home()/state.dbunder the active ContextVar scope), and wrapped inboundget_or_create_sessionin_profile_runtime_scopeso the session is physically created in the routed profile'sstate.db.Result
A WhatsApp group routed to profile X now runs fully as X — its own skills, memory, and
state.db— andsession_searchno longer surfaces another profile's history.Safety
All new behavior is gated on
multiplex_profiles. Multiplex-off behavior is byte-identical.🤖 Generated with Claude Code