fix(mcp): banner shows 'disabled' not 'failed' for enabled:false servers (#38204)

get_mcp_status() treated every non-connected server as a failure, so a
server configured with enabled: false rendered as red '— failed' in the
startup banner even though it was intentionally off. Add a 'disabled'
field derived from the enabled flag and render disabled servers dim as
'— disabled' instead.
This commit is contained in:
Teknium
2026-06-03 05:41:13 -07:00
committed by GitHub
parent ef65298103
commit 1d90b23982
3 changed files with 45 additions and 0 deletions

View File

@ -626,6 +626,11 @@ def build_welcome_banner(console: "Console", model: str, cwd: str,
f"[dim {dim}]{srv['name']}[/] [{text}]({srv['transport']})[/] "
f"[dim {dim}]—[/] [{text}]{srv['tools']} tool(s)[/]"
)
elif srv.get("disabled"):
right_lines.append(
f"[dim {dim}]{srv['name']}[/] [dim]({srv['transport']})[/] "
f"[dim {dim}]— disabled[/]"
)
else:
right_lines.append(
f"[red]{srv['name']}[/] [dim]({srv['transport']})[/] "

View File

@ -133,3 +133,37 @@ def test_build_welcome_banner_title_falls_back_when_no_tag():
raw = buf.getvalue()
assert "Hermes Agent v" in raw, "Version label missing from title"
assert "\x1b]8;" not in raw, "OSC-8 hyperlink should not be emitted without a tag"
def test_build_welcome_banner_disabled_mcp_shows_disabled_not_failed():
"""A disabled MCP server renders '— disabled' (dim), not '— failed' (red)."""
with (
patch.object(model_tools, "check_tool_availability", return_value=(["web"], [])),
patch.object(banner, "get_available_skills", return_value={}),
patch.object(banner, "get_update_result", return_value=None),
patch.object(
tools.mcp_tool,
"get_mcp_status",
return_value=[
{"name": "linear", "transport": "http", "tools": 0,
"connected": False, "disabled": True},
{"name": "broken", "transport": "stdio", "tools": 0,
"connected": False, "disabled": False},
],
),
):
console = Console(record=True, force_terminal=False, color_system=None, width=160)
banner.build_welcome_banner(
console=console, model="anthropic/test-model", cwd="/tmp/project",
tools=[{"function": {"name": "read_file"}}],
get_toolset_for_tool=lambda n: "file",
)
output = console.export_text()
# Disabled server is labeled "disabled", not "failed"
assert "linear" in output
assert "disabled" in output
# A genuinely unreachable server still reads "failed"
assert "broken" in output
assert "failed" in output

View File

@ -3666,6 +3666,7 @@ def get_mcp_status() -> List[dict]:
for name, cfg in configured.items():
transport = cfg.get("transport", "http") if "url" in cfg else "stdio"
enabled = _parse_boolish(cfg.get("enabled", True), default=True)
server = active_servers.get(name)
if server and server.session is not None:
entry = {
@ -3673,16 +3674,21 @@ def get_mcp_status() -> List[dict]:
"transport": transport,
"tools": len(server._registered_tool_names) if hasattr(server, "_registered_tool_names") else len(server._tools),
"connected": True,
"disabled": False,
}
if server._sampling:
entry["sampling"] = dict(server._sampling.metrics)
result.append(entry)
else:
# A server with enabled: false is intentionally not connected — it is
# disabled, not failed. Surface that distinction so consumers (banner,
# TUI) can render "disabled" rather than an alarming "failed".
result.append({
"name": name,
"transport": transport,
"tools": 0,
"connected": False,
"disabled": not enabled,
})
return result