Skip to content

fix(cron): use POSIX paths for bash scripts on native Windows - #23405

Open
CalmProton wants to merge 1 commit into
NousResearch:mainfrom
CalmProton:fix/cron-windows-bash-path
Open

fix(cron): use POSIX paths for bash scripts on native Windows#23405
CalmProton wants to merge 1 commit into
NousResearch:mainfrom
CalmProton:fix/cron-windows-bash-path

Conversation

@CalmProton

@CalmProton CalmProton commented May 10, 2026

Copy link
Copy Markdown
Contributor

Summary

On native Windows with Git Bash, .sh / .bash cron scripts always fail because bash interprets backslashes in the path argument as escape sequences, mangling C:\Users\... into C:Users... (exit code 127, script not found).

This PR gates a one-line fix behind os.name == "nt": use path.as_posix() to produce forward-slash paths (/c/Users/...) that MSYS2 bash understands natively. Non-Windows platforms are completely unaffected — they continue to use str(path).

How to Test

  1. On native Windows with Git Bash, create a cron job with no_agent: true and a .sh script
  2. Confirm the script runs successfully instead of failing with exit code 127

Or use the existing test suite:

python -m pytest tests/cron/test_cron_no_agent.py -v

The test test_run_job_script_shell_script_runs_via_bash already exercises this code path and passes both before and after the change (the test uses POSIX paths, so the fix is invisible to it — as intended).

Platform Tested

  • OS: Windows 11 (native, git-bash)
  • Python: 3.11.14
  • Hermes: v0.13.0

Pre-submit checks

  • scripts/check-windows-footguns.py — zero footguns found
  • tests/cron/test_cron_no_agent.py — all 18 tests pass

Related

Closes #23404

On native Windows, bash (Git Bash / MSYS2) interprets backslashes in path
arguments as escape sequences, silently mangling them — a path like
C:\Users\... becomes C:Users... and the script is never found, causing
all no_agent cron jobs with .sh scripts to fail with exit code 127.

Fix: use path.as_posix() on Windows to produce forward-slash paths
(/c/Users/...) that MSYS2 bash understands natively.  Gated behind
os.name == 'nt' so non-Windows platforms continue to use str(path)
with zero change in behavior.

Closes NousResearch#23404

@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 targeting the native-Windows cron-script failure. The current main path still passes str(path) directly to bash at cron/scheduler.py:2091, so the report remains actionable.

Problems

  • The proposed path.as_posix() call does not produce the /c/Users/... MSYS path stated in the PR. A PureWindowsPath produces C:/Users/...; the repository's established converter at tools/environments/local.py:43-58 performs the needed drive-prefix conversion to /c/....
  • The diff has no regression test. tests/cron/test_cron_no_agent.py:288-309 verifies shell execution, but does not inspect the Windows argv passed to bash.

Suggested changes

  • Reuse or extract the established Windows-to-MSYS conversion behavior for the cron bash argument.
  • Add a host-independent argv-capture test for a drive-qualified Windows path, plus a POSIX no-conversion test.

Automated hermes-sweeper review.

Comment thread cron/scheduler.py
# path.as_posix() normalizes to POSIX forward-slash form
# (/c/Users/...) which MSYS2 bash understands natively.
# On non-Windows platforms, str(path) is already a valid POSIX path.
if os.name == "nt":

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.

Path.as_posix() turns a drive-qualified Windows path into C:/..., not the /c/... MSYS form claimed here. Please reuse or extract the conversion behavior in tools/environments/local.py:_windows_to_msys_path, and add an argv-capture regression test for the exact /c/... result.

@teknium1 teknium1 added sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026
@andrexibiza

Copy link
Copy Markdown
Contributor

Reconciliation (2026-08-03): this PR's fix is covered by the open canonical #77393fix(cron): pass Git-Bash-safe script path to bash on Windows (.sh exit 127 class) — all required checks green, mergeable_state: clean against current main (fe6330de), suite-verified locally (161 passed across the cron + approval suites; the 2 remaining approval failures reproduce on clean main — pre-existing). It fixes the whole class (execution site + the sibling tools/approval.py hint), adds regression tests and docs, and binds Fixes #23404 / Fixes #65317 / Closes #43073.

This branch's CI (last run on the May 10 base) fails only on pre-existing main breakage, none of it in this PR's files:

  • test (9 failed) — gateway/hermes_cli/run_agent tests, all reworked on main since.
  • e2e (9 failed) — reset_session assertions in tests/e2e/test_platform_commands.py, changed on main since.
  • Windows footguns (2) — tools/process_registry.py:588 os.killpg/bare SIGKILL, a file this PR never touches; rewritten on main since.

Recommendation: close as duplicate of #77393. The as_posix() approach here is the same normalization the canonical ships via _bash_safe_path() — nothing is lost by consolidating.

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 sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(cron): bash script path mangled by backslash escaping on native Windows

4 participants