fix: run doctor version checks under fish

The doctor "can find npm/pi" checks wrap the version command in a POSIX
subshell, `(cmd --version 2>&1 || true)`. Fish treats `( ... )` as command
substitution syntax and forbids it in command position, so every fish user saw
false-negative failures:

  fish: command substitutions not allowed in command position
  command -v npm && (npm --version 2>&1 || true)

Branch on the detected service shell and emit fish's `begin; ...; end`
grouping for fish, mirroring the existing fish-aware quoting in
serviceShellQuote. Bash and zsh keep the POSIX subshell.

Also guard the `main()` invocation with an ESM main-module check so the CLI
helpers can be imported by tests without side effects, and add a regression
test covering the bash, zsh, and fish command shapes.
This commit is contained in:
Gilbert
2026-06-25 10:54:49 +08:00
parent deb4f9feca
commit 47c9b66819
3 changed files with 52 additions and 6 deletions
@@ -0,0 +1,9 @@
---
"@jmfederico/pi-web": patch
---
Fix `pi-web doctor` "can find npm/pi" checks on fish. The `--version` check
wrapped the version command in a POSIX subshell `(cmd --version 2>&1 || true)`,
which fish parses as a command substitution in command position and rejects
(`command substitutions not allowed in command position`), producing a false
negative. Emit fish's `begin; ...; end` grouping when the service shell is fish.
+31
View File
@@ -0,0 +1,31 @@
import { afterEach, describe, expect, it } from "vitest";
import { commandWithVersionCheck } from "./cli.js";
const originalShell = process.env["SHELL"];
afterEach(() => {
if (originalShell === undefined) {
delete process.env["SHELL"];
} else {
process.env["SHELL"] = originalShell;
}
});
describe("commandWithVersionCheck", () => {
it("emits a POSIX subshell group for bash", () => {
process.env["SHELL"] = "/bin/bash";
expect(commandWithVersionCheck("npm")).toBe("command -v npm && (npm --version 2>&1 || true)");
});
it("emits a POSIX subshell group for zsh", () => {
process.env["SHELL"] = "/bin/zsh";
expect(commandWithVersionCheck("pi")).toBe("command -v pi && (pi --version 2>&1 || true)");
});
it("uses fish begin/end grouping instead of a POSIX subshell", () => {
process.env["SHELL"] = "/usr/local/bin/fish";
const command = commandWithVersionCheck("npm");
expect(command).toBe("command -v npm && begin; npm --version 2>&1 || true; end");
expect(command).not.toContain("(");
});
});
+12 -6
View File
@@ -902,8 +902,12 @@ function commandCheck(command: string): string {
return `command -v ${command}`;
}
function commandWithVersionCheck(command: string): string {
return `${commandCheck(command)} && (${command} --version 2>&1 || true)`;
export function commandWithVersionCheck(command: string): string {
const found = commandCheck(command);
if (detectServiceShell().name === "fish") {
return `${found} && begin; ${command} --version 2>&1 || true; end`;
}
return `${found} && (${command} --version 2>&1 || true)`;
}
function nodeVersionCheck(): string {
@@ -1088,7 +1092,9 @@ async function main(): Promise<void> {
else throw new Error(`Unknown command: ${command}`);
}
main().catch((error: unknown) => {
console.error(error instanceof Error ? error.message : String(error));
process.exit(1);
});
if (process.argv[1] === fileURLToPath(import.meta.url)) {
main().catch((error: unknown) => {
console.error(error instanceof Error ? error.message : String(error));
process.exit(1);
});
}