From d62899308a9452af02ea1f1d1eb93cac2688ce7c Mon Sep 17 00:00:00 2001 From: Stefano Fiorini Date: Thu, 23 Apr 2026 22:55:41 -0500 Subject: [PATCH] feat(installer): support pi package remove and update --- docs/INSTALLER.md | 13 ++- scripts/lib/skill-manager-core.mjs | 119 ++++++++++++++++++-- scripts/manage-skills.mjs | 19 +++- scripts/tests/skill-manager-core.test.mjs | 130 ++++++++++++++++++++++ 4 files changed, 266 insertions(+), 15 deletions(-) diff --git a/docs/INSTALLER.md b/docs/INSTALLER.md index 06d72f2..998bc60 100644 --- a/docs/INSTALLER.md +++ b/docs/INSTALLER.md @@ -79,11 +79,22 @@ Workflow skills install/update the reviewer-runtime helper bundle automatically ## Pi Package Mode -Pi can be managed as a package install or by manual skill copy. Package mode installs the full Pi bundle and bootstraps packaged runtimes: +Pi can be managed as a package install or by manual skill copy. Package mode always manages the full Pi bundle; per-skill prompts and `--skill` narrowing are ignored for `packageGlobal` and `packageLocal`. + +Package-mode actions: + +- `install`: register the package if needed, list bundled skills, and skip already-bootstrapped runtimes. +- `update`: sync the Pi package mirror, reinstall the package registration, and rerun runtime dependency bootstrapping. +- `reinstall`: same behavior as `update`, kept for action parity with manual skill scopes. +- `remove`: unregister the package with `pi remove`; this does not delete repo files or `node_modules`. + +Examples: ```bash node scripts/manage-skills.mjs --client pi --scope packageGlobal --pi-package --action install --yes node scripts/manage-skills.mjs --client pi --scope packageLocal --pi-package --action install --yes +node scripts/manage-skills.mjs --client pi --scope packageGlobal --pi-package --action update --yes +node scripts/manage-skills.mjs --client pi --scope packageGlobal --pi-package --action remove --yes ``` The compatibility script remains available: diff --git a/scripts/lib/skill-manager-core.mjs b/scripts/lib/skill-manager-core.mjs index 452aa16..ed96d91 100644 --- a/scripts/lib/skill-manager-core.mjs +++ b/scripts/lib/skill-manager-core.mjs @@ -1,4 +1,4 @@ -import { access, cp, lstat, mkdir, realpath, rm, stat, symlink, chmod, unlink } from "node:fs/promises"; +import { access, cp, lstat, mkdir, readFile, realpath, rm, stat, symlink, chmod, unlink } from "node:fs/promises"; import { constants, existsSync } from "node:fs"; import { homedir } from "node:os"; import path from "node:path"; @@ -253,6 +253,55 @@ export async function findInstalledSuperpowers(clientId, cwd = process.cwd()) { return found; } +function piPackageSettingsPath(scope, repoRoot) { + if (scope === "packageLocal") return path.join(repoRoot, ".pi", "settings.json"); + return path.join(homedir(), ".pi", "agent", "settings.json"); +} + +async function readJsonIfExists(filePath) { + try { + return JSON.parse(await readFile(filePath, "utf8")); + } catch (error) { + if (error?.code === "ENOENT") return null; + throw error; + } +} + +export async function isPiPackageInstalled({ scope, repoRoot }) { + const settingsPath = piPackageSettingsPath(scope, repoRoot); + const settings = await readJsonIfExists(settingsPath); + if (!Array.isArray(settings?.packages)) return false; + const settingsDir = path.dirname(settingsPath); + const expected = path.resolve(repoRoot); + return settings.packages.some((packagePath) => path.resolve(settingsDir, packagePath) === expected); +} + +export function isBootstrapInstalled(action, target) { + const scriptsDir = path.join(target, "scripts"); + if (action === "pnpm-install") return existsSync(path.join(scriptsDir, "node_modules")); + if (action === "web-automation") { + return existsSync(path.join(scriptsDir, "node_modules")) + && existsSync(path.join(scriptsDir, "node_modules", ".bin", "cloakbrowser")); + } + return false; +} + +export function piPackageCommand({ action, repoRoot, piInstallArg = "" }) { + if (["install", "update", "reinstall"].includes(action)) { + const args = ["install"]; + if (piInstallArg) args.push(piInstallArg); + args.push(repoRoot); + return ["pi", args]; + } + if (action === "remove") { + const args = ["remove"]; + if (piInstallArg) args.push(piInstallArg); + args.push(repoRoot); + return ["pi", args]; + } + throw new Error(`pi package mode does not support action: ${action}`); +} + export function requiredSuperpowersFor(actions) { const required = new Set(); for (const [skillName, action] of Object.entries(actions || {})) { @@ -274,9 +323,59 @@ export async function buildOperationPlan({ selections, repoRoot = process.cwd(), const scopeInfo = selection.skillsRoot ? { skillsRoot: selection.skillsRoot } : resolveClientScope(clientId, scope, repoRoot); const skillsRoot = scopeInfo.skillsRoot; if (clientId === "pi" && scopeInfo.packageMode) { - operations.push({ kind: "pi-package", clientId, scope, action: selection.action || "install", repoRoot, piInstallArg: scopeInfo.piInstallArg || "" }); - operations.push({ kind: "bootstrap", clientId, scope, skill: "atlassian", action: "pnpm-install", target: path.join(repoRoot, "pi-package", "skills", "atlassian") }); - operations.push({ kind: "bootstrap", clientId, scope, skill: "web-automation", action: "web-automation", target: path.join(repoRoot, "pi-package", "skills", "web-automation") }); + const action = selection.action || "install"; + if (action === "skip") continue; + if (["update", "reinstall"].includes(action)) { + operations.push({ kind: "sync-pi-package", clientId, scope, action: "sync", repoRoot }); + } + const packageInstalled = await isPiPackageInstalled({ scope, repoRoot }); + if (action === "remove") { + operations.push({ + kind: "pi-package", + clientId, + scope, + action, + repoRoot, + piInstallArg: scopeInfo.piInstallArg || "", + status: packageInstalled ? undefined : "skipped", + details: packageInstalled ? "" : "not installed in Pi settings", + }); + continue; + } + operations.push({ + kind: "pi-package", + clientId, + scope, + action, + repoRoot, + piInstallArg: scopeInfo.piInstallArg || "", + status: packageInstalled && action === "install" ? "skipped" : undefined, + details: packageInstalled && action === "install" ? "already installed in Pi settings" : "", + }); + for (const skillName of Object.keys(SKILLS)) { + if (getSkillSource(skillName, clientId, repoRoot)) { + operations.push({ kind: "package-skill", clientId, scope, skill: skillName, action: "included", status: "included", details: "included in Pi package" }); + } + } + for (const bootstrap of [ + { skill: "atlassian", action: "pnpm-install" }, + { skill: "web-automation", action: "web-automation" }, + ]) { + const target = path.join(repoRoot, "pi-package", "skills", bootstrap.skill); + const installed = isBootstrapInstalled(bootstrap.action, target); + const skipBootstrap = action === "install" && installed; + operations.push({ + kind: "bootstrap", + clientId, + scope, + skill: bootstrap.skill, + action: bootstrap.action, + displayAction: "bootstrap-deps", + target, + status: skipBootstrap ? "skipped" : undefined, + details: skipBootstrap ? "runtime dependencies already installed" : target, + }); + } continue; } const installed = await detectInstalledSkills({ skillsRoot, clientId, repoRoot }); @@ -326,7 +425,7 @@ export async function buildOperationPlan({ selections, repoRoot = process.cwd(), client: op.clientId, scope: op.scope, item: op.skill || op.helper || op.kind, - action: op.action, + action: op.displayAction || op.action, status: op.status || "planned", details: op.details || op.target || "", }); @@ -399,18 +498,14 @@ function requireNode20() { export async function executeOperation(op) { if (op.action === "unsupported" || op.status === "skipped") return { ...op, status: "skipped" }; + if (op.kind === "package-skill") return { ...op, status: "included" }; if (op.kind === "sync-pi-package") { runCommand(path.join(op.repoRoot, "scripts", "sync-pi-package-skills.sh"), [], { cwd: op.repoRoot }); return { ...op, status: "ok" }; } if (op.kind === "pi-package") { - if (op.action !== "install" && op.action !== "update" && op.action !== "reinstall") { - throw new Error(`pi package mode does not support action: ${op.action}`); - } - const args = ["install"]; - if (op.piInstallArg) args.push(op.piInstallArg); - args.push(op.repoRoot); - runCommand("pi", args, { cwd: op.repoRoot }); + const [command, args] = piPackageCommand(op); + runCommand(command, args, { cwd: op.repoRoot }); return { ...op, status: "ok" }; } if (op.kind === "skill") { diff --git a/scripts/manage-skills.mjs b/scripts/manage-skills.mjs index 9159469..9cbb3bd 100755 --- a/scripts/manage-skills.mjs +++ b/scripts/manage-skills.mjs @@ -11,6 +11,7 @@ import { detectInstalledClients, detectInstalledSkills, executeOperation, + isPiPackageInstalled, parseReviewerShorthand, resolveClientScope, reviewerRuntimeRoot, @@ -93,6 +94,10 @@ async function buildCliSelection(args) { const clientId = args.client; if (!CLIENTS[clientId]) throw new Error(`Unsupported client: ${clientId}`); const scope = args.scope || (clientId === "pi" && args.piPackage ? "packageGlobal" : "global"); + const scopeInfo = resolveClientScope(clientId, scope, process.cwd()); + if (clientId === "pi" && scopeInfo.packageMode) { + return { selections: [{ clientId, scope, action: args.action || "install", actions: {} }] }; + } const skills = args.skills.length ? args.skills : Object.keys(SKILLS); const action = args.action || "install"; const actions = Object.fromEntries(skills.map((skill) => [skill, action])); @@ -129,7 +134,17 @@ async function interactiveAnswers({ dryRun = false } = {}) { const scopeInfo = resolveClientScope(clientId, scope, process.cwd()); if (scopeInfo.packageMode) { console.log(`${clientId}/${scope}: package mode manages the full Pi package bundle; per-skill prompts are skipped.`); - selections.push({ clientId, scope, action: "install", actions: {} }); + const installed = await isPiPackageInstalled({ scope, repoRoot: process.cwd() }); + const choices = "install/update/reinstall/remove/skip"; + const defaultAction = installed ? "skip" : "install"; + const answer = await rl.question(`${clientId}/${scope} package is ${installed ? "installed" : "not-installed"}; action (${choices}) [${defaultAction}]: `); + const chosen = answer.trim() || defaultAction; + if (!choices.split("/").includes(chosen)) { + console.log(`Invalid action '${chosen}', using skip.`); + selections.push({ clientId, scope, action: "skip", actions: {} }); + } else { + selections.push({ clientId, scope, action: chosen, actions: {} }); + } continue; } const installed = await detectInstalledSkills({ clientId, skillsRoot: scopeInfo.skillsRoot }); @@ -261,7 +276,7 @@ async function main() { client: op.clientId, scope: op.scope, item: op.skill || op.helper || op.kind, - action: op.action, + action: op.displayAction || op.action, status: op.status, details: op.details || op.target || "", })); diff --git a/scripts/tests/skill-manager-core.test.mjs b/scripts/tests/skill-manager-core.test.mjs index d8e2167..c90aa54 100644 --- a/scripts/tests/skill-manager-core.test.mjs +++ b/scripts/tests/skill-manager-core.test.mjs @@ -1,8 +1,10 @@ import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import test from "node:test"; +import { fileURLToPath } from "node:url"; import { CLIENTS, @@ -10,10 +12,13 @@ import { buildOperationPlan, detectInstalledSkills, getSkillSource, + piPackageCommand, parseReviewerShorthand, validateRemoveTarget, } from "../lib/skill-manager-core.mjs"; +const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); + test("manifest records supported variants and helper allowlists", () => { assert.deepEqual(SKILLS["web-automation"].variants, ["codex", "claude-code", "opencode", "pi"]); assert.equal(SKILLS["web-automation"].variants.includes("cursor"), false); @@ -100,6 +105,131 @@ test("pi package mode plans full package install instead of per-skill copy", asy } }); +test("pi package mode surfaces bundled skills and skips already installed package resources", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "skill-manager-pi-package-installed-")); + try { + const repo = path.join(dir, "repo"); + await mkdir(path.join(repo, ".pi"), { recursive: true }); + await writeFile(path.join(repo, ".pi", "settings.json"), JSON.stringify({ packages: [".."] })); + for (const skill of Object.keys(SKILLS)) { + await mkdir(path.join(repo, "pi-package", "skills", skill), { recursive: true }); + } + await mkdir(path.join(repo, "pi-package", "skills", "atlassian", "scripts", "node_modules"), { recursive: true }); + await mkdir(path.join(repo, "pi-package", "skills", "web-automation", "scripts", "node_modules", ".bin"), { recursive: true }); + await writeFile(path.join(repo, "pi-package", "skills", "web-automation", "scripts", "node_modules", ".bin", "cloakbrowser"), ""); + + const plan = await buildOperationPlan({ + selections: [{ clientId: "pi", scope: "packageLocal", action: "install", actions: {} }], + repoRoot: repo, + }); + + const packageInstall = plan.operations.find((op) => op.kind === "pi-package"); + assert.equal(packageInstall.status, "skipped"); + assert.match(packageInstall.details, /already installed/); + assert.deepEqual( + plan.reportRows.filter((row) => row.action === "included").map((row) => row.item).sort(), + Object.keys(SKILLS).sort() + ); + assert.deepEqual( + plan.reportRows.filter((row) => row.action === "bootstrap-deps").map((row) => [row.item, row.status]).sort(), + [["atlassian", "skipped"], ["web-automation", "skipped"]] + ); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("pi package mode remove skips when package is not installed", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "skill-manager-pi-package-remove-")); + try { + const plan = await buildOperationPlan({ + selections: [{ clientId: "pi", scope: "packageLocal", action: "remove", actions: {} }], + repoRoot: dir, + }); + + assert.deepEqual(plan.operations.map((op) => op.kind), ["pi-package"]); + assert.equal(plan.operations[0].action, "remove"); + assert.equal(plan.operations[0].status, "skipped"); + assert.match(plan.operations[0].details, /not installed/); + assert.equal(plan.reportRows[0].item, "pi-package"); + assert.equal(plan.reportRows[0].action, "remove"); + assert.equal(plan.reportRows[0].status, "skipped"); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("pi package mode remove plans removal when package is installed", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "skill-manager-pi-package-remove-installed-")); + try { + const repo = path.join(dir, "repo"); + await mkdir(path.join(repo, ".pi"), { recursive: true }); + await writeFile(path.join(repo, ".pi", "settings.json"), JSON.stringify({ packages: [".."] })); + const plan = await buildOperationPlan({ + selections: [{ clientId: "pi", scope: "packageLocal", action: "remove", actions: {} }], + repoRoot: repo, + }); + + assert.deepEqual(plan.operations.map((op) => op.kind), ["pi-package"]); + assert.equal(plan.operations[0].action, "remove"); + assert.equal(plan.operations[0].status, undefined); + assert.equal(plan.reportRows[0].status, "planned"); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("pi package mode update syncs and forces package reinstall plus dependency bootstrap", async () => { + const dir = await mkdtemp(path.join(tmpdir(), "skill-manager-pi-package-update-")); + try { + const repo = path.join(dir, "repo"); + await mkdir(path.join(repo, ".pi"), { recursive: true }); + await writeFile(path.join(repo, ".pi", "settings.json"), JSON.stringify({ packages: [".."] })); + await mkdir(path.join(repo, "pi-package", "skills", "atlassian", "scripts", "node_modules"), { recursive: true }); + await mkdir(path.join(repo, "pi-package", "skills", "web-automation", "scripts", "node_modules", ".bin"), { recursive: true }); + await writeFile(path.join(repo, "pi-package", "skills", "web-automation", "scripts", "node_modules", ".bin", "cloakbrowser"), ""); + + const plan = await buildOperationPlan({ + selections: [{ clientId: "pi", scope: "packageLocal", action: "update", actions: {} }], + repoRoot: repo, + }); + + assert.equal(plan.operations[0].kind, "sync-pi-package"); + const packageUpdate = plan.operations.find((op) => op.kind === "pi-package"); + assert.equal(packageUpdate.action, "update"); + assert.equal(packageUpdate.status, undefined); + assert.deepEqual( + plan.reportRows.filter((row) => row.action === "bootstrap-deps").map((row) => [row.item, row.status]).sort(), + [["atlassian", "planned"], ["web-automation", "planned"]] + ); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("pi package command helper builds exact install and remove argv", () => { + assert.deepEqual(piPackageCommand({ action: "install", repoRoot: "/repo", piInstallArg: "" }), ["pi", ["install", "/repo"]]); + assert.deepEqual(piPackageCommand({ action: "update", repoRoot: "/repo", piInstallArg: "-l" }), ["pi", ["install", "-l", "/repo"]]); + assert.deepEqual(piPackageCommand({ action: "reinstall", repoRoot: "/repo", piInstallArg: "-l" }), ["pi", ["install", "-l", "/repo"]]); + assert.deepEqual(piPackageCommand({ action: "remove", repoRoot: "/repo", piInstallArg: "" }), ["pi", ["remove", "/repo"]]); + assert.deepEqual(piPackageCommand({ action: "remove", repoRoot: "/repo", piInstallArg: "-l" }), ["pi", ["remove", "-l", "/repo"]]); +}); + +test("cli package mode preserves package action and ignores skill narrowing", () => { + const output = execFileSync(process.execPath, [ + path.join(REPO_ROOT, "scripts", "manage-skills.mjs"), + "--client", "pi", + "--scope", "packageGlobal", + "--pi-package", + "--skill", "create-plan", + "--action", "remove", + "--plan-only", + ], { cwd: REPO_ROOT, encoding: "utf8" }); + const plan = JSON.parse(output); + assert.deepEqual(plan.operations.map((op) => op.kind), ["pi-package"]); + assert.equal(plan.operations[0].action, "remove"); +}); + test("validateRemoveTarget rejects paths outside the manifest root", async () => { const dir = await mkdtemp(path.join(tmpdir(), "skill-manager-safe-")); try {