mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-25 17:18:11 +00:00
fix: scope cron workdir to job session instead of process-global state (#69396)
The cron scheduler was mutating process-global state in two places: 1. no_agent path called os.chdir() which changed the global process cwd, leaking into concurrent gateway sessions. 2. The agent path set os.environ['TERMINAL_CWD'] which any gateway session could read during context-file discovery via resolve_context_cwd/build_context_files_prompt. Fix: - no_agent path: pass workdir as subprocess cwd parameter to _run_job_script() instead of os.chdir(). The Python process cwd is never mutated. - Agent path: in addition to the lock-serialized TERMINAL_CWD, also set the per-context _SESSION_CWD ContextVar from agent.runtime_cwd. This ContextVar is scoped to the current thread/context and NEVER leaks into other sessions. resolve_context_cwd() checks _SESSION_CWD first, so the cron's own context file discovery uses the correct workdir, while gateway sessions (which have no override) fall through to their own TERMINAL_CWD.
This commit is contained in:
Hermes Agent
Teknium
parent
214099d08d
commit
91cf5448d8
1 changed files with 66 additions and 23 deletions
|
|
@ -2202,7 +2202,10 @@ def _windows_cron_python_invocation(python_exe: str) -> tuple[str, dict[str, str
|
|||
return str(interpreter), env_overlay
|
||||
|
||||
|
||||
def _run_job_script(script_path: str) -> tuple[bool, str]:
|
||||
def _run_job_script(
|
||||
script_path: str,
|
||||
workdir: Optional[str] = None,
|
||||
) -> tuple[bool, str]:
|
||||
"""Execute a cron job's data-collection script and capture its output.
|
||||
|
||||
Scripts must reside within HERMES_HOME/scripts/. Both relative and
|
||||
|
|
@ -2228,6 +2231,12 @@ def _run_job_script(script_path: str) -> tuple[bool, str]:
|
|||
script_path: Path to the script. Relative paths are resolved
|
||||
against HERMES_HOME/scripts/. Absolute and ~-prefixed paths
|
||||
are also validated to ensure they stay within the scripts dir.
|
||||
workdir: Optional absolute path to use as the script's cwd.
|
||||
When set, the subprocess runs in this directory instead of
|
||||
the scripts-dir parent. The Python process cwd is NEVER
|
||||
mutated, avoiding the global-side-effect bug where a cron
|
||||
job's ``os.chdir()`` leaks into concurrent gateway sessions
|
||||
(#69396).
|
||||
|
||||
Returns:
|
||||
(success, output) — on failure *output* contains the error message so the
|
||||
|
|
@ -2298,12 +2307,17 @@ def _run_job_script(script_path: str) -> tuple[bool, str]:
|
|||
}
|
||||
env = _sanitize_subprocess_env(os.environ.copy())
|
||||
env.update(env_overlay)
|
||||
# Use the job's workdir as the subprocess cwd when configured,
|
||||
# otherwise default to the scripts-dir parent (back-compat).
|
||||
# NEVER mutate the Python process cwd — that would leak into
|
||||
# concurrent gateway sessions (#69396).
|
||||
_script_cwd = workdir or str(path.parent)
|
||||
result = subprocess.run(
|
||||
argv,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=script_timeout,
|
||||
cwd=str(path.parent),
|
||||
cwd=_script_cwd,
|
||||
env=env,
|
||||
**popen_kwargs,
|
||||
)
|
||||
|
|
@ -2337,7 +2351,7 @@ def _run_job_script(script_path: str) -> tuple[bool, str]:
|
|||
|
||||
|
||||
def _run_job_script_with_claim_heartbeat(
|
||||
job: dict, script_path: str
|
||||
job: dict, script_path: str, workdir: Optional[str] = None,
|
||||
) -> tuple[bool, str]:
|
||||
"""Run a cron script while keeping its owned one-shot claim fresh.
|
||||
|
||||
|
|
@ -2359,7 +2373,7 @@ def _run_job_script_with_claim_heartbeat(
|
|||
and schedule.get("kind") == "once"
|
||||
and owner
|
||||
):
|
||||
return _run_job_script(script_path)
|
||||
return _run_job_script(script_path, workdir=workdir)
|
||||
|
||||
job_id = str(job.get("id") or "")
|
||||
stop = threading.Event()
|
||||
|
|
@ -2390,10 +2404,10 @@ def _run_job_script_with_claim_heartbeat(
|
|||
job_id,
|
||||
exc_info=True,
|
||||
)
|
||||
return _run_job_script(script_path)
|
||||
return _run_job_script(script_path, workdir=workdir)
|
||||
|
||||
try:
|
||||
return _run_job_script(script_path)
|
||||
return _run_job_script(script_path, workdir=workdir)
|
||||
finally:
|
||||
stop.set()
|
||||
# Event.wait() wakes immediately. Keep completion bounded if the
|
||||
|
|
@ -2796,25 +2810,26 @@ def run_job(
|
|||
return False, "", "", err
|
||||
|
||||
# Apply workdir if configured — lets scripts use predictable relative
|
||||
# paths. For no_agent jobs this is just the subprocess cwd (not an
|
||||
# agent TERMINAL_CWD bridge).
|
||||
# paths. For no_agent jobs this is passed as the subprocess cwd so the
|
||||
# Python process cwd is NEVER mutated — avoiding the global-side-effect
|
||||
# bug where os.chdir() leaks into concurrent gateway sessions (#69396).
|
||||
_job_workdir = (job.get("workdir") or "").strip() or None
|
||||
_prior_cwd = None
|
||||
if _job_workdir and Path(_job_workdir).is_dir():
|
||||
_prior_cwd = os.getcwd()
|
||||
try:
|
||||
os.chdir(_job_workdir)
|
||||
except OSError:
|
||||
_prior_cwd = None
|
||||
if _job_workdir and not Path(_job_workdir).is_dir():
|
||||
logger.warning(
|
||||
"Job '%s': configured workdir %r no longer exists — running without it",
|
||||
job_id, _job_workdir,
|
||||
)
|
||||
_job_workdir = None
|
||||
|
||||
try:
|
||||
ok, output = _run_job_script_with_claim_heartbeat(job, script_path)
|
||||
finally:
|
||||
if _prior_cwd is not None:
|
||||
try:
|
||||
os.chdir(_prior_cwd)
|
||||
except OSError:
|
||||
pass
|
||||
ok, output = _run_job_script_with_claim_heartbeat(
|
||||
job, script_path, workdir=_job_workdir,
|
||||
)
|
||||
except Exception:
|
||||
logger.exception(
|
||||
"Job '%s': script execution raised unexpectedly", job_id,
|
||||
)
|
||||
ok, output = False, "Script execution failed"
|
||||
|
||||
now_iso = _hermes_now().strftime("%Y-%m-%d %H:%M:%S")
|
||||
|
||||
|
|
@ -3064,7 +3079,8 @@ def run_job(
|
|||
_VAR_MAP[_var_name].set("")
|
||||
|
||||
# Per-job working directory. When set (and validated at create/update
|
||||
# time), we point TERMINAL_CWD at it so:
|
||||
# time), we point both TERMINAL_CWD and the per-context _SESSION_CWD
|
||||
# at it so:
|
||||
# - build_context_files_prompt() picks up AGENTS.md / CLAUDE.md /
|
||||
# .cursorrules from the job's project dir, AND
|
||||
# - the terminal, file, and code-exec tools run commands from there.
|
||||
|
|
@ -3079,6 +3095,16 @@ def run_job(
|
|||
# file / code-exec commands in the wrong directory. For workdir-less jobs
|
||||
# we leave TERMINAL_CWD untouched — preserves the original behaviour
|
||||
# (skip_context_files=True, tools use whatever cwd the scheduler has).
|
||||
#
|
||||
# In addition to the lock-serialized TERMINAL_CWD, we also set the
|
||||
# per-context _SESSION_CWD (agent.runtime_cwd.set_session_cwd). This
|
||||
# ContextVar is scoped to the current context/thread, so it NEVER leaks
|
||||
# into concurrent gateway sessions — resolving the root cause of #69396.
|
||||
# Gateway sessions that read TERMINAL_CWD while this cron has temporarily
|
||||
# overridden it will still see the cron's workdir, but the critical path
|
||||
# (resolve_context_cwd / build_context_files_prompt) checks _SESSION_CWD
|
||||
# first and finds no override in the gateway's context, so the gateway
|
||||
# session loads the correct AGENTS.md from its own workspace.
|
||||
_job_workdir = (job.get("workdir") or "").strip() or None
|
||||
if _job_workdir and not Path(_job_workdir).is_dir():
|
||||
# Directory was removed between create-time validation and now. Log
|
||||
|
|
@ -3107,9 +3133,19 @@ def run_job(
|
|||
# (every future job blocks on acquire_*); a leaked reader blocks all
|
||||
# future writers. Acquire itself can't leak (it either blocks or returns).
|
||||
try:
|
||||
# Set the per-context _SESSION_CWD before any tool or prompt
|
||||
# construction, so resolve_context_cwd() and resolve_agent_cwd()
|
||||
# prefer this scoped value over the process-global TERMINAL_CWD.
|
||||
# Gateway sessions that lack this override fall through to the
|
||||
# gateway's own TERMINAL_CWD, never picking up the cron's workdir
|
||||
# (#69396).
|
||||
_session_cwd_token = None
|
||||
from agent.runtime_cwd import set_session_cwd, clear_session_cwd
|
||||
|
||||
if _job_workdir:
|
||||
os.environ["TERMINAL_CWD"] = _job_workdir
|
||||
logger.info("Job '%s': using workdir %s", job_id, _job_workdir)
|
||||
_session_cwd_token = set_session_cwd(_job_workdir)
|
||||
|
||||
# Re-read .env and config.yaml fresh every run so provider/key
|
||||
# changes take effect without a gateway restart. Route through
|
||||
|
|
@ -3717,6 +3753,13 @@ def run_job(
|
|||
_terminal_cwd_lock.release_read()
|
||||
# Clean up ContextVar session/delivery state for this job.
|
||||
clear_session_vars(_ctx_tokens)
|
||||
# Clear the per-context workdir override if one was set. This is
|
||||
# the cleanup counterpart to the set_session_cwd() call above.
|
||||
if _session_cwd_token is not None:
|
||||
try:
|
||||
clear_session_cwd()
|
||||
except Exception:
|
||||
pass
|
||||
for _var_name in _cron_delivery_vars:
|
||||
_VAR_MAP[_var_name].set("")
|
||||
if _session_db:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue