From 12e7ebbef58ef707338257fcfe2d6a818ad3f4c8 Mon Sep 17 00:00:00 2001 From: Jeff Watts <186512915+lEWFkRAD@users.noreply.github.com> Date: Thu, 2 Jul 2026 11:02:50 -0400 Subject: [PATCH] fix(tests): stop config.set model tests writing cli-config.yaml into the repo root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five tests in tests/test_tui_gateway_server.py drive a real /model switch through server.handle_request without patching cli.save_config_value. A plain /model persists globally by default (model.persist_switch_by_default → True), so _persist_model_switch ran the real save — and because the hermetic per-test HERMES_HOME tempdir never contains a config.yaml, save_config_value fell through to its project-config fallback and wrote cli-config.yaml into the developer's repo root: model: default: anthropic/claude-sonnet-4.6 provider: anthropic base_url: https://api.anthropic.com The file is gitignored, so the pollution is invisible in git status, and later runs read it back as the project-config fallback — silently breaking tests/hermes_cli/test_ignore_user_config_flags.py and tests/test_hermes_state.py. - Patch cli.save_config_value in the five offending tests (test_config_set_model_requires_confirmation_for_expensive_model, test_config_set_model_explicit_provider_skips_broken_default_init, test_config_set_model_does_not_leak_inference_provider_env, test_config_set_model_records_per_session_override_not_env, test_config_set_model_switches_agent_without_touching_env), matching the existing pattern in test_config_set_model_global_persists. - Add an autouse tripwire fixture in tests/conftest.py that fails any test that creates or mutates the repo-root cli-config.yaml, restoring the pre-test state so one offender can't cascade into unrelated failures runs later. Found during the PR #57066 / issue #57068 triage. Co-Authored-By: Claude Fable 5 --- tests/conftest.py | 51 ++++++++++++++++++++++++++++++++ tests/test_tui_gateway_server.py | 16 ++++++++++ 2 files changed, 67 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 5606300e5dc13..0d56d1da75c0e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -402,6 +402,57 @@ def _isolate_hermes_home(_hermetic_environment): return None +_REPO_ROOT_CLI_CONFIG = Path(__file__).resolve().parent.parent / "cli-config.yaml" + + +@pytest.fixture(autouse=True) +def _guard_repo_root_cli_config(): + """Fail any test that creates or mutates ``/cli-config.yaml``. + + ``cli.save_config_value()`` falls back to writing the PROJECT config — + ``cli-config.yaml`` next to ``cli.py``, i.e. the repo root — whenever + the user config (``cli._hermes_home / 'config.yaml'``) does not exist. + Under ``_hermetic_environment`` the per-test HERMES_HOME tempdir is + always empty, so any test that reaches the real ``save_config_value`` + silently drops a ``cli-config.yaml`` into the developer's checkout. + The file is gitignored (invisible in ``git status``) and is then read + back as the project-config fallback by every later run, breaking e.g. + ``tests/hermes_cli/test_ignore_user_config_flags.py`` and + ``tests/test_hermes_state.py``. + + This tripwire pins the blame on the offending test instead of letting + the pollution surface as unrelated failures three runs later. It also + restores the pre-test state so one offender can't cascade. + """ + before = ( + _REPO_ROOT_CLI_CONFIG.read_bytes() + if _REPO_ROOT_CLI_CONFIG.exists() + else None + ) + yield + after = ( + _REPO_ROOT_CLI_CONFIG.read_bytes() + if _REPO_ROOT_CLI_CONFIG.exists() + else None + ) + if after == before: + return + # Restore first so the pollution doesn't outlive the failing test. + if before is None: + _REPO_ROOT_CLI_CONFIG.unlink() + else: + _REPO_ROOT_CLI_CONFIG.write_bytes(before) + pytest.fail( + "this test wrote to the repo-root cli-config.yaml " + f"({_REPO_ROOT_CLI_CONFIG}). It reached the real " + "cli.save_config_value() with an empty HERMES_HOME, so the write " + "fell through to the project-config fallback in the developer's " + "checkout. Point cli._hermes_home at a tmp_path containing a " + "config.yaml (or monkeypatch cli.save_config_value) instead. " + f"Written content was:\n{(after or b'').decode('utf-8', 'replace')}" + ) + + # ── Module-level state reset — replaced by per-file process isolation ────── # # Each test FILE runs in a freshly-spawned ``python -m pytest `` diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 6d39a252cfe9b..74b719a44f112 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -3338,6 +3338,10 @@ def switch_model(self, **_kwargs): ) monkeypatch.setattr(server, "_restart_slash_worker", lambda sid, session: None) monkeypatch.setattr(server, "_emit", lambda *args, **kwargs: None) + # Plain /model persists globally by default; without this patch the real + # save_config_value() writes a cli-config.yaml into the repo root (the + # per-test HERMES_HOME is empty, so it falls through to project config). + monkeypatch.setattr("cli.save_config_value", lambda _key, _value: True) resp = server.handle_request( { @@ -3436,6 +3440,9 @@ def test_config_set_model_explicit_provider_skips_broken_default_init(monkeypatc monkeypatch.setattr(server, "_wait_agent", lambda *_args: seen.__setitem__("wait", seen["wait"] + 1)) monkeypatch.setattr(server, "_emit", lambda *args, **kwargs: None) monkeypatch.setattr(server, "_restart_slash_worker", lambda *args, **kwargs: None) + # Plain /model persists globally by default; keep the real + # save_config_value() from writing a repo-root cli-config.yaml. + monkeypatch.setattr("cli.save_config_value", lambda _key, _value: True) def fake_runtime_provider(*, requested=None, target_model=None, **_kwargs): seen["requested"].append((requested, target_model)) @@ -3552,6 +3559,9 @@ def switch_model(self, **_kwargs): ) monkeypatch.setattr(server, "_restart_slash_worker", lambda sid, session: None) monkeypatch.setattr(server, "_emit", lambda *args, **kwargs: None) + # Plain /model persists globally by default; keep the real + # save_config_value() from writing a repo-root cli-config.yaml. + monkeypatch.setattr("cli.save_config_value", lambda _key, _value: True) try: server.handle_request( @@ -3613,6 +3623,9 @@ def switch_model(self, **_kwargs): ) monkeypatch.setattr(server, "_restart_slash_worker", lambda sid, session: None) monkeypatch.setattr(server, "_emit", lambda *args, **kwargs: None) + # Plain /model persists globally by default; keep the real + # save_config_value() from writing a repo-root cli-config.yaml. + monkeypatch.setattr("cli.save_config_value", lambda _key, _value: True) try: server.handle_request( @@ -3695,6 +3708,9 @@ def append_message(self, session_id, role, content=None, **_kwargs): monkeypatch.delenv("HERMES_INFERENCE_MODEL", raising=False) monkeypatch.setattr(server, "_restart_slash_worker", lambda sid, session: None) monkeypatch.setattr(server, "_emit", lambda *args, **kwargs: None) + # Plain /model persists globally by default; keep the real + # save_config_value() from writing a repo-root cli-config.yaml. + monkeypatch.setattr("cli.save_config_value", lambda _key, _value: True) def fake_switch_model(**kwargs): return types.SimpleNamespace(