Skip to content

fix(gateway): preserve explicit platform disables - #73303

Open
CryptoDombili wants to merge 1 commit into
NousResearch:mainfrom
CryptoDombili:fix/respect-platform-enabled-false
Open

fix(gateway): preserve explicit platform disables#73303
CryptoDombili wants to merge 1 commit into
NousResearch:mainfrom
CryptoDombili:fix/respect-platform-enabled-false

Conversation

@CryptoDombili

Copy link
Copy Markdown

What does this PR do?

Preserves explicit platforms.<name>.enabled: false settings during environment credential bootstrap.

Environment variables may still populate credentials and defaults, but they no longer re-enable a platform that the operator explicitly disabled in config.yaml.

Related issue

Fixes #73289

Related to #55447, which covers a narrower subset of the same configuration-precedence problem.

Type of change

  • Bug fix
  • Tests
  • New feature
  • Security fix
  • Documentation update
  • Refactor

Changes made

  • add a shared helper for applying environment-driven platform enablement
  • preserve explicit enabled: false values from YAML
  • retain existing auto-enable behavior when a platform is absent or enabled is unspecified
  • apply the same precedence rule to built-in and plugin-registry platform paths
  • add cross-platform regression coverage for explicit disables and env-only configuration

Testing

  • focused precedence regression tests: 22 passed
  • related Gateway test suite: 348 passed
  • Python syntax validation passed

Checklist

  • Explicit operator configuration remains authoritative
  • Env-only platform configuration still auto-enables
  • Built-in and plugin platform paths are covered
  • Regression tests were added
  • No unrelated files were modified

Prevent environment credential bootstrap from overriding explicit platform enabled:false settings and add cross-platform regression coverage.
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery area/config Config system, migrations, profiles sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades P2 Medium — degraded but workaround exists labels Jul 28, 2026
@PRATHAMESH75

Copy link
Copy Markdown
Contributor

Reviewed against #73289 — this fully and correctly closes the issue, including the second location the report called out. Verified against the PR head:

  • The new _enable_from_env(platform) helper (gateway/config.py:1818) is the right centralization: env-only/absent platforms auto-enable, but a platform already present and disabled is only re-enabled when _enabled_explicit is not set — so an explicit platforms.<name>.enabled: false is authoritative while env vars still seed credentials/defaults. That matches the issue's "expected behavior" exactly.
  • Both re-enable paths are covered. The env-bootstrap sites (~all the WhatsApp/Slack/Email/SMS/HASS/API-server/… branches) now route through the helper, and the plugin-registry pass is guarded too (config.py:2481-2486 continues on an explicitly-disabled platform before the enabled = True at 2574). I grepped the head file: the only remaining .enabled = True assignments are those two guarded ones — no straggler unconditional enable.
  • Test matrix is thorough and maps to the failure modes: explicit-disable-wins, disable-preserved-across-yaml-shapes, env-only-still-enables, unspecified-still-enables, and both plugin-registry cases (test_plugin_registry_does_not_probe_or_enable_explicitly_disabled_platform / ..._env_only_auto_enable_is_unchanged). That directly covers the multi-profile token-lock/kill-loop scenario in the issue.

Green on required checks and mergeable (BLOCKED = awaiting maintainer approval, not a failure). Nice fix — LGTM.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused precedence fix. The premise remains verified on current main: gateway/config.py:2081-2089 still force-enables Home Assistant when HASS_TOKEN is present, and equivalent unconditional bootstrap assignments remain for Email, SMS, webhook, DingTalk, Feishu, WeCom, Weixin, BlueBubbles, QQBot, and Yuanbao. The PR replaces those sites with the existing _enable_from_env() contract at gateway/config.py:1820-1835, which preserves an explicit enabled: false while retaining env-only and unspecified-enable auto-activation.

The new tests/gateway/test_platform_env_enablement_precedence.py covers the affected built-in paths, supported YAML forms, env-only activation, unspecified enablement, and the plugin-registry guard. This extends the currently narrow API-server-only regression at tests/gateway/test_config.py:1155-1186.

Automated hermes-sweeper review.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Eight PRs address or reference the environment-versus-YAML platform-precedence defect. #18418 and #36014 are Telegram-focused, #35562 adds Discord and plugin handling, #55447 covers HomeAssistant/Email/SMS, #73376 adds repeated built-in guards, #38745 and #73724 mix the fix with broader changes, and #73303 centralizes the built-in and plugin-registry policy with regression coverage.

Related pull requests

Duplicates

#18418 and #36014 are Telegram-focused variants overlapping #35562. #55447 is the HomeAssistant/Email/SMS subset of #73303; #73376 and the gateway portion of #73724 duplicate #73303's systemic repair, while #38745 overlaps the same precedence policy but adds separate CLI, credential-cleanup, and tool-management scope.

Suggested consolidation

Keep #73303 open with a salvage path: author action: rebase onto main, or split out the part that can merge, preserving its focused gateway/config.py changes, plugin-registry guard, and tests/gateway/test_platform_env_enablement_precedence.py coverage. Close #55447 as a duplicate of #73303 despite its keep_open review because #73303 contains its three-platform helper conversion plus the requested tests; close #73376 as a duplicate of #73303 despite its keep_open review because #73303 already supplies the requested helper reuse and parameterized coverage; and close #73724 as a duplicate of #73303 despite its keep_open review because its diff has the documented MSGraph unbound-local defect, no relevant tests, and unrelated bundled changes. Keep the already-closed #18418, #35562, #36014, and #38745 closed as implemented-on-main, superseded, test-only, or over-broad historical predecessors; this does not discard the recorded best-fix verdicts for #18418 and #35562, whose narrower Telegram issue was explicitly confirmed resolved on main.

Complex graph

flowchart TD
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I73289(["issue #73289 (open)"])
    subgraph Dup38745 ["PRs duplicating each other"]
        P38745["PR #38745 (closed)"]
        P55447["PR #55447 (open)"]
        P73303["PR #73303 (open)"]
        P73376["PR #73376 (open)"]
        P73724["PR #73724 (open)"]
    end
    P73303 -->|best fix| I73289
    class I73289 open
    class P38745 closed
    class P55447 open
    class P73303 open
    class P73376 open
    class P73724 open
    class P73303 best
    class P73303 target
    click I73289 "https://github.com/NousResearch/hermes-agent/issues/73289"
    click P38745 "https://github.com/NousResearch/hermes-agent/pull/38745"
    click P55447 "https://github.com/NousResearch/hermes-agent/pull/55447"
    click P73303 "https://github.com/NousResearch/hermes-agent/pull/73303"
    click P73376 "https://github.com/NousResearch/hermes-agent/pull/73376"
    click P73724 "https://github.com/NousResearch/hermes-agent/pull/73724"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 8 pull requests and 3 issues in this complex. Each diff was read against this issue; Assessment working set: 134 kB of PR diffs, 44 kB of issue/PR text, 16 kB of discussion (25 comments), 23 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@bgrablin

Copy link
Copy Markdown
Contributor

Independent validation against current main:

  • PR head: 987b103a49696b96b34208574e3f9906c8bdfb30
  • Current origin/main: 03fa32c92dd445eb64c7f67434dd91b32c40701d
  • A local merge completed without conflicts and retained the shared _enable_from_env() conversions plus tests/gateway/test_platform_env_enablement_precedence.py.
  • On the exact PR head, the focused precedence/config suite passed: 173 passed in 48.99s.
  • After the local current-main merge, the same two test paths passed: 77 passed in 24.13s. The collection count is lower because the current-main test tree has changed since this PR's base; there were no failures.

Command used:

python -m pytest -q \
  tests/gateway/test_platform_env_enablement_precedence.py \
  tests/gateway/test_config.py

I also reproduced the defect independently on clean current main using isolated temporary HERMES_HOME configurations:

  • platforms.homeassistant.enabled: false plus HASS_TOKEN produced enabled=true.
  • platforms.webhook.enabled: false plus WEBHOOK_ENABLED=true produced enabled=true.
  • As a control, platforms.api_server.enabled: false plus a valid API_SERVER_KEY remained enabled=false.

This confirms that the precedence defect is still present and that this PR's focused systemic repair remains compatible with current main. This is limited to the stated configuration paths and automated tests; I did not rerun the repository's full suite.

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 P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: platforms.*.enabled: false in config.yaml is overridden by env credential bootstrap (all platforms)

6 participants