From 336c3b13aa16f2686695d92ae98321a3ead65d42 Mon Sep 17 00:00:00 2001 From: loes5050 <88768844+loes5050@users.noreply.github.com> Date: Sat, 18 Jul 2026 12:44:41 +0400 Subject: [PATCH] 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). --- hermes_cli/doctor.py | 126 ++++++ tests/hermes_cli/test_config_validation.py | 488 ++++++++++++--------- tests/hermes_cli/test_doctor.py | 168 +++++++ 3 files changed, 575 insertions(+), 207 deletions(-) diff --git a/hermes_cli/doctor.py b/hermes_cli/doctor.py index 6e94996c2740..490683f38d7f 100644 --- a/hermes_cli/doctor.py +++ b/hermes_cli/doctor.py @@ -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: diff --git a/tests/hermes_cli/test_config_validation.py b/tests/hermes_cli/test_config_validation.py index f4423f5d7d1d..3223228e859a 100644 --- a/tests/hermes_cli/test_config_validation.py +++ b/tests/hermes_cli/test_config_validation.py @@ -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) diff --git a/tests/hermes_cli/test_doctor.py b/tests/hermes_cli/test_doctor.py index ba4e3846e8f3..8e7575b86b2e 100644 --- a/tests/hermes_cli/test_doctor.py +++ b/tests/hermes_cli/test_doctor.py @@ -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