From b70e0f460330e646e569fccce70045ac826912b8 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 12 Sep 2026 06:43:26 -0700 Subject: [PATCH] fix(mcp): CLI readers no longer reopen `include: []` as "all tools enabled" The runtime already registers nothing for an explicit empty include list (fb1ec36a4b86), but every CLI reader still coerced `[]` to "no filter": `hermes mcp list` printed "all", `hermes mcp configure` and `hermes tools` pre-checked every tool (so confirming the picker silently re-enabled all of them), and a catalog reinstall pre-checked the manifest defaults over the user's zero-tool choice. `_tool_filters` now returns the list whenever the key holds a list; only an absent/non-list key is None. The pickers and list output branch on `is not None`, matching `tools/mcp_tool_registration.py`. Fixes #12865. Builds on #13096 (@dingn42) and #52874 (@Bartok9). --- hermes_cli/mcp_catalog.py | 4 +++- hermes_cli/mcp_config.py | 23 +++++++++++++---------- hermes_cli/tools_config_mcp.py | 16 +++++++++------- tests/hermes_cli/test_mcp_config.py | 11 +++++++++++ tests/hermes_cli/test_mcp_tools_config.py | 17 +++++++++++++++++ 5 files changed, 53 insertions(+), 18 deletions(-) diff --git a/hermes_cli/mcp_catalog.py b/hermes_cli/mcp_catalog.py index 321a4d67f7..6566fa8276 100644 --- a/hermes_cli/mcp_catalog.py +++ b/hermes_cli/mcp_catalog.py @@ -576,7 +576,9 @@ def _apply_tool_selection( ) return - pre_set = {n for n in (prior_selection or entry.tools.default_enabled or tool_names) if n in tool_names} + # A prior ``include: []`` (user chose zero tools) outranks manifest defaults, like the non-TTY path. + preferred = prior_selection if prior_selection is not None else (entry.tools.default_enabled or tool_names) + pre_set = {n for n in preferred if n in tool_names} pre_indices = {i for i, n in enumerate(tool_names) if n in pre_set} _say(f" Found {len(probed)} tool(s). Pre-checked: {len(pre_indices)}.") diff --git a/hermes_cli/mcp_config.py b/hermes_cli/mcp_config.py index 22a011f55b..066bae156a 100644 --- a/hermes_cli/mcp_config.py +++ b/hermes_cli/mcp_config.py @@ -61,14 +61,18 @@ def _get_mcp_servers(config: Optional[dict] = None) -> Dict[str, dict]: def _tool_filters(cfg: dict) -> Tuple[Optional[list], Optional[list]]: - """Return the ``(include, exclude)`` tool lists from a server config (non-empty lists only).""" + """Return the ``(include, exclude)`` tool lists from a server config; ``None`` = key absent. + + An explicit ``include: []`` is a real (block-all) whitelist — the runtime registers nothing + (tools/mcp_tool_registration.py) — so it must not collapse to "no filter" here (#12865). + """ tools_cfg = cfg.get("tools", {}) if not isinstance(tools_cfg, dict): return None, None include, exclude = tools_cfg.get("include"), tools_cfg.get("exclude") return ( - include if include and isinstance(include, list) else None, - exclude if exclude and isinstance(exclude, list) else None) + include if isinstance(include, list) else None, + exclude if isinstance(exclude, list) else None) def _save_mcp_server(name: str, server_config: dict) -> bool: @@ -565,7 +569,7 @@ def cmd_mcp_list(args=None): transport = transport[:25] + "..." include, exclude = _tool_filters(cfg) - if include: + if include is not None: tools_str = f"{len(include)} selected" elif exclude: tools_str = f"-{len(exclude)} excluded" @@ -814,11 +818,10 @@ def cmd_mcp_configure(args): def matches_name_filter(tool_name, patterns): return tool_name in patterns - patterns = {str(p) for p in (include or exclude or [])} - if patterns: - pre_selected = { - i for i, tn in enumerate(tool_names) if matches_name_filter(tn, patterns) == bool(include) - } + if include is not None: + pre_selected = {i for i, tn in enumerate(tool_names) if matches_name_filter(tn, {str(p) for p in include})} + elif exclude: + pre_selected = {i for i, tn in enumerate(tool_names) if not matches_name_filter(tn, {str(p) for p in exclude})} else: pre_selected = set(range(total)) @@ -835,7 +838,7 @@ def cmd_mcp_configure(args): config = load_config() server_entry = cfg_get(config, "mcp_servers", name, default={}) - exclude_mode = bool(exclude) and not include + exclude_mode = bool(exclude) and include is None if len(chosen) == total and not exclude_mode: server_entry.pop("tools", None) # all selected → register all diff --git a/hermes_cli/tools_config_mcp.py b/hermes_cli/tools_config_mcp.py index 67e9a5250e..dd77eb3048 100644 --- a/hermes_cli/tools_config_mcp.py +++ b/hermes_cli/tools_config_mcp.py @@ -26,7 +26,7 @@ def _mcp_match_filter(): def _mcp_preselected(tool_names: List[str], include_set, exclude_set, match) -> Set[int]: """Indices of tools currently enabled: include mode, exclude mode, or all when unfiltered.""" - if include_set: + if include_set is not None: return {i for i, tn in enumerate(tool_names) if match(tn, include_set)} if exclude_set: return {i for i, tn in enumerate(tool_names) if not match(tn, exclude_set)} @@ -36,7 +36,7 @@ def _mcp_preselected(tool_names: List[str], include_set, exclude_set, match) -> def _apply_mcp_checklist(server_name: str, tools_cfg: dict, tool_names: List[str], chosen: Set[int], include_set, exclude_set, match) -> None: """Write a checklist result back as ``tools.include`` / ``tools.exclude``.""" - exclude_mode = bool(exclude_set) and not include_set + exclude_mode = bool(exclude_set) and include_set is None if len(chosen) == len(tool_names) and not exclude_mode: # All tools enabled — clear filters so tools the server adds later are auto-enabled. @@ -118,8 +118,10 @@ def _configure_mcp_tools_interactive(config: dict): continue tools_cfg = mcp_servers.get(server_name, {}).get("tools") or {} - include_set = {str(p) for p in tools_cfg.get("include") or []} or None - exclude_set = {str(p) for p in tools_cfg.get("exclude") or []} or None + # ``include: []`` is an explicit block-all whitelist, not "unfiltered" (#12865). + include_raw, exclude_raw = tools_cfg.get("include"), tools_cfg.get("exclude") + include_set = {str(p) for p in include_raw} if isinstance(include_raw, list) else None + exclude_set = {str(p) for p in exclude_raw or []} or None labels = [] for tool_name, description in tools: @@ -207,9 +209,9 @@ def _print_tools_list(enabled_toolsets: set, mcp_servers: dict, platform: str = print("MCP servers:") for srv_name, srv_cfg in mcp_servers.items(): tools_cfg = srv_cfg.get("tools") or {} - exclude, include = tools_cfg.get("exclude") or [], tools_cfg.get("include") or [] - if include: - _print_info(f"{srv_name} [include only: {', '.join(include)}]") + exclude, include = tools_cfg.get("exclude") or [], tools_cfg.get("include") + if isinstance(include, list): + _print_info(f"{srv_name} [include only: {', '.join(include) or '(none)'}]") elif exclude: _print_info(f"{srv_name} [excluded: {color(', '.join(exclude), Colors.YELLOW)}]") else: diff --git a/tests/hermes_cli/test_mcp_config.py b/tests/hermes_cli/test_mcp_config.py index a63174ffff..f261c9b1be 100644 --- a/tests/hermes_cli/test_mcp_config.py +++ b/tests/hermes_cli/test_mcp_config.py @@ -845,3 +845,14 @@ class TestMcpReauth: cmd_mcp_reauth(_make_args(name="ghost", all=False)) out = capsys.readouterr().out assert "not found" in out + + +def test_tool_filters_keeps_explicit_empty_include(): + """``include: []`` (block-all, as written by an all-unchecked picker) is a filter, not + "no filter"; only an absent/non-list key is None (#12865).""" + from hermes_cli.mcp_config import _tool_filters + + assert _tool_filters({"tools": {"include": []}}) == ([], None) + assert _tool_filters({"tools": {"include": "bad", "exclude": ["x"]}}) == (None, ["x"]) + assert _tool_filters({}) == (None, None) + diff --git a/tests/hermes_cli/test_mcp_tools_config.py b/tests/hermes_cli/test_mcp_tools_config.py index 98c2ea47ff..b84ca2dc91 100644 --- a/tests/hermes_cli/test_mcp_tools_config.py +++ b/tests/hermes_cli/test_mcp_tools_config.py @@ -73,3 +73,20 @@ def test_empty_tools_server_skipped(capsys): assert len(checklist_calls) == 0 captured = capsys.readouterr() assert "no tools found" in captured.out + + +def test_empty_include_reopens_with_nothing_preselected(): + """``include: []`` is the runtime's block-all whitelist; the picker must not reopen it as + "all tools enabled" and must persist it when the user keeps zero tools checked (#12865).""" + config = {"mcp_servers": {"github": {"command": "npx", "tools": {"include": []}}}} + tools = [("create_issue", "Create an issue"), ("search_repos", "Search repos")] + + with patch(_PROBE, return_value={"github": tools}), \ + patch(_CHECKLIST, side_effect=lambda title, labels, pre, **kw: pre) as checklist, \ + patch(_SAVE) as mock_save: + _configure_mcp_tools_interactive(config) + + assert checklist.call_args.args[2] == set() + mock_save.assert_not_called() + assert config["mcp_servers"]["github"]["tools"] == {"include": []} +