From 68cbe484a4328c112e3cb91ff218707a734b7a4e Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:28:47 +0530 Subject: [PATCH] refactor(config): close empty-list root gap in strict check; assert tolerant returns Review follow-ups on the final salvage stack: - `fast_safe_load(f) or {}` collapsed a falsy non-mapping root (`[]`) to `{}` before the strict isinstance check, so an empty-list config evaded the raise the previous commit added. Only map a None document (empty file) to `{}`; every other non-mapping root now hits the strict branch. - Docstring: the strict flag covers non-mapping roots too, not just parse failures. - Tests: assert the tolerant call's return per shape instead of merely calling it; add the empty-list-root case. --- hermes_cli/config.py | 6 ++++-- tests/hermes_cli/test_config.py | 23 ++++++++++++++++------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index dd0c9f6ea8..893cfadd6f 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -2285,7 +2285,7 @@ def check_config_version(*, raise_on_parse_error: bool = False) -> Tuple[int, in Returns (current_version, latest_version). Tolerant runtime status callers retain the historical latest/latest fallback for malformed YAML. Mutation and explicit validation paths can set ``raise_on_parse_error`` so a parse - failure cannot be mistaken for an up-to-date config. + failure or a non-mapping root cannot be mistaken for an up-to-date config. """ latest = _coerce_config_version(DEFAULT_CONFIG.get("_config_version", 1)) or 1 config_path = get_config_path() @@ -2294,7 +2294,7 @@ def check_config_version(*, raise_on_parse_error: bool = False) -> Tuple[int, in try: with open(config_path, encoding="utf-8") as f: - config = fast_safe_load(f) or {} + config = fast_safe_load(f) except Exception as e: # Invalid YAML needs a parse warning, not an automatic schema rewrite # that could replace the user's broken file with defaults. @@ -2305,6 +2305,8 @@ def check_config_version(*, raise_on_parse_error: bool = False) -> Tuple[int, in ) from e return latest, latest + if config is None: + config = {} # empty file / bare document: valid first-run state if not isinstance(config, dict): # A list/scalar root parses fine but is just as unusable as broken # YAML: save_config() would refuse it later, after .env was already diff --git a/tests/hermes_cli/test_config.py b/tests/hermes_cli/test_config.py index 186cd97882..3ed68bba14 100644 --- a/tests/hermes_cli/test_config.py +++ b/tests/hermes_cli/test_config.py @@ -786,13 +786,22 @@ class TestConfigVersionDetection: assert load_config()["_config_version"] == DEFAULT_CONFIG["_config_version"] assert check_config_version() == (0, DEFAULT_CONFIG["_config_version"]) + _LATEST = DEFAULT_CONFIG["_config_version"] + # (bytes, strict match, tolerant return): tolerant malformed YAML keeps + # the historical latest/latest fallback; a parseable non-mapping root is + # reported as legacy (0). _INVALID_CONFIG_CASES = [ - pytest.param(b"model: [unterminated\n", "not valid YAML", id="malformed-yaml"), - pytest.param(b"- just_a_list\n", "must be a mapping", id="list-root"), + pytest.param( + b"model: [unterminated\n", "not valid YAML", (_LATEST, _LATEST), id="malformed-yaml" + ), + pytest.param(b"- just_a_list\n", "must be a mapping", (0, _LATEST), id="list-root"), + pytest.param(b"[]\n", "must be a mapping", (0, _LATEST), id="empty-list-root"), ] - @pytest.mark.parametrize("config_bytes, match", _INVALID_CONFIG_CASES) - def test_strict_check_rejects_invalid_config(self, tmp_path, config_bytes, match): + @pytest.mark.parametrize("config_bytes, match, tolerant", _INVALID_CONFIG_CASES) + def test_strict_check_rejects_invalid_config( + self, tmp_path, config_bytes, match, tolerant + ): config_path = tmp_path / "config.yaml" config_path.write_bytes(config_bytes) @@ -800,11 +809,11 @@ class TestConfigVersionDetection: with pytest.raises(InvalidUserConfigError, match=match): check_config_version(raise_on_parse_error=True) # Tolerant callers keep the historical non-raising behavior. - check_config_version() + assert check_config_version() == tolerant - @pytest.mark.parametrize("config_bytes, match", _INVALID_CONFIG_CASES) + @pytest.mark.parametrize("config_bytes, match, _tolerant", _INVALID_CONFIG_CASES) def test_migration_rejects_invalid_config_before_sanitizing_env( - self, tmp_path, config_bytes, match + self, tmp_path, config_bytes, match, _tolerant ): config_path = tmp_path / "config.yaml" config_path.write_bytes(config_bytes)