Skip to content

fix(config): distinguish disabled-plugin toolsets from typos in validation - #601

Open
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56731
Open

fix(config): distinguish disabled-plugin toolsets from typos in validation#601
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56731

Conversation

@hashbender

Copy link
Copy Markdown
Owner

What changed

validate_platform_toolsets() (hermes_cli/toolset_validation.py, added for NousResearch#38798) now accepts an optional third argument, known_plugin_toolsets, and cross-references it against unknown toolset names to produce a more accurate warning when the real cause is a disabled/uninstalled plugin rather than a typo.

Why

Hermes tracks toolset names across three separate places:

  • plugins.enabled — which plugins are turned on
  • known_plugin_toolsets — per-platform snapshot of plugin toolset keys seen the last time hermes tools saved that platform (hermes_cli/tools_config.py::_save_platform_tools)
  • platform_toolsets — the actual per-platform toolset selection

validate_platform_toolsets already does excellent work catching corrupted/renamed entries (the NousResearch#38798 incident: hermes-cli silently rewritten to the nonexistent hermes), including a hermes-<platform> "did you mean" guess. But that guess is actively misleading for a very common real-world case: a platform_toolsets entry that names a real plugin toolset whose plugin is now disabled (or its package uninstalled). Since is_valid_toolset (backed by the live toolset registry) only knows about currently-loaded plugins, a disabled plugin's toolset name looks identical to a typo to the existing check — same generic "unknown toolset ''" warning, with no hermes-<platform> hint either (since that guess doesn't happen to match), leaving the real cause unstated.

known_plugin_toolsets already has exactly the information needed to tell these apart: if a name is in known_plugin_toolsets[platform], it was a real, valid plugin toolset the last time hermes tools ran for that platform — so its current invalidity almost certainly means the plugin got disabled or uninstalled, not that the name was ever wrong.

Approach

  • New optional known_plugin_toolsets param, default None (or any non-dict) → identical behavior to before this change, so existing callers/tests aren't broken.
  • When a name fails is_valid_toolset AND is found in known_plugin_toolsets[platform], emit a distinct, more actionable warning pointing at plugins.enabled and plugin installation, instead of the generic unknown-toolset message with its (here, irrelevant) hermes-<platform> guess.
  • Threaded through the one existing call site in hermes_cli/config.py (read_raw_config().get("known_plugin_toolsets")), reusing the same try/except-wrapped, best-effort validation pass that already runs after every config migration.
  • No new call sites, no new validation pass — purely widening the existing one.

Test plan

  • Extended tests/hermes_cli/test_toolset_validation.py with:
    • a disabled-plugin case producing the new plugin-specific warning (and confirming it does not also carry the typo-guess wording)
    • a case where the name is known for a different platform, confirming it still falls back to the generic unknown-toolset warning (the cross-check is platform-scoped)
    • omitted/malformed known_plugin_toolsets (None, [], a string, {} with a non-list value) all falling back to the pre-existing generic behavior
  • Ran scripts/run_tests.sh tests/hermes_cli/test_toolset_validation.py tests/hermes_cli/test_config.py — 153 tests, all passing, no regressions in the config-migration suite that exercises this call site.

Mirror-of: NousResearch#56731
NousResearch#56731

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.

1 participant