Skip to content

fix(gateway): parse named-profile namespaces in session keys (#105931) - #105942

Closed
kokhlo wants to merge 7 commits into
NousResearch:mainfrom
kokhlo:fix-105931-profile-session-keys
Closed

kokhlo wants to merge 7 commits into
NousResearch:mainfrom
kokhlo:fix-105931-profile-session-keys

Conversation

@kokhlo

@kokhlo kokhlo commented Sep 8, 2026

Copy link
Copy Markdown

Summary

Two places hardcoded the agent:main namespace, so under multiplex_profiles named-profile session keys (agent:<profile>:...) were not parsed or matched. This PR fixes both and cleans up a hand-rolled workaround that becomes dead code.

  • _parse_session_key() (gateway/run.py): now accepts any valid namespace slot — main or a profile id matching [a-z0-9][a-z0-9_-]{0,63} (profile ids never contain : — hermes_cli.profiles._PROFILE_ID_RE — so the plain :-split stays unambiguous). A named profile is reported as profile in the result dict; main keys keep their exact historical shape (no profile key), so existing equality assertions stay byte-identical. Consumers in run_notifications.py (watch/completion routing) and run_shutdown.py (shutdown-notice target resolution) no longer degrade to the LRU _session_sources fallback for named profiles.
  • _sibling_thread_run_keys() (gateway/run_busy.py): the busy-ack prefix is built via _session_key_namespace(source.profile) instead of the literal agent:main, so a per-user thread /stop under multiplexing actually finds (and stops) a sibling participant's run on the same named profile — and never crosses into a different profile's run in the same chat/thread.
  • _build_process_event_source() (gateway/run_notifications.py): drops the manual split + agent:main re-wrap; the profile now comes straight from the parsed dict.

Tests

  • named-profile parse (dm / thread-slot / group-suffix-omitted) and unchanged main shape; invalid namespace still → None
  • sibling matching under profile="work" vs the same chat on main (must not cross profiles)
  • _build_process_event_source() resolves platform/chat/profile from an agent:work:... key (previously unresolvable → dropped with a warning)

Local: 34 passed in the two touched suites; adjacent consumers (test_loop_command, test_relay_delivery_followups, test_async_delegation) 72 passed; ruff check clean; no new ruff format complaints on touched lines vs the pristine baseline.

Fixes #105931

_parse_session_key() only accepted the agent:main namespace, so under
multiplex_profiles named-profile keys (agent:<profile>:...) parsed to None and
notification/shutdown routing fell back to the LRU source cache; and
_sibling_thread_run_keys() hardcoded agent:main, so a per-user thread /stop
never matched a sibling run on a named profile. Accept any valid profile-id
namespace slot and report it as profile (main keys keep their exact historical
shape), build the sibling prefix from _session_key_namespace(source.profile),
and drop the manual agent:main re-wrap in _build_process_event_source().
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery area/profiles Multi-profile isolation, HERMES_HOME scoping area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages P2 Medium — degraded but workaround exists labels Sep 8, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Related: #105943 (later, narrower fix for the same two agent:main sites) and #91939 (earlier fix for the run_busy.py sibling-match half only). This PR is the broadest of the cluster; maintainers may want to consolidate.

@kokhlo

kokhlo commented Sep 8, 2026

Copy link
Copy Markdown
Author

The two Python tests / Run tests failures in this window are pre-existing CI flakes, not from this diff:

  • tests/hermes_cli/test_gateway_service.py::TestSystemUnitHermesHome::test_system_unit_orders_after_target_user_manager — fails with PermissionError: [Errno 13] Permission denied: '/root/.local/bin' (runner-environment issue). The same test fails on unrelated PRs in the same window: run 34263901898 (fix/desktop-background-handoffs) and run 34263868646 / 34263189364 (fix/issue-105690-manual-marker-followup, feat/delegation-child-toolsets). Neither PR touches gateway code. The test passes locally both on pristine main and with this diff applied (105 passed).
  • tests/cli/test_subagent_monitor_prompts.py::test_prompt_paint_yields_monitor_but_ordinary_paint_does_not — TUI paint-order test, also passes locally with this diff (2 passed).

The diff only touches gateway/run.py / gateway/run_busy.py / gateway/run_notifications.py session-key parsing plus their tests. Docker Build/Test/Publish and Nix flake check are green on the same head. Re-pushing to get a green window.

@kokhlo

kokhlo commented Sep 8, 2026

Copy link
Copy Markdown
Author

Update on the CI failure: it is deterministic on the current merge-ref, and a fix is already pending in #105932 (treat unreadable user-local bin dirs as absent when building service PATH — touches hermes_cli/gateway.py + tests/hermes_cli/test_gateway_service.py, currently MERGEABLE/CLEAN).

Timeline: the failing test arrived in main today at 17:05 UTC (4c4845f, "order the system unit after the target user's manager"), and every PR whose merge-ref includes that commit fails test_system_unit_orders_after_target_user_manager with PermissionError: /root/.local/bin — confirmed on unrelated PRs (runs 34263901898, 34263868646, 34263189364). My PR's CI will go green once #105932 merges; happy to re-push then, or a maintainer rerun works too. All diff-specific checks (ruff, attribution, macOS/Windows lanes, e2e, Docker Build/Test/Publish, Nix flake check) are green on this head.

teknium1 added a commit that referenced this pull request Sep 11, 2026
With _parse_session_key accepting the agent:<profile>: namespace (salvaged
from #105942), the rewrite-to-agent:main workaround in
_shutdown_notification_target is dead weight: take the profile from the
parsed result like _build_process_event_source now does.

Co-authored-by: Konstantin Khlopkov <konstantin.khlopkov93@gmail.com>
@teknium1

Copy link
Copy Markdown
Collaborator

Landed on main in #108294. Your commit was cherry-picked with authorship preserved as bdb38e3 (_parse_session_key accepts any profile-id namespace and reports it as profile; _sibling_thread_run_keys builds its prefix from _session_key_namespace(source.profile); the manual agent:main re-wrap in _build_process_event_source is gone). The shutdown-notice reader got the same treatment in 5fc784a with Co-authored-by credit to you. Thanks, @kokhlo.

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

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping area/sessions Session lifecycle, resume, persistence, history comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists 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/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

agent:main hardcoded in _parse_session_key and run_busy breaks named-profile session keys (multiplexing)

3 participants