From 45b35f962f2f904616ae2300d67f14b7dc8cb5ba Mon Sep 17 00:00:00 2001
From: Teknium <127238744+teknium1@users.noreply.github.com>
Date: Tue, 25 Aug 2026 04:10:14 -0700
Subject: [PATCH] fix(mcp): tool-selection UIs stay in exclude mode instead of
 freezing include lists
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes the two config-UI halves of the exclude-mode review (GottZ findings
5-7 on #94513):

- hermes mcp configure: on an exclude-mode server, unchecking a tool now
  APPENDS a literal exclude and re-checking drops it — glob patterns are
  preserved so future vendor tools keep getting filtered. Previously one
  uncheck converted the whole config to a frozen include list (globs
  silently deleted, new vendor tools invisible). Re-checked tools still
  shadowed by a kept glob get an explicit warning instead of a silent
  no-op.
- hermes tools MCP checklist: same exclude-mode write-back, plus display
  now matches excludes via matches_name_filter (fnmatch) — glob excludes
  previously rendered as if nothing were excluded.
- klaviyo manifest: post_install no longer tells users to append a param
  the URL already pins; now documents how to get the FULL surface.

Live-verified through the real cmd_mcp_configure path: exclude-mode server
with ['*_secret_*', 'docs'], uncheck beta + re-check docs ->
exclude becomes ['*_secret_*', 'beta'], no include written.
171 tests green across test_mcp_catalog/test_mcp_config/test_mcp_tool.
---
 hermes_cli/mcp_config.py            | 48 +++++++++++++++++++-
 hermes_cli/tools_config.py          | 70 ++++++++++++++++++++++++-----
 optional-mcps/klaviyo/manifest.yaml |  9 ++--
 3 files changed, 111 insertions(+), 16 deletions(-)

diff --git a/hermes_cli/mcp_config.py b/hermes_cli/mcp_config.py
index 99ab692fc6..05adb6bf5b 100644
--- a/hermes_cli/mcp_config.py
+++ b/hermes_cli/mcp_config.py
@@ -1074,9 +1074,55 @@ def cmd_mcp_configure(args):
     config = load_config()
     server_entry = cfg_get(config, "mcp_servers", name, default={})
 
-    if len(chosen) == total:
+    exclude_mode = bool(exclude) and isinstance(exclude, list) and not include
+
+    if len(chosen) == total and not exclude_mode:
         # All selected → remove include/exclude (register all)
         server_entry.pop("tools", None)
+    elif exclude_mode:
+        # Exclude-mode entry (catalog default_excluded or hand-written
+        # tools.exclude): stay in exclude mode instead of demoting the
+        # user's dynamic filter to a frozen include list. Newly-unchecked
+        # tools are appended as literal excludes; re-checked tools have
+        # their literal entries dropped. Glob patterns are preserved —
+        # they keep excluding future vendor tools by design.
+        old_exclude = [str(p) for p in (exclude or [])]
+        glob_entries = [p for p in old_exclude
+                        if "*" in p or "?" in p or "[" in p]
+        literal_entries = {p for p in old_exclude if p not in glob_entries}
+        unchecked = {tool_names[i] for i in range(total) if i not in chosen}
+        checked = {tool_names[i] for i in chosen}
+
+        # Literal excludes: drop re-checked, add newly-unchecked.
+        new_literals = (literal_entries - checked) | {
+            tn for tn in unchecked
+            if not matches_name_filter(tn, set(old_exclude))
+        }
+        new_exclude = glob_entries + sorted(new_literals)
+
+        # A re-checked tool still matched by a kept glob can't be enabled
+        # without dropping the glob — surface that instead of silently
+        # ignoring the click or silently freezing the config.
+        glob_shadowed = sorted(
+            tn for tn in checked
+            if glob_entries and matches_name_filter(tn, set(glob_entries))
+        )
+        if glob_shadowed:
+            _warning(
+                f"{len(glob_shadowed)} re-enabled tool(s) still match glob "
+                f"exclude pattern(s) {glob_entries} and stay excluded: "
+                f"{', '.join(glob_shadowed[:5])}"
+                f"{' ...' if len(glob_shadowed) > 5 else ''}. Remove the "
+                f"pattern from mcp_servers.{name}.tools.exclude in "
+                "config.yaml to enable them."
+            )
+
+        if not new_exclude:
+            server_entry.pop("tools", None)
+        else:
+            server_entry.setdefault("tools", {})
+            server_entry["tools"]["exclude"] = new_exclude
+            server_entry["tools"].pop("include", None)
     else:
         chosen_names = [tool_names[i] for i in sorted(chosen)]
         server_entry.setdefault("tools", {})
diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py
index 46feaf6b74..d37d58f1e1 100644
--- a/hermes_cli/tools_config.py
+++ b/hermes_cli/tools_config.py
@@ -5740,17 +5740,29 @@ def _configure_mcp_tools_interactive(config: dict):
             else:
                 labels.append(tool_name)
 
-        # Determine which tools are currently enabled
+        # Determine which tools are currently enabled. Use the SAME matching
+        # semantics as runtime registration (tools/mcp_tool.py): exact names
+        # or fnmatch globs — a literal `in` check renders glob excludes
+        # (e.g. "*team_member*" from catalog default_excluded manifests) as
+        # if nothing were excluded.
+        try:
+            from tools.mcp_tool import matches_name_filter as _match_filter
+        except ImportError:  # pragma: no cover — defensive fallback
+            def _match_filter(tool_name, patterns):
+                return tool_name in patterns
+
         pre_selected: Set[int] = set()
         tool_names = [t[0] for t in tools]
+        include_set = {str(p) for p in include_list} if include_list else None
+        exclude_set = {str(p) for p in exclude_list} if exclude_list else None
         for i, tool_name in enumerate(tool_names):
-            if include_list:
+            if include_set:
                 # Include mode: only included tools are selected
-                if tool_name in include_list:
+                if _match_filter(tool_name, include_set):
                     pre_selected.add(i)
-            elif exclude_list:
+            elif exclude_set:
                 # Exclude mode: everything except excluded
-                if tool_name not in exclude_list:
+                if not _match_filter(tool_name, exclude_set):
                     pre_selected.add(i)
             else:
                 # No filter: all enabled
@@ -5767,23 +5779,57 @@ def _configure_mcp_tools_interactive(config: dict):
             _print_info(f"  {server_name}: no changes")
             continue
 
-        # Compute new include list (the chosen tools). We standardize on
-        # tools.include across the codebase (catalog installs, hermes mcp
-        # configure, and this UI) so a server\'s on-disk config shape doesn\'t
-        # depend on which UI the user touched last.
-        chosen_names = [tool_names[i] for i in sorted(chosen)]
-
         # Update config
         srv_cfg = mcp_servers.setdefault(server_name, {})
         tools_cfg = srv_cfg.setdefault("tools", {})
 
-        if len(chosen) == len(tools):
+        exclude_mode = bool(exclude_set) and not include_set
+
+        if len(chosen) == len(tools) and not exclude_mode:
             # All tools enabled — clear filters (cleanest config shape; the
             # server\'s native tool set is the active set, and any tools the
             # server adds later are auto-enabled).
             tools_cfg.pop("exclude", None)
             tools_cfg.pop("include", None)
+        elif exclude_mode:
+            # Exclude-mode server (catalog default_excluded / hand-written
+            # tools.exclude): stay in exclude mode — do NOT demote the
+            # dynamic filter to a frozen include list. Unchecked tools are
+            # added as literal excludes; re-checked literals are dropped;
+            # glob patterns are preserved (they intentionally keep matching
+            # tools the vendor ships later).
+            old_exclude = sorted(exclude_set or set())
+            glob_entries = [p for p in old_exclude
+                            if "*" in p or "?" in p or "[" in p]
+            literal_entries = {p for p in old_exclude if p not in glob_entries}
+            unchecked = {tool_names[i] for i in range(len(tools))
+                         if i not in chosen}
+            checked = {tool_names[i] for i in chosen}
+            new_literals = (literal_entries - checked) | {
+                tn for tn in unchecked
+                if not _match_filter(tn, set(old_exclude))
+            }
+            new_exclude = glob_entries + sorted(new_literals)
+            glob_shadowed = sorted(
+                tn for tn in checked
+                if glob_entries and _match_filter(tn, set(glob_entries))
+            )
+            if glob_shadowed:
+                _print_warning(
+                    f"  {server_name}: {len(glob_shadowed)} re-enabled "
+                    f"tool(s) still match glob exclude pattern(s) "
+                    f"{glob_entries} and stay excluded — edit "
+                    f"mcp_servers.{server_name}.tools.exclude in config.yaml "
+                    "to enable them."
+                )
+            if not new_exclude:
+                tools_cfg.pop("exclude", None)
+                tools_cfg.pop("include", None)
+            else:
+                tools_cfg["exclude"] = new_exclude
+                tools_cfg.pop("include", None)
         else:
+            chosen_names = [tool_names[i] for i in sorted(chosen)]
             tools_cfg["include"] = chosen_names
             # Drop any legacy exclude block — we\'re include-mode now.
             tools_cfg.pop("exclude", None)
diff --git a/optional-mcps/klaviyo/manifest.yaml b/optional-mcps/klaviyo/manifest.yaml
index 34d64863eb..3caf051d1d 100644
--- a/optional-mcps/klaviyo/manifest.yaml
+++ b/optional-mcps/klaviyo/manifest.yaml
@@ -35,6 +35,9 @@ post_install: |
   Klaviyo (or run `hermes mcp login klaviyo`). Approve access,
   then restart the session so tools load.
 
-  Requires an Owner/Admin/Manager Klaviyo role. Large tool surface;
-  append ?core-tools-only=true to mcp_servers.klaviyo.url in config.yaml
-  for the trimmed ~40-tool core set.
+  Requires an Owner/Admin/Manager Klaviyo role.
+
+  Hermes pins the trimmed ~40-tool core surface plus Klaviyo's
+  prompt-injection mitigation via URL params. For the full 262-tool
+  surface, remove the query params from mcp_servers.klaviyo.url in
+  config.yaml.