From 47c9b668193d2bcada5fd2fdbd58e419dcc30845 Mon Sep 17 00:00:00 2001 From: Gilbert Date: Thu, 25 Jun 2026 10:54:49 +0800 Subject: [PATCH 1/2] 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. --- .changeset/fix-fish-doctor-version-check.md | 9 ++++++ src/cli.test.ts | 31 +++++++++++++++++++++ src/cli.ts | 18 ++++++++---- 3 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 .changeset/fix-fish-doctor-version-check.md create mode 100644 src/cli.test.ts diff --git a/.changeset/fix-fish-doctor-version-check.md b/.changeset/fix-fish-doctor-version-check.md new file mode 100644 index 0000000..8163c01 --- /dev/null +++ b/.changeset/fix-fish-doctor-version-check.md @@ -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. diff --git a/src/cli.test.ts b/src/cli.test.ts new file mode 100644 index 0000000..cb3849d --- /dev/null +++ b/src/cli.test.ts @@ -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("("); + }); +}); diff --git a/src/cli.ts b/src/cli.ts index 75d2c64..5e5f663 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -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 { 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); + }); +} From 5269a7a7d7d3900afe518d007439ef79b5df613f Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Thu, 25 Jun 2026 14:41:58 +0200 Subject: [PATCH 2/2] fix: support symlinked CLI entrypoint guard --- src/cli.test.ts | 31 ++++++++++++++++++++++++++++++- src/cli.ts | 14 ++++++++++++-- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/cli.test.ts b/src/cli.test.ts index cb3849d..72da8f8 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -1,5 +1,8 @@ +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; -import { commandWithVersionCheck } from "./cli.js"; +import { commandWithVersionCheck, isCliEntrypoint } from "./cli.js"; const originalShell = process.env["SHELL"]; @@ -29,3 +32,29 @@ describe("commandWithVersionCheck", () => { expect(command).not.toContain("("); }); }); + +describe("isCliEntrypoint", () => { + it("matches direct execution paths", () => { + expect(isCliEntrypoint("/tmp/pi-web-cli.js", "/tmp/pi-web-cli.js")).toBe(true); + }); + + it("matches npm-style symlinked bin entrypoints", () => { + const dir = mkdtempSync(join(tmpdir(), "pi-web-cli-test-")); + try { + const target = join(dir, "dist", "cli.js"); + const symlink = join(dir, "bin", "pi-web"); + mkdirSync(join(dir, "dist")); + mkdirSync(join(dir, "bin")); + writeFileSync(target, "#!/usr/bin/env node\n", { mode: 0o755 }); + symlinkSync(target, symlink); + + expect(isCliEntrypoint(symlink, target)).toBe(true); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("does not match unrelated paths", () => { + expect(isCliEntrypoint("/tmp/pi-web", "/tmp/other-pi-web")).toBe(false); + }); +}); diff --git a/src/cli.ts b/src/cli.ts index 5e5f663..00f1409 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1,6 +1,6 @@ #!/usr/bin/env node import { spawnSync } from "node:child_process"; -import { existsSync, readFileSync } from "node:fs"; +import { existsSync, readFileSync, realpathSync } from "node:fs"; import { mkdir, rm, writeFile } from "node:fs/promises"; import { homedir, userInfo } from "node:os"; import { basename, dirname, join, resolve } from "node:path"; @@ -1092,7 +1092,17 @@ async function main(): Promise { else throw new Error(`Unknown command: ${command}`); } -if (process.argv[1] === fileURLToPath(import.meta.url)) { +export function isCliEntrypoint(entrypoint: string | undefined = process.argv[1], modulePath: string = fileURLToPath(import.meta.url)): boolean { + if (entrypoint === undefined) return false; + if (entrypoint === modulePath) return true; + try { + return realpathSync(entrypoint) === realpathSync(modulePath); + } catch { + return false; + } +} + +if (isCliEntrypoint()) { main().catch((error: unknown) => { console.error(error instanceof Error ? error.message : String(error)); process.exit(1);