fix(plugins): discovery must not instantiate an imported base class

[warksit]

The bug

load_plugin()'s preferred detection strategy takes the first class whose name ends in Plugin:

for name, obj in inspect.getmembers(plugin_module, inspect.isclass):
    if name.endswith("Plugin"):
        plugin_instance = obj(self.base)
        break

inspect.getmembers() returns members alphabetically. Every plugin does from plugin_system import PredBatPlugin, so the abstract base class is in the module namespace and is itself a match. Any plugin whose class name sorts after PredBatPlugin therefore gets the base instantiated instead of itself.

When that happens the plugin looks completely healthy in the log:

Initialising plugin class (name-based): PredBatPlugin
Successfully loaded plugin: soc_keep_publish_plugin

…but PredBatPlugin.register_hooks() is a no-op, so no hooks are registered and the plugin silently does nothing. There is no error, no warning, and the entity it was supposed to publish simply goes stale.

Whether a plugin works depends on the luck of its class name. I hit this with a plugin class called SocKeepPublishPlugin; ColdWeatherPlugin and CurtailmentPlugin happen to sort before PredBatPlugin, which is the only reason they were unaffected.

The fix

Require the candidate class to be defined in the plugin file, not merely imported into it:

if name.endswith("Plugin") and obj.__module__ == plugin_module.__name__:

This also covers the general case of a plugin importing any other helper class ending in Plugin.

The two fallback strategies (the PREDBAT_PLUGIN marker and the initialize_plugin() function) are unchanged.

Compatibility

No API change and no behaviour change for any plugin that was already working — a plugin class defined in its own file still matches exactly as before. The only plugins affected are ones that were silently broken.

Testing

Added test_plugin_discovery_skips_imported_base_class to the existing tests/test_plugin_startup.py (no new files). It writes a temp plugin whose class name deliberately sorts after PredBatPlugin, runs real discovery over it, and asserts the correct class was instantiated.

Confirmed it fails without the fix:

ERROR: discovery instantiated PredBatPlugin, expected ZzSortsLastPlugin
**** ERROR: Test plugin_discovery FAILED

and passes with it. Registered in unit_test.py as plugin_discovery. Full unit_test.py --quick passes, pre-commit run clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WY52NvNyVdznsF4VcVG8Q8

[Copilot]

Pull request overview

Fixes Predbat’s plugin discovery so it won’t instantiate an imported base class (e.g., PredBatPlugin) just because it alphabetically sorts first in inspect.getmembers(), preventing silently “loaded” but non-functional plugins.

Changes:

• Tighten name-based plugin discovery to only consider classes defined in the plugin module (obj.__module__ == plugin_module.__name__).
• Add a regression test that creates a temp plugin whose class name sorts after PredBatPlugin and asserts the correct class is instantiated.
• Register the new test in unit_test.py so it runs with the suite.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
apps/predbat/plugin_system.py	Restricts name-based class discovery to classes defined in the plugin file to avoid instantiating imported base classes.
apps/predbat/tests/test_plugin_startup.py	Adds a regression test for the discovery bug using a temporary plugin module.
apps/predbat/unit_test.py	Registers the new plugin discovery regression test in the main test list.

[warksit]

@springfall2008 — gentle nudge on this one, it's been quiet since 26 August and I think it's ready.

Status: CI green (pre-commit pass), no conflicts, and apps/predbat/plugin_system.py has had no upstream commits since this branch was cut, so it still applies cleanly.

Copilot's review point is addressed in 3e1a1a7: strategy 2 (the PREDBAT_PLUGIN attribute fallback) was still exposed, because PredBatPlugin sets that marker as its own discovery flag and so matches inside every plugin that imports it. The fallback now carries the same obj.__module__ == plugin_module.__name__ guard as strategy 1, with test_plugin_discovery_fallback_skips_imported_base_class covering the path where the plugin class name doesn't end in Plugin and discovery falls through to it.

One correction to the description above, which predates that commit: it says the two fallback strategies are unchanged. Strategy 2 is now guarded as well; strategy 3 (initialize_plugin()) is genuinely untouched.

Happy to rebase or split it if that would be easier to take.

[warksit]

@mgazza — you wrote the plugin loader in #2582, so you're the best person to sanity-check this. Could you take a look? @springfall2008 for merge if he's happy.

The bug, in one line: any plugin whose class name sorts alphabetically after PredBatPlugin logs "Successfully loaded plugin" and then does nothing.

Why: load_plugin() takes the first class ending in Plugin from inspect.getmembers(), which is alphabetical. Every plugin imports PredBatPlugin, so the base class is in the module namespace and can win. Its register_hooks() is a no-op, so no hooks are registered and there is no error.

Repro: a plugin file containing class SocKeepPublishPlugin(PredBatPlugin). The log shows Initialising plugin class (name-based): PredBatPlugin.

The fix: only consider classes defined in the plugin's own module (obj.__module__ == plugin_module.__name__), in both the name-based strategy and the PREDBAT_PLUGIN attribute fallback. Plugins that load correctly today are unaffected.

Size: 116 lines added in total. The change to plugin_system.py is 24 lines; the rest is two regression tests in test_plugin_startup.py and their registration.

Rebased on main today (the only conflict was the import line in unit_test.py). CI is green and the PR is mergeable.