diff --git a/core/PluginManager.js b/core/PluginManager.js index 8f9026b..df90376 100644 --- a/core/PluginManager.js +++ b/core/PluginManager.js @@ -351,7 +351,7 @@ class PluginManager { const discovered = this.discoverPlugins(); const ordered = this.sortByDependencies(discovered); if (this.config.commandCollection && discovered.some((plugin) => plugin.disabled)) { - throw new Error("Cannot collect commands with missing plugin dependencies"); + throw new Error("Cannot collect commands with missing or circular plugin dependencies"); } for (const plugin of ordered) { @@ -482,26 +482,36 @@ class PluginManager { } const ordered = []; - const visiting = new Set(); + const chain = []; const visited = new Set(); + // Returns whether the plugin can load. A cycle, or a dependency that is + // itself disabled, costs only the plugins involved: one bad manifest must + // never abort loadAll for every other plugin. const visit = (name) => { - if (visited.has(name)) return; - if (visiting.has(name)) { - throw new Error(`Circular dependency detected at ${name}`); + const plugin = nodes.get(name); + if (visited.has(name)) return !plugin.disabled; + if (chain.includes(name)) { + const cycle = [...chain.slice(chain.indexOf(name)), name]; + this.logger.warn(`Skipping plugins in circular dependency: ${cycle.join(" -> ")}`); + for (const member of cycle) nodes.get(member).disabled = true; + return false; } - visiting.add(name); + chain.push(name); for (const dep of edges.get(name) || []) { - visit(dep); + if (!visit(dep) && !plugin.disabled) { + this.logger.warn(`Plugin ${name} depends on unavailable plugin ${dep}`); + plugin.disabled = true; + } } - visiting.delete(name); + chain.pop(); visited.add(name); - ordered.push(nodes.get(name)); + if (!plugin.disabled) ordered.push(plugin); + return !plugin.disabled; }; for (const plugin of discovered) { - if (plugin.disabled) continue; visit(plugin.name); } diff --git a/test/pluginManager.test.js b/test/pluginManager.test.js index 3bf6ce3..c4481e6 100644 --- a/test/pluginManager.test.js +++ b/test/pluginManager.test.js @@ -1,14 +1,36 @@ const { test } = require("node:test"); const assert = require("node:assert"); +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); const { PluginManager } = require("../core/PluginManager"); -function makePM() { +function makePM(config = {}) { return new PluginManager({ client: { commands: new Map(), on() {}, once() {}, off() {} }, - db: {}, scheduler: {}, hooks: { emitHook: async () => {} }, + db: { getAllEnabledPluginRows: async () => [] }, + scheduler: {}, hooks: { emitHook: async () => {} }, + config, }); } +const discovered = (name, manifest = {}) => ({ name, manifest: { name, ...manifest } }); +const names = (plugins) => plugins.map((plugin) => plugin.name); + +// Real npm-style plugin packages, so loadAll runs actual discovery and loading. +function installPlugins(t, manifests) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "adb-plugin-deps-")); + t.after(() => fs.rmSync(root, { recursive: true, force: true })); + const nodeModulesDir = path.join(root, "node_modules"); + for (const [name, manifest] of Object.entries(manifests)) { + const dir = path.join(nodeModulesDir, name); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, "plugin.json"), JSON.stringify({ name, version: "1.0.0", ...manifest })); + fs.writeFileSync(path.join(dir, "index.js"), "exports.load = async () => {};\n"); + } + return { pluginsDir: path.join(root, "plugins"), nodeModulesDir }; +} + test("getDependents finds plugins depending on target", () => { const pm = makePM(); const a = pm.initPluginState("adb-plugin-a", { dependsOn: [] }); @@ -37,3 +59,63 @@ test("getPluginList exposes core/version/discordPermissions", () => { assert.strictEqual(entry.version, "1.0.0"); assert.deepStrictEqual(entry.discordPermissions, ["BanMembers"]); }); + +test("sortByDependencies loads dependencies before their dependents", () => { + const pm = makePM(); + const ordered = pm.sortByDependencies([ + discovered("adb-plugin-b", { dependsOn: ["adb-plugin-a"] }), + discovered("adb-plugin-a"), + ]); + assert.deepStrictEqual(names(ordered), ["adb-plugin-a", "adb-plugin-b"]); +}); + +test("sortByDependencies skips plugins in a dependency cycle instead of throwing", () => { + const pm = makePM(); + const plugins = [ + discovered("adb-plugin-a", { engines: { plugins: { "adb-plugin-b": "*" } } }), + discovered("adb-plugin-b", { engines: { plugins: { "adb-plugin-a": "*" } } }), + discovered("adb-plugin-self", { dependsOn: ["adb-plugin-self"] }), + discovered("adb-plugin-needs-a", { dependsOn: ["adb-plugin-a"] }), + discovered("adb-plugin-healthy"), + ]; + assert.deepStrictEqual(names(pm.sortByDependencies(plugins)), ["adb-plugin-healthy"]); + assert.deepStrictEqual(names(plugins.filter((plugin) => plugin.disabled)), [ + "adb-plugin-a", "adb-plugin-b", "adb-plugin-self", "adb-plugin-needs-a", + ]); +}); + +test("sortByDependencies never loads a plugin whose dependency is missing, even when another plugin needs it", () => { + for (const order of [["top", "mid"], ["mid", "top"]]) { + const plugins = { + top: discovered("adb-plugin-top", { dependsOn: ["adb-plugin-mid"] }), + mid: discovered("adb-plugin-mid", { dependsOn: ["adb-plugin-not-installed"] }), + }; + const ordered = makePM().sortByDependencies(order.map((key) => plugins[key])); + assert.deepStrictEqual(names(ordered), [], `discovery order: ${order.join(", ")}`); + } +}); + +test("loadAll still loads healthy plugins when an installed plugin has a dependency cycle", async (t) => { + const pm = makePM(installPlugins(t, { + "adb-plugin-a": { engines: { plugins: { "adb-plugin-b": "*" } } }, + "adb-plugin-b": { engines: { plugins: { "adb-plugin-a": "*" } } }, + "adb-plugin-healthy": {}, + })); + pm.loadCore = async () => {}; + pm.setupHotReload = () => {}; + await pm.loadAll(); + assert.deepStrictEqual([...pm.plugins.keys()], ["adb-plugin-healthy"]); + assert.strictEqual(pm.plugins.get("adb-plugin-healthy").loaded, true); +}); + +test("command collection still refuses to deploy when a plugin has a dependency cycle", async (t) => { + const pm = makePM({ + ...installPlugins(t, { + "adb-plugin-self": { dependsOn: ["adb-plugin-self"] }, + "adb-plugin-healthy": {}, + }), + commandCollection: true, + }); + pm.loadCore = async () => {}; + await assert.rejects(pm.loadAll(), /missing or circular plugin dependencies/); +});