mirror of
https://github.com/getcompanion-ai/co-mono.git
synced 2026-04-15 09:01:14 +00:00
fix(coding-agent): centralize package source normalization and local path parsing fixes #1304
This commit is contained in:
parent
8792ee2a66
commit
6b0f1fefdb
5 changed files with 178 additions and 121 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -49,6 +49,8 @@ export interface PackageManager {
|
|||
sources: string[],
|
||||
options?: { local?: boolean; temporary?: boolean },
|
||||
): Promise<ResolvedPaths>;
|
||||
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) {
|
||||
|
|
|
|||
|
|
@ -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<boolean> {
|
||||
const options = parsePackageCommand(args);
|
||||
if (!options) {
|
||||
|
|
@ -223,7 +108,7 @@ async function handlePackageCommand(args: string[]): Promise<boolean> {
|
|||
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<boolean> {
|
|||
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);
|
||||
|
|
|
|||
|
|
@ -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}/`]);
|
||||
|
||||
|
|
|
|||
|
|
@ -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)", () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue