diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 03570bceba49..30a8ae4643c3 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -8097,6 +8097,20 @@ def edit_config(): subprocess.run([editor, str(config_path)]) +def _default_value_for_key(dotted_key: str): + """Return the leaf value declared for *dotted_key* in ``DEFAULT_CONFIG``. + + Unknown keys and non-leaf paths return ``None`` so they retain the legacy + best-effort coercion used by ``config set``. + """ + node = DEFAULT_CONFIG + for part in dotted_key.split("."): + if not isinstance(node, dict) or part not in node: + return None + node = node[part] + return node if not isinstance(node, dict) else None + + def set_config_value(key: str, value: str): """Set a configuration value.""" if is_managed(): @@ -8154,16 +8168,21 @@ def set_config_value(key: str, value: str): # _set_nested which preserves list-typed nodes; before #17876 the # inline navigation here silently overwrote lists with dicts. - # Convert value to appropriate type - if value.lower() in {'true', 'yes', 'on'}: - value = True - elif value.lower() in {'false', 'no', 'off'}: - value = False - elif value.isdigit(): - value = int(value) - elif value.replace('.', '', 1).isdigit(): - value = float(value) + # Preserve values for string-typed settings. In particular, enum members + # such as approvals.mode="off" must not become YAML booleans. Unknown keys + # retain the historical best-effort coercion behavior. + coerced_value: Any = value + if not isinstance(_default_value_for_key(key), str): + if value.lower() in {'true', 'yes', 'on'}: + coerced_value = True + elif value.lower() in {'false', 'no', 'off'}: + coerced_value = False + elif value.isdigit(): + coerced_value = int(value) + elif value.replace('.', '', 1).isdigit(): + coerced_value = float(value) + value = coerced_value _set_nested(user_config, key, value) # Normalize the api_base → base_url alias at set-time too (issue #8919), # so a fresh `hermes config set model.api_base ...` lands on the canonical diff --git a/tests/hermes_cli/test_set_config_value.py b/tests/hermes_cli/test_set_config_value.py index 2405b84a3814..6d31d558f157 100644 --- a/tests/hermes_cli/test_set_config_value.py +++ b/tests/hermes_cli/test_set_config_value.py @@ -249,6 +249,46 @@ class TestListNavigation: assert allowlist[1] == {"name": "bob", "role": "admin"} +# --------------------------------------------------------------------------- +# String-typed config values — regression tests for #47515 +# --------------------------------------------------------------------------- + +class TestStringTypedConfigValues: + @pytest.mark.parametrize("value", ["off", "on", "yes", "no", "true", "false", "01"]) + def test_string_typed_values_are_not_coerced(self, _isolated_hermes_home, value): + """Values stay strings when DEFAULT_CONFIG declares the leaf as a string.""" + set_config_value("approvals.mode", value) + + import yaml + saved = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert saved["approvals"]["mode"] == value + assert isinstance(saved["approvals"]["mode"], str) + + @pytest.mark.parametrize("key, value, expected", [ + ("terminal.persistent_shell", "off", False), + ("approvals.timeout", "30", 30), + ]) + def test_non_string_defaults_keep_existing_coercion( + self, _isolated_hermes_home, key, value, expected + ): + set_config_value(key, value) + + import yaml + saved = yaml.safe_load(_read_config(_isolated_hermes_home)) + node = saved + for part in key.split("."): + node = node[part] + assert node == expected + assert type(node) is type(expected) + + def test_unknown_keys_keep_existing_coercion(self, _isolated_hermes_home): + set_config_value("custom.enabled", "off") + + import yaml + saved = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert saved["custom"]["enabled"] is False + + # --------------------------------------------------------------------------- # Secret redaction in display output (issue #50245) # ---------------------------------------------------------------------------