smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(cli): kept marketplace uninstall scope positional

Browse Source 
The earlier dry-run change moved scope into an options object, silently
ignoring the legacy uninstallPlugin(id, "user") runtime shape still reachable
from compiled or plain-JS callers. Restore scope as the positional second
argument and carry dryRun in a trailing options bag, preserving the existing
call shape while keeping the non-mutating dry-run path.

Fixes #8178
This commit is contained in:
roboomp
2026-08-10 18:09:53 +00:00
parent 15a9346c29
commit b7d83ed931
6 changed files with 14 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/cli/plugin-cli.ts
+2 -2
View File
@@ -482,7 +482,7 @@ async function handleUninstall(
if (flags.dryRun) {
if (viaMarketplace) {
try {
await mktMgr.uninstallPlugin(name, { scope: flags.scope, dryRun: true });
await mktMgr.uninstallPlugin(name, flags.scope, { dryRun: true });
} catch (err) {
console.error(chalk.red(`${theme.status.error} Failed to uninstall ${name}: ${err}`));
process.exit(1);
@@ -508,7 +508,7 @@ async function handleUninstall(
if (viaMarketplace) {
// Exact match against installed marketplace plugin IDs (name@marketplace)
try {
await mktMgr.uninstallPlugin(name, { scope: flags.scope });
await mktMgr.uninstallPlugin(name, flags.scope);
console.log(chalk.green(`${theme.status.success} Uninstalled ${name}`));
} catch (err) {
console.error(chalk.red(`${theme.status.error} Failed to uninstall ${name}: ${err}`));
packages/coding-agent/src/extensibility/plugins/marketplace/manager.ts
+1 -3
View File
@@ -441,15 +441,13 @@ export class MarketplaceManager {
}
/** Validates and removes a marketplace plugin, or only validates when `dryRun` is set. */
async uninstallPlugin(pluginId: string, options?: { scope?: "user" | "project"; dryRun?: boolean }): Promise<void> {
async uninstallPlugin(pluginId: string, scope?: "user" | "project", options?: { dryRun?: boolean }): Promise<void> {
const parsed = parsePluginId(pluginId);
if (!parsed) {
throw new Error(`Invalid plugin ID format: "${pluginId}". Expected "name@marketplace".`);
}
const scope = options?.scope;
const { userEntries, projectEntries, userReg, projectReg } = await this.#findInBothRegistries(pluginId);
const inUser = userEntries && userEntries.length > 0;
const inProject = projectEntries && projectEntries.length > 0;
packages/coding-agent/src/modes/controllers/selector-controller.ts
+1 -1
View File
@@ -1077,7 +1077,7 @@ export class SelectorController {
this.ctx.showStatus(`Uninstalling ${pluginId}...`);
this.ctx.ui.requestRender();
try {
await mgr.uninstallPlugin(pluginId, { scope });
await mgr.uninstallPlugin(pluginId, scope);
this.ctx.showStatus(`Uninstalled ${pluginId}`);
} catch (err) {
this.ctx.showStatus(`Uninstall failed: ${err}`);
packages/coding-agent/src/slash-commands/builtin-marketplace.ts
+2 -2
View File
@@ -182,7 +182,7 @@ export const BUILTIN_MARKETPLACE_SLASH_COMMANDS: ReadonlyArray<SlashCommandSpec>
"Usage: /marketplace uninstall [--scope user|project] <name@marketplace>",
);
if ("error" in parsed) return usage(parsed.error, runtime);
await manager.uninstallPlugin(parsed.pluginId, { scope: parsed.scope });
await manager.uninstallPlugin(parsed.pluginId, parsed.scope);
await runtime.reloadPlugins();
await runtime.output(`Uninstalled ${parsed.pluginId}`);
return commandConsumed();
@@ -337,7 +337,7 @@ export const BUILTIN_MARKETPLACE_SLASH_COMMANDS: ReadonlyArray<SlashCommandSpec>
runtime.ctx.showStatus(uninstArgs.error);
return;
}
await mgr.uninstallPlugin(uninstArgs.pluginId, { scope: uninstArgs.scope });
await mgr.uninstallPlugin(uninstArgs.pluginId, uninstArgs.scope);
runtime.ctx.showStatus(`Uninstalled ${uninstArgs.pluginId}`);
break;
}
packages/coding-agent/test/marketplace/manager.test.ts
+7 -7
View File
@@ -721,10 +721,10 @@ describe("MarketplaceManager", () => {
await ctx.manager.installPlugin("hello-plugin", "test-marketplace", { scope: "user" });
await ctx.manager.installPlugin("hello-plugin", "test-marketplace", { scope: "project" });
await expect(ctx.manager.uninstallPlugin("hello-plugin@test-marketplace", { dryRun: true })).rejects.toThrow(
/both user and project scope/,
);
await ctx.manager.uninstallPlugin("hello-plugin@test-marketplace", { scope: "user", dryRun: true });
await expect(
ctx.manager.uninstallPlugin("hello-plugin@test-marketplace", undefined, { dryRun: true }),
).rejects.toThrow(/both user and project scope/);
await ctx.manager.uninstallPlugin("hello-plugin@test-marketplace", "user", { dryRun: true });
const userReg = await readInstalledPluginsRegistry(path.join(ctx.tmpDir, "installed_plugins.json"));
const projectReg = await readInstalledPluginsRegistry(path.join(ctx.tmpDir, "project_installed_plugins.json"));
@@ -737,7 +737,7 @@ describe("MarketplaceManager", () => {
await ctx.manager.installPlugin("hello-plugin", "test-marketplace", { scope: "project" });
await expect(
ctx.manager.uninstallPlugin("hello-plugin@test-marketplace", { scope: "user", dryRun: true }),
ctx.manager.uninstallPlugin("hello-plugin@test-marketplace", "user", { dryRun: true }),
).rejects.toThrow(/not installed in user scope/);
const projectReg = await readInstalledPluginsRegistry(path.join(ctx.tmpDir, "project_installed_plugins.json"));
@@ -749,7 +749,7 @@ describe("MarketplaceManager", () => {
await ctx.manager.installPlugin("hello-plugin", "test-marketplace", { scope: "user" });
await ctx.manager.installPlugin("hello-plugin", "test-marketplace", { scope: "project" });
await ctx.manager.uninstallPlugin("hello-plugin@test-marketplace", { scope: "user" });
await ctx.manager.uninstallPlugin("hello-plugin@test-marketplace", "user");
const userReg = await readInstalledPluginsRegistry(path.join(ctx.tmpDir, "installed_plugins.json"));
expect(userReg.plugins["hello-plugin@test-marketplace"]).toBeUndefined();
@@ -767,7 +767,7 @@ describe("MarketplaceManager", () => {
// Same plugin+version → same cache path for the project-scope install.
await ctx.manager.installPlugin("hello-plugin", "test-marketplace", { scope: "project" });
await ctx.manager.uninstallPlugin("hello-plugin@test-marketplace", { scope: "user" });
await ctx.manager.uninstallPlugin("hello-plugin@test-marketplace", "user");
// Cache must still exist — project scope still references it.
expect(fs.existsSync(userEntry.installPath)).toBe(true);
packages/coding-agent/test/plugin-uninstall-dry-run.test.ts
+1 -1
View File
@@ -60,7 +60,7 @@ describe("runPluginCommand({ action: 'uninstall', flags: { dryRun } })", () => {
try {
await runPluginCommand({ action: "uninstall", args: ["hello@local"], flags: { dryRun: true, json: true } });
expect(mktUninstall).toHaveBeenCalledTimes(1);
expect(mktUninstall.mock.calls[0]).toEqual(["hello@local", { scope: undefined, dryRun: true }]);
expect(mktUninstall.mock.calls[0]).toEqual(["hello@local", undefined, { dryRun: true }]);
expect(npmUninstall).not.toHaveBeenCalled();
} finally {
npmUninstall.mockRestore();