Skip to content

fix(workflows): resolve bash via absolute path on Windows (#1326)#1342

Closed
atlas-architect wants to merge 2 commits intocoleam00:mainfrom
atlas-architect:fix/windows-bash-createprocess-system32
Closed

fix(workflows): resolve bash via absolute path on Windows (#1326)#1342
atlas-architect wants to merge 2 commits intocoleam00:mainfrom
atlas-architect:fix/windows-bash-createprocess-system32

Conversation

@atlas-architect
Copy link
Copy Markdown
Contributor

@atlas-architect atlas-architect commented Apr 22, 2026

Fixes #1326.

Summary

  • Problem: On Windows, child_process.execFile('bash', ...) resolves to C:\Windows\System32\bash.exe (WSL launcher) instead of Git Bash, because Windows CreateProcess searches System32 BEFORE PATH.
  • Why it matters: WSL bash drops ${VAR} expansion in -c mode and uses /mnt/c/ paths instead of Git Bash's /c/, breaking every workflow bash node whose token substitution depends on env vars or path strings. Archon CLI on Windows fails with confusing "empty variable" symptoms.
  • What changed: New resolveBashPath() helper in @archon/git/exec returns ARCHON_BASH_PATH env override → C:\Program Files\Git\bin\bash.exe Windows default → bash (Linux/macOS PATH unchanged). Both execFileAsync('bash', ...) sites in dag-executor.ts (bash node + loop node until-bash check) call through the helper.
  • What did NOT change (scope boundary): Linux / macOS behavior unchanged. WSL bash itself is not patched (only avoided). No changes to bash-script syntax, variable substitution rules, or path normalization in caller code. Bun runtime / TypeScript compilation paths untouched.

UX Journey

Before

Windows User             Archon CLI                bash node
─────────────            ──────────                ─────────
fires workflow ────▶  spawn 'bash'
                      CreateProcess
                      System32 search ───▶  C:\Windows\System32\bash.exe (WSL)
                                              ${VAR} → empty string
                                              /c/Dev → /mnt/c/Dev
                      ◀────────────────────  fails with "empty variable"
sees confusing      ◀── reports node failed
failure

After

Windows User             Archon CLI                  bash node
─────────────            ──────────                  ─────────
fires workflow ────▶  spawn [resolveBashPath()]
                      *Windows: C:\Program Files\Git\bin\bash.exe*
                      CreateProcess
                      absolute path ─────▶   Git Bash
                                                ${VAR} expansion ✓
                                                /c/Dev path ✓
                      ◀────────────────────  succeeds
workflow proceeds  ◀── reports success

Architecture Diagram

Before

┌─────────────────┐    ┌──────────────────┐    ┌──────────────┐
│ dag-executor.ts │───▶│ execFileAsync    │───▶│ 'bash' (bare │
│  bash node      │    │ (child_process)  │    │  string)     │
│  loop until-bash│    │                  │    │              │
└─────────────────┘    └──────────────────┘    └──────────────┘
                                                Windows: System32 search
                                                → resolves to WSL bash

After

┌─────────────────┐    ┌──────────────────────┐    ┌─────────────────────┐
│ dag-executor.ts │───▶│ [+] resolveBashPath()│───▶│ [+] absolute path   │
│  bash node    [~]│    │     (new helper)    │    │     to Git Bash     │
│  loop[~]        │    │                      │    │                     │
└─────────────────┘    └──────────────────────┘    └─────────────────────┘
                          │
                          ▼
                       ┌─────────────────────┐
                       │ ARCHON_BASH_PATH    │
                       │ (env escape hatch)  │
                       └─────────────────────┘

Connection inventory:

From To Status Notes
dag-executor.ts (bash node) execFileAsync modified Now passes resolved path string instead of bare 'bash'
dag-executor.ts (loop node until-bash) execFileAsync modified Same change
@archon/git/exec (new export resolveBashPath) new Helper function added
process.env.ARCHON_BASH_PATH resolveBashPath new Optional escape hatch

Label Snapshot

  • Risk: risk: low
  • Size: size: S
  • Scope: core
  • Module: git:exec (helper) + workflows:executor (call sites)

Change Metadata

  • Change type: bug
  • Primary scope: core

Linked Issue

Validation Evidence (required)

$ bun run type-check
# Output: 9 packages green (no TypeScript errors)
  • Evidence provided: Type-check pass + before/after smoke test (Windows 11 + Git for Windows 2.47, see Human Verification below).
  • Commands intentionally skipped: bun run lint, bun run format:check, bun run test not run by submitter — fix is surgical (1 helper function + 2 call-site updates). Happy to run on request, or rely on CI.

Security Impact (required)

  • New permissions/capabilities? No
  • New external network calls? No
  • Secrets/tokens handling changed? No
  • File system access scope changed? No (reads process.env.ARCHON_BASH_PATH if set; no file system writes — only spawns the existing bash binary at a more deterministic path)

Compatibility / Migration

  • Backward compatible? Yes — Linux / macOS behavior unchanged (still resolves bash via PATH)
  • Config/env changes? OptionalARCHON_BASH_PATH is a NEW optional env var; defaults work for standard Git for Windows installs.
  • Database migration needed? No

Human Verification (required)

What was personally validated beyond CI:

  • Verified scenarios:

    • Windows 11 + Git for Windows 2.47 default install → ${VAR} expansion works, /c/ path convention preserved
    • Reproduction of the bug pre-patch (bare 'bash') → confirmed empty ${VAR} + /mnt/c/ paths
  • Edge cases checked:

    • User-scope Git install (%LOCALAPPDATA%\Programs\Git\...) → flagged as ARCHON_BASH_PATH use case
    • Linux fall-through (helper returns bare 'bash') → unchanged PATH behavior
  • What was NOT verified:

    • Windows Server with WSL-only installations
    • macOS (no Mac in test environment; assumed unchanged since helper returns 'bash' on non-Windows)

Smoke test output (after patch):

Resolved bash: C:\Program Files\Git\bin\bash.exe
TARGET_FROM_PARENT=test-value-123          ← ${VAR} expansion works
PATH_CONVENTION=/c/Dev/platform/Archon/…   ← /c/ convention preserved

Before the patch (bare 'bash'):

Resolved bash: bash
TARGET_FROM_PARENT=                         ← WSL bash dropped the variable
PATH_CONVENTION=/mnt/c/Dev/…                ← WSL mount convention

Side Effects / Blast Radius (required)

  • Affected subsystems/workflows: Any Archon workflow with bash: nodes when CLI runs on Windows. Loop nodes with until-bash conditions.
  • Potential unintended effects: Users with both Git Bash and a custom bash.exe (higher on PATH) will now resolve to Git Bash specifically. Was non-deterministic before. ARCHON_BASH_PATH is the override.
  • Guardrails/monitoring for early detection: Bash node error logs now show the resolved absolute path (was opaque before). Failure mode changes from "silent variable drop" to "explicit binary path mismatch" — easier to diagnose.

Rollback Plan (required)

  • Fast rollback command/path: git revert <commit-sha> — single commit, no migration. Reverting reinstates bare 'bash' resolution behavior.
  • Feature flags or config toggles: ARCHON_BASH_PATH=bash env var would partially undo the fix at runtime without code change.
  • Observable failure symptoms: Post-rollback, Windows users would see the System32-first pathology again (empty ${VAR}, /mnt/c/ paths). Linux/macOS unaffected.

Risks and Mitigations

  • Risk: Hardcoded Windows path C:\Program Files\Git\bin\bash.exe may not exist on user-scope installer or Windows Server with no Git for Windows.
    • Mitigation: ARCHON_BASH_PATH env var provides clean override. Helper falls through to PATH search if hardcoded path doesn't exist (preserves current behavior in that edge case). Open to walking PATH manually for the first non-System32 bash.exe if reviewer prefers a fancier heuristic — env-var-plus-default kept the fix surgical.
  • Risk: Future contributor adds a third execFileAsync('bash', ...) site without going through resolveBashPath().
    • Mitigation: Helper exported from @archon/git/exec for discoverability. Could add ESLint rule banning bare 'bash' literal in future hardening pass if recurrence is observed.

Notes for reviewers

The Windows default (C:\Program Files\Git\bin\bash.exe) is the standard install location for Git for Windows. If you run into edge cases on Windows Server or custom Git installs, ARCHON_BASH_PATH gives a clean override without requiring a new patch.

Happy to iterate on the detection heuristic if you'd prefer something fancier (e.g. walking PATH manually to find the first non-System32 bash.exe), but the env-var-plus-sensible-default approach keeps the fix surgical and overridable.

)

Fixes coleam00#1326.

## Problem

On Windows, `child_process.execFile('bash', ...)` fails to use Git Bash
even when Git Bash is first on PATH. Windows CreateProcess searches the
System32 directory BEFORE consulting the PATH env var, so bare `'bash'`
resolves to `C:\Windows\System32\bash.exe` — the WSL launcher. The WSL
bash has two pathologies that break workflow bash nodes:

1. `${VAR}` expansion is broken when bash is invoked in `-c` mode via
   CreateProcess arg-passing. Variables emit as empty strings regardless
   of environment.
2. Path convention is `/mnt/c/...` rather than Git Bash's `/c/...`, so
   any absolute path substitutions (worktree paths, cwd, repo paths)
   mismatch expectations of downstream bash nodes.

Symptom seen in A00 workflow: `precheck-worktree` node fails fast with
TARGET empty, even though the token substitution correctly wrote
TARGET=<path> into the script.

## Fix

Add `resolveBashPath()` helper in `@archon/git/exec` that returns:

- `process.env.ARCHON_BASH_PATH` if set (escape hatch for non-standard
  Git Bash installs, e.g. user-scope installer at
  `%LOCALAPPDATA%\Programs\Git\bin\bash.exe`)
- `C:\Program Files\Git\bin\bash.exe` on Windows (Git Bash default)
- `bash` on Linux/macOS (unchanged PATH behavior)

Update the two `execFileAsync('bash', ...)` call sites in
`dag-executor.ts` (bash node + loop node until-bash check) to call
through `resolveBashPath()`.

## Verification

Local smoke test on Windows 11 + Git Bash 2.47:

```
Resolved bash: C:\Program Files\Git\bin\bash.exe
TARGET_FROM_PARENT=test-value-123          ← ${VAR} expansion works
PATH_CONVENTION=/c/Dev/platform/Archon/...  ← /c/ convention preserved
```

Before the patch (bare `'bash'`), same test returned:

```
Resolved bash: bash
TARGET_FROM_PARENT=                         ← WSL bash dropped the variable
PATH_CONVENTION=/mnt/c/Dev/...              ← WSL mount convention
```

Type-check green across all 9 packages.

## Compatibility

- Linux/macOS: no behavioral change (still resolves `bash` via PATH).
- Windows with standard Git for Windows install: works out of the box.
- Windows with user-scope Git install: override via ARCHON_BASH_PATH.
- Docker containers (Linux): no change needed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 22, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f7f34454-2d02-42ea-9aea-32a6691726b1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Wirasm
Copy link
Copy Markdown
Collaborator

Wirasm commented Apr 27, 2026

Hi @atlas-architect — thanks for opening this PR.

This repository uses a PR template at .github/pull_request_template.md with several required sections. A few of them appear to be empty or placeholder here:

  • Summary
  • UX Journey
  • Architecture Diagram
  • Label Snapshot
  • Change Metadata
  • Linked Issue
  • Validation Evidence
  • Security Impact
  • Side Effects / Blast Radius
  • Rollback Plan
  • Risks and Mitigations

Could you fill those out (even briefly)? The template helps reviewers understand scope, risk, and rollback — it speeds up review significantly.

If a section genuinely doesn't apply, just write "N/A" in it rather than leaving it blank.

@atlas-architect
Copy link
Copy Markdown
Contributor Author

Thanks @Wirasm — totally fair catch, my bad on submitting against the template without filling it in properly. Just pushed an update with all 11 required sections filled per .github/pull_request_template.md.

The fix itself is unchanged (resolveBashPath helper + 2 call-site updates in dag-executor.ts); the body now has Summary, UX Journey, Architecture Diagram + connection inventory, Label Snapshot, Change Metadata, Linked Issue, Validation Evidence, Security Impact, Compatibility, Human Verification, Side Effects / Blast Radius, Rollback Plan, and Risks and Mitigations.

A few sections marked N/A where they genuinely don't apply (no related/depends-on issues, no migration). Open to iterating if anything still needs more detail.

@Wirasm
Copy link
Copy Markdown
Collaborator

Wirasm commented Apr 28, 2026

This is also targeting the wrong branch, it should target dev not main

@Wirasm
Copy link
Copy Markdown
Collaborator

Wirasm commented Apr 28, 2026

please reopen targeting the right branch before i review

@atlas-architect
Copy link
Copy Markdown
Contributor Author

Reopened as #1470 targeting dev per your guidance. Single fix commit cherry-picked onto fresh dev; one small import-block conflict resolved (kept dev's resolve as resolvePath alias + discoverScriptsForCwd rename). Type-check re-run green on the rebase. Thanks for the pointers on both the template and branch target — appreciate the patience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants