fix(mcp): show the route's rejection reason on the Integrations form too

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.
This commit is contained in:
Léo
2026-09-30 17:15:52 +02:00
parent 01b8ac5fea
commit 5f18767528
2 changed files with 33 additions and 1 deletions
+6 -1
View File
@@ -5128,7 +5128,12 @@ async function initUnifiedIntegrations() {
} else if (r.ok) { } else if (r.ok) {
el('uf-mcp-msg').textContent = 'Saved'; formEl.style.display = 'none'; await renderList(); el('uf-mcp-msg').textContent = 'Saved'; formEl.style.display = 'none'; await renderList();
} else { } 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'; } } catch (_) { el('uf-mcp-msg').textContent = 'Failed'; }
finally { _setBtnLoading(saveBtn, false, _origLabel); if (cancelBtn) cancelBtn.disabled = false; } finally { _setBtnLoading(saveBtn, false, _origLabel); if (cancelBtn) cancelBtn.disabled = false; }
@@ -8,6 +8,7 @@ subprocess at all, with no error surfaced anywhere.
""" """
import asyncio import asyncio
import json import json
from pathlib import Path
from unittest.mock import AsyncMock, MagicMock from unittest.mock import AsyncMock, MagicMock
import pytest import pytest
@@ -145,3 +146,29 @@ def test_add_server_still_defaults_empty_args_to_empty_list(monkeypatch):
assert result["connected"] is True assert result["connected"] is True
assert manager.connect_server.call_args.kwargs["args"] == [] 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"