fix: bootstrap managed tools and gh auth

Preserve the original #39 fix while adding the missing hardening and regression coverage. Credit to @LuxVTZ for the original fix incorporated here.

src/cli.ts

@@ -6,11 +6,17 @@ import {
   createAgentSession,
   InteractiveMode,
 } from '@mariozechner/pi-coding-agent'
-import { join } from 'path'
+import { join } from 'node:path'
 import { agentDir, sessionsDir, authFilePath } from './app-paths.js'
 import { buildResourceLoader, initResources } from './resource-loader.js'
+import { ensureManagedTools } from './tool-bootstrap.js'
 import { loadStoredEnvKeys, runWizardIfNeeded } from './wizard.js'
 
+// Pi's tool bootstrap can mis-detect already-installed fd/rg on some systems
+// because spawnSync(..., ["--version"]) returns EPERM despite a zero exit code.
+// Provision local managed binaries first so Pi sees them without probing PATH.
+ensureManagedTools(join(agentDir, 'bin'))
+
 const authStorage = AuthStorage.create(authFilePath)
 loadStoredEnvKeys(authStorage)
 await runWizardIfNeeded(authStorage)

src/resources/extensions/github/gh-api.ts

@@ -6,20 +6,44 @@
  * Falls back to raw REST API with GITHUB_TOKEN env var.
  */
 
-import { execSync } from "node:child_process";
+import { execSync, spawnSync, type SpawnSyncReturns } from "node:child_process";
 
 // ─── Auth detection ───────────────────────────────────────────────────────────
 
 let _useGhCli: boolean | null = null;
 
