From 8f4fbb84b1e4b58c4935de4d55972ad57968ef8a Mon Sep 17 00:00:00 2001 From: hoddukzoa12 Date: Fri, 24 Jul 2026 10:29:10 +0900 Subject: [PATCH] fix(config): warn on legacy restart drain setting --- hermes_cli/config.py | 43 +++++++++++++++++++++++++++ tests/hermes_cli/test_config.py | 52 +++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 912472f86b61..e23a102766d5 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -4866,6 +4866,38 @@ def clear_model_endpoint_credentials( _MISSING = object() +_LEGACY_RESTART_DRAIN_TIMEOUT = 180 +_RESTART_DRAIN_TIMEOUT_CONFIG_KEY = "agent.restart_drain_timeout" + + +def _is_exact_legacy_restart_drain_timeout(value: Any) -> bool: + """Return True only for the persisted numeric former default.""" + return ( + not isinstance(value, bool) + and isinstance(value, (int, float)) + and value == _LEGACY_RESTART_DRAIN_TIMEOUT + ) + + +def get_legacy_config_warnings(raw_config: Optional[Dict[str, Any]] = None) -> List[str]: + """Return read-only warnings for persisted values that shadow safer defaults.""" + if raw_config is None: + raw_config = read_raw_config() + + restart_drain_timeout = _get_nested(raw_config, _RESTART_DRAIN_TIMEOUT_CONFIG_KEY) + if not _is_exact_legacy_restart_drain_timeout(restart_drain_timeout): + return [] + + current_default = DEFAULT_CONFIG["agent"]["restart_drain_timeout"] + return [ + ( + f"{_RESTART_DRAIN_TIMEOUT_CONFIG_KEY} is explicitly set to " + f"{_LEGACY_RESTART_DRAIN_TIMEOUT}, which matches the former default; " + f"the current default is {current_default}.\n" + "Long drain windows can delay or interfere with supervised gateway restarts.\n" + "Run: hermes config set agent.restart_drain_timeout 0" + ) + ] def _get_nested(config, dotted_key: str): @@ -9254,6 +9286,17 @@ def config_command(args): print() print(color(f" {len(missing_config)} new config option(s) available", Colors.YELLOW)) print(" Run 'hermes config migrate' to add them") + + legacy_warnings = get_legacy_config_warnings() + if legacy_warnings: + print() + for warning in legacy_warnings: + lines = warning.splitlines() + if not lines: + continue + print(color(f" ⚠️ {lines[0]}", Colors.YELLOW)) + for line in lines[1:]: + print(color(f" {line}", Colors.YELLOW)) print() diff --git a/tests/hermes_cli/test_config.py b/tests/hermes_cli/test_config.py index 196483f4941e..6a36b4274c49 100644 --- a/tests/hermes_cli/test_config.py +++ b/tests/hermes_cli/test_config.py @@ -1,5 +1,6 @@ """Tests for hermes_cli configuration management.""" +import argparse import os from pathlib import Path from unittest.mock import patch @@ -10,6 +11,7 @@ from hermes_cli.config import ( DEFAULT_CONFIG, check_config_version, + config_command, get_hermes_home, ensure_hermes_home, get_compatible_custom_providers, @@ -129,6 +131,56 @@ def test_legacy_root_level_max_turns_migrates_to_agent_config(self, tmp_path): assert "max_turns" not in config +class TestConfigCheckLegacyRestartDrainWarning: + """``hermes config check`` warns only for the persisted former default.""" + + def _run_check(self, capsys) -> str: + args = argparse.Namespace(config_command="check") + config_command(args) + return capsys.readouterr().out + + @pytest.mark.parametrize("value_yaml", ["180", "180.0"]) + def test_warns_for_explicit_legacy_restart_drain_timeout( + self, tmp_path, capsys, value_yaml + ): + config_path = tmp_path / "config.yaml" + config_path.write_text(f"agent:\n restart_drain_timeout: {value_yaml}\n") + before = config_path.read_text() + + with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}): + output = self._run_check(capsys) + + assert "agent.restart_drain_timeout" in output + assert "former default" in output + assert "current default is 0" in output + assert "Long drain windows can delay or interfere with supervised gateway restarts" in output + assert "hermes config set agent.restart_drain_timeout 0" in output + assert config_path.read_text() == before + + @pytest.mark.parametrize( + "config_yaml", + [ + "", + "agent:\n gateway_timeout: 1800\n", + "agent:\n restart_drain_timeout: 0\n", + "agent:\n restart_drain_timeout: 30\n", + "agent:\n restart_drain_timeout: '180'\n", + "agent:\n restart_drain_timeout: true\n", + "agent:\n restart_drain_timeout: null\n", + ], + ) + def test_no_legacy_warning_without_explicit_180(self, tmp_path, capsys, config_yaml): + if config_yaml: + (tmp_path / "config.yaml").write_text(config_yaml) + + with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}): + output = self._run_check(capsys) + + assert "agent.restart_drain_timeout" not in output + assert "former default" not in output + assert "supervised gateway restarts" not in output + + class TestLoadConfigParseFailure: """A YAML parse failure must NOT silently fall back to defaults.