mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-21 16:18:55 +00:00
fix(config): remove unknown-top-level-key warning — top-level keys bridge to env (#67924)
The 'Unknown top-level config key' warning (f5bacee27) assumed a closed-world allowlist of valid roots, but top-level scalars in config.yaml are deliberately bridged into os.environ (gateway/run.py, hermes send) so skills and external apps Hermes drives can read arbitrary env-style keys (DISCORD_HOME_CHANNEL, MY_APP_TOKEN, ...). An allowlist can never enumerate those — two widening follow-ups (7c2ece53c,3c7217706) already proved the whack-a-mole. Drop the generic warning entirely; keep the targeted provider-like-field misplacement hint (base_url/api_key at root).
This commit is contained in:
Teknium
GitHub
parent
1c3a48965b
commit
9bda6438d4
2 changed files with 26 additions and 72 deletions
|
|
@ -5699,28 +5699,21 @@ def validate_config_structure(config: Optional[Dict[str, Any]] = None) -> List["
|
|||
" base_url: https://...",
|
||||
))
|
||||
|
||||
# ── Unknown / misplaced root-level keys ──────────────────────────────
|
||||
# Typos like skillz:/secrity: were previously silent (only provider-like
|
||||
# fields were flagged). Warn on any unknown top-level key so config
|
||||
# hygiene surfaces without breaking startup.
|
||||
# ── Root-level keys that look misplaced ──────────────────────────────
|
||||
# Only provider-like fields (base_url, api_key, …) are flagged. Arbitrary
|
||||
# unknown top-level keys are deliberately NOT warned about: top-level
|
||||
# scalars are bridged into os.environ (gateway/run.py, hermes send) so
|
||||
# users can feed skills and external apps env-style keys from config.yaml
|
||||
# — a closed-world allowlist can never enumerate those.
|
||||
for key in config:
|
||||
if key.startswith("_"):
|
||||
continue
|
||||
if key in _KNOWN_ROOT_KEYS:
|
||||
continue
|
||||
if key in _CUSTOM_PROVIDER_LIKE_FIELDS:
|
||||
if key not in _KNOWN_ROOT_KEYS and key in _CUSTOM_PROVIDER_LIKE_FIELDS:
|
||||
issues.append(ConfigIssue(
|
||||
"warning",
|
||||
f"Root-level key '{key}' looks misplaced — should it be under 'model:' or inside a 'custom_providers' entry?",
|
||||
f"Move '{key}' under the appropriate section",
|
||||
))
|
||||
else:
|
||||
issues.append(ConfigIssue(
|
||||
"warning",
|
||||
f"Unknown top-level config key '{key}' — it will be ignored",
|
||||
"Check for typos, or remove the key if it is not a supported config root. "
|
||||
"Run 'hermes doctor' for more detail.",
|
||||
))
|
||||
|
||||
return issues
|
||||
|
||||
|
|
|
|||
|
|
@ -214,38 +214,27 @@ class TestConfigIssueDataclass:
|
|||
|
||||
|
||||
class TestUnknownTopLevelKeys:
|
||||
"""Unknown top-level keys should warn (not error); known roots stay silent."""
|
||||
"""Arbitrary top-level keys must NOT warn — they are bridged to os.environ.
|
||||
|
||||
def test_typo_top_level_key_warns_with_key_name(self):
|
||||
"""A typo like skillz: must surface as a warning naming the key."""
|
||||
Top-level scalars in config.yaml are forwarded into the environment
|
||||
(gateway/run.py, hermes send) so users can feed skills and external apps
|
||||
env-style keys like DISCORD_HOME_CHANNEL or MY_APP_TOKEN. A closed-world
|
||||
allowlist can never enumerate those, so no "Unknown top-level config key"
|
||||
warning may exist.
|
||||
"""
|
||||
|
||||
def test_arbitrary_top_level_keys_stay_silent(self):
|
||||
"""Env-style and custom keys must produce no unknown-key warnings."""
|
||||
issues = validate_config_structure({
|
||||
"model": {"provider": "openrouter"},
|
||||
"DISCORD_HOME_CHANNEL": "12345",
|
||||
"TELEGRAM_HOME_CHANNEL": "-100987",
|
||||
"DISCORD_ALLOW_ALL_USERS": True,
|
||||
"MY_CUSTOM_SKILL_VAR": "hello",
|
||||
"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]}"
|
||||
assert not any("Unknown top-level config key" in i.message for i in issues)
|
||||
assert issues == []
|
||||
|
||||
def test_known_root_keys_derived_from_default_config(self):
|
||||
"""_KNOWN_ROOT_KEYS must be DEFAULT_CONFIG.keys() plus extras — single source of truth."""
|
||||
|
|
@ -253,30 +242,8 @@ class TestUnknownTopLevelKeys:
|
|||
assert _EXTRA_KNOWN_ROOT_KEYS.issubset(_KNOWN_ROOT_KEYS)
|
||||
assert _KNOWN_ROOT_KEYS == frozenset(DEFAULT_CONFIG.keys()) | _EXTRA_KNOWN_ROOT_KEYS
|
||||
|
||||
def test_hermes_written_roots_not_flagged_as_unknown(self):
|
||||
"""Roots Hermes itself writes/reads must not warn as unknown (#67397)."""
|
||||
issues = validate_config_structure({
|
||||
"model": {"provider": "openrouter"},
|
||||
"known_plugin_toolsets": {"cli": ["spotify"]},
|
||||
"group_sessions_per_user": True,
|
||||
"thread_sessions_per_user": False,
|
||||
"stt_echo_transcripts": True,
|
||||
"reset_triggers": ["/new"],
|
||||
"always_log_local": True,
|
||||
"filter_silence_narration": True,
|
||||
})
|
||||
unknown = [i for i in issues if "Unknown top-level config key" in i.message]
|
||||
messages = " ".join(i.message for i in unknown)
|
||||
assert "known_plugin_toolsets" not in messages
|
||||
assert "group_sessions_per_user" not in messages
|
||||
assert "thread_sessions_per_user" not in messages
|
||||
assert "stt_echo_transcripts" not in messages
|
||||
assert "reset_triggers" not in messages
|
||||
assert "always_log_local" not in messages
|
||||
assert "filter_silence_narration" not in messages
|
||||
|
||||
def test_provider_like_unknown_root_keeps_misplaced_message(self):
|
||||
"""Preserve existing base_url/api_key root-level guidance (not generic unknown)."""
|
||||
"""Preserve existing base_url/api_key root-level guidance."""
|
||||
issues = validate_config_structure({
|
||||
"base_url": "https://example.com/v1",
|
||||
"api_key": "secret",
|
||||
|
|
@ -285,19 +252,13 @@ class TestUnknownTopLevelKeys:
|
|||
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)."""
|
||||
"""Internal keys starting with _ remain ignored."""
|
||||
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)
|
||||
assert issues == []
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue