Merge pull request #32 from gilbertwong96/fix/fish-doctor-version-check

fix: run doctor version checks under fish

.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.

src/cli.test.ts

@@ -0,0 +1,60 @@
+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, isCliEntrypoint } 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("(");
+  });
+});
+
+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);
+  });
+});

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";
@@ -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,19 @@ async function main(): Promise<void> {
   else throw new Error(`Unknown command: ${command}`);
 }
 
+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);
   });
+}