Skip to content

Respect explicitly disabled plugin platforms#32148

Closed
JohnnyMa9ic wants to merge 1 commit into
NousResearch:mainfrom
JohnnyMa9ic:fix/respect-disabled-plugin-platforms
Closed

Respect explicitly disabled plugin platforms#32148
JohnnyMa9ic wants to merge 1 commit into
NousResearch:mainfrom
JohnnyMa9ic:fix/respect-disabled-plugin-platforms

Conversation

@JohnnyMa9ic

Copy link
Copy Markdown

Summary

  • Preserve explicit disabled platform config when plugin platform auto-enablement runs
  • Skip auto-enabling plugin adapters when YAML/env config explicitly set enabled: false
  • Rebased the held local fix onto current upstream main

Tests

  • ./venv/bin/python -m pytest tests/gateway/test_slack_mention.py -q -o 'addopts='

@hclsys hclsys left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The generalization is correct and lands the guard in the right place. I traced producer/consumer:

  • Producer :783 (now under the generalized if "enabled" in plat_block: at :779) sets extra["_enabled_explicit"] = True for any platform with an explicit enabled in YAML — previously Slack-only. Good.
  • The new consumer guard in _apply_env_overrides (:1823) sits inside the general plugin auto-enable loop (platform_registry.plugin_entries(), gateway/config.py:1813+) — which the existing comment block (:1805-1812) identifies as exactly the path that "silently enables platforms the user never opted into" via check_fn(). So skipping auto-enable when _enabled_explicit and not existing_cfg.enabled is the precise fix for the reported bug. Leaving the Slack env-token block (:1300) Slack-scoped is also correct — that path handles SLACK_BOT_TOKEN setup, not generic plugin auto-enable.

One fragility worth a defensive note, not a blocker — the _enabled_explicit marker is consumed with a destructive .pop() at two sites: Slack's env block (:1300) and the new plugin-loop guard (:1823). Today they don't collide because Slack is a built-in platform, not a plugin_entries() member, so only one consumer ever sees a given platform's marker. But if a platform is ever both env-handled and registered as a plugin entry, whichever pop runs first consumes the marker and the second site defaults to False — silently losing the "explicitly disabled" signal. Since the value is a simple bool, reading it non-destructively (extra.get("_enabled_explicit", False)) at the consume sites and clearing it once at the end (or not at all) would remove that ordering dependency. Cheap insurance against a future platform that straddles both paths.

Logic LGTM.

@JohnnyMa9ic

Copy link
Copy Markdown
Author

Closing: opened from a mistaken local repository identity/confusion. This PR does not belong to this project.

@JohnnyMa9ic JohnnyMa9ic deleted the fix/respect-disabled-plugin-platforms branch May 25, 2026 16:28
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins area/config Config system, migrations, profiles labels May 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #31596 (and #31665). All three fix _apply_env_overrides() ignoring explicit enabled:false for plugin platforms. #31596 was first submitted and targets the general plugin platform path.

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

Labels

area/config Config system, migrations, profiles comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants