fix(config): decode JSON array literals into YAML lists on config set

hermes_cli/config.py

@@ -4646,6 +4646,78 @@ def _default_value_for_key(dotted_key: str):
     return node if not isinstance(node, dict) else None
 
 
+def _is_mapping_section_key(dotted_key: str) -> bool:
+    """True when *dotted_key* names a mapping section in DEFAULT_CONFIG.
+
+    ``_default_value_for_key`` returns None for dict nodes (the leaf check
+    requires a non-dict), so top-level mapping sections like ``terminal`` or
+    ``agent`` are invisible to it.  A key is mapping-typed when its first
+    segment's default is a dict (e.g. ``terminal``, ``agent.compression``) or
+    the existing config holds a dict at that path.
+    """
+    segments = dotted_key.split(".")
+    node = DEFAULT_CONFIG
+    for part in segments:
+        if not isinstance(node, dict) or part not in node:
+            return False
+        node = node[part]
+    return isinstance(node, dict)
+
+
+def _decode_json_value(text: str):
+    """Decode a JSON literal (array or object) for ``config set``, or None.
+
+    ``hermes config set <key> '[...]'`` / ``'{...}'`` has no native CLI syntax
+    for structured values, so users pass JSON.  Returns the parsed value when
+    *text* is a JSON array or object; None for scalars (plain strings, ``123``,
+    ``true``) which keep their historical stringification.
+    """
+    try:
+        parsed = json.loads(text)
+    except (ValueError, TypeError):
+        return None
+    return parsed if isinstance(parsed, (list, dict)) else None
+
+
+def _looks_like_allowlist_field(field: str) -> bool:
+    """True when *field* is an allowlist-shaped platform key.
+
+    Allowlist fields are consumed as sets by ``_coerce_allow_set`` (and the
+    telegram env bridge) and are the list-typed platform settings users set
+    with a JSON array.  Matching is by shape (``*_allowed_*`` / ``*_allow_from``)
+    plus the known allowlist names, so it stays in sync with the readers even
+    as platform configs gain fields.
+    """
+    return (
+        "_allow_from" in field
+        or "_allowed_" in field
+        or field in ("allow_from", "allowed_chats", "allowed_topics", "allowed_rooms", "allowed_channels")
+    )
+
+
+def _is_allowlist_platform_field(dotted_key: str) -> bool:
+    """True when *dotted_key* names an allowlist field of a platform config.
+
+    Covers both documented forms:
+    - nested: ``gateway.platforms.<name>.<field>`` / ``platforms.<name>.<field>``
+      (open-dict platform configs; ``gateway.platforms.telegram`` alone is the
+      platform mapping, not a field)
+    - top-level: ``telegram.<field>``, ``discord.<field>``, etc. (bridged into
+      platform extras by ``gateway/config.py``)
+
+    Only allowlist-shaped fields decode; other platform fields (``reactions``,
+    ``streaming``) keep their historical stringification.
+    """
+    segments = dotted_key.split(".")
+    if any(seg in _PLATFORM_CONTAINER_KEYS and idx + 2 < len(segments) for idx, seg in enumerate(segments)):
+        # ``...platforms.<name>.<field>`` — at least one field below name.
+        return _looks_like_allowlist_field(segments[-1])
+    if segments[0] in _SCHEMA_DEFINED_DICT_KEYS and len(segments) > 1:
+        # ``telegram.<field>`` — top-level platform block field.
+        return _looks_like_allowlist_field(segments[1])
+    return False
+
+
 # Known top-level config keys that intentionally accept arbitrary user-supplied
 # child keys ("dictionary-shaped" config: the schema declares the dict but the
 # user populates its keys). Schema validation accepts ANY path below these
@@ -4901,7 +4973,9 @@ def set_config_value(key: str, value: str, force: bool = False):
     # 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):
+    default_value = _default_value_for_key(key)
+    existing_value = _get_nested(user_config, key)
+    if not isinstance(default_value, str):
         if value.lower() in {'true', 'yes', 'on'}:
             coerced_value = True
         elif value.lower() in {'false', 'no', 'off'}:
@@ -4910,6 +4984,35 @@ def set_config_value(key: str, value: str, force: bool = False):
             coerced_value = int(value)
         elif value.replace('.', '', 1).isdigit():
             coerced_value = float(value)
