fix(ui): move MCP env-var loading to React Query to drop lint suppressions

The frontend-lint baseline this branch had grown carried three
react-hooks/set-state-in-effect entries and four raw-fetch entries that were
added rather than fixed. Load the per-user env-var data in UserEnvVarsModal and
mcp_servers through React Query (useQuery/useMutation) so the setState-in-effect
findings go away instead of being baselined, and capture the ?fill_env_vars deep
link in lazy initial state so the modal target is derived during render rather
than set from an effect

Delete the unused clearMCPUserEnvVars wrapper, the one new networking fetch with
no caller. The remaining three wrappers (GET status, GET server vars, POST save)
still need a raw fetch because networking.tsx is the only HTTP layer and React
Query consumes it, so they stay grandfathered; the baseline only ratchets down:
UserEnvVarsModal 1->0, mcp_servers set-state 4->2, networking fetch 274->273
This commit is contained in:
mateo-berri 2026-06-04 16:00:42 +00:00
parent 3ac1c00aa0
commit 67e81f794f
No known key found for this signature in database
5 changed files with 73 additions and 115 deletions

View file

@ -1307,11 +1307,6 @@
"count": 1
}
},
"src/components/mcp_tools/UserEnvVarsModal.tsx": {
"react-hooks/set-state-in-effect": {
"count": 1
}
},
"src/components/mcp_tools/create_mcp_server.tsx": {
"no-restricted-imports": {
"count": 1
@ -1382,7 +1377,7 @@
"count": 1
},
"react-hooks/set-state-in-effect": {
"count": 4
"count": 2
}
},
"src/components/mcp_tools/mcp_tool_configuration.tsx": {
@ -1509,7 +1504,7 @@
"count": 23
},
"no-restricted-syntax": {
"count": 274
"count": 273
}
},
"src/components/object_permissions_view.tsx": {

View file

@ -1,5 +1,6 @@
import React, { useEffect, useState } from "react";
import React from "react";
import { Modal, Form, Input, Button, Alert, Spin, Tag, Typography } from "antd";
import { useMutation, useQuery } from "@tanstack/react-query";
import { MCPServer, MCPUserEnvVarsStatus } from "./types";
import { getMCPUserEnvVars, storeMCPUserEnvVars } from "../networking";
import NotificationsManager from "../molecules/notifications_manager";
@ -23,59 +24,41 @@ interface UserEnvVarsModalProps {
*/
const UserEnvVarsModal: React.FC<UserEnvVarsModalProps> = ({ server, open, accessToken, onClose, onSaved }) => {
const [form] = Form.useForm();
const [status, setStatus] = useState<MCPUserEnvVarsStatus | null>(null);
const [isLoading, setIsLoading] = useState(false);
const [isSaving, setIsSaving] = useState(false);
useEffect(() => {
if (!open || !server || !accessToken) {
return;
}
let cancelled = false;
setIsLoading(true);
(async () => {
try {
const fetched = await getMCPUserEnvVars(accessToken, server.server_id);
if (cancelled) return;
setStatus(fetched);
form.resetFields();
} catch (err) {
if (!cancelled) {
NotificationsManager.fromBackend(
`Failed to load env vars: ${err instanceof Error ? err.message : String(err)}`,
);
}
} finally {
if (!cancelled) setIsLoading(false);
}
})();
return () => {
cancelled = true;
};
}, [open, server, accessToken, form]);
const {
data: status,
isLoading,
isError,
} = useQuery<MCPUserEnvVarsStatus>({
queryKey: ["mcpUserEnvVars", server?.server_id],
queryFn: () => getMCPUserEnvVars(accessToken!, server!.server_id),
enabled: open && !!server && !!accessToken,
});
const handleSave = async (values: Record<string, string>) => {
if (!server || !accessToken) return;
setIsSaving(true);
try {
const trimmed: Record<string, string> = {};
for (const [k, v] of Object.entries(values)) {
trimmed[k] = (v ?? "").trim();
}
const saved = await storeMCPUserEnvVars(accessToken, server.server_id, trimmed);
setStatus(saved);
const saveMutation = useMutation({
mutationFn: (values: Record<string, string>) => storeMCPUserEnvVars(accessToken!, server!.server_id, values),
onSuccess: (saved) => {
NotificationsManager.success("Credentials saved");
if (onSaved) onSaved(saved);
onSaved?.(saved);
onClose();
} catch (err) {
},
onError: (err) => {
NotificationsManager.fromBackend(`Failed to save env vars: ${err instanceof Error ? err.message : String(err)}`);
} finally {
setIsSaving(false);
},
});
const handleSave = (values: Record<string, string>) => {
if (!server || !accessToken) return;
const trimmed: Record<string, string> = {};
for (const [k, v] of Object.entries(values)) {
trimmed[k] = (v ?? "").trim();
}
saveMutation.mutate(trimmed);
};
const displayName = server?.server_name || server?.alias || server?.server_id || "MCP Server";
const required = status?.required ?? [];
const isSaving = saveMutation.isPending;
return (
<Modal
@ -84,6 +67,9 @@ const UserEnvVarsModal: React.FC<UserEnvVarsModalProps> = ({ server, open, acces
footer={null}
width={520}
destroyOnHidden
afterOpenChange={(opened) => {
if (opened) form.resetFields();
}}
title={
<div>
<div className="flex items-center gap-2">
@ -103,6 +89,8 @@ const UserEnvVarsModal: React.FC<UserEnvVarsModalProps> = ({ server, open, acces
<div className="flex items-center justify-center py-8">
<Spin />
</div>
) : isError ? (
<Alert type="error" showIcon message="Failed to load env vars" />
) : required.length === 0 ? (
<Alert type="info" showIcon message="No per-user fields configured for this server." />
) : (

View file

@ -15,6 +15,7 @@ vi.mock("../networking", () => ({
getGeneralSettingsCall: vi.fn().mockResolvedValue([]),
updateConfigFieldSetting: vi.fn().mockResolvedValue(undefined),
deleteConfigFieldSetting: vi.fn().mockResolvedValue(undefined),
listMCPUserEnvVarStatus: vi.fn().mockResolvedValue([]),
}));
// Mock NotificationsManager
@ -213,7 +214,7 @@ describe("MCPServers", () => {
vi.mocked(networking.fetchMCPServers).mockResolvedValue(mockServers);
// Mock health check to never resolve (to test loading state)
vi.mocked(networking.fetchMCPServerHealth).mockImplementation(
() => new Promise(() => { }), // Never resolves
() => new Promise(() => {}), // Never resolves
);
const queryClient = createQueryClient();
@ -330,9 +331,7 @@ describe("MCPServers", () => {
// Find and click on "Team A" option
const dropdownOptions = document.querySelectorAll(".ant-select-item-option");
const teamAOption = Array.from(dropdownOptions).find((option) =>
option.textContent?.includes("Team A"),
);
const teamAOption = Array.from(dropdownOptions).find((option) => option.textContent?.includes("Team A"));
expect(teamAOption).toBeTruthy();
act(() => {
@ -390,9 +389,7 @@ describe("MCPServers", () => {
const oneServer = twoServers.slice(0, 1);
// First call returns two servers; second (after deletion) returns one
vi.mocked(networking.fetchMCPServers)
.mockResolvedValueOnce(twoServers)
.mockResolvedValueOnce(oneServer);
vi.mocked(networking.fetchMCPServers).mockResolvedValueOnce(twoServers).mockResolvedValueOnce(oneServer);
vi.mocked(networking.fetchMCPServerHealth).mockResolvedValue([
{ server_id: "server-1", status: "healthy" },
{ server_id: "server-2", status: "healthy" },

View file

@ -4,6 +4,7 @@ import { Button, Tab, TabGroup, TabList, TabPanel, TabPanels, Text, Title } from
import NewBadge from "../common_components/NewBadge";
import { Descriptions, Empty, Input, Modal, Select, Spin, Tooltip, Typography } from "antd";
import React, { useEffect, useState, useMemo, useCallback } from "react";
import { useQuery } from "@tanstack/react-query";
import { useMCPServers } from "../../app/(dashboard)/hooks/mcpServers/useMCPServers";
import { useMCPServerHealth } from "../../app/(dashboard)/hooks/mcpServers/useMCPServerHealth";
import NotificationsManager from "../molecules/notifications_manager";
@ -112,63 +113,51 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID })
const [prefillData, setPrefillData] = useState<DiscoverableMCPServer | null>(null);
const [isDeletingServer, setIsDeletingServer] = useState(false);
const [byokModalServer, setByokModalServer] = useState<MCPServer | null>(null);
// Per-user env-var fill modal target + bulk status across accessible servers.
// Per-user env-var fill modal target + deep-link source captured once from the URL.
const [envVarsModalServer, setEnvVarsModalServer] = useState<MCPServer | null>(null);
const [envVarStatusByServer, setEnvVarStatusByServer] = useState<Record<string, MCPUserEnvVarsStatus>>({});
const [deepLinkServerId, setDeepLinkServerId] = useState<string | null>(() =>
typeof window === "undefined" ? null : new URLSearchParams(window.location.search).get("fill_env_vars"),
);
const [searchQuery, setSearchQuery] = useState<string>("");
const [sortKey, setSortKey] = useState<SortKey>("created_desc");
const isInternalUser = userRole === "Internal User";
// Single bulk fetch of this user's per-server env-var status. Drives the
// red "N user fields missing" footer on each card with no per-row request.
const refetchEnvVarStatus = useCallback(async () => {
if (!accessToken) {
setEnvVarStatusByServer({});
return;
}
try {
const statuses = await listMCPUserEnvVarStatus(accessToken);
const map: Record<string, MCPUserEnvVarsStatus> = {};
for (const s of statuses) {
map[s.server_id] = s;
}
setEnvVarStatusByServer(map);
} catch (err) {
console.warn("Failed to load MCP env-var status", err);
}
}, [accessToken]);
useEffect(() => {
refetchEnvVarStatus();
}, [refetchEnvVarStatus, mcpServers]);
const { data: envVarStatuses, refetch: refetchEnvVarStatus } = useQuery<MCPUserEnvVarsStatus[]>({
queryKey: ["mcpUserEnvVarStatus"],
queryFn: () => listMCPUserEnvVarStatus(accessToken!),
enabled: !!accessToken,
});
// Per-server list of per-user fields this user still needs to fill in.
const missingFieldsByServer = useMemo(() => {
const map: Record<string, string[]> = {};
for (const [serverId, status] of Object.entries(envVarStatusByServer)) {
map[serverId] = (status.required ?? []).filter((spec) => !spec.is_set).map((spec) => spec.name);
for (const status of envVarStatuses ?? []) {
map[status.server_id] = (status.required ?? []).filter((spec) => !spec.is_set).map((spec) => spec.name);
}
return map;
}, [envVarStatusByServer]);
}, [envVarStatuses]);
// Deep-link via ?fill_env_vars=<server_id> — the link users follow from the
// friendly error the proxy returns when a per-user var is missing. Opens the
// fill modal for the matching server, then strips the param.
// friendly error the proxy returns when a per-user var is missing. The id is
// captured into state above and resolved to a server below; here we only strip
// the param so a refresh doesn't reopen the modal.
useEffect(() => {
if (typeof window === "undefined") return;
if (!serversWithHealth || serversWithHealth.length === 0) return;
if (!deepLinkServerId || typeof window === "undefined") return;
const params = new URLSearchParams(window.location.search);
const targetId = params.get("fill_env_vars");
if (!targetId) return;
const match = serversWithHealth.find((s) => s.server_id === targetId);
if (match) {
setEnvVarsModalServer(match);
params.delete("fill_env_vars");
const newSearch = params.toString();
const newUrl = window.location.pathname + (newSearch ? `?${newSearch}` : "") + window.location.hash;
window.history.replaceState({}, "", newUrl);
}
}, [serversWithHealth]);
if (!params.has("fill_env_vars")) return;
params.delete("fill_env_vars");
const newSearch = params.toString();
const newUrl = window.location.pathname + (newSearch ? `?${newSearch}` : "") + window.location.hash;
window.history.replaceState({}, "", newUrl);
}, [deepLinkServerId]);
const deepLinkServer = useMemo(
() => (deepLinkServerId ? serversWithHealth.find((s) => s.server_id === deepLinkServerId) ?? null : null),
[deepLinkServerId, serversWithHealth],
);
const activeEnvVarsServer = envVarsModalServer ?? deepLinkServer;
useEffect(() => {
if (typeof window === "undefined") {
@ -653,10 +642,13 @@ const MCPServers: React.FC<MCPServerProps> = ({ accessToken, userRole, userID })
{/* Per-user env-var fill modal — backed by /v1/mcp/server/{id}/user-env-vars */}
<UserEnvVarsModal
server={envVarsModalServer}
open={!!envVarsModalServer}
server={activeEnvVarsServer}
open={!!activeEnvVarsServer}
accessToken={accessToken}
onClose={() => setEnvVarsModalServer(null)}
onClose={() => {
setEnvVarsModalServer(null);
setDeepLinkServerId(null);
}}
onSaved={() => {
// Refresh the bulk status so the red "N user fields missing" footer
// on each card clears once the user has filled in their values.

View file

@ -10124,20 +10124,6 @@ export const storeMCPUserEnvVars = async (
return response.json();
};
export const clearMCPUserEnvVars = async (accessToken: string, serverId: string): Promise<MCPUserEnvVarsStatus> => {
const url = proxyBaseUrl
? `${proxyBaseUrl}/v1/mcp/server/${serverId}/user-env-vars`
: `/v1/mcp/server/${serverId}/user-env-vars`;
const response = await fetch(url, {
method: "DELETE",
headers: { [globalLitellmHeaderName]: `Bearer ${accessToken}` },
});
if (!response.ok) {
throw new Error("Failed to clear env vars");
}
return response.json();
};
export const listMCPUserEnvVarStatus = async (accessToken: string): Promise<MCPUserEnvVarsStatus[]> => {
const url = proxyBaseUrl ? `${proxyBaseUrl}/v1/mcp/user-env-vars/status` : `/v1/mcp/user-env-vars/status`;
const response = await fetch(url, {