From 5f18767528bd56ae8c9ab74f0d29f9a2b8f811d7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?L=C3=A9o?= <leograndcontact@gmail.com>
Date: Wed, 30 Sep 2026 17:15:52 +0200
Subject: [PATCH] fix(mcp): show the route's rejection reason on the
 Integrations form too
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The route now answers 400 with a message naming the expected shape.
admin.js was taught to print `data.detail`; the Unified Integrations
form in settings.js still printed `Failed (400)` and dropped it.

That gap is exactly where the new validation bites. The client-side
JSON.parse guard added alongside it catches unparseable input, so the
only values that reach the route's 400 are ones that parse but are not
a list — `"npx"`, `{}`, `null` — and for those the status code alone
tells the user nothing about what is wrong with what they typed.

Adds source-level coverage for both forms; the PR changed two JS files
with no test on either.
---
 static/js/settings.js                        |  7 ++++-
 tests/test_mcp_add_server_args_validation.py | 27 ++++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/static/js/settings.js b/static/js/settings.js
index 431545975..c59b751e1 100644
--- a/static/js/settings.js
+++ b/static/js/settings.js
@@ -5128,7 +5128,12 @@ async function initUnifiedIntegrations() {
           } else if (r.ok) {
             el('uf-mcp-msg').textContent = 'Saved'; formEl.style.display = 'none'; await renderList();
           } else {
-            el('uf-mcp-msg').textContent = `Failed (${r.status})`;
+            // Surface the server's reason. The Args validation above rejects
+            // unparseable JSON, but `"x"` and `{}` parse and are refused by
+            // routes/mcp/mcp_routes.py with a message naming the expected
+            // shape; a bare status code sends the user looking in the wrong
+            // place. Matches what admin.js shows for the same endpoint.
+            el('uf-mcp-msg').textContent = data.detail || `Failed (${r.status})`;
           }
         } catch (_) { el('uf-mcp-msg').textContent = 'Failed'; }
         finally { _setBtnLoading(saveBtn, false, _origLabel); if (cancelBtn) cancelBtn.disabled = false; }
diff --git a/tests/test_mcp_add_server_args_validation.py b/tests/test_mcp_add_server_args_validation.py
index 3550c85dc..7550a99a0 100644
--- a/tests/test_mcp_add_server_args_validation.py
+++ b/tests/test_mcp_add_server_args_validation.py
@@ -8,6 +8,7 @@ subprocess at all, with no error surfaced anywhere.
 """
 import asyncio
 import json
+from pathlib import Path
 from unittest.mock import AsyncMock, MagicMock
 
 import pytest
@@ -145,3 +146,29 @@ def test_add_server_still_defaults_empty_args_to_empty_list(monkeypatch):
 
     assert result["connected"] is True
     assert manager.connect_server.call_args.kwargs["args"] == []
+
+
+# ---------------------------------------------------------------------------
+# The two forms that post to this endpoint
+# ---------------------------------------------------------------------------
+#
+# The route now answers 400 with a message naming the expected shape. That is
+# only worth anything if the form the user is looking at prints it, and the two
+# forms did not agree: admin.js reads `data.detail`, settings.js printed the
+# bare status code. Read as source, because the artifact under test is the
+# string in the file and neither form is reachable without a browser.
+
+_REPO = Path(__file__).resolve().parents[1]
+
+
+def test_admin_form_reports_the_reason_the_route_gave():
+    source = (_REPO / "static" / "js" / "admin.js").read_text(encoding="utf-8")
+    assert "msg.textContent = data.detail || `Failed (${res.status})`;" in source
+
+
+def test_unified_integrations_form_reports_the_reason_the_route_gave():
+    source = (_REPO / "static" / "js" / "settings.js").read_text(encoding="utf-8")
+    assert (
+        "el('uf-mcp-msg').textContent = data.detail || `Failed (${r.status})`;"
+        in source
+    ), "settings.js drops the route's message and prints only the status code"