From 6b0f1fefdb1ac467618734f02c0446a9b74624bf Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Fri, 6 Feb 2026 00:20:52 +0100 Subject: [PATCH] fix(coding-agent): centralize package source normalization and local path parsing fixes #1304 --- packages/coding-agent/CHANGELOG.md | 1 + .../coding-agent/src/core/package-manager.ts | 96 ++++++++++++++ packages/coding-agent/src/main.ts | 123 +----------------- .../test/package-command-paths.test.ts | 16 ++- .../coding-agent/test/package-manager.test.ts | 63 ++++++++- 5 files changed, 178 insertions(+), 121 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index d7f8e29f..869c6061 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixed - Fixed `/quit` being shadowed by fuzzy slash command autocomplete matches from skills by adding `/quit` to built-in command autocomplete, and removed `/exit` command handling ([#1303](https://github.com/badlogic/pi-mono/issues/1303)) +- Fixed local package source parsing and settings normalization regression that misclassified relative paths as git URLs and prevented globally installed local packages from loading after restart ([#1304](https://github.com/badlogic/pi-mono/issues/1304)) ## [0.52.5] - 2026-02-05 diff --git a/packages/coding-agent/src/core/package-manager.ts b/packages/coding-agent/src/core/package-manager.ts index bbb8faa7..4fd10f2b 100644 --- a/packages/coding-agent/src/core/package-manager.ts +++ b/packages/coding-agent/src/core/package-manager.ts @@ -49,6 +49,8 @@ export interface PackageManager { sources: string[], options?: { local?: boolean; temporary?: boolean }, ): Promise; + addSourceToSettings(source: string, options?: { local?: boolean }): boolean; + removeSourceFromSettings(source: string, options?: { local?: boolean }): boolean; setProgressCallback(callback: ProgressCallback | undefined): void; getInstalledPath(source: string, scope: "user" | "project"): string | undefined; } @@ -604,6 +606,43 @@ export class DefaultPackageManager implements PackageManager { this.progressCallback = callback; } + addSourceToSettings(source: string, options?: { local?: boolean }): boolean { + const scope: SourceScope = options?.local ? "project" : "user"; + const currentSettings = + scope === "project" ? this.settingsManager.getProjectSettings() : this.settingsManager.getGlobalSettings(); + const currentPackages = currentSettings.packages ?? []; + const normalizedSource = this.normalizePackageSourceForSettings(source, scope); + const exists = currentPackages.some((existing) => this.packageSourcesMatch(existing, source, scope)); + if (exists) { + return false; + } + const nextPackages = [...currentPackages, normalizedSource]; + if (scope === "project") { + this.settingsManager.setProjectPackages(nextPackages); + } else { + this.settingsManager.setPackages(nextPackages); + } + return true; + } + + removeSourceFromSettings(source: string, options?: { local?: boolean }): boolean { + const scope: SourceScope = options?.local ? "project" : "user"; + const currentSettings = + scope === "project" ? this.settingsManager.getProjectSettings() : this.settingsManager.getGlobalSettings(); + const currentPackages = currentSettings.packages ?? []; + const nextPackages = currentPackages.filter((existing) => !this.packageSourcesMatch(existing, source, scope)); + const changed = nextPackages.length !== currentPackages.length; + if (!changed) { + return false; + } + if (scope === "project") { + this.settingsManager.setProjectPackages(nextPackages); + } else { + this.settingsManager.setPackages(nextPackages); + } + return true; + } + getInstalledPath(source: string, scope: "user" | "project"): string | undefined { const parsed = this.parseSource(source); if (parsed.type === "npm") { @@ -880,6 +919,50 @@ export class DefaultPackageManager implements PackageManager { } } + private getPackageSourceString(pkg: PackageSource): string { + return typeof pkg === "string" ? pkg : pkg.source; + } + + private getSourceMatchKeyForInput(source: string): string { + const parsed = this.parseSource(source); + if (parsed.type === "npm") { + return `npm:${parsed.name}`; + } + if (parsed.type === "git") { + return `git:${parsed.host}/${parsed.path}`; + } + return `local:${this.resolvePath(parsed.path)}`; + } + + private getSourceMatchKeyForSettings(source: string, scope: SourceScope): string { + const parsed = this.parseSource(source); + if (parsed.type === "npm") { + return `npm:${parsed.name}`; + } + if (parsed.type === "git") { + return `git:${parsed.host}/${parsed.path}`; + } + const baseDir = this.getBaseDirForScope(scope); + return `local:${this.resolvePathFromBase(parsed.path, baseDir)}`; + } + + private packageSourcesMatch(existing: PackageSource, inputSource: string, scope: SourceScope): boolean { + const left = this.getSourceMatchKeyForSettings(this.getPackageSourceString(existing), scope); + const right = this.getSourceMatchKeyForInput(inputSource); + return left === right; + } + + private normalizePackageSourceForSettings(source: string, scope: SourceScope): string { + const parsed = this.parseSource(source); + if (parsed.type !== "local") { + return source; + } + const baseDir = this.getBaseDirForScope(scope); + const resolved = this.resolvePath(parsed.path); + const rel = relative(baseDir, resolved); + return rel || "."; + } + private parseSource(source: string): ParsedSource { if (source.startsWith("npm:")) { const spec = source.slice("npm:".length).trim(); @@ -892,6 +975,19 @@ export class DefaultPackageManager implements PackageManager { }; } + const trimmed = source.trim(); + const isWindowsAbsolutePath = /^[A-Za-z]:[\\/]|^\\\\/.test(trimmed); + const isLocalPathLike = + trimmed.startsWith("./") || + trimmed.startsWith("../") || + trimmed.startsWith("/") || + trimmed === "~" || + trimmed.startsWith("~/") || + isWindowsAbsolutePath; + if (isLocalPathLike) { + return { type: "local", path: source }; + } + // Try parsing as git URL const gitParsed = parseGitUrl(source); if (gitParsed) { diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index ef095b1a..99446f54 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -5,8 +5,6 @@ * createAgentSession() options. The SDK does the heavy lifting. */ -import { homedir } from "node:os"; -import { isAbsolute, join, relative, resolve } from "node:path"; import { type ImageContent, modelsAreEqual, supportsXhigh } from "@mariozechner/pi-ai"; import chalk from "chalk"; import { createInterface } from "readline"; @@ -15,7 +13,7 @@ import { selectConfig } from "./cli/config-selector.js"; import { processFileArguments } from "./cli/file-processor.js"; import { listModels } from "./cli/list-models.js"; import { selectSession } from "./cli/session-picker.js"; -import { CONFIG_DIR_NAME, getAgentDir, getModelsPath, VERSION } from "./config.js"; +import { getAgentDir, getModelsPath, VERSION } from "./config.js"; import { AuthStorage } from "./core/auth-storage.js"; import { DEFAULT_THINKING_LEVEL } from "./core/defaults.js"; import { exportFromFile } from "./core/export-html/index.js"; @@ -27,13 +25,12 @@ import { DefaultPackageManager } from "./core/package-manager.js"; import { DefaultResourceLoader } from "./core/resource-loader.js"; import { type CreateAgentSessionOptions, createAgentSession } from "./core/sdk.js"; import { SessionManager } from "./core/session-manager.js"; -import { type PackageSource, SettingsManager } from "./core/settings-manager.js"; +import { SettingsManager } from "./core/settings-manager.js"; import { printTimings, time } from "./core/timings.js"; import { allTools } from "./core/tools/index.js"; import { runMigrations, showDeprecationWarnings } from "./migrations.js"; import { InteractiveMode, runPrintMode, runRpcMode } from "./modes/index.js"; import { initTheme, stopThemeWatcher } from "./modes/interactive/theme/theme.js"; -import { parseGitUrl } from "./utils/git.js"; /** * Read all content from piped stdin. @@ -85,118 +82,6 @@ function parsePackageCommand(args: string[]): PackageCommandOptions | undefined return { command, source: sources[0], local }; } -function expandTildePath(input: string): string { - const trimmed = input.trim(); - if (trimmed === "~") return homedir(); - if (trimmed.startsWith("~/")) return resolve(homedir(), trimmed.slice(2)); - if (trimmed.startsWith("~")) return resolve(homedir(), trimmed.slice(1)); - return trimmed; -} - -function resolveLocalSourceFromInput(source: string, cwd: string): string { - const expanded = expandTildePath(source); - return isAbsolute(expanded) ? resolve(expanded) : resolve(cwd, expanded); -} - -function resolveLocalSourceFromSettings(source: string, baseDir: string): string { - const expanded = expandTildePath(source); - return isAbsolute(expanded) ? expanded : resolve(baseDir, expanded); -} - -function normalizeLocalSourceForSettings(source: string, baseDir: string, cwd: string): string { - const resolved = resolveLocalSourceFromInput(source, cwd); - const rel = relative(baseDir, resolved); - return rel || "."; -} - -function normalizePackageSourceForSettings(source: string, baseDir: string, cwd: string): string { - const normalized = normalizeExtensionSource(source); - if (normalized.type !== "local") { - return source; - } - return normalizeLocalSourceForSettings(source, baseDir, cwd); -} - -function normalizeExtensionSource(source: string): { type: "npm" | "git" | "local"; key: string } { - if (source.startsWith("npm:")) { - const spec = source.slice("npm:".length).trim(); - const match = spec.match(/^(@?[^@]+(?:\/[^@]+)?)(?:@.+)?$/); - return { type: "npm", key: match?.[1] ?? spec }; - } - - // Try parsing as git URL - const parsed = parseGitUrl(source); - if (parsed) { - return { type: "git", key: `${parsed.host}/${parsed.path}` }; - } - - return { type: "local", key: source }; -} - -function normalizeSourceForInput(source: string, cwd: string): { type: "npm" | "git" | "local"; key: string } { - const normalized = normalizeExtensionSource(source); - if (normalized.type !== "local") { - return normalized; - } - return { type: "local", key: resolveLocalSourceFromInput(source, cwd) }; -} - -function normalizeSourceForSettings(source: string, baseDir: string): { type: "npm" | "git" | "local"; key: string } { - const normalized = normalizeExtensionSource(source); - if (normalized.type !== "local") { - return normalized; - } - return { type: "local", key: resolveLocalSourceFromSettings(source, baseDir) }; -} - -function sourcesMatch(a: string, b: string, baseDir: string, cwd: string): boolean { - const left = normalizeSourceForSettings(a, baseDir); - const right = normalizeSourceForInput(b, cwd); - return left.type === right.type && left.key === right.key; -} - -function getPackageSourceString(pkg: PackageSource): string { - return typeof pkg === "string" ? pkg : pkg.source; -} - -function packageSourcesMatch(a: PackageSource, b: string, baseDir: string, cwd: string): boolean { - const aSource = getPackageSourceString(a); - return sourcesMatch(aSource, b, baseDir, cwd); -} - -function updatePackageSources( - settingsManager: SettingsManager, - source: string, - local: boolean, - cwd: string, - agentDir: string, - action: "add" | "remove", -): boolean { - const currentSettings = local ? settingsManager.getProjectSettings() : settingsManager.getGlobalSettings(); - const currentPackages = currentSettings.packages ?? []; - const baseDir = local ? join(cwd, CONFIG_DIR_NAME) : agentDir; - const normalizedSource = normalizePackageSourceForSettings(source, baseDir, cwd); - - let nextPackages: PackageSource[]; - let changed = false; - if (action === "add") { - const exists = currentPackages.some((existing) => packageSourcesMatch(existing, source, baseDir, cwd)); - nextPackages = exists ? currentPackages : [...currentPackages, normalizedSource]; - changed = !exists; - } else { - nextPackages = currentPackages.filter((existing) => !packageSourcesMatch(existing, source, baseDir, cwd)); - changed = nextPackages.length !== currentPackages.length; - } - - if (local) { - settingsManager.setProjectPackages(nextPackages); - } else { - settingsManager.setPackages(nextPackages); - } - - return changed; -} - async function handlePackageCommand(args: string[]): Promise { const options = parsePackageCommand(args); if (!options) { @@ -223,7 +108,7 @@ async function handlePackageCommand(args: string[]): Promise { process.exit(1); } await packageManager.install(options.source, { local: options.local }); - updatePackageSources(settingsManager, options.source, options.local, cwd, agentDir, "add"); + packageManager.addSourceToSettings(options.source, { local: options.local }); console.log(chalk.green(`Installed ${options.source}`)); return true; } @@ -234,7 +119,7 @@ async function handlePackageCommand(args: string[]): Promise { process.exit(1); } await packageManager.remove(options.source, { local: options.local }); - const removed = updatePackageSources(settingsManager, options.source, options.local, cwd, agentDir, "remove"); + const removed = packageManager.removeSourceFromSettings(options.source, { local: options.local }); if (!removed) { console.error(chalk.red(`No matching package found for ${options.source}`)); process.exit(1); diff --git a/packages/coding-agent/test/package-command-paths.test.ts b/packages/coding-agent/test/package-command-paths.test.ts index 539bf7d6..451843b9 100644 --- a/packages/coding-agent/test/package-command-paths.test.ts +++ b/packages/coding-agent/test/package-command-paths.test.ts @@ -1,4 +1,4 @@ -import { mkdirSync, readFileSync, rmSync } from "node:fs"; +import { mkdirSync, readFileSync, realpathSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; @@ -38,6 +38,20 @@ describe("package commands", () => { rmSync(tempDir, { recursive: true, force: true }); }); + it("should persist global relative local package paths relative to settings.json", async () => { + const relativePkgDir = join(projectDir, "packages", "local-package"); + mkdirSync(relativePkgDir, { recursive: true }); + + await main(["install", "./packages/local-package"]); + + const settingsPath = join(agentDir, "settings.json"); + const settings = JSON.parse(readFileSync(settingsPath, "utf-8")) as { packages?: string[] }; + expect(settings.packages?.length).toBe(1); + const stored = settings.packages?.[0] ?? ""; + const resolvedFromSettings = realpathSync(join(agentDir, stored)); + expect(resolvedFromSettings).toBe(realpathSync(relativePkgDir)); + }); + it("should remove local packages using a path with a trailing slash", async () => { await main(["install", `${packageDir}/`]); diff --git a/packages/coding-agent/test/package-manager.test.ts b/packages/coding-agent/test/package-manager.test.ts index 2c7b1bf2..d93da4be 100644 --- a/packages/coding-agent/test/package-manager.test.ts +++ b/packages/coding-agent/test/package-manager.test.ts @@ -1,6 +1,6 @@ import { mkdirSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { join, relative } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { DefaultPackageManager, type ProgressEvent, type ResolvedResource } from "../src/core/package-manager.js"; import { SettingsManager } from "../src/core/settings-manager.js"; @@ -280,6 +280,67 @@ Content`, // Should have attempted clone, not thrown unsupported error expect(events.some((e) => e.type === "start" && e.action === "install")).toBe(true); }); + + it("should parse package source types from docs examples", () => { + expect((packageManager as any).parseSource("npm:@scope/pkg@1.2.3").type).toBe("npm"); + expect((packageManager as any).parseSource("npm:pkg").type).toBe("npm"); + + expect((packageManager as any).parseSource("git:github.com/user/repo@v1").type).toBe("git"); + expect((packageManager as any).parseSource("https://github.com/user/repo@v1").type).toBe("git"); + expect((packageManager as any).parseSource("git@github.com:user/repo@v1").type).toBe("git"); + expect((packageManager as any).parseSource("ssh://git@github.com/user/repo@v1").type).toBe("git"); + + expect((packageManager as any).parseSource("/absolute/path/to/package").type).toBe("local"); + expect((packageManager as any).parseSource("./relative/path/to/package").type).toBe("local"); + expect((packageManager as any).parseSource("../relative/path/to/package").type).toBe("local"); + }); + + it("should never parse dot-relative paths as git", () => { + const dotSlash = (packageManager as any).parseSource("./packages/agent-timers"); + expect(dotSlash.type).toBe("local"); + expect(dotSlash.path).toBe("./packages/agent-timers"); + + const dotDotSlash = (packageManager as any).parseSource("../packages/agent-timers"); + expect(dotDotSlash.type).toBe("local"); + expect(dotDotSlash.path).toBe("../packages/agent-timers"); + }); + }); + + describe("settings source normalization", () => { + it("should store global local packages relative to agent settings base", () => { + const pkgDir = join(tempDir, "packages", "local-global-pkg"); + mkdirSync(join(pkgDir, "extensions"), { recursive: true }); + writeFileSync(join(pkgDir, "extensions", "index.ts"), "export default function() {}"); + + const added = packageManager.addSourceToSettings("./packages/local-global-pkg"); + expect(added).toBe(true); + + const settings = settingsManager.getGlobalSettings(); + expect(settings.packages?.[0]).toBe(relative(agentDir, pkgDir) || "."); + }); + + it("should store project local packages relative to .pi settings base", () => { + const projectPkgDir = join(tempDir, "project-local-pkg"); + mkdirSync(join(projectPkgDir, "extensions"), { recursive: true }); + writeFileSync(join(projectPkgDir, "extensions", "index.ts"), "export default function() {}"); + + const added = packageManager.addSourceToSettings("./project-local-pkg", { local: true }); + expect(added).toBe(true); + + const settings = settingsManager.getProjectSettings(); + expect(settings.packages?.[0]).toBe(relative(join(tempDir, ".pi"), projectPkgDir) || "."); + }); + + it("should remove local package entries using equivalent path forms", () => { + const pkgDir = join(tempDir, "remove-local-pkg"); + mkdirSync(join(pkgDir, "extensions"), { recursive: true }); + writeFileSync(join(pkgDir, "extensions", "index.ts"), "export default function() {}"); + + packageManager.addSourceToSettings("./remove-local-pkg"); + const removed = packageManager.removeSourceFromSettings(`${pkgDir}/`); + expect(removed).toBe(true); + expect(settingsManager.getGlobalSettings().packages ?? []).toHaveLength(0); + }); }); describe("HTTPS git URL parsing (old behavior)", () => {