From 64eb9ce9b5d1ff4fb1f08d9963522c5d32221ba7 Mon Sep 17 00:00:00 2001 From: Pujit Mehrotra Date: Thu, 13 Nov 2025 10:49:50 -0500 Subject: [PATCH] feat(cli): make `unraid-api plugins remove` scriptable (#1774) ## Summary by CodeRabbit * **New Features** * Added --bypass-npm and --npm flags, support for passing plugin names as command args, and a restart option; CLI params now merge with interactive prompts. * **Bug Fixes** * Vendor archive rebuild is performed only when actual uninstalls occur. * Restart behavior uses resolved options for consistent restarts. * Removal can run as "config-only" without running package operations. * **Tests** * Expanded tests for bypass scenarios, prompt flows, config-only removals, and removal control flow. --- .../cli/__test__/plugin.command.test.ts | 65 +++++++++++ .../unraid-api/cli/plugins/plugin.command.ts | 106 +++++++++++++----- .../plugin-management.service.spec.ts | 63 +++++++++++ .../plugin/plugin-management.service.ts | 19 +++- api/src/unraid-api/plugin/plugin.resolver.ts | 2 +- 5 files changed, 223 insertions(+), 32 deletions(-) create mode 100644 api/src/unraid-api/plugin/__test__/plugin-management.service.spec.ts diff --git a/api/src/unraid-api/cli/__test__/plugin.command.test.ts b/api/src/unraid-api/cli/__test__/plugin.command.test.ts index a96847b0c..68b20e663 100644 --- a/api/src/unraid-api/cli/__test__/plugin.command.test.ts +++ b/api/src/unraid-api/cli/__test__/plugin.command.test.ts @@ -36,6 +36,7 @@ const mockPluginManagementService = { addPlugin: vi.fn(), addBundledPlugin: vi.fn(), removePlugin: vi.fn(), + removePluginConfigOnly: vi.fn(), removeBundledPlugin: vi.fn(), plugins: [] as string[], }; @@ -147,6 +148,7 @@ describe('Plugin Commands', () => { '@unraid/plugin-example', '@unraid/plugin-test' ); + expect(mockPluginManagementService.removePluginConfigOnly).not.toHaveBeenCalled(); expect(mockLogger.log).toHaveBeenCalledWith('Removed plugin @unraid/plugin-example'); expect(mockLogger.log).toHaveBeenCalledWith('Removed plugin @unraid/plugin-test'); expect(mockApiConfigPersistence.persist).toHaveBeenCalled(); @@ -178,9 +180,72 @@ describe('Plugin Commands', () => { expect(mockPluginManagementService.removePlugin).toHaveBeenCalledWith( '@unraid/plugin-example' ); + expect(mockPluginManagementService.removePluginConfigOnly).not.toHaveBeenCalled(); expect(mockApiConfigPersistence.persist).toHaveBeenCalled(); expect(mockRestartCommand.run).not.toHaveBeenCalled(); }); + + it('should bypass npm uninstall when bypass flag is provided', async () => { + mockInquirerService.prompt.mockResolvedValue({ + plugins: ['@unraid/plugin-example'], + restart: true, + bypassNpm: true, + }); + + await command.run([], { restart: true, bypassNpm: true }); + + expect(mockPluginManagementService.removePluginConfigOnly).toHaveBeenCalledWith( + '@unraid/plugin-example' + ); + expect(mockPluginManagementService.removePlugin).not.toHaveBeenCalled(); + }); + + it('should preserve cli flags when prompt supplies plugins', async () => { + mockInquirerService.prompt.mockResolvedValue({ + plugins: ['@unraid/plugin-example'], + }); + + await command.run([], { restart: false, bypassNpm: true }); + + expect(mockPluginManagementService.removePluginConfigOnly).toHaveBeenCalledWith( + '@unraid/plugin-example' + ); + expect(mockPluginManagementService.removePlugin).not.toHaveBeenCalled(); + expect(mockRestartCommand.run).not.toHaveBeenCalled(); + }); + + it('should honor prompt restart value when cli flag not provided', async () => { + mockInquirerService.prompt.mockResolvedValue({ + plugins: ['@unraid/plugin-example'], + restart: false, + }); + + await command.run([], {}); + + expect(mockPluginManagementService.removePlugin).toHaveBeenCalledWith( + '@unraid/plugin-example' + ); + expect(mockRestartCommand.run).not.toHaveBeenCalled(); + }); + + it('should respect passed params and skip inquirer', async () => { + await command.run(['@unraid/plugin-example'], { restart: true, bypassNpm: false }); + + expect(mockInquirerService.prompt).not.toHaveBeenCalled(); + expect(mockPluginManagementService.removePlugin).toHaveBeenCalledWith( + '@unraid/plugin-example' + ); + }); + + it('should bypass npm when flag provided with passed params', async () => { + await command.run(['@unraid/plugin-example'], { restart: true, bypassNpm: true }); + + expect(mockInquirerService.prompt).not.toHaveBeenCalled(); + expect(mockPluginManagementService.removePluginConfigOnly).toHaveBeenCalledWith( + '@unraid/plugin-example' + ); + expect(mockPluginManagementService.removePlugin).not.toHaveBeenCalled(); + }); }); describe('ListPluginCommand', () => { diff --git a/api/src/unraid-api/cli/plugins/plugin.command.ts b/api/src/unraid-api/cli/plugins/plugin.command.ts index 556cb02c2..e00c16ada 100644 --- a/api/src/unraid-api/cli/plugins/plugin.command.ts +++ b/api/src/unraid-api/cli/plugins/plugin.command.ts @@ -74,13 +74,15 @@ export class InstallPluginCommand extends CommandRunner { interface RemovePluginCommandOptions { plugins?: string[]; - restart: boolean; + restart?: boolean; + bypassNpm?: boolean; } @SubCommand({ name: 'remove', aliases: ['rm'], description: 'Remove plugin peer dependencies.', + arguments: '[plugins...]', }) export class RemovePluginCommand extends CommandRunner { constructor( @@ -93,9 +95,83 @@ export class RemovePluginCommand extends CommandRunner { super(); } - async run(_passedParams: string[], options?: RemovePluginCommandOptions): Promise { + async run(passedParams: string[], options?: RemovePluginCommandOptions): Promise { + const cliBypass = options?.bypassNpm; + const cliRestart = options?.restart; + const mergedOptions: RemovePluginCommandOptions = { + bypassNpm: cliBypass ?? false, + restart: cliRestart ?? true, + plugins: passedParams.length > 0 ? passedParams : options?.plugins, + }; + + let resolvedOptions = mergedOptions; + if (!mergedOptions.plugins?.length) { + const promptOptions = await this.promptForPlugins(mergedOptions); + if (!promptOptions) { + return; + } + resolvedOptions = { + // precedence: cli > prompt > default (fallback) + bypassNpm: cliBypass ?? promptOptions.bypassNpm ?? mergedOptions.bypassNpm, + restart: cliRestart ?? promptOptions.restart ?? mergedOptions.restart, + // precedence: prompt > default (fallback) + plugins: promptOptions.plugins ?? mergedOptions.plugins, + }; + } + + if (!resolvedOptions.plugins?.length) { + this.logService.warn('No plugins selected for removal.'); + return; + } + + if (resolvedOptions.bypassNpm) { + await this.pluginManagementService.removePluginConfigOnly(...resolvedOptions.plugins); + } else { + await this.pluginManagementService.removePlugin(...resolvedOptions.plugins); + } + for (const plugin of resolvedOptions.plugins) { + this.logService.log(`Removed plugin ${plugin}`); + } + await this.apiConfigPersistence.persist(); + + if (resolvedOptions.restart) { + await this.restartCommand.run(); + } + } + + @Option({ + flags: '--no-restart', + description: 'do NOT restart the service after deploy', + defaultValue: true, + }) + parseRestart(): boolean { + return false; + } + + @Option({ + flags: '-b, --bypass-npm', + description: 'Bypass npm uninstall and only update the config', + defaultValue: false, + name: 'bypassNpm', + }) + parseBypass(): boolean { + return true; + } + + @Option({ + flags: '--npm', + description: 'Run npm uninstall for unbundled plugins (default behavior)', + name: 'bypassNpm', + }) + parseRunNpm(): boolean { + return false; + } + + private async promptForPlugins( + initialOptions: RemovePluginCommandOptions + ): Promise { try { - options = await this.inquirerService.prompt(RemovePluginQuestionSet.name, options); + return await this.inquirerService.prompt(RemovePluginQuestionSet.name, initialOptions); } catch (error) { if (error instanceof NoPluginsFoundError) { this.logService.error(error.message); @@ -108,30 +184,6 @@ export class RemovePluginCommand extends CommandRunner { process.exit(1); } } - - if (!options.plugins || options.plugins.length === 0) { - this.logService.warn('No plugins selected for removal.'); - return; - } - - await this.pluginManagementService.removePlugin(...options.plugins); - for (const plugin of options.plugins) { - this.logService.log(`Removed plugin ${plugin}`); - } - await this.apiConfigPersistence.persist(); - - if (options.restart) { - await this.restartCommand.run(); - } - } - - @Option({ - flags: '--no-restart', - description: 'do NOT restart the service after deploy', - defaultValue: true, - }) - parseRestart(): boolean { - return false; } } diff --git a/api/src/unraid-api/plugin/__test__/plugin-management.service.spec.ts b/api/src/unraid-api/plugin/__test__/plugin-management.service.spec.ts new file mode 100644 index 000000000..1b6c51028 --- /dev/null +++ b/api/src/unraid-api/plugin/__test__/plugin-management.service.spec.ts @@ -0,0 +1,63 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { PluginManagementService } from '@app/unraid-api/plugin/plugin-management.service.js'; + +describe('PluginManagementService', () => { + let service: PluginManagementService; + let configStore: string[]; + let configService: { + get: ReturnType; + set: ReturnType; + }; + let dependencyService: { + npm: ReturnType; + rebuildVendorArchive: ReturnType; + }; + + beforeEach(() => { + configStore = ['unraid-api-plugin-connect', '@unraid/test-plugin']; + configService = { + get: vi.fn((key: string, defaultValue?: unknown) => { + if (key === 'api.plugins') { + return configStore ?? defaultValue ?? []; + } + return defaultValue; + }), + set: vi.fn((key: string, value: unknown) => { + if (key === 'api.plugins' && Array.isArray(value)) { + configStore = [...value]; + } + }), + }; + dependencyService = { + npm: vi.fn().mockResolvedValue(undefined), + rebuildVendorArchive: vi.fn().mockResolvedValue(undefined), + }; + + service = new PluginManagementService(configService as never, dependencyService as never); + }); + + it('rebuilds vendor archive when removing unbundled plugins', async () => { + await service.removePlugin('@unraid/test-plugin'); + + expect(dependencyService.npm).toHaveBeenCalledWith('uninstall', '@unraid/test-plugin'); + expect(dependencyService.rebuildVendorArchive).toHaveBeenCalledTimes(1); + expect(configStore).not.toContain('@unraid/test-plugin'); + }); + + it('skips vendor archive when only bundled plugins are removed', async () => { + await service.removePlugin('unraid-api-plugin-connect'); + + expect(dependencyService.npm).not.toHaveBeenCalled(); + expect(dependencyService.rebuildVendorArchive).not.toHaveBeenCalled(); + expect(configStore).not.toContain('unraid-api-plugin-connect'); + }); + + it('does not rebuild vendor archive when bypassing npm uninstall', async () => { + await service.removePluginConfigOnly('@unraid/test-plugin'); + + expect(dependencyService.npm).not.toHaveBeenCalled(); + expect(dependencyService.rebuildVendorArchive).not.toHaveBeenCalled(); + expect(configStore).not.toContain('@unraid/test-plugin'); + }); +}); diff --git a/api/src/unraid-api/plugin/plugin-management.service.ts b/api/src/unraid-api/plugin/plugin-management.service.ts index 7edeb1588..de7fcf49c 100644 --- a/api/src/unraid-api/plugin/plugin-management.service.ts +++ b/api/src/unraid-api/plugin/plugin-management.service.ts @@ -35,8 +35,10 @@ export class PluginManagementService { */ async removePlugin(...plugins: string[]) { const removed = this.removePluginFromConfig(...plugins); - await this.uninstallPlugins(...removed); - await this.dependencyService.rebuildVendorArchive(); + const { unbundledRemoved } = await this.uninstallPlugins(...removed); + if (unbundledRemoved.length > 0) { + await this.dependencyService.rebuildVendorArchive(); + } } /** @@ -100,12 +102,15 @@ export class PluginManagementService { private async uninstallPlugins(...plugins: string[]) { const bundled = plugins.filter((plugin) => this.isBundled(plugin)); const unbundled = plugins.filter((plugin) => !this.isBundled(plugin)); + if (unbundled.length > 0) { await this.dependencyService.npm('uninstall', ...unbundled); } if (bundled.length > 0) { - await this.removeBundledPlugin(...bundled); + await this.removePluginConfigOnly(...bundled); } + + return { bundledRemoved: bundled, unbundledRemoved: unbundled }; } /**------------------------------------------------------------------------ @@ -125,7 +130,13 @@ export class PluginManagementService { return added; } - async removeBundledPlugin(...plugins: string[]) { + /** + * Removes plugins from the config without touching npm (used for bundled/default bypass flow). + * + * @param plugins - The plugins to remove. + * @returns The list of plugins removed from the config. + */ + async removePluginConfigOnly(...plugins: string[]) { const removed = this.removePluginFromConfig(...plugins); return removed; } diff --git a/api/src/unraid-api/plugin/plugin.resolver.ts b/api/src/unraid-api/plugin/plugin.resolver.ts index 2a2e4e412..cfa9505ba 100644 --- a/api/src/unraid-api/plugin/plugin.resolver.ts +++ b/api/src/unraid-api/plugin/plugin.resolver.ts @@ -75,7 +75,7 @@ export class PluginResolver { }) async removePlugin(@Args('input') input: PluginManagementInput): Promise { if (input.bundled) { - await this.pluginManagementService.removeBundledPlugin(...input.names); + await this.pluginManagementService.removePluginConfigOnly(...input.names); } else { await this.pluginManagementService.removePlugin(...input.names); }