Skip to content

test: let the orchestrator status-vocab suite run on native Windows - #130

Merged
amirlehmam merged 1 commit into
amirlehmam:masterfrom
ConnorRobinson13:pr/windows-bash-paths
Jul 31, 2026
Merged

test: let the orchestrator status-vocab suite run on native Windows#130
amirlehmam merged 1 commit into
amirlehmam:masterfrom
ConnorRobinson13:pr/windows-bash-paths

Conversation

@ConnorRobinson13

Copy link
Copy Markdown
Contributor

tests/unit/orchestration-status-vocab.test.ts fails on native Windows — 5 tests, on a clean checkout of master. wmux is a Windows-only app, so the suite currently can't run on its own platform.

Cause

The suite shells out to bash with a path from path.resolve():

const SCRIPTS = path.resolve(__dirname, '../../resources/wmux-orchestrator/scripts');
execFileSync('bash', [HOOK], { env: { ...process.env, TMPDIR: tmp,} });

On Windows that's C:\dev\wmux\resources\…. bash reads \d, \r etc. as escapes, so the path arrives as C:devwmuxresources… and the hook script is never found. find_active_orch() then returns nothing, the hook exits 0 without touching state, and every assertion about status=exited / complete fails.

The execFileSync('node', …) call on the next line is fine — only the bash one breaks.

Fix

There's no single correct conversion: Git Bash mounts the drive at /c, WSL at /mnt/c, Cygwin at /cygdrive/c. Rather than assume which bash is installed, tests/helpers/bash-path.ts generates every spelling and probes with the bash actually on PATH:

bashPathCandidates('C:\\dev\\wmux')
// → ['/c/dev/wmux', '/mnt/c/dev/wmux', '/cygdrive/c/dev/wmux']

Git Bash wins ties deliberately: it runs in the Windows process tree and can execute the node that on-agent-stop.sh shells out to, whereas WSL bash would need a second toolchain installed inside the distro.

A second Windows-only trap turned up while fixing the first. A WSL bash launched from Windows does not inherit the Windows environment unless WSLENV names each variable — so TMPDIR, WMUX_AGENT_ID and CLAUDE_EXIT_CODE would silently vanish even with a correct path. They're now assigned on the bash command line instead of through env:, which works for every flavour.

Scope

  • No-op on Linux and macOS — the path is already POSIX and bashPathCandidates returns it unchanged.
  • No production code touched. Tests and one test helper only.
  • tests/unit/ was swept for the same execFileSync('bash', …) shape; this was the only occurrence (renderer-typecheck spawns npx, shell-context-menu only mentions bash in a comment).
  • A Windows box with no bash at all skips with an explicit describe.skipIf(!hasBash()) rather than passing silently.

Verification

  • Linux/WSL: npm test → 552 passed. The 3 failures (pty-manager, shell-context-menu) are present identically on unmodified master — both need native Windows.
  • 8 unit tests cover the path conversion itself, including the driveless/UNC case and the probe-ordering rule.
  • Native Windows: the 5 previously-failing tests pass.

`tests/unit/orchestration-status-vocab.test.ts` fails on native Windows —
5 tests, on a clean checkout. `path.resolve()` yields `C:\dev\wmux\…`, and
handing that to bash loses the separators (bash reads `\d` as an escape),
so the hook script arrives as `C:devwmux…` and is never found. wmux is a
Windows-only app whose test suite could not run on Windows.

There is no single right conversion: Git Bash mounts the drive at `/c`,
WSL at `/mnt/c`, Cygwin at `/cygdrive/c`. Rather than guess which bash is
installed, `bashPathCandidates()` generates every spelling and
`toBashPath()` probes with the bash actually on PATH. Git Bash wins ties
because it runs in the Windows process tree and can execute the `node`
the orchestrator scripts shell out to, whereas WSL bash would need a
second toolchain inside the distro.

A second Windows-only trap turned up while fixing the first: a WSL bash
launched from Windows does not inherit the Windows environment unless
WSLENV names each variable, so TMPDIR/WMUX_AGENT_ID/CLAUDE_EXIT_CODE are
now assigned on the bash command line instead of through `env:`. That
works for every flavour.

No-op on Linux and macOS, where the path is already POSIX. The suite
skips with an explicit `describe.skipIf(!hasBash())` on a Windows box with
no bash at all, rather than silently passing.

`tests/unit/` was swept for the same `execFileSync('bash', …)` shape; this
was the only occurrence.
@amirlehmam
amirlehmam merged commit 87c4f92 into amirlehmam:master Jul 31, 2026
@amirlehmam

Copy link
Copy Markdown
Owner

Merged in 87c4f92, shipping in v0.40.0. Thank you — this one is worth more than its diff suggests.

The suite it fixes had been failing on native Windows on a clean checkout, which for a Windows-only app means the platform we ship for was the one platform the tests could not run on. I confirmed it locally after merging: 624/624 passing on Windows 11, including the five that used to fail. Not three pre-existing failures, zero.

Two things I want to call out, because they are the parts a lesser fix would have gotten wrong.

You did not pick a mount prefix. Deciding between /c, /mnt/c and /cygdrive/c from the host is a guess about someone else's machine, and bashPathCandidates + probing with the bash actually on PATH replaces the guess with a question. The tie-break reasoning is the right one too — Git Bash wins because it runs in the Windows process tree and can execute the node that on-agent-stop.sh shells out to, which is a property of the task, not a popularity contest.

And you found the second trap while fixing the first. A WSL bash launched from Windows silently drops TMPDIR, WMUX_AGENT_ID and CLAUDE_EXIT_CODE unless WSLENV names each one, so a correct path alone would have moved the failure rather than removed it. Assigning them on the bash command line sidesteps WSLENV entirely and works for every flavour. That is the kind of thing that only turns up when someone actually runs the thing on the platform.

describe.skipIf(!hasBash()) over a silent pass is also right. A suite that reports green because it never ran is worse than one that reports red.

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