fix(ui): read MCP submission rules from bare-array /config/list response (#44648)

* fix(ui): read MCP submission rules from bare-array /config/list response Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(mcp): cover submission rules contract and dashboard preload Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(ui): disable MCP submission rules editor until rules load Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * style(ui): format MCP submission integration test Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(e2e): restore prior MCP submission rules after the rules spec Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(e2e): move MCP submission rules setup and cleanup into fixtures Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(ui): use Promise.withResolvers in MCP rules loading test Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(ui): clear lint warnings in MCPSubmissionsTab Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * style(ui): drop redundant JSX comments in MCPSubmissionsTab Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: mrinal <mrinal@berri.ai> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2026-10-07 02:59:05 +00:00 · 2026-10-06 10:09:46 -07:00 · 2026-10-06 10:09:46 -07:00 · 090a4c3f24
commit 090a4c3f24
parent 878ba39e7c
4 changed files with 608 additions and 149 deletions
--- a/tests/e2e/ui/tests/mcp/mcpSubmissionRules.spec.ts
+++ b/tests/e2e/ui/tests/mcp/mcpSubmissionRules.spec.ts
@ -0,0 +1,134 @@
+import { test as base, expect } from "@playwright/test";
+import { ADMIN_STORAGE_PATH } from "../../constants";
+import { deleteMcpServerByName } from "../../helpers/mcp";
+import { navigateToPage } from "../../helpers/navigation";
+import { masterKey, rootPath, uniqueSuffix } from "../../helpers/traffic";
+import { Page } from "../../fixtures/pages";
+
+const localServerUrl = process.env.MOCK_LLM_URL ?? "http://127.0.0.1:8090/v1";
+
+interface Fixtures {
+  adminHeaders: { Authorization: string; "Content-Type": string };
+  submissionRules: string[];
+  teamKey: string;
+  submittedServerName: string;
+}
+
+const test = base.extend<Fixtures>({
+  adminHeaders: async ({}, use) => {
+    await use({
+      Authorization: `Bearer ${masterKey()}`,
+      "Content-Type": "application/json",
+    });
+  },
+  submissionRules: async ({ adminHeaders, page }, use) => {
+    const settingsResponse = await page.request.get(`${rootPath()}/config/list?config_type=general_settings`, {
+      headers: adminHeaders,
+    });
+    expect(settingsResponse.status(), await settingsResponse.text()).toBe(200);
+    const settings = (await settingsResponse.json()) as { field_name?: string; field_value?: unknown; stored_in_db?: boolean }[];
+    const row = settings.find((item) => item.field_name === "mcp_required_fields");
+    const previousRules = row?.stored_in_db === true ? (row.field_value ?? null) : null;
+    const rules = ["description", "url", "alias"];
+    const rulesResponse = await page.request.post(`${rootPath()}/config/field/update`, {
+      headers: adminHeaders,
+      data: { field_name: "mcp_required_fields", field_value: rules, config_type: "general_settings" },
+    });
+    expect(rulesResponse.status(), await rulesResponse.text()).toBe(200);
+
+    await use(rules);
+
+    const restoreResponse = Array.isArray(previousRules)
+      ? await page.request.post(`${rootPath()}/config/field/update`, {
+          headers: adminHeaders,
+          data: {
+            field_name: "mcp_required_fields",
+            field_value: previousRules,
+            config_type: "general_settings",
+          },
+        })
+      : await page.request.post(`${rootPath()}/config/field/delete`, {
+          headers: adminHeaders,
+          data: { field_name: "mcp_required_fields", config_type: "general_settings" },
+        });
+    expect(restoreResponse.status(), await restoreResponse.text()).toBe(200);
+  },
+  teamKey: async ({ adminHeaders, page }, use) => {
+    const keyResponse = await page.request.post(`${rootPath()}/key/generate`, {
+      headers: adminHeaders,
+      data: { team_id: "e2e-team-crud", user_id: "e2e-internal-user" },
+    });
+    expect(keyResponse.status(), await keyResponse.text()).toBe(200);
+    const key = (await keyResponse.json()) as { key: string; token?: string; token_id?: string };
+
+    await use(key.key);
+
+    const deleteResponse = await page.request.post(`${rootPath()}/key/delete`, {
+      headers: adminHeaders,
+      data: { keys: [key.token ?? key.token_id ?? key.key] },
+    });
+    expect(deleteResponse.status(), await deleteResponse.text()).toBe(200);
+  },
+  submittedServerName: async ({ submissionRules, teamKey, page }, use) => {
+    void submissionRules;
+    const name = `e2e_rules_${uniqueSuffix().replaceAll("-", "")}`;
+    const registration = await page.request.post(`${rootPath()}/v1/mcp/server/register`, {
+      headers: {
+        "x-litellm-api-key": teamKey,
+        "Content-Type": "application/json",
+      },
+      data: {
+        server_name: name,
+        alias: name,
+        description: "Submitted from the MCP submission rules E2E test",
+        url: localServerUrl,
+        transport: "http",
+      },
+    });
+    expect(registration.status(), await registration.text()).toBe(201);
+
+    await use(name);
+
+    await deleteMcpServerByName(page, name);
+  },
+});
+
+test.use({ storageState: ADMIN_STORAGE_PATH });
+
+test("Submission Rules panel shows saved rules, preloads the editor, and save keeps existing rules", async ({ page, adminHeaders, submissionRules, submittedServerName }) => {
+  await navigateToPage(page, Page.McpServers);
+  await page.getByRole("tab", { name: "Submitted MCPs" }).click();
+
+  await expect(page.getByText("(3 required fields)", { exact: true })).toBeVisible();
+  await expect(page.getByText("no rules set", { exact: true })).toHaveCount(0);
+
+  await page.getByText("Submission Rules", { exact: true }).click();
+  await expect(page.getByRole("checkbox", { name: /^Description\b/ })).toBeChecked();
+  await expect(page.getByRole("checkbox", { name: /^Server URL\b/ })).toBeChecked();
+  await expect(page.getByRole("checkbox", { name: /^Alias\b/ })).toBeChecked();
+  await expect(page.getByRole("checkbox", { name: /^Auth configured\b/ })).not.toBeChecked();
+
+  await page.getByRole("checkbox", { name: /^Auth configured\b/ }).check();
+  await page.getByRole("button", { name: "Save Rules" }).click();
+  await expect(page.getByText("Submission rules saved", { exact: true })).toBeVisible();
+
+  await expect
+    .poll(
+      async () => {
+        const response = await page.request.get(
+          `${rootPath()}/config/list?config_type=general_settings`,
+          { headers: adminHeaders },
+        );
+        if (!response.ok()) return null;
+        const settings = (await response.json()) as { field_name?: string; field_value?: unknown }[];
+        return settings.find((row) => row.field_name === "mcp_required_fields")?.field_value ?? null;
+      },
+      { timeout: 30_000 },
+    )
+    .toEqual([...submissionRules, "auth_type"]);
+
+  await page.reload();
+  await page.getByRole("tab", { name: "Submitted MCPs" }).click();
+  await expect(page.getByText(submittedServerName, { exact: true })).toBeVisible();
+  await expect(page.getByText("3 passing, 1 failing", { exact: true })).toBeVisible();
+});
--- a/tests/integration/mcp/test_mcp_submission_rules.py
+++ b/tests/integration/mcp/test_mcp_submission_rules.py
@ -0,0 +1,149 @@
+import os
+import uuid
+from collections.abc import Iterator
+from contextlib import contextmanager
+from typing import Final
+
+import httpx
+import psycopg
+from integration._support.client import JSON_OBJECT, Gateway, eventually, string_value
+from integration._support.database import read_rows
+from integration._support.mcp import McpPeer, forget_mcp, mcp_peer
+from pydantic import JsonValue, TypeAdapter
+
+_REQUIRED_FIELDS: Final = ("description", "url", "alias")
+_JSON_ROWS: Final = TypeAdapter(list[dict[str, JsonValue]])
+
+
+@contextmanager
+def _submission_rules_lock() -> Iterator[None]:
+    with psycopg.connect(os.environ["DATABASE_URL"], autocommit=True) as connection:
+        connection.execute("SELECT pg_advisory_lock(%s, %s)", (9126, 1))
+        try:
+            yield
+        finally:
+            connection.execute("SELECT pg_advisory_unlock(%s, %s)", (9126, 1))
+
+
+def _set_submission_rules(gateway: Gateway) -> httpx.Response:
+    return gateway.request(
+        "POST",
+        "/config/field/update",
+        {
+            "field_name": "mcp_required_fields",
+            "field_value": list(_REQUIRED_FIELDS),
+            "config_type": "general_settings",
+        },
+    )
+
+
+def _delete_submission_rules(gateway: Gateway) -> None:
+    response: Final = gateway.request(
+        "POST",
+        "/config/field/delete",
+        {"field_name": "mcp_required_fields", "config_type": "general_settings"},
+    )
+    assert response.status_code == 200, response.text
+
+
+def _registration_body(peer: McpPeer, alias: str, description: str | None = None) -> dict[str, JsonValue]:
+    registration: Final = JSON_OBJECT.validate_python(peer.registration())
+    return {
+        "server_name": alias,
+        "alias": alias,
+        **registration,
+        **({"description": description} if description is not None else {}),
+    }
+
+
+def _submit_registration(
+    gateway: Gateway,
+    peer: McpPeer,
+    team_key: str,
+    alias: str,
+    description: str | None = None,
+) -> httpx.Response:
+    return gateway.client.post(
+        "/v1/mcp/server/register",
+        json=_registration_body(peer, alias, description),
+        headers={"x-litellm-api-key": team_key},
+    )
+
+
+def _submission_items(response: httpx.Response) -> list[dict[str, JsonValue]]:
+    summary: Final = JSON_OBJECT.validate_json(response.content)
+    return _JSON_ROWS.validate_python(summary["items"])
+
+
+def test_saved_submission_rules_are_listed_as_a_top_level_array(gateway: Gateway) -> None:
+    with _submission_rules_lock(), gateway.scenario() as scenario:
+        updated: Final = _set_submission_rules(gateway)
+        scenario.cleanups.callback(_delete_submission_rules, gateway)
+        assert updated.status_code == 200, updated.text
+
+        listed: Final = gateway.request("GET", "/config/list", params={"config_type": "general_settings"})
+        assert listed.status_code == 200, listed.text
+        payload: Final = TypeAdapter(JsonValue).validate_json(listed.content)
+        assert isinstance(payload, list), listed.text
+        rows: Final = _JSON_ROWS.validate_python(payload)
+        matching: Final = tuple(row for row in rows if row.get("field_name") == "mcp_required_fields")
+        assert len(matching) == 1, listed.text
+        assert matching[0]["field_value"] == list(_REQUIRED_FIELDS), listed.text
+        assert matching[0]["stored_in_db"] is True, listed.text
+
+        stored: Final = read_rows(
+            'SELECT param_value -> \'mcp_required_fields\' AS field_value FROM "LiteLLM_Config" '
+            "WHERE param_name = %s",
+            ("general_settings",),
+        )
+        assert stored == [{"field_value": list(_REQUIRED_FIELDS)}]
+
+
+def test_team_submission_missing_a_required_field_is_rejected_and_complete_one_is_pending(
+    gateway: Gateway,
+) -> None:
+    with mcp_peer() as peer, _submission_rules_lock(), gateway.scenario() as scenario:
+        updated: Final = _set_submission_rules(gateway)
+        scenario.cleanups.callback(_delete_submission_rules, gateway)
+        assert updated.status_code == 200, updated.text
+
+        team_id: Final = scenario.team()
+        team_key: Final = scenario.key(team_id=team_id)
+
+        def submit_without_description() -> tuple[httpx.Response, str]:
+            alias: Final = f"mcp_rules_{uuid.uuid4().hex}"
+            response: Final = _submit_registration(gateway, peer, team_key, alias)
+            if response.status_code == 201:
+                accepted: Final = JSON_OBJECT.validate_json(response.content)
+                scenario.cleanups.callback(forget_mcp, gateway, string_value(accepted["server_id"]))
+            return response, alias
+
+        missing: Final = eventually(
+            submit_without_description,
+            lambda result: result[0].status_code == 400
+            and "Submission is missing required fields: ['description']" in result[0].text,
+            seconds=30,
+        )
+        missing_response, missing_alias = missing
+        assert missing_response.status_code == 400, missing_response.text
+        assert "Submission is missing required fields: ['description']" in missing_response.text
+        assert (
+            read_rows('SELECT server_id FROM "LiteLLM_MCPServerTable" WHERE alias = %s', (missing_alias,)) == []
+        )
+
+        alias: Final = f"mcp_rules_{uuid.uuid4().hex}"
+        registered: Final = _submit_registration(gateway, peer, team_key, alias, "An integration MCP server")
+        assert registered.status_code == 201, registered.text
+        submission: Final = JSON_OBJECT.validate_json(registered.content)
+        server_id: Final = string_value(submission["server_id"])
+        scenario.cleanups.callback(forget_mcp, gateway, server_id)
+
+        listed: Final = eventually(
+            lambda: gateway.request("GET", "/v1/mcp/server/submissions"),
+            lambda response: response.status_code == 200 and server_id in response.text,
+        )
+        assert listed.status_code == 200, listed.text
+        assert any(
+            item.get("server_id") == server_id and item.get("approval_status") == "pending_review"
+            for item in _submission_items(listed)
+        ), listed.text
--- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPSubmissionsTab.integration.test.tsx
+++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPSubmissionsTab.integration.test.tsx
@ -0,0 +1,141 @@
+import { act, render, screen, waitFor } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import type { MCPServer, MCPSubmissionsSummary } from "@/components/mcp_tools/types";
+import { fetchMCPSubmissions, getGeneralSettingsCall, updateConfigFieldSetting } from "@/components/networking";
+import { MCPSubmissionsTab } from "./MCPSubmissionsTab";
+
+vi.mock("@/components/networking", () => ({
+  approveMCPServer: vi.fn(),
+  fetchMCPSubmissions: vi.fn(),
+  getGeneralSettingsCall: vi.fn(),
+  rejectMCPServer: vi.fn(),
+  updateConfigFieldSetting: vi.fn(),
+}));
+
+vi.mock("@/lib/toast", () => ({
+  toast: { success: vi.fn(), fromError: vi.fn() },
+}));
+
+const token = "token";
+
+const pendingSubmission: MCPServer = {
+  server_id: "submission-1",
+  server_name: "submission",
+  alias: "Submission server",
+  description: "A server with required details",
+  url: "https://example.com/mcp",
+  transport: "http",
+  auth_type: "none",
+  approval_status: "pending_review",
+  created_at: "2026-01-01T00:00:00Z",
+  created_by: "user",
+  updated_at: "2026-01-01T00:00:00Z",
+  updated_by: "user",
+};
+
+const submissionSummary: MCPSubmissionsSummary = {
+  total: 1,
+  pending_review: 1,
+  active: 0,
+  rejected: 0,
+  items: [pendingSubmission],
+};
+
+const configuredRules = [
+  {
+    field_name: "mcp_required_fields",
+    field_type: "List",
+    field_value: ["description", "url", "alias"],
+    field_description: "Required MCP submission fields",
+    stored_in_db: true,
+  },
+];
+
+describe("MCPSubmissionsTab", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    vi.mocked(fetchMCPSubmissions).mockResolvedValue(submissionSummary);
+    vi.mocked(getGeneralSettingsCall).mockResolvedValue(configuredRules);
+    vi.mocked(updateConfigFieldSetting).mockResolvedValue(undefined);
+  });
+
+  it("renders configured submission rules and card compliance checks", async () => {
+    render(<MCPSubmissionsTab accessToken={token} />);
+
+    expect(await screen.findByText("(3 required fields)")).toBeInTheDocument();
+    expect(screen.queryByText("no rules set")).not.toBeInTheDocument();
+    expect(screen.getByText("3 passing, 0 failing")).toBeInTheDocument();
+    expect(screen.getAllByText("Passes")).toHaveLength(3);
+  });
+
+  it("preloads the rules editor and preserves them when adding a field", async () => {
+    const user = userEvent.setup();
+    render(<MCPSubmissionsTab accessToken={token} />);
+
+    await user.click(await screen.findByText("Submission Rules"));
+
+    expect(screen.getByRole("checkbox", { name: /Description/ })).toBeChecked();
+    expect(screen.getByRole("checkbox", { name: /Server URL/ })).toBeChecked();
+    expect(screen.getByRole("checkbox", { name: /Alias/ })).toBeChecked();
+    const authTypeCheckbox = screen.getByRole("checkbox", { name: /Auth configured/ });
+    expect(authTypeCheckbox).not.toBeChecked();
+
+    await user.click(authTypeCheckbox);
+    await user.click(screen.getByRole("button", { name: "Save Rules" }));
+
+    await waitFor(() =>
+      expect(updateConfigFieldSetting).toHaveBeenCalledWith("token", "mcp_required_fields", [
+        "description",
+        "url",
+        "alias",
+        "auth_type",
+      ]),
+    );
+  });
+
+  it("keeps the rules editor disabled until saved rules load", async () => {
+    const user = userEvent.setup();
+    const { promise: settingsPromise, resolve: resolveSettings } =
+      Promise.withResolvers<Awaited<ReturnType<typeof getGeneralSettingsCall>>>();
+    vi.mocked(getGeneralSettingsCall).mockReturnValue(settingsPromise);
+
+    render(<MCPSubmissionsTab accessToken={token} />);
+    await user.click(screen.getByText("Submission Rules"));
+
+    const descriptionCheckbox = screen.getByRole("checkbox", { name: /Description/ });
+    const serverUrlCheckbox = screen.getByRole("checkbox", { name: /Server URL/ });
+    const aliasCheckbox = screen.getByRole("checkbox", { name: /Alias/ });
+    const authTypeCheckbox = screen.getByRole("checkbox", { name: /Auth configured/ });
+    const saveRulesButton = screen.getByRole("button", { name: "Save Rules" });
+
+    expect(descriptionCheckbox).toBeDisabled();
+    expect(serverUrlCheckbox).toBeDisabled();
+    expect(aliasCheckbox).toBeDisabled();
+    expect(authTypeCheckbox).toBeDisabled();
+    expect(saveRulesButton).toBeDisabled();
+    expect(screen.getByRole("button", { name: "Cancel" })).toBeEnabled();
+
+    await act(async () => {
+      resolveSettings(configuredRules);
+    });
+
+    expect(descriptionCheckbox).toBeEnabled();
+    expect(serverUrlCheckbox).toBeEnabled();
+    expect(aliasCheckbox).toBeEnabled();
+    expect(authTypeCheckbox).toBeEnabled();
+    expect(descriptionCheckbox).toBeChecked();
+    expect(serverUrlCheckbox).toBeChecked();
+    expect(aliasCheckbox).toBeChecked();
+    expect(authTypeCheckbox).not.toBeChecked();
+    expect(saveRulesButton).toBeEnabled();
+  });
+
+  it("shows no rules when the general settings array is empty", async () => {
+    vi.mocked(getGeneralSettingsCall).mockResolvedValue([]);
+
+    render(<MCPSubmissionsTab accessToken={token} />);
+
+    expect(await screen.findByText("no rules set")).toBeInTheDocument();
+  });
+});
--- a/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPSubmissionsTab.tsx
+++ b/ui/litellm-dashboard/src/app/(dashboard)/mcp-servers/_components/MCPSubmissionsTab.tsx
@ -19,6 +19,7 @@ import {
  updateConfigFieldSetting,
 } from "@/components/networking";
 import { MCPServer, MCPSubmissionsSummary } from "@/components/mcp_tools/types";
