-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(cli): register marketplace plugin installs #3245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
dc9be7c
1d500ce
77dd83d
661b3e1
bcfb5b5
dc74d09
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,8 @@ import * as os from "node:os"; | |
| import * as path from "node:path"; | ||
|
|
||
| import { isEnoent, logger, pathIsWithin } from "@oh-my-pi/pi-utils"; | ||
| import { normalizePluginRuntimeConfig } from "../runtime-config"; | ||
| import type { PluginRuntimeConfig } from "../types"; | ||
|
|
||
| import { cachePlugin } from "./cache"; | ||
| import { classifySource, fetchMarketplace, parseMarketplaceCatalog, promoteCloneToCache } from "./fetcher"; | ||
|
|
@@ -38,6 +40,16 @@ import type { | |
| } from "./types"; | ||
| import { buildPluginId, parsePluginId } from "./types"; | ||
|
|
||
| const RUNTIME_PACKAGE_NAME_RE = /^(?:@[a-z0-9][a-z0-9._~-]*\/)?[a-z0-9][a-z0-9._~-]*$/; | ||
| const MAX_RUNTIME_PACKAGE_NAME_LENGTH = 214; | ||
|
|
||
| function assertRuntimePackageName(name: string): string { | ||
| if (name.length > MAX_RUNTIME_PACKAGE_NAME_LENGTH || !RUNTIME_PACKAGE_NAME_RE.test(name)) { | ||
| throw new Error(`Invalid marketplace plugin package name: ${JSON.stringify(name)}`); | ||
| } | ||
| return name; | ||
| } | ||
|
|
||
| // ── Options ────────────────────────────────────────────────────────────────── | ||
|
|
||
| export interface MarketplaceManagerOptions { | ||
|
|
@@ -298,6 +310,9 @@ export class MarketplaceManager { | |
| } | ||
| } | ||
|
|
||
| const packageName = await this.#resolvePluginPackageName(cachePath, name); | ||
| const previousPackageNames = await this.#resolveInstalledPackageNames(existing ?? [], name); | ||
|
|
||
| // Only now clean up old entries — new cache succeeded, so it is safe to remove old ones. | ||
| if (existing && existing.length > 0) { | ||
| // Remove from scope-appropriate registry first, then cross-check refs before disk deletion. | ||
|
|
@@ -337,6 +352,13 @@ export class MarketplaceManager { | |
| const newInstReg = addInstalledPlugin(freshInstReg, pluginId, installedEntry); | ||
| await writeInstalledPluginsRegistry(registryPath, newInstReg); | ||
|
|
||
| for (const previousPackageName of previousPackageNames) { | ||
| if (previousPackageName !== packageName) { | ||
| await this.#removeRuntimePlugin(scope, previousPackageName); | ||
| } | ||
| } | ||
| await this.#registerRuntimePlugin(scope, packageName, cachePath, version, wasDisabled ? false : undefined); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
|
|
||
| this.#clearCache(); | ||
|
|
||
| logger.debug("Plugin installed", { pluginId, version, cachePath }); | ||
|
|
@@ -434,6 +456,7 @@ export class MarketplaceManager { | |
| const targetEntries = targetScope === "project" ? projectEntries! : userEntries!; | ||
| const targetReg = targetScope === "project" ? projectReg : userReg; | ||
| const registryPath = this.#registryPath(targetScope); | ||
| const packageNames = await this.#resolveInstalledPackageNames(targetEntries, parsed.name); | ||
|
|
||
| const updatedReg = removeInstalledPlugin(targetReg, pluginId); | ||
| await writeInstalledPluginsRegistry(registryPath, updatedReg); | ||
|
|
@@ -453,6 +476,10 @@ export class MarketplaceManager { | |
| } | ||
| } | ||
|
|
||
| for (const packageName of packageNames) { | ||
| await this.#removeRuntimePlugin(targetScope, packageName); | ||
| } | ||
|
|
||
| this.#clearCache(); | ||
|
|
||
| logger.debug("Plugin uninstalled", { pluginId, scope: targetScope }); | ||
|
|
@@ -539,6 +566,12 @@ export class MarketplaceManager { | |
| }; | ||
| await writeInstalledPluginsRegistry(registryPath, updated); | ||
|
|
||
| const fallbackName = parsePluginId(pluginId)?.name ?? pluginId; | ||
| const packageNames = await this.#resolveInstalledPackageNames(entries, fallbackName); | ||
| for (const packageName of packageNames) { | ||
| await this.#setRuntimePluginEnabled(targetScope, packageName, enabled); | ||
| } | ||
|
|
||
| this.#clearCache(); | ||
|
|
||
| logger.debug("Plugin enabled state changed", { pluginId, enabled, scope: targetScope }); | ||
|
|
@@ -701,6 +734,107 @@ export class MarketplaceManager { | |
|
|
||
| // ── Private helpers ─────────────────────────────────────────────────────── | ||
|
|
||
| #runtimeRoot(scope: "user" | "project"): string { | ||
| return path.dirname(this.#registryPath(scope)); | ||
| } | ||
|
|
||
| #nodeModulesPath(scope: "user" | "project"): string { | ||
| return path.join(this.#runtimeRoot(scope), "node_modules"); | ||
| } | ||
|
|
||
| #runtimeLockPath(scope: "user" | "project"): string { | ||
| return path.join(this.#runtimeRoot(scope), "omp-plugins.lock.json"); | ||
| } | ||
|
|
||
| async #loadRuntimeConfig(scope: "user" | "project"): Promise<PluginRuntimeConfig> { | ||
| try { | ||
| return normalizePluginRuntimeConfig(await Bun.file(this.#runtimeLockPath(scope)).json()); | ||
| } catch (err) { | ||
| if (isEnoent(err)) return normalizePluginRuntimeConfig({}); | ||
| logger.warn("Failed to load marketplace plugin runtime config", { | ||
| path: this.#runtimeLockPath(scope), | ||
| error: String(err), | ||
| }); | ||
| return normalizePluginRuntimeConfig({}); | ||
| } | ||
| } | ||
|
|
||
| async #writeRuntimeConfig(scope: "user" | "project", config: PluginRuntimeConfig): Promise<void> { | ||
| await Bun.write(this.#runtimeLockPath(scope), JSON.stringify(config, null, 2)); | ||
| } | ||
|
|
||
| async #resolvePluginPackageName(installPath: string, fallbackName: string): Promise<string> { | ||
| try { | ||
| const pkg: { name?: unknown } = await Bun.file(path.join(installPath, "package.json")).json(); | ||
| const name = typeof pkg.name === "string" && pkg.name.length > 0 ? pkg.name : fallbackName; | ||
| return assertRuntimePackageName(name); | ||
| } catch (err) { | ||
| if (isEnoent(err)) return assertRuntimePackageName(fallbackName); | ||
| throw err; | ||
| } | ||
| } | ||
|
|
||
| #runtimePackagePath(scope: "user" | "project", packageName: string): string { | ||
| const nodeModules = path.resolve(this.#nodeModulesPath(scope)); | ||
| const linkPath = path.resolve(nodeModules, assertRuntimePackageName(packageName)); | ||
| const relative = path.relative(nodeModules, linkPath); | ||
| if (relative === "" || relative.startsWith("..") || path.isAbsolute(relative)) { | ||
| throw new Error(`Marketplace plugin package path escapes node_modules: ${JSON.stringify(packageName)}`); | ||
| } | ||
| return linkPath; | ||
| } | ||
|
|
||
| async #resolveInstalledPackageNames( | ||
| entries: readonly InstalledPluginEntry[], | ||
| fallbackName: string, | ||
| ): Promise<Set<string>> { | ||
| const packageNames = new Set<string>(); | ||
| for (const entry of entries) { | ||
| packageNames.add(await this.#resolvePluginPackageName(entry.installPath, fallbackName)); | ||
| } | ||
| return packageNames; | ||
| } | ||
|
|
||
| async #registerRuntimePlugin( | ||
| scope: "user" | "project", | ||
| packageName: string, | ||
| cachePath: string, | ||
| version: string, | ||
| enabled: boolean | undefined, | ||
| ): Promise<void> { | ||
| const linkPath = this.#runtimePackagePath(scope, packageName); | ||
| await fs.mkdir(path.dirname(linkPath), { recursive: true }); | ||
| await fs.rm(linkPath, { recursive: true, force: true }); | ||
| await fs.symlink(cachePath, linkPath, process.platform === "win32" ? "junction" : "dir"); | ||
|
Comment on lines
+807
to
+808
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When two installed marketplace entries in the same scope resolve to the same Useful? React with 👍 / 👎. |
||
|
|
||
| const config = await this.#loadRuntimeConfig(scope); | ||
| const previous = config.plugins[packageName]; | ||
| config.plugins[packageName] = { | ||
| version, | ||
| enabledFeatures: previous?.enabledFeatures ?? null, | ||
| enabled: enabled ?? previous?.enabled ?? true, | ||
| }; | ||
| await this.#writeRuntimeConfig(scope, config); | ||
| } | ||
|
|
||
| async #removeRuntimePlugin(scope: "user" | "project", packageName: string): Promise<void> { | ||
| await fs.rm(this.#runtimePackagePath(scope, packageName), { recursive: true, force: true }); | ||
|
|
||
| const config = await this.#loadRuntimeConfig(scope); | ||
| delete config.plugins[packageName]; | ||
| delete config.settings[packageName]; | ||
| await this.#writeRuntimeConfig(scope, config); | ||
| } | ||
|
|
||
| async #setRuntimePluginEnabled(scope: "user" | "project", packageName: string, enabled: boolean): Promise<void> { | ||
| const config = await this.#loadRuntimeConfig(scope); | ||
| const previous = config.plugins[packageName]; | ||
| if (!previous) return; | ||
|
|
||
| config.plugins[packageName] = { ...previous, enabled }; | ||
| await this.#writeRuntimeConfig(scope, config); | ||
| } | ||
|
|
||
| #registryPath(scope: "user" | "project"): string { | ||
| if (scope === "project") { | ||
| if (!this.#opts.projectInstalledRegistryPath) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the same marketplace plugin ID is installed in both user and project scopes but the two versions resolve to different runtime package names (for example, an upgrade adds or renames
package.json#name),listInstalledPluginsmarks the user entry as shadowed by the project entry, but this merge only replaces entries with the same package name. In that project,getEnabledPlugins()will return both the old user package and the project package, so both sets of commands/extensions can load despite the project install being intended to take precedence for that plugin ID.Useful? React with 👍 / 👎.