Fix Windows PATH resolution to respect PATH order instead of System32…

[nooscraft]

Fix Windows PATH resolution to respect PATH order instead of System32 priority

This commit fixes a critical bug on Windows where just would prioritize
C:\Windows\System32 over entries in the PATH environment variable when
resolving executables. This caused issues when users wanted to use executables
like Git Bash's bash.exe but had WSL2 installed, as Windows' CreateProcess
would find System32\bash.exe first and launch WSL2 instead of the user's
preferred bash.

The fix implements proper PATH-based executable resolution that:

• Respects PATH order (first match wins, not System32 priority)
• Checks PATHEXT extensions on Windows (.exe, .bat, .cmd, etc.)
• Works consistently across shell, script-interpreter, and shebang resolution
• Provides clear error messages when executables are not found

Changes:

Core Implementation:

• Add resolve_executable() function in src/which.rs that:
  • Searches PATH entries in order
  • On Windows, checks PATHEXT extensions for each PATH entry
  • Handles absolute paths, relative paths, and simple command names
  • Returns proper error messages when executables aren't found
• Add ExecutableNotFound error variant with helpful error messages

Platform-Specific Updates:

• Update src/platform/windows.rs:
  • Use resolve_executable() for shebang interpreters without forward slashes
  • Maintains existing cygpath behavior for Unix-style paths (with '/')
• Update src/executor.rs:
  • Use resolve_executable() for command interpreters
  • Ensures script interpreters respect PATH order
• Update src/settings.rs:
  • Use resolve_executable() for shell command resolution
  • Pass working directory for proper relative path resolution
• Update src/evaluator.rs:
  • Pass working directory to shell_command() for proper resolution
• Update src/justfile.rs:
  • Pass working directory to shell_command() for proper resolution

Testing:

• Add comprehensive Windows-specific tests in tests/windows_shell.rs:
  • Test PATH order respect (not System32 priority)
  • Test PATHEXT extension checking
  • Test script-interpreter PATH resolution
  • Test shebang interpreter PATH resolution
• Add cross-platform tests in tests/shell.rs:
  • Test PATH order respect
  • Test executable not found error messages
  • Test absolute path handling

Addressing Q&A from issue #2947:

Q: Why does std::process::Command prioritize System32?
A: Rust's std::process::Command doesn't resolve executables itself when no
explicit PATH is set. It delegates to Windows' CreateProcess API, which
checks multiple locations before PATH: current directory, System32, etc.
This is documented Windows behavior but doesn't match user expectations
or how other shells (bash, PowerShell, cmd.exe) behave.

Q: Isn't CreateProcessA's resolution the correct behavior?
A: No. While it's built-in Windows behavior, it doesn't match user
expectations. Other shells intentionally resolve executables according to
PATH order, giving users control over which executables are used. just
should behave consistently with these shells on both Windows and Unix.

Q: What's the problem with using WSL2 bash.exe as shell?
A: WSL2 bash runs in a Linux virtual machine with different filesystem
semantics. Paths like C:\ are mounted differently, which can break
integration tests and scripts expecting Windows path behavior. Users
should be able to choose Git Bash or other Windows-native shells via PATH.

Related Issues:

• Fixes just prioritizes System32 over $Path; is impossible to put bash on $Path if you have WSL2 installed #2947 (Windows PATH resolution)
• Fixes which() does not use PATHEXT on Windows #2926 (PATHEXT extensions)
• Related to Bug: just does not use PATHEXT environment variable when looking for an executable file on Windows, leads to program not found errors #2826 (Windows shell issues)
• Related to Process spawning via PATH is different on unix than it is on windows rust-lang/rust#15149 and Command::spawn has weird rules for finding binaries on Windows rust-lang/rust#37519

[cspotcode]

@nooscraft Looks AI-generated, yeah?

[nooscraft]

@nooscraft Looks AI-generated, yeah?

WDYM? The commit message? Yes, it is, but the changes, nah, not so AI.

[cspotcode]

Ok yeah, I read the commit message and ticket description, and they are waaay too verbose, outright copying and rephrasing sections from #2947. The implementation also has some AI hallmarks. I'm wary of reviewing in-detail, since babysitting someone else's AI output isn't a great use if my time.

Nothing wrong with using AI, I use it too, but I wonder if you should do another human review of its output first.

[nooscraft]

Ok yeah, I read the commit message and ticket description, and they are waaay too verbose, outright copying and rephrasing sections from #2947. The implementation also has some AI hallmarks. I'm wary of reviewing in-detail, since babysitting someone else's AI output isn't a great use if my time.

Nothing wrong with using AI, I use it too, but I wonder if you should do another human review of its output first.

I don’t want to argue. The changes were properly reviewed before submitting the PR. The added tests support the modifications. The commit message was intentionally verbose and partially AI-assisted to highlight the key aspects. I could’ve easily rewritten it to sound more developer-like or less AI-generated, but I chose not to!