[theLeverager1705]

Description

Fixes #73.

sortByDependencies() threw on a dependency cycle and nothing above it catches the error. So one plugin whose manifest names itself, or a peer that names it back, stopped every plugin from loading. At boot the bot exits before the dashboard comes up. After a dashboard install it crash-loops, because the install is persisted before loadAll() runs. The issue has the full trace with line references.

The same walk also ignored disabled: a plugin skipped for a missing dependency still loaded whenever another plugin depended on it.

What changed

All changes are in sortByDependencies() in core/PluginManager.js (+20 / −10):

• visit() now returns whether a plugin can load. When it hits a cycle it logs the path, marks every member disabled, and moves on:
  Skipping plugins in circular dependency: adb-plugin-a -> adb-plugin-b -> adb-plugin-a
• A plugin whose dependency can't load (missing, in a cycle, or transitively either) is skipped too, with a log line naming the dependency. Discovery order doesn't matter.
• Healthy plugins load in the same order as before.
• The command-collection guard in loadAll() already refuses to deploy when any discovered plugin is disabled, so deploy-commands.js still won't push an incomplete set. I only reworded its message to "missing or circular plugin dependencies", since it now covers both.

No changes to the manifest format, the plugin API, or the dashboard.

How I tested it

Five new tests in test/pluginManager.test.js:

Test	Without this fix	With this fix
dependencies load before dependents (baseline)	pass	pass
mutual cycle + self-dependency + a plugin depending on the cycle: only the healthy plugin is returned	fail (throws)	pass
missing dependency with a dependent, both discovery orders: nothing loads	fail (both load)	pass
loadAll() against real adb-plugin-* packages in a temp node_modules, one cycle: resolves, healthy plugin loads	fail (rejects)	pass
command collection with a cycle: still rejects	fail (rejects with the raw cycle error)	pass

Full suite on Linux (GitHub Actions on my fork, with the workflow from #76): 488/488 on Node 20, 22 and 24, plus the dashboard check (15/15) and production build.
Run: https://github.com/theLeverager1705/Advanced-Discord-Bot/actions/runs/39145211627

Locally on Windows with Node 24, 485/488 pass. The 3 failures fail on main on this machine too: the symlink discovery test needs Windows developer mode, and two deploy-CLI tests hit their 10 s spawn timeout on a slow disk. All of them pass on Linux.

I didn't test against a live guild because the change doesn't touch any Discord-facing code path. It only changes which plugins loadAll() hands to loadPlugin().

Type of change

• 🐛 Bug fix

Checklist

• Follows the style of the surrounding code (tabs in core/, 2-space in this test file)
• Self-reviewed
• Commented the non-obvious part (what visit() returns and why)
• Docs: no update needed. The dependency fields behave the same, only the failure mode changed.
• No new warnings
• Added tests that fail without the fix
• Existing tests pass
• Linked the issue

Reviewer notes

• One behaviour change to look at: before this, a plugin skipped for a missing dependsOn / dependencies entry could still load if something depended on it. Now it never loads, and neither do its dependents. For engines.plugins edges checkEngines() already refused them at load time, so nothing changes there.
• Skipped plugins still don't appear in the dashboard's plugin list. That's already true for missing-dependency plugins today. Surfacing them with a lastError would be a useful follow-up, but it changes how loadAll() interacts with the install and update flows, so I kept it out of this PR.

🤖 Generated with Claude Code