diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 8748db2cb136..a2f7b0fd8acd 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -4258,7 +4258,7 @@ def _ensure_hermes_home_managed(home: Path): "category": "setting", }, # HERMES_TOOL_PROGRESS and HERMES_TOOL_PROGRESS_MODE are deprecated — - # now configured via display.tool_progress in config.yaml (off|new|all|verbose|log). + # now configured via display.tool_progress in config.yaml (off|new|all|verbose). # The gateway still falls back to these env vars for backward compatibility, # so they live in _EXTRA_ENV_KEYS (known to .env sanitization/reload) but # are intentionally NOT listed here: OPTIONAL_ENV_VARS feeds user-facing @@ -4489,8 +4489,7 @@ def _normalize_custom_provider_entry( "api_mode", "transport", "model", "default_model", "models", "context_length", "rate_limit_delay", "request_timeout_seconds", "stale_timeout_seconds", - "discover_models", "extra_body", "extra_headers", - "ssl_ca_cert", "ssl_verify", + "discover_models", "extra_body", "ssl_ca_cert", "ssl_verify", } for camel, snake in _CAMEL_ALIASES.items(): if camel in entry and snake not in entry: @@ -4597,15 +4596,6 @@ def _normalize_custom_provider_entry( if isinstance(extra_body, dict): normalized["extra_body"] = dict(extra_body) - # Per-provider extra HTTP headers (proxies, gateways, custom auth). - # Values may carry credentials (e.g. CF-Access-Client-Secret) — never - # log them anywhere downstream. - extra_headers = entry.get("extra_headers") - if isinstance(extra_headers, dict) and extra_headers: - normalized["extra_headers"] = { - str(k): str(v) for k, v in extra_headers.items() if v is not None - } - ssl_ca_cert = entry.get("ssl_ca_cert") if isinstance(ssl_ca_cert, str) and ssl_ca_cert.strip(): normalized["ssl_ca_cert"] = ssl_ca_cert.strip() @@ -4643,7 +4633,6 @@ def _custom_provider_entry_to_provider_config( "rate_limit_delay", "discover_models", "extra_body", - "extra_headers", "ssl_ca_cert", "ssl_verify", ): @@ -4787,69 +4776,6 @@ def apply_custom_provider_tls_to_client_kwargs( client_kwargs["ssl_verify"] = tls["ssl_verify"] -def get_custom_provider_extra_headers( - base_url: str, - custom_providers: Optional[List[Dict[str, Any]]] = None, - config: Optional[Dict[str, Any]] = None, -) -> Dict[str, str]: - """Return ``extra_headers`` from a matching ``providers`` / ``custom_providers`` entry. - - Matches the entry whose ``base_url`` equals *base_url* (trailing-slash and - case insensitive, mirroring :func:`get_custom_provider_tls_settings`) and - returns its ``extra_headers`` dict, or ``{}`` when no entry matches or the - entry declares none. - - SECURITY: header values routinely carry credentials (Cloudflare Access - service tokens, proxy auth, custom bearer schemes). Callers must never - log the returned values. - """ - if custom_providers is None: - try: - custom_providers = get_compatible_custom_providers(config) - except Exception: - custom_providers = [] - if not base_url or not isinstance(custom_providers, list): - return {} - - target_url = (base_url or "").rstrip("/").lower() - for entry in custom_providers: - if not isinstance(entry, dict): - continue - entry_url = (entry.get("base_url") or "").rstrip("/").lower() - if not entry_url or entry_url != target_url: - continue - extra_headers = entry.get("extra_headers") - if isinstance(extra_headers, dict) and extra_headers: - return { - str(k): str(v) for k, v in extra_headers.items() if v is not None - } - return {} - return {} - - -def apply_custom_provider_extra_headers_to_client_kwargs( - client_kwargs: Dict[str, Any], - base_url: str, - custom_providers: Optional[List[Dict[str, Any]]] = None, - config: Optional[Dict[str, Any]] = None, -) -> None: - """Merge per-provider ``extra_headers`` onto OpenAI client ``default_headers``. - - Provider-specific headers win over provider/SDK defaults already present in - ``client_kwargs`` — they are the most specific configuration level. No-op - when the base_url matches no ``providers`` / ``custom_providers`` entry or - the entry declares no headers. - - SECURITY: values may carry credentials — never log them. - """ - extra_headers = get_custom_provider_extra_headers(base_url, custom_providers, config) - if not extra_headers: - return - merged = dict(client_kwargs.get("default_headers") or {}) - merged.update(extra_headers) - client_kwargs["default_headers"] = merged - - def get_custom_provider_context_length( model: str, base_url: str, @@ -7779,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/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/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"] } }