feat: api plugin management via CLI #1416
Conversation
WalkthroughA new "plugins" property was introduced across the API configuration, including schema, config files, and service layers. A Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (Install/Remove/ListPluginCommand)
participant PMS as PluginManagementService
participant DepSvc as DependencyService
participant Config as ConfigService
CLI->>PMS: addPlugin / removePlugin / listPlugins
PMS->>Config: update plugins config & persist
PMS->>DepSvc: npm install/uninstall (for npm plugins)
PMS->>DepSvc: rebuildVendorArchive (if needed)
PMS-->>CLI: operation complete
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (5)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai pause |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
api/src/unraid-api/config/api-config.module.ts (1)
20-21: Default array OK – consider freezing it
plugins: []is a mutable default that gets shared via object spreading. Accidentally pushing to this array before persistence would mutate the singleton object and bleed across requests/tests.- plugins: [], + plugins: Object.freeze([]) as string[],Alternatively, recreate a fresh empty array inside
createDefaultConfig().
[nitpick but avoids sneaky state leakage]api/dev/configs/api.json (1)
8-9: Empty array is fine – add trailing comment or doc entry
"plugins": []matches the new schema but there’s no accompanying documentation inREADME/ sample config. A short inline comment or docs update would help operators understand acceptable values (npm package names, git URLs, etc.).packages/unraid-shared/src/services/api-config.ts (1)
25-29: Validation decorators look good – consider@IsOptional()If
pluginscan legitimately be absent (legacy configs), adding@IsOptional()will keep validation green. Up-migration already supplies[], but optionality makes the contract friendlier.- @IsArray() - @IsString({ each: true }) - plugins!: string[]; + @IsOptional() + @IsArray() + @IsString({ each: true }) + plugins?: string[];api/src/unraid-api/plugin/plugin.module.ts (1)
46-47:PluginServicemissing from CLI providers
PluginCliModule.register()callsPluginService.getPlugins()(static), but any future non-static methods will require DI. AddingPluginServiceto its providers now prevents a later “provider not found” error when refactoring.- providers: [PluginManagementService], + providers: [PluginService, PluginManagementService],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
api/dev/configs/api.json(1 hunks)api/src/unraid-api/cli/plugins/plugin.command.ts(2 hunks)api/src/unraid-api/config/api-config.module.ts(1 hunks)api/src/unraid-api/graph/resolvers/settings/settings.service.ts(1 hunks)api/src/unraid-api/plugin/plugin-management.service.ts(1 hunks)api/src/unraid-api/plugin/plugin.module.ts(3 hunks)packages/unraid-shared/src/services/api-config.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
api/src/unraid-api/plugin/plugin-management.service.ts (1)
api/src/unraid-api/graph/resolvers/settings/settings.service.ts (1)
Injectable(14-160)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Test API
- GitHub Check: Build Web App
- GitHub Check: Build Unraid UI Library (Webcomponent Version)
- GitHub Check: Build API
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages
✅ Actions performedReviews paused. |
doesn't run rn
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
api/src/unraid-api/plugin/plugin-management.service.ts (1)
60-66: Symmetric issue in removalSame undefined risk as above; wrap with default array.
- const pluginSet = new Set(this.plugins); + const pluginSet = new Set(this.plugins ?? []);
🧹 Nitpick comments (7)
api/src/utils.ts (1)
296-310: Edge-case handling inparsePackageArgcould be more robustCurrent heuristic (
lastIndexOf('@'), cheap slash check) is fine for the basic cases, but it silently accepts odd inputs such aspackage@,@scope/pkg@, orpkg@1.0.0@foo, which are not valid npm specifiers. Down-stream code might try to install such strings and fail much later.Consider either:
- Delegating to the battle-tested
npm-package-arglibrary, or- Tightening the parser with a small regex that rejects empty versions and double-@ artefacts.
If you keep the custom impl, at least guard with:
- if (atIndex > 0) { + if (atIndex > 0 && atIndex < packageArg.length - 1) { // avoid trailing '@'and possibly throw on unexpected patterns instead of silently folding them into
name.api/src/unraid-api/cli/log.service.ts (1)
22-26: Routetable()through the configured logger
console.tablewrites directly to stdout, bypassingthis.loggerand any future log transport or formatting you may introduce. Forward throughthis.loggerto stay consistent:- console.table(data, columns); + (this.logger as any).table ? (this.logger as any).table(data, columns) : this.logger.log(data);(Node ≥ 10 ships
console.table, so this keeps compatibility if you ever swap the logger.)api/src/unraid-api/cli/cli.module.ts (1)
59-63: Possible duplicate providers for plugin commands
PluginCliModule.register()already exports the same command classes; adding them again inDEFAULT_COMMANDSmay create multiple provider instances or, at best, unnecessary metadata noise.If
PluginCliModulehandles registration, you can safely drop the explicit entries here.api/src/unraid-api/plugin/plugin.service.ts (1)
74-80: Minor: simplify predicateThe local variable
versionisn’t used – thetypeofcheck suffices on the tuple.- (entry): entry is [string, string] => { - const [pkgName, version] = entry; - return pluginNames.has(pkgName) && typeof version === 'string'; - } + ([pkgName, version]): entry is [string, string] => + pluginNames.has(pkgName) && typeof version === 'string'api/src/unraid-api/app/dependency.service.ts (2)
9-12: Remove pointless constructorThe class defines no state; the empty constructor only adds noise and is flagged by Biome.
- constructor() {}🧰 Tools
🪛 Biome (1.9.4)
[error] 11-11: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
19-24: Avoid recomputingcwd
path.dirname(getPackageJsonPath())is resolved on everynpm()call.
Cache it once for micro-efficiency and cleaner stack traces.- async npm(...npmArgs: string[]) { - return await execa('npm', [...npmArgs], { - stdio: 'inherit', - cwd: path.dirname(getPackageJsonPath()), - }); + private readonly projectRoot = path.dirname(getPackageJsonPath()); + + async npm(...npmArgs: string[]) { + return execa('npm', npmArgs, { + stdio: 'inherit', + cwd: this.projectRoot, + });api/src/unraid-api/cli/plugins/plugin.command.ts (1)
63-65: Option parser ignores supplied value
parseRestartunconditionally returnsfalse.
Return the provided flag value to avoid unexpected behaviour when Commander passes explicit truthy / falsy input.- parseRestart(value: boolean): boolean { - return false; + parseRestart(value: boolean): boolean { + return value; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
api/src/__test__/utils.test.ts(2 hunks)api/src/unraid-api/app/dependency.service.ts(1 hunks)api/src/unraid-api/cli/cli.module.ts(4 hunks)api/src/unraid-api/cli/log.service.ts(1 hunks)api/src/unraid-api/cli/plugins/dependency.service.ts(0 hunks)api/src/unraid-api/cli/plugins/plugin.cli.module.ts(0 hunks)api/src/unraid-api/cli/plugins/plugin.command.ts(5 hunks)api/src/unraid-api/config/api-config.module.ts(2 hunks)api/src/unraid-api/plugin/plugin-management.service.ts(1 hunks)api/src/unraid-api/plugin/plugin.module.ts(3 hunks)api/src/unraid-api/plugin/plugin.service.ts(2 hunks)api/src/utils.ts(1 hunks)plugin/plugins/dynamix.unraid.net.plg(1 hunks)
💤 Files with no reviewable changes (2)
- api/src/unraid-api/cli/plugins/plugin.cli.module.ts
- api/src/unraid-api/cli/plugins/dependency.service.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- api/src/unraid-api/plugin/plugin.module.ts
- api/src/unraid-api/config/api-config.module.ts
🧰 Additional context used
🧠 Learnings (2)
plugin/plugins/dynamix.unraid.net.plg (1)
Learnt from: pujitm
PR: unraid/api#1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.348Z
Learning: For the Unraid Connect plugin, the script `/etc/rc.d/rc.unraid-api` is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.
api/src/unraid-api/app/dependency.service.ts (1)
Learnt from: pujitm
PR: unraid/api#1352
File: api/src/unraid-api/cli/dependency.service.ts:114-117
Timestamp: 2025-04-21T18:25:16.491Z
Learning: Methods in `DependencyService` like `npmInstall` and `rebuildVendorArchive` intentionally propagate errors from execa calls upward rather than handling them internally, allowing callers to implement appropriate error handling. This follows the pattern documented in the `rebuildVendorArchive` method's docstring.
🧬 Code Graph Analysis (3)
api/src/unraid-api/cli/log.service.ts (1)
api/src/core/log.ts (1)
LogLevel(8-8)
api/src/__test__/utils.test.ts (1)
api/src/utils.ts (1)
parsePackageArg(296-310)
api/src/unraid-api/plugin/plugin.service.ts (4)
api/src/unraid-api/config/api-config.module.ts (1)
loadApiConfig(35-51)api/src/unraid-api/plugin/plugin-management.service.ts (1)
plugins(16-18)api/src/utils.ts (1)
parsePackageArg(296-310)api/src/environment.ts (1)
getPackageJson(50-54)
🪛 Biome (1.9.4)
api/src/unraid-api/app/dependency.service.ts
[error] 11-11: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build Web App
- GitHub Check: Test API
- GitHub Check: Build API
🔇 Additional comments (7)
api/src/__test__/utils.test.ts (1)
107-180: Great coverage for tricky package stringsThe suite hits plain, scoped, ranged, tagged and malformed inputs—exactly what
parsePackageArgneeds. 👍plugin/plugins/dynamix.unraid.net.plg (2)
245-250: Good call moving dependency cleanup after installRunning
cleanup-dependenciespost-install ensures the fresh config is present before pruning—prevents accidental deletion of the new archive. Looks solid.
257-258: Double-check plugin add before API start
rc.unraid-api plugins add … --no-restartexecutes while the service is still down.
If the command internally shells into the API runtime (e.g., PM2 RPC) it may fail silently.
Verify that the script can operate with the service stopped, or swap the order:/etc/rc.d/rc.unraid-api start /etc/rc.d/rc.unraid-api plugins add unraid-api-plugin-connect -bapi/src/unraid-api/cli/cli.module.ts (2)
75-76: 👍 DependencyService injected at CLI levelIncluding
DependencyServicehere keeps CLI actions in sync with runtime dependency logic—good alignment.
79-79: Small ordering tweak
LegacyConfigModuledepends on env vars set bySwitchEnvCommand; importing it before CLI execution is fine, but if any@OnModuleInitlogic touches the file system, you might want to load config after environment switching.api/src/unraid-api/plugin/plugin-management.service.ts (2)
75-77:npm iflag choice
--save-peerwas removed in npm 7; peer deps are detected automatically when installing with a semver.
Use the standard peer-dep workflow or pin the npm version.- return this.dependencyService.npm('i', '--save-peer', '--save-exact', ...plugins); + return this.dependencyService.npm('i', '--save-exact', ...plugins);
113-115: Persisting possibly-undefined config
configService.get('api')may yieldundefined, leading to an empty file on disk.
Confirm this can’t happen or coalesce with the default config before persisting.
pujitm
changed the title
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
🤖 I have created a release *beep* *boop* --- ## [4.9.0](v4.8.0...v4.9.0) (2025-07-08) ### Features * add graphql resource for API plugins ([#1420](#1420)) ([642a220](642a220)) * add management page for API keys ([#1408](#1408)) ([0788756](0788756)) * add rclone ([#1362](#1362)) ([5517e75](5517e75)) * API key management ([#1407](#1407)) ([d37dc3b](d37dc3b)) * api plugin management via CLI ([#1416](#1416)) ([3dcbfbe](3dcbfbe)) * build out docker components ([#1427](#1427)) ([711cc9a](711cc9a)) * docker and info resolver issues ([#1423](#1423)) ([9901039](9901039)) * fix shading in UPC to be less severe ([#1438](#1438)) ([b7c2407](b7c2407)) * info resolver cleanup ([#1425](#1425)) ([1b279bb](1b279bb)) * initial codeql setup ([#1390](#1390)) ([2ade7eb](2ade7eb)) * initialize claude code in codebse ([#1418](#1418)) ([b6c4ee6](b6c4ee6)) * move api key fetching to use api key service ([#1439](#1439)) ([86bea56](86bea56)) * move to cron v4 ([#1428](#1428)) ([b8035c2](b8035c2)) * move to iframe for changelog ([#1388](#1388)) ([fcd6fbc](fcd6fbc)) * native slackware package ([#1381](#1381)) ([4f63b4c](4f63b4c)) * send active unraid theme to docs ([#1400](#1400)) ([f71943b](f71943b)) * slightly better watch mode ([#1398](#1398)) ([881f1e0](881f1e0)) * upgrade nuxt-custom-elements ([#1461](#1461)) ([345e83b](345e83b)) * use bigint instead of long ([#1403](#1403)) ([574d572](574d572)) ### Bug Fixes * activation indicator removed ([5edfd82](5edfd82)) * alignment of settings on ManagementAccess settings page ([#1421](#1421)) ([70c790f](70c790f)) * allow rclone to fail to initialize ([#1453](#1453)) ([7c6f02a](7c6f02a)) * always download 7.1 versioned files for patching ([edc0d15](edc0d15)) * api `pnpm type-check` ([#1442](#1442)) ([3122bdb](3122bdb)) * **api:** connect config `email` validation ([#1454](#1454)) ([b9a1b9b](b9a1b9b)) * backport unraid/webgui[#2269](https://github.com/unraid/api/issues/2269) rc.nginx update ([#1436](#1436)) ([a7ef06e](a7ef06e)) * bigint ([e54d27a](e54d27a)) * config migration from `myservers.cfg` ([#1440](#1440)) ([c4c9984](c4c9984)) * **connect:** fatal race-condition in websocket disposal ([#1462](#1462)) ([0ec0de9](0ec0de9)) * **connect:** mothership connection ([#1464](#1464)) ([7be8bc8](7be8bc8)) * console hidden ([9b85e00](9b85e00)) * debounce is too long ([#1426](#1426)) ([f12d231](f12d231)) * delete legacy connect keys and ensure description ([22fe91c](22fe91c)) * **deps:** pin dependencies ([#1465](#1465)) ([ba75a40](ba75a40)) * **deps:** pin dependencies ([#1470](#1470)) ([412b329](412b329)) * **deps:** storybook v9 ([#1476](#1476)) ([45bb49b](45bb49b)) * **deps:** update all non-major dependencies ([#1366](#1366)) ([291ee47](291ee47)) * **deps:** update all non-major dependencies ([#1379](#1379)) ([8f70326](8f70326)) * **deps:** update all non-major dependencies ([#1389](#1389)) ([cb43f95](cb43f95)) * **deps:** update all non-major dependencies ([#1399](#1399)) ([68df344](68df344)) * **deps:** update dependency @types/diff to v8 ([#1393](#1393)) ([00da27d](00da27d)) * **deps:** update dependency cache-manager to v7 ([#1413](#1413)) ([9492c2a](9492c2a)) * **deps:** update dependency commander to v14 ([#1394](#1394)) ([106ea09](106ea09)) * **deps:** update dependency diff to v8 ([#1386](#1386)) ([e580f64](e580f64)) * **deps:** update dependency dotenv to v17 ([#1474](#1474)) ([d613bfa](d613bfa)) * **deps:** update dependency lucide-vue-next to ^0.509.0 ([#1383](#1383)) ([469333a](469333a)) * **deps:** update dependency marked to v16 ([#1444](#1444)) ([453a5b2](453a5b2)) * **deps:** update dependency shadcn-vue to v2 ([#1302](#1302)) ([26ecf77](26ecf77)) * **deps:** update dependency vue-sonner to v2 ([#1401](#1401)) ([53ca414](53ca414)) * disable file changes on Unraid 7.2 ([#1382](#1382)) ([02de89d](02de89d)) * do not start API with doinst.sh ([7d88b33](7d88b33)) * do not uninstall fully on 7.2 ([#1484](#1484)) ([2263881](2263881)) * drop console with terser ([a87d455](a87d455)) * error logs from `cloud` query when connect is not installed ([#1450](#1450)) ([719f460](719f460)) * flash backup integration with Unraid Connect config ([#1448](#1448)) ([038c582](038c582)) * header padding regression ([#1477](#1477)) ([e791cc6](e791cc6)) * incorrect state merging in redux store ([#1437](#1437)) ([17b7428](17b7428)) * lanip copy button not present ([#1459](#1459)) ([a280786](a280786)) * move to bigint scalar ([b625227](b625227)) * node_modules dir removed on plugin update ([#1406](#1406)) ([7b005cb](7b005cb)) * omit Connect actions in UPC when plugin is not installed ([#1417](#1417)) ([8c8a527](8c8a527)) * parsing of `ssoEnabled` in state.php ([#1455](#1455)) ([f542c8e](f542c8e)) * pin ranges ([#1460](#1460)) ([f88400e](f88400e)) * pr plugin promotion workflow ([#1456](#1456)) ([13bd9bb](13bd9bb)) * proper fallback if missing paths config modules ([7067e9e](7067e9e)) * rc.unraid-api now cleans up older dependencies ([#1404](#1404)) ([83076bb](83076bb)) * remote access lifecycle during boot & shutdown ([#1422](#1422)) ([7bc583b](7bc583b)) * sign out correctly on error ([#1452](#1452)) ([d08fc94](d08fc94)) * simplify usb listing ([#1402](#1402)) ([5355115](5355115)) * theme issues when sent from graph ([#1424](#1424)) ([75ad838](75ad838)) * **ui:** notifications positioning regression ([#1445](#1445)) ([f73e5e0](f73e5e0)) * use some instead of every for connect detection ([9ce2fee](9ce2fee)) ### Reverts * revert package.json dependency updates from commit 711cc9a for api and packages/* ([94420e4](94420e4)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit