fix(ui): keep standing MCP grants on team save

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
yassin 2026-09-05 20:41:08 +00:00
parent 94cc52e555
commit ffd8dff5cf
2 changed files with 101 additions and 15 deletions

View file

@ -7,7 +7,12 @@ import { chooseSelectOption, renderWithProviders, testQueryClient } from "../../
import { toast } from "@/lib/toast";
import type { EffectiveMcpServer } from "../mcp_server_management/effectiveMcpServers";
import type { MCPServer } from "../mcp_tools/types";
import TeamInfoView, { grantedMcpServerIds, retainedMcpToolPermissions, type TeamData } from "./TeamInfo";
import TeamInfoView, {
grantedMcpServerIds,
retainedMcpToolPermissions,
standingToolPermissionServerIds,
type TeamData,
} from "./TeamInfo";
const authState = vi.hoisted(() => ({ userRole: "Admin" }));
@ -2286,6 +2291,7 @@ describe("TeamInfo MCP permission retention", () => {
[effective("direct", "direct"), effective("inherited", "toolPermission")],
["ag-1"],
[{ access_group_id: "ag-1", access_mcp_server_ids: ["inherited"] }],
new Set(),
);
expect(resolution.kind).toBe("resolved");
if (resolution.kind !== "resolved") return;
@ -2318,7 +2324,7 @@ describe("TeamInfo MCP permission retention", () => {
});
it("refuses an unresolved selected access group", () => {
expect(grantedMcpServerIds([effective("server-1", "direct")], ["missing"], [])).toEqual({
expect(grantedMcpServerIds([effective("server-1", "direct")], ["missing"], [], new Set())).toEqual({
kind: "unresolvable",
reason: expect.stringMatching(/access groups could not be loaded/),
});
@ -2330,35 +2336,66 @@ describe("TeamInfo MCP permission retention", () => {
});
});
it("retains an ambiguous name-keyed permission when one matching server remains granted", () => {
it("splits an ambiguous permission across the matching granted server", () => {
const catalog = [server("server-1", "shared"), server("server-2", "shared")];
expect(retainedMcpToolPermissions({ shared: ["read"] }, new Set(["server-1"]), catalog)).toEqual({
expect(
retainedMcpToolPermissions({ shared: ["read"], "server-1": ["write"] }, new Set(["server-1"]), catalog),
).toEqual({
"server-1": ["write", "read"],
});
});
it("keeps an ambiguous permission when all matching servers remain granted", () => {
const catalog = [server("server-1", "shared"), server("server-2", "shared")];
expect(retainedMcpToolPermissions({ shared: ["read"] }, new Set(["server-1", "server-2"]), catalog)).toEqual({
shared: ["read"],
});
});
it("identifies standing tool-permission grants not covered by loaded access groups", () => {
expect(
standingToolPermissionServerIds(
[effective("a", "toolPermission"), effective("b", "toolPermission"), effective("c", "direct")],
["ag-1"],
[{ access_group_id: "ag-1", access_mcp_server_ids: ["b"] }],
),
).toEqual(new Set(["a"]));
});
it("includes standing tool-permission grants in the resolved server ids", () => {
expect(grantedMcpServerIds([effective("x", "toolPermission")], [], [], new Set(["x"]))).toEqual({
kind: "resolved",
serverIds: new Set(["x"]),
});
expect(grantedMcpServerIds([effective("x", "toolPermission")], [], [], new Set())).toEqual({
kind: "resolved",
serverIds: new Set(),
});
});
it("retains an indirectly granted server on an unrelated team save", async () => {
const user = userEvent.setup({ delay: null });
const catalog = [server("direct-server", "deploy_tracker"), server("group-server", "issue_tracker")];
const catalog = [server("direct-server", "deploy_tracker"), server("perm-only-server", "issue_tracker")];
mockUseMCPServers.mockReturnValue({ data: catalog, isLoading: false, isError: false } as any);
mockUseMCPToolsets.mockReturnValue({ data: [], isLoading: false, isError: false } as any);
mockUseAccessGroups.mockReturnValue({
data: [{ access_group_id: "ag-1", access_group_name: "Ops", access_mcp_server_ids: ["group-server"] }],
data: [],
isLoading: false,
isError: false,
} as any);
vi.mocked(networking.teamInfoCall).mockResolvedValue(
createMockTeamData({
models: ["gpt-4"],
access_group_ids: ["ag-1"],
access_group_ids: [],
object_permission: {
mcp_servers: ["direct-server"],
mcp_access_groups: [],
mcp_toolsets: [],
mcp_tool_permissions: {
"direct-server": ["create_issue"],
"group-server": ["list_issues"],
"perm-only-server": ["list_issues"],
},
},
}),
@ -2388,7 +2425,7 @@ describe("TeamInfo MCP permission retention", () => {
const [, payload] = vi.mocked(networking.teamUpdateCall).mock.calls[0];
expect(payload.object_permission.mcp_tool_permissions).toEqual({
"direct-server": ["create_issue"],
"group-server": ["list_issues"],
"perm-only-server": ["list_issues"],
});
});

View file

@ -131,10 +131,28 @@ export type McpGrantResolution =
| { readonly kind: "resolved"; readonly serverIds: ReadonlySet<string> }
| { readonly kind: "unresolvable"; readonly reason: string };
export const standingToolPermissionServerIds = (
loadedEffectiveServers: readonly EffectiveMcpServer[],
loadedAccessGroupIds: readonly string[],
accessGroups: readonly Pick<AccessGroupResponse, "access_group_id" | "access_mcp_server_ids">[],
): ReadonlySet<string> => {
const loadedUnifiedServerIds = new Set(
accessGroups
.filter((group) => loadedAccessGroupIds.includes(group.access_group_id))
.flatMap((group) => group.access_mcp_server_ids),
);
return new Set(
loadedEffectiveServers
.filter(({ source, server }) => source.kind === "toolPermission" && !loadedUnifiedServerIds.has(server.server_id))
.map(({ server }) => server.server_id),
);
};
export const grantedMcpServerIds = (
effectiveServers: readonly EffectiveMcpServer[],
selectedAccessGroupIds: readonly string[],
accessGroups: readonly Pick<AccessGroupResponse, "access_group_id" | "access_mcp_server_ids">[],
standingServerIds: ReadonlySet<string>,
): McpGrantResolution => {
const selectedGroups = accessGroups.filter((group) => selectedAccessGroupIds.includes(group.access_group_id));
if (selectedAccessGroupIds.some((id) => !selectedGroups.some((group) => group.access_group_id === id))) {
@ -144,20 +162,36 @@ export const grantedMcpServerIds = (
.filter(({ source }) => source.kind !== "toolPermission")
.map(({ server }) => server.server_id);
const viaUnifiedGroups = selectedGroups.flatMap((group) => group.access_mcp_server_ids);
return { kind: "resolved", serverIds: new Set([...direct, ...viaUnifiedGroups]) };
return { kind: "resolved", serverIds: new Set([...direct, ...viaUnifiedGroups, ...standingServerIds]) };
};
export const retainedMcpToolPermissions = (
toolPermissions: Record<string, string[]>,
grantedServerIds: ReadonlySet<string>,
knownServers: readonly MCPServer[],
): Record<string, string[]> =>
Object.fromEntries(
Object.entries(toolPermissions).filter(([key]) => {
const named = mcpServersForIdentifier(knownServers, key);
return named.length === 0 || named.some((server) => grantedServerIds.has(server.server_id));
): Record<string, string[]> => {
const entries = Object.entries(toolPermissions).flatMap(([key, tools]) => {
const named = mcpServersForIdentifier(knownServers, key);
const granted = named.filter((server) => grantedServerIds.has(server.server_id));
if (named.length === 0 || granted.length === named.length) {
return [[key, tools] as const];
}
if (granted.length === 0) {
return [];
}
return granted.map(({ server_id }) => [
server_id,
[...(toolPermissions[server_id] ?? []), ...tools],
] as const);
});
return entries.reduce<Record<string, string[]>>(
(retained, [key, tools]) => ({
...retained,
[key]: [...new Set([...(retained[key] ?? []), ...tools])],
}),
{},
);
};
export const mcpUnresolvableSaveError = (reason: string): string =>
`Cannot save MCP tool permissions because ${reason}. Retry once the page has finished loading`;
@ -902,6 +936,20 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
toolsets: allMcpToolsets,
toolPermissions: submittedToolPermissions,
};
const loadedObjectPermission = info.object_permission ?? {};
const loadedEffectiveMcpServers = resolveEffectiveMcpServers({
allServers: allMcpServers,
selectedServers: loadedObjectPermission.mcp_servers ?? [],
selectedAccessGroups: loadedObjectPermission.mcp_access_groups ?? [],
selectedToolsets: loadedObjectPermission.mcp_toolsets ?? [],
toolsets: allMcpToolsets,
toolPermissions: loadedObjectPermission.mcp_tool_permissions ?? {},
});
const standingServerIds = standingToolPermissionServerIds(
loadedEffectiveMcpServers,
info.access_group_ids ?? [],
allAccessGroups,
);
const mcpResolution: McpGrantResolution =
mcpLookupFailure !== null
? { kind: "unresolvable", reason: mcpLookupFailure }
@ -909,6 +957,7 @@ const TeamInfoView: React.FC<TeamInfoProps> = ({
resolveEffectiveMcpServers(effectiveMcpInput),
values.access_group_ids || [],
allAccessGroups,
standingServerIds,
);
if (mcpResolution.kind === "unresolvable" && Object.keys(submittedToolPermissions).length > 0) {
toast.fromError(mcpUnresolvableSaveError(mcpResolution.reason));