feat(admin): show "requires newer Etherpad" when installing incompatible plugin (#7763)

admin/src/pages/HomePage.tsx

@@ -68,7 +68,17 @@ export const HomePage = () => {
       )
       setInstalledPlugins(updated)
     }
-    const onFinishedInstall = () => {
+    const onFinishedInstall = (data: {plugin: string; code?: string | null; error?: string | null}) => {
+      if (data?.error) {
+        const key = data.code === 'PLUGIN_REQUIRES_NEWER_ETHERPAD'
+          ? 'admin_plugins.install_error_requires_newer_etherpad'
+          : 'admin_plugins.install_error'
+        useStore.getState().setToastState({
+          open: true,
+          title: t(key, {plugin: data.plugin, error: data.error}),
+          success: false,
+        })
+      }
       pluginsSocket.emit('getInstalled')
     }
     const onFinishedUninstall = () => {

src/locales/en.json

@@ -66,6 +66,8 @@
   "admin_plugins.disables.label": "Disables:",
   "admin_plugins.disables.warning_title": "This plugin intentionally removes the listed Etherpad features.",
   "admin_plugins.error_retrieving": "Error retrieving plugins",
+  "admin_plugins.install_error": "Failed to install {{plugin}}: {{error}}",
+  "admin_plugins.install_error_requires_newer_etherpad": "Cannot install {{plugin}}: it requires a newer version of Etherpad. Please upgrade Etherpad and try again.",
   "admin_plugins.installed": "Installed plugins",
   "admin_plugins.installed_fetching": "Fetching installed plugins…",
   "admin_plugins.installed_nothing": "You haven't installed any plugins yet.",

src/static/js/pluginfw/installer.ts

@@ -17,9 +17,15 @@ import  settings, {
   reloadSettings
 } from '../../../node/utils/Settings';
 import {LinkInstaller} from "./LinkInstaller";
+import {
+  checkEngineCompatibility,
+  EngineIncompatibleError,
+} from './pluginEngineCheck';
+import {InstallerTaskQueue} from './installerTasks';
 
 import {findEtherpadRoot} from '../../../node/utils/AbsolutePaths';
 const logger = log4js.getLogger('plugins');
+const npmRegistry = 'https://registry.npmjs.org';
 
 export const pluginInstallPath = path.join(settings.root, 'src','plugin_packages');
 export const node_modules = path.join(findEtherpadRoot(),'src', 'node_modules');
@@ -38,19 +44,10 @@ const headers = {
   'User-Agent': `Etherpad/${getEpVersion()}`,
 };
 
-let tasks = 0;
-
 export const linkInstaller = new LinkInstaller();
 
-const wrapTaskCb = (cb:Function|null) => {
-  tasks++;
-
-  return (...args: any) => {
-    cb && cb(...args);
-    tasks--;
-    if (tasks === 0) onAllTasksFinished();
-  };
-};
+const taskQueue = new InstallerTaskQueue(onAllTasksFinished);
+const wrapTaskCb = (cb: Function | null) => taskQueue.wrap(cb);
 
 const migratePluginsFromNodeModules = async () => {
   logger.info('start migration of plugins in node_modules');
@@ -156,13 +153,46 @@ export const uninstall = async (pluginName: string, cb:Function|null = null) =>
   cb(null);
 };
 
+// Best-effort lookup of the published plugin's engines.node range. Returns
+// undefined on any failure (network, 404, parse error, timeout) so the
+// caller falls through to the existing install path rather than blocking on
+// a flaky registry call. A 5s AbortSignal.timeout guards against a stalled
+// registry hanging the install promise forever — without it the
+// finished:install socket event would never fire and the admin UI would
+// stay spinning indefinitely.
+const ENGINES_PREFLIGHT_TIMEOUT_MS = 5000;
+const fetchPluginEnginesNode = async (pluginName: string): Promise<string | undefined> => {
+  try {
+    const res = await fetch(
+      `${npmRegistry}/${encodeURIComponent(pluginName)}/latest`,
+      {headers, signal: AbortSignal.timeout(ENGINES_PREFLIGHT_TIMEOUT_MS)},
+    );
+    if (!res.ok) return undefined;
+    const data = await res.json() as {engines?: {node?: string}};
+    return data.engines?.node;
+  } catch (err) {
+    logger.debug(`engines preflight for ${pluginName} fell through: ${err}`);
+    return undefined;
+  }
+};
+
 export const install = async (pluginName: string, cb:Function|null = null) => {
   cb = wrapTaskCb(cb);
   logger.info(`Installing plugin ${pluginName}...`);
-  await linkInstaller.installPlugin(pluginName);
-  logger.info(`Successfully installed plugin ${pluginName}`);
-  await hooks.aCallAll('pluginInstall', {pluginName});
-  cb(null);
+  try {
+    const enginesNode = await fetchPluginEnginesNode(pluginName);
+    const compat = checkEngineCompatibility(enginesNode, process.version);
+    if (!compat.compatible) {
+      throw new EngineIncompatibleError(pluginName, compat.required, compat.current);
+    }
+    await linkInstaller.installPlugin(pluginName);
+    logger.info(`Successfully installed plugin ${pluginName}`);
+    await hooks.aCallAll('pluginInstall', {pluginName});
+    cb(null);
+  } catch (err) {
+    logger.warn(`Failed to install plugin ${pluginName}: ${err}`);
+    cb(err);
+  }
 };
 
 export let availablePlugins:MapArrayType<PackageInfo>|null = null;

src/static/js/pluginfw/installerTasks.ts

@@ -0,0 +1,31 @@
+'use strict';
+
+// Tracks the in-flight batch of install/uninstall operations and decides when
+// to fire the "all tasks finished" side effect (reload settings, restart
+// server). Extracted so it can be unit-tested without dragging in the
+// LinkInstaller circular import.
+//
+// Crucial invariant: onFinished MUST NOT run when every task in the batch
+// failed — that path is reachable now that install() correctly propagates
+// errors to its callback. Restarting the server on a failed install would
+// disconnect every connected pad for no reason.
+export class InstallerTaskQueue {
+  private tasks = 0;
+  private anyTaskSucceeded = false;
+
+  constructor(private readonly onFinished: () => unknown) {}
+
+  wrap(cb: Function | null): (...args: unknown[]) => void {
+    this.tasks++;
+    return (...args: unknown[]) => {
+      if (!args[0]) this.anyTaskSucceeded = true;
+      if (cb) cb(...args);
+      this.tasks--;
+      if (this.tasks === 0) {
+        const shouldFinish = this.anyTaskSucceeded;
+        this.anyTaskSucceeded = false;
+        if (shouldFinish) this.onFinished();
+      }
+    };
+  }
+}

src/static/js/pluginfw/pluginEngineCheck.ts

@@ -0,0 +1,47 @@
+'use strict';
+
+import semver from 'semver';
+
+export type EngineCheckResult =
+  | {compatible: true}
+  | {compatible: false; required: string; current: string};
+
+/**
+ * Compares a plugin's `engines.node` range against the runtime Node version.
+ *
+ * Returned compatibility is used at plugin install time to short-circuit the
+ * install with a user-facing "requires a newer version of Etherpad" message
+ * instead of letting live-plugin-manager unpack a plugin that will fail at
+ * load time on the running Node.
+ *
+ * Unparseable ranges return `compatible: true` deliberately — this preflight
+ * is opportunistic, not a gatekeeper. If we can't make sense of the range
+ * we fall through to the existing install path.
+ */
+export const checkEngineCompatibility = (
+  pluginEnginesNode: string | undefined,
+  currentNodeVersion: string,
+): EngineCheckResult => {
+  if (!pluginEnginesNode) return {compatible: true};
+  const current = currentNodeVersion.replace(/^v/, '');
+  if (!semver.validRange(pluginEnginesNode)) return {compatible: true};
+  if (semver.satisfies(current, pluginEnginesNode, {includePrerelease: true})) {
+    return {compatible: true};
+  }
+  return {compatible: false, required: pluginEnginesNode, current};
+};
+
+export class EngineIncompatibleError extends Error {
+  public readonly code = 'PLUGIN_REQUIRES_NEWER_ETHERPAD';
+  constructor(
+    public readonly pluginName: string,
+    public readonly required: string,
+    public readonly current: string,
+  ) {
+    super(
+      `Plugin ${pluginName} requires a newer version of Etherpad. ` +
+      'Please upgrade Etherpad and try again.',
+    );
+    this.name = 'EngineIncompatibleError';
+  }
+}

src/tests/backend-new/specs/installerTasks.test.ts

@@ -0,0 +1,92 @@
+'use strict';
+
+import {describe, it, expect, vi} from 'vitest';
+import {InstallerTaskQueue} from '../../../static/js/pluginfw/installerTasks';
+
+describe('InstallerTaskQueue', () => {
+  it('fires onFinished after a single successful task', () => {
+    const onFinished = vi.fn();
+    const q = new InstallerTaskQueue(onFinished);
+    const cb = vi.fn();
+    const wrapped = q.wrap(cb);
+
+    wrapped(null);
+
+    expect(cb).toHaveBeenCalledWith(null);
+    expect(onFinished).toHaveBeenCalledTimes(1);
+  });
+
+  it('does NOT fire onFinished when the only task in the batch failed', () => {
+    // Regression: before this fix, a failed install(EngineIncompatibleError)
+    // would still trigger restartServer via onAllTasksFinished, kicking every
+    // connected pad off the server for no benefit. Reported by Qodo on
+    // PR #7771.
+    const onFinished = vi.fn();
+    const q = new InstallerTaskQueue(onFinished);
+    const wrapped = q.wrap(vi.fn());
+
+    wrapped(new Error('plugin requires a newer version of Etherpad'));
+
+    expect(onFinished).not.toHaveBeenCalled();
+  });
+
+  it('fires onFinished when at least one task in a mixed batch succeeded', () => {
+    const onFinished = vi.fn();
+    const q = new InstallerTaskQueue(onFinished);
+    const ok = q.wrap(vi.fn());
+    const bad = q.wrap(vi.fn());
+
+    bad(new Error('boom'));
+    expect(onFinished).not.toHaveBeenCalled();
+    ok(null);
+
+    expect(onFinished).toHaveBeenCalledTimes(1);
+  });
+
+  it('does NOT fire onFinished when every task in a multi-task batch failed', () => {
+    const onFinished = vi.fn();
+    const q = new InstallerTaskQueue(onFinished);
+    const a = q.wrap(vi.fn());
+    const b = q.wrap(vi.fn());
+
+    a(new Error('one'));
+    b(new Error('two'));
+
+    expect(onFinished).not.toHaveBeenCalled();
+  });
+
+  it('resets the success flag between batches', () => {
+    const onFinished = vi.fn();
+    const q = new InstallerTaskQueue(onFinished);
+
+    const ok = q.wrap(vi.fn());
+    ok(null);
+    expect(onFinished).toHaveBeenCalledTimes(1);
+
+    const bad = q.wrap(vi.fn());
+    bad(new Error('next batch all failed'));
+    expect(onFinished).toHaveBeenCalledTimes(1);
+  });
+
+  it('tolerates a null callback', () => {
+    const onFinished = vi.fn();
+    const q = new InstallerTaskQueue(onFinished);
+    const wrapped = q.wrap(null);
+
+    expect(() => wrapped(null)).not.toThrow();
+    expect(onFinished).toHaveBeenCalledTimes(1);
+  });
+
+  it('only fires onFinished once all wrapped tasks have completed', () => {
+    const onFinished = vi.fn();
+    const q = new InstallerTaskQueue(onFinished);
+    const a = q.wrap(vi.fn());
+    const b = q.wrap(vi.fn());
+
+    a(null);
+    expect(onFinished).not.toHaveBeenCalled();
+    b(null);
+
+    expect(onFinished).toHaveBeenCalledTimes(1);
+  });
+});

src/tests/backend-new/specs/pluginEngineCheck.test.ts

@@ -0,0 +1,65 @@
+'use strict';
+
+import {describe, it, expect} from 'vitest';
+import {
+  checkEngineCompatibility,
+  EngineIncompatibleError,
+} from '../../../static/js/pluginfw/pluginEngineCheck';
+
+describe('pluginEngineCheck', () => {
+  describe('checkEngineCompatibility', () => {
+    it('returns compatible when plugin declares no engines.node', () => {
+      expect(checkEngineCompatibility(undefined, 'v22.13.0').compatible).toBe(true);
+    });
+
+    it('returns compatible when the current Node satisfies the range', () => {
+      expect(checkEngineCompatibility('>=22.13.0', 'v22.13.0').compatible).toBe(true);
+      expect(checkEngineCompatibility('>=22.13.0', 'v25.1.0').compatible).toBe(true);
+    });
+
+    it('returns incompatible when the current Node is below the required range', () => {
+      const result = checkEngineCompatibility('>=25.0.0', 'v22.13.0');
+      expect(result.compatible).toBe(false);
+      if (!result.compatible) {
+        expect(result.required).toBe('>=25.0.0');
+        expect(result.current).toBe('22.13.0');
+      }
+    });
+
+    it('strips the leading "v" from process.version-style strings', () => {
+      const result = checkEngineCompatibility('>=25.0.0', 'v22.13.0');
+      expect(result.compatible).toBe(false);
+      if (!result.compatible) expect(result.current).not.toMatch(/^v/);
+    });
+
+    it('treats malformed engines strings as compatible (do not block install)', () => {
+      // If the plugin's engines field is garbage we should not block — let
+      // live-plugin-manager handle it. The whole point of this preflight is
+      // to give a useful message; if we can't parse the range, fall through.
+      expect(checkEngineCompatibility('not-a-range', 'v22.13.0').compatible).toBe(true);
+    });
+  });
+
+  describe('EngineIncompatibleError', () => {
+    it('carries a stable code and the plugin name', () => {
+      const err = new EngineIncompatibleError('ep_test', '>=25.0.0', '22.13.0');
+      expect(err.code).toBe('PLUGIN_REQUIRES_NEWER_ETHERPAD');
+      expect(err.pluginName).toBe('ep_test');
+      expect(err.required).toBe('>=25.0.0');
+      expect(err.current).toBe('22.13.0');
+    });
+
+    it('produces a user-facing message that does not mention Node', () => {
+      // Message reaches the admin UI. Per design, the admin is told to
+      // upgrade Etherpad — Node is an implementation detail.
+      const err = new EngineIncompatibleError('ep_test', '>=25.0.0', '22.13.0');
+      expect(err.message).toMatch(/newer version of Etherpad/);
+      expect(err.message.toLowerCase()).not.toMatch(/node/);
+    });
+
+    it('is an instance of Error', () => {
+      const err = new EngineIncompatibleError('ep_test', '>=25.0.0', '22.13.0');
+      expect(err).toBeInstanceOf(Error);
+    });
+  });
+});