Skip to content

fix(toolsets): validate hermes-<platform> bundles for plugin platforms - #102158

Closed
nunico wants to merge 1 commit into
NousResearch:mainfrom
nunico:fix/t_13402d60-validate-plugin-toolsets
Closed

nunico wants to merge 1 commit into
NousResearch:mainfrom
nunico:fix/t_13402d60-validate-plugin-toolsets

Conversation

@nunico

@nunico nunico commented Sep 3, 2026

Copy link
Copy Markdown

What does this PR do?

validate_toolset() and resolve_toolset() disagree about hermes-<platform>
bundles for platforms that ship as plugins.

resolve_toolset() synthesizes a bundle for any platform in the platform
registry (toolsets.py:812-828), giving it the core tools plus whatever the
plugin registered. validate_toolset() checks only TOOLSETS, the registry's
toolset names, and registry aliases, so it rejects a name that resolves to a
full working tool list. On this install that is 10 of 22 registered platforms:

hermes-a2a          validate=False  resolved_tools=58
hermes-buzz         validate=False  resolved_tools=53
hermes-google_chat  validate=False  resolved_tools=53
hermes-irc          validate=False  resolved_tools=53
hermes-line         validate=False  resolved_tools=53
hermes-ntfy         validate=False  resolved_tools=53
hermes-photon       validate=False  resolved_tools=53
hermes-raft         validate=False  resolved_tools=53
hermes-simplex      validate=False  resolved_tools=53
hermes-teams        validate=False  resolved_tools=53

Config validation treats validate_toolset() as authoritative
(hermes_cli/config.py:2776 feeds it into validate_platform_toolsets()), so
hermes update emits two false warnings per affected platform, one of which
suggests the exact name it just rejected:

platform 'teams' references unknown toolset 'hermes-teams' — did you mean 'hermes-teams'?
platform 'teams' has no valid toolsets configured — the agent will have no tools on this platform.

The second message is the damaging one: it tells the user their platform is
broken when its tools resolve fine, and the remedy it suggests is a no-op.

The fix adds a registry-gated branch to validate_toolset() so the predicate
agrees with the resolver. Because it is gated on
platform_registry.is_registered(), an unregistered name is still rejected, so
typos are still caught, and the lazy import is wrapped so a registry failure
degrades to False rather than propagating into config load.

Scope note

This is not the plugin-discovery-ordering bug (#71650, #91757, #95529, and
PRs #97374, #91761, #89351, #89345, #86233, #84499). That one is about when
validation runs relative to plugin load; those PRs make plugin toolset names
like a2a validate by deferring validation or by consulting
known_plugin_toolsets.

This is a separate defect in what validate_toolset() knows, and it survives
any ordering fix: hermes-teams is missing from TOOLSETS, from the registry's
toolset names, and from known_plugin_toolsets (that key records plugin
toolsets, not platform bundles), so it stays False even with plugins fully
loaded. I deliberately did not touch the ordering half; that work belongs to the
PRs above.

Measured on this install after full plugin discovery, so ordering is not a
factor:

a2a           validate=True   (ordering fixes cover this)
hermes-teams  validate=False  resolved_tools=53   (this PR)

Related Issue

Refs #71650

Using Refs rather than Fixes: #71650 is the ordering bug and is not closed
by this change.

Type of Change

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

Changes Made

  • toolsets.py: new _is_plugin_platform_bundle() helper, and
    validate_toolset() consults it as a final fallback.
  • tests/test_toolsets_plugin_platform_validation.py: 5 pins covering the
    bundle validating, the validate/resolve invariant, an unregistered name still
    being rejected, non-bundle names being unaffected, and a raising registry
    degrading to False.

How to Test

  1. Add a plugin platform's bundle to platform_toolsets in config.yaml
    (e.g. teams: [hermes-teams]), then run hermes update. Before this
    change it prints the two warnings quoted above; after, it prints neither.

  2. Reproduced end-to-end against a copy of a real config.yaml in an isolated
    HERMES_HOME, running the same validate_platform_toolsets() call
    hermes_cli/config.py makes, after import model_tools has forced plugin
    discovery:

    without fix: 4 warnings (hermes-teams and hermes-google_chat, x2 each)
    with fix:    0 warnings
    
  3. Negative control on the new tests: with toolsets.py reverted to
    upstream/main (git diff --quiet upstream/main -- toolsets.py confirmed
    clean), 2 of the 5 pins fail:

    FAILED test_plugin_platform_bundle_validates
    FAILED test_validate_agrees_with_resolve_for_plugin_platform
    2 failed, 3 passed
    

    The 3 that pass either way are the guard tests, which is what I want from
    them: they must hold before and after.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Fedora Silverblue (kernel 7.1.4), Python 3.11.15

Not ticking the full-suite box. I ran the affected suites, not the whole tree:

tests/test_toolsets.py
tests/test_toolsets_plugin_platform_validation.py
tests/hermes_cli/test_toolset_validation.py
tests/tools/test_delegate_composite_toolsets.py       -> 44 passed
tests/hermes_cli/test_tools_config.py
tests/hermes_cli/test_commands.py
tests/tui_gateway/test_gui_surface_toolsets.py
tests/hermes_cli/test_kanban_worker_spawn_toolsets.py -> 124 passed, 6 skipped
tests/ -k toolset (excluding tests/gateway/relay)     -> 253 passed, 11 skipped, 6 failed

The 6 failures in that last run (test_api_server.py::TestToolsetsEndpoint,
test_mcp_reload_refreshes_cached_agents.py, and 4 in
test_multiplex_toolsets_profile_isolation.py) are pre-existing: they fail
identically with toolsets.py reverted to upstream/main. Two collection
errors under tests/gateway/relay/ are a missing pytest_asyncio in my
environment, also present on unmodified main. CI is the authority on the full
tree.

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

No config keys, docs, or tool schemas change. The logic is a pure string check
plus a registry membership test with no platform-specific behaviour, but I ran
it only on Linux.

Blast radius

validate_toolset() gets strictly more permissive, and only for
hermes--prefixed names backed by a registered platform. Callers that gate on
it (model_tools.py:442/:466, hermes_cli/oneshot.py, cli.py:5532,
toolset_distributions.py, tui_gateway/server.py) previously dropped these
bundles or warned about them; they now accept a name whose
resolve_toolset() already returned real tools, which is the behaviour those
call sites assume. Nothing that validated before stops validating.

resolve_toolset() synthesizes a hermes-<platform> bundle for any platform in
the platform registry, so plugin platforms resolve to a real tool list even
though the name is absent from TOOLSETS. validate_toolset() had no matching
branch, so the two disagreed for every platform shipped as a plugin rather
than a built-in: 10 of 22 registered platforms on the reporting install.

Config validation reads validate_toolset() as authoritative, which made
`hermes update` report a resolvable bundle as unknown and then suggest the
name it had just rejected ("references unknown toolset 'hermes-teams', did
you mean 'hermes-teams'?"), followed by a false claim that the platform would
have no tools at all.

The new branch is registry-gated, so an unregistered name is still rejected
and typos are still caught.

Refs NousResearch#71650
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tools Tool registry, model_tools, toolsets comp/plugins Plugin system and bundled plugins duplicate This issue or pull request already exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Sep 3, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Duplicate of #57230. Both PRs implement the same registry-gated hermes-<platform> validation branch in validate_toolset() for dynamic plugin-platform bundles.

@nunico nunico closed this Sep 7, 2026
@nunico
nunico deleted the fix/t_13402d60-validate-plugin-toolsets branch September 7, 2026 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets duplicate This issue or pull request already exists 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.

2 participants