+        # List-typed settings (allowlists like
+        # ``gateway.platforms.telegram.group_allowed_chats``, ``allow_from``,
+        # ``allowed_topics``) have no native CLI syntax, so users pass a JSON
+        # array literal.  Decode it into a real YAML sequence instead of
+        # writing the stringified literal — the readers (_coerce_allow_set and
+        # the telegram env bridge) split scalars on commas, so a quoted
+        # '["-5488240624"]' silently matches nothing (#76457).
+        #
+        # A key is list-typed when its schema default is a list, the existing
+        # user config holds a list at that path, or the key names an allowlist
+        # platform field (``gateway.platforms.<name>.group_allowed_chats`` /
+        # ``telegram.group_allowed_chats`` — open-dict extras with no
+        # DEFAULT_CONFIG entry).  Unknown keys never match and keep their
+        # historical stringification.
+        elif isinstance(default_value, list) or isinstance(existing_value, list) or _is_allowlist_platform_field(key):
+            # List-typed keys accept only JSON arrays — a JSON object is not a
+            # valid list value.
+            decoded = _decode_json_value(value)
+            if isinstance(decoded, list):
+                coerced_value = decoded
+        else:
+            # JSON object literal → YAML mapping, per the spec's ``{`` trigger.
+            # Only for mapping-typed keys (schema default is a mapping section,
+            # or the existing config holds a dict at that path) AND only when
+            # the decoded value is actually a dict — a JSON array on a
+            # mapping-only key stays a string (GottZ triage, PR #76470).
+            _mapping_decoded = _decode_json_value(value)
+            if isinstance(_mapping_decoded, dict) and (_is_mapping_section_key(key) or isinstance(existing_value, dict)):
+                coerced_value = _mapping_decoded
 
     value = coerced_value
     # Normalize a scalar ``model`` key before writing sub-keys so that

tests/hermes_cli/test_set_config_value.py

