diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 2b638305d5..ae64562eb1 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -5202,6 +5202,41 @@ def _validate_config_key(key: str) -> tuple[bool, Optional[str]]: return True, None +def _looks_structured_value(value: str) -> bool: + """Return True when *value* plausibly encodes a YAML/JSON list or mapping. + + Used by :func:`set_config_value` to decide whether to attempt a + ``yaml.safe_load`` structured parse. Deliberately conservative so plain + scalars are never mangled: + + - Flow style: the value starts with ``[`` or ``{`` (JSON is a YAML + subset, so both ``'["a","b"]'`` and ``'{a: 1}'`` qualify). + - Block style: the value spans multiple lines AND at least one line is + shaped like a YAML sequence item (``- item``) or mapping entry + (``key: value``). + + A bare leading ``-`` is NOT a trigger on its own: ``-5``, ``--flag`` and + other dash-prefixed single-line scalars must remain strings. + """ + stripped = value.lstrip() + if stripped[:1] in ('[', '{'): + return True + if '\n' not in value: + return False + for line in value.splitlines(): + item = line.strip() + if item == '-' or item.startswith('- '): + return True + # ``key: value`` / ``key:`` mapping-entry shape (no whitespace in the + # key, colon followed by a space or end-of-line). + head, sep, _rest = item.partition(': ') + if sep and head and ' ' not in head and not head.startswith('#'): + return True + if item.endswith(':') and ' ' not in item[:-1] and item[:-1]: + return True + return False + + def set_config_value(key: str, value: str, force: bool = False): """Set a configuration value. @@ -5292,16 +5327,21 @@ def set_config_value(key: str, value: str, force: bool = False): coerced_value = int(value) elif value.replace('.', '', 1).isdigit(): coerced_value = float(value) - elif value.lstrip()[:1] in ('[', '{'): + elif _looks_structured_value(value): # List/mapping literals -- e.g. # hermes config set platform_toolsets.line '["file","web"]' + # or a multi-line YAML block: + # hermes config set custom_providers '- name: foo + # base_url: https://...' # Without this, such values were stored as a raw STRING, and every # reader that gates on isinstance(..., list) (``_get_platform_tools``, # ``_get_enabled_set``, ...) silently ignored them and fell back to # its default -- the setting looked saved but never took effect. # Folded INSIDE the string-typed guard so a genuinely string-typed # setting whose value merely starts with '[' or '{' is left intact - # (preserves the guard added in e4ea0a0ed). + # (preserves the guard added in e4ea0a0ed). The trigger is + # deliberately conservative (see _looks_structured_value): plain + # scalars like '-5' or '--flag' never reach the YAML parser. try: parsed = yaml.safe_load(value) if isinstance(parsed, (list, dict)): diff --git a/tests/hermes_cli/test_config_set_list_values.py b/tests/hermes_cli/test_config_set_list_values.py index 1dff829007..80c15a6afd 100644 --- a/tests/hermes_cli/test_config_set_list_values.py +++ b/tests/hermes_cli/test_config_set_list_values.py @@ -69,3 +69,93 @@ def test_scalar_values_unaffected(user_home): assert raw["agent"]["max_turns"] == 300 assert raw["display"]["compact"] is True assert raw["tts"]["provider"] == "edge" + + +# --------------------------------------------------------------------------- +# Consolidated-cluster additions: multi-line YAML blocks, string-typed-key +# guard, conservative trigger, and load_config round-trip. +# --------------------------------------------------------------------------- + + +def test_multiline_yaml_list_is_parsed(user_home): + """A multi-line YAML block list must be stored as a real list.""" + from hermes_cli.config import set_config_value, read_raw_config + + set_config_value( + "custom_providers", + "- name: foo\n base_url: https://foo.example/v1\n" + "- name: bar\n base_url: https://bar.example/v1", + ) + raw = read_raw_config() + assert raw["custom_providers"] == [ + {"name": "foo", "base_url": "https://foo.example/v1"}, + {"name": "bar", "base_url": "https://bar.example/v1"}, + ] + + +def test_multiline_yaml_mapping_is_parsed(user_home): + from hermes_cli.config import set_config_value, read_raw_config + + set_config_value( + "display.tool_progress_overrides", + "terminal: off\nbrowser: on", + ) + raw = read_raw_config() + assert raw["display"]["tool_progress_overrides"] == { + "terminal": False, + "browser": True, + } + + +def test_string_typed_key_bracket_value_stays_string(user_home): + """Keys whose DEFAULT_CONFIG type is str must never be coerced — + even when the value looks like a list literal.""" + from hermes_cli.config import set_config_value, read_raw_config + + set_config_value("approvals.mode", "[off]") + raw = read_raw_config() + assert raw["approvals"]["mode"] == "[off]" + assert isinstance(raw["approvals"]["mode"], str) + + +def test_string_typed_key_negative_number_stays_string(user_home): + """'-5' for a string-typed key must remain the string '-5'.""" + from hermes_cli.config import set_config_value, read_raw_config + + set_config_value("approvals.mode", "-5") + raw = read_raw_config() + assert raw["approvals"]["mode"] == "-5" + + +def test_dash_prefixed_scalar_not_treated_as_list(user_home): + """Single-line dash-prefixed scalars ('-5', '--flag') must stay strings + for non-string-typed keys too — the over-broad leading '-' trigger from + #88066 is deliberately avoided.""" + from hermes_cli.config import set_config_value, read_raw_config + + set_config_value("weird.flag", "--verbose") + raw = read_raw_config() + assert raw["weird"]["flag"] == "--verbose" + + +def test_plain_scalar_that_parses_to_scalar_kept_as_string(user_home): + """If yaml.safe_load of a structured-looking value yields a plain scalar, + keep the original string.""" + from hermes_cli.config import set_config_value, read_raw_config + + # '{}' parses to an empty dict — that IS structured, so check a value + # that starts with '[' but parses to a scalar is impossible in YAML; + # instead use a multi-line value whose lines don't match list/dict shape. + set_config_value("some.note", "line one\nline two without yaml shape") + raw = read_raw_config() + assert raw["some"]["note"] == "line one\nline two without yaml shape" + + +def test_round_trip_through_load_config(user_home): + """Structured values written by set_config_value must survive + load_config as real lists/dicts.""" + from hermes_cli.config import set_config_value, load_config + + set_config_value("platform_toolsets.line", '["clarify", "file", "web"]') + cfg = load_config() + assert cfg["platform_toolsets"]["line"] == ["clarify", "file", "web"]