From 4f67c33383204d28f4bf85de7db04f7265a73686 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 19 Jul 2026 00:52:07 -0700 Subject: [PATCH] fix(config): whitelist real non-DEFAULT_CONFIG roots + normalize test line endings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups on the salvaged unknown-root-key warning from PR #67345: - Add image_gen, video_gen, plugins, smart_model_routing, platform_toolsets, session_reset, multiplex_profiles, profile_routes, platforms, require_mention, unauthorized_dm_behavior, and signal to _EXTRA_KNOWN_ROOT_KEYS — all are read from the raw user YAML (gateway, registries, plugin CLI) or written by our own setup wizard, but absent from DEFAULT_CONFIG. Without this, doctor would warn on configs Hermes itself wrote. - Convert tests/hermes_cli/test_config_validation.py back to LF line endings (the PR's rewrite introduced CRLF). --- hermes_cli/config.py | 14 + tests/hermes_cli/test_config_validation.py | 562 ++++++++++----------- 2 files changed, 295 insertions(+), 281 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index c01c45d6b883..94f0843ef6b9 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -5502,6 +5502,20 @@ _EXTRA_KNOWN_ROOT_KEYS = { "custom_providers", # legacy list form; modern equivalent is providers: {} "fallback_model", # optional single dict or chain list; omitted when disabled "mcp_servers", # MCP server definitions written by setup/tools flows + # Roots read from the raw user YAML (or written by our own flows) that are + # intentionally absent from DEFAULT_CONFIG: + "image_gen", # image-generation provider config (agent/image_gen_registry.py) + "video_gen", # video-generation provider config (agent/video_gen_registry.py) + "plugins", # plugin enable/disable lists (hermes_cli/plugins_cmd.py) + "smart_model_routing", # written by the setup wizard (hermes_cli/setup.py) + "platform_toolsets", # written by the setup wizard (hermes_cli/setup.py) + "session_reset", # top-level form read by gateway/config.py + setup + "multiplex_profiles", # top-level form accepted alongside gateway.multiplex_profiles + "profile_routes", # top-level form accepted alongside gateway.profile_routes + "platforms", # top-level per-platform map merged by gateway/config.py + "require_mention", # top-level convenience form honored by the gateway (#3979) + "unauthorized_dm_behavior", # top-level form read by gateway/config.py + "signal", # Signal settings bridged to env vars by gateway/config.py } _KNOWN_ROOT_KEYS = frozenset(DEFAULT_CONFIG.keys()) | _EXTRA_KNOWN_ROOT_KEYS diff --git a/tests/hermes_cli/test_config_validation.py b/tests/hermes_cli/test_config_validation.py index 3223228e859a..bd00a0d301e5 100644 --- a/tests/hermes_cli/test_config_validation.py +++ b/tests/hermes_cli/test_config_validation.py @@ -1,281 +1,281 @@ -"""Tests for config.yaml structure validation (validate_config_structure).""" - - -from hermes_cli.config import ( - DEFAULT_CONFIG, - _EXTRA_KNOWN_ROOT_KEYS, - _KNOWN_ROOT_KEYS, - validate_config_structure, - ConfigIssue, -) - - -class TestCustomProvidersValidation: - """custom_providers must be a YAML list, not a dict.""" - - def test_dict_instead_of_list(self): - """The exact Discord user scenario — custom_providers as flat dict.""" - issues = validate_config_structure({ - "custom_providers": { - "name": "Generativelanguage.googleapis.com", - "base_url": "https://generativelanguage.googleapis.com/v1beta", - "api_key": "xxx", - "model": "models/gemini-2.5-flash", - "rate_limit_delay": 2.0, - "fallback_model": { - "provider": "openrouter", - "model": "qwen/qwen3.6-plus:free", - }, - }, - "fallback_providers": [], - }) - errors = [i for i in issues if i.severity == "error"] - assert any("dict" in i.message and "list" in i.message for i in errors), ( - "Should detect custom_providers as dict instead of list" - ) - - def test_dict_detects_misplaced_fields(self): - """When custom_providers is a dict, detect fields that look misplaced.""" - issues = validate_config_structure({ - "custom_providers": { - "name": "test", - "base_url": "https://example.com", - "api_key": "xxx", - }, - }) - warnings = [i for i in issues if i.severity == "warning"] - # Should flag base_url, api_key as looking like custom_providers entry fields - misplaced = [i for i in warnings if "custom_providers entry fields" in i.message] - assert len(misplaced) == 1 - - def test_dict_detects_nested_fallback(self): - """When fallback_model gets swallowed into custom_providers dict.""" - issues = validate_config_structure({ - "custom_providers": { - "name": "test", - "fallback_model": {"provider": "openrouter", "model": "test"}, - }, - }) - errors = [i for i in issues if i.severity == "error"] - assert any("fallback_model" in i.message and "inside" in i.message for i in errors) - - def test_valid_list_no_issues(self): - """Properly formatted custom_providers should produce no issues.""" - issues = validate_config_structure({ - "custom_providers": [ - {"name": "gemini", "base_url": "https://example.com/v1"}, - ], - "model": {"provider": "custom", "default": "test"}, - }) - assert len(issues) == 0 - - def test_list_entry_missing_name(self): - """List entry without name should warn.""" - issues = validate_config_structure({ - "custom_providers": [{"base_url": "https://example.com/v1"}], - "model": {"provider": "custom"}, - }) - assert any("missing 'name'" in i.message for i in issues) - - def test_list_entry_missing_base_url(self): - """List entry without base_url should warn.""" - issues = validate_config_structure({ - "custom_providers": [{"name": "test"}], - "model": {"provider": "custom"}, - }) - assert any("missing 'base_url'" in i.message for i in issues) - - def test_list_entry_not_dict(self): - """Non-dict list entries should warn.""" - issues = validate_config_structure({ - "custom_providers": ["not-a-dict"], - "model": {"provider": "custom"}, - }) - assert any("not a dict" in i.message for i in issues) - - def test_none_custom_providers_no_issues(self): - """No custom_providers at all should be fine.""" - issues = validate_config_structure({ - "model": {"provider": "openrouter"}, - }) - assert len(issues) == 0 - - -class TestFallbackModelValidation: - """fallback_model should be a top-level dict with provider + model.""" - - def test_missing_provider(self): - issues = validate_config_structure({ - "fallback_model": {"model": "anthropic/claude-sonnet-4"}, - }) - assert any("missing 'provider'" in i.message for i in issues) - - def test_missing_model(self): - issues = validate_config_structure({ - "fallback_model": {"provider": "openrouter"}, - }) - assert any("missing 'model'" in i.message for i in issues) - - def test_valid_fallback(self): - issues = validate_config_structure({ - "fallback_model": { - "provider": "openrouter", - "model": "anthropic/claude-sonnet-4", - }, - }) - # Only fallback-related issues should be absent - fb_issues = [i for i in issues if "fallback" in i.message.lower()] - assert len(fb_issues) == 0 - - def test_non_dict_fallback(self): - issues = validate_config_structure({ - "fallback_model": "openrouter:anthropic/claude-sonnet-4", - }) - assert any("should be a dict" in i.message for i in issues) - - def test_empty_fallback_dict_no_issues(self): - """Empty fallback_model dict means disabled — no warnings needed.""" - issues = validate_config_structure({ - "fallback_model": {}, - }) - fb_issues = [i for i in issues if "fallback" in i.message.lower()] - assert len(fb_issues) == 0 - - def test_valid_fallback_list(self): - """List-form fallback_model (chain) should validate when every entry has provider+model.""" - issues = validate_config_structure({ - "fallback_model": [ - {"provider": "openrouter", "model": "anthropic/claude-sonnet-4"}, - {"provider": "anthropic", "model": "claude-sonnet-4-6"}, - ], - }) - fb_issues = [i for i in issues if "fallback" in i.message.lower()] - assert len(fb_issues) == 0 - - def test_fallback_list_entry_missing_provider(self): - issues = validate_config_structure({ - "fallback_model": [ - {"provider": "openrouter", "model": "anthropic/claude-sonnet-4"}, - {"model": "claude-sonnet-4-6"}, - ], - }) - assert any("fallback_model[1]" in i.message and "provider" in i.message for i in issues) - - def test_fallback_list_entry_missing_model(self): - issues = validate_config_structure({ - "fallback_model": [ - {"provider": "openrouter"}, - ], - }) - assert any("fallback_model[0]" in i.message and "model" in i.message for i in issues) - - def test_fallback_list_entry_not_a_dict(self): - issues = validate_config_structure({ - "fallback_model": ["openrouter:anthropic/claude-sonnet-4"], - }) - assert any("fallback_model[0]" in i.message and "should be a dict" in i.message for i in issues) - - -class TestMissingModelSection: - """Warn when custom_providers exists but model section is missing.""" - - def test_custom_providers_without_model(self): - issues = validate_config_structure({ - "custom_providers": [ - {"name": "test", "base_url": "https://example.com/v1"}, - ], - }) - assert any("no 'model' section" in i.message for i in issues) - - def test_custom_providers_with_model(self): - issues = validate_config_structure({ - "custom_providers": [ - {"name": "test", "base_url": "https://example.com/v1"}, - ], - "model": {"provider": "custom", "default": "test-model"}, - }) - # Should not warn about missing model section - assert not any("no 'model' section" in i.message for i in issues) - - -class TestConfigIssueDataclass: - """ConfigIssue should be a proper dataclass.""" - - def test_fields(self): - issue = ConfigIssue(severity="error", message="test msg", hint="test hint") - assert issue.severity == "error" - assert issue.message == "test msg" - assert issue.hint == "test hint" - - def test_equality(self): - a = ConfigIssue("error", "msg", "hint") - b = ConfigIssue("error", "msg", "hint") - assert a == b - - -class TestUnknownTopLevelKeys: - """Unknown top-level keys should warn (not error); known roots stay silent.""" - - def test_typo_top_level_key_warns_with_key_name(self): - """A typo like skillz: must surface as a warning naming the key.""" - issues = validate_config_structure({ - "model": {"provider": "openrouter"}, - "skillz": {"enabled": True}, - "secrity": {"redact": True}, - }) - warnings = [i for i in issues if i.severity == "warning"] - unknown = [i for i in warnings if "Unknown top-level config key" in i.message] - messages = " ".join(i.message for i in unknown) - assert "skillz" in messages - assert "secrity" in messages - assert all(i.severity == "warning" for i in unknown) - assert not any(i.severity == "error" for i in unknown) - - def test_all_default_config_roots_accepted_without_unknown_warning(self): - """Every DEFAULT_CONFIG root (and legacy extras) must not warn as unknown.""" - config = {key: {} if isinstance(DEFAULT_CONFIG.get(key), dict) else DEFAULT_CONFIG.get(key) - for key in DEFAULT_CONFIG} - for key in _EXTRA_KNOWN_ROOT_KEYS: - if key == "custom_providers": - config[key] = [{"name": "x", "base_url": "https://example.com"}] - config.setdefault("model", {"provider": "custom", "default": "m"}) - elif key == "fallback_model": - config[key] = {"provider": "openrouter", "model": "test"} - else: - config[key] = {} - issues = validate_config_structure(config) - unknown = [i for i in issues if "Unknown top-level config key" in i.message] - assert unknown == [], f"Unexpected unknown-key warnings: {[i.message for i in unknown]}" - - def test_known_root_keys_derived_from_default_config(self): - """_KNOWN_ROOT_KEYS must be DEFAULT_CONFIG.keys() plus extras — single source of truth.""" - assert set(DEFAULT_CONFIG.keys()).issubset(_KNOWN_ROOT_KEYS) - assert _EXTRA_KNOWN_ROOT_KEYS.issubset(_KNOWN_ROOT_KEYS) - assert _KNOWN_ROOT_KEYS == frozenset(DEFAULT_CONFIG.keys()) | _EXTRA_KNOWN_ROOT_KEYS - - def test_provider_like_unknown_root_keeps_misplaced_message(self): - """Preserve existing base_url/api_key root-level guidance (not generic unknown).""" - issues = validate_config_structure({ - "base_url": "https://example.com/v1", - "api_key": "secret", - }) - misplaced = [ - i for i in issues - if i.severity == "warning" and "looks misplaced" in i.message - ] - generic_unknown = [ - i for i in issues - if "Unknown top-level config key" in i.message - ] - assert any("base_url" in i.message for i in misplaced) - assert any("api_key" in i.message for i in misplaced) - assert generic_unknown == [] - - def test_private_underscore_keys_not_flagged(self): - """Internal keys starting with _ remain ignored (except known defaults).""" - issues = validate_config_structure({ - "_internal_scratch": True, - "model": {"provider": "openrouter"}, - }) - assert not any("Unknown top-level" in i.message for i in issues) - assert not any("_internal_scratch" in i.message for i in issues) +"""Tests for config.yaml structure validation (validate_config_structure).""" + + +from hermes_cli.config import ( + DEFAULT_CONFIG, + _EXTRA_KNOWN_ROOT_KEYS, + _KNOWN_ROOT_KEYS, + validate_config_structure, + ConfigIssue, +) + + +class TestCustomProvidersValidation: + """custom_providers must be a YAML list, not a dict.""" + + def test_dict_instead_of_list(self): + """The exact Discord user scenario — custom_providers as flat dict.""" + issues = validate_config_structure({ + "custom_providers": { + "name": "Generativelanguage.googleapis.com", + "base_url": "https://generativelanguage.googleapis.com/v1beta", + "api_key": "xxx", + "model": "models/gemini-2.5-flash", + "rate_limit_delay": 2.0, + "fallback_model": { + "provider": "openrouter", + "model": "qwen/qwen3.6-plus:free", + }, + }, + "fallback_providers": [], + }) + errors = [i for i in issues if i.severity == "error"] + assert any("dict" in i.message and "list" in i.message for i in errors), ( + "Should detect custom_providers as dict instead of list" + ) + + def test_dict_detects_misplaced_fields(self): + """When custom_providers is a dict, detect fields that look misplaced.""" + issues = validate_config_structure({ + "custom_providers": { + "name": "test", + "base_url": "https://example.com", + "api_key": "xxx", + }, + }) + warnings = [i for i in issues if i.severity == "warning"] + # Should flag base_url, api_key as looking like custom_providers entry fields + misplaced = [i for i in warnings if "custom_providers entry fields" in i.message] + assert len(misplaced) == 1 + + def test_dict_detects_nested_fallback(self): + """When fallback_model gets swallowed into custom_providers dict.""" + issues = validate_config_structure({ + "custom_providers": { + "name": "test", + "fallback_model": {"provider": "openrouter", "model": "test"}, + }, + }) + errors = [i for i in issues if i.severity == "error"] + assert any("fallback_model" in i.message and "inside" in i.message for i in errors) + + def test_valid_list_no_issues(self): + """Properly formatted custom_providers should produce no issues.""" + issues = validate_config_structure({ + "custom_providers": [ + {"name": "gemini", "base_url": "https://example.com/v1"}, + ], + "model": {"provider": "custom", "default": "test"}, + }) + assert len(issues) == 0 + + def test_list_entry_missing_name(self): + """List entry without name should warn.""" + issues = validate_config_structure({ + "custom_providers": [{"base_url": "https://example.com/v1"}], + "model": {"provider": "custom"}, + }) + assert any("missing 'name'" in i.message for i in issues) + + def test_list_entry_missing_base_url(self): + """List entry without base_url should warn.""" + issues = validate_config_structure({ + "custom_providers": [{"name": "test"}], + "model": {"provider": "custom"}, + }) + assert any("missing 'base_url'" in i.message for i in issues) + + def test_list_entry_not_dict(self): + """Non-dict list entries should warn.""" + issues = validate_config_structure({ + "custom_providers": ["not-a-dict"], + "model": {"provider": "custom"}, + }) + assert any("not a dict" in i.message for i in issues) + + def test_none_custom_providers_no_issues(self): + """No custom_providers at all should be fine.""" + issues = validate_config_structure({ + "model": {"provider": "openrouter"}, + }) + assert len(issues) == 0 + + +class TestFallbackModelValidation: + """fallback_model should be a top-level dict with provider + model.""" + + def test_missing_provider(self): + issues = validate_config_structure({ + "fallback_model": {"model": "anthropic/claude-sonnet-4"}, + }) + assert any("missing 'provider'" in i.message for i in issues) + + def test_missing_model(self): + issues = validate_config_structure({ + "fallback_model": {"provider": "openrouter"}, + }) + assert any("missing 'model'" in i.message for i in issues) + + def test_valid_fallback(self): + issues = validate_config_structure({ + "fallback_model": { + "provider": "openrouter", + "model": "anthropic/claude-sonnet-4", + }, + }) + # Only fallback-related issues should be absent + fb_issues = [i for i in issues if "fallback" in i.message.lower()] + assert len(fb_issues) == 0 + + def test_non_dict_fallback(self): + issues = validate_config_structure({ + "fallback_model": "openrouter:anthropic/claude-sonnet-4", + }) + assert any("should be a dict" in i.message for i in issues) + + def test_empty_fallback_dict_no_issues(self): + """Empty fallback_model dict means disabled — no warnings needed.""" + issues = validate_config_structure({ + "fallback_model": {}, + }) + fb_issues = [i for i in issues if "fallback" in i.message.lower()] + assert len(fb_issues) == 0 + + def test_valid_fallback_list(self): + """List-form fallback_model (chain) should validate when every entry has provider+model.""" + issues = validate_config_structure({ + "fallback_model": [ + {"provider": "openrouter", "model": "anthropic/claude-sonnet-4"}, + {"provider": "anthropic", "model": "claude-sonnet-4-6"}, + ], + }) + fb_issues = [i for i in issues if "fallback" in i.message.lower()] + assert len(fb_issues) == 0 + + def test_fallback_list_entry_missing_provider(self): + issues = validate_config_structure({ + "fallback_model": [ + {"provider": "openrouter", "model": "anthropic/claude-sonnet-4"}, + {"model": "claude-sonnet-4-6"}, + ], + }) + assert any("fallback_model[1]" in i.message and "provider" in i.message for i in issues) + + def test_fallback_list_entry_missing_model(self): + issues = validate_config_structure({ + "fallback_model": [ + {"provider": "openrouter"}, + ], + }) + assert any("fallback_model[0]" in i.message and "model" in i.message for i in issues) + + def test_fallback_list_entry_not_a_dict(self): + issues = validate_config_structure({ + "fallback_model": ["openrouter:anthropic/claude-sonnet-4"], + }) + assert any("fallback_model[0]" in i.message and "should be a dict" in i.message for i in issues) + + +class TestMissingModelSection: + """Warn when custom_providers exists but model section is missing.""" + + def test_custom_providers_without_model(self): + issues = validate_config_structure({ + "custom_providers": [ + {"name": "test", "base_url": "https://example.com/v1"}, + ], + }) + assert any("no 'model' section" in i.message for i in issues) + + def test_custom_providers_with_model(self): + issues = validate_config_structure({ + "custom_providers": [ + {"name": "test", "base_url": "https://example.com/v1"}, + ], + "model": {"provider": "custom", "default": "test-model"}, + }) + # Should not warn about missing model section + assert not any("no 'model' section" in i.message for i in issues) + + +class TestConfigIssueDataclass: + """ConfigIssue should be a proper dataclass.""" + + def test_fields(self): + issue = ConfigIssue(severity="error", message="test msg", hint="test hint") + assert issue.severity == "error" + assert issue.message == "test msg" + assert issue.hint == "test hint" + + def test_equality(self): + a = ConfigIssue("error", "msg", "hint") + b = ConfigIssue("error", "msg", "hint") + assert a == b + + +class TestUnknownTopLevelKeys: + """Unknown top-level keys should warn (not error); known roots stay silent.""" + + def test_typo_top_level_key_warns_with_key_name(self): + """A typo like skillz: must surface as a warning naming the key.""" + issues = validate_config_structure({ + "model": {"provider": "openrouter"}, + "skillz": {"enabled": True}, + "secrity": {"redact": True}, + }) + warnings = [i for i in issues if i.severity == "warning"] + unknown = [i for i in warnings if "Unknown top-level config key" in i.message] + messages = " ".join(i.message for i in unknown) + assert "skillz" in messages + assert "secrity" in messages + assert all(i.severity == "warning" for i in unknown) + assert not any(i.severity == "error" for i in unknown) + + def test_all_default_config_roots_accepted_without_unknown_warning(self): + """Every DEFAULT_CONFIG root (and legacy extras) must not warn as unknown.""" + config = {key: {} if isinstance(DEFAULT_CONFIG.get(key), dict) else DEFAULT_CONFIG.get(key) + for key in DEFAULT_CONFIG} + for key in _EXTRA_KNOWN_ROOT_KEYS: + if key == "custom_providers": + config[key] = [{"name": "x", "base_url": "https://example.com"}] + config.setdefault("model", {"provider": "custom", "default": "m"}) + elif key == "fallback_model": + config[key] = {"provider": "openrouter", "model": "test"} + else: + config[key] = {} + issues = validate_config_structure(config) + unknown = [i for i in issues if "Unknown top-level config key" in i.message] + assert unknown == [], f"Unexpected unknown-key warnings: {[i.message for i in unknown]}" + + def test_known_root_keys_derived_from_default_config(self): + """_KNOWN_ROOT_KEYS must be DEFAULT_CONFIG.keys() plus extras — single source of truth.""" + assert set(DEFAULT_CONFIG.keys()).issubset(_KNOWN_ROOT_KEYS) + assert _EXTRA_KNOWN_ROOT_KEYS.issubset(_KNOWN_ROOT_KEYS) + assert _KNOWN_ROOT_KEYS == frozenset(DEFAULT_CONFIG.keys()) | _EXTRA_KNOWN_ROOT_KEYS + + def test_provider_like_unknown_root_keeps_misplaced_message(self): + """Preserve existing base_url/api_key root-level guidance (not generic unknown).""" + issues = validate_config_structure({ + "base_url": "https://example.com/v1", + "api_key": "secret", + }) + misplaced = [ + i for i in issues + if i.severity == "warning" and "looks misplaced" in i.message + ] + generic_unknown = [ + i for i in issues + if "Unknown top-level config key" in i.message + ] + assert any("base_url" in i.message for i in misplaced) + assert any("api_key" in i.message for i in misplaced) + assert generic_unknown == [] + + def test_private_underscore_keys_not_flagged(self): + """Internal keys starting with _ remain ignored (except known defaults).""" + issues = validate_config_structure({ + "_internal_scratch": True, + "model": {"provider": "openrouter"}, + }) + assert not any("Unknown top-level" in i.message for i in issues) + assert not any("_internal_scratch" in i.message for i in issues)