Skip to content

fix(config): stop flagging declared plugin toolsets as unknown - #109844

Closed
prasta1 wants to merge 1 commit into
NousResearch:mainfrom
prasta1:fix/plugin-toolset-validation
Closed

prasta1 wants to merge 1 commit into
NousResearch:mainfrom
prasta1:fix/plugin-toolset-validation

Conversation

@prasta1

@prasta1 prasta1 commented Sep 13, 2026

Copy link
Copy Markdown

What does this PR do?

platform_toolsets validation (added for #38798) runs inside migrate_config(), which executes
before plugins load and register their toolsets. validate_toolset() consults the tool
registry, the registry is still empty at that point, so every plugin-provided toolset is reported
as unknown on each hermes update:

⚠  platform 'cli' references unknown toolset 'a2a' — did you mean 'hermes-cli'?
⚠  platform 'telegram' references unknown toolset 'a2a' — did you mean 'hermes-telegram'?
⚠  platform 'discord' references unknown toolset 'a2a' — did you mean 'hermes-discord'?

The names are correct and resolve fine at runtime, so the warning is purely false — and the
suggested "did you mean hermes-cli?" fix would actively make the config worse if followed.

The config already knows these names are legitimate. The hermes tools save flow writes
known_plugin_toolsets on every save (hermes_cli/tools_config.py:703), and reads it back
per-platform at tools_config.py:539. Validation simply never consulted it. This wires it up.

Scoped per platform on purpose: flattening every platform's names into one set would let a
plugin toolset declared for discord mask a genuine typo under cli. Omitting the new argument
reproduces the previous behavior exactly, so no existing caller changes.

Related Issue

No open issue filed for this specific symptom. Prior art, all still unfixed on main:

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/toolset_validation.py — validate_platform_toolsets() gains an optional
    known_plugin_toolsets parameter (the raw config section, {platform: [name, ...]}). A name
    declared for that platform is treated as valid instead of warned about, and is not run through
    is_allowed_for_platform(), which would reject it for the same not-yet-loaded reason. New
    _known_plugin_toolsets() helper normalizes the section and tolerates malformed shapes.
  • hermes_cli/config.py — _warn_invalid_platform_toolsets() reads known_plugin_toolsets
    from the raw config and passes it through (one read_raw_config() call, reused).
  • tests/hermes_cli/test_toolset_validation.py — 5 new tests (11 cases with parametrization).

How to Test

  1. pytest tests/hermes_cli/test_toolset_validation.py -q
  2. Or reproduce end-to-end: enable any plugin that provides a toolset (e.g. the bundled
    a2a-platform), add its toolset name to a platform's list in platform_toolsets, and run
    hermes update. Before: one false "unknown toolset" warning per platform. After: none.
  3. Confirm nothing is over-suppressed — a genuine typo still warns, including a plugin toolset
    name listed under a platform it was not declared for (covered by
    test_plugin_toolset_declared_for_another_platform_still_warns).

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate — see Related Issue
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass — see note below
  • I've added tests for my changes
  • I've tested on my platform: macOS 26 (Darwin 27.0.0), Python 3.14

Note: pytest is not present in the Hermes runtime venv, so I executed this file's 23 tests
(12 pre-existing + 11 new) directly against the runtime interpreter — all pass, no regressions —
and verified end-to-end that a real config that previously emitted 3 warnings now emits 0. The
full pytest tests/ -q sweep was not run locally; happy to post the result if CI doesn't cover it.

Documentation & Housekeeping

  • Docstrings updated on both changed functions — or N/A
  • N/A — no new config keys; known_plugin_toolsets already exists and is already written by hermes tools
  • N/A — no architecture or workflow change
  • Cross-platform impact considered — pure Python, no platform-specific behavior
  • N/A — no tool description/schema change

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard comp/plugins Plugin system and bundled plugins area/config Config system, migrations, profiles sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Sep 13, 2026
platform_toolsets validation (NousResearch#38798) runs during migrate_config(), which is
before plugins load and register their toolsets. validate_toolset() therefore
reports every plugin toolset as unknown, so `hermes update` prints a warning
for a name that resolves correctly at runtime:

    platform 'cli' references unknown toolset 'a2a' — did you mean 'hermes-cli'?

The config already records which plugin toolsets are legitimate: the
`hermes tools` save flow writes known_plugin_toolsets on every save, and
tools_config.py reads it back per platform. Validation just never consulted it.

validate_platform_toolsets() gains an optional known_plugin_toolsets parameter
and accepts names declared there for that platform. Scoped per platform so one
platform's plugin names cannot mask a typo under another. Omitting the argument
reproduces the previous behavior exactly.
@prasta1
prasta1 force-pushed the fix/plugin-toolset-validation branch from 3800504 to 5d2a31a Compare September 24, 2026 17:56
@kshitijk4poor

Copy link
Copy Markdown

Thanks @prasta1. Merged in #127782 (16c59d0 on main). hermes config migrate / hermes update and hermes config check now judge saved platform_toolsets with one shared resolver, hermes_cli.toolset_validation.saved_toolset_resolver. Plugin-provided toolsets are looked up only for names the cheaper checks can't place. That lookup runs plugin discovery when needed, so a plugin toolset is no longer judged against an empty registry during migration.

Verified on 16c59d0, platform_toolsets: {cli: [hermes-cli, a2a], telegram: [hermes-telegram, a2a]}:

  • before: unknown toolset 'a2a' for both platforms
  • after: none

known_plugin_toolsets is deliberately not treated as proof: it would hide toolsets of plugins that have since been uninstalled. Closing as resolved, with credit in #127782's description.

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 comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have 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.

3 participants