From 2ce7820d8cfc556ecb7d687a38c84ccf29966185 Mon Sep 17 00:00:00 2001 From: vedantlavale Date: Fri, 16 Oct 2026 08:28:37 +0530 Subject: [PATCH] fix(ui): show single loading state in agent switcher Hold the agents read until namespaces are known, combine both reads into one loading state, and report failures as errors instead of an empty cluster. Closes #3217 Signed-off-by: vedantlavale --- .../components/agent/AgentSwitcher.test.tsx | 174 ++++++++++++++++++ ui/src/components/agent/AgentSwitcher.tsx | 14 +- 2 files changed, 183 insertions(+), 5 deletions(-) create mode 100644 ui/src/components/agent/AgentSwitcher.test.tsx diff --git a/ui/src/components/agent/AgentSwitcher.test.tsx b/ui/src/components/agent/AgentSwitcher.test.tsx new file mode 100644 index 000000000..6c0d27d33 --- /dev/null +++ b/ui/src/components/agent/AgentSwitcher.test.tsx @@ -0,0 +1,174 @@ +import { render, screen } from "@testing-library/react"; +import { ThemeProvider } from "@emotion/react"; +import { MemoryRouter } from "react-router-dom"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { Agent, ApiResource, NamespaceResponse } from "@/api"; +import { ApiError } from "@/api/ApiError"; +import { themeFor } from "@/theme/theme"; +import { AgentSwitcher } from "./AgentSwitcher"; + +const { useNamespaces, useAgentsAcrossNamespaces } = vi.hoisted(() => ({ + useNamespaces: vi.fn(), + useAgentsAcrossNamespaces: vi.fn(), +})); + +vi.mock("@/api", () => ({ + useNamespaces, + useAgentsAcrossNamespaces, + harnessRefName: (agent: Agent) => agent.resource.spec.harnessRef.name, +})); + +type Definitions = { agents: Agent[]; refused: { namespace: string; reason: string }[] }; + +const namespaces: NamespaceResponse[] = [{ name: "kagent", status: "Active" }]; + +function agent(namespace: string, name: string, harness: string): Agent { + return { + ref: `${namespace}/${name}`, + namespace, + name, + resource: { + metadata: { name }, + spec: { template: { ref: { name: "assistant" } }, harnessRef: { name: harness } }, + }, + }; +} + +const CURRENT = agent("kagent", "k8s", "default"); +const OTHER = agent("kagent", "reporting", "default"); + +function loaded(data: T): ApiResource { + return { + data, + isLoading: false, + isValidating: false, + error: undefined, + isEmpty: Array.isArray(data) ? data.length === 0 : false, + refresh: async () => {}, + }; +} + +function reading(): ApiResource { + return { + data: undefined, + isLoading: true, + isValidating: true, + error: undefined, + isEmpty: false, + refresh: async () => {}, + }; +} + +/** A read held back because its key is not ready: SWR reports this with isLoading false. */ +function held(): ApiResource { + return { + data: undefined, + isLoading: false, + isValidating: false, + error: undefined, + isEmpty: false, + refresh: async () => {}, + }; +} + +function failing(message: string): ApiResource { + return { + ...held(), + error: new ApiError(message, { kind: "http", url: "kagent.test/Read", status: 500 }), + }; +} + +function renderSwitcher() { + return render( + + + {}} /> + + , + ); +} + +afterEach(() => { + vi.clearAllMocks(); +}); + +describe("AgentSwitcher", () => { + it("holds the agents read back until the namespaces are known", () => { + useNamespaces.mockReturnValue(reading()); + useAgentsAcrossNamespaces.mockReturnValue(held()); + + renderSwitcher(); + + expect(useAgentsAcrossNamespaces).toHaveBeenCalledWith(undefined); + }); + + it("shows a placeholder while the namespaces are read, not an empty cluster", () => { + useNamespaces.mockReturnValue(reading()); + useAgentsAcrossNamespaces.mockReturnValue(held()); + + renderSwitcher(); + + expect(screen.getAllByTestId("agent-switcher-loading").length).toBeGreaterThan(0); + expect(screen.queryByTestId("agent-switcher-empty")).toBeNull(); + }); + + it("keeps the placeholder while the agents are read", () => { + useNamespaces.mockReturnValue(loaded(namespaces)); + useAgentsAcrossNamespaces.mockReturnValue(reading()); + + renderSwitcher(); + + expect(screen.getAllByTestId("agent-switcher-loading").length).toBeGreaterThan(0); + expect(screen.queryByTestId("agent-switcher-empty")).toBeNull(); + }); + + it("lists the other agents once both reads land, without the one you are on", () => { + useNamespaces.mockReturnValue(loaded(namespaces)); + useAgentsAcrossNamespaces.mockReturnValue( + loaded({ agents: [CURRENT, OTHER], refused: [] }), + ); + + renderSwitcher(); + + expect(screen.getByTestId(`agent-switcher-option-${OTHER.name}`)).toBeInTheDocument(); + expect(screen.queryByTestId(`agent-switcher-option-${CURRENT.name}`)).toBeNull(); + expect(screen.queryByTestId("agent-switcher-loading")).toBeNull(); + expect(screen.queryByTestId("agent-switcher-empty")).toBeNull(); + }); + + it("says the cluster is empty only after both reads have said so", () => { + useNamespaces.mockReturnValue(loaded(namespaces)); + useAgentsAcrossNamespaces.mockReturnValue(loaded({ agents: [], refused: [] })); + + renderSwitcher(); + + expect(screen.getByTestId("agent-switcher-empty")).toHaveTextContent( + "No other agents on this cluster.", + ); + expect(screen.queryByTestId("agent-switcher-loading")).toBeNull(); + }); + + it("reports a failed agents read as a failure, not as an empty cluster", () => { + useNamespaces.mockReturnValue(loaded(namespaces)); + useAgentsAcrossNamespaces.mockReturnValue(failing("the read failed")); + + renderSwitcher(); + + expect(screen.getByTestId("agent-switcher-error")).toHaveTextContent( + "Could not list agents. the read failed", + ); + expect(screen.queryByTestId("agent-switcher-empty")).toBeNull(); + }); + + it("reports a failed namespaces read rather than an empty cluster", () => { + useNamespaces.mockReturnValue(failing("no namespaces")); + useAgentsAcrossNamespaces.mockReturnValue(held()); + + renderSwitcher(); + + expect(screen.getByTestId("agent-switcher-error")).toHaveTextContent( + "Could not list agents. no namespaces", + ); + expect(screen.queryByTestId("agent-switcher-empty")).toBeNull(); + }); +}); diff --git a/ui/src/components/agent/AgentSwitcher.tsx b/ui/src/components/agent/AgentSwitcher.tsx index 915fd1567..67f2d4619 100644 --- a/ui/src/components/agent/AgentSwitcher.tsx +++ b/ui/src/components/agent/AgentSwitcher.tsx @@ -45,11 +45,15 @@ export function AgentSwitcher({ const navigate = useNavigate(); const namespaces = useNamespaces(); const namespaceNames = useMemo( - () => (namespaces.data ?? []).map((entry) => entry.name), + () => namespaces.data?.map((entry) => entry.name), [namespaces.data], ); const definitions = useAgentsAcrossNamespaces(namespaceNames); + /** Either read failing means the list is unknown, not empty. */ + const readError = namespaces.error ?? definitions.error; + /** One loading state while the namespaces or the agents are on their way. */ + const isReading = namespaces.isLoading || definitions.isLoading; const agents = useMemo(() => definitions.data?.agents ?? [], [definitions.data]); const [query, setQuery] = useState(""); @@ -149,9 +153,9 @@ export function AgentSwitcher({ css={searchInputStyles(theme)} /> - {definitions.error ? ( + {readError ? ( - Could not list agents. {definitions.error.message} + Could not list agents. {readError.message} ) : null} @@ -273,7 +277,7 @@ export function AgentSwitcher({ to be. Standing in for three rows is enough to hold the shape: the panel scrolls beyond that anyway, so being wrong about the count costs nothing. */} - {definitions.isLoading + {isReading ? [0, 1, 2].map((row) => (