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

[atlas-architect]

Note for reviewer: This is the re-opened PR per @Wirasm's request on closed #1342, now targeting dev instead of main. Single fix commit cherry-picked from the original branch onto fresh dev. One small import-block conflict was resolved (kept dev's resolve as resolvePath alias + discoverScriptsForCwd rename, added resolveBashPath to the @archon/git named imports). Function call sites at dag-executor.ts:1329 + :2117 already merged cleanly. bun run type-check re-run green on the dev rebase before pushing.

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.

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

• Closes bug(workflows): bash nodes silently fail on Windows when bash resolves to WSL launcher (System32\bash.exe) — $VAR expansion broken in -c mode #1326
• Related: closed PR fix(workflows): resolve bash via absolute path on Windows (#1326) #1342 (this PR supersedes - re-targeted from main to dev)
• Depends on: N/A
• Supersedes: fix(workflows): resolve bash via absolute path on Windows (#1326) #1342

Validation Evidence (required)

$ bun run type-check
# Output: 10 packages green (no TypeScript errors) - re-verified on dev rebase

• Evidence provided: Type-check pass on dev rebase + 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? Optional - ARCHON_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.

Summary by CodeRabbit

• New Features

  • Platform-aware resolution of the bash executable, including support for an environment-variable override.

• Bug Fixes

  • More robust handling of missing or inaccessible bash: failures now surface immediately instead of silently continuing.
  • Improved subprocess invocation on Windows to avoid incorrect system bash selection.

• Tests

  • Updated tests to validate platform-aware bash invocation and ensure reliable test cleanup.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new exported utility, resolveBashPath(), to determine the correct bash executable path (honoring ARCHON_BASH_PATH and platform defaults). The git package exports it and workflow executor code now uses it when spawning bash processes, with stricter error handling for execution failures.

Changes

Cohort / File(s)	Summary
Git package exports packages/git/src/exec.ts, packages/git/src/index.ts	Add resolveBashPath() that returns a platform-aware bash path, honoring ARCHON_BASH_PATH; exported from package index.
Workflow executor updates packages/workflows/src/dag-executor.ts, packages/workflows/src/dag-executor.test.ts	Replace hardcoded 'bash' with resolved path from git.resolveBashPath() when invoking subprocesses; improve ENOENT/EACCES handling to fail fast for loop nodes; update tests to assert use of resolved path and ensure spy restore in finally.

Sequence Diagram(s)

sequenceDiagram
    participant Executor as Dag Executor
    participant GitPkg as git.resolveBashPath()
    participant Spawn as execFileAsync
    participant OS as System (bash)

    Executor->>GitPkg: request bash path
    GitPkg-->>Executor: return resolved path (ARCHON_BASH_PATH or platform default)
    Executor->>Spawn: execFileAsync(resolvedPath, args, env)
    Spawn->>OS: spawn process at resolvedPath
    OS-->>Spawn: exit code / errors
    Spawn-->>Executor: result / ENOENT|EACCES -> error thrown or logged

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• bug(workflows): bash nodes silently fail on Windows when bash resolves to WSL launcher (System32\bash.exe) — $VAR expansion broken in -c mode #1326: Matches Windows System32/WSL bash resolution bug; resolveBashPath() implements the platform-aware behavior described.

Possibly related PRs

• fix(workflows): export ARTIFACTS_DIR, LOG_DIR, BASE_BRANCH to bash nodes #1387: Touches the same executeBashNode code path (subprocess env/exec construction), likely related and should be reviewed together.

Poem

🐰 I sniffed the PATH and hopped with care,
ARCHON_BASH_PATH led me there.
No System32 tricks, no WSL haze,
A platform-wise bash for all our days. 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main fix: resolving bash via absolute path on Windows to avoid System32 WSL bash resolution issues, which is the core objective of the changeset.
Description check	✅ Passed	The description is comprehensive and follows the template structure with all required sections completed: Summary with Problem/Why it matters/What changed, UX/Architecture diagrams, Label Snapshot, Change Metadata, Linked Issues, Validation Evidence, Security Impact, Compatibility, Human Verification, Side Effects, Rollback Plan, and Risks & Mitigations.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

packages/workflows/src/dag-executor.ts (2)

1323-1327: ⚠️ Potential issue | 🟡 Minor

Update ENOENT message to reflect resolved absolute bash path.

After Line 1323, the ENOENT branch (Line 1376) can be inaccurate because resolution may use an absolute path, not PATH search. This makes troubleshooting harder.

💡 Suggested patch

-    const { stdout, stderr } = await execFileAsync(resolveBashPath(), ['-c', finalScript], {
+    const bashPath = resolveBashPath();
+    const { stdout, stderr } = await execFileAsync(bashPath, ['-c', finalScript], {
       cwd,
       timeout,
       env: subprocessEnv,
     });
@@
-    } else if (err.message?.includes('ENOENT')) {
-      errorMsg = `Bash node '${node.id}' failed: bash executable not found in PATH`;
+    } else if (err.message?.includes('ENOENT')) {
+      errorMsg = `Bash node '${node.id}' failed: bash executable not found at '${bashPath}'. Set ARCHON_BASH_PATH if Git Bash is installed elsewhere.`;

As per coding guidelines, "handle expected failures gracefully ... surface ... errors to users for actionable issues."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/dag-executor.ts` around lines 1323 - 1327, The
catch/ENOENT branch that handles failures from the execFileAsync call using
resolveBashPath() should include the resolved absolute bash path in its error
message to avoid misleading PATH-based suggestions; update the error handling
around the execFileAsync(resolveBashPath(), ['-c', finalScript], {cwd, timeout,
env: subprocessEnv}) invocation to capture the value returned by
resolveBashPath() (store it in a local variable) and, when errno === 'ENOENT',
log/throw an error that mentions that exact resolved path (and the cwd) so users
see the absolute path that was attempted rather than implying a PATH search.

---

2111-2123: ⚠️ Potential issue | 🟠 Major

Don’t swallow until_bash infrastructure errors as “condition not met.”

At Line 2111, system-level execution failures (e.g., ENOENT, EACCES) are treated like a normal false condition. That can hide environment breakage and waste iterations instead of failing immediately.

💡 Suggested patch

-        await execFileAsync(resolveBashPath(), ['-c', substitutedBash], { cwd });
+        const bashPath = resolveBashPath();
+        await execFileAsync(bashPath, ['-c', substitutedBash], { cwd });
         bashComplete = true; // exit 0 = complete
       } catch (e) {
         const bashErr = e as NodeJS.ErrnoException;
-        // ENOENT or other system errors are unexpected — log them
-        if (bashErr.code === 'ENOENT') {
-          getLog().warn(
-            { err: bashErr, nodeId: node.id, iteration: i },
-            'loop_node.until_bash_exec_error'
-          );
-        }
-        bashComplete = false; // non-zero exit = not complete
+        if (bashErr.code === 'ENOENT' || bashErr.code === 'EACCES') {
+          throw new Error(
+            `Loop node '${node.id}' until_bash failed: cannot execute '${bashPath}'. Set ARCHON_BASH_PATH if needed.`
+          );
+        }
+        // Non-zero exit from bash script means completion condition is false.
+        bashComplete = false;
       }

As per coding guidelines, "handle expected failures gracefully ... surface ... errors to users for actionable issues."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/dag-executor.ts` around lines 2111 - 2123, The current
catch in the until_bash execution swallows system-level errors (caught around
execFileAsync/resolveBashPath) and treats them as a simple "condition not met"
by setting bashComplete=false; instead detect infrastructure errors (e.g.,
bashErr.code === 'ENOENT' or 'EACCES'), log them as errors via getLog()
including node.id and iteration i, and rethrow or propagate the error so the DAG
run fails immediately rather than looping; update the catch handling around
execFileAsync/resolveBashPath to only treat non-system (command exit) failures
as condition=false (bashComplete=false) while surfacing system-level errors
upward.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@packages/workflows/src/dag-executor.ts`:
- Around line 1323-1327: The catch/ENOENT branch that handles failures from the
execFileAsync call using resolveBashPath() should include the resolved absolute
bash path in its error message to avoid misleading PATH-based suggestions;
update the error handling around the execFileAsync(resolveBashPath(), ['-c',
finalScript], {cwd, timeout, env: subprocessEnv}) invocation to capture the
value returned by resolveBashPath() (store it in a local variable) and, when
errno === 'ENOENT', log/throw an error that mentions that exact resolved path
(and the cwd) so users see the absolute path that was attempted rather than
implying a PATH search.
- Around line 2111-2123: The current catch in the until_bash execution swallows
system-level errors (caught around execFileAsync/resolveBashPath) and treats
them as a simple "condition not met" by setting bashComplete=false; instead
detect infrastructure errors (e.g., bashErr.code === 'ENOENT' or 'EACCES'), log
them as errors via getLog() including node.id and iteration i, and rethrow or
propagate the error so the DAG run fails immediately rather than looping; update
the catch handling around execFileAsync/resolveBashPath to only treat non-system
(command exit) failures as condition=false (bashComplete=false) while surfacing
system-level errors upward.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dec77c3a-d03c-4163-a0f4-8eec0cc9671f

📥 Commits

Reviewing files that changed from the base of the PR and between 7d06773 and 58921b2.

📒 Files selected for processing (3)

• packages/git/src/exec.ts
• packages/git/src/index.ts
• packages/workflows/src/dag-executor.ts

[atlas-architect]

Pushed 3ea82e9 addressing the CodeRabbit review + Windows CI failure root cause. Three fixes:

1. Test fix (the Windows-CI cascade root cause): the dag-executor.test.ts line 1236 assertion compared the bash command against literal 'bash'. Our patch returns resolveBashPath() which is 'bash' on Linux/macOS but C:\Program Files\Git\bin\bash.exe on Windows. The Windows assertion therefore failed — but execSpy.mockRestore() was on the line after the assertion, so when expect() threw, the spy was never restored and bled into 7 unrelated downstream tests (script nodes > inline bun script output, > non-zero exit code, > timeout kills subprocess, > stderr output, etc. — all hitting the leaked git.execFileAsync spy and producing mysterious 'Use: ok' / 'ok' outputs from the previous test). Wrapped expect + mockRestore in try/finally and updated the expected first arg to git.resolveBashPath() so the assertion is platform-aware.

2. CodeRabbit minor (line 1322-1377): captured bashPath once at the top of executeBashNode() + updated the ENOENT error message to include the resolved path + suggest ARCHON_BASH_PATH for non-standard Git installs (e.g. user-scope installer at %LOCALAPPDATA%\Programs\Git\bin\bash.exe). The previous "not found in PATH" message was misleading after the resolution change.

3. CodeRabbit major (line 2112-2128): loop node until_bash catch was treating ENOENT/EACCES as "condition not met" — silently looping against a broken bash binary. Now distinguishes system-level errors (ENOENT/EACCES → throw immediately, surface env breakage) from non-zero script exit (= condition false, keep looping).

Local Windows verification: bun test src/dag-executor.test.ts = 199 pass, 0 fail. Type-check green across all 10 packages.

Thanks @coderabbitai for the review — the ENOENT message + loop-error-handling catches were both real bugs.

[coderabbitai]

@atlas-architect: Excellent follow-up — the root-cause analysis on the Windows CI cascade is particularly valuable. The leaked spy producing 'Use: ok' / 'ok' outputs in unrelated script-node tests is a classic hard-to-diagnose failure mode, and the try/finally fix is the correct pattern. All three items look solid:

1. Test robustness — try/finally around expect + mockRestore ensures the spy is always cleaned up regardless of assertion outcome, and git.resolveBashPath() makes the assertion platform-aware.
2. ENOENT message — Including the resolved path and the ARCHON_BASH_PATH hint turns a confusing error into an actionable one, especially for user-scope Git installs at %LOCALAPPDATA%\Programs\Git\bin\bash.exe.
3. Loop-node error handling — Distinguishing system-level failures (ENOENT/EACCES → throw) from logical failures (non-zero exit → keep looping) is the right semantic split; silently looping on a missing binary would have been very difficult to debug in production.

Triggering a re-review of the updated commit.

---

✏️ Learnings added

Learnt from: atlas-architect
URL: https://github.com/coleam00/Archon/pull/1470

Timestamp: 2026-04-28T21:00:22.937Z
Learning: In `packages/workflows/src/dag-executor.test.ts` (Bun test suite): always wrap `expect(...)` assertions and `spy.mockRestore()` in a `try/finally` block so the spy is restored even when the assertion throws. A leaked `execFileAsync` spy bleeds into downstream unrelated tests and produces misleading outputs (e.g. `'Use: ok'`). Pattern: `try { expect(spy).toHaveBeenCalledWith(...) } finally { spy.mockRestore() }`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

🧠 Learnings used

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-28T11:00:25.790Z
Learning: Applies to **/*.{ts,tsx} : Use `execFileAsync` (not `exec`) when calling git or external commands directly from TypeScript; handle expected failures gracefully (missing directories during cleanup); surface git errors to users for actionable issues (conflicts, uncommitted changes)

---

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-28T11:00:25.790Z
Learning: Applies to **/*.{ts,tsx} : Use `archon/git` functions for git operations; use `execFileAsync` (not `exec`) when calling git directly; never run `git clean -fd` (use `git checkout .` instead)

---

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-28T11:00:25.790Z
Learning: Applies to packages/git/src/**/*.ts : archon/git: git operations (worktrees, branches, repos, exec wrappers); depends only on archon/paths

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/workflows/src/dag-executor.ts (1)

2122-2125: Align the new log event key with the repo’s structured naming convention.

loop_node.until_bash_exec_error does not follow {domain}.{action}_{state} with standard states (e.g., _failed). Consider renaming to something like loop.until_bash_failed.

As per coding guidelines: "Structured logging with Pino: use event naming format {domain}.{action}_{state} (standard states: _started, _completed, _failed, _validated, _rejected)".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/dag-executor.ts` around lines 2122 - 2125, The log
event key 'loop_node.until_bash_exec_error' doesn't follow the repo's structured
naming ({domain}.{action}_{state}); update the call to getLog().error(...) to
use a compliant event key such as 'loop.until_bash_failed' (preserving the same
metadata: err: bashErr, nodeId: node.id, iteration: i) so callers still receive
bashErr, node.id and i but the event name matches the {domain}.{action}_{state}
convention.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/workflows/src/dag-executor.ts`:
- Around line 1376-1378: The ENOENT branch currently emits a Windows-specific
hint and recomputes the bash path; update it to reuse the already-resolved
loopBashPath variable (instead of recomputing bashPath) and emit a
platform-aware message: on Windows mention setting ARCHON_BASH_PATH and an
example Git Bash path, on POSIX mention installing bash or ensuring it’s on
PATH; include the node id (node.id) and the loopBashPath value in the errorMsg
for clear diagnostics.

---

Nitpick comments:
In `@packages/workflows/src/dag-executor.ts`:
- Around line 2122-2125: The log event key 'loop_node.until_bash_exec_error'
doesn't follow the repo's structured naming ({domain}.{action}_{state}); update
the call to getLog().error(...) to use a compliant event key such as
'loop.until_bash_failed' (preserving the same metadata: err: bashErr, nodeId:
node.id, iteration: i) so callers still receive bashErr, node.id and i but the
event name matches the {domain}.{action}_{state} convention.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8e9c5e69-7996-4e80-9a85-e2afdaf88992

📥 Commits

Reviewing files that changed from the base of the PR and between 58921b2 and 3ea82e9.

📒 Files selected for processing (2)

• packages/workflows/src/dag-executor.test.ts
• packages/workflows/src/dag-executor.ts

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Use platform-aware ENOENT hints and reuse the already-resolved bash path.

Current ENOENT text is Windows-specific on all platforms, and Line 2127 recomputes the path instead of using loopBashPath (can desync diagnostics if env changes).

🔧 Suggested patch

+function getBashResolutionHint(): string {
+  return process.platform === 'win32'
+    ? 'Set ARCHON_BASH_PATH if Git Bash is installed elsewhere (e.g. user-scope installer at %LOCALAPPDATA%\\Programs\\Git\\bin\\bash.exe).'
+    : "Ensure 'bash' is installed or set ARCHON_BASH_PATH to a valid bash executable.";
+}
+
 ...
-    } else if (err.message?.includes('ENOENT')) {
-      errorMsg = `Bash node '${node.id}' failed: bash executable not found at '${bashPath}'. Set ARCHON_BASH_PATH if Git Bash is installed elsewhere (e.g. user-scope installer at %LOCALAPPDATA%\\Programs\\Git\\bin\\bash.exe).`;
+    } else if (err.message?.includes('ENOENT')) {
+      errorMsg = `Bash node '${node.id}' failed: bash executable not found at '${bashPath}'. ${getBashResolutionHint()}`;
 ...
-          throw new Error(
-            `Loop node '${node.id}' until_bash failed: cannot execute bash at '${resolveBashPath()}' (${bashErr.code}). Set ARCHON_BASH_PATH if Git Bash is installed elsewhere.`
-          );
+          throw new Error(
+            `Loop node '${node.id}' until_bash failed: cannot execute bash at '${loopBashPath}' (${bashErr.code}). ${getBashResolutionHint()}`
+          );

Also applies to: 2126-2128

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/workflows/src/dag-executor.ts` around lines 1376 - 1378, The ENOENT
branch currently emits a Windows-specific hint and recomputes the bash path;
update it to reuse the already-resolved loopBashPath variable (instead of
recomputing bashPath) and emit a platform-aware message: on Windows mention
setting ARCHON_BASH_PATH and an example Git Bash path, on POSIX mention
installing bash or ensuring it’s on PATH; include the node id (node.id) and the
loopBashPath value in the errorMsg for clear diagnostics.

[Wirasm]

Review Summary

Verdict: minor-fixes-needed

Good fix — the Windows bash path resolution is solid and the WSL mis-resolution is a real pain point. Three correctness issues in error-handling paths and one missing docs entry need to be addressed before merge. None are large, but the err.message leak and missing docs entry are user-facing.

Blocking issues

• packages/docs-web/src/content/docs/reference/configuration.md: ARCHON_BASH_PATH is used in error messages but undocumented. Users will see Set ARCHON_BASH_PATH if Git Bash is installed elsewhere with no docs to reference. Add it to the configuration.md env vars table:

  | `ARCHON_BASH_PATH` | Override the bash executable path used by bash nodes and loop `until_bash`. Default: `'bash'` on Linux/macOS; `'C:\Program Files\Git\bin\bash.exe'` on Windows | -- |

Suggested fixes

• packages/workflows/src/dag-executor.ts:1375: The else branch still leaks err.message to the user — this is a pre-existing bug this PR propagates further. Replace it with a clean message:

  errorMsg = `Bash node '${node.id}' failed with exit code ${err.code ?? 'unknown'}`;

• packages/workflows/src/dag-executor.ts:2126: loopBashPath is declared but the error calls resolveBashPath() again. Use the already-resolved variable:

  throw new Error(
    `Loop node '${node.id}' until_bash failed: cannot execute bash at '${loopBashPath}' (${bashErr.code}). Set ARCHON_BASH_PATH if Git Bash is installed elsewhere.`
  );

• packages/workflows/src/dag-executor.ts:1367: Change err.message?.includes('ENOENT') to err.code === 'ENOENT' — consistent with the EACCES check below and the loop node pattern.

• packages/workflows/src/dag-executor.ts:1327: Add ENOTDIR to the error condition — if ARCHON_BASH_PATH points to a directory, execFileAsync throws ENOTDIR, not ENOENT:

  } else if (err.message?.includes('ENOENT') || err.code === 'ENOTDIR') {

Minor / nice-to-have

• Remove the coleam00/Archon#1326 issue references from exec.ts:27 and dag-executor.test.ts:1237 — they add noise and become meaningless over time. The docstring is justified; the test comment is not.
• The loop node until_bash error branches (EACCES throw, ENOENT behavior change from silent → throw) have no test coverage. Add two cases in the existing test suite.
• Condense the resolveBashPath docstring from 4 detail layers to 2 — keep the non-obvious WHY (WSL launcher issues), drop the extra context.
• Add a one-line fast-fail note to guides/loop-nodes.md under "Troubleshooting": "If bash is not found, the loop fails immediately. Set ARCHON_BASH_PATH to resolve this."
• Add a CHANGELOG entry under [Unreleased] for this fix — it's a real user-visible improvement worth tracking.

Compliments

• The ENOENT error message in executeBashNode now includes the resolved path and the ARCHON_BASH_PATH fix — much more actionable than the generic "not found in PATH" it replaces.
• The test's try/finally guard against spy leakage is good discipline that most PRs skip.

---

Reviewed via maintainer-review-pr workflow (Pi/Minimax). Aspects run: code-review, error-handling, test-coverage, comment-quality, docs-impact.