diff --git a/src/api/servers.test.ts b/src/api/servers.test.ts index 09f1cca9..dee7741b 100644 --- a/src/api/servers.test.ts +++ b/src/api/servers.test.ts @@ -497,6 +497,20 @@ describe("serversApi", () => { expect(result).toEqual({ id: "get-evict" }); expect(mockFetch).toHaveBeenCalledTimes(2); }); + + it("invalidateGetCache drops the cached entry so the next get refetches", async () => { + mockFetch.mockResolvedValueOnce(jsonResponse({ id: "get-invalidate", enabled: true })); + const before = await serversApi.get("get-invalidate"); + expect(before).toEqual({ id: "get-invalidate", enabled: true }); + + serversApi.invalidateGetCache("get-invalidate"); + + mockFetch.mockResolvedValueOnce(jsonResponse({ id: "get-invalidate", enabled: false })); + const after = await serversApi.get("get-invalidate"); + + expect(after).toEqual({ id: "get-invalidate", enabled: false }); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); }); describe("testConnection", () => { diff --git a/src/api/servers.ts b/src/api/servers.ts index 44ec17ea..8c8a4012 100644 --- a/src/api/servers.ts +++ b/src/api/servers.ts @@ -103,6 +103,16 @@ export const serversApi = { return request; }, + /** + * Drop the cached single-server request so the next `get` hits the network. + * + * Call this after a mutation (activate/deactivate/refresh) before re-reading + * the server, otherwise `get` would return the pre-mutation snapshot. + */ + invalidateGetCache: (id: string): void => { + serverByIdRequestCache.delete(validateServerId(id)); + }, + /** * Delete an MCP server */ diff --git a/src/pages/Servers.test.tsx b/src/pages/Servers.test.tsx index b94e3675..f67534ea 100644 --- a/src/pages/Servers.test.tsx +++ b/src/pages/Servers.test.tsx @@ -616,7 +616,7 @@ describe("Servers", () => { }); }); - it("calls refetch and clears error when toggleEnabled succeeds", async () => { + it("patches only the toggled server into the list instead of refetching", async () => { const user = userEvent.setup(); vi.mocked(api.get).mockResolvedValueOnce({ @@ -630,12 +630,12 @@ describe("Servers", () => { expect(screen.getByText("Test Server 0")).toBeInTheDocument(); }); - vi.mocked(api.post).mockResolvedValueOnce({}); + vi.mocked(api.post).mockResolvedValueOnce({ status: "success", message: "ok" }); - // api.get should be called again for refetch + // Single-server fetch for the targeted patch (not a list refetch). vi.mocked(api.get).mockResolvedValueOnce({ - gateways: createMockServers(0, 1), - nextCursor: null, + ...createMockServers(0, 1)[0], + enabled: false, }); const actionsButtons = screen.getAllByRole("button", { name: /actions for/i }); @@ -648,9 +648,18 @@ describe("Servers", () => { expect(api.post).toHaveBeenCalledWith(expect.stringContaining("activate=false")); }); + // The page re-reads just the one server: one list fetch + one single-server fetch. await waitFor(() => { - expect(api.get).toHaveBeenCalledTimes(2); // Initial fetch + refetch + expect(api.get).toHaveBeenCalledWith("/v1/mcp-servers/server-0"); }); + expect(api.get).toHaveBeenCalledTimes(2); + + // The row stays mounted (no spinner swap) and reflects the new state. + expect(screen.getByText("Test Server 0")).toBeInTheDocument(); + expect(screen.queryByRole("status", { name: /loading/i })).not.toBeInTheDocument(); + const reopenedButtons = screen.getAllByRole("button", { name: /actions for/i }); + await user.click(reopenedButtons[0]); + expect(await screen.findByRole("menuitem", { name: /activate/i })).toBeInTheDocument(); }); it("no longer offers Test Connection in the actions menu (moved to the details drawer)", async () => { @@ -1132,10 +1141,10 @@ describe("Servers", () => { durationMs: 100, refreshedAt: "2025-09-09T00:00:00Z", }); - // Refetch after a successful refresh. + // Targeted patch after a successful refresh: re-read just the one server. vi.mocked(api.get).mockResolvedValueOnce({ - gateways: createMockServers(0, 1), - nextCursor: null, + ...createMockServers(0, 1)[0], + toolCount: 8, }); const actionsButtons = screen.getAllByRole("button", { name: /actions for/i }); @@ -1154,6 +1163,54 @@ describe("Servers", () => { }); }); + it("patches the refreshed server's counts without refetching the list", async () => { + const user = userEvent.setup(); + + vi.mocked(api.get).mockResolvedValueOnce({ + gateways: [{ ...createMockServers(0, 1)[0], toolCount: 5 }], + nextCursor: null, + }); + + renderWithRouter(); + + await waitFor(() => { + expect(screen.getByText("5 tools")).toBeInTheDocument(); + }); + + vi.mocked(api.post).mockResolvedValueOnce({ + gatewayId: "server-0", + success: true, + toolsAdded: 3, + toolsUpdated: 0, + toolsRemoved: 0, + durationMs: 100, + refreshedAt: "2025-09-09T00:00:00Z", + }); + // Re-read just the one server after refresh. + vi.mocked(api.get).mockResolvedValueOnce({ + ...createMockServers(0, 1)[0], + toolCount: 8, + }); + + const actionsButtons = screen.getAllByRole("button", { name: /actions for/i }); + await user.click(actionsButtons[0]); + + const refreshItem = await screen.findByRole("menuitem", { name: /^refresh$/i }); + await user.click(refreshItem); + + await waitFor(() => { + expect(api.get).toHaveBeenCalledWith("/v1/mcp-servers/server-0"); + }); + // One list fetch + one single-server fetch: no whole-list refetch. + expect(api.get).toHaveBeenCalledTimes(2); + + // The row stays mounted (no spinner swap) and shows the new counts. + await waitFor(() => { + expect(screen.getByText("8 tools")).toBeInTheDocument(); + }); + expect(screen.queryByRole("status", { name: /loading/i })).not.toBeInTheDocument(); + }); + it("shows a warning toast when refresh returns success: true with validationErrors", async () => { const user = userEvent.setup(); diff --git a/src/pages/Servers.tsx b/src/pages/Servers.tsx index 67065a7b..7c6c2a78 100644 --- a/src/pages/Servers.tsx +++ b/src/pages/Servers.tsx @@ -123,6 +123,24 @@ export function Servers() { // Convert query error to string for display const error = queryError ? queryError.message : null; + // Patch a single server into local state (and the query cache) instead of + // refetching the whole list: a refetch() remounts the table behind the + // page-level isLoading spinner, which reads as flicker. Same approach as + // Tools.tsx handleToggleTool. + const patchServer = useCallback( + (updated: MCPServer) => { + setAllServers((prev) => prev.map((s) => (s.id === updated.id ? updated : s))); + // Keep the query cache aligned so the refetch useEffect doesn't restore + // the pre-mutation row. + setQueryData((prev) => + prev + ? { ...prev, gateways: prev.gateways.map((s) => (s.id === updated.id ? updated : s)) } + : prev, + ); + }, + [setQueryData], + ); + const handleEdit = (id: string) => { setUpdateServerId(id); setIsFormOpen(true); @@ -197,14 +215,22 @@ export function Servers() { setToggleError(null); try { await serversApi.toggleEnabled(id, enabled); - await refetch(); + try { + // Refresh only the toggled server and patch it into the list, instead + // of refetching the whole page (which remounts the table and flickers). + serversApi.invalidateGetCache(id); + const updated = await serversApi.get(id); + patchServer(updated); + } catch (refreshErr) { + console.error("Failed to refresh server after toggle:", sanitizeError(refreshErr)); + } } catch (err) { const errorMsg = sanitizeError(err); setToggleError(errorMsg); console.error("Failed to toggle server state:", errorMsg); } }, - [refetch], + [patchServer], ); const handleAuthorize = useCallback( @@ -316,7 +342,11 @@ export function Servers() { toast.success(successMessage); } try { - await refetch(); + // Patch just the refreshed server into the list instead of refetching + // the whole page (which remounts the table and flickers). + serversApi.invalidateGetCache(id); + const updated = await serversApi.get(id); + patchServer(updated); } catch (refreshErr) { console.error("Failed to refresh servers after refresh:", sanitizeError(refreshErr)); } @@ -340,7 +370,7 @@ export function Servers() { }); } }, - [refreshingServerIds, allServers, intl, refetch], + [refreshingServerIds, allServers, intl, patchServer], ); const handleViewDetails = useCallback((id: string) => {