fix(skills): decode JSON-array string skills.disabled in panel read and toggle write (#7120) (#7134)

api/routes.py

@@ -817,12 +817,52 @@ def _get_disabled_skill_names_for_profile() -> set:
     return _normalize_disabled_set(skills_cfg.get("disabled"))
 
 
+def _parse_config_string_list(value) -> list:
+    """Decode a config value that may hold a JSON-array string into a list.
+
+    ``hermes config set`` (and JSON-mode editor saves) store lists as quoted
+    JSON strings (``'[\"a\",\"b\"]'`` or the Python-literal ``\"['a']\"``), so a
+    disabled list read from ``config.yaml`` can arrive as a single string
+    instead of a YAML list. Treating it as one literal name makes the Skills
+    panel show every skill as enabled and makes the toggle write a destructive
+    single-entry list (hermes-webui#7120).
+
+    Reuses ``agent.skill_utils.parse_config_string_list`` (hermes-agent #86661
+    fix) when the bundled agent source is importable, and mirrors its logic
+    otherwise so the two surfaces cannot drift. A scalar string still means one
+    name.
+    """
+    try:
+        from agent.skill_utils import parse_config_string_list
+
+        return parse_config_string_list(value)
+    except ImportError:
+        pass
+    import ast
+
+    if value is None:
+        return []
+    if isinstance(value, str):
+        stripped = value.strip()
+        if stripped.startswith("["):
+            try:
+                parsed = ast.literal_eval(stripped)
+            except (ValueError, SyntaxError):
+                parsed = None
+            if isinstance(parsed, list):
+                return [str(item) for item in parsed]
+        return [value]
+    if isinstance(value, (list, tuple, set, frozenset)):
+        return [str(item) for item in value]
+    return []
+
+
 def _normalize_disabled_set(values) -> set:
     """Normalize a YAML disabled list into a set of stripped strings."""
     if values is None:
         return set()
     if isinstance(values, str):
-        values = [values]
+        values = _parse_config_string_list(values)
     return {str(v).strip() for v in values if str(v).strip()}
 
 
@@ -26841,7 +26881,7 @@ def _normalize_names_list(names) -> list[str]:
     if names is None:
         return []
     if isinstance(names, str):
-        names = [names]
+        names = _parse_config_string_list(names)
     elif not isinstance(names, list):
         names = list(names) if names else []
     return list(dict.fromkeys(str(d).strip() for d in names if str(d).strip()))

tests/test_issue3066_disabled_read_profile.py

@@ -161,3 +161,58 @@ def test_normalize_disabled_set_handles_edge_cases():
     assert _normalize_disabled_set(["a", "b"]) == {"a", "b"}
     assert _normalize_disabled_set([" spaced ", "  "]) == {"spaced"}
     assert _normalize_disabled_set([]) == set()
+
+
+def test_disabled_read_decodes_json_array_string(tmp_path, monkeypatch):
+    """Issue #7120: skills.disabled stored as a JSON-array string (the shape
+    produced by `hermes config set skills.disabled ...`) must decode to the
+    real names, not one literal entry."""
+    from api import routes
+
+    config_path = tmp_path / "config.yaml"
+    _write_config(config_path, {"skills": {"disabled": '["skill-x", "skill-y"]'}})
+    monkeypatch.setattr("api.routes._get_config_path", lambda: config_path)
+
+    result = routes._get_disabled_skill_names_for_profile()
+    assert result == {"skill-x", "skill-y"}
+
+
+def test_disabled_read_platform_webui_decodes_json_array_string(tmp_path, monkeypatch):
+    """Issue #7120: platform_disabled.webui stored as a JSON-array string is
+    decoded the same way as the global disabled list."""
+    from api import routes
+
+    config_path = tmp_path / "config.yaml"
+    _write_config(config_path, {
+        "skills": {
+            "disabled": ["global-disabled"],
+            "platform_disabled": {"webui": '["webui-disabled-a", "webui-disabled-b"]'},
+        }
+    })
+    monkeypatch.setattr("api.routes._get_config_path", lambda: config_path)
+
+    result = routes._get_disabled_skill_names_for_profile()
+    assert result == {"webui-disabled-a", "webui-disabled-b"}
+    assert "global-disabled" not in result
+
+
+@requires_agent_modules
+def test_skills_list_disabled_decodes_json_array_string(tmp_path, monkeypatch):
+    """Issue #7120: the Skills panel (skills list endpoint) must report both
+    skills as disabled when config.yaml stores disabled as a JSON-array string."""
+    from api import routes
+
+    skills_dir = tmp_path / "skills"
+    _write_skill(skills_dir, "skill-a")
+    _write_skill(skills_dir, "skill-b")
+
+    config_path = tmp_path / "config.yaml"
+    _write_config(config_path, {"skills": {"disabled": '["skill-a", "skill-b"]'}})
+    monkeypatch.setattr("api.routes._get_config_path", lambda: config_path)
+    monkeypatch.setattr("api.routes._active_skills_dir", lambda: skills_dir)
+
+    result = routes._skills_list_from_dir(skills_dir)
+    skills = {s["name"]: s for s in result["skills"]}
+
+    assert skills["skill-a"]["disabled"] is True
+    assert skills["skill-b"]["disabled"] is True

tests/test_skills_toggle.py

@@ -197,3 +197,43 @@ def test_platform_disabled_no_write_through_when_key_absent(tmp_path, monkeypatc
     assert "skill-b" in cfg_after["skills"]["disabled"]
     # platform_disabled was never created
     assert "platform_disabled" not in cfg_after.get("skills", {})
+
+
+def test_toggle_preserves_json_array_string_entries(tmp_path, monkeypatch):
+    """Issue #7120: when skills.disabled is stored as a JSON-array string,
+    a toggle must decode it before add/remove so the other entries survive
+    the write (previously the whole string was treated as one name and the
+    list was destroyed on the next save)."""
+    from unittest.mock import MagicMock
+    from api.routes import _handle_skill_toggle
+    from api.config import _load_yaml_config_file
+
+    config_path = tmp_path / "config.yaml"
+    monkeypatch.setattr("api.routes._get_config_path", lambda: config_path)
+
+    import yaml
+
+    config = {"skills": {"disabled": '["skill-a", "skill-b"]'}}
+    config_path.parent.mkdir(parents=True, exist_ok=True)
+    config_path.write_text(yaml.dump(config), encoding="utf-8")
+
+    fake_dir = tmp_path / "skills" / "skill-c"
+    fake_dir.mkdir(parents=True, exist_ok=True)
+    fake_md = fake_dir / "SKILL.md"
+    fake_md.write_text("---\nname: skill-c\n---\nC skill", encoding="utf-8")
+    monkeypatch.setattr(
+        "api.routes._find_skill_in_dirs",
+        lambda name, dirs: (fake_dir, fake_md),
+    )
+
+    handler = MagicMock()
+
+    # Toggle skill-c OFF — must append without dropping skill-a/skill-b
+    _handle_skill_toggle(handler, {"name": "skill-c", "enabled": False})
+    cfg_after = _load_yaml_config_file(config_path)
+    assert cfg_after["skills"]["disabled"] == ["skill-a", "skill-b", "skill-c"]
+
+    # Toggle skill-a ON — must remove only skill-a, preserving skill-b
+    _handle_skill_toggle(handler, {"name": "skill-a", "enabled": True})
+    cfg_after2 = _load_yaml_config_file(config_path)
+    assert cfg_after2["skills"]["disabled"] == ["skill-b", "skill-c"]