diff --git a/gateway/session_context.py b/gateway/session_context.py index b9fdcdfaf7..7f8aca3eb9 100644 --- a/gateway/session_context.py +++ b/gateway/session_context.py @@ -37,18 +37,24 @@ needs to replace the import + call site: """ from contextvars import ContextVar +from typing import Any + +# Sentinel to distinguish "never set in this context" from "explicitly set to empty". +# When a contextvar holds _UNSET, we fall back to os.environ (CLI/cron compat). +# When it holds "" (after clear_session_vars resets it), we return "" — no fallback. +_UNSET: Any = object() # --------------------------------------------------------------------------- # Per-task session variables # --------------------------------------------------------------------------- -_SESSION_PLATFORM: ContextVar[str] = ContextVar("HERMES_SESSION_PLATFORM", default="") -_SESSION_CHAT_ID: ContextVar[str] = ContextVar("HERMES_SESSION_CHAT_ID", default="") -_SESSION_CHAT_NAME: ContextVar[str] = ContextVar("HERMES_SESSION_CHAT_NAME", default="") -_SESSION_THREAD_ID: ContextVar[str] = ContextVar("HERMES_SESSION_THREAD_ID", default="") -_SESSION_USER_ID: ContextVar[str] = ContextVar("HERMES_SESSION_USER_ID", default="") -_SESSION_USER_NAME: ContextVar[str] = ContextVar("HERMES_SESSION_USER_NAME", default="") -_SESSION_KEY: ContextVar[str] = ContextVar("HERMES_SESSION_KEY", default="") +_SESSION_PLATFORM: ContextVar = ContextVar("HERMES_SESSION_PLATFORM", default=_UNSET) +_SESSION_CHAT_ID: ContextVar = ContextVar("HERMES_SESSION_CHAT_ID", default=_UNSET) +_SESSION_CHAT_NAME: ContextVar = ContextVar("HERMES_SESSION_CHAT_NAME", default=_UNSET) +_SESSION_THREAD_ID: ContextVar = ContextVar("HERMES_SESSION_THREAD_ID", default=_UNSET) +_SESSION_USER_ID: ContextVar = ContextVar("HERMES_SESSION_USER_ID", default=_UNSET) +_SESSION_USER_NAME: ContextVar = ContextVar("HERMES_SESSION_USER_NAME", default=_UNSET) +_SESSION_KEY: ContextVar = ContextVar("HERMES_SESSION_KEY", default=_UNSET) _VAR_MAP = { "HERMES_SESSION_PLATFORM": _SESSION_PLATFORM, @@ -91,10 +97,17 @@ def set_session_vars( def clear_session_vars(tokens: list) -> None: - """Restore session context variables to their pre-handler values.""" - if not tokens: - return - vars_in_order = [ + """Mark session context variables as explicitly cleared. + + Sets all variables to ``""`` so that ``get_session_env`` returns an empty + string instead of falling back to (potentially stale) ``os.environ`` + values. The *tokens* argument is accepted for API compatibility with + callers that saved the return value of ``set_session_vars``, but the + actual clearing uses ``var.set("")`` rather than ``var.reset(token)`` + to ensure the "explicitly cleared" state is distinguishable from + "never set" (which holds the ``_UNSET`` sentinel). + """ + for var in ( _SESSION_PLATFORM, _SESSION_CHAT_ID, _SESSION_CHAT_NAME, @@ -102,9 +115,8 @@ def clear_session_vars(tokens: list) -> None: _SESSION_USER_ID, _SESSION_USER_NAME, _SESSION_KEY, - ] - for var, token in zip(vars_in_order, tokens): - var.reset(token) + ): + var.set("") def get_session_env(name: str, default: str = "") -> str: @@ -113,8 +125,13 @@ def get_session_env(name: str, default: str = "") -> str: Drop-in replacement for ``os.getenv("HERMES_SESSION_*", default)``. Resolution order: - 1. Context variable (set by the gateway for concurrency-safe access) - 2. ``os.environ`` (used by CLI, cron scheduler, and tests) + 1. Context variable (set by the gateway for concurrency-safe access). + If the variable was explicitly set (even to ``""``) via + ``set_session_vars`` or ``clear_session_vars``, that value is + returned — **no fallback to os.environ**. + 2. ``os.environ`` (only when the context variable was never set in + this context — i.e. CLI, cron scheduler, and test processes that + don't use ``set_session_vars`` at all). 3. *default* """ import os @@ -122,7 +139,7 @@ def get_session_env(name: str, default: str = "") -> str: var = _VAR_MAP.get(name) if var is not None: value = var.get() - if value: + if value is not _UNSET: return value # Fall back to os.environ for CLI, cron, and test compatibility return os.getenv(name, default) diff --git a/tests/gateway/test_session_env.py b/tests/gateway/test_session_env.py index 5a643a1efb..85899e2fdd 100644 --- a/tests/gateway/test_session_env.py +++ b/tests/gateway/test_session_env.py @@ -1,6 +1,8 @@ import asyncio import os +import pytest + from gateway.config import Platform from gateway.run import GatewayRunner from gateway.session import SessionContext, SessionSource @@ -8,9 +10,26 @@ from gateway.session_context import ( get_session_env, set_session_vars, clear_session_vars, + _VAR_MAP, + _UNSET, ) +@pytest.fixture(autouse=True) +def _reset_contextvars(): + """Reset all session contextvars to _UNSET between tests. + + In production each asyncio.Task gets a fresh context copy where the + defaults are _UNSET. In tests all functions share the same thread + context, so a clear_session_vars() from test A (which sets vars to "") + would leak into test B. This fixture ensures each test starts clean. + """ + yield + for var in _VAR_MAP.values(): + # Can't use var.reset() without a token; just set back to sentinel. + var.set(_UNSET) + + def test_set_session_env_sets_contextvars(monkeypatch): """_set_session_env should populate contextvars, not os.environ.""" runner = object.__new__(GatewayRunner) @@ -98,9 +117,11 @@ def test_get_session_env_falls_back_to_os_environ(monkeypatch): tokens = set_session_vars(platform="telegram") assert get_session_env("HERMES_SESSION_PLATFORM") == "telegram" - # Restore — should fall back to os.environ again + # After clear — should return "" (explicitly cleared), NOT fall back + # to os.environ. This is the fix for #10304: stale os.environ values + # must not leak through after a gateway session is cleaned up. clear_session_vars(tokens) - assert get_session_env("HERMES_SESSION_PLATFORM") == "discord" + assert get_session_env("HERMES_SESSION_PLATFORM") == "" def test_get_session_env_default_when_nothing_set(monkeypatch): @@ -164,9 +185,9 @@ def test_session_key_falls_back_to_os_environ(monkeypatch): tokens = set_session_vars(session_key="ctx-session-456") assert get_session_env("HERMES_SESSION_KEY") == "ctx-session-456" - # Restore — should fall back to os.environ + # After clear — should return "" (explicitly cleared), not os.environ (#10304) clear_session_vars(tokens) - assert get_session_env("HERMES_SESSION_KEY") == "env-session-123" + assert get_session_env("HERMES_SESSION_KEY") == "" def test_set_session_env_includes_session_key():