Skip to content

fix(plugins): evict failed sibling exec from sys.modules so later imports raise ModuleNotFoundError - #112444

Closed
Mzzj114 wants to merge 1 commit into
NousResearch:mainfrom
Mzzj114:fix/plugin-loader-failed-sibling-leak
Closed

Mzzj114 wants to merge 1 commit into
NousResearch:mainfrom
Mzzj114:fix/plugin-loader-failed-sibling-leak

Conversation

@Mzzj114

@Mzzj114 Mzzj114 commented Sep 16, 2026

Copy link
Copy Markdown

What

In plugins/plugin_loader.py::load_plugin_module, add the same sys.modules cleanup on failure for the sibling loop that already exists for the main module path (sys.modules.pop(full_sub_name, None) when _exec(sub_mod, ...) returns False).

Why

_new_module() inserts the module into sys.modules before execution; _exec() intentionally leaves the entry on failure ("callers needing a clean retry pop it themselves"), but the sibling loop is a caller that never retries — so a half-initialized module stayed cached. A later relative import of one of its names then raised ImportError: cannot import name ... (error.name == None, uncatchable as ModuleNotFoundError) exactly where the plugin's own handler expects ModuleNotFoundError. Repro and code evidence in #112096.

Fixes #112096

How to test

scripts/run_tests.sh tests/plugins/test_plugin_loader_sibling_cleanup.py -q — 4 tests:

  • red on main, green here: failed sibling evicted from sys.modules
  • red on main, green here: later importlib.import_module raises ModuleNotFoundError, not ImportError
  • healthy sibling still loaded and bound (no behavior change)
  • main-module failure path unchanged (pop + return None)

Platforms tested

Linux (Ubuntu 22.04, Python 3.11). Change is platform-neutral module bookkeeping.

Notes

  • 5 unrelated tests/plugins/ files fail locally both with and without this change (pre-existing on this machine; likely side-dependent tests/optional deps) — not touched here.
  • The issue's edge case (a sibling importing a not-yet-exec'd sibling whose entry then gets evicted) is strictly better than the status quo: the entry is re-created on re-exec, versus a permanently poisoned cache today.

…orts raise ModuleNotFoundError

load_plugin_module's sibling loop kept a half-initialized module in
sys.modules when exec_module raised, so a plugin's later relative import
raised uncatchable 'ImportError: cannot import name ...' instead of the
ModuleNotFoundError its handlers expect. Mirror the main-module cleanup
(sys.modules.pop on failure) in the sibling loop.

Tests: tests/plugins/test_plugin_loader_sibling_cleanup.py — red on base
for the two regression assertions, green with the fix.

Regression for NousResearch#112096
@Mzzj114 Mzzj114 closed this Sep 16, 2026
@Mzzj114

Mzzj114 commented Sep 16, 2026

Copy link
Copy Markdown
Author

Duplicate of #112097, which was opened earlier for the same fix in plugins/plugin_loader.py — I should have searched open PRs per CONTRIBUTING.md before pushing; withdrawing this so the earlier PR can land.

@Mzzj114 Mzzj114 mentioned this pull request Sep 16, 2026
16 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant