From 1abcccdeba966137d8651f2dda4681d60a3267ec Mon Sep 17 00:00:00 2001 From: Turgut Kural <58116817+TurgutKural@users.noreply.github.com> Date: Sun, 19 Jul 2026 14:35:43 +0300 Subject: [PATCH] fix(mcp): read opt-out toggle from auxiliary.mcp, not top-level mcp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The opt-out default was declared in DEFAULT_CONFIG["auxiliary"]["mcp"][...] but the watcher in _check_config_mcp_changes() read top-level load_config().get("mcp") — a key that does not exist in the loaded config shape. Consequently the declared default was never observed and the fallback stayed True at runtime: setting auto_reload_on_config_change to false in config.yaml silently did nothing. Resolve through the same path the default is declared on: cfg["auxiliary"]["mcp"]["auto_reload_on_config_change"] Tests: - test_optout_disables_auto_reload: mocked config now mirrors the real DEFAULT_CONFIG shape (auxiliary.mcp), so the test exercises the actual lookup path instead of a separately mocked shape. - test_optout_path_is_auxiliary_mcp_not_top_level: regression guard — a config that sets ONLY top-level mcp.auto_reload_on_config_change=false must NOT disable the reload. This pins the config-path contract so a future regression to _cfg.get("mcp") is caught. Addresses sweeper review: the declared default was never observed at runtime because the watcher read a different config path than the one where the default was defined. Co-authored-by: Turgut Kural --- cli.py | 8 ++++- tests/cli/test_cli_mcp_config_watch.py | 42 ++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/cli.py b/cli.py index 9b5a83cd47cd..c4d12a9f4ece 100644 --- a/cli.py +++ b/cli.py @@ -10061,10 +10061,16 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): # agent tool surface and INVALIDATES the provider prompt cache (the # next message re-sends the full input prefix, which is expensive on # long-context / high-reasoning models). + # + # The toggle lives under ``auxiliary.mcp.auto_reload_on_config_change`` + # in DEFAULT_CONFIG (same section as the MCP aux-task provider + # settings), so resolve it through that path, not a top-level ``mcp`` + # key that does not exist in the loaded config shape. try: from hermes_cli.config import load_config as _load_cfg _cfg = _load_cfg() - _mcp = _cfg.get("mcp") if isinstance(_cfg, dict) else None + _aux = _cfg.get("auxiliary") if isinstance(_cfg, dict) else None + _mcp = _aux.get("mcp") if isinstance(_aux, dict) else None _auto = _mcp.get("auto_reload_on_config_change", True) if isinstance(_mcp, dict) else True except Exception: _auto = True diff --git a/tests/cli/test_cli_mcp_config_watch.py b/tests/cli/test_cli_mcp_config_watch.py index e5a81669d082..fa1eb142b798 100644 --- a/tests/cli/test_cli_mcp_config_watch.py +++ b/tests/cli/test_cli_mcp_config_watch.py @@ -106,12 +106,17 @@ class TestMCPConfigWatch: obj._reload_mcp.assert_not_called() def test_optout_disables_auto_reload(self, tmp_path, capsys): - """When mcp.auto_reload_on_config_change is False, a changed + """When auxiliary.mcp.auto_reload_on_config_change is False, a changed mcp_servers section must NOT trigger an automatic reload — but the change is still detected and the user is told how to apply it. This protects the provider prompt cache: every automatic reload rebuilds the agent tool surface and invalidates cached prefixes. + + The toggle lives under ``auxiliary.mcp`` in DEFAULT_CONFIG (alongside + the MCP aux-task provider settings), so the mocked config must mirror + that shape — a top-level ``mcp`` key does not exist in the loaded + config and the watcher resolves through ``auxiliary.mcp``. """ import yaml obj, cfg_file = _make_cli( @@ -124,8 +129,9 @@ class TestMCPConfigWatch: obj._config_mtime = 0.0 # force stale mtime # Opt out via the loaded config (the watcher reads load_config(), - # not obj.config, so we patch the loader). - mocked_cfg = {"mcp": {"auto_reload_on_config_change": False}} + # not obj.config, so we patch the loader). Match the real shape: + # DEFAULT_CONFIG["auxiliary"]["mcp"]["auto_reload_on_config_change"]. + mocked_cfg = {"auxiliary": {"mcp": {"auto_reload_on_config_change": False}}} with patch("hermes_cli.config.get_config_path", return_value=cfg_file), \ patch("hermes_cli.config.load_config", return_value=mocked_cfg): obj._check_config_mcp_changes() @@ -136,3 +142,33 @@ class TestMCPConfigWatch: assert "reload skipped" in out assert "/reload-mcp" in out assert "prompt cache" in out + + def test_optout_path_is_auxiliary_mcp_not_top_level(self, tmp_path, capsys): + """Regression guard: the opt-out toggle lives under + ``auxiliary.mcp.auto_reload_on_config_change`` in DEFAULT_CONFIG, + NOT under a top-level ``mcp`` key. + + A config that sets ONLY ``mcp.auto_reload_on_config_change: false`` + (top-level, wrong path) must NOT disable the reload — otherwise the + watcher is reading the wrong key and the declared default never + takes effect at runtime. This test pins the config-path contract + so a future regression to ``_cfg.get("mcp")`` is caught. + """ + import yaml + obj, cfg_file = _make_cli( + tmp_path, + mcp_servers={}, + ) + + cfg_file.write_text(yaml.dump({"mcp_servers": {"github": {"url": "https://mcp.github.com"}}})) + obj._config_mtime = 0.0 + + # Wrong shape: top-level "mcp" (not where DEFAULT_CONFIG puts the + # toggle). The watcher must NOT honour this, so a reload is expected. + wrong_shape_cfg = {"mcp": {"auto_reload_on_config_change": False}} + with patch("hermes_cli.config.get_config_path", return_value=cfg_file), \ + patch("hermes_cli.config.load_config", return_value=wrong_shape_cfg): + obj._check_config_mcp_changes() + + # Reload happened because the wrong-path opt-out is ignored. + obj._reload_mcp.assert_called()