diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9737c111f..c54c23593 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -5,6 +5,7 @@ - Fixed chat transcript updates after submitting input so frozen scrollback is only thawed when native scrollback replay succeeds, preventing misplaced or duplicated rows when the viewport is not at the tail - Fixed `read` of `.zip` archives to list the central directory without inflating every member, so large or corrupt zip payloads no longer freeze directory reads; member contents are inflated only when a specific entry is read. +- Fixed `omp plugin install ` failing with `Invalid package name: .` (and similar) for cwd-relative (`.`, `./pkg`), absolute (`/abs`, `C:\…`, `\\unc`), and tilde-prefixed (`~/pkg`) specs. `classifyInstallTarget` now returns a `local` arm in addition to `marketplace`/`npm`, and `plugin install` routes those specs to `PluginManager.link()` — the same code path as `omp plugin link`. ([#1945](https://github.com/can1357/oh-my-pi/issues/1945)) ## [15.9.3] - 2026-06-05 diff --git a/packages/coding-agent/src/cli/classify-install-target.ts b/packages/coding-agent/src/cli/classify-install-target.ts index fadd1fab9..fdbb9e848 100644 --- a/packages/coding-agent/src/cli/classify-install-target.ts +++ b/packages/coding-agent/src/cli/classify-install-target.ts @@ -1,7 +1,11 @@ /** - * Classify an install spec as a marketplace plugin reference or a plain npm package. + * Classify an install spec as a local path, marketplace plugin reference, or + * plain npm package. * * Rules (applied in order): + * 0. Looks like a filesystem path (`.`, `..`, `./…`, `..\…`, `/…`, `~/…`, + * `C:\…`, `\\unc`) -> local. Routed through `PluginManager.link()` so the + * `omp plugin install ` and `omp plugin link ` flows agree. * 1. Starts with `@` (scoped npm) -> always npm. * 2. Contains `@` after the first character -> split on the LAST `@`. * If the right-hand side is a known marketplace name, it's a marketplace ref. @@ -25,10 +29,32 @@ const NPM_DIST_TAGS = new Set([ // Semver-like: starts with digit, or contains version range prefixes const LOOKS_LIKE_VERSION = /^[\d~^>=<]/; -export function classifyInstallTarget( - spec: string, - knownMarketplaces: Set, -): { type: "marketplace"; name: string; marketplace: string } | { type: "npm"; spec: string } { +/** + * Detect specs that name a filesystem path rather than a package: bare `.` / + * `..`, cwd-relative (`./`, `../`, `.\`, `..\`), absolute (`/`, `C:\`, `C:/`, + * UNC `\\`), and tilde-prefixed (`~`, `~/`, `~\`). Tilde paths still rely on + * the shell or the caller for expansion — we only classify them so they reach + * the link path instead of npm-name validation. + */ +function isLocalPathSpec(spec: string): boolean { + if (spec === "." || spec === ".." || spec === "~") return true; + if (spec.startsWith("./") || spec.startsWith("../")) return true; + if (spec.startsWith(".\\") || spec.startsWith("..\\")) return true; + if (spec.startsWith("~/") || spec.startsWith("~\\")) return true; + if (spec.startsWith("/")) return true; + if (spec.startsWith("\\\\")) return true; + if (/^[A-Za-z]:[\\/]/.test(spec)) return true; + return false; +} + +export type ClassifiedInstallTarget = + | { type: "local"; path: string } + | { type: "marketplace"; name: string; marketplace: string } + | { type: "npm"; spec: string }; + +export function classifyInstallTarget(spec: string, knownMarketplaces: Set): ClassifiedInstallTarget { + // Rule 0: filesystem path — bypass npm/marketplace validation entirely. + if (isLocalPathSpec(spec)) return { type: "local", path: spec }; // Rule 1: scoped npm package — @ at position 0 is never a marketplace separator. if (spec.startsWith("@")) return { type: "npm", spec }; // Rule 2: @ somewhere after the first character. diff --git a/packages/coding-agent/src/cli/plugin-cli.ts b/packages/coding-agent/src/cli/plugin-cli.ts index ec21b01ed..5e9c219f9 100644 --- a/packages/coding-agent/src/cli/plugin-cli.ts +++ b/packages/coding-agent/src/cli/plugin-cli.ts @@ -354,6 +354,7 @@ async function handleInstall( console.error(chalk.dim(` ${APP_NAME} plugin install name@marketplace`)); console.error(chalk.dim(` ${APP_NAME} plugin install github:user/repo`)); console.error(chalk.dim(` ${APP_NAME} plugin install https://github.com/user/repo#v1.0`)); + console.error(chalk.dim(` ${APP_NAME} plugin install ./path/to/local/plugin`)); process.exit(1); } @@ -382,6 +383,49 @@ async function handleInstall( continue; } + if (target.type === "local") { + // Local paths route to link(): symlink the directory into the plugins + // node_modules tree so source edits show up without a reinstall. Matches + // `omp plugin link ` so users can use either verb interchangeably. + if (flags.scope) { + console.error( + chalk.yellow( + `Warning: --scope is only supported for marketplace installs (name@marketplace). Ignoring for ${spec}.`, + ), + ); + } + if (flags.force) { + console.error( + chalk.yellow( + `Warning: --force has no effect for local path installs (link is already idempotent). Ignoring for ${spec}.`, + ), + ); + } + if (flags.dryRun) { + if (flags.json) { + console.log(JSON.stringify({ dryRun: true, action: "link", path: target.path }, null, 2)); + } else { + console.log(chalk.dim(`[dry-run] Would link ${spec}`)); + } + continue; + } + try { + const result = await manager.link(target.path); + if (flags.json) { + console.log(JSON.stringify(result, null, 2)); + } else { + console.log(chalk.green(`${theme.status.success} Linked ${result.name} from ${spec}`)); + if (result.manifest.description) { + console.log(chalk.dim(` ${result.manifest.description}`)); + } + } + } catch (err) { + console.error(chalk.red(`${theme.status.error} Failed to install ${spec}: ${err}`)); + process.exit(1); + } + continue; + } + // --scope only applies to marketplace installs; warn when it would be silently no-op'd for npm. if (flags.scope) { console.error( @@ -923,6 +967,7 @@ ${chalk.bold("Sources:")} github:user/repo[#ref] GitHub shorthand (also gitlab:, bitbucket:, codeberg:, sourcehut:) https://github.com/user/repo Full git URL (https, ssh, or git protocol) name@marketplace Marketplace plugin (see marketplace command) + ./path, ../path, /abs, ~/path Local plugin directory (symlinked, same as plugin link) ${chalk.bold("Config Subcommands:")} config list List all settings diff --git a/packages/coding-agent/test/marketplace/cli.test.ts b/packages/coding-agent/test/marketplace/cli.test.ts index 04549070c..e087a6175 100644 --- a/packages/coding-agent/test/marketplace/cli.test.ts +++ b/packages/coding-agent/test/marketplace/cli.test.ts @@ -49,4 +49,35 @@ describe("classifyInstallTarget", () => { const result = classifyInstallTarget("some-pkg@my-marketplace", KNOWN); expect(result).toEqual({ type: "marketplace", name: "some-pkg", marketplace: "my-marketplace" }); }); + + describe("local paths take precedence over npm classification", () => { + const cases: Array<[string, string]> = [ + [".", "bare cwd"], + ["..", "bare parent"], + ["~", "bare home"], + ["./pkg", "cwd-relative"], + ["../pkg", "parent-relative"], + [".\\pkg", "cwd-relative (windows)"], + ["..\\pkg", "parent-relative (windows)"], + ["~/pkg", "tilde-prefixed (posix)"], + ["~\\pkg", "tilde-prefixed (windows)"], + ["/abs/path", "posix absolute"], + ["C:\\abs\\path", "windows absolute (backslash)"], + ["C:/abs/path", "windows absolute (forward slash)"], + ["\\\\server\\share", "windows UNC"], + ]; + for (const [spec, label] of cases) { + it(`classifies ${label} (${JSON.stringify(spec)}) as local`, () => { + expect(classifyInstallTarget(spec, KNOWN)).toEqual({ type: "local", path: spec }); + }); + } + + it("does not misclassify package names that merely contain dots", () => { + expect(classifyInstallTarget("my.plugin", KNOWN)).toEqual({ type: "npm", spec: "my.plugin" }); + }); + + it("does not misclassify dist-tags or version specifiers as local", () => { + expect(classifyInstallTarget("pkg@1.2.3", KNOWN)).toEqual({ type: "npm", spec: "pkg@1.2.3" }); + }); + }); }); diff --git a/packages/coding-agent/test/plugin-install-local.test.ts b/packages/coding-agent/test/plugin-install-local.test.ts new file mode 100644 index 000000000..206236bb5 --- /dev/null +++ b/packages/coding-agent/test/plugin-install-local.test.ts @@ -0,0 +1,143 @@ +/** + * Routing tests for `omp plugin install ` (#1945). + * + * Two layers of coverage: + * 1. Spy-based: `runPluginCommand` with a local path calls + * `PluginManager.link` and NEVER `PluginManager.install` (the npm path + * that produced `Invalid package name: .`). + * 2. End-to-end: with a real on-disk plugin directory, the install routes + * through `link` and produces the symlink + lockfile entry users expect. + * + * `flags.json` is set everywhere so the renderer takes the JSON branch and + * avoids the theme (`runPluginCommand` does not initialize the theme on its + * own — `commands/plugin.ts` does). + */ +import { afterEach, beforeEach, describe, expect, type Mock, mock, spyOn, test } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { runPluginCommand } from "@oh-my-pi/pi-coding-agent/cli/plugin-cli"; +import { PluginManager } from "@oh-my-pi/pi-coding-agent/extensibility/plugins/manager"; +import { MarketplaceManager } from "@oh-my-pi/pi-coding-agent/extensibility/plugins/marketplace"; +import type { InstalledPlugin } from "@oh-my-pi/pi-coding-agent/extensibility/plugins/types"; +import * as piUtils from "@oh-my-pi/pi-utils"; + +const FAKE_INSTALLED: InstalledPlugin = { + name: "kimi-datasource", + version: "1.0.0", + path: "/tmp/fake/plugins/node_modules/kimi-datasource", + manifest: { version: "1.0.0" }, + enabledFeatures: null, + enabled: true, +}; + +describe("runPluginCommand({ action: 'install', args: [] })", () => { + let tmpRoot: string; + let logSpy: Mock; + let errSpy: Mock; + + beforeEach(async () => { + tmpRoot = await fs.mkdtemp(path.join(os.tmpdir(), "omp-plugin-install-local-")); + const pluginsDir = path.join(tmpRoot, "plugins"); + await fs.mkdir(path.join(pluginsDir, "node_modules"), { recursive: true }); + + spyOn(piUtils, "getPluginsDir").mockReturnValue(pluginsDir); + spyOn(piUtils, "getPluginsNodeModules").mockReturnValue(path.join(pluginsDir, "node_modules")); + spyOn(piUtils, "getPluginsPackageJson").mockReturnValue(path.join(pluginsDir, "package.json")); + spyOn(piUtils, "getPluginsLockfile").mockReturnValue(path.join(tmpRoot, "omp-plugins.lock.json")); + spyOn(piUtils, "getProjectDir").mockReturnValue(tmpRoot); + spyOn(piUtils, "getProjectPluginOverridesPath").mockReturnValue(path.join(tmpRoot, "plugin-overrides.json")); + // runPluginCommand always builds a MarketplaceManager to enumerate + // registered marketplaces. Stub the registry list so classification has + // no marketplace candidates to confuse local paths with. + spyOn(MarketplaceManager.prototype, "listMarketplaces").mockResolvedValue([]); + + // Swallow CLI output so test logs stay clean. + logSpy = spyOn(console, "log").mockImplementation(() => undefined); + errSpy = spyOn(console, "error").mockImplementation(() => undefined); + }); + afterEach(async () => { + // Restore every spy installed in beforeEach plus the per-test + // linkSpy/installSpy/console spies. Without this, the piUtils.* + // stubs leak into sibling test files (e.g. marketplace/manager.test.ts + // breaks because listMarketplaces() still returns []). + mock.restore(); + await fs.rm(tmpRoot, { recursive: true, force: true }); + }); + + for (const spec of [".", "./pkg", "../pkg", "/abs/pkg", "~/pkg"]) { + test(`dispatches ${JSON.stringify(spec)} to link() instead of install()`, async () => { + const linkSpy = spyOn(PluginManager.prototype, "link").mockResolvedValue(FAKE_INSTALLED); + const installSpy = spyOn(PluginManager.prototype, "install").mockResolvedValue(FAKE_INSTALLED); + try { + await runPluginCommand({ action: "install", args: [spec], flags: { json: true } }); + expect(linkSpy).toHaveBeenCalledTimes(1); + expect(linkSpy.mock.calls[0]?.[0]).toBe(spec); + expect(installSpy).not.toHaveBeenCalled(); + } finally { + linkSpy.mockRestore(); + installSpy.mockRestore(); + } + }); + } + + test("npm-style spec still dispatches to install(), not link()", async () => { + // Guard against an overly-eager local detector: a bare package name with + // no path-like prefix must continue down the npm path. + const linkSpy = spyOn(PluginManager.prototype, "link").mockResolvedValue(FAKE_INSTALLED); + const installSpy = spyOn(PluginManager.prototype, "install").mockResolvedValue(FAKE_INSTALLED); + try { + await runPluginCommand({ action: "install", args: ["some-pkg"], flags: { json: true } }); + expect(installSpy).toHaveBeenCalledTimes(1); + expect(installSpy.mock.calls[0]?.[0]).toBe("some-pkg"); + expect(linkSpy).not.toHaveBeenCalled(); + } finally { + linkSpy.mockRestore(); + installSpy.mockRestore(); + } + }); + + test("--dry-run on a local path neither links nor installs", async () => { + const linkSpy = spyOn(PluginManager.prototype, "link").mockResolvedValue(FAKE_INSTALLED); + const installSpy = spyOn(PluginManager.prototype, "install").mockResolvedValue(FAKE_INSTALLED); + try { + await runPluginCommand({ action: "install", args: ["."], flags: { dryRun: true, json: true } }); + expect(linkSpy).not.toHaveBeenCalled(); + expect(installSpy).not.toHaveBeenCalled(); + } finally { + linkSpy.mockRestore(); + installSpy.mockRestore(); + } + }); + + test("real local plugin directory: install symlinks it like link would", async () => { + // End-to-end: stage a real plugin folder, route through plugin-cli + // (no spies on PluginManager.link), and verify the resulting symlink + // + lockfile entry. Pins the contract that local-path installs + // symlink rather than copy-install, matching `omp plugin link`. + const localPlugin = path.join(tmpRoot, "kimi-datasource"); + await fs.mkdir(localPlugin, { recursive: true }); + await Bun.write( + path.join(localPlugin, "package.json"), + JSON.stringify({ + name: "kimi-datasource", + version: "1.0.0", + omp: { extensions: ["./src/extension.ts"] }, + }), + ); + + await runPluginCommand({ action: "install", args: [localPlugin], flags: { json: true } }); + + const linkTarget = path.join(tmpRoot, "plugins", "node_modules", "kimi-datasource"); + const stat = await fs.lstat(linkTarget); + expect(stat.isSymbolicLink()).toBe(true); + expect(await fs.readlink(linkTarget)).toBe(localPlugin); + + const lock = await Bun.file(path.join(tmpRoot, "omp-plugins.lock.json")).json(); + expect(lock.plugins["kimi-datasource"]).toEqual({ + version: "1.0.0", + enabledFeatures: null, + enabled: true, + }); + }); +});