fix(cron): resolve .sh script bash via Git for Windows probe before WSL stub

[LiteSoul]

Problem

The cron scheduler's _run_job_script picks the bash interpreter for .sh/.bash scripts with:

_bash = shutil.which("bash") or ("/bin/bash" if os.path.isfile("/bin/bash") else None)

On a common Windows configuration this resolves to the WSL launcher stub and every .sh cron script silently fails:

1. Git for Windows installed with its default PATH option — the installer's recommended default adds Git\cmd (ships git.exe) but NOT Git\bin / Git\usr\bin (which ship bash.exe). The third option that adds usr\bin is explicitly flagged in the installer as risky because it overrides Windows builtins like find.exe / sort.exe, so most users (and most CI base images) leave it on the safer default.
2. WSL enabled with no distributions installed — Windows ships C:\Windows\System32\bash.exe (the WSL launcher stub) whenever the WSL optional feature is on, prevalent across modern Windows 10/11 SKUs.

In that configuration — not a one-in-a-thousand setup — shutil.which("bash") resolves to C:\Windows\System32\bash.exe (the WSL stub), and every .sh cron script fails:

WSL_E_DEFAULT_DISTRO_NOT_FOUND
Windows Subsystem for Linux has no installed distributions.

The if _bash is None fall-back error message ("bash not found, install Git for Windows") never fires because the WSL stub does satisfy which("bash"). The "bash found, but it's the wrong bash" case is unhandled.

The failure is invisible to the interactive terminal tool (which goes through the git-bash terminal backend whose PATH prepends Git\usr\bin), so the bug only surfaces on scheduled ticks — making it a particularly confusing trap to diagnose. (For full disclosure: I hit this on my own setup with a .sh-wrapped Brainiac data-collection cron job, and spent four rounds of misdiagnosis before reading the source. The repro below is the actual configuration. The bug is real, just narrow.)

Repro

On Windows 11 with Git for Windows installed using the default PATH option and WSL enabled with no distributions:

import shutil
print(shutil.which("bash"))
# -> C:\Windows\System32\bash.EXE  (the WSL launcher stub, not Git Bash)

from cron.scheduler import _run_job_script
# brainiac_collect.sh contains: #!/usr/bin/env bash / echo hello
success, output = _run_job_script("brainiac_collect.sh")
# success=False
# output contains: "WSL_E_DEFAULT_DISTRO_NOT_FOUND" / "Windows Subsystem for Linux has no installed distributions."

Fix

Extract two helpers next to _run_job_script:

• _is_wsl_launcher(path) — best-effort classifier: any bash resolved under system32/ or WindowsApps/ is the WSL launcher stub, not real Git Bash.
• _resolve_bash() — pick a bash interpreter with this precedence:
  1. Windows: probe well-known Git for Windows install paths first — ProgramFiles\Git\bin\bash.exe, ProgramFiles\Git\usr\bin\bash.exe (plus (x86) variants). Beats which("bash") so the WSL stub under system32 never wins when WSL has no distros.
  2. shutil.which("bash") — the historical resolution, correct on Linux/macOS and on Windows hosts that did add Git\usr\bin to PATH — unless it resolves to the WSL stub (then skip and keep looking).
  3. /bin/bash — the POSIX fallback when nothing is on PATH.

The 'bash not found, install Git for Windows' error message is preserved verbatim for the genuine no-bash case. The else branch (sys.executable for .py scripts) is untouched. Total diff: +76 lines in scheduler.py.

Tests

New TestResolveBashWindows class in tests/cron/test_cron_script.py (this was the first test class exercising the .sh interpreter branch — the existing suite only covered the .py path, which is part of why this bug went unnoticed):

• _is_wsl_launcher: system32 / WindowsApps → true; Git\bin / Git\usr\bin / /bin/bash / empty → false.
• _resolve_bash precedence: probe-beats-WSL-stub (simulated), no-Git-returns-skip-stub (never returns the stub path), which-when-not-stub (uses a non-stub which hit), posix-prefers-which.
• End-to-end: a .sh cron script runs via the resolved interpreter (probe-beats-stub proves itself with no WSL errors in output).

Cross-platform-runnable via monkeypatched sys.platform + platform.system + platform.release (per the existing tests/agent/test_prompt_builder.py::TestEnvironmentHints pattern called out in AGENTS.md), so Linux CI exercises the Windows branch and a Windows runner exercises the real-host probe. @pytest.mark.skipif guards split the host-conditional ones inversely.

Verification:

• New TestResolveBashWindows: 7 pass, 3 skip (the inversely-guarded host-conditional ones) on this Windows host.
• Full tests/cron/test_cron_script.py: 44 pass, 4 skip, 0 failures — no regressions to existing tests.

Scope

This is a narrow, surgical fix to one function (_run_job_script) and adds 220 lines of dedicated test coverage for code paths that previously had none. It does not touch:

• the agent loop, prompt caching, or system prompt;
• the else → sys.executable branch (.py scripts unaffected);
• any tool schemas or core tools (cron is a feature, not core).

