Bool coercion corrupting string enum config values

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #63499 fix(config): preserve string-typed config values

  merged to main

  files: hermes_cli/config.py, tests/hermes_cli/test_set_config_value.py

  Show the diff

  • 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

  • tests/hermes_cli/test_set_config_value.py

    @@ -249,6 +249,46 @@ def test_deeper_nesting_through_list(self, _isolated_hermes_home):
             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)
     # ---------------------------------------------------------------------------