Skip to content

Commit 41fe68b

Browse files
authored
fix(mcp): make agentic MCP server removal actually delete the row (#14272)
remove_agentic_mcp_server called update_server without delete=True under a comment claiming "Empty config removes the server". That has never been true: the flag-less call is a full replace, so users who had the langflow-agentic server were left with an empty-config row, and users who never had it gained a brand-new empty-config row (the create path runs when no row exists) — a broken entry in their MCP servers list either way. The same behavior existed under the old file-based store (the entry was set to {}). Pass delete=True so the row is actually removed. For users without the server, update_server raises HTTPException, which the existing per-user except/continue block already handles. The helper currently has no callers (the agentic-experience disable path was never wired up), so nothing ships the broken behavior today; this fixes the latent utility and pins the semantics with tests before anyone wires it up.
1 parent edfe9dc commit 41fe68b

2 files changed

Lines changed: 110 additions & 2 deletions

File tree

src/backend/base/langflow/api/utils/mcp/agentic_mcp.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,14 +151,18 @@ async def remove_agentic_mcp_server(session: AsyncSession) -> None:
151151

152152
for user in users:
153153
try:
154-
# Remove the server by passing empty config
154+
# An empty config without delete=True would *replace* the config (or even
155+
# create an empty-config row for users who never had the server), leaving a
156+
# broken entry in the server list. delete=True removes the row; a user
157+
# without the server raises HTTPException, caught below.
155158
await update_server(
156159
server_name=server_name,
157-
server_config={}, # Empty config removes the server
160+
server_config={},
158161
current_user=user,
159162
session=session,
160163
storage_service=storage_service,
161164
settings_service=settings_service,
165+
delete=True,
162166
)
163167

164168
servers_removed += 1
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
"""Tests for the agentic MCP server removal helper.
2+
3+
``remove_agentic_mcp_server`` previously called ``update_server`` without
4+
``delete=True`` under a comment claiming "Empty config removes the server". That
5+
was never true: the call *replaced* the config with ``{}`` for users who had the
6+
server, and — worse — *created* an empty-config ``langflow-agentic`` row for
7+
users who never had it (the flag-less call takes the create path when no row
8+
exists). These tests pin the corrected behavior against a real SQLite DB.
9+
"""
10+
11+
import uuid
12+
from types import SimpleNamespace
13+
from unittest.mock import MagicMock, patch
14+
15+
import langflow.services.database.models # noqa: F401 (register SQLModel tables)
16+
import pytest
17+
from langflow.api.utils.mcp.agentic_mcp import remove_agentic_mcp_server
18+
from langflow.api.v2.mcp import get_server_list, update_server
19+
from langflow.services.database.models import MCPServer
20+
from langflow.services.database.models.user.model import User
21+
from sqlalchemy.ext.asyncio import create_async_engine
22+
from sqlmodel import SQLModel, select
23+
from sqlmodel.ext.asyncio.session import AsyncSession
24+
from sqlmodel.pool import StaticPool
25+
26+
# _clear_server_cache() calls the shared component cache service, which isn't booted
27+
# in a bare unit test; no-op it (orthogonal to the behavior under test).
28+
CACHE_PATCH = {
29+
"get_shared_component_cache_service": MagicMock(return_value=SimpleNamespace()),
30+
"safe_cache_get": MagicMock(return_value={}),
31+
"safe_cache_set": MagicMock(),
32+
}
33+
34+
# remove_agentic_mcp_server resolves storage/settings services at call time; neither
35+
# is used by the DB-backed update_server, so plain mocks suffice.
36+
SERVICE_PATCH = {
37+
"get_service": MagicMock(return_value=MagicMock()),
38+
"get_settings_service": MagicMock(return_value=MagicMock()),
39+
}
40+
41+
42+
async def _engine():
43+
engine = create_async_engine("sqlite+aiosqlite://", connect_args={"check_same_thread": False}, poolclass=StaticPool)
44+
async with engine.begin() as conn:
45+
await conn.run_sync(SQLModel.metadata.create_all)
46+
return engine
47+
48+
49+
async def _add_user(session: AsyncSession, username: str) -> User:
50+
user = User(id=uuid.uuid4(), username=username, password="pw", is_active=True) # noqa: S106
51+
session.add(user)
52+
await session.commit()
53+
return user
54+
55+
56+
@pytest.mark.asyncio
57+
async def test_remove_agentic_mcp_server_removes_row():
58+
"""Removal deletes the langflow-agentic row and leaves other servers untouched."""
59+
engine = await _engine()
60+
61+
with patch.multiple("langflow.api.v2.mcp", **CACHE_PATCH):
62+
async with AsyncSession(engine, expire_on_commit=False) as session:
63+
user = await _add_user(session, "agentic_user")
64+
await update_server(
65+
"langflow-agentic",
66+
{"command": "python", "args": ["-m", "langflow.agentic.mcp"]},
67+
user,
68+
session,
69+
None,
70+
None,
71+
)
72+
await update_server("other", {"url": "https://example.com/mcp"}, user, session, None, None)
73+
74+
with patch.multiple("langflow.api.utils.mcp.agentic_mcp", **SERVICE_PATCH):
75+
await remove_agentic_mcp_server(session)
76+
77+
servers = (await get_server_list(user, session, None, None))["mcpServers"]
78+
79+
await engine.dispose()
80+
assert "langflow-agentic" not in servers, f"agentic server must be removed, got {sorted(servers)}"
81+
assert "other" in servers, "removal must not touch unrelated servers"
82+
assert servers["other"] == {"url": "https://example.com/mcp"}
83+
84+
85+
@pytest.mark.asyncio
86+
async def test_remove_agentic_mcp_server_absent_is_noop():
87+
"""A user without the server must not gain an empty-config row, and removal must not raise.
88+
89+
The pre-fix flag-less call created a `langflow-agentic` row with `{}` config for such
90+
users (create path of update_server), surfacing a broken entry in the servers list.
91+
"""
92+
engine = await _engine()
93+
94+
with patch.multiple("langflow.api.v2.mcp", **CACHE_PATCH):
95+
async with AsyncSession(engine, expire_on_commit=False) as session:
96+
user = await _add_user(session, "no_server_user")
97+
98+
with patch.multiple("langflow.api.utils.mcp.agentic_mcp", **SERVICE_PATCH):
99+
await remove_agentic_mcp_server(session)
100+
101+
rows = (await session.exec(select(MCPServer).where(MCPServer.user_id == user.id))).all()
102+
103+
await engine.dispose()
104+
assert rows == [], f"no row may be created for a user who never had the server, got {rows}"

0 commit comments

Comments
 (0)