Skip to content

fix(toolsets): platform plugins without a static bundle silently get zero tools; config set stores list literals as strings - #57063

Closed
sam7894604 wants to merge 3 commits into
NousResearch:mainfrom
sam7894604:fix/platform-toolset-silent-failures
Closed

fix(toolsets): platform plugins without a static bundle silently get zero tools; config set stores list literals as strings#57063
sam7894604 wants to merge 3 commits into
NousResearch:mainfrom
sam7894604:fix/platform-toolset-silent-failures

Conversation

@sam7894604

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes two silent-failure paths that leave a gateway platform running with zero tools while everything looks configured.

1. Eight bundled platform plugins ship without a hermes-<platform> static bundle

_get_platform_tools falls back to hermes-<platform> for any platform missing from config's platform_toolsets, and resolve_toolset returns [] for an unknown name (the #38798 shape). The runtime auto-generate in resolve_toolset only fires once gateway.platform_registry has the platform registered, so outside the gateway process (cron delivery, kanban dispatch, hermes tools, doctor) — and in the explicit-config composite expansion, which skips names absent from TOOLSETS — these platforms silently degrade to zero tools:

line, google_chat, teams, irc, ntfy, photon, simplex, raft

LINE shipped this way in #23197 ("zero core edits" by design). Real-world impact we hit: a LINE deployment ran for weeks with a zero-tool agent — no read_file, no memory, no MCP servers — so the model could only ask the user for information it was supposed to look up; every turn was api_calls=1 with no diagnostic anywhere.

Fix: add the 8 missing static bundles (core tools, mirroring hermes-telegram / hermes-signal), include them in the hermes-gateway composite, and add a regression test that walks plugins/platforms/ so the next platform plugin cannot ship without a bundle.

2. hermes config set stores list/mapping literals as strings

set_config_value only coerces bool/int/float. A list literal (e.g. hermes config set platform_toolsets.discord '["clarify","file",...]') is stored as a raw string with no warning — and every reader gated on isinstance(..., list) (_get_platform_tools, _get_enabled_set, _get_disabled_set) silently ignores it and falls back to its default. The setting looks saved but never takes effect. We found both a platform_toolsets entry and a plugins.enabled entry in this state in the wild.

