fix(pr): is_command_available walks PATH instead of probing --version
CI surfaced the failure: `Test (ubuntu-latest)` panicked in
`is_command_available_detects_present_and_absent_binaries` with
"POSIX `sh` should be on PATH". Root cause: Ubuntu's `/bin/sh` is
`dash`, and `dash --version` exits with status 2 ("invalid option")
because dash doesn't recognize the flag. The previous helper invoked
`Command::new(name).arg("--version").output()` and treated a non-zero
exit as "missing", which incorrectly classified every `dash`-style
shell as absent. macOS happens to use bash as `sh`, which honors
`--version`, so the bug was invisible locally.
Fix: skip the probe entirely. Walk `$PATH` for an executable file
with the given name. Windows additionally probes `name + .exe` when
`name` has no extension so `gh` resolves as `gh.exe` the same way the
shell would. No behavior change on the happy path; the only change
is that present-but-`--version`-rejecting binaries (dash, busybox,
some embedded shells) are now correctly classified as available.
Restores PR #519 CI to green so the v0.8.8 release can land.
This commit is contained in:
+32
-2
@@ -2513,9 +2513,39 @@ async fn run_pr(
|
||||
run_interactive(cli, config, resume_session_id, Some(prompt)).await
|
||||
}
|
||||
|
||||
/// Return true if `name` resolves to an executable on the current `PATH`.
|
||||
///
|
||||
/// Walks `$PATH` directly instead of probing with `--version`. The
|
||||
/// previous implementation invoked `Command::new(name).arg("--version")`,
|
||||
/// which fails on the Ubuntu CI runner because `/bin/sh` is `dash` —
|
||||
/// `dash --version` exits with status 2 ("invalid option") even though
|
||||
/// `sh` is plainly on PATH. macOS happens to ship bash as `sh`, which
|
||||
/// does honor `--version`, so the bug was invisible locally and only
|
||||
/// surfaced in CI logs.
|
||||
///
|
||||
/// Windows: also checks the `.exe` extension when `name` doesn't have
|
||||
/// one, matching the platform's PATHEXT lookup behavior for the common
|
||||
/// case.
|
||||
fn is_command_available(name: &str) -> bool {
|
||||
let probe = Command::new(name).arg("--version").output();
|
||||
probe.is_ok_and(|out| out.status.success())
|
||||
let Some(path) = std::env::var_os("PATH") else {
|
||||
return false;
|
||||
};
|
||||
for dir in std::env::split_paths(&path) {
|
||||
let candidate = dir.join(name);
|
||||
if candidate.is_file() {
|
||||
return true;
|
||||
}
|
||||
#[cfg(windows)]
|
||||
{
|
||||
// PATHEXT gives `.exe`/`.cmd`/`.bat` etc. priority — we only
|
||||
// probe `.exe` because that's the case that actually trips
|
||||
// up the negative case (`gh` resolves as `gh.exe`).
|
||||
if candidate.extension().is_none() && candidate.with_extension("exe").is_file() {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Default)]
|
||||
|
||||
Reference in New Issue
Block a user