diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 4dc42e702ba1..a2f7b0fd8acd 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -7705,6 +7705,30 @@ def set_config_value(key: str, value: str): value = int(value) elif value.replace('.', '', 1).isdigit(): value = float(value) + elif value.lstrip()[:1] in ('[', '{'): + # List/mapping literals — e.g. + # hermes config set platform_toolsets.line '["file","web"]' + # Before this branch such values were stored as a raw STRING, and every + # reader that gates on isinstance(..., list) (``_get_platform_tools``, + # ``_get_enabled_set``, ...) silently ignored them and fell back to its + # default — the setting looked saved but never took effect. + try: + parsed = yaml.safe_load(value) + if isinstance(parsed, (list, dict)): + value = parsed + else: + print( + f"Warning: value for '{key}' looks like a list/mapping but " + f"parsed as {type(parsed).__name__}; storing as string.", + file=sys.stderr, + ) + except yaml.YAMLError: + print( + f"Warning: value for '{key}' looks like a list/mapping but is " + f"not valid YAML/JSON; storing as string. Most isinstance-gated " + f"readers will ignore a string here.", + file=sys.stderr, + ) _set_nested(user_config, key, value) # Normalize the api_base → base_url alias at set-time too (issue #8919), diff --git a/hermes_cli/plugins_cmd.py b/hermes_cli/plugins_cmd.py index fc66810489ef..d69e05809f3a 100644 --- a/hermes_cli/plugins_cmd.py +++ b/hermes_cli/plugins_cmd.py @@ -810,6 +810,57 @@ def _set_plugin_entry_flag(plugin_id: str, key: str, value: bool) -> None: save_config(config) +# Plugin kinds that the general loader does NOT gate on plugins.enabled / +# plugins.disabled: model providers register through providers/__init__.py's +# own discovery (selected via `hermes model` / model.provider), and exclusive +# category plugins (memory providers) activate via `.provider`. +# `plugins enable`/`disable` used to accept these and print a success message +# while having zero effect — the flag was written but nothing ever read it, +# which misled users into thinking they had switched something on or off. +_PASSIVE_PLUGIN_KINDS = {"model-provider", "exclusive"} + + +def _plugin_kind(key: str) -> str: + """Manifest ``kind`` for a discovered plugin key (default: ``standalone``).""" + for entry in _discover_all_plugins(): + # entry = (name, version, description, source, dir_path, key) + if entry[5] == key: + d = Path(entry[4]) + for fname in ("plugin.yaml", "plugin.yml"): + mf = d / fname + if mf.exists(): + try: + import yaml + + data = yaml.safe_load(mf.read_text(encoding="utf-8")) or {} + return str(data.get("kind", "standalone")) + except Exception: + return "standalone" + return "standalone" + + +def _print_passive_kind_hint(console, key: str, kind: str) -> None: + """Explain how a passive-kind plugin is actually controlled.""" + if kind == "model-provider": + console.print( + f"[yellow]![/yellow] [bold]{key}[/bold] is a model provider — it is " + "not controlled by plugins.enabled/disabled (providers register " + "automatically at startup).\n" + " 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" + "Nothing was changed." + ) + else: # exclusive + console.print( + f"[yellow]![/yellow] [bold]{key}[/bold] is an exclusive category " + "plugin — it is activated by its category's provider key (e.g. " + "[dim]memory.provider[/dim]), not by plugins.enabled/disabled.\n" + "Nothing was changed." + ) + + def cmd_enable(name: str, allow_tool_override: Optional[bool] = None) -> None: """Add a plugin to the enabled allow-list (and remove it from disabled). @@ -831,6 +882,11 @@ def cmd_enable(name: str, allow_tool_override: Optional[bool] = None) -> None: sys.exit(1) key, source = resolved + kind = _plugin_kind(key) + if kind in _PASSIVE_PLUGIN_KINDS: + _print_passive_kind_hint(console, key, kind) + return + enabled = _get_enabled_set() disabled = _get_disabled_set() @@ -910,6 +966,11 @@ def cmd_disable(name: str) -> None: console.print(f"[red]Plugin '{name}' is not installed or bundled.[/red]") sys.exit(1) + kind = _plugin_kind(key) + if kind in _PASSIVE_PLUGIN_KINDS: + _print_passive_kind_hint(console, key, kind) + return + enabled = _get_enabled_set() disabled = _get_disabled_set() diff --git a/tests/hermes_cli/test_config_set_list_values.py b/tests/hermes_cli/test_config_set_list_values.py new file mode 100644 index 000000000000..1dff82900725 --- /dev/null +++ b/tests/hermes_cli/test_config_set_list_values.py @@ -0,0 +1,71 @@ +"""``hermes config set`` must parse list/mapping literals, not store them as strings. + +Before this fix, ``hermes config set platform_toolsets.discord '["file","web"]'`` +stored the value as a raw STRING. Every reader that gates on +``isinstance(..., list)`` — ``_get_platform_tools``, ``_get_enabled_set``, +``_get_disabled_set`` — then silently ignored it and fell back to its default, +so the setting looked saved but never took effect (observed in the wild as a +platform running on the wrong toolset bundle for weeks). +""" +import pytest + + +@pytest.fixture +def user_home(tmp_path, monkeypatch): + home = tmp_path / "home" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.delenv("HERMES_MANAGED_DIR", raising=False) + import hermes_cli.config as cfg + from hermes_cli import managed_scope + + cfg._LOAD_CONFIG_CACHE.clear() + cfg._RAW_CONFIG_CACHE.clear() + managed_scope.invalidate_managed_cache() + return home + + +def test_list_literal_is_parsed_to_list(user_home): + from hermes_cli.config import set_config_value, read_raw_config + + set_config_value("platform_toolsets.line", '["clarify", "file", "web"]') + raw = read_raw_config() + assert raw["platform_toolsets"]["line"] == ["clarify", "file", "web"] + + +def test_mapping_literal_is_parsed_to_dict(user_home): + from hermes_cli.config import set_config_value, read_raw_config + + set_config_value("display.tool_progress_overrides", '{"terminal": "off"}') + raw = read_raw_config() + assert raw["display"]["tool_progress_overrides"] == {"terminal": "off"} + + +def test_yaml_flow_list_is_parsed(user_home): + from hermes_cli.config import set_config_value, read_raw_config + + set_config_value("plugins.enabled", "[model-providers/gemini]") + raw = read_raw_config() + assert raw["plugins"]["enabled"] == ["model-providers/gemini"] + + +def test_invalid_list_literal_warns_and_stores_string(user_home, capsys): + from hermes_cli.config import set_config_value, read_raw_config + + set_config_value("platform_toolsets.line", '["unclosed') + captured = capsys.readouterr() + assert "not valid" in captured.err.lower() or "warning" in captured.err.lower() + raw = read_raw_config() + assert raw["platform_toolsets"]["line"] == '["unclosed' + + +def test_scalar_values_unaffected(user_home): + from hermes_cli.config import set_config_value, read_raw_config + + set_config_value("agent.max_turns", "300") + set_config_value("display.compact", "true") + set_config_value("tts.provider", "edge") + raw = read_raw_config() + assert raw["agent"]["max_turns"] == 300 + assert raw["display"]["compact"] is True + assert raw["tts"]["provider"] == "edge" diff --git a/tests/hermes_cli/test_plugins_enable_passive_kinds.py b/tests/hermes_cli/test_plugins_enable_passive_kinds.py new file mode 100644 index 000000000000..368cf3128e50 --- /dev/null +++ b/tests/hermes_cli/test_plugins_enable_passive_kinds.py @@ -0,0 +1,91 @@ +"""``hermes plugins enable/disable`` must not silently no-op on passive kinds. + +Model providers (``kind: model-provider``) register through +``providers/__init__.py``'s own discovery and are selected via ``hermes model`` +/ ``model.provider``; exclusive plugins (``kind: exclusive``, e.g. memory +providers) activate via ``.provider``. The general plugin loader +skips both kinds, so a ``plugins.enabled``/``disabled`` entry for them is dead +config. Before this fix, ``hermes plugins enable gemini`` printed a green +success message while changing nothing that any loader would ever read. +""" +import pytest + + +@pytest.fixture +def fake_plugins(tmp_path, monkeypatch): + """A discovery view with one plugin per kind, backed by real manifests.""" + import hermes_cli.plugins_cmd as pc + + entries = [] + for name, kind in [ + ("gemini", "model-provider"), + ("honcho", "exclusive"), + ("nemo_relay", "standalone"), + ]: + d = tmp_path / name + d.mkdir() + (d / "plugin.yaml").write_text( + f"name: {name}\nkind: {kind}\nversion: 1.0.0\n", encoding="utf-8" + ) + key = f"model-providers/{name}" if kind == "model-provider" else name + entries.append((name, "1.0.0", "desc", "bundled", str(d), key)) + + monkeypatch.setattr(pc, "_discover_all_plugins", lambda: entries) + # config writes must land in a scratch HERMES_HOME + home = tmp_path / "home" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + import hermes_cli.config as cfg + + cfg._LOAD_CONFIG_CACHE.clear() + cfg._RAW_CONFIG_CACHE.clear() + return pc + + +def _enabled_disabled(pc): + return pc._get_enabled_set(), pc._get_disabled_set() + + +def test_enable_model_provider_is_refused_with_hint(fake_plugins, capsys): + pc = fake_plugins + pc.cmd_enable("gemini") + out = capsys.readouterr().out + assert "model provider" in out + assert "hermes model" in out + assert "Nothing was changed" in out + enabled, disabled = _enabled_disabled(pc) + assert "model-providers/gemini" not in enabled + assert "model-providers/gemini" not in disabled + + +def test_disable_model_provider_is_refused_with_hint(fake_plugins, capsys): + pc = fake_plugins + pc.cmd_disable("gemini") + out = capsys.readouterr().out + assert "model provider" in out + enabled, disabled = _enabled_disabled(pc) + assert "model-providers/gemini" not in disabled + + +def test_enable_exclusive_is_refused_with_hint(fake_plugins, capsys): + pc = fake_plugins + pc.cmd_enable("honcho") + out = capsys.readouterr().out + assert "exclusive" in out + assert "provider" in out + enabled, _ = _enabled_disabled(pc) + assert "honcho" not in enabled + + +def test_enable_standalone_still_works(fake_plugins, capsys): + pc = fake_plugins + pc.cmd_enable("nemo_relay", allow_tool_override=False) + out = capsys.readouterr().out + assert "enabled" in out + enabled, _ = _enabled_disabled(pc) + assert "nemo_relay" in enabled + + +def test_plugin_kind_defaults_to_standalone(fake_plugins): + pc = fake_plugins + assert pc._plugin_kind("no-such-key") == "standalone" diff --git a/tests/test_toolsets.py b/tests/test_toolsets.py index f9e4969b7bea..770976f2380e 100644 --- a/tests/test_toolsets.py +++ b/tests/test_toolsets.py @@ -291,3 +291,70 @@ def test_all_alias_accepts_include_registry(self): def test_registry_only_toolset_static_view_is_empty(self): assert resolve_toolset("__definitely_not_a_real_toolset__", include_registry=False) == [] + + +class TestBundledPlatformBundles: + """Every bundled platform plugin must ship a static ``hermes-`` bundle. + + ``_get_platform_tools`` falls back to ``hermes-`` for any platform + missing from config's ``platform_toolsets``, and ``resolve_toolset`` returns + ``[]`` for an unknown name. The runtime auto-generate path only fires once + ``gateway.platform_registry`` has the platform registered, so outside the + gateway process (cron, kanban dispatch, ``hermes tools``, doctor) — and in + the explicit-config composite expansion, which skips names absent from + ``TOOLSETS`` — a bundled platform plugin without a static bundle silently + degrades to ZERO tools. LINE shipped this way (#23197): every platform + listed in ``plugins/platforms/`` must have a matching static bundle. + """ + + def test_every_bundled_platform_plugin_has_a_static_bundle(self): + import pathlib + + platforms_dir = ( + pathlib.Path(__file__).resolve().parent.parent / "plugins" / "platforms" + ) + missing = [] + for child in sorted(platforms_dir.iterdir()): + 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( + bundle, include_registry=False + ): + missing.append(bundle) + assert not missing, ( + f"Bundled platform plugins without a static toolset bundle: {missing}. " + f"Add a 'hermes-' entry to TOOLSETS in toolsets.py — without " + f"it the platform silently resolves to zero tools (see #38798 shape)." + ) + + def test_new_platform_bundles_resolve_to_core_tools(self): + for platform in ( + "line", + "google_chat", + "teams", + "irc", + "ntfy", + "photon", + "simplex", + "raft", + ): + tools = resolve_toolset(f"hermes-{platform}", include_registry=False) + for core_tool in ("terminal", "read_file", "memory", "web_search", "todo"): + assert core_tool in tools, ( + f"hermes-{platform} is missing core tool {core_tool!r}" + ) + + def test_gateway_composite_includes_all_platform_bundles(self): + gateway_includes = set(TOOLSETS["hermes-gateway"]["includes"]) + for platform in ( + "line", + "google_chat", + "teams", + "irc", + "ntfy", + "photon", + "simplex", + "raft", + ): + assert f"hermes-{platform}" in gateway_includes diff --git a/toolsets.py b/toolsets.py index 03e64fdba4c0..d8a03689254a 100644 --- a/toolsets.py +++ b/toolsets.py @@ -574,10 +574,64 @@ "includes": [] }, + # Bundled platform plugins added without a matching ``hermes-`` + # bundle used to silently resolve to ZERO tools: ``_get_platform_tools`` + # falls back to ``hermes-`` for any platform missing from + # ``platform_toolsets``, and ``resolve_toolset`` returns ``[]`` for an + # unknown name (#38798 shape). Every bundled platform plugin below now has + # an explicit core bundle, mirroring hermes-telegram/hermes-signal. + "hermes-line": { + "description": "LINE bot toolset - personal messaging via LINE Messaging API (full access)", + "tools": _HERMES_CORE_TOOLS, + "includes": [] + }, + + "hermes-google_chat": { + "description": "Google Chat bot toolset - workspace messaging (full access)", + "tools": _HERMES_CORE_TOOLS, + "includes": [] + }, + + "hermes-teams": { + "description": "Microsoft Teams bot toolset - workspace messaging (full access)", + "tools": _HERMES_CORE_TOOLS, + "includes": [] + }, + + "hermes-irc": { + "description": "IRC bot toolset - classic internet relay chat (full access)", + "tools": _HERMES_CORE_TOOLS, + "includes": [] + }, + + "hermes-ntfy": { + "description": "ntfy toolset - push notification channel (full access)", + "tools": _HERMES_CORE_TOOLS, + "includes": [] + }, + + "hermes-photon": { + "description": "Photon toolset - Lemmy client messaging (full access)", + "tools": _HERMES_CORE_TOOLS, + "includes": [] + }, + + "hermes-simplex": { + "description": "SimpleX Chat bot toolset - private messaging (full access)", + "tools": _HERMES_CORE_TOOLS, + "includes": [] + }, + + "hermes-raft": { + "description": "Raft toolset - Hermes-to-Hermes relay platform (full access)", + "tools": _HERMES_CORE_TOOLS, + "includes": [] + }, + "hermes-gateway": { "description": "Gateway toolset - union of all messaging platform tools", "tools": [], - "includes": ["hermes-telegram", "hermes-discord", "hermes-whatsapp", "hermes-slack", "hermes-signal", "hermes-bluebubbles", "hermes-homeassistant", "hermes-email", "hermes-sms", "hermes-mattermost", "hermes-matrix", "hermes-dingtalk", "hermes-feishu", "hermes-wecom", "hermes-wecom-callback", "hermes-weixin", "hermes-qqbot", "hermes-webhook", "hermes-yuanbao"] + "includes": ["hermes-telegram", "hermes-discord", "hermes-whatsapp", "hermes-slack", "hermes-signal", "hermes-bluebubbles", "hermes-homeassistant", "hermes-email", "hermes-sms", "hermes-mattermost", "hermes-matrix", "hermes-dingtalk", "hermes-feishu", "hermes-wecom", "hermes-wecom-callback", "hermes-weixin", "hermes-qqbot", "hermes-webhook", "hermes-yuanbao", "hermes-line", "hermes-google_chat", "hermes-teams", "hermes-irc", "hermes-ntfy", "hermes-photon", "hermes-simplex", "hermes-raft"] } }