fix(mcp): one reader for mcp_servers.<name>.enabled (#119567)

The `enabled` key had four parsers. The MCP client (`_parse_boolish`) read
`enabled: 0` as on; the toolset resolver and editor (`_parse_enabled_flag`)
read it as off. The server list (`summarize_server`, `/api/mcp/servers`) read
any non-`False` value as on, so `enabled: "false"` showed on while the agent
skipped it. The catalog and `hermes mcp list` accepted only true/1/yes, so
`enabled: on` showed off while the server ran.

`tools/mcp_tool_common.py::mcp_server_enabled` is now the only reader, and
every surface calls it. `_parse_boolish` treats YAML numbers by truthiness
(0 off, other numbers on). Everything else keeps the client's semantics:
the off words are off, absent / null / junk stay on, with the existing
warning for junk.

The desktop MCP page mirrors the rule in `serverEnabled`
(`apps/desktop/src/lib/mcp-servers.ts`). One case table
(`mcp-enabled-cases.json`) drives the Python invariant test and the vitest
test, so the page and the runtime cannot drift apart again.
This commit is contained in:
Siddharth Balyan
2026-09-23 03:30:24 +05:30
committed by GitHub
parent ec21bd7674
commit 3e00a356a4
20 changed files with 132 additions and 52 deletions

View File

@@ -122,11 +122,11 @@ _FALSE_WORDS = frozenset({"false", "0", "no", "off"})
def _parse_boolish(value: Any, default: bool = True) -> bool:
"""Parse a bool-like config value with safe fallback."""
"""Parse a bool-like config value with safe fallback (YAML ``0``/``1`` are numbers, not words)."""
if value is None:
return default
if isinstance(value, bool):
return value
if isinstance(value, (bool, int, float)):
return bool(value)
if isinstance(value, str):
lowered = value.strip().lower()
if lowered in _TRUE_WORDS:
@@ -137,6 +137,13 @@ def _parse_boolish(value: Any, default: bool = True) -> bool:
return default
def mcp_server_enabled(cfg: dict) -> bool:
"""Whether ``mcp_servers.<name>`` is on. The ONE reader of the ``enabled`` key: the MCP client,
the toolset resolver, the profile editor, and every list/status surface call it, so a value
can never be on for one surface and off for another. Absent, ``null`` or unparseable = on."""
return _parse_boolish(cfg.get("enabled", True), default=True)
def _get_lifecycle_seconds(config: dict, key: str) -> Optional[float]:
"""Optional positive lifecycle timeout from top-level/nested ``lifecycle`` config (``0``
disables; negatives and non-numbers are warned about and ignored)."""

View File

@@ -11,7 +11,7 @@ import time
from contextlib import contextmanager
from pathlib import Path
from typing import Dict, List, Optional, Tuple
from tools.mcp_tool_common import _core, _parse_boolish
from tools.mcp_tool_common import _core, _parse_boolish, mcp_server_enabled
from tools import mcp_tool_config as _config
from tools import mcp_tool_errors as _errors
from tools import mcp_tool_lifecycle as _lifecycle
@@ -66,10 +66,6 @@ def _connect_cooldown_active(server_name: str) -> bool:
return deadline is not None and time.monotonic() < deadline
def _enabled(cfg: dict) -> bool:
return _parse_boolish(cfg.get("enabled", True), default=True)
def _owner_scope_home() -> Optional[Path]:
"""The profile home whose secret scope MCP credential reads must resolve under, or None when
the caller is already scoped or this is a single-profile process (scope key ``None``).
@@ -335,9 +331,9 @@ def _select_new_servers(servers: Dict[str, dict]) -> Dict[str, dict]:
k: v for k, v in servers.items()
if keys[k] not in _core._servers and keys[k] not in _core._server_connecting
and keys[k] not in _core._lazy_server_configs
and _enabled(v) and not _connect_cooldown_active(k)}
and mcp_server_enabled(v) and not _connect_cooldown_active(k)}
stale_cached = [_core._servers[keys[k]] for k, v in servers.items()
if keys[k] in _core._servers and _enabled(v)
if keys[k] in _core._servers and mcp_server_enabled(v)
and getattr(_core._servers[keys[k]], "session", None) is None]
for srv_name in new_servers:
_core._server_connecting.add(keys[srv_name])
@@ -587,7 +583,7 @@ def discover_mcp_tools(allowed_mcp_names: Optional[List[str]] = None) -> List[st
keys = {name: _resolve_server_key(name) for name in servers}
new_server_names = [name for name, cfg in servers.items()
if keys[name] not in _core._servers and keys[name] not in _core._server_connecting
and _enabled(cfg)]
and mcp_server_enabled(cfg)]
prior_lazy = set(_core._lazy_server_configs)
tool_names = register_mcp_servers(servers)
if new_server_names:
@@ -619,7 +615,7 @@ def reconcile_mcp_servers_with_config() -> Dict[str, List[str]]:
``{"removed": [...], "added": [...], "pending": [...]}``; a no-op when nothing changed."""
with _owner_secret_scope():
servers = _config._load_mcp_config()
wanted = {name for name, cfg in servers.items() if _enabled(cfg)}
wanted = {name for name, cfg in servers.items() if mcp_server_enabled(cfg)}
scope = _core._mcp_registry_scope()
with _core._lock:
owned = [key for key, owner in _core._server_scope_keys.items() if owner == scope]
@@ -708,7 +704,7 @@ def get_mcp_status(configured: Optional[Dict[str, dict]] = None, *, include_runt
result: List[dict] = []
for name, cfg in configured.items():
enabled = _enabled(cfg) # evaluated unconditionally: malformed values warn even when connected
enabled = mcp_server_enabled(cfg) # evaluated unconditionally: malformed values warn even when connected
server = active_servers.get(name)
live = server is not None and server.session is not None
# An in-flight or failed first-use connect outranks "lazy": that server is no longer
@@ -754,7 +750,7 @@ def probe_mcp_server_tools() -> Dict[str, List[tuple]]:
if not _core._ensure_mcp_sdk():
return {}
with _owner_secret_scope():
enabled = {k: v for k, v in (_config._load_mcp_config() or {}).items() if _enabled(v)}
enabled = {k: v for k, v in (_config._load_mcp_config() or {}).items() if mcp_server_enabled(v)}
if not enabled:
return {}
_loop._ensure_mcp_loop()

View File

@@ -9,7 +9,7 @@ import threading
from dataclasses import dataclass
from types import SimpleNamespace
from typing import TYPE_CHECKING, Any, Callable, Dict, Iterable, List, Optional
from tools.mcp_tool_common import _parse_boolish, _core, _resolve_tool_timeout, mcp_field
from tools.mcp_tool_common import _parse_boolish, _core, _resolve_tool_timeout, mcp_field, mcp_server_enabled
from tools import mcp_tool_handlers as _handlers
from tools import mcp_tool_schema as _schema
from tools.mcp_tool_handlers import (
@@ -407,10 +407,6 @@ def _register_server_tools(name: str, server: "MCPServerTask", config: dict) ->
return registered
def _server_enabled(config: dict) -> bool:
return _parse_boolish(config.get("enabled", True), default=True)
def _connection_identity(config: dict) -> tuple:
"""What makes one live connection reusable for another profile: the route fingerprint PLUS
everything that authenticates it (``config_fingerprint`` deliberately excludes credentials so
@@ -473,7 +469,7 @@ def _register_connected_into_current_scope(servers: dict) -> int:
server = _core._servers.get(key)
config = servers.get(_key_name(key))
cross_profile = _key_scope(key) != scope
if (config is None or not _server_enabled(config) or server is None
if (config is None or not mcp_server_enabled(config) or server is None
or getattr(server, "session", None) is None
or not _same_server_route(server, config, cross_profile=cross_profile)):
stale.append(key)
@@ -482,7 +478,7 @@ def _register_connected_into_current_scope(servers: dict) -> int:
registered_servers = 0
for name, config in servers.items():
if not _server_enabled(config):
if not mcp_server_enabled(config):
continue
with _core._lock:
if _server_key(name, scope, current=False) in _core._servers:

View File

@@ -249,8 +249,8 @@ class MCPServerRunMixin:
entry = (_config._load_mcp_config() or {}).get(self.name)
if entry is None:
return False
from tools.mcp_tool_common import _parse_boolish
return _parse_boolish(entry.get("enabled", True), default=True)
from tools.mcp_tool_common import mcp_server_enabled
return mcp_server_enabled(entry)
except Exception:
return True