From f1c89b6918e8014038212c88cd05bd1f0674e2f7 Mon Sep 17 00:00:00 2001 From: Abdeslam Yassine Agmar Date: Tue, 24 Feb 2026 08:33:51 +0000 Subject: [PATCH 1/9] fix(npm): run npm via node npm-cli on Windows --- src/packages/discovery.ts | 10 ++++----- src/packages/install.ts | 4 ++-- src/ui/remote.ts | 4 ++-- src/utils/auto-update.ts | 4 ++-- src/utils/npm-exec.ts | 47 +++++++++++++++++++++++++++++++++++++++ test/npm-exec.test.ts | 27 ++++++++++++++++++++++ 6 files changed, 84 insertions(+), 12 deletions(-) create mode 100644 src/utils/npm-exec.ts create mode 100644 test/npm-exec.test.ts diff --git a/src/packages/discovery.ts b/src/packages/discovery.ts index e8eada8..e3ab18d 100644 --- a/src/packages/discovery.ts +++ b/src/packages/discovery.ts @@ -13,6 +13,7 @@ import { CACHE_TTL, TIMEOUTS } from "../constants.js"; import { readSummary } from "../utils/fs.js"; import { parseNpmSource } from "../utils/format.js"; import { getPackageSourceKind, splitGitRepoAndRef } from "../utils/package-source.js"; +import { execNpm } from "../utils/npm-exec.js"; let searchCache: SearchCache | null = null; @@ -67,9 +68,8 @@ export async function searchNpmPackages( ctx.ui.notify(`Searching npm for "${query}"...`, "info"); } - const res = await pi.exec("npm", ["search", "--json", `--searchlimit=${searchLimit}`, query], { + const res = await execNpm(pi, ["search", "--json", `--searchlimit=${searchLimit}`, query], ctx, { timeout: TIMEOUTS.npmSearch, - cwd: ctx.cwd, }); if (res.code !== 0) { @@ -375,9 +375,8 @@ async function fetchPackageSize( try { // Try to get unpacked size from npm view - const res = await pi.exec("npm", ["view", pkgName, "dist.unpackedSize", "--json"], { + const res = await execNpm(pi, ["view", pkgName, "dist.unpackedSize", "--json"], ctx, { timeout: TIMEOUTS.npmView, - cwd: ctx.cwd, }); if (res.code === 0) { try { @@ -441,9 +440,8 @@ async function addPackageMetadata( if (cached?.description) { pkg.description = cached.description; } else { - const res = await pi.exec("npm", ["view", pkgName, "description", "--json"], { + const res = await execNpm(pi, ["view", pkgName, "description", "--json"], ctx, { timeout: TIMEOUTS.npmView, - cwd: ctx.cwd, }); if (res.code === 0) { try { diff --git a/src/packages/install.ts b/src/packages/install.ts index c00ccd6..ed5d8b6 100644 --- a/src/packages/install.ts +++ b/src/packages/install.ts @@ -16,6 +16,7 @@ import { notify, error as notifyError, success } from "../utils/notify.js"; import { confirmAction, confirmReload, showProgress } from "../utils/ui-helpers.js"; import { tryOperation } from "../utils/mode.js"; import { updateExtmgrStatus } from "../utils/status.js"; +import { execNpm } from "../utils/npm-exec.js"; import { TIMEOUTS } from "../constants.js"; export type InstallScope = "global" | "project"; @@ -291,9 +292,8 @@ export async function installPackageLocally( await mkdir(extensionDir, { recursive: true }); showProgress(ctx, "Fetching", packageName); - const viewRes = await pi.exec("npm", ["view", packageName, "--json"], { + const viewRes = await execNpm(pi, ["view", packageName, "--json"], ctx, { timeout: TIMEOUTS.fetchPackageInfo, - cwd: ctx.cwd, }); if (viewRes.code !== 0) { diff --git a/src/ui/remote.ts b/src/ui/remote.ts index dee920f..7544e9e 100644 --- a/src/ui/remote.ts +++ b/src/ui/remote.ts @@ -15,6 +15,7 @@ import { isCacheValid, } from "../packages/discovery.js"; import { installPackage, installPackageLocally } from "../packages/install.js"; +import { execNpm } from "../utils/npm-exec.js"; import { notify } from "../utils/notify.js"; interface PackageInfoCacheEntry { @@ -143,9 +144,8 @@ async function buildPackageInfoText( } const [infoRes, weeklyDownloads] = await Promise.all([ - pi.exec("npm", ["view", packageName, "--json"], { + execNpm(pi, ["view", packageName, "--json"], ctx, { timeout: TIMEOUTS.npmView, - cwd: ctx.cwd, }), fetchWeeklyDownloads(packageName), ]); diff --git a/src/utils/auto-update.ts b/src/utils/auto-update.ts index 9679e45..930ac4e 100644 --- a/src/utils/auto-update.ts +++ b/src/utils/auto-update.ts @@ -18,6 +18,7 @@ import { type AutoUpdateConfig, } from "./settings.js"; import { parseNpmSource } from "./format.js"; +import { execNpm } from "./npm-exec.js"; import { TIMEOUTS } from "../constants.js"; import { startTimer, stopTimer, isTimerRunning } from "./timer.js"; @@ -134,9 +135,8 @@ async function checkPackageUpdate( if (!pkgName) return false; try { - const res = await pi.exec("npm", ["view", pkgName, "version", "--json"], { + const res = await execNpm(pi, ["view", pkgName, "version", "--json"], ctx, { timeout: TIMEOUTS.npmView, - cwd: ctx.cwd, }); if (res.code !== 0) return false; diff --git a/src/utils/npm-exec.ts b/src/utils/npm-exec.ts new file mode 100644 index 0000000..28c5b1d --- /dev/null +++ b/src/utils/npm-exec.ts @@ -0,0 +1,47 @@ +import path from "node:path"; +import { execPath, platform } from "node:process"; +import type { ExtensionAPI } from "@mariozechner/pi-coding-agent"; + +interface NpmCommandResolutionOptions { + platform?: NodeJS.Platform; + nodeExecPath?: string; +} + +interface NpmExecOptions { + timeout: number; +} + +function getNpmCliPath(nodeExecPath: string, runtimePlatform: NodeJS.Platform): string { + const pathImpl = runtimePlatform === "win32" ? path.win32 : path; + return pathImpl.join(pathImpl.dirname(nodeExecPath), "node_modules", "npm", "bin", "npm-cli.js"); +} + +export function resolveNpmCommand( + npmArgs: string[], + options?: NpmCommandResolutionOptions +): { command: string; args: string[] } { + const runtimePlatform = options?.platform ?? platform; + + if (runtimePlatform === "win32") { + const nodeBinary = options?.nodeExecPath ?? execPath; + return { + command: nodeBinary, + args: [getNpmCliPath(nodeBinary, runtimePlatform), ...npmArgs], + }; + } + + return { command: "npm", args: npmArgs }; +} + +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); + return pi.exec(resolved.command, resolved.args, { + timeout: options.timeout, + cwd: ctx.cwd, + }); +} diff --git a/test/npm-exec.test.ts b/test/npm-exec.test.ts new file mode 100644 index 0000000..73518ba --- /dev/null +++ b/test/npm-exec.test.ts @@ -0,0 +1,27 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { resolveNpmCommand } from "../src/utils/npm-exec.js"; + +void test("resolveNpmCommand uses npm directly on non-windows", () => { + const resolved = resolveNpmCommand(["view", "pi-extmgr", "version", "--json"], { + platform: "linux", + }); + + assert.equal(resolved.command, "npm"); + assert.deepEqual(resolved.args, ["view", "pi-extmgr", "version", "--json"]); +}); + +void test("resolveNpmCommand uses node + npm-cli.js on windows", () => { + const resolved = resolveNpmCommand(["search", "--json", "pi-extmgr"], { + platform: "win32", + nodeExecPath: "C:\\Program Files\\nodejs\\node.exe", + }); + + assert.equal(resolved.command, "C:\\Program Files\\nodejs\\node.exe"); + assert.deepEqual(resolved.args, [ + "C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js", + "search", + "--json", + "pi-extmgr", + ]); +}); From 106bdc5d2b0ae821be4705bc5ce894bf5fc1191b Mon Sep 17 00:00:00 2001 From: Abdeslam Yassine Agmar Date: Tue, 24 Feb 2026 08:39:56 +0000 Subject: [PATCH 2/9] fix(auto-update): stop stale timers on session switches --- src/index.ts | 7 +++- test/auto-update.test.ts | 88 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/src/index.ts b/src/index.ts index 087f95f..7da52e3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -70,12 +70,17 @@ export default function extensionsManager(pi: ExtensionAPI) { // Restore persisted auto-update config into session entries so sync lookups are valid. await hydrateAutoUpdateConfig(pi, ctx); - if (!ctx.hasUI) return; + if (!ctx.hasUI) { + stopAutoUpdateTimer(); + return; + } const config = getAutoUpdateConfig(ctx); if (config.enabled && config.intervalMs > 0) { const getCtx: ContextProvider = () => ctx; startAutoUpdateTimer(pi, getCtx, createAutoUpdateNotificationHandler(ctx)); + } else { + stopAutoUpdateTimer(); } setImmediate(() => { diff --git a/test/auto-update.test.ts b/test/auto-update.test.ts index d61fc51..dfdc689 100644 --- a/test/auto-update.test.ts +++ b/test/auto-update.test.ts @@ -1,6 +1,12 @@ import test from "node:test"; import assert from "node:assert/strict"; -import { checkForUpdates } from "../src/utils/auto-update.js"; +import type { ExtensionAPI } from "@mariozechner/pi-coding-agent"; +import extensionsManager from "../src/index.js"; +import { + checkForUpdates, + isAutoUpdateRunning, + stopAutoUpdateTimer, +} from "../src/utils/auto-update.js"; import { parseDuration } from "../src/utils/settings.js"; import { createMockHarness, type ExecResult } from "./helpers/mocks.js"; @@ -85,3 +91,83 @@ void test("checkForUpdates handles scoped npm packages", async () => { assert.deepEqual(updates, ["@scope/demo-pkg"]); assert.ok(npmViewCalls.includes("@scope/demo-pkg")); }); + +void test("session switch to disabled auto-update stops existing timer", async () => { + interface SessionCtx { + hasUI: true; + cwd: string; + ui: { + notify: (message: string, level?: string) => void; + setStatus: (key: string, value: string | undefined) => void; + theme: { fg: (name: string, text: string) => string }; + }; + sessionManager: { + getEntries: () => { type: "custom"; customType: string; data: unknown }[]; + }; + } + + const handlers: Record Promise) | undefined> = + {}; + + const pi = { + registerCommand: () => undefined, + on: (event: string, handler: (event: unknown, ctx: SessionCtx) => Promise) => { + handlers[event] = handler; + }, + exec: (command: string, args: string[]) => + Promise.resolve( + command === "pi" && args[0] === "list" + ? { code: 0, stdout: "No packages installed", stderr: "", killed: false } + : { code: 0, stdout: "", stderr: "", killed: false } + ), + appendEntry: () => undefined, + }; + + const ui: SessionCtx["ui"] = { + notify: () => undefined, + setStatus: () => undefined, + theme: { fg: (_name: string, text: string) => text }, + }; + + const enabledCtx: SessionCtx = { + hasUI: true, + cwd: "/tmp", + ui, + sessionManager: { + getEntries: () => [ + { + type: "custom", + customType: "extmgr-auto-update", + data: { enabled: true, intervalMs: 60 * 60 * 1000, displayText: "1 hour" }, + }, + ], + }, + }; + + const disabledCtx: SessionCtx = { + hasUI: true, + cwd: "/tmp", + ui, + sessionManager: { + getEntries: () => [ + { + type: "custom", + customType: "extmgr-auto-update", + data: { enabled: false, intervalMs: 0, displayText: "off" }, + }, + ], + }, + }; + + try { + extensionsManager(pi as unknown as ExtensionAPI); + + await handlers["session_start"]?.({}, enabledCtx); + assert.equal(isAutoUpdateRunning(), true); + + await handlers["session_switch"]?.({}, disabledCtx); + assert.equal(isAutoUpdateRunning(), false); + } finally { + stopAutoUpdateTimer(); + } +}); From 32d116456e1586fca3493472ca1bc357c9e79569 Mon Sep 17 00:00:00 2001 From: Abdeslam Yassine Agmar Date: Tue, 24 Feb 2026 08:40:20 +0000 Subject: [PATCH 3/9] fix(parser): support quoted args and preserve UNC paths --- src/utils/command.ts | 72 +++++++++++++++++++++++++++++++++- test/command-tokenizer.test.ts | 44 +++++++++++++++++++++ 2 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 test/command-tokenizer.test.ts diff --git a/src/utils/command.ts b/src/utils/command.ts index c221b06..101d6fe 100644 --- a/src/utils/command.ts +++ b/src/utils/command.ts @@ -3,7 +3,77 @@ */ export function tokenizeArgs(input: string): string[] { - return input.trim().split(/\s+/).filter(Boolean); + const tokens: string[] = []; + let current = ""; + let inSingleQuote = false; + let inDoubleQuote = false; + + const pushCurrent = () => { + if (current.length > 0) { + tokens.push(current); + current = ""; + } + }; + + for (let i = 0; i < input.length; i++) { + const char = input[i]!; + const next = input[i + 1]; + + if (inSingleQuote) { + if (char === "'") { + inSingleQuote = false; + } else { + current += char; + } + continue; + } + + if (inDoubleQuote) { + if (char === '"') { + inDoubleQuote = false; + continue; + } + + if (char === "\\" && next === '"') { + current += next; + i++; + continue; + } + + current += char; + continue; + } + + if (/\s/.test(char)) { + pushCurrent(); + continue; + } + + if (char === "'") { + inSingleQuote = true; + continue; + } + + if (char === '"') { + inDoubleQuote = true; + continue; + } + + if (char === "\\" && (next === '"' || next === "'" || /\s/.test(next ?? ""))) { + if (next) { + current += next; + i++; + } else { + current += char; + } + continue; + } + + current += char; + } + + pushCurrent(); + return tokens; } export function splitCommandArgs(input: string): { subcommand: string; args: string[] } { diff --git a/test/command-tokenizer.test.ts b/test/command-tokenizer.test.ts new file mode 100644 index 0000000..28ce458 --- /dev/null +++ b/test/command-tokenizer.test.ts @@ -0,0 +1,44 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { tokenizeArgs } from "../src/utils/command.js"; + +void test("tokenizeArgs preserves legacy whitespace splitting", () => { + assert.deepEqual(tokenizeArgs("install npm:pi-extmgr --project"), [ + "install", + "npm:pi-extmgr", + "--project", + ]); +}); + +void test("tokenizeArgs supports quoted values", () => { + assert.deepEqual(tokenizeArgs('install "./extensions/My Cool Extension" --project'), [ + "install", + "./extensions/My Cool Extension", + "--project", + ]); + + assert.deepEqual(tokenizeArgs("install 'git@github.com:user/my repo.git'"), [ + "install", + "git@github.com:user/my repo.git", + ]); +}); + +void test("tokenizeArgs keeps windows paths intact", () => { + assert.deepEqual(tokenizeArgs('install "C:\\Users\\Aya\\Pi Extensions\\ext.ts" --global'), [ + "install", + "C:\\Users\\Aya\\Pi Extensions\\ext.ts", + "--global", + ]); + + assert.deepEqual(tokenizeArgs('install "\\\\server\\share\\ext.ts" --global'), [ + "install", + "\\\\server\\share\\ext.ts", + "--global", + ]); + + assert.deepEqual(tokenizeArgs("install \\\\server\\share\\ext.ts --global"), [ + "install", + "\\\\server\\share\\ext.ts", + "--global", + ]); +}); From 1e6141de7d35904efdf4d711182cd00ab6068fd6 Mon Sep 17 00:00:00 2001 From: Abdeslam Yassine Agmar Date: Tue, 24 Feb 2026 08:40:51 +0000 Subject: [PATCH 4/9] fix(packages): normalize source handling and update state --- src/packages/discovery.ts | 11 +- src/packages/management.ts | 39 ++++- src/utils/format.ts | 24 ++- src/utils/package-source.ts | 4 + test/discovery-parser.test.ts | 42 ++++++ test/install-remove.test.ts | 269 +++++++++++++++++++++++++++++++++- 6 files changed, 381 insertions(+), 8 deletions(-) diff --git a/src/packages/discovery.ts b/src/packages/discovery.ts index e3ab18d..93e1893 100644 --- a/src/packages/discovery.ts +++ b/src/packages/discovery.ts @@ -118,7 +118,16 @@ function sanitizeListSourceSuffix(source: string): string { } function normalizeSourceIdentity(source: string): string { - return sanitizeListSourceSuffix(source).replace(/\\/g, "/").toLowerCase(); + const sanitized = sanitizeListSourceSuffix(source).replace(/\\/g, "/"); + const kind = getPackageSourceKind(sanitized); + + if (kind === "local") { + const isWindowsPath = + /^[a-zA-Z]:\//.test(sanitized) || sanitized.startsWith("//") || source.includes("\\"); + return isWindowsPath ? sanitized.toLowerCase() : sanitized; + } + + return sanitized.toLowerCase(); } function isScopeHeader(lowerTrimmed: string, scope: "global" | "project"): boolean { diff --git a/src/packages/management.ts b/src/packages/management.ts index 34b8f20..7cee211 100644 --- a/src/packages/management.ts +++ b/src/packages/management.ts @@ -33,12 +33,18 @@ const NO_PACKAGE_MUTATION_OUTCOME: PackageMutationOutcome = { reloaded: false, }; +const BULK_UPDATE_LABEL = "all packages"; + function packageMutationOutcome( overrides: Partial ): PackageMutationOutcome { return { ...NO_PACKAGE_MUTATION_OUTCOME, ...overrides }; } +function isUpToDateOutput(stdout: string): boolean { + return /already\s+up\s+to\s+date/i.test(stdout) || /\bpinned\b/i.test(stdout); +} + async function updatePackageInternal( source: string, ctx: ExtensionCommandContext, @@ -60,9 +66,10 @@ async function updatePackageInternal( } const stdout = res.stdout || ""; - if (stdout.includes("already up to date") || stdout.includes("pinned")) { + if (isUpToDateOutput(stdout)) { notify(ctx, `${source} is already up to date (or pinned).`, "info"); logPackageUpdate(pi, source, source, undefined, true); + clearUpdatesAvailable(pi, ctx); void updateExtmgrStatus(ctx, pi); return NO_PACKAGE_MUTATION_OUTCOME; } @@ -87,18 +94,23 @@ async function updatePackagesInternal( const res = await pi.exec("pi", ["update"], { timeout: TIMEOUTS.packageUpdateAll, cwd: ctx.cwd }); if (res.code !== 0) { - notifyError(ctx, `Update failed: ${res.stderr || res.stdout || `exit ${res.code}`}`); + const errorMsg = `Update failed: ${res.stderr || res.stdout || `exit ${res.code}`}`; + logPackageUpdate(pi, BULK_UPDATE_LABEL, BULK_UPDATE_LABEL, undefined, false, errorMsg); + notifyError(ctx, errorMsg); void updateExtmgrStatus(ctx, pi); return NO_PACKAGE_MUTATION_OUTCOME; } const stdout = res.stdout || ""; - if (stdout.includes("already up to date") || stdout.trim() === "") { + if (isUpToDateOutput(stdout) || stdout.trim() === "") { notify(ctx, "All packages are already up to date.", "info"); + logPackageUpdate(pi, BULK_UPDATE_LABEL, BULK_UPDATE_LABEL, undefined, true); + clearUpdatesAvailable(pi, ctx); void updateExtmgrStatus(ctx, pi); return NO_PACKAGE_MUTATION_OUTCOME; } + logPackageUpdate(pi, BULK_UPDATE_LABEL, BULK_UPDATE_LABEL, undefined, true); success(ctx, "Packages updated"); clearUpdatesAvailable(pi, ctx); @@ -139,6 +151,14 @@ export async function updatePackagesWithOutcome( return updatePackagesInternal(ctx, pi); } +function normalizeLocalSourceIdentity(source: string): string { + const normalized = source.replace(/\\/g, "/"); + const looksWindowsPath = + /^[a-zA-Z]:\//.test(normalized) || normalized.startsWith("//") || source.includes("\\"); + + return looksWindowsPath ? normalized.toLowerCase() : normalized; +} + function packageIdentity(source: string, fallbackName?: string): string { const npm = parseNpmSource(source); if (npm?.name) { @@ -151,6 +171,10 @@ function packageIdentity(source: string, fallbackName?: string): string { return `git:${repo}`; } + if (getPackageSourceKind(source) === "local") { + return `src:${normalizeLocalSourceIdentity(source)}`; + } + if (fallbackName) { return `name:${fallbackName}`; } @@ -338,8 +362,10 @@ async function removePackageInternal( clearUpdatesAvailable(pi, ctx); } + const successfulRemovalCount = targets.length - failures.length; + // Wait for selected targets to disappear from their target scopes before reloading. - if (failures.length === 0 && targets.length > 0) { + if (successfulRemovalCount > 0 && failures.length === 0 && targets.length > 0) { notify(ctx, "Waiting for removal to complete...", "info"); const isRemoved = await waitForCondition( async () => { @@ -360,6 +386,11 @@ async function removePackageInternal( } } + if (successfulRemovalCount === 0) { + void updateExtmgrStatus(ctx, pi); + return NO_PACKAGE_MUTATION_OUTCOME; + } + const reloaded = await confirmReload(ctx, "Removal complete."); if (!reloaded) { void updateExtmgrStatus(ctx, pi); diff --git a/src/utils/format.ts b/src/utils/format.ts index 0036e77..e5fa3f9 100644 --- a/src/utils/format.ts +++ b/src/utils/format.ts @@ -59,6 +59,9 @@ export function formatBytes(bytes: number): string { const GIT_PATTERNS = { gitPrefix: /^git:/, + gitPlusHttpPrefix: /^git\+https?:\/\//, + gitPlusSshPrefix: /^git\+ssh:\/\//, + gitPlusGitPrefix: /^git\+git:\/\//, httpPrefix: /^https?:\/\//, sshPrefix: /^ssh:\/\//, gitProtoPrefix: /^git:\/\//, @@ -78,6 +81,9 @@ const LOCAL_PATH_PATTERNS = { function isGitLikeSource(source: string): boolean { return ( GIT_PATTERNS.gitPrefix.test(source) || + GIT_PATTERNS.gitPlusHttpPrefix.test(source) || + GIT_PATTERNS.gitPlusSshPrefix.test(source) || + GIT_PATTERNS.gitPlusGitPrefix.test(source) || GIT_PATTERNS.httpPrefix.test(source) || GIT_PATTERNS.sshPrefix.test(source) || GIT_PATTERNS.gitProtoPrefix.test(source) || @@ -97,15 +103,29 @@ function isLocalPathSource(source: string): boolean { ); } +function unwrapQuotedSource(source: string): string { + const trimmed = source.trim(); + if (trimmed.length < 2) return trimmed; + + const first = trimmed[0]; + const last = trimmed[trimmed.length - 1]; + + if ((first === '"' && last === '"') || (first === "'" && last === "'")) { + return trimmed.slice(1, -1).trim(); + } + + return trimmed; +} + export function isPackageSource(str: string): boolean { - const source = str.trim(); + const source = unwrapQuotedSource(str); if (!source) return false; return source.startsWith("npm:") || isGitLikeSource(source) || isLocalPathSource(source); } export function normalizePackageSource(source: string): string { - const trimmed = source.trim(); + const trimmed = unwrapQuotedSource(source); if (!trimmed) return trimmed; if (GIT_PATTERNS.gitSsh.test(trimmed)) { diff --git a/src/utils/package-source.ts b/src/utils/package-source.ts index 3f1fefc..04efe65 100644 --- a/src/utils/package-source.ts +++ b/src/utils/package-source.ts @@ -18,6 +18,10 @@ export function getPackageSourceKind(source: string): PackageSourceKind { if ( normalized.startsWith("git:") || + normalized.startsWith("git+http://") || + normalized.startsWith("git+https://") || + normalized.startsWith("git+ssh://") || + normalized.startsWith("git+git://") || normalized.startsWith("http://") || normalized.startsWith("https://") || normalized.startsWith("ssh://") || diff --git a/test/discovery-parser.test.ts b/test/discovery-parser.test.ts index 01c4c11..1dbd245 100644 --- a/test/discovery-parser.test.ts +++ b/test/discovery-parser.test.ts @@ -120,6 +120,25 @@ void test("isSourceInstalled supports scope-aware checks", async () => { ); }); +void test("isSourceInstalled keeps case-sensitive local paths distinct", async () => { + const { pi, ctx } = createMockHarness({ + execImpl: (command, args) => { + if (command === "pi" && args[0] === "list") { + return { + code: 0, + stdout: "Global:\n /opt/extensions/Foo/index.ts\n", + stderr: "", + killed: false, + }; + } + return { code: 1, stdout: "", stderr: "unexpected", killed: false }; + }, + }); + + assert.equal(await isSourceInstalled("/opt/extensions/Foo/index.ts", ctx, pi), true); + assert.equal(await isSourceInstalled("/opt/extensions/foo/index.ts", ctx, pi), false); +}); + void test("parseInstalledPackagesOutput parses ssh git sources", () => { const input = ` Global: @@ -164,6 +183,17 @@ Global: assert.equal(result[0]?.name, "super-ext"); }); +void test("parseInstalledPackagesOutput parses git:// sources", () => { + const input = ` +Global: + git://github.com/user/super-ext.git +`; + + const result = parseInstalledPackagesOutput(input); + assert.equal(result.length, 1); + assert.equal(result[0]?.name, "super-ext"); +}); + void test("normalizePackageSource preserves git and local path sources", () => { assert.equal( normalizePackageSource("git@github.com:user/repo.git"), @@ -173,14 +203,24 @@ void test("normalizePackageSource preserves git and local path sources", () => { normalizePackageSource("ssh://git@github.com/user/repo.git"), "ssh://git@github.com/user/repo.git" ); + assert.equal( + normalizePackageSource("git+https://github.com/user/repo.git"), + "git+https://github.com/user/repo.git" + ); assert.equal(normalizePackageSource("~/dev/ext"), "~/dev/ext"); assert.equal(normalizePackageSource(".\\extensions\\demo"), ".\\extensions\\demo"); + assert.equal( + normalizePackageSource('"./extensions/My Cool Extension.ts"'), + "./extensions/My Cool Extension.ts" + ); + assert.equal(normalizePackageSource("'@scope/pkg'"), "npm:@scope/pkg"); assert.equal(normalizePackageSource("@scope/pkg"), "npm:@scope/pkg"); }); void test("isPackageSource recognizes git ssh and local path sources", () => { assert.equal(isPackageSource("git@github.com:user/repo.git"), true); assert.equal(isPackageSource("ssh://git@github.com/user/repo.git"), true); + assert.equal(isPackageSource("git+https://github.com/user/repo.git"), true); assert.equal(isPackageSource("~/dev/ext"), true); assert.equal(isPackageSource(".\\extensions\\demo"), true); assert.equal(isPackageSource("pi-extmgr"), false); @@ -200,6 +240,8 @@ void test("getPackageSourceKind classifies npm/git/local sources", () => { assert.equal(getPackageSourceKind("npm:pi-extmgr"), "npm"); assert.equal(getPackageSourceKind("git:https://github.com/user/repo.git@main"), "git"); assert.equal(getPackageSourceKind("https://github.com/user/repo@main"), "git"); + assert.equal(getPackageSourceKind("git+https://github.com/user/repo.git"), "git"); + assert.equal(getPackageSourceKind("git://github.com/user/repo.git"), "git"); assert.equal(getPackageSourceKind("git@github.com:user/repo"), "git"); assert.equal(getPackageSourceKind("./vendor/demo"), "local"); assert.equal(getPackageSourceKind(".\\vendor\\demo"), "local"); diff --git a/test/install-remove.test.ts b/test/install-remove.test.ts index 08b403d..30f09f9 100644 --- a/test/install-remove.test.ts +++ b/test/install-remove.test.ts @@ -1,7 +1,7 @@ import test from "node:test"; import assert from "node:assert/strict"; import { installPackage } from "../src/packages/install.js"; -import { removePackage } from "../src/packages/management.js"; +import { removePackage, updatePackage, updatePackages } from "../src/packages/management.js"; import { createMockHarness } from "./helpers/mocks.js"; void test("installPackage calls pi install with normalized npm source", async () => { @@ -35,3 +35,270 @@ void test("removePackage calls pi remove", async () => { assert.equal(removeCalls.length, 1); assert.deepEqual(removeCalls[0]?.args, ["remove", "npm:pi-extmgr"]); }); + +void test("removePackage does not request reload when removal fails", async () => { + const output: string[] = []; + const originalLog = console.log; + console.log = (...args: unknown[]) => { + output.push(args.map(String).join(" ")); + }; + + try { + const { pi, ctx } = createMockHarness({ + execImpl: (command, args) => { + if (command === "pi" && args[0] === "list") { + return { + code: 0, + stdout: "Global:\n npm:pi-extmgr\n", + stderr: "", + killed: false, + }; + } + + if (command === "pi" && args[0] === "remove") { + return { + code: 1, + stdout: "", + stderr: "permission denied", + killed: false, + }; + } + + return { code: 0, stdout: "", stderr: "", killed: false }; + }, + }); + + await removePackage("npm:pi-extmgr", ctx, pi); + } finally { + console.log = originalLog; + } + + assert.equal( + output.some((line) => line.includes("Reload pi to apply changes. (Removal complete.)")), + false + ); +}); + +void test("removePackage targets exact local source when names collide", async () => { + const installed = ["/opt/extensions/alpha/index.ts", "/opt/extensions/beta/index.ts"]; + + const { pi, ctx, calls } = createMockHarness({ + execImpl: (command, args) => { + if (command === "pi" && args[0] === "list") { + const lines = ["Global:", ...installed.map((source) => ` ${source}`), ""]; + return { + code: 0, + stdout: lines.join("\n"), + stderr: "", + killed: false, + }; + } + + if (command === "pi" && args[0] === "remove") { + const source = args[1]; + const index = installed.indexOf(source ?? ""); + if (index >= 0) installed.splice(index, 1); + return { code: 0, stdout: "Removed", stderr: "", killed: false }; + } + + return { code: 0, stdout: "", stderr: "", killed: false }; + }, + }); + + await removePackage("/opt/extensions/beta/index.ts", ctx, pi); + + const removeCalls = calls.filter((c) => c.command === "pi" && c.args[0] === "remove"); + assert.equal(removeCalls.length, 1); + assert.deepEqual(removeCalls[0]?.args, ["remove", "/opt/extensions/beta/index.ts"]); +}); + +void test("removePackage keeps case-sensitive local paths distinct", async () => { + const installed = ["/opt/extensions/Foo/index.ts", "/opt/extensions/foo/index.ts"]; + + const { pi, ctx, calls } = createMockHarness({ + execImpl: (command, args) => { + if (command === "pi" && args[0] === "list") { + const lines = ["Global:", ...installed.map((source) => ` ${source}`), ""]; + return { + code: 0, + stdout: lines.join("\n"), + stderr: "", + killed: false, + }; + } + + if (command === "pi" && args[0] === "remove") { + const source = args[1]; + const index = installed.indexOf(source ?? ""); + if (index >= 0) installed.splice(index, 1); + return { code: 0, stdout: "Removed", stderr: "", killed: false }; + } + + return { code: 0, stdout: "", stderr: "", killed: false }; + }, + }); + + await removePackage("/opt/extensions/foo/index.ts", ctx, pi); + + const removeCalls = calls.filter((c) => c.command === "pi" && c.args[0] === "remove"); + assert.equal(removeCalls.length, 1); + assert.deepEqual(removeCalls[0]?.args, ["remove", "/opt/extensions/foo/index.ts"]); +}); + +void test("updatePackage treats case-variant already-up-to-date output as no-op", async () => { + const output: string[] = []; + const originalLog = console.log; + console.log = (...args: unknown[]) => { + output.push(args.map(String).join(" ")); + }; + + let autoUpdateEntries: unknown[] = []; + + try { + const { pi, ctx, entries } = createMockHarness({ + execImpl: (command, args) => { + if (command === "pi" && args[0] === "update") { + return { + code: 0, + stdout: "Already up to date", + stderr: "", + killed: false, + }; + } + + if (command === "pi" && args[0] === "list") { + return { code: 0, stdout: "No packages installed", stderr: "", killed: false }; + } + + return { code: 0, stdout: "", stderr: "", killed: false }; + }, + }); + + entries.push({ + type: "custom", + customType: "extmgr-auto-update", + data: { + enabled: true, + intervalMs: 60 * 60 * 1000, + displayText: "1 hour", + updatesAvailable: ["pi-extmgr"], + }, + }); + + await updatePackage("npm:pi-extmgr", ctx, pi); + + autoUpdateEntries = entries + .filter((entry) => entry.customType === "extmgr-auto-update") + .map((entry) => entry.data); + } finally { + console.log = originalLog; + } + + assert.equal( + output.some((line) => line.includes("Reload pi to apply changes. (Package updated.)")), + false + ); + + const latestAutoUpdate = autoUpdateEntries[autoUpdateEntries.length - 1] as + | { updatesAvailable?: string[] } + | undefined; + assert.deepEqual(latestAutoUpdate?.updatesAvailable ?? [], []); +}); + +void test("updatePackages treats case-variant already-up-to-date output as no-op", async () => { + const output: string[] = []; + const originalLog = console.log; + console.log = (...args: unknown[]) => { + output.push(args.map(String).join(" ")); + }; + + let autoUpdateEntries: unknown[] = []; + let historyEntries: unknown[] = []; + + try { + const { pi, ctx, entries } = createMockHarness({ + execImpl: (command, args) => { + if (command === "pi" && args[0] === "update") { + return { + code: 0, + stdout: "All packages are Already Up To Date", + stderr: "", + killed: false, + }; + } + + if (command === "pi" && args[0] === "list") { + return { code: 0, stdout: "No packages installed", stderr: "", killed: false }; + } + + return { code: 0, stdout: "", stderr: "", killed: false }; + }, + }); + + entries.push({ + type: "custom", + customType: "extmgr-auto-update", + data: { + enabled: true, + intervalMs: 60 * 60 * 1000, + displayText: "1 hour", + updatesAvailable: ["pi-extmgr"], + }, + }); + + await updatePackages(ctx, pi); + + autoUpdateEntries = entries + .filter((entry) => entry.customType === "extmgr-auto-update") + .map((entry) => entry.data); + historyEntries = entries + .filter((entry) => entry.customType === "extmgr-change") + .map((entry) => entry.data); + } finally { + console.log = originalLog; + } + + assert.equal( + output.some((line) => line.includes("Reload pi to apply changes. (Packages updated.)")), + false + ); + + const latestAutoUpdate = autoUpdateEntries[autoUpdateEntries.length - 1] as + | { updatesAvailable?: string[] } + | undefined; + assert.deepEqual(latestAutoUpdate?.updatesAvailable ?? [], []); + + const latestHistory = historyEntries[historyEntries.length - 1] as + | { action?: string; success?: boolean; packageName?: string } + | undefined; + assert.equal(latestHistory?.action, "package_update"); + assert.equal(latestHistory?.success, true); + assert.equal(latestHistory?.packageName, "all packages"); +}); + +void test("updatePackages logs failure in history", async () => { + const { pi, ctx, entries } = createMockHarness({ + execImpl: (command, args) => { + if (command === "pi" && args[0] === "update") { + return { code: 1, stdout: "", stderr: "network timeout", killed: false }; + } + + return { code: 0, stdout: "", stderr: "", killed: false }; + }, + }); + + await updatePackages(ctx, pi); + + const historyEntries = entries + .filter((entry) => entry.customType === "extmgr-change") + .map((entry) => entry.data); + + const latestHistory = historyEntries[historyEntries.length - 1] as + | { action?: string; success?: boolean; packageName?: string; error?: string } + | undefined; + + assert.equal(latestHistory?.action, "package_update"); + assert.equal(latestHistory?.success, false); + assert.equal(latestHistory?.packageName, "all packages"); + assert.match(latestHistory?.error ?? "", /network timeout/i); +}); From 08ea8d36a39e797406dee3b4decaa37c243defdb Mon Sep 17 00:00:00 2001 From: Abdeslam Yassine Agmar Date: Tue, 24 Feb 2026 08:42:03 +0000 Subject: [PATCH 5/9] fix(unified): preserve POSIX case in duplicate path checks --- src/ui/unified.ts | 6 +++++- test/unified-items.test.ts | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/ui/unified.ts b/src/ui/unified.ts index 6452fa9..1fa55ad 100644 --- a/src/ui/unified.ts +++ b/src/ui/unified.ts @@ -263,7 +263,11 @@ async function showInteractiveOnce( } function normalizePathForDuplicateCheck(value: string): string { - return value.replace(/\\/g, "/").toLowerCase(); + const normalized = value.replace(/\\/g, "/"); + const looksWindowsPath = + /^[a-zA-Z]:\//.test(normalized) || normalized.startsWith("//") || value.includes("\\"); + + return looksWindowsPath ? normalized.toLowerCase() : normalized; } export function buildUnifiedItems( diff --git a/test/unified-items.test.ts b/test/unified-items.test.ts index a30272b..51674ac 100644 --- a/test/unified-items.test.ts +++ b/test/unified-items.test.ts @@ -89,6 +89,26 @@ void test("buildUnifiedItems omits duplicate package rows with mixed path separa assert.equal(items[0]?.type, "local"); }); +void test("buildUnifiedItems keeps case-sensitive POSIX paths distinct", () => { + const localEntries = [createLocalEntry("/opt/extensions/Foo/index.ts", "Foo/index.ts")]; + const installedPackages: InstalledPackage[] = [ + { + source: "npm:foo", + name: "foo", + scope: "global", + resolvedPath: "/opt/extensions/foo", + }, + ]; + + const items = buildUnifiedItems(localEntries, installedPackages, new Set()); + + assert.equal(items.length, 2); + assert.deepEqual( + items.map((item) => item.type), + ["local", "package"] + ); +}); + void test("integration: pi list fixture with single-entry npm packages renders package rows once", async () => { const cwd = await mkdtemp(join(tmpdir(), "pi-extmgr-unified-")); From 54a9aa6226c6279c8bc574705672aba37211f98b Mon Sep 17 00:00:00 2001 From: Abdeslam Yassine Agmar Date: Tue, 24 Feb 2026 13:52:54 +0000 Subject: [PATCH 6/9] fix(parser): preserve empty quoted arguments --- src/utils/command.ts | 8 +++++++- test/command-tokenizer.test.ts | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/utils/command.ts b/src/utils/command.ts index 101d6fe..0243ae2 100644 --- a/src/utils/command.ts +++ b/src/utils/command.ts @@ -7,11 +7,13 @@ export function tokenizeArgs(input: string): string[] { let current = ""; let inSingleQuote = false; let inDoubleQuote = false; + let tokenStarted = false; const pushCurrent = () => { - if (current.length > 0) { + if (tokenStarted) { tokens.push(current); current = ""; + tokenStarted = false; } }; @@ -51,15 +53,18 @@ export function tokenizeArgs(input: string): string[] { if (char === "'") { inSingleQuote = true; + tokenStarted = true; continue; } if (char === '"') { inDoubleQuote = true; + tokenStarted = true; continue; } if (char === "\\" && (next === '"' || next === "'" || /\s/.test(next ?? ""))) { + tokenStarted = true; if (next) { current += next; i++; @@ -69,6 +74,7 @@ export function tokenizeArgs(input: string): string[] { continue; } + tokenStarted = true; current += char; } diff --git a/test/command-tokenizer.test.ts b/test/command-tokenizer.test.ts index 28ce458..19aa588 100644 --- a/test/command-tokenizer.test.ts +++ b/test/command-tokenizer.test.ts @@ -21,6 +21,10 @@ void test("tokenizeArgs supports quoted values", () => { "install", "git@github.com:user/my repo.git", ]); + + assert.deepEqual(tokenizeArgs('cmd ""'), ["cmd", ""]); + assert.deepEqual(tokenizeArgs("cmd ''"), ["cmd", ""]); + assert.deepEqual(tokenizeArgs('cmd "" --flag'), ["cmd", "", "--flag"]); }); void test("tokenizeArgs keeps windows paths intact", () => { From ce2d3c6a578cc42842ff6d8796333d9d35f33661 Mon Sep 17 00:00:00 2001 From: Abdeslam Yassine Agmar Date: Tue, 24 Feb 2026 13:53:37 +0000 Subject: [PATCH 7/9] refactor(format): avoid double source unwrapping --- src/utils/format.ts | 2 +- test/discovery-parser.test.ts | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/utils/format.ts b/src/utils/format.ts index e5fa3f9..57379da 100644 --- a/src/utils/format.ts +++ b/src/utils/format.ts @@ -132,7 +132,7 @@ export function normalizePackageSource(source: string): string { return `git:${trimmed}`; } - if (isPackageSource(trimmed)) { + if (trimmed.startsWith("npm:") || isGitLikeSource(trimmed) || isLocalPathSource(trimmed)) { return trimmed; } diff --git a/test/discovery-parser.test.ts b/test/discovery-parser.test.ts index 1dbd245..90097bb 100644 --- a/test/discovery-parser.test.ts +++ b/test/discovery-parser.test.ts @@ -209,12 +209,15 @@ void test("normalizePackageSource preserves git and local path sources", () => { ); assert.equal(normalizePackageSource("~/dev/ext"), "~/dev/ext"); assert.equal(normalizePackageSource(".\\extensions\\demo"), ".\\extensions\\demo"); + assert.equal(normalizePackageSource("@scope/pkg"), "npm:@scope/pkg"); +}); + +void test("normalizePackageSource unwraps quoted sources", () => { assert.equal( normalizePackageSource('"./extensions/My Cool Extension.ts"'), "./extensions/My Cool Extension.ts" ); assert.equal(normalizePackageSource("'@scope/pkg'"), "npm:@scope/pkg"); - assert.equal(normalizePackageSource("@scope/pkg"), "npm:@scope/pkg"); }); void test("isPackageSource recognizes git ssh and local path sources", () => { From d86cc5a2d2406073513929c983d1ce82e3905926 Mon Sep 17 00:00:00 2001 From: Abdeslam Yassine Agmar Date: Tue, 24 Feb 2026 13:54:07 +0000 Subject: [PATCH 8/9] fix(packages): handle partial remove waits and path identity --- src/packages/discovery.ts | 14 ++--- src/packages/management.ts | 51 +++++++++++------- src/utils/package-source.ts | 8 +++ test/install-remove.test.ts | 105 ++++++++++++++++++++++++++++++++++++ 4 files changed, 153 insertions(+), 25 deletions(-) diff --git a/src/packages/discovery.ts b/src/packages/discovery.ts index 93e1893..21f36c7 100644 --- a/src/packages/discovery.ts +++ b/src/packages/discovery.ts @@ -12,7 +12,11 @@ import type { InstalledPackage, NpmPackage, SearchCache } from "../types/index.j import { CACHE_TTL, TIMEOUTS } from "../constants.js"; import { readSummary } from "../utils/fs.js"; import { parseNpmSource } from "../utils/format.js"; -import { getPackageSourceKind, splitGitRepoAndRef } from "../utils/package-source.js"; +import { + getPackageSourceKind, + normalizeLocalSourceIdentity, + splitGitRepoAndRef, +} from "../utils/package-source.js"; import { execNpm } from "../utils/npm-exec.js"; let searchCache: SearchCache | null = null; @@ -118,16 +122,14 @@ function sanitizeListSourceSuffix(source: string): string { } function normalizeSourceIdentity(source: string): string { - const sanitized = sanitizeListSourceSuffix(source).replace(/\\/g, "/"); + const sanitized = sanitizeListSourceSuffix(source); const kind = getPackageSourceKind(sanitized); if (kind === "local") { - const isWindowsPath = - /^[a-zA-Z]:\//.test(sanitized) || sanitized.startsWith("//") || source.includes("\\"); - return isWindowsPath ? sanitized.toLowerCase() : sanitized; + return normalizeLocalSourceIdentity(sanitized); } - return sanitized.toLowerCase(); + return sanitized.replace(/\\/g, "/").toLowerCase(); } function isScopeHeader(lowerTrimmed: string, scope: "global" | "project"): boolean { diff --git a/src/packages/management.ts b/src/packages/management.ts index 7cee211..27c9eab 100644 --- a/src/packages/management.ts +++ b/src/packages/management.ts @@ -11,7 +11,11 @@ import { } from "./discovery.js"; import { waitForCondition } from "../utils/retry.js"; import { formatInstalledPackageLabel, parseNpmSource } from "../utils/format.js"; -import { getPackageSourceKind, splitGitRepoAndRef } from "../utils/package-source.js"; +import { + getPackageSourceKind, + normalizeLocalSourceIdentity, + splitGitRepoAndRef, +} from "../utils/package-source.js"; import { logPackageUpdate, logPackageRemove } from "../utils/history.js"; import { clearUpdatesAvailable } from "../utils/settings.js"; import { notify, error as notifyError, success } from "../utils/notify.js"; @@ -42,7 +46,8 @@ function packageMutationOutcome( } function isUpToDateOutput(stdout: string): boolean { - return /already\s+up\s+to\s+date/i.test(stdout) || /\bpinned\b/i.test(stdout); + const pinnedAsStatus = /^\s*pinned\b(?!\s+dependency\b)(?:\s*$|\s*[:(-])/im.test(stdout); + return /already\s+up\s+to\s+date/i.test(stdout) || pinnedAsStatus; } async function updatePackageInternal( @@ -151,14 +156,6 @@ export async function updatePackagesWithOutcome( return updatePackagesInternal(ctx, pi); } -function normalizeLocalSourceIdentity(source: string): string { - const normalized = source.replace(/\\/g, "/"); - const looksWindowsPath = - /^[a-zA-Z]:\//.test(normalized) || normalized.startsWith("//") || source.includes("\\"); - - return looksWindowsPath ? normalized.toLowerCase() : normalized; -} - function packageIdentity(source: string, fallbackName?: string): string { const npm = parseNpmSource(source); if (npm?.name) { @@ -262,12 +259,18 @@ function formatRemovalTargets(targets: RemovalTarget[]): string { return targets.map((t) => `${t.scope}: ${t.source}`).join("\n"); } +interface RemovalExecutionResult { + target: RemovalTarget; + success: boolean; + error?: string; +} + async function executeRemovalTargets( targets: RemovalTarget[], ctx: ExtensionCommandContext, pi: ExtensionAPI -): Promise { - const failures: string[] = []; +): Promise { + const results: RemovalExecutionResult[] = []; for (const target of targets) { showProgress(ctx, "Removing", `${target.source} (${target.scope})`); @@ -278,14 +281,15 @@ async function executeRemovalTargets( if (res.code !== 0) { const errorMsg = `Remove failed (${target.scope}): ${res.stderr || res.stdout || `exit ${res.code}`}`; logPackageRemove(pi, target.source, target.name, false, errorMsg); - failures.push(errorMsg); + results.push({ target, success: false, error: errorMsg }); continue; } logPackageRemove(pi, target.source, target.name, true); + results.push({ target, success: true }); } - return failures; + return results; } function notifyRemovalSummary( @@ -350,9 +354,18 @@ async function removePackageInternal( return NO_PACKAGE_MUTATION_OUTCOME; } - const failures = await executeRemovalTargets(targets, ctx, pi); + const results = await executeRemovalTargets(targets, ctx, pi); clearSearchCache(); + const failures = results + .filter((result): result is RemovalExecutionResult & { success: false; error: string } => + Boolean(!result.success && result.error) + ) + .map((result) => result.error); + const successfulTargets = results + .filter((result) => result.success) + .map((result) => result.target); + const remaining = (await getInstalledPackagesAllScopes(ctx, pi)).filter( (p) => packageIdentity(p.source, p.name) === identity ); @@ -362,15 +375,15 @@ async function removePackageInternal( clearUpdatesAvailable(pi, ctx); } - const successfulRemovalCount = targets.length - failures.length; + const successfulRemovalCount = successfulTargets.length; - // Wait for selected targets to disappear from their target scopes before reloading. - if (successfulRemovalCount > 0 && failures.length === 0 && targets.length > 0) { + // Wait for successfully removed targets to disappear from their target scopes before reloading. + if (successfulTargets.length > 0) { notify(ctx, "Waiting for removal to complete...", "info"); const isRemoved = await waitForCondition( async () => { const installedChecks = await Promise.all( - targets.map((target) => + successfulTargets.map((target) => isSourceInstalled(target.source, ctx, pi, { scope: target.scope, }) diff --git a/src/utils/package-source.ts b/src/utils/package-source.ts index 04efe65..cfd8766 100644 --- a/src/utils/package-source.ts +++ b/src/utils/package-source.ts @@ -47,6 +47,14 @@ export function getPackageSourceKind(source: string): PackageSourceKind { return "unknown"; } +export function normalizeLocalSourceIdentity(source: string): string { + const normalized = source.replace(/\\/g, "/"); + const looksWindowsPath = + /^[a-zA-Z]:\//.test(normalized) || normalized.startsWith("//") || source.includes("\\"); + + return looksWindowsPath ? normalized.toLowerCase() : normalized; +} + export function splitGitRepoAndRef(gitSpec: string): { repo: string; ref?: string | undefined } { const lastAt = gitSpec.lastIndexOf("@"); if (lastAt <= 0) { diff --git a/test/install-remove.test.ts b/test/install-remove.test.ts index 30f09f9..ef38ba3 100644 --- a/test/install-remove.test.ts +++ b/test/install-remove.test.ts @@ -1,5 +1,6 @@ import test from "node:test"; import assert from "node:assert/strict"; +import type { ExtensionAPI, ExtensionCommandContext } from "@mariozechner/pi-coding-agent"; import { installPackage } from "../src/packages/install.js"; import { removePackage, updatePackage, updatePackages } from "../src/packages/management.js"; import { createMockHarness } from "./helpers/mocks.js"; @@ -79,6 +80,76 @@ void test("removePackage does not request reload when removal fails", async () = ); }); +void test("removePackage waits for successful removals on partial failures", async () => { + const entries: { type: "custom"; customType: string; data: unknown }[] = []; + const calls: { command: string; args: string[] }[] = []; + let listCalls = 0; + let globalInstalled = true; + let projectInstalled = true; + + const pi = { + exec: (command: string, args: string[]) => { + calls.push({ command, args }); + + if (command === "pi" && args[0] === "list") { + listCalls += 1; + const lines: string[] = []; + if (globalInstalled) { + lines.push("Global:", " npm:demo@1.0.0"); + } + if (projectInstalled) { + lines.push("Project:", " npm:demo@1.0.0"); + } + if (lines.length === 0) { + lines.push("No packages installed"); + } + return Promise.resolve({ code: 0, stdout: lines.join("\n"), stderr: "", killed: false }); + } + + if (command === "pi" && args[0] === "remove") { + if (args.includes("-l")) { + return Promise.resolve({ + code: 1, + stdout: "", + stderr: "permission denied", + killed: false, + }); + } + + globalInstalled = false; + return Promise.resolve({ code: 0, stdout: "removed", stderr: "", killed: false }); + } + + return Promise.resolve({ code: 0, stdout: "", stderr: "", killed: false }); + }, + appendEntry: (customType: string, data: unknown) => { + entries.push({ type: "custom", customType, data }); + }, + } as unknown as ExtensionAPI; + + const ctx = { + hasUI: true, + cwd: "/tmp", + ui: { + notify: () => undefined, + select: (title: string) => + Promise.resolve(title === "Remove scope" ? "Both global + project" : undefined), + confirm: (title: string) => Promise.resolve(title === "Remove Package"), + setStatus: () => undefined, + theme: { fg: (_name: string, text: string) => text }, + }, + sessionManager: { + getEntries: () => entries, + }, + } as unknown as ExtensionCommandContext; + + await removePackage("npm:demo@1.0.0", ctx, pi); + + const removeCalls = calls.filter((call) => call.command === "pi" && call.args[0] === "remove"); + assert.equal(removeCalls.length, 2); + assert.ok(listCalls >= 3); +}); + void test("removePackage targets exact local source when names collide", async () => { const installed = ["/opt/extensions/alpha/index.ts", "/opt/extensions/beta/index.ts"]; @@ -205,6 +276,40 @@ void test("updatePackage treats case-variant already-up-to-date output as no-op" assert.deepEqual(latestAutoUpdate?.updatesAvailable ?? [], []); }); +void test("updatePackage does not treat pinned dependency messages as no-op", async () => { + const output: string[] = []; + const originalLog = console.log; + console.log = (...args: unknown[]) => { + output.push(args.map(String).join(" ")); + }; + + try { + const { pi, ctx } = createMockHarness({ + execImpl: (command, args) => { + if (command === "pi" && args[0] === "update") { + return { + code: 0, + stdout: "Updated npm:pi-extmgr\npinned dependency foo skipped", + stderr: "", + killed: false, + }; + } + + return { code: 0, stdout: "", stderr: "", killed: false }; + }, + }); + + await updatePackage("npm:pi-extmgr", ctx, pi); + } finally { + console.log = originalLog; + } + + assert.equal( + output.some((line) => line.includes("Reload pi to apply changes. (Package updated.)")), + true + ); +}); + void test("updatePackages treats case-variant already-up-to-date output as no-op", async () => { const output: string[] = []; const originalLog = console.log; From 2927a9c96029ba7244a662adc3f495e25a79f2f0 Mon Sep 17 00:00:00 2001 From: Abdeslam Yassine Agmar Date: Tue, 24 Feb 2026 17:10:27 +0000 Subject: [PATCH 9/9] refactor(packages): reuse source kind in identity checks --- src/packages/management.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/packages/management.ts b/src/packages/management.ts index 27c9eab..e4fcf62 100644 --- a/src/packages/management.ts +++ b/src/packages/management.ts @@ -162,13 +162,15 @@ function packageIdentity(source: string, fallbackName?: string): string { return `npm:${npm.name}`; } - if (getPackageSourceKind(source) === "git") { + const sourceKind = getPackageSourceKind(source); + + if (sourceKind === "git") { const gitSpec = source.startsWith("git:") ? source.slice(4) : source; const { repo } = splitGitRepoAndRef(gitSpec); return `git:${repo}`; } - if (getPackageSourceKind(source) === "local") { + if (sourceKind === "local") { return `src:${normalizeLocalSourceIdentity(source)}`; }