diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index a4d66e8d2..ba17d3964 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `omp plugin uninstall --dry-run` actually removing the plugin on both the npm and marketplace routes; dry-run now reports what would be removed and leaves installed plugin state unchanged ([#8178](https://github.com/can1357/oh-my-pi/issues/8178)). + ## [17.2.12] - 2026-08-08 ### Fixed diff --git a/packages/coding-agent/src/cli/plugin-cli.ts b/packages/coding-agent/src/cli/plugin-cli.ts index 5875345c7..468d8f7e7 100644 --- a/packages/coding-agent/src/cli/plugin-cli.ts +++ b/packages/coding-agent/src/cli/plugin-cli.ts @@ -464,7 +464,7 @@ async function handleInstall( async function handleUninstall( manager: PluginManager, packages: string[], - flags: { json?: boolean; scope?: "user" | "project" }, + flags: { json?: boolean; dryRun?: boolean; scope?: "user" | "project" }, ): Promise { if (packages.length === 0) { console.error(chalk.red(`Usage: ${APP_NAME} plugin uninstall ...`)); @@ -477,7 +477,26 @@ async function handleUninstall( const installedPlugins = new Set((await mktMgr.listInstalledPlugins()).map(p => p.id)); for (const name of packages) { - if (installedPlugins.has(name)) { + const viaMarketplace = installedPlugins.has(name); + + if (flags.dryRun) { + // Dry-run is non-mutating: report the resolved removal without touching state. + if (flags.json) { + console.log( + JSON.stringify({ + dryRun: true, + action: "uninstall", + plugin: name, + source: viaMarketplace ? "marketplace" : "npm", + }), + ); + } else { + console.log(chalk.dim(`[dry-run] Would uninstall ${name}`)); + } + continue; + } + + if (viaMarketplace) { // Exact match against installed marketplace plugin IDs (name@marketplace) try { await mktMgr.uninstallPlugin(name, flags.scope); diff --git a/packages/coding-agent/test/plugin-uninstall-dry-run.test.ts b/packages/coding-agent/test/plugin-uninstall-dry-run.test.ts new file mode 100644 index 000000000..9f03cd3e7 --- /dev/null +++ b/packages/coding-agent/test/plugin-uninstall-dry-run.test.ts @@ -0,0 +1,81 @@ +/** + * Regression tests for `omp plugin uninstall --dry-run` (#8178). + * + * `--dry-run` must be non-mutating: it reports what would be removed and + * leaves the installed plugin list untouched. Before the fix, `handleUninstall` + * dropped the parsed `dryRun` flag and unconditionally called the removal + * methods, so a dry-run actually uninstalled the plugin on both the npm and + * marketplace routes. + * + * `flags.json` is set so the renderer takes the JSON branch and avoids the + * theme (`runPluginCommand` does not initialize the theme on its own). + */ +import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from "bun:test"; +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 type { InstalledPluginSummary } from "@oh-my-pi/pi-coding-agent/extensibility/plugins/marketplace"; +import { MarketplaceManager } from "@oh-my-pi/pi-coding-agent/extensibility/plugins/marketplace"; + +describe("runPluginCommand({ action: 'uninstall', flags: { dryRun } })", () => { + beforeEach(() => { + spyOn(console, "log").mockImplementation(() => undefined); + spyOn(console, "error").mockImplementation(() => undefined); + }); + afterEach(() => { + mock.restore(); + }); + + test("npm route: --dry-run never calls PluginManager.uninstall", async () => { + // No marketplace-installed plugins → the name routes down the npm path. + spyOn(MarketplaceManager.prototype, "listInstalledPlugins").mockResolvedValue([]); + const npmUninstall = spyOn(PluginManager.prototype, "uninstall").mockResolvedValue(undefined); + const mktUninstall = spyOn(MarketplaceManager.prototype, "uninstallPlugin").mockResolvedValue(undefined); + try { + await runPluginCommand({ action: "uninstall", args: ["zmarketplace"], flags: { dryRun: true, json: true } }); + expect(npmUninstall).not.toHaveBeenCalled(); + expect(mktUninstall).not.toHaveBeenCalled(); + } finally { + npmUninstall.mockRestore(); + mktUninstall.mockRestore(); + } + }); + + test("marketplace route: --dry-run never calls MarketplaceManager.uninstallPlugin", async () => { + const installed: InstalledPluginSummary = { + id: "hello@local", + scope: "user", + entries: [ + { + scope: "user", + installPath: "/tmp/hello", + version: "1.0.0", + installedAt: new Date().toISOString(), + lastUpdated: new Date().toISOString(), + }, + ], + }; + spyOn(MarketplaceManager.prototype, "listInstalledPlugins").mockResolvedValue([installed]); + const npmUninstall = spyOn(PluginManager.prototype, "uninstall").mockResolvedValue(undefined); + const mktUninstall = spyOn(MarketplaceManager.prototype, "uninstallPlugin").mockResolvedValue(undefined); + try { + await runPluginCommand({ action: "uninstall", args: ["hello@local"], flags: { dryRun: true, json: true } }); + expect(mktUninstall).not.toHaveBeenCalled(); + expect(npmUninstall).not.toHaveBeenCalled(); + } finally { + npmUninstall.mockRestore(); + mktUninstall.mockRestore(); + } + }); + + test("without --dry-run the npm route still uninstalls", async () => { + spyOn(MarketplaceManager.prototype, "listInstalledPlugins").mockResolvedValue([]); + const npmUninstall = spyOn(PluginManager.prototype, "uninstall").mockResolvedValue(undefined); + try { + await runPluginCommand({ action: "uninstall", args: ["zmarketplace"], flags: { json: true } }); + expect(npmUninstall).toHaveBeenCalledTimes(1); + expect(npmUninstall.mock.calls[0]?.[0]).toBe("zmarketplace"); + } finally { + npmUninstall.mockRestore(); + } + }); +});