fix(cron): resolve bash for .sh jobs via shared Windows-aware resolver (#46332) - #77532
fix(cron): resolve bash for .sh jobs via shared Windows-aware resolver (#46332)#77532andrexibiza wants to merge 1 commit into
Conversation
NousResearch#46332) cron _run_job_script() resolved bash with a raw shutil.which("bash"); on native Windows that returns the System32 WSL launcher before Git for Windows, so .sh cron jobs ran under WSL bash (exit 127 / silent failure) whenever Git Bash wasn't first on PATH. Route through tools/environments.local._find_bash() — the shared resolver (portable Git → Git for Windows dirs → PATH lookup with start probe) that already encodes the Git-Bash-over-WSL preference. RuntimeError (no usable bash on Windows) surfaces the actionable error; any other resolver failure degrades to the historical PATH lookup. Adds regression tests (resolver wins over raw which, RuntimeError message, non-RuntimeError fallback). Prior art: NousResearch#46364 and NousResearch#52204 diagnosed the cron-side launcher selection; main's resolver shipped via NousResearch#47837 — this takes the shared path rather than re-implementing precedence (the flaw the NousResearch#46332 triage flagged in NousResearch#52204). Signed-off-by: andrexibiza <84248988+andrexibiza@users.noreply.github.com>
Native Win11 verification (monerostar)Host: Windows 11 (build 26200) / MINGW64, Python 3.11.15. Checked out this branch against current upstream Bug class is real on this machine
Live spawn of a
TestsNotes
(Approve blocked for non-maintainer; treating this as a verified +1 with live evidence.) |
|
Thank you for the native Win11 verification — the resolver-vs-which table is exactly the evidence the fix needed. The System32-first PATH case (WSL bash + mangled path → rc=127) is the reported #46332 failure mode, and your live spawn shows |
Real-environment verification (Windows native + git-bash, 2026-08-19)We hit this exact bug class in production: cron jobs Verified
End-to-end: executed a Conclusion: this PR's fix is effective on our real Windows environment (Win11 + Git for Windows, system PATH with System32 first). Repro env: Windows 11, Python 3.11.15, MINGW64 git-bash, |
|
感谢你提供这份真实环境的闭环验证——这正是这个修复最需要的现场证据。 你们复现出的环境分界非常关键:在 Git Bash 终端里手动执行时,终端会注入自己的 PATH,因此脚本正常;而 gateway/cron 进程继承的是纯系统 PATH, 更重要的是,你们在同一份模拟 cron 宿主环境的纯净 PATH 下验证了 这份结果与 @monerostar 之前的 Win11 验证一致,同时补上了真实 Hermes cron 任务、gateway 继承环境以及“手动正常 / cron 失败”这一侧的关键证据。我会把它视为本 PR 在原生 Windows 环境中的一份完整验证记录。感谢你们把触发条件拆得这么清楚并跑完闭环。 |
Related #23405 #23489 #44350 #46332 #46364 #47837 #52204 #60617 #72697 #77393
What does this PR do?
Fixes Layer 1 of issue #46332 (the interpreter-selection half): cron
.sh/.bashjobs on native Windows must resolve bash through the repository's shared Windows-aware resolver, never a rawshutil.which("bash")that lands on the System32 WSL launcher.cron/scheduler.py_run_job_script()still did:On Windows 10/11,
shutil.which("bash")returnsC:\Windows\System32\bash.exe(the WSL launcher — on PATH by default, ahead of Git for Windows). With no distro installed it exits 1; with a distro it cannot run Windows paths at all — either way,.shcron watchdogs fail with exit 127 even though Git Bash is installed.Now it routes through
tools.environments.local._find_bash()— the shared resolver that already encodes the correct precedence (Hermes portable Git → Git for Windows install dirs → PATH lookup with a start probe), merged via #47837:RuntimeError(Windows, no usable bash anywhere — the only "bash" is a WSL stub that cannot start) → the actionable error message, instead of spawning the stub.Layer 2 (Windows bash-path conversion) is already covered by the sibling PR #77393 (
_bash_safe_path, the MSYS convention) — this PR deliberately does not re-touch that line, so the two halves merge without colliding.Addressing existing review commentary
This PR implements exactly the first requirement by reusing the shared resolver instead of re-implementing precedence — sidestepping the PATH-continuation flaw the triage flagged in #52204. The path conversion half ships separately in #77393.
Agreed — this PR does the opposite: it calls the shared resolver directly (full PortableGit/known-install precedence inherited), and the regression tests force the Windows resolution path via monkeypatched
_find_bash/shutil.whichon any host.Prior art and consolidation map
The #46332 umbrella has accumulated 13+ PRs. Current state on this class:
CLEANCredit where due: #46364 (Tranquil-Flow) and #52204 (Frowtek) diagnosed the cron-side launcher selection and proposed Git-Bash-over-WSL resolution; #60617 (paulhopcraft-dot) proposed skipping the launcher stub. This PR builds on their diagnosis with the already-merged shared resolver, which covers their cases and the portable-Git install location theirs missed.
How to Test
uv run --frozen pytest -q tests/cron/test_cron_no_agent.py tests/cron/test_scheduler.pyuv run --frozen ruff check cron/scheduler.py tests/cron/test_cron_no_agent.pyVerified locally on this branch (Windows host, main venv): 71 passed (cron suite incl. 3 new resolver regression tests), ruff clean,
git diff --checkclean. The regression tests force the Windows resolution path via monkeypatches so they exercise the fix on any host; the shared resolver itself is covered by its own test file (tests/tools/test_windows_native_support.py).Checklist
tools/approval.pyhint is covered by fix(cron): pass Git-Bash-safe script path to bash on Windows (.sh exit 127 class) #77393)git diff --checkclean, ruff cleanFixes #46332