Fix: values starting with [ or { are parsed with yaml.safe_load; non-list/dict results and YAML errors warn on stderr and keep the legacy string behavior.

Tests

  • tests/test_toolsets.py::TestBundledPlatformBundles — every plugins/platforms/<name>/ with a plugin.yaml must have a non-empty static bundle (include_registry=False); the 8 new bundles resolve to core tools; gateway composite includes them.
  • tests/hermes_cli/test_config_set_list_values.py — list/mapping literals parse to real lists/dicts, YAML flow lists work, invalid literals warn and keep string behavior, scalars unaffected.
  • Full tests/test_toolsets.py, tests/hermes_cli/test_tools_config.py, tests/hermes_cli/test_managed_scope_writeguard.py, tests/test_toolset_distributions.py: 170 passed.

Related

Every bundled platform plugin without a matching hermes-<platform> static
bundle silently resolved to ZERO tools outside the gateway process:
_get_platform_tools falls back to hermes-<platform> for platforms missing
from platform_toolsets, resolve_toolset returns [] for unknown names
(NousResearch#38798 shape), and the runtime auto-generate only fires once
gateway.platform_registry has the platform registered. The explicit-config
composite expansion also skips names absent from TOOLSETS.

LINE shipped without a bundle in NousResearch#23197 (zero core edits by design) and
ran with zero tools for any deployment that never hand-listed it in
platform_toolsets. Same for google_chat, teams, irc, ntfy, photon,
simplex, raft.

Adds the 8 missing static bundles (core tools, mirroring
hermes-telegram/hermes-signal), includes them in the hermes-gateway
composite, and adds a regression test that walks plugins/platforms/ so
the next platform plugin cannot ship without a bundle.
hermes config set only coerced bool/int/float; a list or mapping literal
(e.g. platform_toolsets.line set to a JSON-style list) was stored as a
raw STRING with no warning. Every reader gated on isinstance(..., list)
— _get_platform_tools, _get_enabled_set, _get_disabled_set — then
silently ignored the value and fell back to its default, so the setting
looked saved but never took effect (observed in the wild: a platform
running on the wrong toolset bundle for weeks, and a plugins.enabled
entry that never enabled anything).

Values starting with '[' or '{' are now parsed with yaml.safe_load;
non-list/dict results and YAML errors warn on stderr and keep the
legacy string behavior.
@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists labels Jul 2, 2026
…o the real switch

`hermes plugins enable gemini` printed a green success message and wrote
model-providers/gemini into plugins.enabled — but nothing ever reads that
flag for model providers: the general loader explicitly skips
kind: model-provider (handled by providers/__init__.py's own discovery,
selected via `hermes model` / model.provider) and kind: exclusive
(activated via `<category>.provider`). Same for disable: the entry lands
in plugins.disabled, the loader records it for introspection, and the
provider registers anyway.

The success message misleads users into believing they switched a
provider on or off. Found in the wild: a config with
model-providers/gemini in BOTH plugins.enabled (as a stray string) and
plugins.disabled, while the gemini provider had been registered and
usable the whole time.

enable/disable now detect the manifest kind and print what actually
controls the plugin, changing nothing:

  ! model-providers/gemini is a model provider — it is not controlled by
    plugins.enabled/disabled (providers register automatically at startup).
      To use it:       run `hermes model` and pick it, or set model.provider.
      To stop using it: select a different provider; remove its API key.
    Nothing was changed.
HexLab98 added a commit to HexLab98/hermes-agent-fork that referenced this pull request Jul 5, 2026
…silently dropping tools

A YAML indentation slip nests toolset names under a mapping, e.g.

    discord:
      - hermes-discord:
          - browser
          - terminal

which parses to `[{'hermes-discord': ['browser', 'terminal', ...]}]`.
`_get_platform_tools` normalised with `[str(ts) for ts in toolset_names]`,
turning the mapping into the literal string `"{'hermes-discord': [...]}"` —
a name that matches no toolset. `has_explicit_config` stayed False for the
real toolsets and every nested toolset was silently dropped, so the platform
loaded almost no tools (the model could reach cronjob/tts but not
terminal/file/web/browser) while config.yaml looked correct, with no warning
anywhere.

Sibling fixes (NousResearch#38798/NousResearch#52920 invalid names -> zero tools, NousResearch#57063 missing
plugin bundles / list-stored-as-string) don't cover this shape: the result
is non-empty-but-wrong, so their zero-tools guards never fire. Add
`_flatten_toolset_names`, which recovers the intended names (mapping keys +
nested values) so the platform still works and logs a loud warning so the
malformed config is visible and fixable. Well-formed flat lists pass through
unchanged with no warning.

This resolves through the shared resolver, so it fixes CLI, messaging
gateway, and TUI alike.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing two real configuration failure modes. The list/mapping coercion gap remains on current main (hermes_cli/config.py:8264-8279), but the platform-bundle part needs a narrower current-main repro before salvage.

Problems

  • Current platform plugins deliberately resolve dynamically: commit 52d9e578 introduced support without toolsets.py entries, and toolsets.py:731-750 supplies core tools for registered platforms. Deferred loaders count as registered (gateway/platform_registry.py:271-276). The new static-bundle invariant (tests/test_toolsets.py:321-328) reverses that design without demonstrating a live path where dynamic registration is unavailable.
  • _plugin_kind() reads raw kind text (hermes_cli/plugins_cmd.py:835-836), but the loader normalizes and heuristically detects kind-less providers (hermes_cli/plugins.py:1583-1627). Those providers can still bypass the proposed guard.
  • The new provider hint hardcodes ~/.hermes/.env (hermes_cli/plugins_cmd.py:852), which is not profile-safe.

Suggested changes

  • Reproduce and target the exact resolver boundary that loses tools, preserving dynamic plugin-platform support.
  • Share the loader's kind classification, and use display_hermes_home() in the hint.

Automated hermes-sweeper review.

Comment thread tests/test_toolsets.py
if not child.is_dir() or not (child / "plugin.yaml").exists():
continue
bundle = f"hermes-{child.name}"
if bundle not in TOOLSETS or not resolve_toolset(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This permanently requires static core bundles for every platform plugin, but current dynamic-platform support was introduced specifically to avoid toolsets.py entries (commit 52d9e57; current toolsets.py:731-750). Please first demonstrate the current-main path where the deferred registry cannot resolve the platform, then constrain the regression to that path rather than reversing the plugin contract.

Comment thread hermes_cli/plugins_cmd.py
import yaml

data = yaml.safe_load(mf.read_text(encoding="utf-8")) or {}
return str(data.get("kind", "standalone"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This raw manifest read disagrees with the loader: PluginManager normalizes kind and heuristically classifies kind-less memory/model providers (hermes_cli/plugins.py:1583-1627). Such a provider will still bypass this guard and receive the misleading success path. Reuse the canonical classification or duplicate its normalization and heuristic with coverage.

Comment thread hermes_cli/plugins_cmd.py
" To use it: run [bold]hermes model[/bold] and pick it, or set "
"[dim]model.provider[/dim] in config.yaml.\n"
" To stop using it: select a different provider; remove its API key "
"from ~/.hermes/.env to make it unselectable.\n"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use display_hermes_home() here. A literal ~/.hermes/.env is incorrect for named profiles and custom HERMES_HOME directories.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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
@cadamec

cadamec commented Aug 5, 2026

Copy link
Copy Markdown

Empirical status check on current main (Aug 5, 2026) — one of the two failure modes no longer reproduces.

I ran the exact resolver boundary the hermes-sweeper review asked to narrow down:

$ python -c "from hermes_cli.plugins import PluginManager; m=PluginManager(); m.discover_and_load()
from toolsets import resolve_toolset
[print(p, len(resolve_toolset(f'hermes-{p}'))) for p in [...]]"
line        -> 61 tools (core)
google_chat -> 61 tools
teams       -> 61 tools
irc         -> 61 tools
ntfy        -> 61 tools
photon      -> 61 tools
simplex     -> 61 tools
raft        -> 61 tools
a2a         -> 61 tools (core only — client tools still missing, see #78050)
telegram    -> 61 tools

After plugin discovery (which any CLI/cron/doctor process runs), all eight platforms from the PR's list resolve to the full core toolset via the dynamic auto-generation in resolve_toolset (52d9e57, 'feat: dynamic toolset generation for plugin platforms'). The 'platform runs with zero tools' failure mode from the PR body no longer reproduces on current main — deferred loaders count as registered (platform_registry.is_registered), so the hermes-<platform> fallback gets core tools in any process, gateway or not.

What still reproduces, confirmed live:

  1. a2a client tools (a2a_call etc.) resolve to zero after discovery in a CLI process — the dynamic path only supplies core tools; plugin-registered tools still require the deferred adapter to materialize. That's A2A client tools invisible to CLI/TUI sessions — deferred platform plugin never registers its toolset outside gateway processes #78050, and it's the same deferred-loader mechanism this PR originally targeted.
  2. hermes config set list/dict literals stored as strings — still present at hermes_cli/config.py:4903-4914 (only bool/int/float coercion). Confirmed with platform_toolsets.discord '["clarify","file"]' landing in config.yaml as a string.

I'm working a PR that fixes both remaining halves (a2a client tools at discovery + config-set list/dict coercion) with tests. The 8 static bundles part of this PR looks dead on current main — the dynamic generation made it obsolete — so I'd suggest scoping the salvage to the config-set coercion half.

@cadamec

cadamec commented Aug 5, 2026

Copy link
Copy Markdown

I rebased this PR onto current main and narrowed it to the two halves that are still live. The rebased branch is at cadamec:rebase-57063-live-halves — cherry-pick or diff it however you prefer; authorship on both commits is preserved (Sam Liu).

What I did:

  1. Rebased onto current main — the branch was ~15 commits behind and CONFLICTING. One conflict in hermes_cli/config.py (the coercion block) resolved by keeping main's string-preservation guard (_default_value_for_key) and folding in the list/dict parsing.

  2. Dropped the static-bundles commit (eb1a9a7fb) — it's obsolete on current main. Commit 52d9e57's dynamic toolset generation already fixed the 'eight platforms resolve to zero tools' case; I verified all eight now resolve to the full 61-tool core toolset after discovery in any process. The PR's own test for it (test_every_bundled_platform_plugin_has_a_static_bundle) now fails on main because new platforms (a2a, buzz) don't have bundles — the test itself proves the approach can't keep up.

  3. Kept the two live fixes:

    • fix(config): parse list/mapping literals in hermes config set — still broken on main, verified live
    • fix(plugins): refuse enable/disable on passive kinds — still broken on main, verified live (the _plugin_kind/_print_passive_kind_hint helpers don't exist in current plugins_cmd.py)

Verification: 191 tests pass on the rebased branch (config coercion, passive kinds, plugins, toolsets suites).

If you'd rather I open this as a fresh PR (with you credited as original author), say the word — or take the branch and force-push it to your own. Goal is the two live fixes landing on current main.

@teknium1

Copy link
Copy Markdown
Contributor

Partial overlap note: PR #86660 (#86660) fixed the deferred-platform client-tool registration class via a provides_tools manifest opt-in — a2a is covered, and any of the platforms listed here that ship outbound client tools can opt in by declaring the field. The config set list-literal half of this issue is untouched and still open.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(toolsets): platform plugins without a static bundle silently get zero tools; config set stores list literals as strings

Two solid fixes with good tests. Observations:

  1. hermes_cli/config.py set_config_value() — the coercion fires on ANY key whose value starts with [ or {, including legitimately string-typed values ("[", "{a: b}" as text, a display prefix like "[debug] "). "[a, b]" silently becomes a list even when the user meant a literal string. Consider scoping the coercion to keys whose readers expect collections (or strict JSON first with YAML flow only as fallback), and document an escape hatch for literal strings.

  2. yaml.safe_load accepts YAML 1.1 flow quirks (trailing commas, bare on/off keys) that strict JSON would reject — a typo like ["file", "web",] parses silently, which is inconsistent with the JSON-style input users will naturally type. Minor.

  3. tests/test_toolsets.pytest_new_platform_bundles_resolve_to_core_tools and test_gateway_composite_includes_all_platform_bundles hardcode the 8 platforms, so a new platform plugin must be added in three places (toolsets.py + both test lists). The first test (every bundled platform has a static bundle) already enforces the invariant; derive the other two from the platforms directory so they cannot drift.

  4. hermes_cli/plugins_cmd.py _plugin_kind() silently returns "standalone" on any manifest read error — a corrupted plugin.yaml (e.g. kind: model-provider with broken YAML) makes the passive-kind guard not fire and plugins enable silently writes the dead flag again, which is exactly what this PR is eliminating. Distinguish "no manifest" from "unreadable manifest".

@alt-glitch alt-glitch added P3 Low — cosmetic, nice to have 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 and removed P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 16, 2026
@sam7894604

Copy link
Copy Markdown
Contributor Author

Note upfront: this comment was written and posted by an AI agent on behalf of
@sam7894604, who asked me to go through their open PRs here and clean up the
stale ones. They contribute to this project in their spare time and hadn't
caught up on notifications for a while, which is why this has sat since July.

Closing this PR.

First, thanks are owed. @cadamec — the empirical status check on Aug 5 was
useful on its own, but rebasing the branch to
cadamec:rebase-57063-live-halves, keeping authorship intact on both commits,
and then offering to open it as a fresh PR with credit back to the original
author was a genuinely considerate way to handle someone else's stale work.
@Enough1122, thanks for the review — the _plugin_kind() point is a real bug
and is addressed below. @teknium1, thanks for the pointer to #86660.

The reason for closing is that this PR bundled several separate problems, and
they have each gone their own way since July:

  1. config set storing list/mapping literals as strings — landed. It was
    split out as fix(config): parse list/mapping literals in hermes config set #59182, picked up as the cherry-picked base for fix(config): parse structured list/dict values in hermes config set (consolidates 8-PR cluster) #88163, and
    merged today, with the trigger widened to multi-line YAML blocks on top.

  2. Platform plugins without a static bundle resolving to zero tools
    superseded by 52d9e578 (dynamic toolset generation). @cadamec's check
    confirmed all eight platforms now resolve to the full core toolset after
    discovery in any process. The rest of it is worth stating plainly: this
    PR's own test_every_bundled_platform_plugin_has_a_static_bundle fails on
    current main, because newer platforms (a2a, buzz) ship without a bundle.
    The test written to protect the invariant is exactly what demonstrates that
    a hand-maintained static bundle list cannot keep pace with new platforms.
    The dynamic approach is the right one.

  3. a2a client tools resolving to zero after discovery — covered by fix(plugins): register deferred platform client tools at discovery; admit plugin toolsets in config and hermes tools (#81163) #86660's
    provides_tools manifest opt-in, which is a cleaner fit than anything in
    here, since it lets any deferred-loading platform declare its outbound
    tools instead of special-casing them.

  4. Refusing plugins enable/disable on passive plugin kinds — still
    live, and already split out as fix(plugins): refuse enable/disable on passive plugin kinds with a pointer to the real switch #59183. Work continues there.
    @Enough1122's point applies to that PR too: _plugin_kind() falls back to
    "standalone" when a manifest cannot be read, so a corrupted plugin.yaml
    makes the guard silently not fire, which is worse than having no guard. It
    will distinguish "no manifest" from "unreadable manifest" before that PR
    goes back up for review.

The takeaway on this end is to split work up front rather than ship a
multi-part PR and watch the parts age at different rates. No attachment to
whose branch a fix comes from — the better implementations landing is the
point.

@sam7894604 sam7894604 closed this Aug 17, 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 comp/plugins Plugin system and bundled plugins comp/tools Tool registry, model_tools, toolsets 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.

5 participants