It clears the "real bugs, well… fix the whole bug class, sibling call paths included" bar from CONTRIBUTING/AGENTS.md: every .sh cron job on an affected Windows host is fixed, not just the one this PR was motivated by.

Out of scope (worth flagging)

The deeper class — "Windows tools that silently resolve to wrong executables" — has analogs in other parts of the codebase that probe shutil.which for shell-ish tools. I'm deliberately not touching those in this PR; that's a separate audit and would explode the diff.

[teknium1]

Thanks for identifying a real native-Windows cron path: current main still executes the raw shutil.which("bash") result in cron/scheduler.py:2083.

Problems

• The new resolver is a second, incomplete Windows discovery chain. tools/environments/local.py:532-565 already supports HERMES_GIT_BASH_PATH, Hermes-managed PortableGit, and a LocalAppData Git install; the PR's resolver omits them, so cron can still miss the interpreter used by the native terminal path.
• The unguarded Windows-path assertions in tests/cron/test_cron_script.py:450 cannot pass on POSIX: the proposed classifier splits using host os.sep, which is / on Linux and does not split C:\\Windows\\System32\\bash.exe.
• tests/cron/test_cron_script.py:649 creates directories with tempfile.mkdtemp; assigning monkeypatch._tmp_path does not register cleanup.

Suggested changes

• Share/extend the established resolver instead of duplicating a subset, and add WSL-stub rejection there.
• Normalize both path separator styles in the classifier and use tmp_path for test directories.

Automated hermes-sweeper review.

[teknium1]

Please share or extend the native resolver in tools/environments/local.py:521-574 rather than introduce this subset. That resolver honors HERMES_GIT_BASH_PATH, installer-managed PortableGit, and LocalAppData Git locations; this one skips all of them, so cron can still diverge from Hermes's supported Windows interpreter discovery.

[LiteSoul]

Resolved in ece1a9578c: the parallel _resolve_bash / _is_wsl_launcher / _GIT_WIN_BASH_PATHS block is deleted from cron/scheduler.py; _run_job_script now lazy-imports and calls tools.environments.local._find_bash() directly. Cron now uses the exact same chain as the native terminal path — HERMES_GIT_BASH_PATH, Hermes-managed PortableGit, LocalAppData Git, ProgramFiles\Git, the usr/bin MinGit fallback, and the new WSL-stub rejection — so it can no longer diverge from Hermes's supported Windows interpreter discovery.

The WSL-stub rejection was added to _find_bash itself (lines 521+ and the final shutil.which("bash") fallback) rather than left in a cron-local helper, so every existing caller of _find_bash (native terminal spawn_local, _find_shell delegation) gets the fix too. TestSharedFindBashUsedEverywhere::test_cron_scheduler_delegates_to_find_bash is a new source guard pinning this invariant.

[teknium1]

This unguarded assertion fails on POSIX CI: the proposed classifier splits with host os.sep (/ on Linux), which does not split this backslash-separated Windows path. Normalize both separator styles, or otherwise make the classifier host-independent before keeping these cross-platform tests.

[LiteSoul]

Resolved in ece1a9578c: the classifier now uses pathlib.PureWindowsPath instead of os.sep splitting, so it splits Windows-style paths (C:\Windows\System32\bash.exe) correctly on Linux/macOS CI runners — not just on a real Windows host. test_forward_slash_separated_windows_path_still_classified in TestIsWSLLauncher pins this explicitly (a forward-slash-separated C:/Windows/System32/bash.exe is still recognized).

The os.sep-based classifier and all the host-dependent assertions that failed outside Windows are gone; TestIsWSLLauncher runs five cases (system32, WindowsApps alias, Git Bash locations NOT flagged, POSIX paths + empty NOT flagged, forward-slash Windows path) and all pass on this Windows host without any skip guard.

[teknium1]

tempfile.mkdtemp() is not cleaned up by assigning a private attribute to monkeypatch. Accept pytest's tmp_path fixture in the tests/helper instead so these directories are managed and removed by pytest.

[LiteSoul]

Resolved in ece1a9578c: the tempfile.mkdtemp() + monkeypatch._tmp_path helper is deleted. All relocated tests in tests/tools/test_find_shell.py accept pytest's tmp_path fixture, so the temp directories are managed and removed by pytest. The monkeypatch_tmp_root helper and the whole TestResolveBashWindows block were removed from tests/cron/test_cron_script.py (220 lines) and the tests rewritten under TestFindBashSkipsWSLStub / TestIsWSLLauncher in test_find_shell.py, the natural home alongside the existing _find_bash tests.

[LiteSoul]

@teknium1 — addressed all three points in ece1a9578c. The commit supersedes the v1 cron-only _resolve_bash per your A2 suggestion ("share/extend the established resolver instead of duplicating a subset, and add WSL-stub rejection there").

Per-thread replies map each of the three line comments to the fix. Summary of what changed:

1. Share/extend, don't duplicate. _resolve_bash / _is_wsl_launcher / _GIT_WIN_BASH_PATHS deleted from cron/scheduler.py; _run_job_script lazy-imports tools.environments.local._find_bash and calls it directly. Cron now gets HERMES_GIT_BASH_PATH, PortableGit, LocalAppData Git, ProgramFiles\Git, and the new WSL-stub rejection for free — identical chain to the native terminal path. See thread on cron/scheduler.py:2064.