+import type { generalSettingsItem } from "@/app/(dashboard)/router-settings/_components/general_settings";
 import { FIELD_GROUPS, MCP_REQUIRED_FIELD_DEFS, SETTINGS_KEY } from "./MCPStandardsSettings";
 import { toast } from "@/lib/toast";

@ -45,6 +46,14 @@ const STATUS_CONFIG: Record<MCPStatus, { label: string; bg: string; text: string
  },
 };

+const EMPTY_SUMMARY: MCPSubmissionsSummary = {
+  total: 0,
+  pending_review: 0,
+  active: 0,
+  rejected: 0,
+  items: [],
+};
+
 function formatDate(value: string | null | undefined): string {
  if (!value) return "—";
  try {
@ -141,9 +150,10 @@ type SubmissionRulesPanelProps = {
  onChange: (fields: string[]) => void;
  onSave: () => Promise<void>;
  isSaving: boolean;
+  disabled: boolean;
 };

-function SubmissionRulesPanel({ requiredFields, onChange, onSave, isSaving }: SubmissionRulesPanelProps) {
+function SubmissionRulesPanel({ requiredFields, onChange, onSave, isSaving, disabled }: SubmissionRulesPanelProps) {
  const [expanded, setExpanded] = useState(false);
  const activeLabels = MCP_REQUIRED_FIELD_DEFS.filter((f) => requiredFields.includes(f.key));

@ -153,7 +163,6 @@ function SubmissionRulesPanel({ requiredFields, onChange, onSave, isSaving }: Su

  return (
    <div className="mb-5 border border-border rounded-lg bg-card overflow-hidden">
-      {/* Header — always visible */}
      <div
        className="flex items-center justify-between px-4 py-3 cursor-pointer select-none"
        onClick={() => setExpanded((v) => !v)}
@ -170,7 +179,6 @@ function SubmissionRulesPanel({ requiredFields, onChange, onSave, isSaving }: Su
          )}
        </div>
        <div className="flex items-center gap-3">
-          {/* Active rule chips — collapsed view */}
          {!expanded && activeLabels.length > 0 && (
            <div className="flex flex-wrap gap-1.5 max-w-md">
              {activeLabels.map((f) => (
@ -192,7 +200,6 @@ function SubmissionRulesPanel({ requiredFields, onChange, onSave, isSaving }: Su
        </div>
      </div>

-      {/* Expanded editor */}
      {expanded && (
        <div className="border-t border-border px-4 pt-4 pb-4">
          <p className="text-xs text-muted-foreground mb-4">
@ -214,6 +221,7 @@ function SubmissionRulesPanel({ requiredFields, onChange, onSave, isSaving }: Su
                          type="checkbox"
                          checked={active}
                          onChange={() => toggle(field.key)}
+                          disabled={disabled}
                          className="mt-0.5 h-4 w-4 rounded-sm border-border text-info focus:ring-ring cursor-pointer"
                        />
                        <div>
@ -232,7 +240,7 @@ function SubmissionRulesPanel({ requiredFields, onChange, onSave, isSaving }: Su
          <div className="mt-5 flex items-center gap-3">
            <button
              type="button"
-              disabled={isSaving}
+              disabled={isSaving || disabled}
              onClick={async () => {
                await onSave();
                setExpanded(false);
@ -262,23 +270,164 @@ type MCPServerCardProps = {
  requiredFields: string[];
 };

-function MCPServerCard({ server, onApprove, onReject, requiredFields }: MCPServerCardProps) {
-  const approvalStatus = (server.approval_status ?? "active") as MCPStatus;
-  const statusCfg = STATUS_CONFIG[approvalStatus] ?? STATUS_CONFIG["active"];
+type ComplianceCheck = {
+  key: string;
+  label: string;
+  description: string;
+  passed: boolean;
+};

-  const checks = MCP_REQUIRED_FIELD_DEFS.filter((f) => requiredFields.includes(f.key)).map((f) => ({
-    key: f.key,
-    label: f.label,
-    description: f.description,
-    passed: f.check(server),
-  }));
+type CardReviewActionsProps = {
+  approvalStatus: MCPStatus;
+  onApprove: () => void;
+  onReject: () => void;
+};
+
+function CardReviewActions({ approvalStatus, onApprove, onReject }: CardReviewActionsProps) {
+  return (
+    <>
+      {approvalStatus !== "rejected" && (
+        <div className="flex items-center gap-2 shrink-0">
+          {approvalStatus !== "active" && (
+            <button
+              type="button"
+              onClick={onApprove}
+              className="text-xs bg-success hover:bg-success/80 text-success-foreground px-3 py-1.5 rounded-md transition-colors font-medium"
+            >
+              Approve
+            </button>
+          )}
+          <button
+            type="button"
+            onClick={onReject}
+            className="text-xs border border-destructive/30 text-destructive hover:bg-destructive/10 px-3 py-1.5 rounded-md transition-colors font-medium"
+          >
+            Reject
+          </button>
+        </div>
+      )}
+      {approvalStatus === "rejected" && (
+        <div className="flex items-center gap-2 shrink-0">
+          <button
+            type="button"
+            onClick={onApprove}
+            className="text-xs bg-success hover:bg-success/80 text-success-foreground px-3 py-1.5 rounded-md transition-colors font-medium"
+          >
+            Re-approve
+          </button>
+        </div>
+      )}
+    </>
+  );
+}
+
+type ComplianceChecksPanelProps = {
+  checks: ComplianceCheck[];
+  approvalStatus: MCPStatus;
+  onApprove: () => void;
+  onReject: () => void;
+};
+
+function ComplianceChecksPanel({ checks, approvalStatus, onApprove, onReject }: ComplianceChecksPanelProps) {
  const passCount = checks.filter((c) => c.passed).length;
  const failCount = checks.length - passCount;
  const allPassed = checks.length > 0 && failCount === 0;

+  return (
+    <div className="border-t border-border">
+      <div
+        className={`flex items-center gap-3 px-4 py-3 ${
+          allPassed ? "bg-success/10 border-b border-success/15" : "bg-destructive/10 border-b border-destructive/15"
+        }`}
+      >
+        <div
+          className={`w-8 h-8 rounded-full flex items-center justify-center shrink-0 ${
+            allPassed ? "bg-success" : "bg-destructive"
+          }`}
+        >
+          {allPassed ? (
+            <CheckIcon className="h-4 w-4 text-success-foreground" />
+          ) : (
+            <XIcon className="h-4 w-4 text-destructive-foreground" />
+          )}
+        </div>
+        <div className="flex-1 min-w-0">
+          <div className={`text-sm font-semibold leading-tight ${allPassed ? "text-success" : "text-destructive"}`}>
+            {allPassed ? "All checks passed" : `${failCount} check${failCount !== 1 ? "s" : ""} failed`}
+          </div>
+          <div className="text-xs text-muted-foreground mt-0.5">
+            {passCount} passing, {failCount} failing
+          </div>
+        </div>
+        <div className="flex items-center gap-2 shrink-0">
+          {approvalStatus !== "active" && approvalStatus !== "rejected" && (
+            <button
+              type="button"
+              onClick={onApprove}
+              className="text-xs bg-success hover:bg-success/80 text-success-foreground px-3 py-1.5 rounded-md transition-colors font-medium"
+            >
+              Approve
+            </button>
+          )}
+          {approvalStatus === "rejected" && (
+            <button
+              type="button"
+              onClick={onApprove}
+              className="text-xs bg-success hover:bg-success/80 text-success-foreground px-3 py-1.5 rounded-md transition-colors font-medium"
+            >
+              Re-approve
+            </button>
+          )}
+          {approvalStatus !== "rejected" && (
+            <button
+              type="button"
+              onClick={onReject}
+              className="text-xs border border-destructive/30 text-destructive hover:bg-destructive/10 bg-card px-3 py-1.5 rounded-md transition-colors font-medium"
+            >
+              Reject
+            </button>
+          )}
+        </div>
+      </div>
+
+      <div className="divide-y divide-border">
+        {checks.map((c) => (
+          <div key={c.key} className="flex items-center gap-3 px-4 py-2.5">
+            <div
+              className={`w-5 h-5 rounded-full flex items-center justify-center shrink-0 ${
+                c.passed ? "bg-success/15" : "bg-destructive/15"
+              }`}
+            >
+              {c.passed ? (
+                <CheckIcon className="h-3 w-3 text-success" />
+              ) : (
+                <XIcon className="h-3 w-3 text-destructive" />
+              )}
+            </div>
+            <span className={`text-sm flex-1 ${c.passed ? "text-foreground" : "text-foreground"}`}>{c.label}</span>
+            <span className={`text-xs ${c.passed ? "text-success" : "text-destructive"}`}>
+              {c.passed ? "Passes" : "Missing"}
+            </span>
+          </div>
+        ))}
+      </div>
+    </div>
+  );
+}
+
+function MCPServerCard({ server, onApprove, onReject, requiredFields }: MCPServerCardProps) {
+  const approvalStatus = (server.approval_status ?? "active") as MCPStatus;
+  const statusCfg = STATUS_CONFIG[approvalStatus] ?? STATUS_CONFIG["active"];
+
+  const checks: ComplianceCheck[] = MCP_REQUIRED_FIELD_DEFS.filter((f) => requiredFields.includes(f.key)).map((f) => ({
+    key: f.key,
+    label: f.label,
+    description: f.description,
+    passed: f.check(server),
+  }));
+
  return (
    <div className="bg-card border border-border rounded-lg overflow-hidden">
-      {/* Server info */}
      <div className="px-4 pt-4 pb-3">
        <div className="flex items-start justify-between gap-4">
          <div className="flex-1 min-w-0">
@ -317,128 +466,19 @@ function MCPServerCard({ server, onApprove, onReject, requiredFields }: MCPServe
              <p className="text-xs text-destructive mt-1.5">Rejection reason: {server.review_notes}</p>
            )}
          </div>
-          {/* Approve/Reject when no checks panel (no rules configured) */}
-          {checks.length === 0 && approvalStatus !== "rejected" && (
-            <div className="flex items-center gap-2 shrink-0">
-              {approvalStatus !== "active" && (
-                <button
-                  type="button"
-                  onClick={onApprove}
-                  className="text-xs bg-success hover:bg-success/80 text-success-foreground px-3 py-1.5 rounded-md transition-colors font-medium"
-                >
-                  Approve
-                </button>
-              )}
-              <button
-                type="button"
-                onClick={onReject}
-                className="text-xs border border-destructive/30 text-destructive hover:bg-destructive/10 px-3 py-1.5 rounded-md transition-colors font-medium"
-              >
-                Reject
-              </button>
-            </div>
-          )}
-          {checks.length === 0 && approvalStatus === "rejected" && (
-            <div className="flex items-center gap-2 shrink-0">
-              <button
-                type="button"
-                onClick={onApprove}
-                className="text-xs bg-success hover:bg-success/80 text-success-foreground px-3 py-1.5 rounded-md transition-colors font-medium"
-              >
-                Re-approve
-              </button>
-            </div>
+          {checks.length === 0 && (
+            <CardReviewActions approvalStatus={approvalStatus} onApprove={onApprove} onReject={onReject} />
          )}
        </div>
      </div>

-      {/* GitHub-style checks panel */}
      {checks.length > 0 && (
-        <div className="border-t border-border">
-          {/* Overall status header */}
-          <div
-            className={`flex items-center gap-3 px-4 py-3 ${
-              allPassed
-                ? "bg-success/10 border-b border-success/15"
-                : "bg-destructive/10 border-b border-destructive/15"
-            }`}
-          >
-            {/* Large status circle */}
-            <div
-              className={`w-8 h-8 rounded-full flex items-center justify-center shrink-0 ${
-                allPassed ? "bg-success" : "bg-destructive"
-              }`}
-            >
-              {allPassed ? (
-                <CheckIcon className="h-4 w-4 text-success-foreground" />
-              ) : (
-                <XIcon className="h-4 w-4 text-destructive-foreground" />
-              )}
-            </div>
-            <div className="flex-1 min-w-0">
-              <div className={`text-sm font-semibold leading-tight ${allPassed ? "text-success" : "text-destructive"}`}>
-                {allPassed ? "All checks passed" : `${failCount} check${failCount !== 1 ? "s" : ""} failed`}
-              </div>
-              <div className="text-xs text-muted-foreground mt-0.5">
-                {passCount} passing, {failCount} failing
-              </div>
-            </div>
-            {/* Approve / Reject in header */}
-            <div className="flex items-center gap-2 shrink-0">
-              {approvalStatus !== "active" && approvalStatus !== "rejected" && (
-                <button
-                  type="button"
-                  onClick={onApprove}
-                  className="text-xs bg-success hover:bg-success/80 text-success-foreground px-3 py-1.5 rounded-md transition-colors font-medium"
-                >
-                  Approve
-                </button>
-              )}
-              {approvalStatus === "rejected" && (
-                <button
-                  type="button"
-                  onClick={onApprove}
-                  className="text-xs bg-success hover:bg-success/80 text-success-foreground px-3 py-1.5 rounded-md transition-colors font-medium"
-                >
-                  Re-approve
-                </button>
-              )}
-              {approvalStatus !== "rejected" && (
-                <button
-                  type="button"
-                  onClick={onReject}
-                  className="text-xs border border-destructive/30 text-destructive hover:bg-destructive/10 bg-card px-3 py-1.5 rounded-md transition-colors font-medium"
-                >
-                  Reject
-                </button>
-              )}
-            </div>
-          </div>
-
-          {/* Individual check rows */}
-          <div className="divide-y divide-border">
-            {checks.map((c) => (
-              <div key={c.key} className="flex items-center gap-3 px-4 py-2.5">
-                {/* Small circle icon */}
-                <div
-                  className={`w-5 h-5 rounded-full flex items-center justify-center shrink-0 ${
-                    c.passed ? "bg-success/15" : "bg-destructive/15"
-                  }`}
-                >
-                  {c.passed ? (
-                    <CheckIcon className="h-3 w-3 text-success" />
-                  ) : (
-                    <XIcon className="h-3 w-3 text-destructive" />
-                  )}
-                </div>
-                <span className={`text-sm flex-1 ${c.passed ? "text-foreground" : "text-foreground"}`}>{c.label}</span>
-                <span className={`text-xs ${c.passed ? "text-success" : "text-destructive"}`}>
-                  {c.passed ? "Passes" : "Missing"}
-                </span>
-              </div>
-            ))}
-          </div>
-        </div>
+        <ComplianceChecksPanel
+          checks={checks}
+          approvalStatus={approvalStatus}
+          onApprove={onApprove}
+          onReject={onReject}
+        />
      )}
    </div>
  );
@ -449,13 +489,7 @@ interface MCPSubmissionsTabProps {
 }

 export function MCPSubmissionsTab({ accessToken }: MCPSubmissionsTabProps) {
-  const [summary, setSummary] = useState<MCPSubmissionsSummary>({
-    total: 0,
-    pending_review: 0,
-    active: 0,
-    rejected: 0,
-    items: [],
-  });
+  const [summary, setSummary] = useState<MCPSubmissionsSummary>(EMPTY_SUMMARY);
  const [search, setSearch] = useState("");
  const [statusFilter, setStatusFilter] = useState<"all" | MCPStatus>("all");
  const [confirmAction, setConfirmAction] = useState<{
@ -485,10 +519,8 @@ export function MCPSubmissionsTab({ accessToken }: MCPSubmissionsTabProps) {
        }),
      ]);
      setSummary(res);
-      if (settings?.data && Array.isArray(settings.data)) {
-        const row = settings.data.find(
-          (r: { field_name: string; field_value: unknown }) => r.field_name === SETTINGS_KEY,
-        );
+      if (Array.isArray(settings)) {
+        const row = settings.find((r: generalSettingsItem) => r.field_name === SETTINGS_KEY);
        if (row && Array.isArray(row.field_value)) {
          setRequiredFields(row.field_value as string[]);
        }
@ -528,6 +560,8 @@ export function MCPSubmissionsTab({ accessToken }: MCPSubmissionsTabProps) {
    return true;
  });

+  const showEmptyState = !isLoading && !error && filtered.length === 0;
+
  async function handleApprove(serverId: string, serverName: string) {
    if (!accessToken) return;
    try {
@ -556,12 +590,12 @@ export function MCPSubmissionsTab({ accessToken }: MCPSubmissionsTabProps) {

  return (
    <div className="p-6">
-      {/* Submission Rules panel */}
      <SubmissionRulesPanel
        requiredFields={requiredFields}
        onChange={setRequiredFields}
        onSave={handleSaveRules}
        isSaving={isSavingRules}
+        disabled={isLoading}
      />

      <div className="grid grid-cols-4 gap-4 mb-6">
@ -597,7 +631,7 @@ export function MCPSubmissionsTab({ accessToken }: MCPSubmissionsTabProps) {
      <div className="space-y-3">
        {isLoading && <div className="text-center py-12 text-muted-foreground text-sm">Loading submissions…</div>}
        {error && <div className="text-center py-12 text-destructive text-sm">{error}</div>}
-        {!isLoading && !error && filtered.length === 0 && (
+        {showEmptyState && (
          <div className="text-center py-12 text-muted-foreground text-sm">
            No MCP server submissions match your filters.
          </div>
@ -616,14 +650,15 @@ export function MCPSubmissionsTab({ accessToken }: MCPSubmissionsTabProps) {
                  action: "approve",
                })
              }
-              onReject={() =>
-                setConfirmAction({
+              onReject={() => {
+                const rejectAction: NonNullable<typeof confirmAction> = {
                  serverId: server.server_id,
                  serverName: server.alias ?? server.server_name ?? server.server_id,
                  action: "reject",
                  isCurrentlyActive: server.approval_status === "active",
-                })
-              }
+                };
+                setConfirmAction(rejectAction);
+              }}
            />
          ))}
      </div>