Skip to content

fix(cron): catch RuntimeError from Path.expanduser() in lifecycle guard - #56517

Open
srojk34 wants to merge 1 commit into
NousResearch:mainfrom
srojk34:fix/cron-lifecycle-guard-expanduser-runtimeerror
Open

fix(cron): catch RuntimeError from Path.expanduser() in lifecycle guard#56517
srojk34 wants to merge 1 commit into
NousResearch:mainfrom
srojk34:fix/cron-lifecycle-guard-expanduser-runtimeerror

Conversation

@srojk34

@srojk34 srojk34 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

cron/lifecycle_guard.py::_resolve_script_path calls Path(script_path).expanduser() to resolve a cron job's script path before scanning it for gateway-lifecycle commands (check_gateway_lifecycle, enforced in cron.jobs.create_job — reachable via both hermes cron create and the agent's cronjob model tool directly).

Path.expanduser() raises RuntimeError (not OSError) for a ~user-shaped path with no matching system user:

>>> from pathlib import Path
>>> Path('~nonexistentuser12345/foo').expanduser()
RuntimeError: Can't determine home directory for 'nonexistentuser12345'

_read_script_for_scanning's except OSError: doesn't catch this, so an LLM-authored script value that happens to start with ~someuser/... (a plausible thing for an agent to produce — the same trigger shape already fixed for agent/subdirectory_hints.py in #c126a99fc, "LLMs use ~ for 'approximately'" or a guessed/malformed path) crashes cron job creation with an unhandled RuntimeError instead of the intended graceful GatewayLifecycleBlocked/pass-through behavior the module is designed around.

Fix

Add RuntimeError to the caught exceptions in _read_script_for_scanning, mirroring the exact fix already applied to agent/subdirectory_hints.py's three Path.expanduser()/Path.home() call sites in #c126a99fc.

Test plan

  • New regression test test_tilde_unknown_user_script_does_not_crash — confirmed it fails with an uncaught RuntimeError on the pre-fix code (via git stash) and passes after the fix
  • pytest tests/hermes_cli/test_gateway_restart_loop.py -q — 66 passed
  • ruff check on all changed files — clean
  • Checked for competitors: PR refactor: sweep remaining bare expanduser() to safe_expanduser() (depends on #41870) #41881 ("sweep remaining bare expanduser() to safe_expanduser()") is a 2-file refactor (cron/jobs.py, cron/scheduler.py) that does not include cron/lifecycle_guard.py — confirmed via gh pr view --json files. No overlap.

cron/lifecycle_guard.py::_resolve_script_path calls
Path(script_path).expanduser() to resolve a cron job's script path before
scanning it for gateway-lifecycle commands. Path.expanduser() raises
RuntimeError (not OSError) for a ~user-shaped path with no matching
system user -- e.g. an LLM-authored script value that happens to start
with ~someuser/... -- but _read_script_for_scanning only caught OSError,
so the RuntimeError propagated uncaught through cron.jobs.create_job,
crashing cron job creation (reachable via both the CLI and the agent's
cronjob model tool) instead of degrading to "can't scan it".

Same bug class already fixed in agent/subdirectory_hints.py (c126a99).
Add RuntimeError to the caught exceptions, verified live: the new
regression test fails with an uncaught RuntimeError before this change
and passes after.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the creation-time failure and adding a targeted regression test. The premise is confirmed on current main: cron/lifecycle_guard.py:89 expands the supplied script path while :108 catches only OSError; cron/jobs.py:1158-1159 invokes that guard from the shared creation chokepoint.

Problems

  • The same accepted script value still reaches an unguarded Path(script_path).expanduser() in cron/scheduler.py:2050, before _run_job_script's error-to-result handling. With this PR, a ~unknown-user/... script no longer fails at creation but can still raise when the job fires.

Suggested changes

  • Convert that scheduler-side RuntimeError into the normal failed-script result and cover the runtime path with the same unknown-user tilde input.
  • Add a create_job-level assertion for the new creation-time contract, in addition to the direct guard test.

Automated hermes-sweeper review.

Comment thread cron/lifecycle_guard.py
"utf-8", errors="replace"
)
except OSError:
except (OSError, RuntimeError):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please handle the same ~unknown-user input in cron/scheduler.py:2050: _run_job_script() calls Path(script_path).expanduser() before its exception-to-result handling. This catch now permits creation, but the scheduled run can still raise RuntimeError instead of returning the documented failed-script tuple.

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 P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants