feat(gateway): per-topic profile isolation for Telegram forum topics - #53048
feat(gateway): per-topic profile isolation for Telegram forum topics#53048VKirill wants to merge 5 commits into
Conversation
b1ad467 to
e36d5b8
Compare
Related to merged #64835 and open #18510. #64835 now provides static platform/chat/thread profile_routes; this still-open branch carries a large residual per-turn isolation bundle. Rebase and split the residual work before a maintainer chooses an implementation; it is not a duplicate. |
e36d5b8 to
e3db1ef
Compare
Bind a chat/forum topic to a named profile (`/profile <name>`) and run the entire turn scoped to that profile's `HERMES_HOME` — its own model, SOUL.md, memory, session history, skills, MCP servers, and credentials. Topics with no binding keep current behaviour exactly (zero regression). Mechanism: - `_profile_runtime_scope`: per-turn context that layers a home override plus a profile secret-scope (no os.environ mutation), restored on exit. - `RoutingSessionStoreProxy` / `RoutingSessionDBProxy`: resolve the active profile home on every call so session history persists to the profile's state.db even from background tasks. - `.hermes_profile.json` identity marker: validated when resolving a profile home; auto-migrated if missing and logs a loud warning instead of silently falling back to the global home. - MCP servers are fingerprinted per profile so same-named servers with different credentials get separate connections; file tools are hard-guarded to the profile home; subprocess/cron inherit the scoped home. - Per-topic `/model` and `/profile` bindings persist across `/new` and restart (`topic_models.json` / `topic_profiles.json`). Adds tests covering routing, session/DB isolation, MCP and skills isolation, and subprocess home isolation.
|
Reopening after a clean rebuild on top of the latest What changed since the previous version
Approach (vs the routing-engine in #18510) Testing
Happy to adjust naming or split anything out if it helps a maintainer pick this as the canonical mechanism for the #10143 / #4321 cluster. |
# Conflicts: # cron/jobs.py # gateway/run.py # tools/mcp_tool.py
Per-topic profile isolation gives each routed profile its own SessionStore backed by a SQLite connection (db + WAL + SHM = 3 fds) to that profile's state.db. Two defects made a long-lived multi-profile gateway leak file descriptors until it hit EMFILE ([Errno 24] Too many open files): 1. `_profile_session_stores` and `_profile_session_dbs` were unbounded dicts that only ever grew and were closed solely at shutdown. Every distinct profile ever routed to kept its connection open for the whole process lifetime. 2. `_session_db` opened its OWN SessionDB to `<home>/state.db`, a SECOND connection to the exact file the profile's SessionStore already had open — doubling the fds held per profile. On macOS the launchd gateway inherits RLIMIT_NOFILE=256, so a handful of profiles plus normal sockets crossed the limit. Once over, every new open() failed — including the kanban dispatcher's `kanban.db` open, which then spun retrying on its tick loop and pinned a core. Fix, mirroring the existing `_agent_cache` LRU pattern: - `_profile_session_stores` becomes an OrderedDict with `move_to_end()` on hit and an LRU cap (`_PROFILE_STORE_CACHE_MAX_SIZE`, default 16); the evicted (least-recently-used, idle) store's connection is closed off-thread via `_close_evicted_profile_store` (SessionDB.close runs a WAL checkpoint and releases the db/WAL/SHM fds). The victim is never the profile served this turn, so live sessions aren't torn down mid-write. - `_session_db` now reuses the profile SessionStore's connection instead of opening a second handle; an explicit per-home override (the setter, used by tests and back-compat call sites) still wins, preserving the attribute contract. The redundant default-home SessionDB built in __init__ is dropped (the store already owns it); the NFS/locking init warning is preserved. Adds tests for LRU eviction + close, connection reuse, and override precedence.
5658098 to
8644434
Compare
# Conflicts: # gateway/run.py # hermes_cli/auth.py # tools/registry.py
…onDB Conflict resolutions and follow-through: - _session_db property now returns a cached per-profile AsyncSessionDB facade, so upstream's awaited call sites work against per-profile handles; explicit overrides keep the broad as-assigned contract. - Handoff watcher keeps the per-profile-home sweep, awaiting through the facade instead of asyncio.to_thread on raw handles. - Session-expiry finalization keeps the per-profile loop and grafts upstream's model_override reset at the conversation boundary. - Topic->profile binding lookups no longer call the DB-touching source normalizer bare on the loop: async boundaries offload it via asyncio.to_thread (upstream contract), sync helpers take the already-normalized source. - file_tools resolver keeps profile_home tilde expansion on top of upstream's container-path handling; runtime_provider/auth keep the profile-scoped env parameter alongside upstream's new helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Been tracking this since #18510 — the scoped per-topic isolation here is much cleaner than the full routing-engine approach. Currently running a frozen fork of pr-18510 but eager to migrate to this implementation. Happy to validate on macOS or help with tests if it moves the review forward. Any blockers or rough ETA for merge? |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused per-topic routing work. The requested capability remains absent on current main: /profile is display-only (gateway/slash_commands.py:332-345), while the current multiplex adapter path refuses duplicate platform credentials (gateway/run.py:8586-8600), so one bot cannot yet serve multiple profiles by topic.
Problems
- Blocking:
gateway/run.py:16939-16948catches profile identity failures and falls back toget_hermes_home(). For a persisted bound topic, an invalid, moved, or deleted profile would therefore run under the global profile rather than fail closed. That violates the PR's advertised isolation boundary.
Suggested changes
- Reject the routed event before agent/session execution when profile validation fails; send a clear topic-local administrative error instead of selecting the global home.
- Add coverage for a stale binding and invalid/missing identity marker, asserting that global sessions, identity, and credentials are not used.
- Integrate the binding layer with current main's multiplex scope (
gateway/run.py:1413-1444) and AsyncSessionDB architecture.
Automated hermes-sweeper review.
| "home — profile isolation is DEGRADED for this turn", | ||
| name, exc, | ||
| ) | ||
| return get_hermes_home() |
There was a problem hiding this comment.
Blocking: this fallback routes a topic that is explicitly bound to an invalid/deleted/rejected profile into the global profile. Do not continue the turn under get_hermes_home(); fail closed and return a topic-local administrative error so the requested profile boundary cannot silently degrade.
Related to merged #64835 and open #18510. #64835 now provides static platform/chat/thread profile_routes; this still-open branch carries a large residual per-turn isolation bundle. Rebase and split the residual work before a maintainer chooses an implementation; it is not a duplicate. |
|
Heads-up: the routing entry point of this PR (Telegram forum topic → profile) has now landed on main via PR #64835 (salvage of #20096 by @Burgunthy) — What this PR still offers beyond main:
These are isolation-hardening layers underneath the routing decision. If you're up for rebasing onto the merged routing layer and splitting the hardening into focused PRs, they'd be much easier to review than the current 4.4k-line combined change. Leaving open for now. |
|
Running a frozen
These five areas are independent enough that they could each be a focused PR rebased onto the merged Happy to validate any of these on macOS + real Telegram forum topics if that helps move review forward. |
What does this PR do?
Routes different Telegram forum topics to different Hermes profiles under a single bot, with full data isolation per profile. Bind a topic with
/profile <name>; the entire turn then runs scoped to that profile's home, so each topic gets its own model,SOUL.md/identity, memory, sessions, skills, MCP servers, credentials, and background processes.The problem: today one bot token = one profile, so running specialised agents (coding, research, marketing…) per forum topic requires N separate bot tokens and N gateway processes. This lets a single gateway dispatch each topic to a dedicated profile with hard data isolation, while unrouted topics are completely unchanged (zero-regression).
This is a smaller, scoped take on the approach explored in #18510 and #20096: the routing entry point reuses the existing per-topic binding, and isolation is applied as a per-turn
HERMES_HOME+ secret scope rather than a large standalone routing engine.Related Issue
Fixes #10143
Fixes #4321
Type of Change
Changes Made
gateway/run.py— per-turn_profile_runtime_scopefor routed topics;_resolve_profile_home_for_sourcevalidates the profile identity, auto-migrates a missing.hermes_profile.jsonmarker, and logs a loud warning instead of silently degrading to the global home;RoutingSessionStoreProxy/RoutingSessionDBProxyre-resolve the active profile home on every call so session history is persisted to the profile'sstate.dbeven from background/executor tasks.hermes_cli/profiles.py— identity marker (.hermes_profile.json) write/validate with ancestry + symlink-escape checks;hermes profile audit-isolation <name>diagnostic (reports cross-profile secret/config overlap without printing secrets).tools/mcp_tool.py— MCP connections keyed by a config fingerprint so same-named servers across profiles with different.envtokens get separate connections; tool names namespaced.tools/file_tools.py— hard-guard blocks writes outside the active profile home (including the global / another profile'sSOUL.md); guard runs even when path resolution fails.agent/auxiliary_client.py,hermes_cli/{auth,runtime_provider,env_loader}.py— provider /auth.json/ credential-pool resolution scoped to the profile's.envwithout mutatingos.environ; fail-closed on missing scoped credentials.tools/{process_registry,skills_tool,skill_manager_tool,terminal_tool,memory_tool}.py,cron/jobs.py,cron/suggestions.py— process registry, skills, subprocess HOME, cron paths scoped to the active profile.tests/gateway/,tests/tools/,tests/hermes_cli/,tests/agent/for routing, session/DB scoping, MCP fingerprint isolation, skills isolation, subprocess HOME isolation, identity marker, and credential scoping.How to Test
pytest tests/gateway/test_profile_isolation_rework_suite.py \ tests/gateway/test_topic_profile_routing.py \ tests/tools/test_mcp_profile_isolation.py \ tests/tools/test_skills_profile_isolation.py \ tests/test_subprocess_home_isolation.py \ tests/hermes_cli/test_profiles.py -qManual / live (single Telegram bot, 3 routed topics):
~/.hermes/profiles/<name>/with its ownconfig.yaml,SOUL.md,.env; bind a topic with/profile <name>.state.db; the globalstate.dbis not written.write_file("SOUL.md", …)in a routed topic → it writes toprofiles/<name>/SOUL.md; the global~/.hermes/SOUL.mdis untouched.github) in two profiles with different.envtokens → each resolves to a separate connection.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qhas order-dependent flakiness already present onmain, unrelated to this PR; the suites touched by this change are green.Documentation & Housekeeping
docs/, docstrings) — user-facing docs for per-topic profile isolation are a planned follow-up; happy to add todocs/in this PR if preferredcli-config.yaml.exampleif I added/changed config keys — N/A (binding is via the existing/profileruntime command + per-profileconfig.yaml; no new global config keys)CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/AScreenshots / Logs
Targeted suite result: