hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 28 Packages Projects Releases Wiki Activity

fix(mcp): CLI readers no longer reopen include: [] as "all tools enabled"

Browse Source 
The runtime already registers nothing for an explicit empty include list
(fb1ec36a4b), 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).
This commit is contained in:
teknium1
2026-09-12 06:43:26 -07:00
committed by Teknium
parent 88d84beede
commit b70e0f4603
5 changed files with 53 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
hermes_cli/mcp_catalog.py
View File

@@ -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)}.")

23
hermes_cli/mcp_config.py
View File

@@ -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

16
hermes_cli/tools_config_mcp.py
View File

@@ -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:

11
tests/hermes_cli/test_mcp_config.py
View File

@@ -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)

17
tests/hermes_cli/test_mcp_tools_config.py
View File

@@ -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": []}