diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 6fd0cb52e..a0a52d0ec 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -564,35 +564,18 @@ async def _ssrf_redirect_guard(response): # (e.g. Telegram file URLs expire after ~1 hour). # --------------------------------------------------------------------------- -# Default location: {HERMES_HOME}/cache/images/ (legacy: image_cache/) -# -# NOTE: These module-level constants are the import-time DEFAULTS. They exist -# for two reasons: (1) backward-compatible references elsewhere, and (2) tests -# monkeypatch them (e.g. ``monkeypatch.setattr("...IMAGE_CACHE_DIR", tmp)``). -# The ``get_*_cache_dir()`` getters below re-resolve through ``get_hermes_dir()`` -# on every call so the context-local profile override -# (``set_hermes_home_override``) is honored — freezing the resolved path at -# import pinned every profile to whichever one first imported this module -# (cross-profile leak in single-process multi-profile desktop runtime). When a -# test has monkeypatched the constant away from its import-time default, that -# override wins (preserves the existing test seam). +# Import-time default. Tests monkeypatch this; the get_*_cache_dir() getters +# re-resolve per call so the active profile override is honored. IMAGE_CACHE_DIR = get_hermes_dir("cache/images", "image_cache") def _resolve_cache_dir(constant_name: str, new_subpath: str, old_name: str) -> Path: - """Resolve a cache dir, honoring profile override and test monkeypatches. - - Precedence: - 1. If the module constant ``constant_name`` was monkeypatched away from - its import-time default, return the patched value (test seam). - 2. Otherwise resolve fresh via ``get_hermes_dir`` so the active profile's - ``set_hermes_home_override`` is reflected per-call. - """ + """Resolve fresh via get_hermes_dir (active profile), unless a test has + monkeypatched the constant away from its import-time default.""" fresh = get_hermes_dir(new_subpath, old_name) current = globals().get(constant_name) default = _CACHE_DIR_IMPORT_DEFAULTS.get(constant_name) if current is not None and default is not None and current != default: - # A test (or caller) replaced the module constant — respect it. return Path(current) return fresh @@ -968,10 +951,8 @@ def cache_video_from_bytes(data: bytes, ext: str = ".mp4") -> str: DOCUMENT_CACHE_DIR = get_hermes_dir("cache/documents", "document_cache") SCREENSHOT_CACHE_DIR = get_hermes_dir("cache/screenshots", "browser_screenshots") -# Import-time defaults for the cache-dir constants. ``_resolve_cache_dir`` -# compares the live module value against these to detect a test monkeypatch -# (in which case the patched value wins) vs. an unmodified constant (in which -# case it re-resolves through the active profile override). +# Import-time defaults; _resolve_cache_dir compares against these to tell a +# test monkeypatch from an unmodified constant. _CACHE_DIR_IMPORT_DEFAULTS = { "IMAGE_CACHE_DIR": IMAGE_CACHE_DIR, "AUDIO_CACHE_DIR": AUDIO_CACHE_DIR, diff --git a/gateway/rich_sent_store.py b/gateway/rich_sent_store.py index 414e48736..090019212 100644 --- a/gateway/rich_sent_store.py +++ b/gateway/rich_sent_store.py @@ -25,11 +25,9 @@ _MAX_TEXT_CHARS = 2000 def _store_path() -> str: - # Resolve through get_hermes_home() so the context-local profile override - # (set_hermes_home_override) is honored. Reading os.environ["HERMES_HOME"] - # directly bypassed the override and leaked every profile's rich-sent index - # into the launch/default profile in single-process multi-profile runtimes - # (desktop tui_gateway). + # Resolve via get_hermes_home() so the profile override is honored; reading + # os.environ["HERMES_HOME"] directly bypassed it and leaked the index into + # the launch profile (multi-profile tui_gateway / gateway). from hermes_constants import get_hermes_home home = get_hermes_home() diff --git a/model_tools.py b/model_tools.py index a69f9f84e..9e00286f9 100644 --- a/model_tools.py +++ b/model_tools.py @@ -144,12 +144,9 @@ def _run_async(coro): worker_loop.close() pool = concurrent.futures.ThreadPoolExecutor(max_workers=1) - # Propagate the parent thread's ContextVars (notably the - # _HERMES_HOME_OVERRIDE profile scope) and approval/sudo callbacks into - # the worker thread. Without this, any async tool that resolves - # get_hermes_home() inside its coroutine falls back to the launch/default - # profile in single-process multi-profile runtimes (desktop tui_gateway), - # leaking one profile's reads/writes into another. + # Propagate the profile override + approval/sudo callbacks into the + # worker so async tools resolving get_hermes_home() see the active + # profile, not the launch one (multi-profile tui_gateway / gateway). from tools.thread_context import propagate_context_to_thread future = pool.submit(propagate_context_to_thread(_run_in_worker)) diff --git a/run_agent.py b/run_agent.py index 37e9a09b7..f511f14f7 100644 --- a/run_agent.py +++ b/run_agent.py @@ -1532,12 +1532,8 @@ class AIAgent: review_memory=review_memory, review_skills=review_skills, ) - # Propagate the spawning turn's ContextVars (notably the - # _HERMES_HOME_OVERRIDE profile scope) into the review thread. A bare - # threading.Thread starts with an empty context, so the review would - # resolve get_hermes_home() to the launch/default profile and write - # MEMORY.md / skill review into the wrong profile in single-process - # multi-profile runtimes (desktop tui_gateway) — see #54937. + # Propagate the profile override into the review thread, else it writes + # MEMORY.md / skill review into the launch profile (#54937). t = threading.Thread( target=propagate_context_to_thread(target), daemon=True, name="bg-review" ) diff --git a/tools/async_delegation.py b/tools/async_delegation.py index 987935df4..6e0d515a6 100644 --- a/tools/async_delegation.py +++ b/tools/async_delegation.py @@ -249,10 +249,8 @@ def dispatch_async_delegation( _finalize(delegation_id, result, status) try: - # Capture the dispatching turn's ContextVars (notably the - # _HERMES_HOME_OVERRIDE profile scope) so the detached child resolves - # get_hermes_home() under the right profile in single-process - # multi-profile runtimes (desktop tui_gateway). + # Propagate the dispatching profile so the detached child resolves + # get_hermes_home() under the right profile. executor.submit(propagate_context_to_thread(_worker)) except Exception as exc: # pragma: no cover — pool submit failure is rare with _records_lock: @@ -438,9 +436,7 @@ def dispatch_async_delegation_batch( _finalize_batch(delegation_id, combined, status) try: - # Capture the dispatching turn's ContextVars (notably the - # _HERMES_HOME_OVERRIDE profile scope) so the detached batch children - # resolve get_hermes_home() under the right profile. + # Propagate the dispatching profile to the detached batch children. executor.submit(propagate_context_to_thread(_worker)) except Exception as exc: # pragma: no cover with _records_lock: diff --git a/tools/skills_hub.py b/tools/skills_hub.py index b7d0237d7..d0ebecd25 100644 --- a/tools/skills_hub.py +++ b/tools/skills_hub.py @@ -46,30 +46,16 @@ logger = logging.getLogger(__name__) # --------------------------------------------------------------------------- # Paths # --------------------------------------------------------------------------- -# -# These directories MUST resolve through ``get_hermes_home()`` on every access -# so the context-local profile override (``set_hermes_home_override``) is -# honored. Freezing them as module-level constants at import time pinned every -# later profile to whichever profile first imported this module — a cross- -# profile data leak in single-process multi-profile runtimes (desktop -# ``tui_gateway``). See the profile-isolation fix. -# -# Backward compatibility: external callers do ``from tools.skills_hub import -# SKILLS_DIR`` (always inside a function body, re-evaluated per call). The -# module-level ``__getattr__`` below makes those names resolve dynamically, so -# no external call site needs to change. +# Resolved per-call (not frozen at import) so the profile override is honored; +# import-time constants leaked across profiles in single-process multi-profile +# runtimes. Legacy names (SKILLS_DIR, ...) are re-exposed via __getattr__ below +# so external `from tools.skills_hub import SKILLS_DIR` callers still work. -INDEX_CACHE_TTL = 3600 # 1 hour (defined early; referenced below) +INDEX_CACHE_TTL = 3600 # 1 hour -# The legacy path names (SKILLS_DIR, HUB_DIR, ...) are not real module -# attributes — they are synthesized on access by the PEP 562 ``__getattr__`` -# below so they reflect the active profile override. Tests, however, set them -# as *real* module attributes via ``patch.object(hub, "SKILLS_DIR", ...)`` / -# ``monkeypatch.setattr``. ``_override`` lets each resolver honor such an -# injected real attribute (the test seam) before falling back to dynamic -# resolution. ``globals().get`` returns None when only the __getattr__-backed -# name exists (no real attribute set), so dynamic resolution wins by default. +# _override lets a test-injected real module attribute (patch.object/monkeypatch +# on SKILLS_DIR etc.) win over dynamic resolution; None means resolve live. def _override(name: str): return globals().get(name) @@ -126,14 +112,8 @@ _DYNAMIC_PATH_RESOLVERS = { def __getattr__(name: str): - """Resolve the legacy path constants dynamically per access. - - PEP 562 module ``__getattr__``: only called for names NOT found as real - module attributes, so it does not slow down ordinary lookups and a test's - ``patch.object``-set real attribute shadows it. This lets - ``tools.skills_hub.SKILLS_DIR`` (and the rest) reflect the active profile - override instead of an import-time snapshot. - """ + """Resolve legacy path constants dynamically (PEP 562) so they reflect the + active profile override; a test's patch.object-set real attribute shadows it.""" resolver = _DYNAMIC_PATH_RESOLVERS.get(name) if resolver is not None: return resolver()