mirror of
https://github.com/davidkaya/aryx.git
synced 2026-07-30 08:28:48 +02:00
feat: add server-level MCP auto-approval for wildcard tool servers
MCP servers configured with empty tools arrays (wildcard) now appear in the auto-approval pill with a server-level toggle. When toggled, a server-level approval key (mcp_server:<name>) is added to autoApprovedToolNames. The sidecar matches this key against PermissionRequestMcp.ServerName to auto- approve all tools from that server without needing individual tool names. - Add buildMcpServerApprovalKey/isMcpServerApprovalKey helpers - Create approval groups for all MCP servers including empty-tools ones - Add serverApprovalKey to ApprovalToolGroup for server-level toggles - Update sidecar RequiresToolCallApproval to check server-level keys - Include server-level keys in listApprovalToolNames for pruning safety - Add tests for both shared domain and sidecar approval matching Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -57,9 +57,10 @@ internal sealed class CopilotApprovalCoordinator
|
||||
{
|
||||
string? toolName = ResolveApprovalToolName(request, toolNamesByCallId);
|
||||
string? autoApprovedToolName = ResolveAutoApprovedToolName(request);
|
||||
string? mcpServerApprovalKey = ResolveMcpServerApprovalKey(request);
|
||||
string? approvalCacheKey = ResolveApprovalCacheKey(toolName, autoApprovedToolName);
|
||||
if (IsToolApprovedForRequest(command.RequestId, approvalCacheKey)
|
||||
|| !RequiresToolCallApproval(command.Pattern.ApprovalPolicy, agent.Id, toolName, autoApprovedToolName))
|
||||
|| !RequiresToolCallApproval(command.Pattern.ApprovalPolicy, agent.Id, toolName, autoApprovedToolName, mcpServerApprovalKey))
|
||||
{
|
||||
return CreateApprovalResult(PermissionRequestResultKind.Approved);
|
||||
}
|
||||
@@ -227,7 +228,8 @@ internal sealed class CopilotApprovalCoordinator
|
||||
ApprovalPolicyDto? approvalPolicy,
|
||||
string agentId,
|
||||
string? toolName,
|
||||
string? autoApprovedToolName = null)
|
||||
string? autoApprovedToolName = null,
|
||||
string? mcpServerApprovalKey = null)
|
||||
{
|
||||
if (approvalPolicy?.Rules is null || approvalPolicy.Rules.Count == 0)
|
||||
{
|
||||
@@ -245,7 +247,8 @@ internal sealed class CopilotApprovalCoordinator
|
||||
return true;
|
||||
}
|
||||
|
||||
return !MatchesAutoApprovedTool(autoApprovedToolNames, toolName, autoApprovedToolName);
|
||||
return !MatchesAutoApprovedTool(autoApprovedToolNames, toolName, autoApprovedToolName)
|
||||
&& !MatchesAutoApprovedToolName(autoApprovedToolNames, mcpServerApprovalKey);
|
||||
}
|
||||
|
||||
internal static bool TryGetApprovalToolName(
|
||||
@@ -327,6 +330,19 @@ internal sealed class CopilotApprovalCoordinator
|
||||
return GetFallbackToolName(request);
|
||||
}
|
||||
|
||||
private const string McpServerApprovalPrefix = "mcp_server:";
|
||||
|
||||
private static string? ResolveMcpServerApprovalKey(PermissionRequest request)
|
||||
{
|
||||
if (request is not PermissionRequestMcp mcp)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
|
||||
string? serverName = NormalizeOptionalString(mcp.ServerName);
|
||||
return serverName is not null ? $"{McpServerApprovalPrefix}{serverName}" : null;
|
||||
}
|
||||
|
||||
private static string? ResolveApprovalCacheKey(
|
||||
string? toolName,
|
||||
string? autoApprovedToolName)
|
||||
|
||||
@@ -843,6 +843,36 @@ public sealed class CopilotWorkflowRunnerTests
|
||||
Assert.True(CopilotApprovalCoordinator.RequiresToolCallApproval(policy, "agent-1", "git.status"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void RequiresToolCallApproval_HonorsMcpServerLevelApprovalKey()
|
||||
{
|
||||
ApprovalPolicyDto policy = new()
|
||||
{
|
||||
Rules =
|
||||
[
|
||||
new ApprovalCheckpointRuleDto
|
||||
{
|
||||
Kind = "tool-call",
|
||||
},
|
||||
],
|
||||
AutoApprovedToolNames = ["mcp_server:Git MCP"],
|
||||
};
|
||||
|
||||
// Server-level key approves any tool from that server
|
||||
Assert.False(CopilotApprovalCoordinator.RequiresToolCallApproval(
|
||||
policy, "agent-1", "git.status", null, "mcp_server:Git MCP"));
|
||||
Assert.False(CopilotApprovalCoordinator.RequiresToolCallApproval(
|
||||
policy, "agent-1", "git.diff", null, "mcp_server:Git MCP"));
|
||||
|
||||
// Different server still requires approval
|
||||
Assert.True(CopilotApprovalCoordinator.RequiresToolCallApproval(
|
||||
policy, "agent-1", "fs.read", null, "mcp_server:Filesystem"));
|
||||
|
||||
// Non-MCP tools unaffected
|
||||
Assert.True(CopilotApprovalCoordinator.RequiresToolCallApproval(
|
||||
policy, "agent-1", "unknown_tool"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TryGetApprovalToolName_ResolvesDirectNamesAndRuntimeFallbacks()
|
||||
{
|
||||
|
||||
@@ -26,6 +26,8 @@ import { type PatternDefinition, type ReasoningEffort } from '@shared/domain/pat
|
||||
import { isScratchpadProject, type ProjectRecord } from '@shared/domain/project';
|
||||
import { resolveSessionToolingSelection, type SessionRecord } from '@shared/domain/session';
|
||||
import {
|
||||
groupApprovalToolsByProvider,
|
||||
isMcpServerApprovalKey,
|
||||
listApprovalToolDefinitions,
|
||||
type RuntimeToolDefinition,
|
||||
type SessionToolingSelection,
|
||||
@@ -128,10 +130,19 @@ export function ChatPane({
|
||||
),
|
||||
[isApprovalOverridden, session.approvalSettings, pattern.approvalPolicy],
|
||||
);
|
||||
const effectiveAutoApprovedCount = useMemo(
|
||||
() => approvalTools.filter((t) => effectiveAutoApproved.has(t.id)).length,
|
||||
[approvalTools, effectiveAutoApproved],
|
||||
);
|
||||
const effectiveAutoApprovedCount = useMemo(() => {
|
||||
const groups = groupApprovalToolsByProvider(approvalTools, toolingSettings);
|
||||
let count = 0;
|
||||
for (const group of groups) {
|
||||
if (group.serverApprovalKey && effectiveAutoApproved.has(group.serverApprovalKey)) {
|
||||
// Server-level approval: count as 1 approved group even with 0 declared tools
|
||||
count += Math.max(group.tools.length, 1);
|
||||
} else {
|
||||
count += group.tools.filter((t) => effectiveAutoApproved.has(t.id)).length;
|
||||
}
|
||||
}
|
||||
return count;
|
||||
}, [approvalTools, effectiveAutoApproved, toolingSettings]);
|
||||
|
||||
useEffect(() => {
|
||||
transcriptRef.current?.scrollTo({
|
||||
|
||||
@@ -367,7 +367,11 @@ export function InlineApprovalPill({
|
||||
[approvalTools, toolingSettings],
|
||||
);
|
||||
|
||||
const showSearch = approvalTools.length > SEARCH_THRESHOLD;
|
||||
const totalItemCount = groups.reduce(
|
||||
(sum, g) => sum + Math.max(g.tools.length, g.serverApprovalKey ? 1 : 0),
|
||||
0,
|
||||
);
|
||||
const showSearch = totalItemCount > SEARCH_THRESHOLD;
|
||||
const searchLower = search.toLowerCase().trim();
|
||||
|
||||
const filteredGroups = useMemo(() => {
|
||||
@@ -382,7 +386,7 @@ export function InlineApprovalPill({
|
||||
|| group.label.toLowerCase().includes(searchLower),
|
||||
),
|
||||
}))
|
||||
.filter((g) => g.tools.length > 0);
|
||||
.filter((g) => g.tools.length > 0 || g.label.toLowerCase().includes(searchLower));
|
||||
}, [groups, searchLower]);
|
||||
|
||||
function toggleTool(toolId: string) {
|
||||
@@ -396,18 +400,45 @@ export function InlineApprovalPill({
|
||||
}
|
||||
|
||||
function toggleGroup(group: ApprovalToolGroup) {
|
||||
const allApproved = group.tools.every((t) => effectiveAutoApproved.has(t.id));
|
||||
const next = new Set(effectiveAutoApproved);
|
||||
for (const tool of group.tools) {
|
||||
if (allApproved) {
|
||||
next.delete(tool.id);
|
||||
|
||||
if (group.serverApprovalKey) {
|
||||
// MCP servers use server-level approval key
|
||||
if (next.has(group.serverApprovalKey)) {
|
||||
next.delete(group.serverApprovalKey);
|
||||
} else {
|
||||
next.add(tool.id);
|
||||
next.add(group.serverApprovalKey);
|
||||
}
|
||||
// Also remove individual tool entries when toggling server-level
|
||||
for (const tool of group.tools) {
|
||||
next.delete(tool.id);
|
||||
}
|
||||
} else {
|
||||
// Non-MCP groups: toggle individual tools
|
||||
const allApproved = group.tools.every((t) => next.has(t.id));
|
||||
for (const tool of group.tools) {
|
||||
if (allApproved) {
|
||||
next.delete(tool.id);
|
||||
} else {
|
||||
next.add(tool.id);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
onUpdate({ autoApprovedToolNames: [...next] });
|
||||
}
|
||||
|
||||
function isGroupApproved(group: ApprovalToolGroup): 'all' | 'some' | 'none' {
|
||||
if (group.serverApprovalKey && effectiveAutoApproved.has(group.serverApprovalKey)) {
|
||||
return 'all';
|
||||
}
|
||||
if (group.tools.length === 0) return 'none';
|
||||
const approvedCount = group.tools.filter((t) => effectiveAutoApproved.has(t.id)).length;
|
||||
if (approvedCount === group.tools.length) return 'all';
|
||||
if (approvedCount > 0) return 'some';
|
||||
return 'none';
|
||||
}
|
||||
|
||||
function toggleExpanded(groupId: string) {
|
||||
setExpandedGroups((prev) => {
|
||||
const next = new Set(prev);
|
||||
@@ -442,7 +473,7 @@ export function InlineApprovalPill({
|
||||
type="button"
|
||||
>
|
||||
<ShieldCheck className="size-2.5" />
|
||||
<span>{effectiveAutoApprovedCount}/{approvalTools.length} auto-approved</span>
|
||||
<span>{effectiveAutoApprovedCount}/{totalItemCount} auto-approved</span>
|
||||
<ChevronDown className={`size-2.5 transition ${open ? 'rotate-180' : ''}`} />
|
||||
</button>
|
||||
|
||||
@@ -494,9 +525,12 @@ export function InlineApprovalPill({
|
||||
const isBuiltin = group.kind === 'builtin';
|
||||
const isCollapsible = !isBuiltin;
|
||||
const expanded = isBuiltin || isGroupExpanded(group.id);
|
||||
const approvedCount = group.tools.filter((t) => effectiveAutoApproved.has(t.id)).length;
|
||||
const allApproved = approvedCount === group.tools.length;
|
||||
const someApproved = approvedCount > 0 && !allApproved;
|
||||
const groupState = isGroupApproved(group);
|
||||
const allApproved = groupState === 'all';
|
||||
const someApproved = groupState === 'some';
|
||||
const approvedLabel = group.serverApprovalKey && allApproved
|
||||
? 'all'
|
||||
: `${group.tools.filter((t) => effectiveAutoApproved.has(t.id)).length}/${group.tools.length}`;
|
||||
|
||||
return (
|
||||
<div key={group.id}>
|
||||
@@ -511,10 +545,14 @@ export function InlineApprovalPill({
|
||||
onClick={() => toggleExpanded(group.id)}
|
||||
type="button"
|
||||
>
|
||||
<ChevronRight className={`size-3 shrink-0 text-zinc-600 transition ${expanded ? 'rotate-90' : ''}`} />
|
||||
{group.tools.length > 0 ? (
|
||||
<ChevronRight className={`size-3 shrink-0 text-zinc-600 transition ${expanded ? 'rotate-90' : ''}`} />
|
||||
) : (
|
||||
<Server className="size-3 shrink-0 text-zinc-600" />
|
||||
)}
|
||||
<span className="min-w-0 flex-1 truncate text-[12px] font-medium text-zinc-300">{group.label}</span>
|
||||
<span className="shrink-0 rounded-full bg-zinc-800/80 px-1.5 py-px text-[9px] font-medium tabular-nums text-zinc-500">
|
||||
{approvedCount}/{group.tools.length}
|
||||
{approvedLabel}
|
||||
</span>
|
||||
<GroupToggle
|
||||
allApproved={allApproved}
|
||||
|
||||
@@ -149,6 +149,16 @@ const fallbackRuntimeApprovalTools: ReadonlyArray<RuntimeToolDefinition> = [
|
||||
{ id: 'store_memory', label: 'Store memory' },
|
||||
];
|
||||
|
||||
const MCP_SERVER_APPROVAL_PREFIX = 'mcp_server:';
|
||||
|
||||
export function buildMcpServerApprovalKey(serverName: string): string {
|
||||
return `${MCP_SERVER_APPROVAL_PREFIX}${serverName}`;
|
||||
}
|
||||
|
||||
export function isMcpServerApprovalKey(key: string): boolean {
|
||||
return key.startsWith(MCP_SERVER_APPROVAL_PREFIX);
|
||||
}
|
||||
|
||||
export function createWorkspaceSettings(): WorkspaceSettings {
|
||||
return {
|
||||
theme: 'dark',
|
||||
@@ -262,7 +272,11 @@ export function listApprovalToolNames(
|
||||
tooling: WorkspaceToolingSettings,
|
||||
runtimeTools?: ReadonlyArray<RuntimeToolDefinition>,
|
||||
): string[] {
|
||||
return listApprovalToolDefinitions(tooling, runtimeTools).map((tool) => tool.id);
|
||||
const toolNames = listApprovalToolDefinitions(tooling, runtimeTools).map((tool) => tool.id);
|
||||
for (const server of tooling.mcpServers) {
|
||||
toolNames.push(buildMcpServerApprovalKey(server.name));
|
||||
}
|
||||
return [...new Set(toolNames)];
|
||||
}
|
||||
|
||||
export interface ApprovalToolGroup {
|
||||
@@ -270,6 +284,7 @@ export interface ApprovalToolGroup {
|
||||
label: string;
|
||||
kind: ApprovalToolKind;
|
||||
tools: ApprovalToolDefinition[];
|
||||
serverApprovalKey?: string;
|
||||
}
|
||||
|
||||
const approvalToolGroupKindOrder: ApprovalToolKind[] = ['builtin', 'mcp', 'lsp', 'mixed'];
|
||||
@@ -292,6 +307,18 @@ export function groupApprovalToolsByProvider(
|
||||
group.tools.push(tool);
|
||||
}
|
||||
|
||||
// Ensure every MCP server has a group (even with no declared tools) and
|
||||
// attach server-level approval keys.
|
||||
for (const server of tooling.mcpServers) {
|
||||
const groupId = `mcp:${server.id}`;
|
||||
let group = groups.get(groupId);
|
||||
if (!group) {
|
||||
group = { id: groupId, label: server.name, kind: 'mcp', tools: [] };
|
||||
groups.set(groupId, group);
|
||||
}
|
||||
group.serverApprovalKey = buildMcpServerApprovalKey(server.name);
|
||||
}
|
||||
|
||||
return [...groups.values()].sort((a, b) => {
|
||||
const kindDiff = approvalToolGroupKindOrder.indexOf(a.kind) - approvalToolGroupKindOrder.indexOf(b.kind);
|
||||
if (kindDiff !== 0) return kindDiff;
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
|
||||
import {
|
||||
buildMcpServerApprovalKey,
|
||||
groupApprovalToolsByProvider,
|
||||
listApprovalToolDefinitions,
|
||||
listApprovalToolNames,
|
||||
normalizeWorkspaceSettings,
|
||||
resolveProjectToolingSettings,
|
||||
resolveToolLabel,
|
||||
@@ -441,7 +443,7 @@ describe('groupApprovalToolsByProvider', () => {
|
||||
expect(builtinGroups[0].tools.length).toBe(5);
|
||||
});
|
||||
|
||||
test('multi-provider tools go into first provider group', () => {
|
||||
test('multi-provider tools go into first provider group, second provider gets empty group', () => {
|
||||
const tooling = makeTooling([
|
||||
makeMcpServer('git-a', 'Git A', ['git.status']),
|
||||
makeMcpServer('git-b', 'Git B', ['git.status']),
|
||||
@@ -450,9 +452,13 @@ describe('groupApprovalToolsByProvider', () => {
|
||||
const groups = groupApprovalToolsByProvider(tools, tooling);
|
||||
|
||||
const mcpGroups = groups.filter((g) => g.kind === 'mcp');
|
||||
expect(mcpGroups.length).toBe(1);
|
||||
expect(mcpGroups[0].label).toBe('Git A');
|
||||
expect(mcpGroups[0].tools[0].providerNames).toEqual(['Git A', 'Git B']);
|
||||
expect(mcpGroups.length).toBe(2);
|
||||
const gitA = mcpGroups.find((g) => g.label === 'Git A');
|
||||
expect(gitA).toBeDefined();
|
||||
expect(gitA!.tools[0].providerNames).toEqual(['Git A', 'Git B']);
|
||||
const gitB = mcpGroups.find((g) => g.label === 'Git B');
|
||||
expect(gitB).toBeDefined();
|
||||
expect(gitB!.tools.length).toBe(0);
|
||||
});
|
||||
|
||||
test('sorts builtin first, then MCP by name, then LSP by name', () => {
|
||||
@@ -472,4 +478,61 @@ describe('groupApprovalToolsByProvider', () => {
|
||||
const mcpLabels = groups.filter((g) => g.kind === 'mcp').map((g) => g.label);
|
||||
expect(mcpLabels).toEqual(['Alpha', 'Zebra']);
|
||||
});
|
||||
|
||||
test('creates groups with serverApprovalKey for MCP servers', () => {
|
||||
const tooling = makeTooling([
|
||||
makeMcpServer('git', 'Git MCP', ['git.status']),
|
||||
]);
|
||||
const tools = listApprovalToolDefinitions(tooling);
|
||||
const groups = groupApprovalToolsByProvider(tools, tooling);
|
||||
|
||||
const mcpGroup = groups.find((g) => g.kind === 'mcp');
|
||||
expect(mcpGroup).toBeDefined();
|
||||
expect(mcpGroup!.serverApprovalKey).toBe('mcp_server:Git MCP');
|
||||
});
|
||||
|
||||
test('creates groups for MCP servers with empty tools array', () => {
|
||||
const tooling = makeTooling([
|
||||
makeMcpServer('empty', 'Empty Server', []),
|
||||
]);
|
||||
const tools = listApprovalToolDefinitions(tooling);
|
||||
const groups = groupApprovalToolsByProvider(tools, tooling);
|
||||
|
||||
const mcpGroups = groups.filter((g) => g.kind === 'mcp');
|
||||
expect(mcpGroups.length).toBe(1);
|
||||
expect(mcpGroups[0].label).toBe('Empty Server');
|
||||
expect(mcpGroups[0].tools.length).toBe(0);
|
||||
expect(mcpGroups[0].serverApprovalKey).toBe('mcp_server:Empty Server');
|
||||
});
|
||||
|
||||
test('builtin groups do not have serverApprovalKey', () => {
|
||||
const tooling = makeTooling();
|
||||
const tools = listApprovalToolDefinitions(tooling);
|
||||
const groups = groupApprovalToolsByProvider(tools, tooling);
|
||||
|
||||
const builtinGroup = groups.find((g) => g.kind === 'builtin');
|
||||
expect(builtinGroup!.serverApprovalKey).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('server-level approval keys', () => {
|
||||
test('buildMcpServerApprovalKey produces mcp_server: prefixed key', () => {
|
||||
expect(buildMcpServerApprovalKey('My Server')).toBe('mcp_server:My Server');
|
||||
expect(buildMcpServerApprovalKey('git')).toBe('mcp_server:git');
|
||||
});
|
||||
|
||||
test('listApprovalToolNames includes server-level keys', () => {
|
||||
const tooling: WorkspaceToolingSettings = {
|
||||
mcpServers: [{
|
||||
id: 'git', name: 'Git MCP', transport: 'local', command: 'node', args: [],
|
||||
tools: [], createdAt: '2026-03-28T00:00:00.000Z', updatedAt: '2026-03-28T00:00:00.000Z',
|
||||
}],
|
||||
lspProfiles: [],
|
||||
};
|
||||
const names = listApprovalToolNames(tooling);
|
||||
expect(names).toContain('mcp_server:Git MCP');
|
||||
// Also includes builtins
|
||||
expect(names).toContain('read');
|
||||
expect(names).toContain('shell');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user