Skip to content

fix(cron): propagate HERMES_HOME to script subprocesses - #19810

Closed
alexzhu0 wants to merge 1 commit into
NousResearch:mainfrom
alexzhu0:fix/cron-scheduler-propagate-hermes-home
Closed

alexzhu0 wants to merge 1 commit into
NousResearch:mainfrom
alexzhu0:fix/cron-scheduler-propagate-hermes-home

Conversation

@alexzhu0

@alexzhu0 alexzhu0 commented May 4, 2026

Copy link
Copy Markdown
Contributor

Partial fix for the process-spawn dimension of #18594.

What this fixes

#18746 (@liuhao1024) already added a one-shot stderr warning when get_hermes_home() falls back under an active profile. That was defense-in-depth — the observability layer. This PR addresses one of the concrete spawn sites that triggers the warning: the cron script runner.

Root cause in scope

cron/scheduler.py::_run_job_script() calls subprocess.run(...) without env=, so the child inherits only the shell environment. On systemd / launchd / docker-exec deployments where the cron daemon's unit file doesn't ship an explicit Environment=HERMES_HOME=..., the child starts with HERMES_HOME unset and get_hermes_home() falls back to ~/.hermes (default profile) — silently corrupting cross-profile data as described in the incident report in #18594.

Fix shape

Inject the scheduler's resolved _hermes_home into the child env via setdefault semantics:

child_env = dict(os.environ)
child_env.setdefault("HERMES_HOME", str(_hermes_home))
  • setdefault semantics: if the shell env already has HERMES_HOME (user explicitly ran hermes with an override), that wins.
  • dict(os.environ) base: all other env vars (PATH, HOME, HERMES_PROFILE, user custom markers) still flow through — no clean-env regression.

Why only cron/scheduler.py

I audited all subprocess spawns under cron/, hermes_cli/, and tools/:

Spawn site Passes env= ? HERMES_HOME propagated?
cron/scheduler.py:595 _run_job_script ❌ no env This PR
hermes_cli/kanban_db.py:2117 kanban dispatcher env = dict(os.environ) ✅ inherits naturally
tools/skills_hub.py:193 gh auth lookup (not scope — reads GitHub token, doesn't need HERMES_HOME) n/a
tools/environments/docker.py ✅ explicit env mapping
tools/terminal_tool.py ✅ passes configured env

So this is the single narrow fix that was needed at the cron layer. Other layers aren't regressed.

Not in scope

  • Changes to get_hermes_home() itself — @liuhao1024's call was that classic-mode users rely on the ~/.hermes fallback, so strict raising isn't viable. Agreed. Spawn-layer propagation is the right fix.
  • Changes to the systemd gateway unit template — that's a separate file and already has Environment=HERMES_HOME=... where needed.
  • Audit of MCP stdio subprocesses — different failure mode, different fix.

Tests

3 new in TestRunJobScript, 7 existing → 10/10 pass:

  • test_script_sees_hermes_home_when_shell_env_dropped — the core regression: scheduler has _hermes_home resolved, shell env drops HERMES_HOME, child must still see it
  • test_script_env_does_not_overwrite_explicit_shell_valuesetdefault semantics guard
  • test_script_inherits_other_env_varsdict(os.environ) base semantics guard

Full file: 40/40 pass.

Files

  • cron/scheduler.py: +11 / -1
  • tests/cron/test_cron_script.py: +96

Partial fix for the process-spawn dimension of #18594.

## Root cause

`cron/scheduler.py::_run_job_script()` calls ``subprocess.run(...)`` with
no ``env=`` passed, so the child inherits only whatever's in the shell
environment. On systemd / launchd / docker-exec deployments where the
cron daemon's unit file doesn't ship an explicit ``Environment=HERMES_HOME=...``,
the child starts with an empty ``HERMES_HOME`` and falls back to
``~/.hermes`` (the default profile) via ``get_hermes_home()``.

The defense-in-depth warning from #18746 (@liuhao1024) already catches
this and logs once to stderr. This commit removes the condition that
triggers the warning for cron scripts: pass the scheduler's resolved
``_hermes_home`` into the child via ``env=``.

## Scope

Only ``cron/scheduler.py::_run_job_script()``. This is the single subprocess
spawn in the cron subsystem that runs user-provided scripts. Other spawn
paths (kanban dispatcher in ``hermes_cli/kanban_db.py``, mcp stdio
subprocess, browser daemon) already handle env propagation explicitly.

## Behaviour

- If shell env has ``HERMES_HOME``: use that (``setdefault`` semantics).
  A user who launched the hermes process with an explicit ``HERMES_HOME``
  override still wins over the scheduler's resolved value.
- If shell env does NOT have ``HERMES_HOME``: inject the scheduler's
  resolved ``_hermes_home``. Child sees the active profile.
- All other env vars still inherit via ``dict(os.environ)``. PATH, HOME,
  HERMES_PROFILE, HERMES_CUSTOM_MARKER, etc. keep flowing to the child.

## Tests

3 new in ``TestRunJobScript``:
- ``test_script_sees_hermes_home_when_shell_env_dropped`` — the bug
  case. Scheduler resolved hermes_home from config, shell env got
  dropped by systemd / launchd / docker, child must still see it.
- ``test_script_env_does_not_overwrite_explicit_shell_value`` —
  regression guard for ``setdefault`` semantics.
- ``test_script_inherits_other_env_vars`` — regression guard that we
  use ``dict(os.environ)`` as the base, not a clean env.

Full file: 40/40 pass.

## Files

- ``cron/scheduler.py``: +11 / -1
- ``tests/cron/test_cron_script.py``: +96

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management labels May 4, 2026
@alexzhu0

Copy link
Copy Markdown
Contributor Author

Status / gentle ping — opened 2026-05-04, no review yet.

This is a partial fix for the spawn dimension of #18594. #18746 (@liuhao1024) already added the observability layer (one-shot stderr warning when get_hermes_home() falls back). This PR closes one of the concrete spawn sites — cron's script runner was launching subprocesses without propagating HERMES_HOME, so cron-spawned scripts under a non-default profile would silently fall back to ~/.hermes/ instead of the active profile dir.

Re: failing CInix (ubuntu-latest), Scan PR for critical supply chain risks, and test are all failing on main for unrelated reasons (Anthropic 1M-beta header config mismatch, OpenAI max_completion_tokens rename, matrix _auto_create_thread await semantics). None touch cron/ or subprocess spawn code. Happy to rebase if a fresh run would help clear the noise.

cc @teknium1 — small, surgical fix; would appreciate eyes when you get a moment.

@alexzhu0

Copy link
Copy Markdown
Contributor Author

Closing — defense-in-depth piece (warning when fallback hits) shipped via @liuhao1024's #18746 in v0.12.0, and the cron subsystem has shifted significantly enough that this branch is now DIRTY against main. The narrower spawn-propagation fix would need to be re-derived against the new tenant/profile-aware kanban dispatcher. Not worth a stale rebase against the new layout — happy to revisit if the warning-channel telemetry shows actual occurrences in the wild.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants