From 10a16e449e699b431be452700d9db477e2a18fd8 Mon Sep 17 00:00:00 2001 From: Greg MacDonald Date: Sat, 8 Aug 2026 22:39:19 -0500 Subject: [PATCH] fix(config): parse list/dict literals in config set; tolerate string-typed scan roots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two sides of one bug class: `hermes config set desktop.repo_scan_roots '["~/src"]'` stored the literal as a quoted STRING, and the desktop repo-discovery policy loader's isinstance-list guard silently discarded it — falling back to roots=[] (scan all of $HOME). The discovered_repos cache kept refilling with stale entries and the Projects sidebar showed long-dead repos no matter how often the cache was purged. - hermes_cli/config.py set_config_value: values starting with [ or { for non-string-typed keys are YAML/JSON-parsed and stored as real collections. String-typed keys (display.skin etc.) keep literal text; malformed literals stay strings. - tui_gateway/server.py _repo_discovery_policy: new _coerce_scan_path_list parses a string-typed list literal already sitting in user config (with a warning), accepts a bare path as a one-element list, and only then falls back to defaults — so existing broken configs heal instead of silently scanning $HOME. --- hermes_cli/config.py | 16 +++++++- tests/hermes_cli/test_set_config_value.py | 39 +++++++++++++++++++ tests/tui_gateway/test_projects_rpc.py | 42 ++++++++++++++++++++ tui_gateway/server.py | 47 ++++++++++++++++++----- 4 files changed, 133 insertions(+), 11 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index e9aece05fb9bc..a786174d74c66 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -4926,7 +4926,21 @@ def set_config_value(key: str, value: str, force: bool = False): # 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'}: + stripped = value.strip() + if stripped[:1] in ("[", "{"): + # A JSON/YAML collection literal (e.g. `hermes config set + # desktop.repo_scan_roots '["~/src"]'`). Before this parse, the + # literal was stored as a quoted STRING, which list-typed readers + # (isinstance(..., list) guards) silently rejected — falling back + # to defaults with no warning (#desktop repo_scan_roots scanning + # all of $HOME despite a configured root). + try: + parsed = fast_safe_load(stripped) + except Exception: + parsed = None + if isinstance(parsed, (list, dict)): + coerced_value = parsed + elif value.lower() in {'true', 'yes', 'on'}: coerced_value = True elif value.lower() in {'false', 'no', 'off'}: coerced_value = False diff --git a/tests/hermes_cli/test_set_config_value.py b/tests/hermes_cli/test_set_config_value.py index 9489f2ca70339..c852abe36a756 100644 --- a/tests/hermes_cli/test_set_config_value.py +++ b/tests/hermes_cli/test_set_config_value.py @@ -723,3 +723,42 @@ 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 + + +# --------------------------------------------------------------------------- +# Collection literals (issue: repo_scan_roots stored as a quoted string) +# --------------------------------------------------------------------------- + +class TestCollectionLiteralCoercion: + """`config set` of a JSON/YAML list literal must store a real list. + + Regression: `hermes config set desktop.repo_scan_roots '["~/src"]'` stored + the literal as a STRING; the desktop policy loader's isinstance-list guard + silently fell back to roots=[] ("scan all of $HOME") and stale repos kept + resurfacing in the Projects sidebar. + """ + + def test_list_literal_becomes_yaml_list(self, _isolated_hermes_home): + import yaml + set_config_value("desktop.repo_scan_roots", '["/tmp/a", "/tmp/b"]') + cfg = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert cfg["desktop"]["repo_scan_roots"] == ["/tmp/a", "/tmp/b"] + + def test_empty_list_literal(self, _isolated_hermes_home): + import yaml + set_config_value("desktop.repo_scan_exclude_paths", "[]") + cfg = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert cfg["desktop"]["repo_scan_exclude_paths"] == [] + + def test_string_typed_default_is_not_parsed(self, _isolated_hermes_home): + """String-typed settings keep literal text (existing contract).""" + import yaml + set_config_value("display.skin", "[weird-but-literal]") + cfg = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert cfg["display"]["skin"] == "[weird-but-literal]" + + def test_malformed_literal_stays_string(self, _isolated_hermes_home): + import yaml + set_config_value("desktop.repo_scan_roots", "[not: valid: yaml: [") + cfg = yaml.safe_load(_read_config(_isolated_hermes_home)) + assert cfg["desktop"]["repo_scan_roots"] == "[not: valid: yaml: [" diff --git a/tests/tui_gateway/test_projects_rpc.py b/tests/tui_gateway/test_projects_rpc.py index cb9638420fc52..6af7052af15d3 100644 --- a/tests/tui_gateway/test_projects_rpc.py +++ b/tests/tui_gateway/test_projects_rpc.py @@ -379,3 +379,45 @@ def test_nondefault_policy_rejects_stale_or_legacy_results(monkeypatch, tmp_path assert any(item["root"] == str(root) for item in accepted["repos"]) + + +# --------------------------------------------------------------------------- +# _repo_discovery_policy tolerance for string-typed list settings +# --------------------------------------------------------------------------- + +class TestScanPathCoercion: + """The policy loader must not silently discard a configured root. + + Regression: a string-typed `repo_scan_roots: '["~/src"]'` (written by older + `hermes config set`) failed the isinstance-list guard and fell back to + roots=[] — scanning all of $HOME and refilling the discovered-repos cache + with stale entries. + """ + + def test_string_list_literal_is_parsed(self): + from tui_gateway.server import _repo_discovery_policy + policy = _repo_discovery_policy({"repo_scan_roots": '["/tmp/x"]'}) + assert policy["roots"] == ["/tmp/x"] + + def test_bare_string_path_becomes_single_root(self): + from tui_gateway.server import _repo_discovery_policy + policy = _repo_discovery_policy({"repo_scan_roots": "/tmp/x"}) + assert policy["roots"] == ["/tmp/x"] + + def test_real_list_unchanged(self): + from tui_gateway.server import _repo_discovery_policy + policy = _repo_discovery_policy({"repo_scan_roots": ["/tmp/a", " /tmp/b "]}) + assert policy["roots"] == ["/tmp/a", "/tmp/b"] + + def test_garbage_falls_back_to_default(self): + from hermes_cli.config import DEFAULT_CONFIG + from tui_gateway.server import _repo_discovery_policy + policy = _repo_discovery_policy({"repo_scan_roots": 42}) + assert policy["roots"] == list(DEFAULT_CONFIG["desktop"]["repo_scan_roots"]) + + def test_string_excludes_also_parsed(self): + from tui_gateway.server import _repo_discovery_policy + policy = _repo_discovery_policy( + {"repo_scan_exclude_paths": '["/tmp/skip"]'} + ) + assert policy["exclude_paths"] == ["/tmp/skip"] diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 67083a69269f6..6ac15cfff5ba1 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -11556,6 +11556,39 @@ def _is_session_cwd_junk(cwd: str) -> bool: return real == home or real == hermes_home +def _coerce_scan_path_list(value: Any, default: list) -> list[str]: + """Normalize a scan-path setting to a clean list of strings. + + Tolerates a string-typed collection literal (``'["~/src"]'``) left behind + by older ``hermes config set`` versions, which stored list values as quoted + strings. Silently falling back to the default here meant an empty-roots + policy — i.e. "scan all of $HOME" — despite the user having configured a + root, and the stale-repo cache kept refilling with no hint why. + """ + if isinstance(value, str): + stripped = value.strip() + if stripped[:1] == "[": + try: + from utils import fast_safe_load + + parsed = fast_safe_load(stripped) + except Exception: + parsed = None + if isinstance(parsed, list): + logger.warning( + "desktop scan path setting is a string-typed list literal " + "(%r) — parsed it, but re-save it as a real YAML list", + stripped, + ) + value = parsed + elif stripped: + # A bare single path — accept it as a one-element list. + value = [stripped] + if not isinstance(value, list): + return list(default) + return [v.strip() for v in value if isinstance(v, str) and v.strip()] + + def _repo_discovery_policy(raw: dict | None = None) -> dict: """Return the effective, profile-local Desktop repository scan policy.""" from hermes_cli.config import DEFAULT_CONFIG @@ -11574,16 +11607,10 @@ def _repo_discovery_policy(raw: dict | None = None) -> dict: return { "enabled": enabled if isinstance(enabled, bool) else defaults["repo_scan_enabled"], - "roots": [value.strip() for value in roots if isinstance(value, str) and value.strip()] - if isinstance(roots, list) - else list(defaults["repo_scan_roots"]), - "exclude_paths": [ - value.strip() - for value in excludes - if isinstance(value, str) and value.strip() - ] - if isinstance(excludes, list) - else list(defaults["repo_scan_exclude_paths"]), + "roots": _coerce_scan_path_list(roots, defaults["repo_scan_roots"]), + "exclude_paths": _coerce_scan_path_list( + excludes, defaults["repo_scan_exclude_paths"] + ), }