2. Host-independent classifier. _is_wsl_launcher uses PureWindowsPath (not os.sep), so it correctly classifies C:\Windows\System32\bash.exe on the Linux/macOS CI runners that audit Windows behaviour. TestIsWSLLauncher::test_forward_slash_separated_windows_path_still_classified pins this. See thread on tests/cron/test_cron_script.py:450.

3. tmp_path fixture. The mkdtemp + private monkeypatch attr helper is gone; all relocated tests use tmp_path (pytest-managed cleanup). See thread on tests/cron/test_cron_script.py:649.

Bonus — sibling bug fixed. While auditing the shutil.which("bash") or (...) anti-pattern across the codebase I found a verbatim copy in gateway/platforms/webhook_filters.py:run_route_script (.sh webhook route scripts). It would have hit the same WSL-stub silent-failure mode. It now delegates to _find_bash() too — TestSharedFindBashUsedEverywhere::test_webhook_filters_delegates_to_find_bash is a source guard pinning this.

Stale source guard updated. tests/tools/test_windows_native_support.py::TestCronSchedulerBashResolution::test_source_uses_shutil_which_for_bash (which asserted shutil.which("bash") literally appears in cron source — written to kill a previous hardcoded-path bug) is renamed to test_source_delegates_to_shared_find_bash and now asserts delegation + the absence of the inline shutil.which("bash") or ("/bin/bash" ...) fallback.

Tests verified locally on this Windows host: 63 passed / 2 skipped / 2 pre-existing failures across the three test files after the v1 patch (retested post-rebase — see follow-up comment). The 2 failures (TestFindShellPrefersUserShell::test_returns_shell_env_when_set_and_exists, test_honours_allowlisted_bash_and_dash) are pre-existing POSIX-only-in-spirit tests lacking a sys.platform == "win32" skip guard — I confirmed they fail identically on the clean PR HEAD before any of my A2 edits. Out of scope for this PR; happy to add the skip guard in a follow-up if you want.

Out-of-scope findings (not touched, FYI): TestConfigureWindowsStdio::test_no_op_on_posix and TestSigkillFallback::test_getattr_fallback_prefers_sigkill_when_present are also pre-existing POSIX-only failures on Windows for the same reason (no skip guard). Same offer — I can address in a separate small PR if useful.

[LiteSoul]

Update — rebased onto upstream main (live SHA is now ece1a9578c; supersedes eda7938972).

Triggered by the "this branch has conflicts that must be resolved" banner. The actual conflict was one hunk in tools/environments/local.py — upstream's #63955 modified _find_bash itself (added the _bash_starts() behavioural probe + HERMES_GIT_BASH_PATH-fails-probe fallthrough for the broken-system-Git case), which overlaps with my WSL-stub classifier changes to the same function.

I chose git rebase rather than the GitHub "Resolve conflicts" button (which would have produced a merge commit and avoided the force-push) because I wanted linear history. That was a stylistic choice on my end, not a necessity — I owe a small apology for the resulting SHA churn; the earlier thread replies and summary referenced eda7938972 which no longer exists after the force-push. The live head is ece1a9578c. A merge-based resolution would have left the history alone.

What the integration looks like in local.py::_find_bash:

• Upstream's _bash_starts() behavioural probe is kept as the selection mechanism (first candidate that actually starts wins — catches a broken HERMES_GIT_BASH_PATH that my path classifier can't see).
• My _is_wsl_launcher() path classifier is kept as a candidate-collection filter (skip the WSL stub before appending to candidates, so we never even spawn a ~15ms subprocess probe per candidate — and we keep the "skipped WSL stub" signal obvious in debug logs, and we stay correct if WSL later gains a distro and the stub DOES start).
• The two are complementary; they catch different failure modes. The commit message spells out the three reasons the path filter still pulls its weight alongside the probe.

Tests updated for the new design:

• TestFindBashSkipsWSLStub::_patch_windows now monkeypatches _bash_starts (mirrors upstream's TestFindBashSkipsBrokenCustomPath pattern), so the tests are hermetic and run identically on Linux/macOS CI — they no longer accidentally rely on the host having a real C:\Program Files\Git install.
• test_hermes_git_bash_path_overrides_everything corrected: post-fix(windows): survive broken Git Bash login shells #63955, a broken HERMES_GIT_BASH_PATH no longer unconditionally wins (it's probed and skipped). The test now reflects the real design — a healthy override wins outright; a broken one is covered by upstream's TestFindBashSkipsBrokenCustomPath.

Verification (post-rebase): 53 passed / 1 skipped / 0 failed locally across the bash-resolution + cron suites (15 of my new tests + upstream's TestFindBashSkipsBrokenCustomPath + the cron + windows_native_support suites). The 2 pre-existing TestFindShellPrefersUserShell failures (POSIX-only tests, no Windows skip guard) remain out of scope and untouched, as before.