mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-21 16:18:55 +00:00
feat(config): warn on unknown top-level keys + report deprecated keys/env in doctor
Two config-hygiene improvements (warning-only, non-blocking): 1) Unknown top-level config keys now surface a warning naming the key (known roots derived from DEFAULT_CONFIG.keys() as single source of truth) so typos like 'skillz:'/'secrity:' are no longer silently ignored. Provider.* unknown-key behavior preserved. 2) hermes doctor reports deprecated/legacy config keys (display.tool_progress_overrides, delegation.max_async_children, compression.summary_*) and legacy env vars (HERMES_TOOL_PROGRESS*, TERMINAL_CWD, QQ_HOME_CHANNEL*) with their modern replacements, as non-failing warnings. No auto-delete/migrate. Tests: config_validation + doctor suites green (100 passed).
This commit is contained in:
loes5050
Teknium
parent
8ddc05b801
commit
336c3b13aa
3 changed files with 575 additions and 207 deletions
|
|
@ -201,6 +201,100 @@ def _fail_and_issue(text: str, detail: str, fix: str, issues: list[str]) -> None
|
|||
issues.append(fix)
|
||||
|
||||
|
||||
# Deprecated / legacy config keys still read for back-compat. Doctor surfaces
|
||||
# them as non-failing warnings with the modern replacement — it does not
|
||||
# auto-migrate or delete (migrations live in config.py version steps).
|
||||
_DEPRECATED_CONFIG_KEYS: tuple[tuple[str, str, str], ...] = (
|
||||
# (section, key, replacement)
|
||||
("display", "tool_progress_overrides", "display.platforms"),
|
||||
("delegation", "max_async_children", "delegation.max_concurrent_children"),
|
||||
)
|
||||
|
||||
# compression.summary_* → auxiliary.compression (model/provider/base_url)
|
||||
_DEPRECATED_COMPRESSION_SUMMARY_KEYS: tuple[str, ...] = (
|
||||
"summary_model",
|
||||
"summary_provider",
|
||||
"summary_base_url",
|
||||
)
|
||||
|
||||
# Deprecated env vars (checked in the .env file, not process env, so config→env
|
||||
# bridges like terminal.cwd → TERMINAL_CWD do not false-positive).
|
||||
_DEPRECATED_ENV_VARS: tuple[tuple[str, str], ...] = (
|
||||
("HERMES_TOOL_PROGRESS", "display.tool_progress in config.yaml"),
|
||||
("HERMES_TOOL_PROGRESS_MODE", "display.tool_progress in config.yaml"),
|
||||
("TERMINAL_CWD", "terminal.cwd in config.yaml"),
|
||||
("MESSAGING_CWD", "terminal.cwd in config.yaml"),
|
||||
("QQ_HOME_CHANNEL", "QQBOT_HOME_CHANNEL"),
|
||||
("QQ_HOME_CHANNEL_NAME", "QQBOT_HOME_CHANNEL_NAME"),
|
||||
)
|
||||
|
||||
|
||||
def collect_deprecated_config_keys(raw_config: dict | None) -> list[tuple[str, str]]:
|
||||
"""Return ``(legacy_path, replacement)`` for deprecated keys present in *raw_config*.
|
||||
|
||||
Only keys that appear in the on-disk YAML are reported (raw file load, not
|
||||
merged defaults). Empty containers still count — presence of the legacy
|
||||
key is the signal that the user should migrate.
|
||||
"""
|
||||
findings: list[tuple[str, str]] = []
|
||||
if not isinstance(raw_config, dict):
|
||||
return findings
|
||||
|
||||
for section, key, replacement in _DEPRECATED_CONFIG_KEYS:
|
||||
section_val = raw_config.get(section)
|
||||
if isinstance(section_val, dict) and key in section_val:
|
||||
findings.append((f"{section}.{key}", replacement))
|
||||
|
||||
compression = raw_config.get("compression")
|
||||
if isinstance(compression, dict):
|
||||
for key in _DEPRECATED_COMPRESSION_SUMMARY_KEYS:
|
||||
if key in compression:
|
||||
findings.append((f"compression.{key}", "auxiliary.compression"))
|
||||
|
||||
return findings
|
||||
|
||||
|
||||
def collect_deprecated_env_vars(env_map: dict | None) -> list[tuple[str, str]]:
|
||||
"""Return ``(legacy_env, replacement)`` for deprecated vars present in *env_map*.
|
||||
|
||||
*env_map* should come from the on-disk ``.env`` (e.g. ``load_env()``), not
|
||||
``os.environ``, so bridged runtime vars do not trigger false positives.
|
||||
"""
|
||||
findings: list[tuple[str, str]] = []
|
||||
if not isinstance(env_map, dict):
|
||||
return findings
|
||||
for name, replacement in _DEPRECATED_ENV_VARS:
|
||||
val = env_map.get(name)
|
||||
if val is not None and str(val).strip() != "":
|
||||
findings.append((name, replacement))
|
||||
return findings
|
||||
|
||||
|
||||
def report_deprecated_config_and_env(
|
||||
raw_config: dict | None = None,
|
||||
env_map: dict | None = None,
|
||||
) -> list[tuple[str, str]]:
|
||||
"""Emit non-failing doctor warnings for deprecated config keys and env vars.
|
||||
|
||||
Returns the list of ``(legacy, replacement)`` findings that were reported
|
||||
(empty when nothing deprecated is present). Does not mutate config/env and
|
||||
does not append to the blocking ``issues`` list.
|
||||
"""
|
||||
findings = collect_deprecated_config_keys(raw_config)
|
||||
findings.extend(collect_deprecated_env_vars(env_map))
|
||||
if not findings:
|
||||
check_ok("No deprecated config keys or env vars")
|
||||
return findings
|
||||
|
||||
for legacy, replacement in findings:
|
||||
check_warn(
|
||||
f"Deprecated: {legacy}",
|
||||
f"(use {replacement} instead)",
|
||||
)
|
||||
check_info(f"Replace {legacy} → {replacement} (warn-only; not auto-migrated here)")
|
||||
return findings
|
||||
|
||||
|
||||
def _enabled_cli_toolsets_for_doctor() -> set[str] | None:
|
||||
"""Return toolsets enabled for the CLI, or None if config resolution fails."""
|
||||
try:
|
||||
|
|
@ -1059,6 +1153,25 @@ def run_doctor(args):
|
|||
except Exception:
|
||||
pass
|
||||
|
||||
# Surface deprecated/legacy config keys and env vars (warn-only).
|
||||
# Migrations may still live in config.py version steps; doctor does
|
||||
# not auto-delete here — only tells the user the modern replacement.
|
||||
try:
|
||||
import yaml as _yaml_depr
|
||||
from hermes_cli.config import load_env as _load_env_depr
|
||||
|
||||
with open(config_path, encoding="utf-8") as _f_depr:
|
||||
_raw_for_depr = _yaml_depr.safe_load(_f_depr) or {}
|
||||
# Prefer the on-disk .env so bridged process env (e.g. TERMINAL_CWD
|
||||
# from terminal.cwd) does not false-positive.
|
||||
try:
|
||||
_env_for_depr = _load_env_depr()
|
||||
except Exception:
|
||||
_env_for_depr = {}
|
||||
report_deprecated_config_and_env(_raw_for_depr, _env_for_depr)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# Validate config structure (catches malformed custom_providers, etc.)
|
||||
try:
|
||||
from hermes_cli.config import validate_config_structure
|
||||
|
|
@ -1077,6 +1190,19 @@ def run_doctor(args):
|
|||
except Exception:
|
||||
pass
|
||||
|
||||
if not config_path.exists():
|
||||
# No config.yaml — still surface deprecated env vars from .env.
|
||||
try:
|
||||
from hermes_cli.config import load_env as _load_env_depr
|
||||
|
||||
try:
|
||||
_env_for_depr = _load_env_depr()
|
||||
except Exception:
|
||||
_env_for_depr = {}
|
||||
report_deprecated_config_and_env({}, _env_for_depr)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
_section("xAI Model Retirement (May 15, 2026)")
|
||||
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -1,207 +1,281 @@
|
|||
"""Tests for config.yaml structure validation (validate_config_structure)."""
|
||||
|
||||
|
||||
from hermes_cli.config import 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
|
||||
"""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)
|
||||
|
|
|
|||
|
|
@ -1488,3 +1488,171 @@ def test_npm_audit_fix_hint_avoids_crashing_workspace_flag(monkeypatch, tmp_path
|
|||
assert "build-time tooling" in out
|
||||
assert "known npm bug" in out
|
||||
assert "lockfile bump" in out
|
||||
|
||||
|
||||
class TestDoctorDeprecatedConfigAndEnv:
|
||||
"""Doctor must surface deprecated/legacy config keys and env vars with
|
||||
modern replacements as non-failing warnings — without auto-migrating.
|
||||
"""
|
||||
|
||||
def test_collect_deprecated_config_keys_flags_legacy(self):
|
||||
raw = {
|
||||
"display": {"tool_progress_overrides": {"telegram": "all"}},
|
||||
"delegation": {"max_async_children": 5, "max_concurrent_children": 3},
|
||||
"compression": {"summary_model": "gpt-4o-mini", "enabled": True},
|
||||
}
|
||||
findings = doctor_mod.collect_deprecated_config_keys(raw)
|
||||
paths = {legacy for legacy, _ in findings}
|
||||
assert "display.tool_progress_overrides" in paths
|
||||
assert "delegation.max_async_children" in paths
|
||||
assert "compression.summary_model" in paths
|
||||
by_key = dict(findings)
|
||||
assert by_key["display.tool_progress_overrides"] == "display.platforms"
|
||||
assert by_key["delegation.max_async_children"] == (
|
||||
"delegation.max_concurrent_children"
|
||||
)
|
||||
assert by_key["compression.summary_model"] == "auxiliary.compression"
|
||||
|
||||
def test_collect_deprecated_config_keys_clean(self):
|
||||
raw = {
|
||||
"display": {"platforms": {"telegram": {"tool_progress": "all"}}},
|
||||
"delegation": {"max_concurrent_children": 3},
|
||||
"compression": {"enabled": True},
|
||||
}
|
||||
assert doctor_mod.collect_deprecated_config_keys(raw) == []
|
||||
|
||||
def test_collect_deprecated_env_vars(self):
|
||||
env = {
|
||||
"HERMES_TOOL_PROGRESS": "true",
|
||||
"TERMINAL_CWD": "/tmp/proj",
|
||||
"QQ_HOME_CHANNEL": "12345",
|
||||
"OPENAI_API_KEY": "sk-test", # not deprecated
|
||||
}
|
||||
findings = doctor_mod.collect_deprecated_env_vars(env)
|
||||
names = {n for n, _ in findings}
|
||||
assert "HERMES_TOOL_PROGRESS" in names
|
||||
assert "TERMINAL_CWD" in names
|
||||
assert "QQ_HOME_CHANNEL" in names
|
||||
assert "OPENAI_API_KEY" not in names
|
||||
by_name = dict(findings)
|
||||
assert "display.tool_progress" in by_name["HERMES_TOOL_PROGRESS"]
|
||||
assert "terminal.cwd" in by_name["TERMINAL_CWD"]
|
||||
assert by_name["QQ_HOME_CHANNEL"] == "QQBOT_HOME_CHANNEL"
|
||||
|
||||
def test_collect_deprecated_env_vars_ignores_empty(self):
|
||||
assert doctor_mod.collect_deprecated_env_vars({"TERMINAL_CWD": " "}) == []
|
||||
assert doctor_mod.collect_deprecated_env_vars({}) == []
|
||||
assert doctor_mod.collect_deprecated_env_vars(None) == []
|
||||
|
||||
def _run_doctor_with_config(self, monkeypatch, tmp_path, *, config_yaml: str, env_text: str = ""):
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir(parents=True)
|
||||
(hermes_home / "config.yaml").write_text(config_yaml, encoding="utf-8")
|
||||
env_body = env_text if env_text else "OPENAI_API_KEY=sk-test\n"
|
||||
(hermes_home / ".env").write_text(env_body, encoding="utf-8")
|
||||
|
||||
monkeypatch.setattr(doctor_mod, "HERMES_HOME", hermes_home)
|
||||
monkeypatch.setattr(doctor_mod, "get_hermes_home", lambda: hermes_home)
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
# Clear process-level legacy env so tests only see the on-disk .env.
|
||||
for k in (
|
||||
"HERMES_TOOL_PROGRESS",
|
||||
"HERMES_TOOL_PROGRESS_MODE",
|
||||
"TERMINAL_CWD",
|
||||
"MESSAGING_CWD",
|
||||
"QQ_HOME_CHANNEL",
|
||||
"QQ_HOME_CHANNEL_NAME",
|
||||
):
|
||||
monkeypatch.delenv(k, raising=False)
|
||||
|
||||
fake_model_tools = types.SimpleNamespace(
|
||||
check_tool_availability=lambda *a, **kw: (_ for _ in ()).throw(SystemExit(0)),
|
||||
TOOLSET_REQUIREMENTS={},
|
||||
)
|
||||
monkeypatch.setitem(sys.modules, "model_tools", fake_model_tools)
|
||||
|
||||
buf = io.StringIO()
|
||||
with contextlib.redirect_stdout(buf), pytest.raises(SystemExit):
|
||||
doctor_mod.run_doctor(Namespace(fix=False))
|
||||
return buf.getvalue(), hermes_home
|
||||
|
||||
def test_doctor_warns_on_tool_progress_overrides_and_max_async_children(
|
||||
self, monkeypatch, tmp_path
|
||||
):
|
||||
cfg = """\
|
||||
display:
|
||||
tool_progress_overrides:
|
||||
telegram: all
|
||||
delegation:
|
||||
max_async_children: 8
|
||||
max_concurrent_children: 3
|
||||
"""
|
||||
out, hermes_home = self._run_doctor_with_config(monkeypatch, tmp_path, config_yaml=cfg)
|
||||
assert "Deprecated: display.tool_progress_overrides" in out
|
||||
assert "display.platforms" in out
|
||||
assert "Deprecated: delegation.max_async_children" in out
|
||||
assert "max_concurrent_children" in out
|
||||
# Warn-only: must not mutate config.
|
||||
on_disk = (hermes_home / "config.yaml").read_text(encoding="utf-8")
|
||||
assert "tool_progress_overrides" in on_disk
|
||||
assert "max_async_children" in on_disk
|
||||
|
||||
def test_doctor_warns_on_compression_summary_and_legacy_env(
|
||||
self, monkeypatch, tmp_path
|
||||
):
|
||||
cfg = """\
|
||||
compression:
|
||||
summary_model: gpt-4o-mini
|
||||
summary_provider: openai
|
||||
"""
|
||||
env = (
|
||||
"OPENAI_API_KEY=sk-test\n"
|
||||
"HERMES_TOOL_PROGRESS=true\n"
|
||||
"TERMINAL_CWD=/old/path\n"
|
||||
"QQ_HOME_CHANNEL=999\n"
|
||||
)
|
||||
out, _ = self._run_doctor_with_config(
|
||||
monkeypatch, tmp_path, config_yaml=cfg, env_text=env
|
||||
)
|
||||
assert "Deprecated: compression.summary_model" in out
|
||||
assert "auxiliary.compression" in out
|
||||
assert "Deprecated: HERMES_TOOL_PROGRESS" in out
|
||||
assert "display.tool_progress" in out
|
||||
assert "Deprecated: TERMINAL_CWD" in out
|
||||
assert "terminal.cwd" in out
|
||||
assert "Deprecated: QQ_HOME_CHANNEL" in out
|
||||
assert "QQBOT_HOME_CHANNEL" in out
|
||||
|
||||
def test_doctor_clean_config_has_no_deprecated_warning(self, monkeypatch, tmp_path):
|
||||
cfg = """\
|
||||
display:
|
||||
platforms:
|
||||
telegram:
|
||||
tool_progress: all
|
||||
delegation:
|
||||
max_concurrent_children: 3
|
||||
compression:
|
||||
enabled: true
|
||||
terminal:
|
||||
cwd: /project
|
||||
"""
|
||||
out, _ = self._run_doctor_with_config(monkeypatch, tmp_path, config_yaml=cfg)
|
||||
assert "Deprecated: display.tool_progress_overrides" not in out
|
||||
assert "Deprecated: delegation.max_async_children" not in out
|
||||
assert "Deprecated: compression.summary_model" not in out
|
||||
assert "Deprecated: HERMES_TOOL_PROGRESS" not in out
|
||||
assert "Deprecated: TERMINAL_CWD" not in out
|
||||
assert "Deprecated: QQ_HOME_CHANNEL" not in out
|
||||
assert "No deprecated config keys or env vars" in out
|
||||
|
||||
def test_report_does_not_count_as_blocking_issue(self, monkeypatch, tmp_path, capsys):
|
||||
"""report_deprecated_config_and_env is warn-only — no issues list mutation."""
|
||||
findings = doctor_mod.report_deprecated_config_and_env(
|
||||
{"delegation": {"max_async_children": 2}},
|
||||
{"HERMES_TOOL_PROGRESS_MODE": "verbose"},
|
||||
)
|
||||
out = capsys.readouterr().out
|
||||
assert len(findings) == 2
|
||||
assert "Deprecated: delegation.max_async_children" in out
|
||||
assert "Deprecated: HERMES_TOOL_PROGRESS_MODE" in out
|
||||
assert "⚠" in out or "Deprecated" in out
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue