From 9c48d47aaf0e959a4eda939ac9b9087171c96f30 Mon Sep 17 00:00:00 2001 From: Gianfranco Piana <52470719+gianfrancopiana@users.noreply.github.com> Date: Mon, 18 May 2026 11:47:44 -0300 Subject: [PATCH] fix(cron): isolate profile job env --- cron/scheduler.py | 27 ++++++++++----- tests/cron/test_cron_profile.py | 60 ++++++++++++++++++++++++++++++++- tools/cronjob_tools.py | 2 +- 3 files changed, 78 insertions(+), 11 deletions(-) diff --git a/cron/scheduler.py b/cron/scheduler.py index 14d2a9bb7e8..1e28711b16c 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -151,12 +151,16 @@ def _job_profile_context(job_id: str, profile: Optional[str]): """Temporarily run a job under a specific Hermes profile. 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, - The scheduler's test/override hook and a context-local Hermes home override + an individual job can opt into a different runtime profile. While active, + the scheduler's test/override hook and a context-local Hermes home override both point at the resolved profile directory so _get_hermes_home(), .env/config loading, script resolution, AIAgent construction, and downstream - get_hermes_home() callers agree on the same home without mutating the - process-wide environment seen by other threads. + get_hermes_home() callers agree on the same home. + + Some existing provider/config paths still load profile .env values through + os.environ, so profile jobs also snapshot and restore the process + environment on exit. tick() runs profile jobs sequentially to keep that + temporary mutation isolated from other scheduled jobs. """ raw_profile = str(profile or "").strip() if not raw_profile: @@ -165,6 +169,7 @@ def _job_profile_context(job_id: str, profile: Optional[str]): global _hermes_home prior_override = _hermes_home + env_snapshot = os.environ.copy() from hermes_cli.profiles import normalize_profile_name, resolve_profile_env from hermes_constants import reset_hermes_home_override, set_hermes_home_override @@ -187,6 +192,8 @@ def _job_profile_context(job_id: str, profile: Optional[str]): _hermes_home = prior_override if override_token is not None: reset_hermes_home_override(override_token) + os.environ.clear() + os.environ.update(env_snapshot) def _resolve_origin(job: dict) -> Optional[dict]: @@ -1843,11 +1850,13 @@ def tick(verbose: bool = True, adapters=None, loop=None) -> int: mark_job_run(job["id"], False, str(e)) return False - # Partition due jobs: jobs with a per-job workdir and/or profile mutate - # process-global runtime state inside run_job (TERMINAL_CWD, - # HERMES_HOME, and the scheduler's _hermes_home hook), so they MUST run + # Partition due jobs: jobs with a per-job workdir and/or profile touch + # process-global runtime state inside run_job. Workdir jobs temporarily + # set os.environ["TERMINAL_CWD"]; profile jobs use a context-local + # Hermes home override, scheduler _hermes_home hook, and temporary + # profile .env load into os.environ with snapshot/restore. They MUST run # sequentially to avoid corrupting each other. Jobs without either field - # leave those env overrides untouched and stay parallel-safe. + # stay parallel-safe. sequential_jobs = [ j for j in due_jobs if (j.get("workdir") or "").strip() or (j.get("profile") or "").strip() @@ -1859,7 +1868,7 @@ def tick(verbose: bool = True, adapters=None, loop=None) -> int: _results: list = [] - # Sequential pass for env-mutating jobs. + # Sequential pass for env/context-mutating jobs. for job in sequential_jobs: _ctx = contextvars.copy_context() _results.append(_ctx.run(_process_job, job)) diff --git a/tests/cron/test_cron_profile.py b/tests/cron/test_cron_profile.py index de9b3b0d9ed..a8f438c185d 100644 --- a/tests/cron/test_cron_profile.py +++ b/tests/cron/test_cron_profile.py @@ -151,7 +151,11 @@ class TestCronjobToolProfile: assert "profile" in CRONJOB_SCHEMA["parameters"]["properties"] desc = CRONJOB_SCHEMA["parameters"]["properties"]["profile"]["description"] - assert "hermes profile" in desc.lower() + desc_lower = desc.lower() + assert "hermes profile" in desc_lower + assert "context-local" in desc_lower + assert "subprocess" in desc_lower + assert "temporarily sets hermes_home" not in desc_lower class TestRunJobProfileContext: @@ -165,6 +169,12 @@ class TestRunJobProfileContext: from hermes_constants import get_hermes_home observed["env_home_during_init"] = os.environ.get("HERMES_HOME") + observed["profile_env_only_during_init"] = os.environ.get( + "HERMES_PROFILE_TEST_ONLY" + ) + observed["profile_env_shared_during_init"] = os.environ.get( + "HERMES_PROFILE_TEST_SHARED" + ) observed["hermes_home_during_init"] = str(get_hermes_home()) observed["scheduler_home_during_init"] = str(sched._get_hermes_home()) observed["skip_context_files"] = kwargs.get("skip_context_files") @@ -173,6 +183,12 @@ class TestRunJobProfileContext: from hermes_constants import get_hermes_home observed["env_home_during_run"] = os.environ.get("HERMES_HOME") + observed["profile_env_only_during_run"] = os.environ.get( + "HERMES_PROFILE_TEST_ONLY" + ) + observed["profile_env_shared_during_run"] = os.environ.get( + "HERMES_PROFILE_TEST_SHARED" + ) observed["hermes_home_during_run"] = str(get_hermes_home()) observed["scheduler_home_during_run"] = str(sched._get_hermes_home()) return {"final_response": "done", "messages": []} @@ -245,6 +261,48 @@ class TestRunJobProfileContext: assert os.environ["HERMES_HOME"] == str(root) assert sched._get_hermes_home() == root + def test_profile_dotenv_environment_is_restored( + self, isolated_cron_profile_home, monkeypatch + ): + import dotenv + import cron.scheduler as sched + + root, profile_home = isolated_cron_profile_home + observed: dict = {} + self._install_agent_stubs(monkeypatch, observed) + monkeypatch.setenv("HERMES_PROFILE_TEST_SHARED", "outer") + monkeypatch.delenv("HERMES_PROFILE_TEST_ONLY", raising=False) + + def fake_load_dotenv(path, *_a, **_kw): + observed.setdefault("dotenv_paths", []).append(str(path)) + os.environ["HERMES_PROFILE_TEST_SHARED"] = "profile-value" + os.environ["HERMES_PROFILE_TEST_ONLY"] = "profile-only" + os.environ["HERMES_CRON_TIMEOUT"] = "123" + return True + + monkeypatch.setattr(dotenv, "load_dotenv", fake_load_dotenv) + + job = { + "id": "env-profile", + "name": "profile-env-job", + "profile": "support", + "schedule_display": "manual", + } + + success, _output, _response, error = sched.run_job(job) + + assert success is True, error + assert observed["dotenv_paths"] == [str(profile_home / ".env")] + assert observed["profile_env_only_during_init"] == "profile-only" + assert observed["profile_env_shared_during_init"] == "profile-value" + assert observed["profile_env_only_during_run"] == "profile-only" + assert observed["profile_env_shared_during_run"] == "profile-value" + assert os.environ["HERMES_PROFILE_TEST_SHARED"] == "outer" + assert "HERMES_PROFILE_TEST_ONLY" not in os.environ + assert os.environ["HERMES_CRON_TIMEOUT"] == "0" + assert os.environ["HERMES_HOME"] == str(root) + assert sched._get_hermes_home() == root + def test_no_agent_profile_uses_profile_scripts_dir_and_restores_env( self, isolated_cron_profile_home, monkeypatch ): diff --git a/tools/cronjob_tools.py b/tools/cronjob_tools.py index 5d91a6700d8..ea5df132712 100644 --- a/tools/cronjob_tools.py +++ b/tools/cronjob_tools.py @@ -666,7 +666,7 @@ Important safety rule: cron-run sessions should not recursively schedule more cr }, "profile": { "type": "string", - "description": "Optional Hermes profile name to run the job under. When set, the scheduler resolves that profile and temporarily sets HERMES_HOME before loading .env/config.yaml and running the job. Use 'default' for the root Hermes profile. Named profiles must already exist. When unset (default), preserves the scheduler's existing profile. On update, pass an empty string to clear. Jobs with profile run sequentially (not parallel) to keep process-global profile state isolated." + "description": "Optional Hermes profile name to run the job under. When set, the scheduler resolves that profile, applies a context-local Hermes home override, loads that profile's config/.env for the run, and bridges HERMES_HOME into subprocesses. Any temporary process-environment changes from profile .env loading are restored after the job exits. Use 'default' for the root Hermes profile. Named profiles must already exist. When unset (default), preserves the scheduler's existing profile. On update, pass an empty string to clear. Jobs with profile run sequentially (not parallel) to keep profile-scoped runtime state isolated." }, }, "required": ["action"]