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"] + ), }