From b1614bdeeec630dc4b6a4909f00eb2e89150e59b Mon Sep 17 00:00:00 2001 From: Drexuxux Date: Tue, 4 Aug 2026 14:26:01 +0300 Subject: [PATCH] fix(gateway): run /insights, /debug and /goal draft inside the routed profile The multiplexed inbound handler wraps every message in _profile_runtime_scope, which installs the routed profile's HERMES_HOME override and its secret scope as contextvars. A bare loop.run_in_executor(None, fn) starts the worker with an EMPTY context, so neither reaches the blocking work. GatewaySlashCommandsMixin already knows this -- /compress goes through _run_in_executor_with_context and the call site says why. Three siblings in the same file still used the bare hop: /insights SessionDB() with no explicit path resolves get_hermes_home() at call time (_default_db_path), so the worker opened the DEFAULT profile's state.db. Under multiplexing the command reported another profile's conversations, session counts and sources to this profile's user. /debug collects that home's logs/config and uploads them to a public paste, so it published the default profile's diagnostics from another profile's chat. /goal draft calls the auxiliary LLM, whose provider/credential resolution reads the profile secret scope -- unscoped it falls back to process-global os.environ, which under multiplexing may hold a different profile's keys. Route all three through _run_in_executor_with_context. /reload-skills is deliberately left alone: tools.skills_tool binds SKILLS_DIR at import time, so it does not follow the contextvar either way. Fixing that needs the module-global retarget web_server._profile_scope performs under a lock, which is a different change from context propagation. Single-profile gateways never enter the scope, so their behaviour is unchanged. --- gateway/slash_commands.py | 26 ++- .../test_slash_command_profile_scope.py | 168 ++++++++++++++++++ 2 files changed, 186 insertions(+), 8 deletions(-) create mode 100644 tests/gateway/test_slash_command_profile_scope.py diff --git a/gateway/slash_commands.py b/gateway/slash_commands.py index 7b87e435055cb..994caae3921da 100644 --- a/gateway/slash_commands.py +++ b/gateway/slash_commands.py @@ -2691,8 +2691,12 @@ async def _handle_goal_command(self, event: "MessageEvent") -> str: import asyncio from hermes_cli.goals import draft_contract - draft_contract_obj = await asyncio.get_running_loop().run_in_executor( - None, draft_contract, objective + # _run_in_executor_with_context, not a bare hop: drafting a + # contract calls the auxiliary LLM, whose provider/credential + # resolution reads the profile secret scope — a contextvar that + # a default-executor hop drops, leaving it unscoped. + draft_contract_obj = await self._run_in_executor_with_context( + draft_contract, objective ) except Exception as exc: logger.debug("goal draft failed: %s", exc) @@ -5012,8 +5016,6 @@ async def _handle_insights_command(self, event: MessageEvent) -> str: from hermes_state import SessionDB from agent.insights import InsightsEngine - loop = asyncio.get_running_loop() - def _run_insights(): db = SessionDB() engine = InsightsEngine(db) @@ -5022,7 +5024,13 @@ def _run_insights(): db.close() return result - return await loop.run_in_executor(None, _run_insights) + # _run_in_executor_with_context, not a bare hop: ``SessionDB()`` + # with no explicit path resolves ``get_hermes_home()`` at call + # time, and that override is a contextvar installed by + # ``_profile_runtime_scope``. A default-executor hop starts the + # worker with an EMPTY context, so /insights read the DEFAULT + # profile's state.db and reported another profile's conversations. + return await self._run_in_executor_with_context(_run_insights) except Exception as e: logger.error("Insights command error: %s", e, exc_info=True) return t("gateway.insights.error", error=e) @@ -5360,8 +5368,6 @@ async def _handle_debug_command(self, event: MessageEvent) -> str: _GATEWAY_PRIVACY_NOTICE, _best_effort_sweep_expired_pastes, ) - loop = asyncio.get_running_loop() - # Run blocking I/O (dump capture, log reads, uploads) in a thread. def _collect_and_upload(): _best_effort_sweep_expired_pastes() @@ -5388,7 +5394,11 @@ def _collect_and_upload(): lines.append(t("gateway.debug.share_hint")) return "\n".join(lines) - return await loop.run_in_executor(None, _collect_and_upload) + # _run_in_executor_with_context, not a bare hop: this collects the + # profile's logs/config off ``get_hermes_home()`` and uploads them to a + # public paste. Losing the contextvar override would publish the DEFAULT + # profile's diagnostics from another profile's chat. + return await self._run_in_executor_with_context(_collect_and_upload) async def _handle_update_command(self, event: MessageEvent) -> str: """Handle /update command — update Hermes Agent to the latest version. diff --git a/tests/gateway/test_slash_command_profile_scope.py b/tests/gateway/test_slash_command_profile_scope.py new file mode 100644 index 0000000000000..6dbe37f43ae5d --- /dev/null +++ b/tests/gateway/test_slash_command_profile_scope.py @@ -0,0 +1,168 @@ +"""Gateway slash commands must do their blocking work inside the routed profile. + +The multiplexed inbound handler wraps the whole message in +``_profile_runtime_scope``, which installs the routed profile's ``HERMES_HOME`` +override and its secret scope as **contextvars**. + +``GatewaySlashCommandsMixin`` already knows a bare executor hop drops them — it +routes ``/compress`` through ``_run_in_executor_with_context`` and says so at +the call site. Three siblings in the same file still used +``loop.run_in_executor(None, ...)``, which starts the worker with an EMPTY +context: + +* ``/insights`` — ``SessionDB()`` with no explicit path resolves + ``get_hermes_home()`` at call time, so it read the DEFAULT profile's + ``state.db`` and reported another profile's conversations. +* ``/debug`` — collects that home's logs/config and uploads them to a public + paste, so it published the wrong profile's diagnostics. +* ``/goal draft`` — calls the auxiliary LLM, whose credential resolution reads + the profile secret scope. + +Drives the real mixin methods and the real ``_profile_runtime_scope``: the +contextvar loss is a property of the hop, so mocking the hop away would test +nothing. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + + +@pytest.fixture +def profile_home(tmp_path, monkeypatch): + root = tmp_path / ".hermes" + home = root / "profiles" / "coder" + home.mkdir(parents=True) + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setenv("HERMES_HOME", str(root)) + return home + + +@pytest.fixture +def runner(): + """Minimal host exposing the mixin plus the runner's executor helpers.""" + from gateway.run import GatewayRunner + from gateway.slash_commands import GatewaySlashCommandsMixin + + class _Runner(GatewaySlashCommandsMixin): + _run_in_executor_with_context = GatewayRunner._run_in_executor_with_context + _get_executor = GatewayRunner._get_executor + + return _Runner() + + +class _Event: + def __init__(self, args: str = ""): + self._args = args + + def get_command_args(self) -> str: + return self._args + + +class TestInsightsReadsTheRoutedProfilesDatabase: + @pytest.mark.asyncio + async def test_session_db_opens_under_the_profile_home( + self, runner, profile_home, monkeypatch + ): + import hermes_state + from gateway.run import _profile_runtime_scope + from hermes_constants import get_hermes_home + + seen: dict = {} + + class _RecordingDB: + def __init__(self, *a, **kw): + seen["home"] = str(get_hermes_home()) + + def close(self): + pass + + class _Engine: + def __init__(self, db): + pass + + def generate(self, **kw): + return {} + + def format_gateway(self, report): + return "ok" + + monkeypatch.setattr(hermes_state, "SessionDB", _RecordingDB) + import agent.insights as insights_mod + + monkeypatch.setattr(insights_mod, "InsightsEngine", _Engine) + + with _profile_runtime_scope(profile_home): + result = await runner._handle_insights_command(_Event("")) + + assert result == "ok" + assert seen["home"] == str(profile_home) + + @pytest.mark.asyncio + async def test_without_a_scope_it_still_uses_the_launch_home( + self, runner, profile_home, tmp_path, monkeypatch + ): + """Single-profile gateways never enter the scope — behaviour unchanged.""" + import hermes_state + from hermes_constants import get_hermes_home + + seen: dict = {} + + class _RecordingDB: + def __init__(self, *a, **kw): + seen["home"] = str(get_hermes_home()) + + def close(self): + pass + + class _Engine: + def __init__(self, db): + pass + + def generate(self, **kw): + return {} + + def format_gateway(self, report): + return "ok" + + monkeypatch.setattr(hermes_state, "SessionDB", _RecordingDB) + import agent.insights as insights_mod + + monkeypatch.setattr(insights_mod, "InsightsEngine", _Engine) + + await runner._handle_insights_command(_Event("")) + + assert seen["home"] == str(tmp_path / ".hermes") + + +class TestExecutorHelperContract: + """The mechanism all three call sites now rely on. + + ``/insights`` is driven end-to-end above. ``/debug`` and ``/goal draft`` + take the identical one-line substitution but sit behind adapter and + goal-manager scaffolding, so their shared guarantee is pinned here rather + than through a fake deep enough to stop testing the real thing. + """ + + @pytest.mark.asyncio + async def test_helper_preserves_the_override_a_bare_hop_drops( + self, runner, profile_home + ): + import asyncio + + from gateway.run import _profile_runtime_scope + from hermes_constants import get_hermes_home + + def _probe(): + return str(get_hermes_home()) + + loop = asyncio.get_running_loop() + with _profile_runtime_scope(profile_home): + via_helper = await runner._run_in_executor_with_context(_probe) + via_bare = await loop.run_in_executor(None, _probe) + + assert via_helper == str(profile_home) + # The defect this fix closes: the bare hop cannot see the override. + assert via_bare != str(profile_home)