Skip to content

Commit 6818b87

Browse files
committed
fix(mcp): re-create when a full replace races a concurrent delete
The merge-PATCH path already turns a row deleted between its read and its guarded UPDATE into a clean 404. The full-replace path never checked rowcount: the same race made its unguarded UPDATE match nothing, the loop broke, and the request returned None with the write silently lost. Check rowcount after the replace UPDATE; on a miss, re-read the row and loop. A still-missing row takes the create path on the next iteration, so the replace converges to a re-create (fresh row, version 1) instead of a silent no-op. Covered by a race-injection test in the same style as test_patch_losing_to_concurrent_delete_returns_404.
1 parent 6774157 commit 6818b87

2 files changed

Lines changed: 56 additions & 2 deletions

File tree

src/backend/base/langflow/api/v2/mcp.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ async def update_server(
506506
# (version = version + 1) so it stays strictly monotonic even when our ORM copy
507507
# is stale. An ORM `+= 1` off a stale read could reuse a version a concurrent
508508
# PATCH already consumed, letting a later guarded PATCH pass its version check.
509-
await session.execute(
509+
replaced = await session.execute(
510510
update(MCPServer)
511511
.where(MCPServer.id == existing.id)
512512
.values(
@@ -517,7 +517,16 @@ async def update_server(
517517
)
518518
)
519519
await session.commit()
520-
break
520+
if replaced.rowcount == 1:
521+
break
522+
# Row deleted under us: re-read, and a still-missing row takes the create path
523+
# next iteration — the replace converges to a re-create instead of silently
524+
# returning None with the write lost.
525+
session.expire(existing)
526+
result = await session.exec(
527+
select(MCPServer).where(MCPServer.user_id == user_id, MCPServer.name == server_name)
528+
)
529+
existing = result.first()
521530
else:
522531
raise HTTPException(status_code=409, detail="MCP server was updated concurrently; please retry.")
523532

src/backend/tests/unit/api/v2/test_mcp_db_store.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,3 +444,48 @@ def derive_then_advance(config):
444444
await engine.dispose()
445445
assert row.version == 6, f"replace must bump DB-side (5 + 1 = 6), got {row.version} (stale/reused version)"
446446
assert row.config.get("replaced") == "yes", "replace must overwrite the config"
447+
448+
449+
@pytest.mark.asyncio
450+
async def test_replace_losing_to_concurrent_delete_recreates(tmp_path):
451+
"""A full replace whose row is deleted between its read and its UPDATE re-creates the row.
452+
453+
Without the rowcount check the unguarded UPDATE matches nothing, the loop breaks, and the
454+
request returns None with the write silently lost (200, no row). With it, the re-read finds
455+
no row and the next iteration takes the create path, so the replace converges to a re-create.
456+
"""
457+
engine = await _file_engine(tmp_path / "mcp.db")
458+
db_path = str(tmp_path / "mcp.db")
459+
user = SimpleNamespace(id=uuid.uuid4())
460+
461+
import langflow.api.v2.mcp as mcp_mod
462+
463+
real_derive = mcp_mod._derive_transport
464+
raced = {"done": False}
465+
466+
def derive_then_delete(config):
467+
# The replace calls _derive_transport between its read and its unguarded UPDATE;
468+
# delete the row right there (raw sqlite = synchronous) so the UPDATE matches 0 rows.
469+
if not raced["done"]:
470+
raced["done"] = True
471+
con = sqlite3.connect(db_path, timeout=30)
472+
con.execute("DELETE FROM mcp_server WHERE name = 'svc'")
473+
con.commit()
474+
con.close()
475+
return real_derive(config)
476+
477+
with patch.multiple("langflow.api.v2.mcp", **CACHE_PATCH):
478+
async with AsyncSession(engine, expire_on_commit=False) as session:
479+
await update_server("svc", {"url": "https://x", "old": "1"}, user, session, None, None)
480+
with patch.object(mcp_mod, "_derive_transport", derive_then_delete):
481+
async with AsyncSession(engine, expire_on_commit=False) as session:
482+
result = await update_server("svc", {"url": "https://y", "replaced": "yes"}, user, session, None, None)
483+
async with AsyncSession(engine, expire_on_commit=False) as session:
484+
row = (await session.exec(select(MCPServer).where(MCPServer.name == "svc"))).first()
485+
486+
await engine.dispose()
487+
assert row is not None, "replace raced by a delete must re-create the row, not silently no-op"
488+
assert row.version == 1, f"re-created row starts fresh at version 1, got {row.version}"
489+
assert row.config.get("replaced") == "yes", "re-created row must carry the replace's config"
490+
assert row.config.get("old") is None, "re-created row must not resurrect the deleted config"
491+
assert result == {"url": "https://y", "replaced": "yes"}, f"replace must return the written config, got {result!r}"

0 commit comments

Comments
 (0)