@@ -723,3 +723,135 @@ def test_unset_config_value_refuses_broken_yaml(self, _isolated_hermes_home, cap
         assert "Cannot parse" in captured.out or "Cannot parse" in captured.err
         raw = _read_config(_isolated_hermes_home)
         assert raw == self.BROKEN_CONFIG
+
+
+class TestJsonListValues:
+    """hermes config set must decode JSON array literals into real YAML lists.
+
+    Allowlist keys (``group_allowed_chats``, ``allow_from``, ``allowed_topics``)
+    have no native CLI list syntax, so users pass a JSON array.  Before #76457
+    the literal was written as a quoted scalar string; readers that split
+    scalars on commas then matched nothing, silently breaking authorization
+    allowlists.  Decode only when the key is list-typed (schema default is a
+    list, the existing config holds a list, or the key names an allowlist
+    platform field); all other values keep their historical stringification.
+    """
+
+    def test_json_array_written_as_yaml_list(self, _isolated_hermes_home):
+        set_config_value("gateway.platforms.telegram.group_allowed_chats", '["-5488240624"]')
+
+        import yaml
+        reloaded = yaml.safe_load(_read_config(_isolated_hermes_home))
+        value = reloaded["gateway"]["platforms"]["telegram"]["group_allowed_chats"]
+        assert isinstance(value, list)
+        assert value == ["-5488240624"]
+
+    def test_plain_scalar_stays_a_string(self, _isolated_hermes_home):
+        set_config_value("gateway.platforms.telegram.group_allowed_chats", "-5488240624")
+
+        import yaml
+        reloaded = yaml.safe_load(_read_config(_isolated_hermes_home))
+        assert reloaded["gateway"]["platforms"]["telegram"]["group_allowed_chats"] == "-5488240624"
+
+    def test_comma_string_stays_a_string(self, _isolated_hermes_home):
+        set_config_value("gateway.platforms.telegram.group_allowed_chats", "123,456")
+
+        import yaml
+        reloaded = yaml.safe_load(_read_config(_isolated_hermes_home))
+        assert reloaded["gateway"]["platforms"]["telegram"]["group_allowed_chats"] == "123,456"
+
+    def test_json_object_is_not_decoded(self, _isolated_hermes_home):
+        # A JSON object on a LIST-typed key is not a valid list value — it
+        # stays a string (the section-write guard governs mapping keys).
+        set_config_value("gateway.platforms.telegram.group_allowed_chats", '{"not": "an array"}')
+
+        import yaml
+        reloaded = yaml.safe_load(_read_config(_isolated_hermes_home))
+        assert reloaded["gateway"]["platforms"]["telegram"]["group_allowed_chats"] == '{"not": "an array"}'
+
+    def test_json_object_on_mapping_key_written_as_mapping(self, _isolated_hermes_home):
+        # The spec's ``{`` trigger: a JSON object literal on a mapping-typed
+        # key becomes a YAML mapping, not a quoted scalar.
+        set_config_value("terminal", '{"backend": "local", "cwd": "/repo"}')
+
+        import yaml
+        reloaded = yaml.safe_load(_read_config(_isolated_hermes_home))
+        assert reloaded["terminal"] == {"backend": "local", "cwd": "/repo"}
+
+    def test_json_array_on_mapping_key_stays_a_string(self, _isolated_hermes_home):
+        # A JSON array on a mapping-typed key must NOT be decoded — only JSON
+        # objects should become YAML mappings.  Without this guard, the mapping
+        # branch would accept any structured _decode_json_value result and turn
+        # an array into a YAML sequence (GottZ triage, PR #76470).
+        set_config_value("terminal", '["a", "b"]')
+
+        import yaml
+        reloaded = yaml.safe_load(_read_config(_isolated_hermes_home))
+        assert reloaded["terminal"] == '["a", "b"]'
+
+    def test_string_default_key_keeps_json_array_as_string(self, _isolated_hermes_home):
+        # String-typed defaults must not be decoded — the user asked for the
+        # literal text (e.g. a JSON document stored as a string).
+        set_config_value("telegram.allowed_chats", '["a", "b"]')
+
+        import yaml
+        reloaded = yaml.safe_load(_read_config(_isolated_hermes_home))
+        assert reloaded["telegram"]["allowed_chats"] == '["a", "b"]'
+
+    def test_unknown_key_with_json_array_stays_a_string(self, _isolated_hermes_home):
+        # Unknown keys (schema default None) are intentionally supported as
+        # arbitrary string configuration for skills/external apps.  A JSON
+        # array must stay a string, not become a YAML sequence.
+        set_config_value("custom.payload", '["a", "b"]')
+
+        import yaml
+        reloaded = yaml.safe_load(_read_config(_isolated_hermes_home))
+        assert reloaded["custom"]["payload"] == '["a", "b"]'
+
+    def test_non_list_default_with_json_array_stays_a_string(self, _isolated_hermes_home):
+        # A known key whose default is NOT a list (bool/int/float) must not be
+        # decoded either — only list-typed allowlist keys are.
+        set_config_value("telegram.reactions", '["a", "b"]')
+
+        import yaml
+        reloaded = yaml.safe_load(_read_config(_isolated_hermes_home))
+        assert reloaded["telegram"]["reactions"] == '["a", "b"]'
+
+    def test_top_level_platform_field_written_as_list(self, _isolated_hermes_home):
+        # The documented top-level form (telegram.group_allowed_chats, not
+        # nested under gateway.platforms) is bridged into platform extras by
+        # gateway/config.py and must decode JSON arrays too.
+        set_config_value("telegram.group_allowed_chats", '["-5488240624"]')
+
+        import yaml
+        reloaded = yaml.safe_load(_read_config(_isolated_hermes_home))
+        value = reloaded["telegram"]["group_allowed_chats"]
+        assert isinstance(value, list)
+        assert value == ["-5488240624"]
+
+    def test_platform_mapping_alone_is_not_decoded(self, _isolated_hermes_home):
+        # ``gateway.platforms.telegram`` is the platform mapping itself (no
+        # field below it) — a JSON array there is not an allowlist write.
+        set_config_value("gateway.platforms.telegram", '["a", "b"]')
+
+        import yaml
+        reloaded = yaml.safe_load(_read_config(_isolated_hermes_home))
+        assert reloaded["gateway"]["platforms"]["telegram"] == '["a", "b"]'
+
+    def test_loaded_through_gateway_config_is_a_list(self, _isolated_hermes_home):
+        # End-to-end: the written YAML list must survive load_gateway_config
+        # as a real list (the sweeper's suggested regression).  No fallback —
+        # a failure here must surface, not silently skip.
+        set_config_value("telegram.group_allowed_chats", '["-5488240624"]')
+
+        import gateway.config as gc
+
+        cfg = gc.load_gateway_config()
+        telegram_cfg = cfg.platforms.get(gc.Platform.TELEGRAM)
+        assert telegram_cfg is not None
+        # group_allowed_chats lands in PlatformConfig.extra (bridged from the
+        # top-level telegram block by gateway/config.py).
+        value = (telegram_cfg.extra or {}).get("group_allowed_chats")
+        assert isinstance(value, list), f"expected list, got {type(value)}: {value!r}"
+        assert value == ["-5488240624"]
+