Respect explicitly disabled plugin platforms#32148
Conversation
hclsys
left a comment
There was a problem hiding this comment.
The generalization is correct and lands the guard in the right place. I traced producer/consumer:
- Producer
:783(now under the generalizedif "enabled" in plat_block:at:779) setsextra["_enabled_explicit"] = Truefor any platform with an explicitenabledin 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" viacheck_fn(). So skipping auto-enable when_enabled_explicit and not existing_cfg.enabledis the precise fix for the reported bug. Leaving the Slack env-token block (:1300) Slack-scoped is also correct — that path handlesSLACK_BOT_TOKENsetup, 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.
|
Closing: opened from a mistaken local repository identity/confusion. This PR does not belong to this project. |
Summary
Tests