From 294fd7b131dd17c805bf5adb01df8a2120edab66 Mon Sep 17 00:00:00 2001 From: Team Gaya Date: Sun, 18 Oct 2026 21:57:29 +0530 Subject: [PATCH] fix: a plugin dependency cycle no longer stops every plugin from loading sortByDependencies threw on a dependency cycle and loadAll does not catch it, so one installed plugin whose manifest names itself (or a peer that names it back) in dependsOn, dependencies or engines.plugins aborted the whole load. At boot that rejects startADB(), and the process exits before the dashboard starts listening. Dashboard installs are recorded in data/installed-plugins.json before loadAll runs, so reconcile reinstalls the plugin on every restart and the container crash-loops until someone removes it by hand. The same walk also ignored the disabled flag on a plugin skipped for a missing dependency: if another plugin depended on it, visit() still put it in the load order, so it loaded anyway, and so did its dependents. visit() now reports whether each plugin can load. Cycle members are skipped with the cycle path in the log, and a plugin whose dependency was skipped is skipped too, whatever the discovery order. Healthy plugins load as before, and command collection still refuses to deploy an incomplete set. Co-Authored-By: Claude Opus 5.5 --- core/PluginManager.js | 30 ++++++++----- test/pluginManager.test.js | 86 +++++++++++++++++++++++++++++++++++++- 2 files changed, 104 insertions(+), 12 deletions(-) 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/); +});