fix(#5567): pin worker profile via hermes-agent v0.18.0 context-local HERMES_HOME override
profile_env_for_background_worker mirrored the profile home into process-global os.environ["HERMES_HOME"] and yielded the worker body OUTSIDE the setup lock, so a concurrent cross-profile worker (e.g. background title generation on another profile) could clobber os.environ mid-body — the agent config reader then resolved the WRONG profile's config (intermittent turn-init failures citing an unused provider; b3nw's v0.51.849 repro). Now installs hermes-agent v0.18.0's context-local home override (set_hermes_home_override, a ContextVar that get_hermes_home() consults before os.environ) for the resolved non-default profile home — set before yield, reset in finally — so a config read in the worker body resolves THIS profile's home from task-local state, immune to a concurrent os.environ clobber. No worker serialization, no os.environ mutation, no detection heuristic (both earlier WebUI-only mitigations were rejected as regression-unsafe). Graceful degradation: the override module is resolved lazily+optionally (_resolve_hermes_home_override, mirroring _resolve_secret_scope_module). On agents < v0.18.0 the symbol is absent -> returns None -> pre-existing os.environ-mirror behavior unchanged (no regression, no new failure mode). Self-built (nesquena-hermes); nesquena independent review APPROVED.
This commit is contained in:
1 parent
73643f21b6
commit
019a4814b7
2 files changed
+184
No files matched your search
@@ -1060,6 +1060,41 @@ def _resolve_secret_scope_module():
|
||||
return None
|
||||
|
||||
|
||||
# #5567: hermes-agent v0.18.0+ exposes a CONTEXT-LOCAL Hermes-home override
|
||||
# (`hermes_constants.set_hermes_home_override`) that `get_hermes_home()` — and
|
||||
# therefore `hermes_cli.config.get_config_path()` / `load_config()` — consults
|
||||
# BEFORE the process-global `os.environ["HERMES_HOME"]`. Installing it inside the
|
||||
# profile worker scope eliminates the cross-profile HERMES_HOME race at the
|
||||
# reader (a config read resolves the task-local profile home even if another
|
||||
# thread clobbers os.environ mid-body) WITHOUT serializing workers or mutating
|
||||
# shared state. Resolved lazily + optionally so OLDER agents (no override symbol)
|
||||
# degrade gracefully to the pre-existing os.environ-mirror behavior — unchanged.
|
||||
_hermes_home_override_available = None
|
||||
|
||||
|
||||
def _resolve_hermes_home_override():
|
||||
"""Return the hermes_constants module iff it exposes the v0.18.0+ context-local
|
||||
home override (set/reset), else None. Cached; import-safe on older agents."""
|
||||
global _hermes_home_override_available
|
||||
import sys as _sys
|
||||
if _hermes_home_override_available is False:
|
||||
return None
|
||||
mod = _sys.modules.get('hermes_constants')
|
||||
if mod is None and _hermes_home_override_available is None:
|
||||
try:
|
||||
import hermes_constants as mod # noqa: F811
|
||||
except Exception:
|
||||
_hermes_home_override_available = False
|
||||
return None
|
||||
if mod is not None and hasattr(mod, 'set_hermes_home_override') and hasattr(
|
||||
mod, 'reset_hermes_home_override'
|
||||
):
|
||||
_hermes_home_override_available = True
|
||||
return mod
|
||||
_hermes_home_override_available = False
|
||||
return None
|
||||
|
||||
|
||||
@contextmanager
|
||||
def profile_env_for_background_worker(
|
||||
session,
|
||||
@@ -1117,6 +1152,10 @@ def profile_env_for_background_worker(
|
||||
)
|
||||
_scope_token = None
|
||||
_has_scope = False
|
||||
# #5567: context-local Hermes-home override (hermes-agent v0.18.0+). None on
|
||||
# older agents → graceful no-op (falls back to the os.environ mirror below).
|
||||
_home_override_mod = None
|
||||
_home_override_token = None
|
||||
try:
|
||||
_set_thread_env(**thread_env)
|
||||
_thread_ctx.block_process_env_fallback = True
|
||||
@@ -1129,6 +1168,20 @@ def profile_env_for_background_worker(
|
||||
_has_scope = True
|
||||
except Exception:
|
||||
pass
|
||||
# #5567: install the context-local Hermes-home override so the agent
|
||||
# config reader (get_hermes_home -> get_config_path/load_config) resolves
|
||||
# THIS profile's home from task-local state, immune to a concurrent
|
||||
# cross-profile os.environ["HERMES_HOME"] clobber during the worker body.
|
||||
# No-op on agents < v0.18.0 (resolver returns None) → os.environ mirror
|
||||
# below remains the behavior, exactly as today.
|
||||
_home_override_mod = _resolve_hermes_home_override()
|
||||
if _home_override_mod is not None:
|
||||
try:
|
||||
_home_override_token = _home_override_mod.set_hermes_home_override(
|
||||
str(profile_home_path)
|
||||
)
|
||||
except Exception:
|
||||
_home_override_token = None
|
||||
with _ENV_LOCK:
|
||||
old_runtime_env = _apply_profile_env_to_process(
|
||||
os.environ,
|
||||
@@ -1151,6 +1204,12 @@ def profile_env_for_background_worker(
|
||||
)
|
||||
yield
|
||||
finally:
|
||||
# #5567: pop the context-local home override first (reverse of setup order).
|
||||
if _home_override_mod is not None and _home_override_token is not None:
|
||||
try:
|
||||
_home_override_mod.reset_hermes_home_override(_home_override_token)
|
||||
except Exception:
|
||||
pass
|
||||
if _has_scope and _secret_scope_mod is not None:
|
||||
try:
|
||||
_secret_scope_mod.reset_secret_scope(_scope_token)
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
"""Regression test for #5567 — cross-profile HERMES_HOME race at the config reader.
|
||||
|
||||
Root cause: `profile_env_for_background_worker` mirrors the profile's HERMES_HOME
|
||||
into the process-global `os.environ`, and the worker body runs outside the setup
|
||||
lock. A concurrent cross-profile worker can clobber `os.environ["HERMES_HOME"]`
|
||||
mid-body, so the agent config reader (`hermes_cli.config.get_config_path` /
|
||||
`load_config`, which read `get_hermes_home()`) resolves the WRONG profile's
|
||||
config — intermittent turn-init failures referencing another profile's provider.
|
||||
|
||||
Fix (#5567): when hermes-agent >= v0.18.0 exposes the context-local home
|
||||
override (`hermes_constants.set_hermes_home_override`), the worker scope installs
|
||||
it so `get_hermes_home()` resolves THIS task's profile home from task-local state,
|
||||
immune to the process-global clobber — without serializing workers.
|
||||
|
||||
Per #2321's acceptance criteria, this exercises the REAL
|
||||
`hermes_cli.config.load_config()` against a non-default profile with an
|
||||
intentional mid-body `os.environ` clobber and NO mocking of the production reader.
|
||||
|
||||
Degrades gracefully on agents without the override (skips with a clear reason).
|
||||
"""
|
||||
import os
|
||||
import textwrap
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
# The production reader — imported unmocked, exactly as #2321 requires. Skip the
|
||||
# whole module if the agent isn't importable in this environment.
|
||||
config_mod = pytest.importorskip("hermes_cli.config")
|
||||
hermes_constants = pytest.importorskip("hermes_constants")
|
||||
|
||||
HAS_OVERRIDE = hasattr(hermes_constants, "set_hermes_home_override") and hasattr(
|
||||
hermes_constants, "get_hermes_home"
|
||||
)
|
||||
|
||||
from api import profiles as profiles_api # noqa: E402
|
||||
|
||||
|
||||
def _seed_profile_home(base: Path, name: str, provider: str, model: str) -> Path:
|
||||
home = base / name
|
||||
home.mkdir(parents=True, exist_ok=True)
|
||||
(home / "config.yaml").write_text(
|
||||
textwrap.dedent(
|
||||
f"""\
|
||||
model:
|
||||
default: {model}
|
||||
provider: {provider}
|
||||
"""
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
return home
|
||||
|
||||
|
||||
@pytest.mark.skipif(
|
||||
not HAS_OVERRIDE,
|
||||
reason="hermes-agent < v0.18.0: no set_hermes_home_override; WebUI degrades to the os.environ mirror",
|
||||
)
|
||||
def test_load_config_resolves_worker_profile_despite_env_clobber(tmp_path, monkeypatch):
|
||||
"""The crux (#2321 criterion): inside profile_env_for_background_worker(A),
|
||||
a concurrent clobber of os.environ['HERMES_HOME']=B must NOT make the real
|
||||
load_config() read B — the context-local override pins A."""
|
||||
home_a = _seed_profile_home(tmp_path, "alpha", provider="anthropic", model="claude-x")
|
||||
home_b = _seed_profile_home(tmp_path, "beta", provider="ollama", model="llama-y")
|
||||
|
||||
# The CM's INPUT (which profile home to scope to) — this is not the reader
|
||||
# under test; the reader is the real hermes_cli.config below.
|
||||
monkeypatch.setattr(profiles_api, "get_hermes_home_for_profile", lambda name: home_a)
|
||||
|
||||
# Establish a benign starting env, then simulate the race: while the worker
|
||||
# body for profile A runs, a sibling profile-B worker clobbers the global.
|
||||
monkeypatch.setenv("HERMES_HOME", str(home_a))
|
||||
|
||||
# Clear any cached config so load_config actually hits the resolver.
|
||||
for fn in ("reload_config", "_reset_config_cache", "clear_config_cache"):
|
||||
if hasattr(config_mod, fn):
|
||||
try:
|
||||
getattr(config_mod, fn)()
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
with profiles_api.profile_env_for_background_worker("alpha", "test worker"):
|
||||
# The clobber: another profile's worker overwrites the process global.
|
||||
os.environ["HERMES_HOME"] = str(home_b)
|
||||
# get_config_path must resolve profile A via the context-local override,
|
||||
# NOT profile B from the clobbered os.environ.
|
||||
resolved = config_mod.get_config_path()
|
||||
assert resolved == home_a / "config.yaml", (
|
||||
f"config path must resolve profile A ({home_a}) via the context-local "
|
||||
f"override despite os.environ clobbered to B ({home_b}); got {resolved}"
|
||||
)
|
||||
# And the real load_config() must read A's model, not B's.
|
||||
cfg = config_mod.load_config()
|
||||
model_default = (cfg.get("model") or {}).get("default")
|
||||
assert model_default == "claude-x", (
|
||||
f"load_config must read profile A's model 'claude-x' despite the "
|
||||
f"HERMES_HOME clobber to B; got {model_default!r} (B is 'llama-y')"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.skipif(
|
||||
not HAS_OVERRIDE,
|
||||
reason="requires the v0.18.0 override to assert the override is cleared on exit",
|
||||
)
|
||||
def test_override_is_cleared_after_worker_exits(tmp_path, monkeypatch):
|
||||
"""The context-local override must not leak past the worker scope."""
|
||||
home_a = _seed_profile_home(tmp_path, "alpha", provider="anthropic", model="claude-x")
|
||||
monkeypatch.setattr(profiles_api, "get_hermes_home_for_profile", lambda name: home_a)
|
||||
|
||||
assert hermes_constants.get_hermes_home_override() is None
|
||||
with profiles_api.profile_env_for_background_worker("alpha", "test worker"):
|
||||
assert hermes_constants.get_hermes_home_override() == str(home_a)
|
||||
# Cleared on exit — no leak into subsequent tasks on this context.
|
||||
assert hermes_constants.get_hermes_home_override() is None
|
||||
|
||||
|
||||
def test_graceful_degradation_resolver_is_optional():
|
||||
"""On an agent WITHOUT the override, the resolver returns None and the CM
|
||||
falls back to the pre-existing os.environ mirror — never raises. We assert
|
||||
the resolver is import-safe and boolean-clean regardless of agent version."""
|
||||
mod = profiles_api._resolve_hermes_home_override()
|
||||
if HAS_OVERRIDE:
|
||||
assert mod is not None and hasattr(mod, "set_hermes_home_override")
|
||||
else:
|
||||
assert mod is None # older agent: graceful no-op, os.environ mirror stays
|
||||
Reference in new issue
Block a user