fix(ui): keep colliding MCP names distinct and show unrestricted and no-tool grants

The MCP selector typed only toolset values, so a stored server whose ID or alias matched an access group became that group on the next edit and widened the grant. Servers and access groups now carry their own prefixes

The customer MCP filter called unrestricted customers 'No MCP access'. It now separates limited, denied and unrestricted customers, matching the table column

Server details showed an empty tool allowlist like an unrestricted server. It now says no tools are allowed
This commit is contained in:
lewis 2026-10-17 18:27:10 -04:00
parent d15a0151de
commit c849cbc0bd
10 changed files with 145 additions and 55 deletions

View file

@ -66,7 +66,7 @@ describe("CustomersTable controls", () => {
expect(screen.getByRole("combobox", { name: "Budget" })).toHaveTextContent(/^All\b/);
await chooseSelectOption(user, screen.getByRole("combobox", { name: "Status" }), "Active");
await chooseSelectOption(user, screen.getByRole("combobox", { name: "Budget" }), "Has budget");
await chooseSelectOption(user, screen.getByRole("combobox", { name: "MCP access" }), "Has MCP access");
await chooseSelectOption(user, screen.getByRole("combobox", { name: "MCP access" }), "Limited MCP access");
expect(screen.getByRole("combobox", { name: "Budget" })).toHaveTextContent(/^Has budget\b/);
await user.click(screen.getByRole("button", { name: "Apply Filters" }));
@ -78,7 +78,7 @@ describe("CustomersTable controls", () => {
const params = onUrlUpdate.mock.calls.at(-1)?.[0].searchParams;
expect(params.get("filter_blocked")).toBe("active");
expect(params.get("filter_max_budget")).toBe("yes");
expect(params.get("filter_mcp_access")).toBe("yes");
expect(params.get("filter_mcp_access")).toBe("limited");
await user.click(screen.getByRole("button", { name: /^Filters/ }));
await user.click(screen.getByRole("button", { name: "Reset" }));
await user.click(screen.getByRole("button", { name: "Apply Filters" }));

View file

@ -22,7 +22,7 @@ import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/comp
import { cn } from "@/lib/cva.config";
import { copyToClipboard } from "@/utils/dataUtils";
import { getCustomerResourceCounts } from "./customerResources";
import { customerMcpScope, getCustomerResourceCounts } from "./customerResources";
export type CustomerTableActions = {
canModify: boolean;
@ -102,8 +102,9 @@ export function CustomerBlockAction({
const countLabel = (count: number, noun: string): string => (count ? `${count} ${noun}${count === 1 ? "" : "s"}` : "");
export function describeMcpGrant(permission: EndUser["object_permission"]): string {
const servers = permission?.mcp_servers ?? [];
if (servers.includes(NO_MCP_SERVERS_SENTINEL)) return "No MCP access";
const scope = customerMcpScope(permission);
if (scope === "denied") return "No MCP access";
if (scope === "unrestricted") return "No restriction";
const counts = getCustomerResourceCounts({ object_permission: permission });
const toolPermissionKeys = Object.keys(permission?.mcp_tool_permissions ?? {}).filter(
(server) => server !== ALL_PROXY_MCP_SERVERS_SENTINEL && server !== NO_MCP_SERVERS_SENTINEL,
@ -114,10 +115,7 @@ export function describeMcpGrant(permission: EndUser["object_permission"]): stri
countLabel(counts.toolsets, "toolset"),
countLabel(toolPermissionKeys.length, "tool limit"),
].filter(Boolean);
const hasUnsupportedAll =
servers.includes(ALL_PROXY_MCP_SERVERS_SENTINEL) ||
Object.hasOwn(permission?.mcp_tool_permissions ?? {}, ALL_PROXY_MCP_SERVERS_SENTINEL);
return parts.join(", ") || (hasUnsupportedAll ? "No MCP access" : "No restriction");
return parts.join(", ");
}
const RESOURCE_ITEMS = [

View file

@ -20,6 +20,11 @@ const permission: NonNullable<EndUser["object_permission"]> = {
vector_stores: [],
};
const MCP_SCOPES = ["limited", "denied", "unrestricted"] as const;
const scopesMatching = (row: EndUser): string[] =>
MCP_SCOPES.filter((scope) => customerMatchesFilter(row, "mcp_access", scope));
describe("customer filters", () => {
it("searches ID and alias case-insensitively and trims the search", () => {
expect(customerMatchesSearch(customer, " CUSTOMER-1 ")).toBe(true);
@ -52,34 +57,34 @@ describe("customer filters", () => {
{ mcp_tool_permissions: { "server-1": ["read"] } },
])("detects configured MCP grants %j", (grant) => {
const row = { ...customer, object_permission: { ...permission, ...grant } };
expect(customerMatchesFilter(row, "mcp_access", "yes")).toBe(true);
expect(customerMatchesFilter(row, "mcp_access", "no")).toBe(false);
expect(scopesMatching(row)).toEqual(["limited"]);
});
it("does not mistake absent, empty, or explicitly denied MCP grants for access", () => {
const denied = { ...permission, mcp_servers: [NO_MCP_SERVERS_SENTINEL], mcp_access_groups: ["group-1"] };
for (const object_permission of [
undefined,
null,
permission,
denied,
{ ...permission, mcp_servers: [ALL_PROXY_MCP_SERVERS_SENTINEL] },
]) {
expect(customerMatchesFilter({ ...customer, object_permission }, "mcp_access", "yes")).toBe(false);
expect(customerMatchesFilter({ ...customer, object_permission }, "mcp_access", "no")).toBe(true);
}
it.each([
{ mcp_servers: [NO_MCP_SERVERS_SENTINEL], mcp_access_groups: ["group-1"] },
{ mcp_servers: [ALL_PROXY_MCP_SERVERS_SENTINEL] },
])("matches denied MCP grants only as No MCP access %j", (grant) => {
expect(scopesMatching({ ...customer, object_permission: { ...permission, ...grant } })).toEqual(["denied"]);
});
it.each([undefined, null, permission])(
"matches customers without a grant only as No MCP restriction (%j)",
(object_permission) => {
expect(scopesMatching({ ...customer, object_permission })).toEqual(["unrestricted"]);
},
);
it("ignores invalid URL filter values and unrelated columns", () => {
expect(customerMatchesFilter(customer, "blocked", "unexpected")).toBe(true);
expect(customerMatchesFilter(customer, "max_budget", false)).toBe(true);
expect(customerMatchesFilter(customer, "mcp_access", "unexpected")).toBe(true);
expect(customerMatchesFilter(customer, "mcp_access", "yes")).toBe(true);
expect(customerMatchesFilter(customer, "unrelated", "yes")).toBe(true);
});
it("formats filter chips with the labels shown in the drawer", () => {
expect(formatCustomerFilterValue("blocked", "active")).toBe("Active");
expect(formatCustomerFilterValue("max_budget", "yes")).toBe("Has budget");
expect(formatCustomerFilterValue("mcp_access", "no")).toBe("No MCP access");
expect(formatCustomerFilterValue("mcp_access", "unrestricted")).toBe("No MCP restriction");
});
});

View file

@ -2,7 +2,7 @@ import type { FilterFn } from "@tanstack/react-table";
import type { EndUser } from "@/app/(dashboard)/hooks/customers/useCustomers";
import { customerHasMcpAccess } from "./customerResources";
import { customerMcpScope } from "./customerResources";
export const CUSTOMER_FILTER_FIELDS = [
{
@ -25,8 +25,9 @@ export const CUSTOMER_FILTER_FIELDS = [
id: "mcp_access",
label: "MCP access",
options: [
{ value: "yes", label: "Has MCP access" },
{ value: "no", label: "No MCP access" },
{ value: "limited", label: "Limited MCP access" },
{ value: "denied", label: "No MCP access" },
{ value: "unrestricted", label: "No MCP restriction" },
],
},
] as const;
@ -40,9 +41,12 @@ export const customerMatchesFilter = (customer: EndUser, columnId: string, value
if (value === "active") return !customer.blocked;
return true;
}
if (columnId === "mcp_access") {
if (value !== "limited" && value !== "denied" && value !== "unrestricted") return true;
return customerMcpScope(customer.object_permission) === value;
}
if (value !== "yes" && value !== "no") return true;
if (columnId === "max_budget") return (customer.litellm_budget_table?.max_budget != null) === (value === "yes");
if (columnId === "mcp_access") return customerHasMcpAccess(customer) === (value === "yes");
return true;
};

View file

@ -3,7 +3,7 @@ import { describe, expect, it } from "vitest";
import type { EndUser } from "@/app/(dashboard)/hooks/customers/useCustomers";
import { ALL_PROXY_MCP_SERVERS_SENTINEL, NO_MCP_SERVERS_SENTINEL } from "@/components/mcp_tools/constants";
import { customerHasMcpAccess, getCustomerResourceCounts, type CustomerResourceCounts } from "./customerResources";
import { customerMcpScope, getCustomerResourceCounts, type CustomerResourceCounts } from "./customerResources";
const emptyPermission: NonNullable<EndUser["object_permission"]> = {
object_permission_id: "permission-id",
@ -55,7 +55,7 @@ describe("getCustomerResourceCounts", () => {
},
};
expect(getCustomerResourceCounts(configured).servers).toBe(2);
expect(customerHasMcpAccess(configured)).toBe(true);
expect(customerMcpScope(configured.object_permission)).toBe("limited");
});
it("excludes the unsupported all-MCP sentinel from customer server counts", () => {
@ -64,7 +64,7 @@ describe("getCustomerResourceCounts", () => {
object_permission: { ...emptyPermission, mcp_servers: [ALL_PROXY_MCP_SERVERS_SENTINEL, "server-a"] },
};
expect(getCustomerResourceCounts(configured).servers).toBe(1);
expect(customerHasMcpAccess(configured)).toBe(true);
expect(customerMcpScope(configured.object_permission)).toBe("limited");
});
it("does not claim MCP access from unsupported all-MCP sentinel-only grants", () => {
@ -77,7 +77,7 @@ describe("getCustomerResourceCounts", () => {
},
};
expect(getCustomerResourceCounts(configured).servers).toBe(0);
expect(customerHasMcpAccess(configured)).toBe(false);
expect(customerMcpScope(configured.object_permission)).toBe("denied");
});
it("zeroes every MCP grant when no-MCP overrides other grants", () => {
@ -92,7 +92,7 @@ describe("getCustomerResourceCounts", () => {
},
};
expect(getCustomerResourceCounts(configured)).toEqual(noMcpCounts);
expect(customerHasMcpAccess(configured)).toBe(false);
expect(customerMcpScope(configured.object_permission)).toBe("denied");
});
it("excludes permission-table models and resources not enforced for customers", () => {
@ -110,19 +110,19 @@ describe("getCustomerResourceCounts", () => {
},
};
expect(getCustomerResourceCounts(configured)).toEqual({ ...noMcpCounts, models: 1 });
expect(customerHasMcpAccess(configured)).toBe(false);
expect(customerMcpScope(configured.object_permission)).toBe("unrestricted");
});
});
describe("customerHasMcpAccess", () => {
describe("customerMcpScope", () => {
it.each([
undefined,
null,
emptyPermission,
{ ...emptyPermission, mcp_servers: null, mcp_access_groups: null, mcp_toolsets: null },
])("treats an empty customer-level grant as not configured (%j)", (object_permission) => {
])("treats an empty customer-level grant as unrestricted (%j)", (object_permission) => {
const unconfigured = { ...customer, object_permission };
expect(customerHasMcpAccess(unconfigured)).toBe(false);
expect(customerMcpScope(object_permission)).toBe("unrestricted");
expect(getCustomerResourceCounts(unconfigured)).toEqual(noMcpCounts);
});
@ -132,6 +132,6 @@ describe("customerHasMcpAccess", () => {
{ mcp_toolsets: ["toolset-id"] },
{ mcp_tool_permissions: { "server-id": ["read"] } },
])("recognizes a grant from each MCP source (%j)", (grant) => {
expect(customerHasMcpAccess({ ...customer, object_permission: { ...emptyPermission, ...grant } })).toBe(true);
expect(customerMcpScope({ ...emptyPermission, ...grant })).toBe("limited");
});
});

View file

@ -30,7 +30,15 @@ export function getCustomerResourceCounts(
};
}
export function customerHasMcpAccess(customer: EndUser): boolean {
const counts = getCustomerResourceCounts(customer);
return counts.servers > 0 || counts.accessGroups > 0 || counts.toolsets > 0;
export type CustomerMcpScope = "limited" | "denied" | "unrestricted";
export function customerMcpScope(permission: EndUser["object_permission"]): CustomerMcpScope {
const servers = permission?.mcp_servers ?? [];
if (servers.includes(NO_MCP_SERVERS_SENTINEL)) return "denied";
const counts = getCustomerResourceCounts({ object_permission: permission });
if (counts.servers > 0 || counts.accessGroups > 0 || counts.toolsets > 0) return "limited";
const onlyUnsupportedAll =
servers.includes(ALL_PROXY_MCP_SERVERS_SENTINEL) ||
Object.hasOwn(permission?.mcp_tool_permissions ?? {}, ALL_PROXY_MCP_SERVERS_SENTINEL);
return onlyUnsupportedAll ? "denied" : "unrestricted";
}

View file

@ -258,3 +258,42 @@ describe("MCPServerSelector scoping", () => {
expect(optionByLabel("Wiki A")).toBeDefined();
});
});
describe("MCPServerSelector name collisions", () => {
beforeEach(() => {
vi.clearAllMocks();
mockUseMCPServers.mockReturnValue({
data: [
{ server_id: "shared", server_name: "Shared" },
{ server_id: "srv-x", server_name: "Other" },
],
isLoading: false,
} as any);
mockUseMCPAccessGroups.mockReturnValue({ data: ["shared", "alias-a"], isLoading: false } as any);
mockUseMCPToolsets.mockReturnValue({ data: [], isLoading: false } as any);
});
it.each(["shared", "alias-a"])(
"keeps the stored server %s a server when it matches an access group name",
async (server) => {
const user = userEvent.setup();
const onChange = vi.fn();
renderWithProviders(
<MCPServerSelector accessToken="tok" onChange={onChange} value={{ servers: [server], accessGroups: [] }} />,
);
await openSelector(user);
await user.click(optionByLabel("Other")!);
expect(onChange).toHaveBeenCalledWith({ servers: [server, "srv-x"], accessGroups: [], toolsets: [] });
},
);
it("shows a stored server alias by its name", () => {
renderWithProviders(
<MCPServerSelector accessToken="tok" onChange={vi.fn()} value={{ servers: ["alias-a"], accessGroups: [] }} />,
);
expect(screen.getByLabelText("alias-a")).toBeInTheDocument();
});
});

View file

@ -26,6 +26,14 @@ interface MCPServerSelectorProps {
}
const TOOLSET_PREFIX = "toolset:";
const ACCESS_GROUP_PREFIX = "access-group:";
const SERVER_PREFIX = "server:";
const SENTINELS: readonly string[] = [ALL_PROXY_MCP_SERVERS_SENTINEL, NO_MCP_SERVERS_SENTINEL];
const serverValue = (server: string): string => (SENTINELS.includes(server) ? server : `${SERVER_PREFIX}${server}`);
const valuesWithPrefix = (values: readonly string[], prefix: string): string[] =>
values.filter((value) => value.startsWith(prefix)).map((value) => value.slice(prefix.length));
export const serversWithinAccessGroups = (
servers: readonly MCPServer[],
@ -60,19 +68,18 @@ const MCPServerSelector: React.FC<MCPServerSelectorProps> = ({
const loading = serversLoading || groupsLoading || toolsetsLoading;
const accessGroupSet = new Set(accessGroups);
const visibleServers = serversWithinAccessGroups(mcpServers, value?.accessGroups ?? [], value?.servers ?? []);
// Combine options: access groups + servers + toolsets
const options = [
...accessGroups.map((group) => ({
label: group,
value: group,
value: `${ACCESS_GROUP_PREFIX}${group}`,
description: "Access Group",
})),
...visibleServers.map((server) => ({
label: `${server.server_name || server.server_id} (${server.server_id})`,
value: server.server_id,
value: serverValue(server.server_id),
description: "MCP Server",
})),
...toolsets.map((toolset) => ({
@ -82,13 +89,24 @@ const MCPServerSelector: React.FC<MCPServerSelectorProps> = ({
})),
];
// Flatten value for Select — prefix toolset IDs
const selectedValues = [
...(value?.servers || []),
...(value?.accessGroups || []),
...(value?.servers || []).map(serverValue),
...(value?.accessGroups || []).map((group) => `${ACCESS_GROUP_PREFIX}${group}`),
...(value?.toolsets || []).map((id) => `${TOOLSET_PREFIX}${id}`),
];
const listedValues = new Set(options.map((option) => option.value));
const unlistedSelections = [
...(value?.servers || [])
.filter((server) => !SENTINELS.includes(server))
.map((server) => ({ label: server, value: serverValue(server), description: "MCP Server" })),
...(value?.accessGroups || []).map((group) => ({
label: group,
value: `${ACCESS_GROUP_PREFIX}${group}`,
description: "Access Group",
})),
].filter((option) => !listedValues.has(option.value));
const hasNoMcpServersSelected = allowNoMcpServers && selectedValues.includes(NO_MCP_SERVERS_SENTINEL);
const hasAllProxyMcpServersSelected = selectedValues.includes(ALL_PROXY_MCP_SERVERS_SENTINEL);
@ -103,13 +121,11 @@ const MCPServerSelector: React.FC<MCPServerSelectorProps> = ({
onChange({ servers: [NO_MCP_SERVERS_SENTINEL], accessGroups: [], toolsets: [] });
return;
}
const toolsetsSelected = selected
.filter((v) => v.startsWith(TOOLSET_PREFIX))
.map((v) => v.slice(TOOLSET_PREFIX.length));
const rest = selected.filter((v) => !v.startsWith(TOOLSET_PREFIX));
const servers = rest.filter((v) => !accessGroupSet.has(v));
const accessGroupsSelected = rest.filter((v) => accessGroupSet.has(v));
onChange({ servers, accessGroups: accessGroupsSelected, toolsets: toolsetsSelected });
onChange({
servers: [...selected.filter((v) => SENTINELS.includes(v)), ...valuesWithPrefix(selected, SERVER_PREFIX)],
accessGroups: valuesWithPrefix(selected, ACCESS_GROUP_PREFIX),
toolsets: valuesWithPrefix(selected, TOOLSET_PREFIX),
});
};
const selectOptions: MultiSelectOption[] = [
@ -119,7 +135,7 @@ const MCPServerSelector: React.FC<MCPServerSelectorProps> = ({
...(allowNoMcpServers
? [{ label: "No MCP Servers", value: NO_MCP_SERVERS_SENTINEL, description: "Block all" }]
: []),
...options.map((opt) => ({
...[...options, ...unlistedSelections].map((opt) => ({
...opt,
disabled: hasNoMcpServersSelected || hasAllProxyMcpServersSelected,
})),

View file

@ -157,6 +157,24 @@ describe("MCPServerPermissions", () => {
expect(screen.queryByText(/tool/)).not.toBeInTheDocument();
});
it("labels an empty tool allowlist as allowing no tools", async () => {
vi.mocked(networking.fetchMCPServers).mockResolvedValue([
{ server_id: mockServerId1, server_name: mockServerName1, alias: mockServerName1 },
]);
render(
<MCPServerPermissions
mcpServers={[]}
mcpToolPermissions={{ [mockServerId1]: [] }}
entityType="customer"
accessToken={mockAccessToken}
/>,
);
expect(await screen.findByText(/DW_MCP/)).toBeInTheDocument();
expect(screen.getByText("No tools allowed")).toBeInTheDocument();
});
it("should display access groups correctly", async () => {
/**
* Tests that access groups are displayed with the correct styling

View file

@ -165,6 +165,7 @@ export function MCPServerPermissions({
{mergedItems.map((item, index) => {
const toolsForServer = item.type === "server" ? getToolPermissionsFor(item.value) : undefined;
const hasToolRestrictions = toolsForServer && toolsForServer.length > 0;
const allowsNoTools = toolsForServer?.length === 0;
const isExpanded = expandedServers.has(item.value);
return (
@ -197,6 +198,7 @@ export function MCPServerPermissions({
)}
</div>
{allowsNoTools && <span className="shrink-0 text-xs text-muted-foreground">No tools allowed</span>}
{hasToolRestrictions && (
<div className="flex items-center gap-1 shrink-0 whitespace-nowrap">
<span className="text-xs font-medium text-muted-foreground">{toolsForServer.length}</span>