From 4677e035471bbeb521f82a0b76a2de3b560b404e Mon Sep 17 00:00:00 2001 From: Evan Ryan Gunter Date: Sat, 11 Jul 2026 17:12:33 -0700 Subject: [PATCH 01/11] Isolate plugin-import failures from plugin remove/disable A plugin that fails to import (e.g. a provider whose dependency shipped a breaking release) previously crashed every mngr command at import time, including the plugin remove/disable commands used to recover -- locking the user out of the tool needed to uninstall the broken plugin. mngr plugin remove and mngr plugin disable now run without importing third-party plugin entry points, so a broken plugin cannot crash the very command used to remove or disable it. These two commands always skip the entry-point load (detected from argv before Click parses it) and parse config leniently, so a [providers.] block belonging to the not-loaded plugin does not turn recovery into a new error. Every other command is unchanged: it still loads all plugins and fails loudly if one is broken, rather than running in a degraded state. The recovery-detection token sets are hardcoded (the decision must be made from sys.argv at import time) and kept in sync with the real plugin command tree by a test. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../changelog/ev-plugin-import-isolation.md | 5 + libs/mngr/imbue/mngr/cli/plugin.py | 28 +++++- libs/mngr/imbue/mngr/cli/test_plugin.py | 55 +++++++++++ libs/mngr/imbue/mngr/main.py | 59 ++++++++++- libs/mngr/imbue/mngr/main_test.py | 97 +++++++++++++++++++ 5 files changed, 238 insertions(+), 6 deletions(-) create mode 100644 libs/mngr/changelog/ev-plugin-import-isolation.md diff --git a/libs/mngr/changelog/ev-plugin-import-isolation.md b/libs/mngr/changelog/ev-plugin-import-isolation.md new file mode 100644 index 0000000000..0151985522 --- /dev/null +++ b/libs/mngr/changelog/ev-plugin-import-isolation.md @@ -0,0 +1,5 @@ +A plugin that fails to import (e.g. a provider whose dependency published a breaking release) no longer bricks the whole `mngr` CLI for the two commands you need to recover. + +`mngr plugin remove` and `mngr plugin disable` now run without importing third-party plugin entry points, so a broken plugin can no longer crash the very command used to remove or disable it. To keep their behavior predictable, these two commands always skip the entry-point load (not only when something is broken), and they parse config leniently so a `[providers.]` block belonging to the not-loaded plugin does not turn recovery into a new error. + +Every other command is unchanged: it still loads all plugins and fails loudly if one is broken, rather than running in a degraded state. diff --git a/libs/mngr/imbue/mngr/cli/plugin.py b/libs/mngr/imbue/mngr/cli/plugin.py index 1c21e92f8b..15dedbc6a3 100644 --- a/libs/mngr/imbue/mngr/cli/plugin.py +++ b/libs/mngr/imbue/mngr/cli/plugin.py @@ -645,10 +645,17 @@ def _plugin_add_impl(ctx: click.Context) -> None: def _plugin_remove_impl(ctx: click.Context) -> None: """Implementation of plugin remove command.""" + # ``plugin remove`` is a recovery command: it runs without loading third-party plugin + # entry points (see create_plugin_manager) so a plugin that fails to import cannot brick + # it. That means the plugin being removed is not registered, so its ``[providers.]`` + # block would look like an unknown backend. Parse leniently (as ``plugin add`` does) so a + # config that references the plugin being removed does not turn recovery into a new error. mngr_ctx, output_opts, opts = setup_command_context( ctx=ctx, command_name="plugin", command_class=PluginCliOptions, + strict=False, + silent_unknown_fields=True, ) # Validate arguments before checking uv tool receipt so users get clear @@ -744,22 +751,35 @@ def _plugin_enable_impl(ctx: click.Context, **kwargs: Any) -> None: def _plugin_disable_impl(ctx: click.Context, **kwargs: Any) -> None: """Implementation of plugin disable command.""" - _plugin_set_enabled_impl(ctx, is_enabled=False) + # ``disable`` is a recovery command: create_plugin_manager skips loading third-party + # entry points for it so a plugin that fails to import cannot brick the command used to + # disable it. ``enable`` is not a recovery command and loads plugins normally. + _plugin_set_enabled_impl(ctx, is_enabled=False, is_recovery=True) -def _plugin_set_enabled_impl(ctx: click.Context, *, is_enabled: bool) -> None: - """Shared implementation for plugin enable/disable commands.""" +def _plugin_set_enabled_impl(ctx: click.Context, *, is_enabled: bool, is_recovery: bool = False) -> None: + """Shared implementation for plugin enable/disable commands. + + When ``is_recovery`` is True the command runs without third-party plugin entry points + loaded (see _plugin_disable_impl). Config is then parsed leniently -- the plugin being + disabled is not registered, so its ``[providers.]`` block would otherwise error as an + unknown backend -- and the soft name check is skipped, since with nothing loaded it could + only ever emit a misleading "not registered" warning. + """ mngr_ctx, output_opts, opts = setup_command_context( ctx=ctx, command_name="plugin", command_class=PluginCliOptions, + strict=False if is_recovery else None, + silent_unknown_fields=is_recovery, ) name = opts.name if name is None: raise AbortError("Plugin name is required") - _validate_plugin_name_is_known(name, mngr_ctx) + if not is_recovery: + _validate_plugin_name_is_known(name, mngr_ctx) root_name = os.environ.get("MNGR_ROOT_NAME", "mngr") scope = ConfigScope((opts.scope or "project").upper()) diff --git a/libs/mngr/imbue/mngr/cli/test_plugin.py b/libs/mngr/imbue/mngr/cli/test_plugin.py index 07c013ddd1..c4a7e62267 100644 --- a/libs/mngr/imbue/mngr/cli/test_plugin.py +++ b/libs/mngr/imbue/mngr/cli/test_plugin.py @@ -190,6 +190,61 @@ def test_plugin_disable_writes_enabled_false_to_project_toml( assert "enabled = false" in content +def test_plugin_disable_unknown_plugin_does_not_warn( + cli_runner: CliRunner, + plugin_manager: pluggy.PluginManager, + temp_git_repo_cwd: Path, + mngr_test_root_name: str, +) -> None: + """disable is a recovery command, so it skips the soft "not registered" name check. + + Unlike enable (see test_plugin_enable_unknown_plugin_warns_but_succeeds), disable runs + without third-party plugins loaded, so that check could only ever misfire -- it is + skipped, and disable still writes the config. + """ + result = cli_runner.invoke( + plugin, + ["disable", "definitely-not-a-real-plugin"], + obj=plugin_manager, + catch_exceptions=False, + ) + + assert result.exit_code == 0 + assert "not currently registered" not in result.output + config_path = temp_git_repo_cwd / f".{mngr_test_root_name}" / "settings.toml" + assert "enabled = false" in config_path.read_text() + + +def test_plugin_disable_succeeds_with_unknown_backend_in_config( + cli_runner: CliRunner, + plugin_manager: pluggy.PluginManager, + temp_git_repo_cwd: Path, + mngr_test_root_name: str, +) -> None: + """disable parses config leniently so a provider block for the not-loaded plugin is OK. + + A recovery run imports no third-party plugins, so a ``[providers.]`` block whose + backend is provided by an unloaded plugin looks like an unknown backend. Under the + default strict parse that raises ConfigParseError; disable must instead tolerate it so + that a config referencing the plugin being disabled does not block the disable. + """ + config_dir = temp_git_repo_cwd / f".{mngr_test_root_name}" + config_dir.mkdir(parents=True, exist_ok=True) + (config_dir / "settings.toml").write_text( + 'is_allowed_in_pytest = true\n\n[providers.fakeprovider]\nbackend = "fakebackend"\n' + ) + + result = cli_runner.invoke( + plugin, + ["disable", "fakebackend"], + obj=plugin_manager, + catch_exceptions=False, + ) + + assert result.exit_code == 0 + assert "enabled = false" in (config_dir / "settings.toml").read_text() + + def test_plugin_enable_json_format_returns_valid_json( cli_runner: CliRunner, plugin_manager: pluggy.PluginManager, diff --git a/libs/mngr/imbue/mngr/main.py b/libs/mngr/imbue/mngr/main.py index 19f4a729b9..f3dab53493 100644 --- a/libs/mngr/imbue/mngr/main.py +++ b/libs/mngr/imbue/mngr/main.py @@ -1,7 +1,9 @@ import bdb import os import sys +from collections.abc import Sequence from typing import Any +from typing import Final import click import pluggy @@ -67,6 +69,41 @@ _plugin_manager_container: dict[str, pluggy.PluginManager | None] = {"pm": None} +# Names (canonical + aliases) under which the `mngr plugin` group is reachable, and the +# subcommands of that group that must keep working even when a third-party plugin fails to +# import. `plugin remove` / `plugin disable` are the escape hatches a user runs to recover +# from a broken plugin, so for those invocations we deliberately skip loading setuptools +# entry points (see create_plugin_manager): a broken plugin must not be able to brick the +# very command that removes or disables it. Every OTHER command still loads all plugins and +# fails loudly if one is broken -- mngr does not run in a degraded state. +# +# These are hardcoded because the decision must be made from sys.argv at import time, before +# Click parses arguments. test_recovery_invocation_constants_match_click_tree keeps them in +# sync with the real `plugin` command tree so the sets cannot silently drift. +_PLUGIN_GROUP_INVOCATION_NAMES: Final[frozenset[str]] = frozenset({"plugin", "plug"}) +_PLUGIN_RECOVERY_SUBCOMMANDS: Final[frozenset[str]] = frozenset({"remove", "disable"}) + + +def _is_plugin_recovery_invocation(argv: Sequence[str]) -> bool: + """Return True iff ``argv`` invokes ``mngr plugin remove`` or ``mngr plugin disable``. + + ``argv`` is sys.argv-style (``argv[0]`` is the program name). The top-level mngr group + takes no value-consuming options before the subcommand -- only the eager ``--version`` / + ``--help`` flags, which exit before any command runs -- so ``argv[1]`` is reliably the + subcommand token and ``argv[2]`` its subcommand. That makes a simple token match both + sufficient and safe from false positives: no other top-level command is named ``plugin`` + (or ``plug``), and the non-recovery ``plugin`` subcommands (``list``/``add``/``enable``) + do not match, so they still trigger a full plugin load. + + A false negative (e.g. an unusual abbreviation that dodges the match) merely falls back to + the normal load path -- i.e. the pre-existing crash on a broken plugin -- so erring toward + not matching is safe. + """ + return ( + len(argv) >= 3 and argv[1] in _PLUGIN_GROUP_INVOCATION_NAMES and argv[2] in _PLUGIN_RECOVERY_SUBCOMMANDS + ) + + def _call_on_error_hook(ctx: click.Context, error: BaseException) -> None: """Call the on_error hook if command metadata was stored by setup_command_context. @@ -267,7 +304,7 @@ def load_plugin_hookspecs(pm: pluggy.PluginManager) -> None: pm.add_hookspecs(hookspec_module) -def create_plugin_manager() -> pluggy.PluginManager: +def create_plugin_manager(load_entry_points: bool | None = None) -> pluggy.PluginManager: """ Initializes the plugin manager and loads all plugin registries. @@ -279,8 +316,18 @@ def create_plugin_manager() -> pluggy.PluginManager: config-based blocking so that tooling (e.g. doc generation) can load every provider regardless of local configuration. + ``load_entry_points`` controls whether third-party plugin entry points are + imported. When left as None it is derived from the current invocation: it is + False for the ``mngr plugin remove`` / ``mngr plugin disable`` recovery + commands (see _is_plugin_recovery_invocation) so a plugin that fails to import + cannot brick the very command used to remove or disable it, and True for every + other command. Pass it explicitly to override the derivation (used by tests). + This should only really be called once from the main command (or during testing). """ + if load_entry_points is None: + load_entry_points = not _is_plugin_recovery_invocation(sys.argv) + # Create plugin manager and load registries first (needed for config parsing) pm = pluggy.PluginManager("mngr") pm.add_hookspecs(hookspecs) @@ -289,12 +336,20 @@ def create_plugin_manager() -> pluggy.PluginManager: # load_setuptools_entrypoints so disabled plugins are never registered. # MNGR_LOAD_ALL_PLUGINS overrides this so that tooling (e.g. doc generation) # can produce output that reflects all providers regardless of local config. + # This runs even for the recovery commands (which skip the load below): it does + # not import anything, and it marks the disabled names as blocked so the strict + # re-block in load_config does not trip on a name that is neither registered + # (nothing was loaded) nor blocked. if not parse_bool_env(os.environ.get("MNGR_LOAD_ALL_PLUGINS", "")): block_disabled_plugins(pm, read_disabled_plugins()) # Automatically discover and load plugins registered via setuptools entry points. # External packages can register hooks by adding an entry point for the "mngr" group. - pm.load_setuptools_entrypoints("mngr") + # Skipped for the recovery commands (load_entry_points=False) so a broken plugin cannot + # crash them; in that case the manager holds only mngr's built-ins, which is all those + # commands need. + if load_entry_points: + pm.load_setuptools_entrypoints("mngr") # Allow plugins to register their own hookspec modules (for plugin-specific hooks). load_plugin_hookspecs(pm) diff --git a/libs/mngr/imbue/mngr/main_test.py b/libs/mngr/imbue/mngr/main_test.py index 2d0794f55e..9856dc09b3 100644 --- a/libs/mngr/imbue/mngr/main_test.py +++ b/libs/mngr/imbue/mngr/main_test.py @@ -1,10 +1,15 @@ """Unit tests for create_plugin_manager.""" +import importlib.metadata import os from pathlib import Path import pytest +from imbue.mngr.main import _PLUGIN_GROUP_INVOCATION_NAMES +from imbue.mngr.main import _PLUGIN_RECOVERY_SUBCOMMANDS +from imbue.mngr.main import _is_plugin_recovery_invocation +from imbue.mngr.main import cli from imbue.mngr.main import create_plugin_manager from imbue.mngr.utils.env_utils import parse_bool_env @@ -49,3 +54,95 @@ def test_create_plugin_manager_skips_blocking_when_load_all_plugins_set( pm = create_plugin_manager() assert not pm.is_blocked("modal") + + +@pytest.mark.parametrize( + "argv, expected", + [ + (["mngr", "plugin", "remove", "imbue-mngr-fake"], True), + (["mngr", "plugin", "disable", "fake"], True), + (["mngr", "plug", "remove", "imbue-mngr-fake"], True), + (["mngr", "plug", "disable", "fake"], True), + (["mngr", "plugin", "disable"], True), + # Non-recovery plugin subcommands must still load plugins (fail loudly if broken). + (["mngr", "plugin", "list"], False), + (["mngr", "plugin", "add", "imbue-mngr-fake"], False), + (["mngr", "plugin", "enable", "fake"], False), + # Unrelated commands, bare group, and too-short argv. + (["mngr", "create", "remove"], False), + (["mngr", "remove"], False), + (["mngr", "plugin"], False), + (["mngr"], False), + ([], False), + ], +) +def test_is_plugin_recovery_invocation(argv: list[str], expected: bool) -> None: + """Only `plugin remove` / `plugin disable` (and the `plug` alias) are recovery invocations.""" + assert _is_plugin_recovery_invocation(argv) is expected + + +def test_recovery_invocation_constants_match_click_tree() -> None: + """The hardcoded recovery-detection sets must stay in sync with the real command tree. + + The detection in _is_plugin_recovery_invocation is hardcoded because it runs from + sys.argv before Click parses arguments. This test fails if the `plugin` group's names + (canonical + aliases) or its subcommands drift away from those hardcoded sets, so the + fast-path cannot silently diverge from what Click actually dispatches. + """ + plugin_group = cli.commands["plugin"] + + reachable_names = frozenset(name for name, command in cli.commands.items() if command is plugin_group) + assert reachable_names == _PLUGIN_GROUP_INVOCATION_NAMES, ( + f"The `plugin` group is reachable as {sorted(reachable_names)}, but recovery detection " + f"matches {sorted(_PLUGIN_GROUP_INVOCATION_NAMES)}. Update _PLUGIN_GROUP_INVOCATION_NAMES." + ) + + subcommand_names = frozenset(plugin_group.commands.keys()) + assert _PLUGIN_RECOVERY_SUBCOMMANDS <= subcommand_names, ( + f"Recovery subcommands {sorted(_PLUGIN_RECOVERY_SUBCOMMANDS)} are not all real `mngr plugin` " + f"subcommands {sorted(subcommand_names)}. Update _PLUGIN_RECOVERY_SUBCOMMANDS." + ) + + +def test_create_plugin_manager_skips_entry_points_in_recovery( + project_config_dir: Path, + temp_git_repo_cwd: Path, +) -> None: + """With load_entry_points=False, no third-party setuptools entry point is imported. + + All provider/agent plugins come from setuptools entry points; skipping them is what lets + `plugin remove` / `plugin disable` run even when one of those entry points fails to + import. mngr's own built-ins (registered directly, not via entry points) still load. + """ + (project_config_dir / "settings.toml").write_text("is_allowed_in_pytest = true\n") + + pm = create_plugin_manager(load_entry_points=False) + + entry_point_names = {ep.name for ep in importlib.metadata.entry_points(group="mngr")} + # list_name_plugin() includes blocked names with a None plugin object, so filter to + # names that were actually registered. + registered_names = {name for name, plugin in pm.list_name_plugin() if plugin is not None} + assert registered_names.isdisjoint(entry_point_names), ( + f"recovery mode registered entry-point plugins: {sorted(registered_names & entry_point_names)}" + ) + # A built-in is still registered, so recovery is not a completely empty manager. + assert pm.get_plugin("builtin_help_topics") is not None + + +def test_create_plugin_manager_blocks_disabled_plugins_even_in_recovery( + project_config_dir: Path, + temp_git_repo_cwd: Path, +) -> None: + """Blocking of config-disabled plugins still runs when entry points are skipped. + + Blocking imports nothing, and it marks disabled names as blocked so the strict re-block + in load_config does not trip on a name that is neither registered (nothing was loaded in + recovery mode) nor blocked. + """ + (project_config_dir / "settings.toml").write_text( + "is_allowed_in_pytest = true\n\n[plugins.modal]\nenabled = false\n" + ) + + pm = create_plugin_manager(load_entry_points=False) + + assert pm.is_blocked("modal") From b7393a9f89331bf4b96133bfc3c31f54f23ff9ed Mon Sep 17 00:00:00 2001 From: Evan Ryan Gunter Date: Sat, 11 Jul 2026 17:41:41 -0700 Subject: [PATCH 02/11] Make plugin-manager recovery flags required, not defaulted Address review: the style guide discourages default arguments. Make _plugin_set_enabled_impl's is_recovery required (enable passes False, disable passes True), and make create_plugin_manager's load_entry_points required. The sys.argv-based derivation now lives in get_or_create_plugin_manager (the CLI singleton path) instead of a None sentinel default, so create_plugin_manager no longer reads global state. Direct callers (the lima release helper, tests) pass an explicit bool. Co-Authored-By: Claude Opus 4.8 (1M context) --- libs/mngr/imbue/mngr/cli/plugin.py | 6 +++-- libs/mngr/imbue/mngr/main.py | 23 +++++++++++-------- libs/mngr/imbue/mngr/main_test.py | 4 ++-- .../changelog/ev-plugin-import-isolation.md | 1 + .../mngr_lima/_lima_btrfs_release_helper.py | 2 +- 5 files changed, 21 insertions(+), 15 deletions(-) create mode 100644 libs/mngr_lima/changelog/ev-plugin-import-isolation.md diff --git a/libs/mngr/imbue/mngr/cli/plugin.py b/libs/mngr/imbue/mngr/cli/plugin.py index 15dedbc6a3..1e208dda38 100644 --- a/libs/mngr/imbue/mngr/cli/plugin.py +++ b/libs/mngr/imbue/mngr/cli/plugin.py @@ -746,7 +746,9 @@ def plugin_disable(ctx: click.Context, **kwargs: Any) -> None: def _plugin_enable_impl(ctx: click.Context, **kwargs: Any) -> None: """Implementation of plugin enable command.""" - _plugin_set_enabled_impl(ctx, is_enabled=True) + # ``enable`` is not a recovery command: it loads plugins normally (and fails loudly if + # one is broken). Only ``disable`` -- see _plugin_disable_impl -- runs in recovery mode. + _plugin_set_enabled_impl(ctx, is_enabled=True, is_recovery=False) def _plugin_disable_impl(ctx: click.Context, **kwargs: Any) -> None: @@ -757,7 +759,7 @@ def _plugin_disable_impl(ctx: click.Context, **kwargs: Any) -> None: _plugin_set_enabled_impl(ctx, is_enabled=False, is_recovery=True) -def _plugin_set_enabled_impl(ctx: click.Context, *, is_enabled: bool, is_recovery: bool = False) -> None: +def _plugin_set_enabled_impl(ctx: click.Context, *, is_enabled: bool, is_recovery: bool) -> None: """Shared implementation for plugin enable/disable commands. When ``is_recovery`` is True the command runs without third-party plugin entry points diff --git a/libs/mngr/imbue/mngr/main.py b/libs/mngr/imbue/mngr/main.py index f3dab53493..1673767173 100644 --- a/libs/mngr/imbue/mngr/main.py +++ b/libs/mngr/imbue/mngr/main.py @@ -304,7 +304,7 @@ def load_plugin_hookspecs(pm: pluggy.PluginManager) -> None: pm.add_hookspecs(hookspec_module) -def create_plugin_manager(load_entry_points: bool | None = None) -> pluggy.PluginManager: +def create_plugin_manager(load_entry_points: bool) -> pluggy.PluginManager: """ Initializes the plugin manager and loads all plugin registries. @@ -317,17 +317,13 @@ def create_plugin_manager(load_entry_points: bool | None = None) -> pluggy.Plugi every provider regardless of local configuration. ``load_entry_points`` controls whether third-party plugin entry points are - imported. When left as None it is derived from the current invocation: it is - False for the ``mngr plugin remove`` / ``mngr plugin disable`` recovery - commands (see _is_plugin_recovery_invocation) so a plugin that fails to import - cannot brick the very command used to remove or disable it, and True for every - other command. Pass it explicitly to override the derivation (used by tests). + imported. Pass False for the ``mngr plugin remove`` / ``mngr plugin disable`` + recovery commands so a plugin that fails to import cannot brick the very + command used to remove or disable it (the CLI derives this via + get_or_create_plugin_manager); pass True everywhere else. This should only really be called once from the main command (or during testing). """ - if load_entry_points is None: - load_entry_points = not _is_plugin_recovery_invocation(sys.argv) - # Create plugin manager and load registries first (needed for config parsing) pm = pluggy.PluginManager("mngr") pm.add_hookspecs(hookspecs) @@ -373,9 +369,16 @@ def get_or_create_plugin_manager() -> pluggy.PluginManager: This is used during CLI initialization to apply plugin-registered options to commands before argument parsing happens. The singleton ensures that plugins are only loaded once even if this is called multiple times. + + Third-party entry points are skipped for the ``mngr plugin remove`` / + ``mngr plugin disable`` recovery commands (detected from sys.argv, since the + singleton is first built at import time before Click parses arguments) so a + plugin that fails to import cannot brick the command used to remove or + disable it. See _is_plugin_recovery_invocation. """ if _plugin_manager_container["pm"] is None: - _plugin_manager_container["pm"] = create_plugin_manager() + load_entry_points = not _is_plugin_recovery_invocation(sys.argv) + _plugin_manager_container["pm"] = create_plugin_manager(load_entry_points=load_entry_points) return _plugin_manager_container["pm"] diff --git a/libs/mngr/imbue/mngr/main_test.py b/libs/mngr/imbue/mngr/main_test.py index 9856dc09b3..5a00dfe643 100644 --- a/libs/mngr/imbue/mngr/main_test.py +++ b/libs/mngr/imbue/mngr/main_test.py @@ -35,7 +35,7 @@ def test_create_plugin_manager_blocks_disabled_plugins( "is_allowed_in_pytest = true\n\n[plugins.modal]\nenabled = false\n" ) - pm = create_plugin_manager() + pm = create_plugin_manager(load_entry_points=True) assert pm.is_blocked("modal") @@ -51,7 +51,7 @@ def test_create_plugin_manager_skips_blocking_when_load_all_plugins_set( ) monkeypatch.setenv("MNGR_LOAD_ALL_PLUGINS", "1") - pm = create_plugin_manager() + pm = create_plugin_manager(load_entry_points=True) assert not pm.is_blocked("modal") diff --git a/libs/mngr_lima/changelog/ev-plugin-import-isolation.md b/libs/mngr_lima/changelog/ev-plugin-import-isolation.md new file mode 100644 index 0000000000..4449433ed2 --- /dev/null +++ b/libs/mngr_lima/changelog/ev-plugin-import-isolation.md @@ -0,0 +1 @@ +Internal: `create_plugin_manager` now requires an explicit `load_entry_points` argument, so the lima btrfs release helper passes `load_entry_points=True`. No user-visible change. diff --git a/libs/mngr_lima/imbue/mngr_lima/_lima_btrfs_release_helper.py b/libs/mngr_lima/imbue/mngr_lima/_lima_btrfs_release_helper.py index 85f4253034..f73edd7b9c 100644 --- a/libs/mngr_lima/imbue/mngr_lima/_lima_btrfs_release_helper.py +++ b/libs/mngr_lima/imbue/mngr_lima/_lima_btrfs_release_helper.py @@ -63,7 +63,7 @@ def _build_provider(profile_dir: Path) -> tuple[LimaProviderInstance, Concurrenc # ~10-15 min; the default 600s is for KVM-accelerated boots. vm_start_timeout_seconds=1500.0, ) - pm = create_plugin_manager() + pm = create_plugin_manager(load_entry_points=True) mngr_config = MngrConfig.model_construct( prefix="mngr-", default_host_dir=Path("/mngr"), From 7f752fabffe036f0bd218b71782fe89a36593b73 Mon Sep 17 00:00:00 2001 From: Evan Ryan Gunter Date: Sat, 11 Jul 2026 18:01:48 -0700 Subject: [PATCH 03/11] Pass load_entry_points=True in fd_leak repro scripts Problem: create_plugin_manager was changed on this branch to require an explicit load_entry_points argument (no default), but 9 repro scripts under scripts/qi/fd_leak/ still called create_plugin_manager() with no arguments. Those scripts are not in the tool.ty exclude list, so the repo-wide ty check (test_no_type_errors) reported error[missing-argument] for each and the branch failed the type-check CI gate. Fix: pass load_entry_points=True at each of the 9 callsites, preserving the pre-change behavior (all provider entry points loaded, which these FD-leak repros rely on). Add a dev changelog entry for the scripts/ change. Co-Authored-By: Claude Opus 4.8 (1M context) --- dev/changelog/ev-plugin-import-isolation.md | 1 + scripts/qi/fd_leak/isolate_01_baseline.py | 2 +- scripts/qi/fd_leak/isolate_02_sequential.py | 2 +- scripts/qi/fd_leak/isolate_03_parallel_noop.py | 2 +- scripts/qi/fd_leak/isolate_04_parallel_real_providers.py | 2 +- scripts/qi/fd_leak/isolate_05_parallel_one_real_one_noop.py | 2 +- scripts/qi/fd_leak/isolate_08_new_providers_each_time.py | 2 +- scripts/qi/fd_leak/repro_fd_leak_discover_only.py | 2 +- scripts/qi/fd_leak/repro_grpclib_fd_leak.py | 2 +- scripts/qi/fd_leak/repro_list_agents_fd_leak.py | 2 +- 10 files changed, 10 insertions(+), 9 deletions(-) create mode 100644 dev/changelog/ev-plugin-import-isolation.md diff --git a/dev/changelog/ev-plugin-import-isolation.md b/dev/changelog/ev-plugin-import-isolation.md new file mode 100644 index 0000000000..2cd96a3ae2 --- /dev/null +++ b/dev/changelog/ev-plugin-import-isolation.md @@ -0,0 +1 @@ +Update the `scripts/qi/fd_leak/` FD-leak repro scripts to pass `load_entry_points=True` to `create_plugin_manager`, which now requires that argument. Keeps their behavior unchanged (all providers loaded) and keeps the repo-wide type check green. diff --git a/scripts/qi/fd_leak/isolate_01_baseline.py b/scripts/qi/fd_leak/isolate_01_baseline.py index 923fcbc9ec..4e5d07f5cc 100644 --- a/scripts/qi/fd_leak/isolate_01_baseline.py +++ b/scripts/qi/fd_leak/isolate_01_baseline.py @@ -31,7 +31,7 @@ def count_real_fds() -> int: def main() -> None: cg = ConcurrencyGroup(name="repro") with cg: - pm = create_plugin_manager() + pm = create_plugin_manager(load_entry_points=True) mngr_ctx = load_config(pm, cg) gc.collect() diff --git a/scripts/qi/fd_leak/isolate_02_sequential.py b/scripts/qi/fd_leak/isolate_02_sequential.py index b86d2ca604..aaf0735b39 100644 --- a/scripts/qi/fd_leak/isolate_02_sequential.py +++ b/scripts/qi/fd_leak/isolate_02_sequential.py @@ -31,7 +31,7 @@ def count_real_fds() -> int: def main() -> None: cg = ConcurrencyGroup(name="repro") with cg: - pm = create_plugin_manager() + pm = create_plugin_manager(load_entry_points=True) mngr_ctx = load_config(pm, cg) providers = get_all_provider_instances(mngr_ctx, None) diff --git a/scripts/qi/fd_leak/isolate_03_parallel_noop.py b/scripts/qi/fd_leak/isolate_03_parallel_noop.py index 0724fab344..43fd329fa3 100644 --- a/scripts/qi/fd_leak/isolate_03_parallel_noop.py +++ b/scripts/qi/fd_leak/isolate_03_parallel_noop.py @@ -35,7 +35,7 @@ def noop() -> None: def main() -> None: cg = ConcurrencyGroup(name="repro") with cg: - pm = create_plugin_manager() + pm = create_plugin_manager(load_entry_points=True) mngr_ctx = load_config(pm, cg) gc.collect() diff --git a/scripts/qi/fd_leak/isolate_04_parallel_real_providers.py b/scripts/qi/fd_leak/isolate_04_parallel_real_providers.py index 5acd12f099..eafadae053 100644 --- a/scripts/qi/fd_leak/isolate_04_parallel_real_providers.py +++ b/scripts/qi/fd_leak/isolate_04_parallel_real_providers.py @@ -48,7 +48,7 @@ def discover_one_provider( def main() -> None: cg = ConcurrencyGroup(name="repro") with cg: - pm = create_plugin_manager() + pm = create_plugin_manager(load_entry_points=True) mngr_ctx = load_config(pm, cg) providers = get_all_provider_instances(mngr_ctx, None) diff --git a/scripts/qi/fd_leak/isolate_05_parallel_one_real_one_noop.py b/scripts/qi/fd_leak/isolate_05_parallel_one_real_one_noop.py index 066b2c8df0..470afcab55 100644 --- a/scripts/qi/fd_leak/isolate_05_parallel_one_real_one_noop.py +++ b/scripts/qi/fd_leak/isolate_05_parallel_one_real_one_noop.py @@ -52,7 +52,7 @@ def discover_one_provider( def main() -> None: cg = ConcurrencyGroup(name="repro") with cg: - pm = create_plugin_manager() + pm = create_plugin_manager(load_entry_points=True) mngr_ctx = load_config(pm, cg) providers = get_all_provider_instances(mngr_ctx, None) provider_map = {p.name: p for p in providers} diff --git a/scripts/qi/fd_leak/isolate_08_new_providers_each_time.py b/scripts/qi/fd_leak/isolate_08_new_providers_each_time.py index fef9797041..7190363805 100644 --- a/scripts/qi/fd_leak/isolate_08_new_providers_each_time.py +++ b/scripts/qi/fd_leak/isolate_08_new_providers_each_time.py @@ -32,7 +32,7 @@ def count_real_fds() -> int: def main() -> None: cg = ConcurrencyGroup(name="repro") with cg: - pm = create_plugin_manager() + pm = create_plugin_manager(load_entry_points=True) mngr_ctx = load_config(pm, cg) gc.collect() diff --git a/scripts/qi/fd_leak/repro_fd_leak_discover_only.py b/scripts/qi/fd_leak/repro_fd_leak_discover_only.py index b3e5287ba0..d2d26d7061 100644 --- a/scripts/qi/fd_leak/repro_fd_leak_discover_only.py +++ b/scripts/qi/fd_leak/repro_fd_leak_discover_only.py @@ -33,7 +33,7 @@ def main() -> None: cg = ConcurrencyGroup(name="repro") with cg: - pm = create_plugin_manager() + pm = create_plugin_manager(load_entry_points=True) mngr_ctx = load_config(pm, cg) print("\n--- Test 1: discover local only ---") diff --git a/scripts/qi/fd_leak/repro_grpclib_fd_leak.py b/scripts/qi/fd_leak/repro_grpclib_fd_leak.py index 45028335fc..510e9d3c7d 100644 --- a/scripts/qi/fd_leak/repro_grpclib_fd_leak.py +++ b/scripts/qi/fd_leak/repro_grpclib_fd_leak.py @@ -46,7 +46,7 @@ def main() -> None: cg = ConcurrencyGroup(name="repro") with cg: - pm = create_plugin_manager() + pm = create_plugin_manager(load_entry_points=True) mngr_ctx = load_config(pm, cg) print("\n--- Test 1: discover both providers in parallel (via ConcurrencyGroupExecutor) ---") diff --git a/scripts/qi/fd_leak/repro_list_agents_fd_leak.py b/scripts/qi/fd_leak/repro_list_agents_fd_leak.py index ca7d679740..1f89029b06 100644 --- a/scripts/qi/fd_leak/repro_list_agents_fd_leak.py +++ b/scripts/qi/fd_leak/repro_list_agents_fd_leak.py @@ -78,7 +78,7 @@ def main() -> None: cg = ConcurrencyGroup(name="repro") with cg: - pm = create_plugin_manager() + pm = create_plugin_manager(load_entry_points=True) mngr_ctx = load_config(pm, cg) initial_fds = count_open_fds() From d8edb404aa3c11567e605d402fe2b20d75c9d2bc Mon Sep 17 00:00:00 2001 From: Evan Ryan Gunter Date: Sat, 11 Jul 2026 18:11:28 -0700 Subject: [PATCH 04/11] Slim recovery-constants comment; fix ruff format and ty error - Shorten the _PLUGIN_GROUP_INVOCATION_NAMES / _PLUGIN_RECOVERY_SUBCOMMANDS comment (the full rationale already lives in _is_plugin_recovery_invocation and create_plugin_manager). - Apply ruff format (collapse the _is_plugin_recovery_invocation return), which test_no_ruff_errors (ruff format --check, repo-wide) was flagging. - Fix a ty type error in test_recovery_invocation_constants_match_click_tree: cli.commands["plugin"] is statically typed click.Command (no .commands), so import the plugin Group object directly and compare cli.commands values against it. Type-correct and still verifies the group name/alias wiring. Co-Authored-By: Claude Opus 4.8 (1M context) --- libs/mngr/imbue/mngr/main.py | 19 +++++-------------- libs/mngr/imbue/mngr/main_test.py | 3 +-- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/libs/mngr/imbue/mngr/main.py b/libs/mngr/imbue/mngr/main.py index 1673767173..ce3f07d1ed 100644 --- a/libs/mngr/imbue/mngr/main.py +++ b/libs/mngr/imbue/mngr/main.py @@ -69,17 +69,10 @@ _plugin_manager_container: dict[str, pluggy.PluginManager | None] = {"pm": None} -# Names (canonical + aliases) under which the `mngr plugin` group is reachable, and the -# subcommands of that group that must keep working even when a third-party plugin fails to -# import. `plugin remove` / `plugin disable` are the escape hatches a user runs to recover -# from a broken plugin, so for those invocations we deliberately skip loading setuptools -# entry points (see create_plugin_manager): a broken plugin must not be able to brick the -# very command that removes or disables it. Every OTHER command still loads all plugins and -# fails loudly if one is broken -- mngr does not run in a degraded state. -# -# These are hardcoded because the decision must be made from sys.argv at import time, before -# Click parses arguments. test_recovery_invocation_constants_match_click_tree keeps them in -# sync with the real `plugin` command tree so the sets cannot silently drift. +# The `mngr plugin` group's names (canonical + aliases) and the subcommands that must keep +# working when a third-party plugin fails to import (see _is_plugin_recovery_invocation). +# Hardcoded because the decision is made from sys.argv at import time, before Click parses +# args; test_recovery_invocation_constants_match_click_tree keeps them in sync with the tree. _PLUGIN_GROUP_INVOCATION_NAMES: Final[frozenset[str]] = frozenset({"plugin", "plug"}) _PLUGIN_RECOVERY_SUBCOMMANDS: Final[frozenset[str]] = frozenset({"remove", "disable"}) @@ -99,9 +92,7 @@ def _is_plugin_recovery_invocation(argv: Sequence[str]) -> bool: the normal load path -- i.e. the pre-existing crash on a broken plugin -- so erring toward not matching is safe. """ - return ( - len(argv) >= 3 and argv[1] in _PLUGIN_GROUP_INVOCATION_NAMES and argv[2] in _PLUGIN_RECOVERY_SUBCOMMANDS - ) + return len(argv) >= 3 and argv[1] in _PLUGIN_GROUP_INVOCATION_NAMES and argv[2] in _PLUGIN_RECOVERY_SUBCOMMANDS def _call_on_error_hook(ctx: click.Context, error: BaseException) -> None: diff --git a/libs/mngr/imbue/mngr/main_test.py b/libs/mngr/imbue/mngr/main_test.py index 5a00dfe643..eccae129d7 100644 --- a/libs/mngr/imbue/mngr/main_test.py +++ b/libs/mngr/imbue/mngr/main_test.py @@ -6,6 +6,7 @@ import pytest +from imbue.mngr.cli.plugin import plugin as plugin_group from imbue.mngr.main import _PLUGIN_GROUP_INVOCATION_NAMES from imbue.mngr.main import _PLUGIN_RECOVERY_SUBCOMMANDS from imbue.mngr.main import _is_plugin_recovery_invocation @@ -89,8 +90,6 @@ def test_recovery_invocation_constants_match_click_tree() -> None: (canonical + aliases) or its subcommands drift away from those hardcoded sets, so the fast-path cannot silently diverge from what Click actually dispatches. """ - plugin_group = cli.commands["plugin"] - reachable_names = frozenset(name for name, command in cli.commands.items() if command is plugin_group) assert reachable_names == _PLUGIN_GROUP_INVOCATION_NAMES, ( f"The `plugin` group is reachable as {sorted(reachable_names)}, but recovery detection " From 3568488d8c3c034d4cf61eb98cafe5c21f2cc062 Mon Sep 17 00:00:00 2001 From: Evan Ryan Gunter Date: Sat, 11 Jul 2026 18:54:31 -0700 Subject: [PATCH 05/11] Add test: broken plugin crashes full load but not recovery load Crystallize the manual end-to-end verification. Inject a fake `mngr` entry point whose module raises ImportError on import (mimicking a provider whose dependency shipped a breaking release), then assert that create_plugin_manager(load_entry_points=False) -- the path taken by `mngr plugin remove` / `mngr plugin disable` -- survives, while create_plugin_manager(load_entry_points=True) -- every other command -- raises the broken import. Uses monkeypatch.syspath_prepend so importlib.metadata discovers the .dist-info; teardown removes it so it does not leak into other tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- libs/mngr/imbue/mngr/main_test.py | 38 +++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/libs/mngr/imbue/mngr/main_test.py b/libs/mngr/imbue/mngr/main_test.py index eccae129d7..a417ae4913 100644 --- a/libs/mngr/imbue/mngr/main_test.py +++ b/libs/mngr/imbue/mngr/main_test.py @@ -145,3 +145,41 @@ def test_create_plugin_manager_blocks_disabled_plugins_even_in_recovery( pm = create_plugin_manager(load_entry_points=False) assert pm.is_blocked("modal") + + +def test_create_plugin_manager_broken_entry_point_only_crashes_full_load( + project_config_dir: Path, + temp_git_repo_cwd: Path, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A plugin whose import raises crashes a full load but not a recovery load. + + This is the core guarantee behind ``mngr plugin remove`` / ``mngr plugin disable``: those + commands build the manager with load_entry_points=False (see get_or_create_plugin_manager), + so a plugin whose import blows up (e.g. a provider whose dependency shipped a breaking + release) cannot brick them, while every other command (load_entry_points=True) still fails + loudly rather than running in a degraded state. + """ + (project_config_dir / "settings.toml").write_text("is_allowed_in_pytest = true\n") + + # Register a fake "mngr" entry point whose module raises on import. syspath_prepend makes + # importlib.metadata discover the .dist-info and makes the (broken) module importable; + # both are undone on test teardown. + (tmp_path / "broken_ep_mod.py").write_text('raise ImportError("simulated broken plugin")\n') + dist_info = tmp_path / "imbue_mngr_brokentest-0.0.0.dist-info" + dist_info.mkdir() + (dist_info / "METADATA").write_text("Metadata-Version: 2.1\nName: imbue-mngr-brokentest\nVersion: 0.0.0\n") + (dist_info / "entry_points.txt").write_text("[mngr]\nbrokentest = broken_ep_mod\n") + monkeypatch.syspath_prepend(str(tmp_path)) + + # Recovery load (what plugin remove / disable use) survives: the broken entry point is + # never imported, and the manager still has mngr's built-ins. + recovery_pm = create_plugin_manager(load_entry_points=False) + assert not recovery_pm.has_plugin("brokentest") + assert recovery_pm.get_plugin("builtin_help_topics") is not None + + # Full load (what every other command uses) crashes on the broken import. The match= + # ensures it fails on *our* broken plugin, not some unrelated load error. + with pytest.raises(ImportError, match="simulated broken plugin"): + create_plugin_manager(load_entry_points=True) From 9bf4d99d59f2ed6f902c256ed7f6c1c9b70ddfc0 Mon Sep 17 00:00:00 2001 From: Evan Ryan Gunter Date: Sat, 11 Jul 2026 19:24:11 -0700 Subject: [PATCH 06/11] Explain why the broken-plugin test needs monkeypatch.syspath_prepend Add a comment on the syspath_prepend call: pluggy discovers plugins by scanning importlib.metadata.distributions() over sys.path, so the fake .dist-info must be on sys.path to be found -- the only way to exercise the real entry-point load with a broken import. Co-Authored-By: Claude Opus 4.8 (1M context) --- libs/mngr/imbue/mngr/main_test.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libs/mngr/imbue/mngr/main_test.py b/libs/mngr/imbue/mngr/main_test.py index a417ae4913..afdebf4672 100644 --- a/libs/mngr/imbue/mngr/main_test.py +++ b/libs/mngr/imbue/mngr/main_test.py @@ -163,14 +163,17 @@ def test_create_plugin_manager_broken_entry_point_only_crashes_full_load( """ (project_config_dir / "settings.toml").write_text("is_allowed_in_pytest = true\n") - # Register a fake "mngr" entry point whose module raises on import. syspath_prepend makes - # importlib.metadata discover the .dist-info and makes the (broken) module importable; - # both are undone on test teardown. + # Register a fake "mngr" entry point whose module raises on import. (tmp_path / "broken_ep_mod.py").write_text('raise ImportError("simulated broken plugin")\n') dist_info = tmp_path / "imbue_mngr_brokentest-0.0.0.dist-info" dist_info.mkdir() (dist_info / "METADATA").write_text("Metadata-Version: 2.1\nName: imbue-mngr-brokentest\nVersion: 0.0.0\n") (dist_info / "entry_points.txt").write_text("[mngr]\nbrokentest = broken_ep_mod\n") + # syspath_prepend is required: pluggy's load_setuptools_entrypoints discovers plugins by + # scanning importlib.metadata.distributions() over sys.path, so the fake .dist-info must be + # on sys.path to be found (and its module importable). This is the only way to exercise the + # real entry-point load with a broken import -- a pm.register() fake never imports anything. + # It is a pytest built-in (not setattr, so no ratchet) and is undone on teardown. monkeypatch.syspath_prepend(str(tmp_path)) # Recovery load (what plugin remove / disable use) survives: the broken entry point is From a73b0793ab68580560c4b4d77d06de8ba46eb774 Mon Sep 17 00:00:00 2001 From: Evan Ryan Gunter Date: Sat, 11 Jul 2026 19:28:49 -0700 Subject: [PATCH 07/11] Reword syspath_prepend comment to contrast with pm.register fakes Co-Authored-By: Claude Opus 4.8 (1M context) --- libs/mngr/imbue/mngr/main_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/mngr/imbue/mngr/main_test.py b/libs/mngr/imbue/mngr/main_test.py index afdebf4672..80e6ba068b 100644 --- a/libs/mngr/imbue/mngr/main_test.py +++ b/libs/mngr/imbue/mngr/main_test.py @@ -171,9 +171,9 @@ def test_create_plugin_manager_broken_entry_point_only_crashes_full_load( (dist_info / "entry_points.txt").write_text("[mngr]\nbrokentest = broken_ep_mod\n") # syspath_prepend is required: pluggy's load_setuptools_entrypoints discovers plugins by # scanning importlib.metadata.distributions() over sys.path, so the fake .dist-info must be - # on sys.path to be found (and its module importable). This is the only way to exercise the - # real entry-point load with a broken import -- a pm.register() fake never imports anything. - # It is a pytest built-in (not setattr, so no ratchet) and is undone on teardown. + # on sys.path (and its module importable) to be found. Registering a fake object with + # pm.register(), as the other plugin tests do, does not work here: it never imports + # anything, so it cannot reproduce an import failure. Undone on teardown. monkeypatch.syspath_prepend(str(tmp_path)) # Recovery load (what plugin remove / disable use) survives: the broken entry point is From 2bf7a18170f69060244c7063fe22cbf134a59263 Mon Sep 17 00:00:00 2001 From: Evan Ryan Gunter Date: Sat, 11 Jul 2026 19:32:26 -0700 Subject: [PATCH 08/11] Break the test plugin via a missing import, assert on ModuleNotFoundError.name More naturalistic (a real plugin breaks when a dependency is missing or was renamed) and lets the test assert on the structured ModuleNotFoundError.name attribute instead of matching the exception message text. Co-Authored-By: Claude Opus 4.8 (1M context) --- libs/mngr/imbue/mngr/main_test.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/libs/mngr/imbue/mngr/main_test.py b/libs/mngr/imbue/mngr/main_test.py index 80e6ba068b..48e7c81140 100644 --- a/libs/mngr/imbue/mngr/main_test.py +++ b/libs/mngr/imbue/mngr/main_test.py @@ -153,18 +153,22 @@ def test_create_plugin_manager_broken_entry_point_only_crashes_full_load( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - """A plugin whose import raises crashes a full load but not a recovery load. + """A plugin whose import fails crashes a full load but not a recovery load. This is the core guarantee behind ``mngr plugin remove`` / ``mngr plugin disable``: those commands build the manager with load_entry_points=False (see get_or_create_plugin_manager), - so a plugin whose import blows up (e.g. a provider whose dependency shipped a breaking - release) cannot brick them, while every other command (load_entry_points=True) still fails + so a plugin whose import blows up (e.g. a provider whose dependency is missing or was + renamed) cannot brick them, while every other command (load_entry_points=True) still fails loudly rather than running in a degraded state. """ (project_config_dir / "settings.toml").write_text("is_allowed_in_pytest = true\n") - # Register a fake "mngr" entry point whose module raises on import. - (tmp_path / "broken_ep_mod.py").write_text('raise ImportError("simulated broken plugin")\n') + # Register a fake "mngr" entry point whose module imports a nonexistent dependency, so + # loading it raises ModuleNotFoundError -- exactly how a real plugin breaks when a + # dependency is missing or was renamed. The sentinel name lets us assert the failure is + # ours (via ModuleNotFoundError.name) without matching on the message text. + missing_dep = "mngr_missing_dependency_for_broken_plugin_test" + (tmp_path / "broken_ep_mod.py").write_text(f"import {missing_dep}\n") dist_info = tmp_path / "imbue_mngr_brokentest-0.0.0.dist-info" dist_info.mkdir() (dist_info / "METADATA").write_text("Metadata-Version: 2.1\nName: imbue-mngr-brokentest\nVersion: 0.0.0\n") @@ -182,7 +186,8 @@ def test_create_plugin_manager_broken_entry_point_only_crashes_full_load( assert not recovery_pm.has_plugin("brokentest") assert recovery_pm.get_plugin("builtin_help_topics") is not None - # Full load (what every other command uses) crashes on the broken import. The match= - # ensures it fails on *our* broken plugin, not some unrelated load error. - with pytest.raises(ImportError, match="simulated broken plugin"): + # Full load (what every other command uses) crashes on the broken import. Asserting on + # ModuleNotFoundError.name confirms it failed on *our* plugin, not some unrelated error. + with pytest.raises(ModuleNotFoundError) as excinfo: create_plugin_manager(load_entry_points=True) + assert excinfo.value.name == missing_dep From d2134eaa02c43fbcbdc327fe226baec9ef399b6e Mon Sep 17 00:00:00 2001 From: Evan Ryan Gunter Date: Sat, 11 Jul 2026 19:32:45 -0700 Subject: [PATCH 09/11] Simplify broken-plugin test docstring wording Co-Authored-By: Claude Opus 4.8 (1M context) --- libs/mngr/imbue/mngr/main_test.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libs/mngr/imbue/mngr/main_test.py b/libs/mngr/imbue/mngr/main_test.py index 48e7c81140..4670607e8b 100644 --- a/libs/mngr/imbue/mngr/main_test.py +++ b/libs/mngr/imbue/mngr/main_test.py @@ -157,9 +157,8 @@ def test_create_plugin_manager_broken_entry_point_only_crashes_full_load( This is the core guarantee behind ``mngr plugin remove`` / ``mngr plugin disable``: those commands build the manager with load_entry_points=False (see get_or_create_plugin_manager), - so a plugin whose import blows up (e.g. a provider whose dependency is missing or was - renamed) cannot brick them, while every other command (load_entry_points=True) still fails - loudly rather than running in a degraded state. + so a plugin with a missing dependency cannot brick them, while every other command + (load_entry_points=True) still fails loudly rather than running in a degraded state. """ (project_config_dir / "settings.toml").write_text("is_allowed_in_pytest = true\n") From 2911b077382a9a30646a93c3dd9867f9c98a98de Mon Sep 17 00:00:00 2001 From: Evan Ryan Gunter Date: Sat, 11 Jul 2026 19:33:28 -0700 Subject: [PATCH 10/11] Trim broken-plugin test comment Co-Authored-By: Claude Opus 4.8 (1M context) --- libs/mngr/imbue/mngr/main_test.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libs/mngr/imbue/mngr/main_test.py b/libs/mngr/imbue/mngr/main_test.py index 4670607e8b..7661d1e3ab 100644 --- a/libs/mngr/imbue/mngr/main_test.py +++ b/libs/mngr/imbue/mngr/main_test.py @@ -163,9 +163,7 @@ def test_create_plugin_manager_broken_entry_point_only_crashes_full_load( (project_config_dir / "settings.toml").write_text("is_allowed_in_pytest = true\n") # Register a fake "mngr" entry point whose module imports a nonexistent dependency, so - # loading it raises ModuleNotFoundError -- exactly how a real plugin breaks when a - # dependency is missing or was renamed. The sentinel name lets us assert the failure is - # ours (via ModuleNotFoundError.name) without matching on the message text. + # loading it raises ModuleNotFoundError. missing_dep = "mngr_missing_dependency_for_broken_plugin_test" (tmp_path / "broken_ep_mod.py").write_text(f"import {missing_dep}\n") dist_info = tmp_path / "imbue_mngr_brokentest-0.0.0.dist-info" From b94479cab4b157313bef6289808beb4080e0b24d Mon Sep 17 00:00:00 2001 From: Evan Ryan Gunter Date: Sat, 11 Jul 2026 19:40:24 -0700 Subject: [PATCH 11/11] Drop needless is_allowed_in_pytest writes from config-free recovery tests The pytest opt-in guard only fires for config files that are actually loaded, so a test that reads no config content does not need one. Two of the create_plugin_manager recovery tests wrote a settings.toml containing only the flag -- the write existed solely to satisfy a guard on the file it was creating. Remove those writes (and the now-unused project_config_dir fixture); temp_git_repo_cwd still isolates cwd so no real config is picked up. The tests that write actual config (disabling modal) keep the flag, as they must. Co-Authored-By: Claude Opus 4.8 (1M context) --- libs/mngr/imbue/mngr/main_test.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/libs/mngr/imbue/mngr/main_test.py b/libs/mngr/imbue/mngr/main_test.py index 7661d1e3ab..43c8da61e2 100644 --- a/libs/mngr/imbue/mngr/main_test.py +++ b/libs/mngr/imbue/mngr/main_test.py @@ -104,7 +104,6 @@ def test_recovery_invocation_constants_match_click_tree() -> None: def test_create_plugin_manager_skips_entry_points_in_recovery( - project_config_dir: Path, temp_git_repo_cwd: Path, ) -> None: """With load_entry_points=False, no third-party setuptools entry point is imported. @@ -113,8 +112,6 @@ def test_create_plugin_manager_skips_entry_points_in_recovery( `plugin remove` / `plugin disable` run even when one of those entry points fails to import. mngr's own built-ins (registered directly, not via entry points) still load. """ - (project_config_dir / "settings.toml").write_text("is_allowed_in_pytest = true\n") - pm = create_plugin_manager(load_entry_points=False) entry_point_names = {ep.name for ep in importlib.metadata.entry_points(group="mngr")} @@ -148,7 +145,6 @@ def test_create_plugin_manager_blocks_disabled_plugins_even_in_recovery( def test_create_plugin_manager_broken_entry_point_only_crashes_full_load( - project_config_dir: Path, temp_git_repo_cwd: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, @@ -160,8 +156,6 @@ def test_create_plugin_manager_broken_entry_point_only_crashes_full_load( so a plugin with a missing dependency cannot brick them, while every other command (load_entry_points=True) still fails loudly rather than running in a degraded state. """ - (project_config_dir / "settings.toml").write_text("is_allowed_in_pytest = true\n") - # Register a fake "mngr" entry point whose module imports a nonexistent dependency, so # loading it raises ModuleNotFoundError. missing_dep = "mngr_missing_dependency_for_broken_plugin_test"