Skip to content

feat(registry): add opt-out for entry-point plugin autoloading - #164

Open
0xjunger wants to merge 2 commits into
robocurve:mainfrom
0xjunger:feature/plugin-autoload-opt-out
Open

feat(registry): add opt-out for entry-point plugin autoloading#164
0xjunger wants to merge 2 commits into
robocurve:mainfrom
0xjunger:feature/plugin-autoload-opt-out

Conversation

@0xjunger

Copy link
Copy Markdown

Motivation

registry._ensure_loaded() imports every installed inspect_robots.* entry
point on first discovery (list / resolve). Locked-down or
reproducibility-sensitive eval environments currently have no way to disable
that automatic import.

Change

Adds INSPECT_ROBOTS_DISABLE_PLUGIN_AUTOLOAD. Any non-empty value skips
entry-point discovery, so only in-tree builtins and hand-registered components
resolve. This mirrors pytest's PYTEST_DISABLE_PLUGIN_AUTOLOAD.

  • Read at discovery time and not latched: clearing the var re-enables
    discovery within the same process.
  • Builtins always load, opt-out or not.

Scope / non-goals

This is a defense-in-depth and reproducibility switch, not a security
boundary: an installed package can still run code when imported. It only stops
the framework from importing plugins on your behalf during discovery.

Tests

Two new tests covering the skip path and the not-latched behavior. Full core
suite passes at 100% coverage.

0xjunger and others added 2 commits July 24, 2026 11:15
Discovery imports every installed inspect_robots.* entry point on first
lookup (list/resolve). Locked-down and reproducibility-sensitive eval
environments have no way to turn that off.

Add INSPECT_ROBOTS_DISABLE_PLUGIN_AUTOLOAD: any non-empty value skips
entry-point discovery so only in-tree builtins and hand-registered
components resolve. Mirrors pytest's PYTEST_DISABLE_PLUGIN_AUTOLOAD.

The flag is read at discovery time and deliberately not latched, so
clearing it re-enables discovery within the same process. This is a
defense-in-depth and reproducibility switch, not a security boundary: an
installed package can still run code when imported.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jeqcho

jeqcho commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Can you open an issue describing the situation that prompted this? I'd like the use case on record before committing to an env var, since that contract is permanent once it ships.

The implementation does work, I checked rather than assumed. With the var set, registered() returns builtins only across all five workspace plugins, and clearing it mid-process really does re-enable discovery, so the not-latched behaviour is genuine and not just monkeypatch-deep. Builtins load either way, as you say.

What gives me pause is the motivation. An environment locked down enough to care about discovery-time imports also controls what gets pip-installed, and the PR says up front this isn't a security boundary. The case I find more convincing is the mundane one: muting a misbehaving installed plugin without uninstalling it, and keeping eval manifests independent of whatever happens to be in the venv. If that's closer to what you're actually hitting, say so in the issue and this gets a lot easier to say yes to.

One thing worth fixing regardless. Setting the var breaks two existing tests, test_entrypoint_discovery and test_entrypoint_load_failure_is_skipped_with_warning, because discovery skips and the fake plugin never registers. tests/conftest.py exists to keep the suite hermetic against the developer's machine and already shields .env for exactly this reason, so this wants an autouse monkeypatch.delenv(reg.DISABLE_AUTOLOAD_ENV, raising=False) with your two new tests opting in via setenv. As it stands, anyone who actually uses the feature gets a red suite locally, which is a bad first experience of it.

CONTRIBUTING.md:131 also wants a CHANGELOG entry under Unreleased, and docs/guide/plugins.md is where people would look for this.

@jeqcho jeqcho left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this — the feature is well-motivated, the pytest precedent is a good one to mirror, and the core design is sound and pleasantly minimal. The not-latched semantics in src/inspect_robots/registry.py:104-106 are implemented correctly (the early return happens before _loaded_entrypoints is set, so clearing the variable re-enables discovery), builtins load unconditionally, INSPECT_ROBOTS_DISABLE_PLUGIN_AUTOLOAD is consistent with the existing INSPECT_ROBOTS_POLICY / INSPECT_ROBOTS_EMBODIMENT naming, and the module docstring is honest about this being defense-in-depth rather than a security boundary. registry.py is also the only discovery point in the package, so there is no second cache to invalidate. CI is green across the matrix and the full suite passes locally.

Three things before this merges:

  1. Missing CHANGELOG.md entry. CONTRIBUTING.md (step 4 under "Submitting changes") asks for an entry under "Unreleased" — this is a user-visible feature, so it should get an ### Added line.

  2. No user-facing documentation. The env var currently only lives in the module docstring. docs/guide/plugins.md is where users will look for plugin behavior, and docs/guide/cli.md:34 already documents the other INSPECT_ROBOTS_* variables. A short section in plugins.md (a couple of sentences — what it does, that builtins still load, that it mirrors pytest's variable, and that it's not a security boundary) would close the gap. Worth stating there that the switch is read at discovery time, so setting it after discovery has already run does not unregister plugins that were loaded earlier in the process.

  3. test_autoload_opt_out_skips_entrypoints_but_keeps_builtins cannot fail on regression (tests/test_registry_cli.py:80-84). The _FakeEP.load() tripwire raises AssertionError, but _ensure_loaded wraps ep.load() in a blanket except Exception that converts it to a RuntimeWarning — and the suite does not run with warnings-as-errors. Verified empirically: with _autoload_disabled() hardcoded to False (simulating a broken opt-out), this test still passes; only test_autoload_opt_out_is_not_latched catches it. Suggested fix: have load() set a called flag (or return a real factory) and assert not called / probe not in policies, or wrap the call in pytest.warns/recwarn assertions. As written, the builtins assertion is the only part of this test doing real work.

Two optional notes, no action required:

  • Per the docstring, any non-empty value disables autoload, so INSPECT_ROBOTS_DISABLE_PLUGIN_AUTOLOAD=0 also disables it. That matches pytest's behavior and is documented, but it is a common surprise — worth calling out explicitly in the guide docs from item 2.
  • The pre-existing sentence at registry.py:5-7 ("appears in inspect-robots list without being imported first") contradicts what discovery actually does — ep.load() imports the plugin — and your new paragraph sits directly beneath it, making the tension more visible. Not this PR's bug, but since you are already editing the docstring, tightening that sentence would be a welcome drive-by.

Happy to re-review once the changelog, docs, and test tweak land — the core implementation itself looks ready. Thanks again! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants