Skip to content

fix(cli): emit clear warning when toolset belongs to disabled category (#59547) - #59653

Open
SquabbyZ wants to merge 1 commit into
NousResearch:mainfrom
SquabbyZ:fix/easy-08-issue-59547
Open

fix(cli): emit clear warning when toolset belongs to disabled category (#59547)#59653
SquabbyZ wants to merge 1 commit into
NousResearch:mainfrom
SquabbyZ:fix/easy-08-issue-59547

Conversation

@SquabbyZ

@SquabbyZ SquabbyZ commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #59547

Summary

When a configured toolset belongs to a plugin that's been disabled or uninstalled, the validator was emitting a misleading 'unknown toolset' warning — often paired with a wrong hermes-<platform> typo guess that sent users hunting for a misspelling that didn't exist.

What changed

  • hermes_cli/toolset_validation.py::validate_platform_toolsets() now takes an optional known_plugin_toolsets map. When a toolset name is not in the registered catalog but IS found under known_plugin_toolsets[platform] (written by _save_platform_tools), the validator emits a distinct 'plugin is disabled or uninstalled — re-enable it in plugins.enabled' warning and skips the typo guess.
  • The new parameter is threaded through the single production call site in hermes_cli/config.py (post-migration validation pass) via read_raw_config().get('known_plugin_toolsets'). Defaults to None, so all 13 existing tests/hermes_cli/test_toolset_validation.py tests still pass with zero regression.
  • Genuine typos still fall through to the existing generic warning (no regression).
  • Zero-valid safety net from bug: config migration (v25->v26) corrupts platform_toolsets, silently kills all tools #38798 still fires when all references are stale (no regression).

How to test

# 1. Disable a plugin whose toolset is configured
hermes plugins disable my-plugin
hermes config validate                    # → 'plugin disabled' warning, NOT 'unknown toolset'

# 2. Genuine typo still caught
hermes config set toolsets.enabled herms-cli     # → existing 'unknown toolset' warning

# 3. Healthy toolset — no warning
hermes config set toolsets.enabled hermes-cli
hermes config validate                    # → no warning

Test plan

  • 17 new tests in tests/cli/test_toolset_validation_warnings.py covering: disabled-plugin → plugin warning; genuine typo → falls through to generic warning; valid toolset → no warning; platform-scoped known-map isolation; malformed known_plugin_toolsets falls back to pre-fix behavior; zero-valid safety net from bug: config migration (v25->v26) corrupts platform_toolsets, silently kills all tools #38798 still fires.
  • All 17 new tests pass.
  • All 13 existing tests/hermes_cli/test_toolset_validation.py tests still pass (no regression).

Platforms tested

  • Linux (CI-equivalent: TZ=UTC LANG=C.UTF-8 PYTHONHASHSEED=0 python -m pytest)

AI-assisted contribution

This PR was drafted as part of an automated contribution sweep driven by https://github.com/SquabbyZ/peaks-loop. The first attempt was opened against the wrong base repo (the fork instead of NousResearch/hermes-agent) and was closed before this one was opened. All code changes were generated by an AI coding assistant from the issue body — please flag anything that looks off.

NousResearch#59547)

validate_platform_toolsets() in hermes_cli/toolset_validation.py previously
emitted a generic 'unknown toolset <name>' warning (sometimes paired with an
irrelevant 'hermes-<platform>' typo guess) whenever a toolset name in
platform_toolsets wasn't in the live toolset registry. That conflates two
distinct failure modes: a real typo vs. a previously-valid plugin toolset
whose plugin has since been disabled or uninstalled.

Cross-reference known_plugin_toolsets (per-platform snapshot written by
_save_platform_tools in hermes_cli/tools_config.py) BEFORE falling back to
the generic warning. When a name is found there, emit a distinct message
pointing at plugins.enabled and the likely package-uninstall path, and skip
the misleading hermes-<platform> guess.

The new known_plugin_toolsets parameter is optional and defaults to None,
so existing callers and tests aren't affected. The single production call
site in hermes_cli/config.py is updated to pass
read_raw_config().get('known_plugin_toolsets'), reusing the same
try/except-wrapped best-effort validation pass that already runs after
every config migration.

Tests in tests/cli/test_toolset_validation_warnings.py cover:
  - known-but-now-missing plugin toolset -> plugin warning, no typo guess
  - platform-scoped known map (no leakage to other platforms)
  - genuine typo falls through to the existing generic warning
  - valid toolset produces no warning (pre-NousResearch#59547 regression guard)
  - malformed/missing known_plugin_toolsets falls back to pre-NousResearch#59547 output
  - the zero-valid safety net from NousResearch#38798 still fires when all references
    on a platform are stale

AI-assisted fix by https://github.com/SquabbyZ/peaks-loop
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles P3 Low — cosmetic, nice to have labels Jul 6, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: this competes with the open PR #56731 for the same issue #59547 via the same approach (thread known_plugin_toolsets into validate_platform_toolsets() to distinguish a disabled/uninstalled plugin from a genuine typo). Flagging for a maintainer to pick the canonical fix.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused config-warning fix. The premise remains present on current main: hermes_cli/toolset_validation.py:58-66 emits the generic warning for every unloaded toolset, while hermes_cli/tools_config.py:1989-1996 already persists the per-platform plugin snapshot needed to distinguish this case.

Problems

  • The new tests call the helper directly, but the behavioral change also depends on the production propagation at hermes_cli/config.py:6153-6155. There is no temp-HERMES_HOME migrate_config() regression test proving known_plugin_toolsets is read and forwarded; a wiring regression there would silently restore the generic warning.

Suggested changes

  • Add a tests/hermes_cli/test_config.py migration test with both config maps and assert results["warnings"] contains the plugin-specific warning.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
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/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Validation warning is misleading when a toolset belongs to a disabled plugin

3 participants