From 1b0162bbfb3d571461b63c126c90bcb786d65a5d Mon Sep 17 00:00:00 2001 From: Joshua Valluru <326636767+joshua-berri@users.noreply.github.com> Date: Sun, 27 Sep 2026 13:41:49 -0700 Subject: [PATCH] refactor(agents): defer optional team selector cleanup --- .../agent_management/AgentSelector.test.tsx | 29 -------------- .../agent_management/AgentSelector.tsx | 11 ++---- .../MCPServerSelector.test.tsx | 38 ------------------- .../MCPServerSelector.tsx | 14 ++----- .../src/components/team/TeamInfo.test.tsx | 4 +- .../src/components/team/TeamInfo.tsx | 10 ++--- 6 files changed, 13 insertions(+), 93 deletions(-) diff --git a/ui/litellm-dashboard/src/components/agent_management/AgentSelector.test.tsx b/ui/litellm-dashboard/src/components/agent_management/AgentSelector.test.tsx index 894af218801..ed022711ab2 100644 --- a/ui/litellm-dashboard/src/components/agent_management/AgentSelector.test.tsx +++ b/ui/litellm-dashboard/src/components/agent_management/AgentSelector.test.tsx @@ -82,35 +82,6 @@ describe("AgentSelector", () => { expect(screen.getByRole("option", { name: /group-b/ })).toBeInTheDocument(); }); - it("offers only individual agents when legacy groups are disabled", async () => { - const user = userEvent.setup(); - const onChange = vi.fn(); - render(); - await user.click(screen.getByRole("combobox")); - await user.click(await screen.findByRole("option", { name: /Agent One/ })); - expect(screen.queryByRole("option", { name: /group-a/ })).not.toBeInTheDocument(); - expect(onChange).toHaveBeenCalledWith({ agents: ["agent-1"], accessGroups: [] }); - }); - - it("preserves a saved legacy group while adding an agent with legacy groups disabled", async () => { - const user = userEvent.setup(); - const onChange = vi.fn(); - render( - , - ); - await user.click(screen.getByRole("combobox")); - expect(await screen.findByRole("option", { name: /retired-group/ })).toHaveTextContent( - "Existing legacy agent group", - ); - await user.click(await screen.findByRole("option", { name: /Agent One/ })); - expect(onChange).toHaveBeenCalledWith({ agents: ["agent-1"], accessGroups: ["retired-group"] }); - }); - it("respects disabled prop", () => { render(); expect(screen.getByRole("combobox")).toBeDisabled(); diff --git a/ui/litellm-dashboard/src/components/agent_management/AgentSelector.tsx b/ui/litellm-dashboard/src/components/agent_management/AgentSelector.tsx index 7215bcf25e8..d26772d5c9a 100644 --- a/ui/litellm-dashboard/src/components/agent_management/AgentSelector.tsx +++ b/ui/litellm-dashboard/src/components/agent_management/AgentSelector.tsx @@ -19,7 +19,6 @@ interface AgentSelectorProps { accessToken: string; placeholder?: string; disabled?: boolean; - allowAccessGroups?: boolean; } const AgentSelector: React.FC = ({ @@ -29,7 +28,6 @@ const AgentSelector: React.FC = ({ accessToken, placeholder = "Select agents", disabled = false, - allowAccessGroups = true, }) => { const [agents, setAgents] = useState([]); const [accessGroups, setAccessGroups] = useState([]); @@ -62,15 +60,12 @@ const AgentSelector: React.FC = ({ fetchData(); }, [accessToken]); - const selectableGroups = allowAccessGroups - ? Array.from(new Set([...accessGroups, ...(value?.accessGroups ?? [])])) - : value?.accessGroups ?? []; - + // Combine options, access groups first const options: MultiSelectOption[] = [ - ...selectableGroups.map((group) => ({ + ...accessGroups.map((group) => ({ label: group, value: `group:${group}`, - description: allowAccessGroups ? "Access Group" : "Existing legacy agent group", + description: "Access Group", })), ...agents.map((agent) => ({ label: `${agent.agent_name || agent.agent_id}`, diff --git a/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.test.tsx b/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.test.tsx index 66748d3cae7..47fb5643b22 100644 --- a/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.test.tsx +++ b/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.test.tsx @@ -170,41 +170,3 @@ describe("MCPServerSelector all-proxy-mcpservers option", () => { expect(optionByLabel("Server One")).toHaveAttribute("aria-disabled", "true"); }); }); - -describe("MCPServerSelector unified group flow", () => { - beforeEach(() => { - vi.clearAllMocks(); - setupMcpMocks(); - mockUseMCPAccessGroups.mockReturnValue({ data: ["legacy-group"], isLoading: false } as ReturnType< - typeof useMCPAccessGroups - >); - }); - - it("offers servers without legacy groups when disabled", async () => { - const user = userEvent.setup(); - const onChange = vi.fn(); - renderWithProviders(); - await openSelector(user); - expect(optionByLabel("legacy-group")).toBeUndefined(); - await user.click(optionByLabel("Server One")!); - expect(onChange).toHaveBeenCalledWith({ servers: ["srv-1"], accessGroups: [], toolsets: [] }); - }); - - it("preserves a saved legacy group even if discovery no longer returns it", async () => { - const user = userEvent.setup(); - const onChange = vi.fn(); - renderWithProviders( - , - ); - await openSelector(user); - expect(optionByLabel("retired-group")).toHaveTextContent("Existing legacy MCP group"); - expect(optionByLabel("legacy-group")).toBeUndefined(); - await user.click(optionByLabel("Server One")!); - expect(onChange).toHaveBeenCalledWith({ servers: ["srv-1"], accessGroups: ["retired-group"], toolsets: [] }); - }); -}); diff --git a/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.tsx b/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.tsx index fe23fd87b76..969db57462f 100644 --- a/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.tsx +++ b/ui/litellm-dashboard/src/components/mcp_server_management/MCPServerSelector.tsx @@ -18,15 +18,11 @@ interface MCPServerSelectorProps { disabled?: boolean; teamId?: string | null; allowNoMcpServers?: boolean; - allowAccessGroups?: boolean; allowAllProxyMcpServers?: boolean; } const TOOLSET_PREFIX = "toolset:"; -const selectableLegacyGroups = (available: string[], selected: string[] = [], allowNew: boolean): string[] => - allowNew ? Array.from(new Set([...available, ...selected])) : selected; - const MCPServerSelector: React.FC = ({ onChange, value, @@ -36,24 +32,22 @@ const MCPServerSelector: React.FC = ({ disabled = false, teamId, allowNoMcpServers = false, - allowAccessGroups = true, allowAllProxyMcpServers = false, }) => { const { data: mcpServers = [], isLoading: serversLoading } = useMCPServers(teamId); const { data: accessGroups = [], isLoading: groupsLoading } = useMCPAccessGroups(); const { data: toolsets = [], isLoading: toolsetsLoading } = useMCPToolsets(); - const loading = [serversLoading, groupsLoading, toolsetsLoading].some(Boolean); + const loading = serversLoading || groupsLoading || toolsetsLoading; - const selectableGroups = selectableLegacyGroups(accessGroups, value?.accessGroups, allowAccessGroups); - const accessGroupSet = new Set(selectableGroups); + const accessGroupSet = new Set(accessGroups); // Combine options: access groups + servers + toolsets const options = [ - ...selectableGroups.map((group) => ({ + ...accessGroups.map((group) => ({ label: group, value: group, - description: allowAccessGroups ? "Access Group" : "Existing legacy MCP group", + description: "Access Group", })), ...mcpServers.map((server) => ({ label: `${server.server_name || server.server_id} (${server.server_id})`, diff --git a/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx b/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx index bbabcf2458e..a693ee971d4 100644 --- a/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamInfo.test.tsx @@ -2249,9 +2249,9 @@ describe("TeamInfoView - the exact bytes the update call sends", () => { await openEditorWithAgents(user); await user.click(within(screen.getByLabelText("agent-1")).getByRole("button")); - await user.click(within(screen.getByLabelText("group-a")).getByRole("button")); + await user.click(within(screen.getByLabelText("group:group-a")).getByRole("button")); expect(screen.queryByLabelText("agent-1")).not.toBeInTheDocument(); - expect(screen.queryByLabelText("group-a")).not.toBeInTheDocument(); + expect(screen.queryByLabelText("group:group-a")).not.toBeInTheDocument(); const payload = await save(user); diff --git a/ui/litellm-dashboard/src/components/team/TeamInfo.tsx b/ui/litellm-dashboard/src/components/team/TeamInfo.tsx index e03a08dfabd..3845f94593d 100644 --- a/ui/litellm-dashboard/src/components/team/TeamInfo.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamInfo.tsx @@ -2039,14 +2039,13 @@ const TeamInfoView: React.FC = ({ )} - + {({ value, onChange }) => ( )} @@ -2063,14 +2062,13 @@ const TeamInfoView: React.FC = ({ /> - + {({ value, onChange }) => ( )}