fix: remove connect api plugin upon removal of Connect Unraid plugin (#1548)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Improved plugin removal process on Unraid 7.2 and above by ensuring
the associated API plugin component is actively uninstalled during
plugin removal.
* **Enhancements**
* API version is now consistently set during application startup and
configuration migration.
* Configuration file writing logs now include detailed file paths for
better traceability.
  * File operations now use atomic writes for increased reliability.
* **Chores**
  * Updated dependencies to include atomic file writing support.
* Removed redundant configuration persistence calls after plugin
changes.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

api/package.json

@@ -79,6 +79,7 @@
         "@unraid/libvirt": "2.1.0",
         "@unraid/shared": "workspace:*",
         "accesscontrol": "2.2.1",
+        "atomically": "2.0.3",
         "bycontract": "2.0.11",
         "bytes": "3.1.2",
         "cache-manager": "7.0.1",

api/src/unraid-api/config/api-config.module.ts

@@ -1,4 +1,4 @@
-import { Injectable, Logger, Module } from '@nestjs/common';
+import { Injectable, Logger, Module, OnApplicationBootstrap } from '@nestjs/common';
 import { ConfigService, registerAs } from '@nestjs/config';
 import path from 'path';
 
@@ -50,7 +50,10 @@ export const loadApiConfig = async () => {
 export const apiConfig = registerAs<ApiConfig>('api', loadApiConfig);
 
 @Injectable()
-export class ApiConfigPersistence extends ConfigFilePersister<ApiConfig> {
+export class ApiConfigPersistence
+    extends ConfigFilePersister<ApiConfig>
+    implements OnApplicationBootstrap
+{
     constructor(configService: ConfigService) {
         super(configService);
     }
@@ -79,12 +82,17 @@ export class ApiConfigPersistence extends ConfigFilePersister<ApiConfig> {
         return createDefaultConfig();
     }
 
+    async onApplicationBootstrap() {
+        this.configService.set('api.version', API_VERSION);
+    }
+
     async migrateConfig(): Promise<ApiConfig> {
         const legacyConfig = this.configService.get('store.config', {});
         const migrated = this.convertLegacyConfig(legacyConfig);
         return {
             ...this.defaultConfig(),
             ...migrated,
+            version: API_VERSION,
         };
     }
 

api/src/unraid-api/plugin/plugin-management.service.ts

@@ -4,14 +4,12 @@ import { ConfigService } from '@nestjs/config';
 import { ApiConfig } from '@unraid/shared/services/api-config.js';
 
 import { DependencyService } from '@app/unraid-api/app/dependency.service.js';
-import { ApiConfigPersistence } from '@app/unraid-api/config/api-config.module.js';
 
 @Injectable()
 export class PluginManagementService {
     constructor(
         private readonly configService: ConfigService<{ api: ApiConfig }, true>,
-        private readonly dependencyService: DependencyService,
+        private readonly dependencyService: DependencyService
-        private readonly apiConfigPersistence: ApiConfigPersistence
     ) {}
 
     get plugins() {
@@ -20,14 +18,12 @@ export class PluginManagementService {
 
     async addPlugin(...plugins: string[]) {
         const added = this.addPluginToConfig(...plugins);
-        await this.persistConfig();
         await this.installPlugins(...added);
         await this.dependencyService.rebuildVendorArchive();
     }
 
     async removePlugin(...plugins: string[]) {
         const removed = this.removePluginFromConfig(...plugins);
-        await this.persistConfig();
         await this.uninstallPlugins(...removed);
         await this.dependencyService.rebuildVendorArchive();
     }
@@ -101,17 +97,11 @@ export class PluginManagementService {
 
     async addBundledPlugin(...plugins: string[]) {
         const added = this.addPluginToConfig(...plugins);
-        await this.persistConfig();
         return added;
     }
 
     async removeBundledPlugin(...plugins: string[]) {
         const removed = this.removePluginFromConfig(...plugins);
-        await this.persistConfig();
         return removed;
     }
-
-    private async persistConfig() {
-        return await this.apiConfigPersistence.persist();
-    }
 }

packages/unraid-shared/package.json

@@ -49,6 +49,7 @@
         "@nestjs/common": "11.1.5",
         "@nestjs/config": "4.0.2",
         "@nestjs/graphql": "13.1.0",
+        "atomically": "2.0.3",
         "class-validator": "0.14.2",
         "graphql": "16.11.0",
         "graphql-scalars": "1.24.2",

packages/unraid-shared/src/util/config-file-handler.ts

@@ -1,5 +1,5 @@
 import { Logger } from "@nestjs/common";
-import { readFile, writeFile } from "node:fs/promises";
+import { readFile, writeFile } from "atomically";
 import { isEqual } from "lodash-es";
 import { ConfigDefinition } from "./config-definition.js";
 import { fileExists } from "./file.js";
@@ -122,7 +122,7 @@ export class ConfigFileHandler<T extends object> {
 
     try {
       const data = JSON.stringify(config, null, 2);
-      this.logger.verbose("Writing config");
+      this.logger.verbose(`Writing config to ${this.definition.configPath()}`);
       await writeFile(this.definition.configPath(), data);
       return true;
     } catch (error) {

packages/unraid-shared/src/util/file.ts

@@ -1,8 +1,8 @@
-import { accessSync } from 'fs';
+import { accessSync } from "fs";
-import { access, mkdir, writeFile } from 'fs/promises';
+import { access, mkdir, writeFile } from "fs/promises";
-import { mkdirSync, writeFileSync } from 'fs';
+import { mkdirSync, writeFileSync } from "fs";
-import { F_OK } from 'node:constants';
+import { F_OK } from "node:constants";
-import { dirname } from 'path';
+import { dirname } from "path";
 
 /**
  * Checks if a file exists asynchronously.
@@ -10,9 +10,9 @@ import { dirname } from 'path';
  * @returns Promise that resolves to true if file exists, false otherwise
  */
 export const fileExists = async (path: string) =>
-    access(path, F_OK)
+  access(path, F_OK)
-        .then(() => true)
+    .then(() => true)
-        .catch(() => false);
+    .catch(() => false);
 
 /**
  * Checks if a file exists synchronously.
@@ -20,51 +20,50 @@ export const fileExists = async (path: string) =>
  * @returns true if file exists, false otherwise
  */
 export const fileExistsSync = (path: string) => {
-    try {
+  try {
-        accessSync(path, F_OK);
+    accessSync(path, F_OK);
-        return true;
+    return true;
-    } catch (error: unknown) {
+  } catch (error: unknown) {
-        return false;
+    return false;
-    }
+  }
 };
 
 /**
  * Writes data to a file, creating parent directories if they don't exist.
- * 
+ *
  * This function ensures the directory structure exists before writing the file,
  * equivalent to `mkdir -p` followed by file writing.
- * 
+ *
  * @param path - The file path to write to
  * @param data - The data to write (string or Buffer)
  * @throws {Error} If path is invalid (null, empty, or not a string)
  * @throws {Error} For any file system errors (EACCES, EPERM, ENOSPC, EISDIR, etc.)
  */
 export const ensureWrite = async (path: string, data: string | Buffer) => {
-    if (!path || typeof path !== 'string') {
+  if (!path || typeof path !== "string") {
-        throw new Error(`Invalid path provided: ${path}`);
+    throw new Error(`Invalid path provided: ${path}`);
-    }
+  }
 
-    await mkdir(dirname(path), { recursive: true });
+  await mkdir(dirname(path), { recursive: true });
-    return await writeFile(path, data);
+  return await writeFile(path, data);
 };
 
 /**
  * Writes data to a file synchronously, creating parent directories if they don't exist.
- * 
+ *
  * This function ensures the directory structure exists before writing the file,
  * equivalent to `mkdir -p` followed by file writing.
- * 
+ *
  * @param path - The file path to write to
  * @param data - The data to write (string or Buffer)
  * @throws {Error} If path is invalid (null, empty, or not a string)
  * @throws {Error} For any file system errors (EACCES, EPERM, ENOSPC, EISDIR, etc.)
  */
 export const ensureWriteSync = (path: string, data: string | Buffer) => {
-    if (!path || typeof path !== 'string') {
+  if (!path || typeof path !== "string") {
-        throw new Error(`Invalid path provided: ${path}`);
+    throw new Error(`Invalid path provided: ${path}`);
-    }
+  }
 
-    mkdirSync(dirname(path), { recursive: true });
+  mkdirSync(dirname(path), { recursive: true });
-    return writeFileSync(path, data);
+  return writeFileSync(path, data);
 };
-

plugin/plugins/dynamix.unraid.net.plg

@@ -198,7 +198,10 @@ exit 0
       
       if [ "$is_7_2_or_higher" = true ]; then
         echo "Unraid 7.2+ detected. Using safe removal method."
-        
+        if ! /etc/rc.d/rc.unraid-api plugins remove unraid-api-plugin-connect -b; then
+          echo "Warning: Failed to remove connect API plugin"
+        fi
+
         # Send notification to user
         /usr/local/emhttp/webGui/scripts/notify \
           -e "Unraid Connect" \

pnpm-lock.yaml

@@ -118,6 +118,9 @@ importers:
       accesscontrol:
         specifier: 2.2.1
         version: 2.2.1
+      atomically:
+        specifier: 2.0.3
+        version: 2.0.3
       bycontract:
         specifier: 2.0.11
         version: 2.0.11
@@ -721,6 +724,9 @@ importers:
       '@nestjs/config':
         specifier: 4.0.2
         version: 4.0.2(@nestjs/common@11.1.5(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.1.14)(rxjs@7.8.2))(rxjs@7.8.2)
+      atomically:
+        specifier: 2.0.3
+        version: 2.0.3
       rxjs:
         specifier: 7.8.2
         version: 7.8.2
@@ -6051,6 +6057,9 @@ packages:
     resolution: {integrity: sha512-kNOjDqAh7px0XWNI+4QbzoiR/nTkHAWNud2uvnJquD1/x5a7EQZMJT0AczqK0Qn67oY/TTQ1LbUKajZpp3I9tQ==}
     engines: {node: '>=8.0.0'}
 
+  atomically@2.0.3:
+    resolution: {integrity: sha512-kU6FmrwZ3Lx7/7y3hPS5QnbJfaohcIul5fGqf7ok+4KklIEk9tJ0C2IQPdacSbVUWv6zVHXEBWoWd6NrVMT7Cw==}
+
   auto-bind@4.0.0:
     resolution: {integrity: sha512-Hdw8qdNiqdJ8LqT0iK0sVzkFbzg6fhnQqqfWhBDxcHZvU75+B+ayzTy8x+k5Ix0Y92XOhOUlx74ps+bA6BeYMQ==}
     engines: {node: '>=8'}
@@ -12290,6 +12299,9 @@ packages:
   structured-clone-es@1.0.0:
     resolution: {integrity: sha512-FL8EeKFFyNQv5cMnXI31CIMCsFarSVI2bF0U0ImeNE3g/F1IvJQyqzOXxPBRXiwQfyBTlbNe88jh1jFW0O/jiQ==}
 
+  stubborn-fs@1.2.5:
+    resolution: {integrity: sha512-H2N9c26eXjzL/S/K+i/RHHcFanE74dptvvjM8iwzwbVcWY/zjBbgRqF3K0DY4+OD+uTTASTBvDoxPDaPN02D7g==}
+
   stylehacks@7.0.5:
     resolution: {integrity: sha512-5kNb7V37BNf0Q3w+1pxfa+oiNPS++/b4Jil9e/kPDgrk1zjEd6uR7SZeJiYaLYH6RRSC1XX2/37OTeU/4FvuIA==}
     engines: {node: ^18.12.0 || ^20.9.0 || >=22.0}
@@ -13516,6 +13528,9 @@ packages:
     resolution: {integrity: sha512-f+Gy33Oa5Z14XY9679Zze+7VFhbsQfBFXodnU2x589l4kxGM9L5Y8zETTmcMR5pWOPQyRv4Z0lNax6xCO0NSlA==}
     engines: {node: '>=18'}
 
+  when-exit@2.1.4:
+    resolution: {integrity: sha512-4rnvd3A1t16PWzrBUcSDZqcAmsUIy4minDXT/CZ8F2mVDgd65i4Aalimgz1aQkRGU0iH5eT5+6Rx2TK8o443Pg==}
+
   which-boxed-primitive@1.1.1:
     resolution: {integrity: sha512-TbX3mj8n0odCBFVlY8AxkqcHASw3L60jIuF8jFP78az3C2YhmGvqbHBpAjTRH2/xqYunrJ9g1jSyjCjpoWzIAA==}
     engines: {node: '>= 0.4'}
@@ -19249,6 +19264,11 @@ snapshots:
 
   atomic-sleep@1.0.0: {}
 
+  atomically@2.0.3:
+    dependencies:
+      stubborn-fs: 1.2.5
+      when-exit: 2.1.4
+
   auto-bind@4.0.0: {}
 
   autoprefixer@10.4.21(postcss@8.5.6):
@@ -26570,6 +26590,8 @@ snapshots:
 
   structured-clone-es@1.0.0: {}
 
+  stubborn-fs@1.2.5: {}
+
   stylehacks@7.0.5(postcss@8.5.6):
     dependencies:
       browserslist: 4.25.0
@@ -27909,7 +27931,7 @@ snapshots:
       '@webassemblyjs/wasm-edit': 1.14.1
       '@webassemblyjs/wasm-parser': 1.14.1
       acorn: 8.15.0
-      browserslist: 4.25.0
+      browserslist: 4.25.1
       chrome-trace-event: 1.0.4
       enhanced-resolve: 5.18.1
       es-module-lexer: 1.7.0
@@ -27956,6 +27978,8 @@ snapshots:
 
   wheel-gestures@2.2.48: {}
 
+  when-exit@2.1.4: {}
+
   which-boxed-primitive@1.1.1:
     dependencies:
       is-bigint: 1.1.0