fix(cli): profile apply honors package scope; network abort timers don't leak (audit)

- profile apply dropped the recorded scope, installing project-scoped packages
  into user scope. Scope is now threaded through the plan (pi's -l passed for
  project-scoped entries), and a user↔project scope move is reported as a change
  rather than silently kept.
- The abort timer in the three network helpers (pi.ts x2, catalog.ts) leaked on a
  settled/rejected fetch, keeping the event loop alive for the full timeout.
  clearTimeout now runs in a finally.

plan-builder scope regression tests. 25 pass, typecheck clean.

Author: hypnguyen1209

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@pify/cli",
-  "version": "0.4.6",
+  "version": "0.4.7",
   "description": "Front door for the Pify suite: install and update the pi coding agent, manage @pify packages, and scaffold Pi Packages",
   "keywords": [
     "pify",

src/catalog.ts

@@ -170,16 +170,19 @@ export async function refreshCatalog(): Promise<{ ok: boolean; catalog: Catalog
 async function fetchRemoteCatalog(defaultUrl: string, timeoutMs: number): Promise<Catalog | null> {
   if (isOffline()) return null;
   const url = process.env.PIFY_CATALOG_URL || defaultUrl;
+  const controller = new AbortController();
+  // Cleared in finally so a settled request (resolved or rejected) never keeps
+  // the event loop alive for the full timeout.
+  const timer = setTimeout(() => controller.abort(), timeoutMs);
   try {
-    const controller = new AbortController();
-    const timer = setTimeout(() => controller.abort(), timeoutMs);
     const res = await fetch(url, { signal: controller.signal });
-    clearTimeout(timer);
     if (!res.ok) return null;
     const data: unknown = await res.json();
     return validateCatalog(data) ? data : null;
   } catch {
     return null;
+  } finally {
+    clearTimeout(timer);
   }
 }
 

src/commands/profile.ts

@@ -86,6 +86,13 @@ export interface PlanRow {
   action: PlanAction;
   from: string | null;
   to: string | null;
+  /**
+   * Where the package should end up. For install/change/keep this is the
+   * profile entry's recorded scope, so applying installs into the same scope it
+   * was saved from (project scope needs pi's `-l`). For `extra` rows there is
+   * no profile entry, so it reflects where the package is currently installed.
+   */
+  scope: "user" | "project";
 }
 
 /**
@@ -99,19 +106,23 @@ export function planApply(profile: Profile, installedNow: Map<string, { scope: "
   for (const entry of profile.packages) {
     const current = installedNow.get(entry.name);
     if (!current) {
-      rows.push({ name: entry.name, action: "install", from: null, to: entry.version });
+      rows.push({ name: entry.name, action: "install", from: null, to: entry.version, scope: entry.scope });
       continue;
     }
     const onDisk = installedVersionOnDisk(entry.name, current.scope).version;
-    if (entry.version && onDisk && entry.version !== onDisk) {
-      rows.push({ name: entry.name, action: "change", from: onDisk, to: entry.version });
+    // A user↔project mismatch is a change even at the same version: the package
+    // has to be reinstalled into the scope the profile recorded.
+    const scopeMismatch = current.scope !== entry.scope;
+    const versionMismatch = Boolean(entry.version && onDisk && entry.version !== onDisk);
+    if (scopeMismatch || versionMismatch) {
+      rows.push({ name: entry.name, action: "change", from: onDisk, to: entry.version, scope: entry.scope });
     } else {
-      rows.push({ name: entry.name, action: "keep", from: onDisk, to: entry.version });
+      rows.push({ name: entry.name, action: "keep", from: onDisk, to: entry.version, scope: entry.scope });
     }
   }
-  for (const [name] of installedNow) {
+  for (const [name, cur] of installedNow) {
     if (!profile.packages.some((p) => p.name === name)) {
-      rows.push({ name, action: "extra", from: installedVersionOnDisk(name, "user").version, to: null });
+      rows.push({ name, action: "extra", from: installedVersionOnDisk(name, cur.scope).version, to: null, scope: cur.scope });
     }
   }
   return rows.sort((a, b) => a.name.localeCompare(b.name));
@@ -123,13 +134,14 @@ export interface ProfileOptions {
 }
 
 function describe(row: PlanRow): string {
+  const scope = row.scope === "project" ? " [project]" : "";
   switch (row.action) {
     case "install":
-      return `install ${row.to ? `@ ${row.to}` : "(latest)"}`;
+      return `install ${row.to ? `@ ${row.to}` : "(latest)"}${scope}`;
     case "change":
-      return `${row.from} → ${row.to}`;
+      return `${row.from ?? "unknown"} → ${row.to ?? "latest"}${scope}`;
     case "keep":
-      return `already ${row.from ?? "installed"}`;
+      return `already ${row.from ?? "installed"}${scope}`;
     case "extra":
       return `installed here, not in the profile (left alone)`;
   }
@@ -189,6 +201,9 @@ export async function profile(args: string[], opts: ProfileOptions): Promise<num
     for (const row of work) {
       const spec = row.to ? `npm:@pify/${row.name}@${row.to}` : `npm:@pify/${row.name}`;
       const argv = ["install", spec];
+      // Mirror install.ts's piArgs: project-scoped entries need pi's `-l`, or
+      // they land in the user scope instead of the one the profile recorded.
+      if (row.scope === "project") argv.push("-l");
       step(`pi ${argv.join(" ")}`);
       if ((await delegate(argv)) !== 0) failed.push(row.name);
     }

src/pi.ts

@@ -36,19 +36,22 @@ export function piStatus(): PiStatus {
 /** Latest pi version from pi's own release endpoint. Null offline/on error. */
 export async function fetchLatestPiVersion(): Promise<string | null> {
   if (isOffline()) return null;
+  const controller = new AbortController();
+  // Cleared in finally so a settled request (resolved or rejected) never keeps
+  // the event loop alive for the full timeout.
+  const timer = setTimeout(() => controller.abort(), 5000);
   try {
-    const controller = new AbortController();
-    const timer = setTimeout(() => controller.abort(), 5000);
     const res = await fetch(LATEST_VERSION_URL, {
       signal: controller.signal,
       headers: { "user-agent": `pify/${VERSION}` },
     });
-    clearTimeout(timer);
     if (!res.ok) return null;
     const data = (await res.json()) as { version?: unknown };
     return typeof data.version === "string" ? data.version : null;
   } catch {
     return null;
+  } finally {
+    clearTimeout(timer);
   }
 }
 
@@ -392,20 +395,23 @@ export interface OnDiskState {
  */
 export async function fetchLatestPackageVersion(name: string): Promise<string | null> {
   if (isOffline()) return null;
+  const controller = new AbortController();
+  // Cleared in finally so a settled request (resolved or rejected) never keeps
+  // the event loop alive for the full timeout.
+  const timer = setTimeout(() => controller.abort(), 5000);
   try {
-    const controller = new AbortController();
-    const timer = setTimeout(() => controller.abort(), 5000);
     const res = await fetch(`https://registry.npmjs.org/@pify/${encodeURIComponent(name)}/latest`, {
       signal: controller.signal,
       headers: { "user-agent": `pify/${VERSION}`, accept: "application/json" },
       redirect: "error",
     });
-    clearTimeout(timer);
     if (!res.ok) return null;
     const data = (await res.json()) as { version?: unknown };
     return typeof data.version === "string" ? data.version : null;
   } catch {
     return null;
+  } finally {
+    clearTimeout(timer);
   }
 }
 

test/unit.test.js

@@ -326,6 +326,49 @@ test("v0.4 planApply says what would change and never proposes a removal", () =>
   );
 });
 
+test("v0.4 planApply threads the recorded scope through every row", () => {
+  // The scope on install/change/keep rows is the profile entry's scope — it
+  // never depends on disk state, so this is deterministic. Regression: apply
+  // used to drop the scope and install project packages into the user scope.
+  const profile = {
+    version: 1,
+    createdAt: "",
+    packages: [
+      { name: "goal", version: null, scope: "project" },
+      { name: "btw", version: null, scope: "user" },
+    ],
+  };
+  const rows = planApply(profile, new Map());
+  const byName = new Map(rows.map((r) => [r.name, r]));
+
+  // Absent packages install into the scope the profile recorded.
+  assert.equal(byName.get("goal").action, "install");
+  assert.equal(byName.get("goal").scope, "project", "project scope must survive to the plan row");
+  assert.equal(byName.get("btw").action, "install");
+  assert.equal(byName.get("btw").scope, "user");
+});
+
+test("v0.4 planApply reports a user↔project scope move as a change", () => {
+  // A package installed in one scope but recorded in the other must be a
+  // "change" (reinstall into the recorded scope), even at the same version.
+  const profile = {
+    version: 1,
+    createdAt: "",
+    packages: [{ name: "goal", version: null, scope: "project" }],
+  };
+  // Installed in the user scope; the profile wants it in the project scope.
+  const installed = new Map([["goal", { scope: "user" }]]);
+  const rows = planApply(profile, installed);
+  const goal = rows.find((r) => r.name === "goal");
+
+  assert.equal(goal.action, "change", "a scope mismatch is a change, not a keep");
+  assert.equal(goal.scope, "project", "the change targets the recorded scope");
+
+  // Same package, same scope on both sides, no version pin -> nothing to move.
+  const matched = planApply(profile, new Map([["goal", { scope: "project" }]]));
+  assert.equal(matched.find((r) => r.name === "goal").action, "keep");
+});
+
 test("v0.4 the catalog declares conflicts, and only npm-shaped names", () => {
   const catalog = loadBundledCatalog();
   const withConflicts = catalog.packages.filter((p) => p.conflicts?.length);