From 680fa8a92dfec39b1de1baae40de4457c291b611 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Thu, 24 Sep 2026 17:10:14 +0530 Subject: [PATCH] test(config): one parametrized every-seeder test + template parity, drop duplicates The stack's regression tests pasted the same platform-parity loop into four files and added eight cases for one invariant ("a seeded config must not pin a global display key"), over the AGENTS.md invariant-test budget. - assert_keeps_platform_display_defaults() in test_display_config.py is the single parity helper. - test_shipped_template_keeps_every_platform_default moves out of TestYAMLNormalisation (it was attached there only by indentation) into TestInstallerSeededConfigThroughGatewayResolver, reuses _seed_like_installer and absorbs the _resolve_gateway_display_bool qqbot hop. - test_config_edit_seed's test becomes the one parametrized every-seeder test: config edit (template / no template), `hermes setup agent` with Enter (the W1 leak fixed in this stack), _apply_default_agent_settings and blank slate. The per-file copies in test_setup_agent_settings / test_setup_blank_slate are removed. - Dropped test_template_seeded_home_keeps_qqbot_reasoning_off (duplicate; its dead HERMES_HOME set + raising=False patch went with it) and test_show_reasoning_stays_a_known_config_key (change-detector for the rejected #121232 approach). The explicit opt-in control (#7148) stays. On origin/main's production files the template test and all five seeder cases fail; the control passes. --- tests/gateway/test_display_config.py | 64 ++++++----------- tests/hermes_cli/test_config_edit_seed.py | 68 ++++++++++++++----- tests/hermes_cli/test_setup_agent_settings.py | 18 ----- tests/hermes_cli/test_setup_blank_slate.py | 9 --- 4 files changed, 73 insertions(+), 86 deletions(-) diff --git a/tests/gateway/test_display_config.py b/tests/gateway/test_display_config.py index 893d67de4b..d0ab3bee9a 100644 --- a/tests/gateway/test_display_config.py +++ b/tests/gateway/test_display_config.py @@ -139,36 +139,25 @@ class TestYAMLNormalisation: # --------------------------------------------------------------------------- - def test_shipped_template_keeps_every_platform_default(self, tmp_path): - """The installers, the Docker first boot and ``doctor --fix`` copy - cli-config.yaml.example verbatim, so an uncommented ``display.`` there - becomes an explicit global value that beats every platform tier.""" - import shutil - from pathlib import Path +def assert_keeps_platform_display_defaults(cfg): + """Every platform resolves every tier key (and tool_progress) exactly as with no config at all. - from gateway.display_config import _PLATFORM_DEFAULTS, resolve_display_setting, resolve_tool_progress - from gateway.run import _load_gateway_config + Shared by every config seeder's regression test: a seeded global ``display.`` beats each + platform tier, because the gateway loader merges no DEFAULT_CONFIG (#121230).""" + from gateway.display_config import _PLATFORM_DEFAULTS, resolve_display_setting, resolve_tool_progress - template = Path(__file__).resolve().parents[2] / "cli-config.yaml.example" - shutil.copy(template, tmp_path / "config.yaml") - seeded = _load_gateway_config(tmp_path / "config.yaml") - assert "display" in seeded # the loader fails open to {}, which would pass vacuously - - tier_keys = {key for tier in _PLATFORM_DEFAULTS.values() for key in tier} - for platform in _PLATFORM_DEFAULTS: - assert resolve_tool_progress(seeded, platform) == resolve_tool_progress({}, platform), platform - for key in tier_keys: - assert resolve_display_setting(seeded, platform, key) == resolve_display_setting({}, platform, key), ( - platform, key) + tier_keys = {key for tier in _PLATFORM_DEFAULTS.values() for key in tier} + for platform in _PLATFORM_DEFAULTS: + assert resolve_tool_progress(cfg, platform) == resolve_tool_progress({}, platform), platform + for key in tier_keys: + assert resolve_display_setting(cfg, platform, key) == resolve_display_setting({}, platform, key), ( + platform, key) class TestInstallerSeededConfigThroughGatewayResolver: """Regression for #121230: a fresh install copies cli-config.yaml.example to config.yaml, and the gateway then rendered reasoning into QQBot/Telegram/... because the template pinned a global ``display.show_reasoning: true`` over every platform's ``False`` default. - - Goes through the resolver the gateway turn actually calls (``gateway/run.py``), with the same - arguments as ``gateway/run_turn.py``, not only ``resolve_display_setting``. """ @staticmethod @@ -181,23 +170,21 @@ class TestInstallerSeededConfigThroughGatewayResolver: shutil.copy(template, home / "config.yaml") return home / "config.yaml" - def test_template_seeded_home_keeps_qqbot_reasoning_off(self, tmp_path, monkeypatch): + def test_shipped_template_keeps_every_platform_default(self, tmp_path): + """The installers, the Docker first boot and ``doctor --fix`` copy cli-config.yaml.example + verbatim, so an uncommented ``display.`` there becomes an explicit global value.""" from gateway.config import Platform from gateway.run import _load_gateway_config, _resolve_gateway_display_bool - home = tmp_path / "hermes-home" - self._seed_like_installer(home) - monkeypatch.setenv("HERMES_HOME", str(home)) - monkeypatch.setattr("gateway.run._gateway_config_home", lambda: home, raising=False) + seeded = _load_gateway_config(self._seed_like_installer(tmp_path / "hermes-home")) + assert "display" in seeded # the loader fails open to {}, which would pass vacuously - seeded = _load_gateway_config() - assert "display" in seeded # loader fails open to {}, which would make the assertion below vacuous - - for platform in (Platform.QQBOT, Platform.TELEGRAM, Platform.DISCORD): - assert _resolve_gateway_display_bool( - seeded, platform.value, "show_reasoning", default=False, platform=platform, - require_platform_override_for={Platform.MATTERMOST}, - ) is False, platform + assert_keeps_platform_display_defaults(seeded) + # ...and through the resolver the gateway turn actually calls (same arguments as gateway/run_turn.py). + assert _resolve_gateway_display_bool( + seeded, "qqbot", "show_reasoning", default=False, platform=Platform.QQBOT, + require_platform_override_for={Platform.MATTERMOST}, + ) is False def test_explicit_global_opt_in_still_reaches_gateway_platforms(self, tmp_path): """Control: an operator who deliberately writes ``display.show_reasoning: true`` still gets it (#7148).""" @@ -211,13 +198,6 @@ class TestInstallerSeededConfigThroughGatewayResolver: require_platform_override_for={Platform.MATTERMOST}, ) is True - def test_show_reasoning_stays_a_known_config_key(self): - """Control: the fix must not delete the key from DEFAULT_CONFIG, or - ``hermes config set display.show_reasoning false`` gets rejected as unknown.""" - from hermes_cli.config import _validate_config_key - - assert _validate_config_key("display.show_reasoning") == (True, None) - # --------------------------------------------------------------------------- # Config migration: tool_progress_overrides → display.platforms diff --git a/tests/hermes_cli/test_config_edit_seed.py b/tests/hermes_cli/test_config_edit_seed.py index 8651f0552d..148466b706 100644 --- a/tests/hermes_cli/test_config_edit_seed.py +++ b/tests/hermes_cli/test_config_edit_seed.py @@ -1,30 +1,64 @@ -"""``hermes config edit`` on a home with no config.yaml seeds one. The seed must not pin a display value over -any messaging platform's own default (the gateway loader merges no DEFAULT_CONFIG, so every written key is explicit).""" +"""Every writer that seeds or first-configures config.yaml must leave each messaging platform's display defaults +alone (#121230). The gateway loader merges no DEFAULT_CONFIG, so any written global ``display.`` beats every +platform tier (e.g. Telegram/Slack tool_progress ``off`` -> ``all``, QQBot show_reasoning ``False`` -> ``True``).""" import pytest +from tests.gateway.test_display_config import assert_keeps_platform_display_defaults -@pytest.mark.parametrize("seed", ["template", "no-template"]) -def test_config_edit_seed_keeps_every_platform_display_default(tmp_path, monkeypatch, seed): + +def _config_edit(tmp_path, monkeypatch, cfg, *, template): + """`hermes config edit` on a home with no config.yaml (seeds via seed_config_file, like `doctor --fix`).""" + monkeypatch.setenv("EDITOR", "true") + monkeypatch.setattr(cfg.subprocess, "run", lambda *a, **k: None) + if not template: + monkeypatch.setattr(cfg, "get_project_root", lambda: tmp_path / "no-checkout") + cfg.edit_config() + + +def _setup_agent_enter(tmp_path, monkeypatch, cfg): + """`hermes setup agent` on a fresh home, pressing Enter (the offered default) on every prompt.""" + import hermes_cli.setup as setup + + monkeypatch.setattr(setup, "prompt", lambda question, default=None, *a, **k: default or "") + monkeypatch.setattr(setup, "prompt_yes_no", lambda *a, **k: False) + setup.setup_agent_settings(cfg.load_config()) + + +def _apply_default_agent_settings(tmp_path, monkeypatch, cfg): + """Quick and full first-time setup.""" + from hermes_cli.setup import _apply_default_agent_settings + + _apply_default_agent_settings(cfg.load_config()) + + +def _blank_slate(tmp_path, monkeypatch, cfg): + from hermes_cli.setup_quick import _blank_slate_minimize_config + + config = cfg.load_config() + _blank_slate_minimize_config(config) + cfg.save_config(config) + + +SEEDERS = { + "config-edit-template": lambda *a: _config_edit(*a, template=True), + "config-edit-no-template": lambda *a: _config_edit(*a, template=False), + "setup-agent-enter": _setup_agent_enter, + "apply-default-agent-settings": _apply_default_agent_settings, + "blank-slate": _blank_slate, +} + + +@pytest.mark.parametrize("seeder", list(SEEDERS)) +def test_every_seeder_keeps_every_platform_display_default(tmp_path, monkeypatch, seeder): import hermes_cli.config as cfg - from gateway.display_config import _PLATFORM_DEFAULTS, resolve_display_setting, resolve_tool_progress from gateway.run import _load_gateway_config home = tmp_path / "home" home.mkdir() monkeypatch.setenv("HERMES_HOME", str(home)) - monkeypatch.setenv("EDITOR", "true") - monkeypatch.setattr(cfg.subprocess, "run", lambda *a, **k: None) - if seed == "no-template": - monkeypatch.setattr(cfg, "get_project_root", lambda: tmp_path / "no-checkout") - cfg.edit_config() + SEEDERS[seeder](tmp_path, monkeypatch, cfg) config_path = home / "config.yaml" assert config_path.exists() # the resolution checks below would pass on a missing file - seeded = _load_gateway_config(config_path) - tier_keys = {key for tier in _PLATFORM_DEFAULTS.values() for key in tier} - for platform in _PLATFORM_DEFAULTS: - assert resolve_tool_progress(seeded, platform) == resolve_tool_progress({}, platform), platform - for key in tier_keys: - assert resolve_display_setting(seeded, platform, key) == resolve_display_setting({}, platform, key), ( - platform, key) + assert_keeps_platform_display_defaults(_load_gateway_config(config_path)) diff --git a/tests/hermes_cli/test_setup_agent_settings.py b/tests/hermes_cli/test_setup_agent_settings.py index 68429d3cff..6aa725fb25 100644 --- a/tests/hermes_cli/test_setup_agent_settings.py +++ b/tests/hermes_cli/test_setup_agent_settings.py @@ -46,21 +46,3 @@ def test_setup_agent_settings_prefers_config_over_stale_env(tmp_path, monkeypatc assert "Press Enter to keep 60." not in out # And the stale .env entry gets cleaned up assert "HERMES_MAX_ITERATIONS" in removed_keys - - -def test_first_time_defaults_keep_every_platform_tool_progress_default(tmp_path, monkeypatch): - """Quick and full first-time setup run this. A global display.tool_progress it - writes beats every platform tier, so Telegram and Slack went from off to all.""" - from gateway.display_config import _PLATFORM_DEFAULTS, resolve_tool_progress - from gateway.run import _load_gateway_config - from hermes_cli.config import load_config - from hermes_cli.setup import _apply_default_agent_settings - - monkeypatch.setenv("HERMES_HOME", str(tmp_path)) - - _apply_default_agent_settings(load_config()) - - on_disk = _load_gateway_config(tmp_path / "config.yaml") - assert on_disk.get("agent", {}).get("max_turns") == 150 # the save really landed - for platform in _PLATFORM_DEFAULTS: - assert resolve_tool_progress(on_disk, platform) == resolve_tool_progress({}, platform), platform diff --git a/tests/hermes_cli/test_setup_blank_slate.py b/tests/hermes_cli/test_setup_blank_slate.py index d54d01bf74..6c16ea4425 100644 --- a/tests/hermes_cli/test_setup_blank_slate.py +++ b/tests/hermes_cli/test_setup_blank_slate.py @@ -83,15 +83,6 @@ class TestBlankSlateMinimizeConfig: assert cfg["checkpoints"]["enabled"] is False assert cfg["smart_model_routing"]["enabled"] is False - def test_messaging_platforms_keep_their_tool_progress_default(self): - """A global display.tool_progress beats every platform tier (Telegram/Slack off).""" - from gateway.display_config import _PLATFORM_DEFAULTS, resolve_tool_progress - - cfg = {} - _blank_slate_minimize_config(cfg) - for platform in _PLATFORM_DEFAULTS: - assert resolve_tool_progress(cfg, platform) == resolve_tool_progress({}, platform), platform - class TestBlankSlateFork: """The post-baseline fork: finish now vs walk through configurations."""