fix: avoid process-wide cron profile home mutation
This commit is contained in:
committed by
daimon-nous[bot]
parent
bb9ecb2178
commit
544406ef23
@@ -152,10 +152,11 @@ def _job_profile_context(job_id: str, profile: Optional[str]):
|
|||||||
|
|
||||||
Cron jobs are stored and scheduled by the profile running the scheduler, but
|
Cron jobs are stored and scheduled by the profile running the scheduler, but
|
||||||
an individual job can opt into a different runtime profile. While active,
|
an individual job can opt into a different runtime profile. While active,
|
||||||
HERMES_HOME and the scheduler's test/override hook both point at the
|
The scheduler's test/override hook and a context-local Hermes home override
|
||||||
resolved profile directory so _get_hermes_home(), .env/config loading, script
|
both point at the resolved profile directory so _get_hermes_home(),
|
||||||
resolution, AIAgent construction, and downstream get_hermes_home() callers
|
.env/config loading, script resolution, AIAgent construction, and downstream
|
||||||
agree on the same home.
|
get_hermes_home() callers agree on the same home without mutating the
|
||||||
|
process-wide environment seen by other threads.
|
||||||
"""
|
"""
|
||||||
raw_profile = str(profile or "").strip()
|
raw_profile = str(profile or "").strip()
|
||||||
if not raw_profile:
|
if not raw_profile:
|
||||||
@@ -163,16 +164,17 @@ def _job_profile_context(job_id: str, profile: Optional[str]):
|
|||||||
return
|
return
|
||||||
|
|
||||||
global _hermes_home
|
global _hermes_home
|
||||||
prior_env = os.environ.get("HERMES_HOME", "_UNSET_")
|
|
||||||
prior_override = _hermes_home
|
prior_override = _hermes_home
|
||||||
|
|
||||||
from hermes_cli.profiles import normalize_profile_name, resolve_profile_env
|
from hermes_cli.profiles import normalize_profile_name, resolve_profile_env
|
||||||
|
from hermes_constants import reset_hermes_home_override, set_hermes_home_override
|
||||||
|
|
||||||
normalized_profile = normalize_profile_name(raw_profile)
|
normalized_profile = normalize_profile_name(raw_profile)
|
||||||
profile_home = Path(resolve_profile_env(normalized_profile)).resolve()
|
profile_home = Path(resolve_profile_env(normalized_profile)).resolve()
|
||||||
|
|
||||||
|
override_token = None
|
||||||
try:
|
try:
|
||||||
os.environ["HERMES_HOME"] = str(profile_home)
|
override_token = set_hermes_home_override(profile_home)
|
||||||
_hermes_home = profile_home
|
_hermes_home = profile_home
|
||||||
logger.info(
|
logger.info(
|
||||||
"Job '%s': using Hermes profile '%s' (%s)",
|
"Job '%s': using Hermes profile '%s' (%s)",
|
||||||
@@ -183,10 +185,8 @@ def _job_profile_context(job_id: str, profile: Optional[str]):
|
|||||||
yield normalized_profile
|
yield normalized_profile
|
||||||
finally:
|
finally:
|
||||||
_hermes_home = prior_override
|
_hermes_home = prior_override
|
||||||
if prior_env == "_UNSET_":
|
if override_token is not None:
|
||||||
os.environ.pop("HERMES_HOME", None)
|
reset_hermes_home_override(override_token)
|
||||||
else:
|
|
||||||
os.environ["HERMES_HOME"] = prior_env
|
|
||||||
|
|
||||||
|
|
||||||
def _resolve_origin(job: dict) -> Optional[dict]:
|
def _resolve_origin(job: dict) -> Optional[dict]:
|
||||||
@@ -776,8 +776,6 @@ def _run_job_script(script_path: str) -> tuple[bool, str]:
|
|||||||
(success, output) — on failure *output* contains the error message so the
|
(success, output) — on failure *output* contains the error message so the
|
||||||
LLM can report the problem to the user.
|
LLM can report the problem to the user.
|
||||||
"""
|
"""
|
||||||
from hermes_constants import get_hermes_home
|
|
||||||
|
|
||||||
scripts_dir = _get_hermes_home() / "scripts"
|
scripts_dir = _get_hermes_home() / "scripts"
|
||||||
scripts_dir.mkdir(parents=True, exist_ok=True)
|
scripts_dir.mkdir(parents=True, exist_ok=True)
|
||||||
scripts_dir_resolved = scripts_dir.resolve()
|
scripts_dir_resolved = scripts_dir.resolve()
|
||||||
@@ -829,6 +827,17 @@ def _run_job_script(script_path: str) -> tuple[bool, str]:
|
|||||||
else:
|
else:
|
||||||
argv = [sys.executable, str(path)]
|
argv = [sys.executable, str(path)]
|
||||||
|
|
||||||
|
run_env = os.environ.copy()
|
||||||
|
run_env["HERMES_HOME"] = str(_get_hermes_home())
|
||||||
|
try:
|
||||||
|
from hermes_constants import get_subprocess_home
|
||||||
|
|
||||||
|
profile_home = get_subprocess_home()
|
||||||
|
if profile_home:
|
||||||
|
run_env["HOME"] = profile_home
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
|
||||||
try:
|
try:
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
argv,
|
argv,
|
||||||
@@ -836,6 +845,7 @@ def _run_job_script(script_path: str) -> tuple[bool, str]:
|
|||||||
text=True,
|
text=True,
|
||||||
timeout=script_timeout,
|
timeout=script_timeout,
|
||||||
cwd=str(path.parent),
|
cwd=str(path.parent),
|
||||||
|
env=run_env,
|
||||||
)
|
)
|
||||||
stdout = (result.stdout or "").strip()
|
stdout = (result.stdout or "").strip()
|
||||||
stderr = (result.stderr or "").strip()
|
stderr = (result.stderr or "").strip()
|
||||||
|
|||||||
@@ -5,10 +5,38 @@ without risk of circular imports.
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
import os
|
import os
|
||||||
|
from contextvars import ContextVar, Token
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
|
||||||
_profile_fallback_warned: bool = False
|
_profile_fallback_warned: bool = False
|
||||||
|
_UNSET = object()
|
||||||
|
_HERMES_HOME_OVERRIDE: ContextVar[str | object] = ContextVar(
|
||||||
|
"_HERMES_HOME_OVERRIDE", default=_UNSET
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def set_hermes_home_override(path: str | Path | None) -> Token:
|
||||||
|
"""Set a context-local Hermes home override and return its reset token.
|
||||||
|
|
||||||
|
This is for in-process, per-task scoping. It deliberately does not mutate
|
||||||
|
``os.environ`` because that is shared by every thread in the process.
|
||||||
|
"""
|
||||||
|
value: str | object = _UNSET if path is None else str(path)
|
||||||
|
return _HERMES_HOME_OVERRIDE.set(value)
|
||||||
|
|
||||||
|
|
||||||
|
def reset_hermes_home_override(token: Token) -> None:
|
||||||
|
"""Restore the previous context-local Hermes home override."""
|
||||||
|
_HERMES_HOME_OVERRIDE.reset(token)
|
||||||
|
|
||||||
|
|
||||||
|
def get_hermes_home_override() -> str | None:
|
||||||
|
"""Return the active context-local Hermes home override, if any."""
|
||||||
|
override = _HERMES_HOME_OVERRIDE.get()
|
||||||
|
if override is _UNSET or not override:
|
||||||
|
return None
|
||||||
|
return str(override)
|
||||||
|
|
||||||
|
|
||||||
def get_hermes_home() -> Path:
|
def get_hermes_home() -> Path:
|
||||||
@@ -27,6 +55,10 @@ def get_hermes_home() -> Path:
|
|||||||
template in ``hermes_cli/gateway.py`` and the kanban dispatcher in
|
template in ``hermes_cli/gateway.py`` and the kanban dispatcher in
|
||||||
``hermes_cli/kanban_db.py``). See https://github.com/NousResearch/hermes-agent/issues/18594.
|
``hermes_cli/kanban_db.py``). See https://github.com/NousResearch/hermes-agent/issues/18594.
|
||||||
"""
|
"""
|
||||||
|
override = get_hermes_home_override()
|
||||||
|
if override:
|
||||||
|
return Path(override)
|
||||||
|
|
||||||
val = os.environ.get("HERMES_HOME", "").strip()
|
val = os.environ.get("HERMES_HOME", "").strip()
|
||||||
if val:
|
if val:
|
||||||
return Path(val)
|
return Path(val)
|
||||||
@@ -179,7 +211,7 @@ def get_subprocess_home() -> str | None:
|
|||||||
Activation is directory-based: if the ``home/`` subdirectory doesn't
|
Activation is directory-based: if the ``home/`` subdirectory doesn't
|
||||||
exist, returns ``None`` and behavior is unchanged.
|
exist, returns ``None`` and behavior is unchanged.
|
||||||
"""
|
"""
|
||||||
hermes_home = os.getenv("HERMES_HOME")
|
hermes_home = get_hermes_home_override() or os.getenv("HERMES_HOME")
|
||||||
if not hermes_home:
|
if not hermes_home:
|
||||||
return None
|
return None
|
||||||
profile_home = os.path.join(hermes_home, "home")
|
profile_home = os.path.join(hermes_home, "home")
|
||||||
|
|||||||
@@ -162,12 +162,18 @@ class TestRunJobProfileContext:
|
|||||||
|
|
||||||
class FakeAgent:
|
class FakeAgent:
|
||||||
def __init__(self, **kwargs):
|
def __init__(self, **kwargs):
|
||||||
observed["hermes_home_during_init"] = os.environ.get("HERMES_HOME")
|
from hermes_constants import get_hermes_home
|
||||||
|
|
||||||
|
observed["env_home_during_init"] = os.environ.get("HERMES_HOME")
|
||||||
|
observed["hermes_home_during_init"] = str(get_hermes_home())
|
||||||
observed["scheduler_home_during_init"] = str(sched._get_hermes_home())
|
observed["scheduler_home_during_init"] = str(sched._get_hermes_home())
|
||||||
observed["skip_context_files"] = kwargs.get("skip_context_files")
|
observed["skip_context_files"] = kwargs.get("skip_context_files")
|
||||||
|
|
||||||
def run_conversation(self, *_a, **_kw):
|
def run_conversation(self, *_a, **_kw):
|
||||||
observed["hermes_home_during_run"] = os.environ.get("HERMES_HOME")
|
from hermes_constants import get_hermes_home
|
||||||
|
|
||||||
|
observed["env_home_during_run"] = os.environ.get("HERMES_HOME")
|
||||||
|
observed["hermes_home_during_run"] = str(get_hermes_home())
|
||||||
observed["scheduler_home_during_run"] = str(sched._get_hermes_home())
|
observed["scheduler_home_during_run"] = str(sched._get_hermes_home())
|
||||||
return {"final_response": "done", "messages": []}
|
return {"final_response": "done", "messages": []}
|
||||||
|
|
||||||
@@ -229,6 +235,8 @@ class TestRunJobProfileContext:
|
|||||||
|
|
||||||
assert success is True, f"run_job failed: error={error!r} response={response!r}"
|
assert success is True, f"run_job failed: error={error!r} response={response!r}"
|
||||||
assert observed["dotenv_paths"] == [str(profile_home / ".env")]
|
assert observed["dotenv_paths"] == [str(profile_home / ".env")]
|
||||||
|
assert observed["env_home_during_init"] == str(root)
|
||||||
|
assert observed["env_home_during_run"] == str(root)
|
||||||
assert observed["hermes_home_during_init"] == str(profile_home.resolve())
|
assert observed["hermes_home_during_init"] == str(profile_home.resolve())
|
||||||
assert observed["hermes_home_during_run"] == str(profile_home.resolve())
|
assert observed["hermes_home_during_run"] == str(profile_home.resolve())
|
||||||
assert observed["scheduler_home_during_init"] == str(profile_home.resolve())
|
assert observed["scheduler_home_during_init"] == str(profile_home.resolve())
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ See: https://github.com/NousResearch/hermes-agent/issues/4426
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
import os
|
import os
|
||||||
|
import threading
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
@@ -68,10 +69,50 @@ class TestGetSubprocessHome:
|
|||||||
monkeypatch.setenv("HERMES_HOME", str(base / "beta"))
|
monkeypatch.setenv("HERMES_HOME", str(base / "beta"))
|
||||||
home_b = get_subprocess_home()
|
home_b = get_subprocess_home()
|
||||||
|
|
||||||
|
assert home_a is not None
|
||||||
|
assert home_b is not None
|
||||||
assert home_a != home_b
|
assert home_a != home_b
|
||||||
assert home_a.endswith("alpha/home")
|
assert home_a.endswith("alpha/home")
|
||||||
assert home_b.endswith("beta/home")
|
assert home_b.endswith("beta/home")
|
||||||
|
|
||||||
|
def test_context_override_is_thread_local(self, tmp_path, monkeypatch):
|
||||||
|
root = tmp_path / "root"
|
||||||
|
profile = tmp_path / "profile"
|
||||||
|
root.mkdir()
|
||||||
|
profile.mkdir()
|
||||||
|
monkeypatch.setenv("HERMES_HOME", str(root))
|
||||||
|
|
||||||
|
from hermes_constants import (
|
||||||
|
get_hermes_home,
|
||||||
|
reset_hermes_home_override,
|
||||||
|
set_hermes_home_override,
|
||||||
|
)
|
||||||
|
|
||||||
|
ready = threading.Event()
|
||||||
|
release = threading.Event()
|
||||||
|
seen: list[str] = []
|
||||||
|
|
||||||
|
def read_from_other_thread():
|
||||||
|
ready.set()
|
||||||
|
release.wait(timeout=5)
|
||||||
|
seen.append(str(get_hermes_home()))
|
||||||
|
|
||||||
|
thread = threading.Thread(target=read_from_other_thread)
|
||||||
|
thread.start()
|
||||||
|
assert ready.wait(timeout=5)
|
||||||
|
|
||||||
|
token = set_hermes_home_override(profile)
|
||||||
|
try:
|
||||||
|
assert get_hermes_home() == profile
|
||||||
|
release.set()
|
||||||
|
thread.join(timeout=5)
|
||||||
|
finally:
|
||||||
|
reset_hermes_home_override(token)
|
||||||
|
release.set()
|
||||||
|
|
||||||
|
assert seen == [str(root)]
|
||||||
|
assert get_hermes_home() == root
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# _make_run_env() injection
|
# _make_run_env() injection
|
||||||
@@ -116,6 +157,28 @@ class TestMakeRunEnvHomeInjection:
|
|||||||
|
|
||||||
assert result["HOME"] == "/home/user"
|
assert result["HOME"] == "/home/user"
|
||||||
|
|
||||||
|
def test_context_override_bridges_to_subprocess_env(self, tmp_path, monkeypatch):
|
||||||
|
root = tmp_path / "root"
|
||||||
|
profile = tmp_path / "profile"
|
||||||
|
root.mkdir()
|
||||||
|
profile.mkdir()
|
||||||
|
(profile / "home").mkdir()
|
||||||
|
monkeypatch.setenv("HERMES_HOME", str(root))
|
||||||
|
monkeypatch.setenv("HOME", "/root")
|
||||||
|
monkeypatch.setenv("PATH", "/usr/bin:/bin")
|
||||||
|
|
||||||
|
from hermes_constants import reset_hermes_home_override, set_hermes_home_override
|
||||||
|
from tools.environments.local import _make_run_env
|
||||||
|
|
||||||
|
token = set_hermes_home_override(profile)
|
||||||
|
try:
|
||||||
|
result = _make_run_env({})
|
||||||
|
finally:
|
||||||
|
reset_hermes_home_override(token)
|
||||||
|
|
||||||
|
assert result["HERMES_HOME"] == str(profile)
|
||||||
|
assert result["HOME"] == str(profile / "home")
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# _sanitize_subprocess_env() injection
|
# _sanitize_subprocess_env() injection
|
||||||
@@ -147,6 +210,27 @@ class TestSanitizeSubprocessEnvHomeInjection:
|
|||||||
|
|
||||||
assert result["HOME"] == "/root"
|
assert result["HOME"] == "/root"
|
||||||
|
|
||||||
|
def test_context_override_bridges_to_background_env(self, tmp_path, monkeypatch):
|
||||||
|
root = tmp_path / "root"
|
||||||
|
profile = tmp_path / "profile"
|
||||||
|
root.mkdir()
|
||||||
|
profile.mkdir()
|
||||||
|
(profile / "home").mkdir()
|
||||||
|
monkeypatch.setenv("HERMES_HOME", str(root))
|
||||||
|
|
||||||
|
base_env = {"HOME": "/root", "PATH": "/usr/bin"}
|
||||||
|
from hermes_constants import reset_hermes_home_override, set_hermes_home_override
|
||||||
|
from tools.environments.local import _sanitize_subprocess_env
|
||||||
|
|
||||||
|
token = set_hermes_home_override(profile)
|
||||||
|
try:
|
||||||
|
result = _sanitize_subprocess_env(base_env)
|
||||||
|
finally:
|
||||||
|
reset_hermes_home_override(token)
|
||||||
|
|
||||||
|
assert result["HERMES_HOME"] == str(profile)
|
||||||
|
assert result["HOME"] == str(profile / "home")
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Profile bootstrap
|
# Profile bootstrap
|
||||||
|
|||||||
@@ -170,6 +170,18 @@ def _build_provider_env_blocklist() -> frozenset:
|
|||||||
_HERMES_PROVIDER_ENV_BLOCKLIST = _build_provider_env_blocklist()
|
_HERMES_PROVIDER_ENV_BLOCKLIST = _build_provider_env_blocklist()
|
||||||
|
|
||||||
|
|
||||||
|
def _inject_context_hermes_home(env: dict) -> None:
|
||||||
|
"""Bridge the context-local Hermes home override into subprocess env."""
|
||||||
|
try:
|
||||||
|
from hermes_constants import get_hermes_home_override
|
||||||
|
|
||||||
|
value = get_hermes_home_override()
|
||||||
|
if value:
|
||||||
|
env["HERMES_HOME"] = value
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
def _sanitize_subprocess_env(base_env: dict | None, extra_env: dict | None = None) -> dict:
|
def _sanitize_subprocess_env(base_env: dict | None, extra_env: dict | None = None) -> dict:
|
||||||
"""Filter Hermes-managed secrets from a subprocess environment."""
|
"""Filter Hermes-managed secrets from a subprocess environment."""
|
||||||
try:
|
try:
|
||||||
@@ -192,6 +204,8 @@ def _sanitize_subprocess_env(base_env: dict | None, extra_env: dict | None = Non
|
|||||||
elif key not in _HERMES_PROVIDER_ENV_BLOCKLIST or _is_passthrough(key):
|
elif key not in _HERMES_PROVIDER_ENV_BLOCKLIST or _is_passthrough(key):
|
||||||
sanitized[key] = value
|
sanitized[key] = value
|
||||||
|
|
||||||
|
_inject_context_hermes_home(sanitized)
|
||||||
|
|
||||||
# Per-profile HOME isolation for background processes (same as _make_run_env).
|
# Per-profile HOME isolation for background processes (same as _make_run_env).
|
||||||
from hermes_constants import get_subprocess_home
|
from hermes_constants import get_subprocess_home
|
||||||
_profile_home = get_subprocess_home()
|
_profile_home = get_subprocess_home()
|
||||||
@@ -292,6 +306,8 @@ def _make_run_env(env: dict) -> dict:
|
|||||||
if not _IS_WINDOWS and "/usr/bin" not in existing_path.split(":"):
|
if not _IS_WINDOWS and "/usr/bin" not in existing_path.split(":"):
|
||||||
run_env["PATH"] = f"{existing_path}:{_SANE_PATH}" if existing_path else _SANE_PATH
|
run_env["PATH"] = f"{existing_path}:{_SANE_PATH}" if existing_path else _SANE_PATH
|
||||||
|
|
||||||
|
_inject_context_hermes_home(run_env)
|
||||||
|
|
||||||
# Per-profile HOME isolation: redirect system tool configs (git, ssh, gh,
|
# Per-profile HOME isolation: redirect system tool configs (git, ssh, gh,
|
||||||
# npm …) into {HERMES_HOME}/home/ when that directory exists. Only the
|
# npm …) into {HERMES_HOME}/home/ when that directory exists. Only the
|
||||||
# subprocess sees the override — the Python process keeps the real HOME.
|
# subprocess sees the override — the Python process keeps the real HOME.
|
||||||
|
|||||||
Reference in New Issue
Block a user