-function hasGhCli(): boolean {
+let ghSpawnImpl = (args: string[], input?: string, cwd?: string): SpawnSyncReturns<string> =>
+	spawnSync("gh", args, {
+		cwd,
+		encoding: "utf8",
+		stdio: ["pipe", "pipe", "pipe"],
+		input,
+	});
+
+function ghSpawn(args: string[], input?: string, cwd?: string): SpawnSyncReturns<string> {
+	return ghSpawnImpl(args, input, cwd);
+}
+
+export function resetGhCliDetectionForTests(): void {
+	_useGhCli = null;
+	ghSpawnImpl = (args: string[], input?: string, cwd?: string): SpawnSyncReturns<string> =>
+		spawnSync("gh", args, {
+			cwd,
+			encoding: "utf8",
+			stdio: ["pipe", "pipe", "pipe"],
+			input,
+		});
+}
+
+export function setGhSpawnForTests(fn: (args: string[], input?: string, cwd?: string) => SpawnSyncReturns<string>): void {
+	ghSpawnImpl = fn;
+	_useGhCli = null;
+}
+
+export function hasGhCli(): boolean {
 	if (_useGhCli !== null) return _useGhCli;
-	try {
-		execSync("gh auth status", { encoding: "utf8", stdio: ["pipe", "pipe", "pipe"] });
-		_useGhCli = true;
-	} catch {
-		_useGhCli = false;
-	}
+	const result = ghSpawn(["auth", "token"]);
+	_useGhCli = result.status === 0 && !result.error && !!result.stdout?.trim();
 	return _useGhCli;
 }
 
@@ -120,11 +144,6 @@ export async function ghApi<T = unknown>(
 	return fetchApi<T>(endpoint, method, options.params, options.body, token);
 }
 
-function shellEscape(s: string): string {
-	// Single-quote wrapping, escaping any existing single quotes
-	return "'" + s.replace(/'/g, "'\\''") + "'";
-}
-
 function ghCliApi<T>(
 	endpoint: string,
 	method: string,
@@ -132,39 +151,36 @@ function ghCliApi<T>(
 	body?: Record<string, unknown>,
 	cwd?: string,
 ): T {
-	const parts = ["gh", "api", shellEscape(endpoint), "--method", method];
+	const args = ["api", endpoint, "--method", method];
 
 	if (params) {
 		for (const [key, val] of Object.entries(params)) {
 			if (val === undefined) continue;
 			if (Array.isArray(val)) {
 				for (const v of val) {
-					parts.push("-f", shellEscape(`${key}[]=${v}`));
+					args.push("-f", `${key}[]=${v}`);
 				}
 			} else {
-				parts.push("-f", shellEscape(`${key}=${String(val)}`));
+				args.push("-f", `${key}=${String(val)}`);
 			}
 		}
 	}
 
 	if (body) {
-		parts.push("--input", "-");
+		args.push("--input", "-");
 	}
 
-	try {
-		const result = execSync(parts.join(" "), {
-			cwd: cwd ?? process.cwd(),
-			encoding: "utf8",
-			stdio: ["pipe", "pipe", "pipe"],
-			input: body ? JSON.stringify(body) : undefined,
-		});
-		if (!result.trim()) return {} as T;
-		return JSON.parse(result) as T;
-	} catch (e: unknown) {
-		const err = e as { stderr?: string; stdout?: string; message?: string };
-		const msg = err.stderr?.trim() || err.stdout?.trim() || err.message || String(e);
-		throw new Error(`gh api error: ${msg}`);
+	const result = ghSpawn(args, body ? JSON.stringify(body) : undefined, cwd ?? process.cwd());
+
+	const stdout = result.stdout?.trim() ?? "";
+	const stderr = result.stderr?.trim() ?? "";
+
+	if (result.status !== 0) {
+		throw new Error(`gh api error: ${stderr || stdout || result.error?.message || `exit code ${result.status}`}`);
 	}
+
+	if (!stdout) return {} as T;
+	return JSON.parse(stdout) as T;
 }
 
 async function fetchApi<T>(

src/tests/gh-api.test.ts

@@ -0,0 +1,52 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { spawnSync as realSpawnSync } from "node:child_process";
+
+import * as ghApiModule from "../resources/extensions/github/gh-api.ts";
+
+function makeSpawnResult(overrides: Partial<ReturnType<typeof realSpawnSync>>): ReturnType<typeof realSpawnSync> {
+  return {
+    status: 0,
+    stdout: "",
+    stderr: "",
+    output: [null, "", ""],
+    pid: 1,
+    signal: null,
+    ...overrides,
+  } as ReturnType<typeof realSpawnSync>;
+}
+
+test("hasGhCli treats zero-exit token output as authenticated", () => {
+  ghApiModule.setGhSpawnForTests(() => makeSpawnResult({ stdout: "gho_test\n" }));
+
+  try {
+    assert.equal(ghApiModule.hasGhCli(), true);
+    assert.equal(ghApiModule.authMethod(), "gh CLI");
+  } finally {
+    ghApiModule.resetGhCliDetectionForTests();
+  }
+});
+
+test("hasGhCli rejects zero-exit responses with empty stdout", () => {
+  ghApiModule.setGhSpawnForTests(() => makeSpawnResult({ stdout: "" }));
+
+  try {
+    assert.equal(ghApiModule.hasGhCli(), false);
+  } finally {
+    ghApiModule.resetGhCliDetectionForTests();
+  }
+});
+
+test("hasGhCli rejects spawnSync error even with zero exit", () => {
+  ghApiModule.setGhSpawnForTests(() => makeSpawnResult({
+    stdout: "gho_test\n",
+    stderr: "EPERM",
+    error: new Error("spawnSync gh EPERM"),
+  }));
+
+  try {
+    assert.equal(ghApiModule.hasGhCli(), false);
+  } finally {
+    ghApiModule.resetGhCliDetectionForTests();
+  }
+});

src/tests/tool-bootstrap.test.ts

@@ -0,0 +1,73 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { chmodSync, existsSync, lstatSync, mkdtempSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs";
+import { join } from "node:path";
+import { tmpdir } from "node:os";
+
+import { ensureManagedTools, resolveToolFromPath } from "../tool-bootstrap.js";
+
+function makeExecutable(dir: string, name: string, content = "#!/bin/sh\nexit 0\n"): string {
+  const file = join(dir, name);
+  writeFileSync(file, content);
+  chmodSync(file, 0o755);
+  return file;
+}
+
+test("resolveToolFromPath finds fd via fdfind fallback", () => {
+  const tmp = mkdtempSync(join(tmpdir(), "gsd-tool-bootstrap-resolve-"));
+  try {
+    makeExecutable(tmp, "fdfind");
+    const resolved = resolveToolFromPath("fd", tmp);
+    assert.equal(resolved, join(tmp, "fdfind"));
+  } finally {
+    rmSync(tmp, { recursive: true, force: true });
+  }
+});
+
+test("ensureManagedTools provisions fd and rg into managed bin dir", () => {
+  const tmp = mkdtempSync(join(tmpdir(), "gsd-tool-bootstrap-provision-"));
+  const sourceBin = join(tmp, "source-bin");
+  const targetBin = join(tmp, "target-bin");
+
+  mkdirSync(sourceBin, { recursive: true });
+  mkdirSync(targetBin, { recursive: true });
+
+  try {
+    makeExecutable(sourceBin, "fdfind");
+    makeExecutable(sourceBin, "rg");
+
+    const provisioned = ensureManagedTools(targetBin, sourceBin);
+
+    assert.equal(provisioned.length, 2);
+    assert.ok(existsSync(join(targetBin, "fd")));
+    assert.ok(existsSync(join(targetBin, "rg")));
+    assert.ok(lstatSync(join(targetBin, "fd")).isSymbolicLink() || lstatSync(join(targetBin, "fd")).isFile());
+    assert.ok(lstatSync(join(targetBin, "rg")).isSymbolicLink() || lstatSync(join(targetBin, "rg")).isFile());
+  } finally {
+    rmSync(tmp, { recursive: true, force: true });
+  }
+});
+
+test("ensureManagedTools copies executable when symlink target already exists as a broken link", () => {
+  const tmp = mkdtempSync(join(tmpdir(), "gsd-tool-bootstrap-copy-"));
+  const sourceBin = join(tmp, "source-bin");
+  const targetBin = join(tmp, "target-bin");
+  const targetFd = join(targetBin, "fd");
+
+  mkdirSync(sourceBin, { recursive: true });
+  mkdirSync(targetBin, { recursive: true });
+
+  try {
+    makeExecutable(sourceBin, "fdfind", "#!/bin/sh\necho fd\n");
+    makeExecutable(sourceBin, "rg", "#!/bin/sh\necho rg\n");
+    symlinkSync(join(tmp, "missing-target"), targetFd);
+
+    const provisioned = ensureManagedTools(targetBin, sourceBin);
+
+    assert.equal(provisioned.length, 2);
+    assert.ok(lstatSync(targetFd).isFile(), "fd fallback should replace broken symlink with a copied file");
+    assert.match(readFileSync(targetFd, "utf8"), /echo fd/);
+  } finally {
+    rmSync(tmp, { recursive: true, force: true });
+  }
+});

src/tool-bootstrap.ts

@@ -0,0 +1,85 @@
+import { chmodSync, copyFileSync, existsSync, lstatSync, mkdirSync, rmSync, symlinkSync } from "node:fs";
+import { delimiter, join } from "node:path";
+
+type ManagedTool = "fd" | "rg";
+
+interface ToolSpec {
+  targetName: string;
+  candidates: string[];
+}
+
+const TOOL_SPECS: Record<ManagedTool, ToolSpec> = {
+  fd: {
+    targetName: process.platform === "win32" ? "fd.exe" : "fd",
+    candidates: process.platform === "win32" ? ["fd.exe", "fd", "fdfind.exe", "fdfind"] : ["fd", "fdfind"],
+  },
+  rg: {
+    targetName: process.platform === "win32" ? "rg.exe" : "rg",
+    candidates: process.platform === "win32" ? ["rg.exe", "rg"] : ["rg"],
+  },
+};
+
+function splitPath(pathValue: string | undefined): string[] {
+  if (!pathValue) return [];
+  return pathValue.split(delimiter).map((segment) => segment.trim()).filter(Boolean);
+}
+
+function getCandidateNames(name: string): string[] {
+  if (process.platform !== "win32") return [name];
+  const lower = name.toLowerCase();
+  if (lower.endsWith(".exe") || lower.endsWith(".cmd") || lower.endsWith(".bat")) return [name];
+  return [name, `${name}.exe`, `${name}.cmd`, `${name}.bat`];
+}
+
+function isRegularFile(path: string): boolean {
+  try {
+    return lstatSync(path).isFile() || lstatSync(path).isSymbolicLink();
+  } catch {
+    return false;
+  }
+}
+
+export function resolveToolFromPath(tool: ManagedTool, pathValue: string | undefined = process.env.PATH): string | null {
+  const spec = TOOL_SPECS[tool];
+  for (const dir of splitPath(pathValue)) {
+    for (const candidate of spec.candidates) {
+      for (const name of getCandidateNames(candidate)) {
+        const fullPath = join(dir, name);
+        if (existsSync(fullPath) && isRegularFile(fullPath)) {
+          return fullPath;
+        }
+      }
+    }
+  }
+  return null;
+}
+
+function provisionTool(targetDir: string, tool: ManagedTool, sourcePath: string): string {
+  const targetPath = join(targetDir, TOOL_SPECS[tool].targetName);
+  if (existsSync(targetPath)) return targetPath;
+
+  mkdirSync(targetDir, { recursive: true });
+
+  try {
+    symlinkSync(sourcePath, targetPath);
+  } catch {
+    rmSync(targetPath, { force: true });
+    copyFileSync(sourcePath, targetPath);
+    chmodSync(targetPath, 0o755);
+  }
+
+  return targetPath;
+}
+
+export function ensureManagedTools(targetDir: string, pathValue: string | undefined = process.env.PATH): string[] {
+  const provisioned: string[] = [];
+
+  for (const tool of Object.keys(TOOL_SPECS) as ManagedTool[]) {
+    if (existsSync(join(targetDir, TOOL_SPECS[tool].targetName))) continue;
+    const sourcePath = resolveToolFromPath(tool, pathValue);
+    if (!sourcePath) continue;
+    provisioned.push(provisionTool(targetDir, tool, sourcePath));
+  }
+
+  return provisioned;
+}