refactor(agents): defer optional team selector cleanup

This commit is contained in:
Joshua Valluru 2026-09-27 13:41:49 -07:00
parent d8b188207a
commit 1b0162bbfb
6 changed files with 13 additions and 93 deletions

View file

@ -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(<AgentSelector {...defaultProps} onChange={onChange} allowAccessGroups={false} />);
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(
<AgentSelector
{...defaultProps}
onChange={onChange}
allowAccessGroups={false}
value={{ agents: [], accessGroups: ["retired-group"] }}
/>,
);
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(<AgentSelector {...defaultProps} disabled />);
expect(screen.getByRole("combobox")).toBeDisabled();

View file

@ -19,7 +19,6 @@ interface AgentSelectorProps {
accessToken: string;
placeholder?: string;
disabled?: boolean;
allowAccessGroups?: boolean;
}
const AgentSelector: React.FC<AgentSelectorProps> = ({
@ -29,7 +28,6 @@ const AgentSelector: React.FC<AgentSelectorProps> = ({
accessToken,
placeholder = "Select agents",
disabled = false,
allowAccessGroups = true,
}) => {
const [agents, setAgents] = useState<Agent[]>([]);
const [accessGroups, setAccessGroups] = useState<string[]>([]);
@ -62,15 +60,12 @@ const AgentSelector: React.FC<AgentSelectorProps> = ({
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}`,

View file

@ -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(<MCPServerSelector accessToken="tok" onChange={onChange} allowAccessGroups={false} />);
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(
<MCPServerSelector
accessToken="tok"
onChange={onChange}
allowAccessGroups={false}
value={{ servers: [], accessGroups: ["retired-group"] }}
/>,
);
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: [] });
});
});

View file

@ -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<MCPServerSelectorProps> = ({
onChange,
value,
@ -36,24 +32,22 @@ const MCPServerSelector: React.FC<MCPServerSelectorProps> = ({
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})`,

View file

@ -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);

View file

@ -2039,14 +2039,13 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
)}
</FormField>
<FormField control={form.control} name="mcp_servers_and_groups" label="MCP Servers">
<FormField control={form.control} name="mcp_servers_and_groups" label="MCP Servers / Access Groups">
{({ value, onChange }) => (
<MCPServerSelector
allowAccessGroups={false}
onChange={onChange}
value={value}
accessToken={accessToken || ""}
placeholder="Select MCP servers or toolsets (optional)"
placeholder="Select MCP servers or access groups (optional)"
allowAllProxyMcpServers={is_proxy_admin}
/>
)}
@ -2063,14 +2062,13 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
/>
</div>
<FormField control={form.control} name="agents_and_groups" label="Agents">
<FormField control={form.control} name="agents_and_groups" label="Agents / Access Groups">
{({ value, onChange }) => (
<AgentSelector
allowAccessGroups={false}
onChange={onChange}
value={value}
accessToken={accessToken || ""}
placeholder="Select agents (optional)"
placeholder="Select agents or access groups (optional)"
/>
)}
</FormField>