Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ If Pi is already running, use `/reload`.

Requires Node.js `>=22.20.0`.

### npm prefix permissions

Pi global package installs use `npm install -g`. If `npm_config_prefix` points at an
unwritable prefix such as `/usr/local`, install or reload can fail with `EACCES`.
Use a writable npm prefix, configure Pi's `npmCommand` setting for your Node
version manager, or install project-local with `pi install npm:pi-extmgr -l`.
Comment thread
coderabbitai[bot] marked this conversation as resolved.

## Features

- **Unified manager UI**
Expand Down
30 changes: 19 additions & 11 deletions src/packages/extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
normalizeRelativePath,
resolveRelativePathSelection,
} from "../utils/relative-path-selection.js";
import { resolveNpmCommand } from "../utils/npm-exec.js";
import { resolveConfiguredNpmRootCommand } from "../utils/npm-exec.js";

interface PackageSettingsObject {
source: string;
Expand All @@ -39,7 +39,7 @@ export interface PackageManifest {
}

const execFileAsync = promisify(execFile);
let globalNpmRootCache: string | null | undefined;
let globalNpmRootCache: { key: string; root: string | null } | undefined;

function normalizeSource(source: string): string {
return source
Expand All @@ -58,24 +58,32 @@ function normalizePackageRootCandidate(candidate: string): string {
return resolved;
}

async function getGlobalNpmRoot(): Promise<string | undefined> {
if (globalNpmRootCache !== undefined) {
return globalNpmRootCache ?? undefined;
async function getGlobalNpmRoot(cwd: string): Promise<string | undefined> {
let npmCommand: ReturnType<typeof resolveConfiguredNpmRootCommand>;
try {
npmCommand = resolveConfiguredNpmRootCommand(cwd);
} catch {
return undefined;
}

const cacheKey = [npmCommand.command, ...npmCommand.args].join("\0");

if (globalNpmRootCache?.key === cacheKey) {
return globalNpmRootCache.root ?? undefined;
}

try {
const npmCommand = resolveNpmCommand(["root", "-g"]);
const { stdout } = await execFileAsync(npmCommand.command, npmCommand.args, {
timeout: 2_000,
windowsHide: true,
});
const root = stdout.trim();
globalNpmRootCache = root || null;
const root = npmCommand.getRoot(stdout);
globalNpmRootCache = { key: cacheKey, root: root || null };
} catch {
globalNpmRootCache = null;
globalNpmRootCache = { key: cacheKey, root: null };
}

return globalNpmRootCache ?? undefined;
return globalNpmRootCache.root ?? undefined;
}

async function resolveNpmPackageRoot(
Expand All @@ -96,7 +104,7 @@ async function resolveNpmPackageRoot(
const packageDir = process.env.PI_PACKAGE_DIR || join(homedir(), ".pi", "agent");
const globalCandidates = [join(packageDir, "npm", "node_modules", packageName)];

const npmGlobalRoot = await getGlobalNpmRoot();
const npmGlobalRoot = await getGlobalNpmRoot(cwd);
if (npmGlobalRoot) {
globalCandidates.unshift(join(npmGlobalRoot, packageName));
}
Expand Down
74 changes: 71 additions & 3 deletions src/utils/npm-exec.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
import path from "node:path";
import { execPath, platform } from "node:process";
import { type ExtensionAPI } from "@mariozechner/pi-coding-agent";
import { type ExtensionAPI, getAgentDir, SettingsManager } from "@mariozechner/pi-coding-agent";

interface NpmCommandResolutionOptions {
platform?: NodeJS.Platform;
nodeExecPath?: string;
npmCommand?: readonly string[] | undefined;
}

interface ResolvedNpmCommand {
command: string;
args: string[];
}

interface ResolvedNpmRootCommand extends ResolvedNpmCommand {
getRoot(stdout: string): string;
}

interface NpmExecOptions {
Expand All @@ -17,10 +27,37 @@ function getNpmCliPath(nodeExecPath: string, runtimePlatform: NodeJS.Platform):
return pathImpl.join(pathImpl.dirname(nodeExecPath), "node_modules", "npm", "bin", "npm-cli.js");
}

function getConfiguredNpmBase(
npmCommand?: readonly string[] | undefined
): ResolvedNpmCommand | undefined {
if (!npmCommand || npmCommand.length === 0) {
return undefined;
}

const [command, ...args] = npmCommand;
if (!command?.trim()) {
throw new Error("Invalid npmCommand: first array entry must be a non-empty command");
}

return { command, args: [...args] };
}

function getSettingsNpmCommand(cwd: string): string[] | undefined {
return SettingsManager.create(cwd, getAgentDir()).getNpmCommand();
}

export function resolveNpmCommand(
npmArgs: string[],
options?: NpmCommandResolutionOptions
): { command: string; args: string[] } {
): ResolvedNpmCommand {
const configured = getConfiguredNpmBase(options?.npmCommand);
if (configured) {
return {
command: configured.command,
args: [...configured.args, ...npmArgs],
};
}

const runtimePlatform = options?.platform ?? platform;

if (runtimePlatform === "win32") {
Expand All @@ -34,13 +71,44 @@ export function resolveNpmCommand(
return { command: "npm", args: npmArgs };
}

export function resolveConfiguredNpmCommand(npmArgs: string[], cwd: string): ResolvedNpmCommand {
return resolveNpmCommand(npmArgs, { npmCommand: getSettingsNpmCommand(cwd) });
}

export function resolveNpmRootCommand(
options?: NpmCommandResolutionOptions
): ResolvedNpmRootCommand {
const configured = getConfiguredNpmBase(options?.npmCommand);

if (configured?.command === "bun") {
return {
command: configured.command,
args: [...configured.args, "pm", "bin", "-g"],
getRoot: (stdout) => {
const binDir = stdout.trim();
return binDir ? path.join(path.dirname(binDir), "install", "global", "node_modules") : "";
},
};
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

const resolved = resolveNpmCommand(["root", "-g"], options);
return {
...resolved,
getRoot: (stdout) => stdout.trim(),
};
}

export function resolveConfiguredNpmRootCommand(cwd: string): ResolvedNpmRootCommand {
return resolveNpmRootCommand({ npmCommand: getSettingsNpmCommand(cwd) });
}

export async function execNpm(
pi: ExtensionAPI,
npmArgs: string[],
ctx: { cwd: string },
options: NpmExecOptions
): Promise<{ code: number; stdout: string; stderr: string; killed: boolean }> {
const resolved = resolveNpmCommand(npmArgs);
const resolved = resolveConfiguredNpmCommand(npmArgs, ctx.cwd);
return pi.exec(resolved.command, resolved.args, {
timeout: options.timeout,
cwd: ctx.cwd,
Expand Down
14 changes: 10 additions & 4 deletions src/utils/ui-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
import { type ExtensionCommandContext } from "@mariozechner/pi-coding-agent";
import { UI } from "../constants.js";
import { notify } from "./notify.js";
import { error as notifyError, notify } from "./notify.js";

/**
* Confirm and trigger reload
Expand All @@ -20,12 +20,18 @@ export async function confirmReload(

const confirmed = await ctx.ui.confirm("Reload Required", `${reason}\nReload pi now?`);

if (confirmed) {
if (!confirmed) {
return false;
}

try {
await ctx.reload();
return true;
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
notifyError(ctx, `Reload failed: ${message}`);
return false;
}

return false;
}

/**
Expand Down
31 changes: 30 additions & 1 deletion test/npm-exec.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import assert from "node:assert/strict";
import test from "node:test";
import { resolveNpmCommand } from "../src/utils/npm-exec.js";
import { resolveNpmCommand, resolveNpmRootCommand } from "../src/utils/npm-exec.js";

void test("resolveNpmCommand uses npm directly on non-windows", () => {
const resolved = resolveNpmCommand(["view", "pi-extmgr", "version", "--json"], {
Expand All @@ -25,3 +25,32 @@ void test("resolveNpmCommand uses node + npm-cli.js on windows", () => {
"pi-extmgr",
]);
});

void test("resolveNpmCommand honors Pi npmCommand settings", () => {
const resolved = resolveNpmCommand(["view", "pi-extmgr", "version", "--json"], {
npmCommand: ["mise", "exec", "node@22", "--", "npm"],
});

assert.equal(resolved.command, "mise");
assert.deepEqual(resolved.args, [
"exec",
"node@22",
"--",
"npm",
"view",
"pi-extmgr",
"version",
"--json",
]);
});

void test("resolveNpmRootCommand follows Pi's bun global root convention", () => {
const resolved = resolveNpmRootCommand({ npmCommand: ["bun"] });

assert.equal(resolved.command, "bun");
assert.deepEqual(resolved.args, ["pm", "bin", "-g"]);
assert.equal(
resolved.getRoot("/home/alice/.bun/bin\n"),
"/home/alice/.bun/install/global/node_modules"
);
});
28 changes: 28 additions & 0 deletions test/ui-helpers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import assert from "node:assert/strict";
import test from "node:test";
import { type ExtensionCommandContext } from "@mariozechner/pi-coding-agent";
import { confirmReload } from "../src/utils/ui-helpers.js";

void test("confirmReload reports reload failures without throwing", async () => {
const notifications: { message: string; level: string | undefined }[] = [];
const ctx = {
hasUI: true,
ui: {
confirm: () => Promise.resolve(true),
notify: (message: string, level?: string) => {
notifications.push({ message, level });
},
},
reload: () => Promise.reject(new Error("npm install -g pi-extmgr failed with code 243")),
} as unknown as ExtensionCommandContext;

const reloaded = await confirmReload(ctx, "Package updated.");

assert.equal(reloaded, false);
assert.deepEqual(notifications, [
{
message: "Reload failed: npm install -g pi-extmgr failed with code 243",
level: "error",
},
]);
});
Loading