From 53a08e0ed462d3abd7c4f85384eef1a66ec3463e Mon Sep 17 00:00:00 2001 From: David Kaya Date: Sat, 28 Mar 2026 18:18:17 +0100 Subject: [PATCH] feat: redesign auto-approval pill with server-level grouping and batch toggle Group MCP tools by server and LSP tools by profile in the approval popover instead of showing a flat list. Each server/profile group gets a collapsible header with a batch toggle to approve/unapprove all tools at once. Add a search filter (visible when >10 tools) and partial-state indicators for groups with mixed approval. Add groupApprovalToolsByProvider shared helper with tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/renderer/components/ChatPane.tsx | 2 + src/renderer/components/chat/InlinePills.tsx | 245 +++++++++++++++---- src/shared/domain/tooling.ts | 63 +++++ tests/shared/tooling.test.ts | 95 +++++++ 4 files changed, 352 insertions(+), 53 deletions(-) diff --git a/src/renderer/components/ChatPane.tsx b/src/renderer/components/ChatPane.tsx index 117d79f..478592a 100644 --- a/src/renderer/components/ChatPane.tsx +++ b/src/renderer/components/ChatPane.tsx @@ -484,6 +484,7 @@ export function ChatPane({ effectiveAutoApprovedCount={effectiveAutoApprovedCount} isOverridden={isApprovalOverridden} onUpdate={onUpdateSessionApprovalSettings} + toolingSettings={toolingSettings} /> )} {primaryAgent && ( @@ -539,6 +540,7 @@ export function ChatPane({ effectiveAutoApprovedCount={effectiveAutoApprovedCount} isOverridden={isApprovalOverridden} onUpdate={onUpdateSessionApprovalSettings} + toolingSettings={toolingSettings} /> )} diff --git a/src/renderer/components/chat/InlinePills.tsx b/src/renderer/components/chat/InlinePills.tsx index 4c15ab0..c74f0be 100644 --- a/src/renderer/components/chat/InlinePills.tsx +++ b/src/renderer/components/chat/InlinePills.tsx @@ -1,10 +1,11 @@ -import { useState } from 'react'; -import { ChevronDown, Sparkles } from 'lucide-react'; +import { useMemo, useState } from 'react'; +import { ChevronDown, ChevronRight, Minus, Search, Sparkles } from 'lucide-react'; import { ProviderIcon } from '@renderer/components/ProviderIcons'; import { PopoverToggleRow } from '@renderer/components/ui'; import { useClickOutside } from '@renderer/hooks/useClickOutside'; -import type { ApprovalToolDefinition, ApprovalToolKind, LspProfileDefinition, McpServerDefinition, SessionToolingSelection } from '@shared/domain/tooling'; +import type { ApprovalToolDefinition, LspProfileDefinition, McpServerDefinition, SessionToolingSelection, WorkspaceToolingSettings } from '@shared/domain/tooling'; +import { groupApprovalToolsByProvider, type ApprovalToolGroup } from '@shared/domain/tooling'; import { findModel, inferProvider, providerMeta, type ModelDefinition } from '@shared/domain/models'; import { reasoningEffortOptions, type ReasoningEffort } from '@shared/domain/pattern'; import { RotateCcw, Server, ShieldCheck } from 'lucide-react'; @@ -337,16 +338,11 @@ function McpServerGroup({ /* ── InlineApprovalPill ────────────────────────────────────── */ -const approvalKindOrder: ApprovalToolKind[] = ['builtin', 'mcp', 'lsp', 'mixed']; -const approvalKindLabels: Record = { - builtin: 'Built-in', - mcp: 'MCP Servers', - lsp: 'Language Servers', - mixed: 'Other', -}; +const SEARCH_THRESHOLD = 10; export function InlineApprovalPill({ approvalTools, + toolingSettings, effectiveAutoApproved, effectiveAutoApprovedCount, isOverridden, @@ -354,6 +350,7 @@ export function InlineApprovalPill({ onUpdate, }: { approvalTools: ApprovalToolDefinition[]; + toolingSettings: WorkspaceToolingSettings; effectiveAutoApproved: Set; effectiveAutoApprovedCount: number; isOverridden: boolean; @@ -361,7 +358,32 @@ export function InlineApprovalPill({ onUpdate: (settings: { autoApprovedToolNames?: string[] }) => void; }){ const [open, setOpen] = useState(false); - const ref = useClickOutside(() => setOpen(false), open); + const [search, setSearch] = useState(''); + const [expandedGroups, setExpandedGroups] = useState>(new Set()); + const ref = useClickOutside(() => { setOpen(false); setSearch(''); }, open); + + const groups = useMemo( + () => groupApprovalToolsByProvider(approvalTools, toolingSettings), + [approvalTools, toolingSettings], + ); + + const showSearch = approvalTools.length > SEARCH_THRESHOLD; + const searchLower = search.toLowerCase().trim(); + + const filteredGroups = useMemo(() => { + if (!searchLower) return groups; + return groups + .map((group) => ({ + ...group, + tools: group.tools.filter( + (t) => + t.label.toLowerCase().includes(searchLower) + || t.id.toLowerCase().includes(searchLower) + || group.label.toLowerCase().includes(searchLower), + ), + })) + .filter((g) => g.tools.length > 0); + }, [groups, searchLower]); function toggleTool(toolId: string) { const next = new Set(effectiveAutoApproved); @@ -373,10 +395,35 @@ export function InlineApprovalPill({ onUpdate({ autoApprovedToolNames: [...next] }); } - const groups = approvalKindOrder - .map((kind) => ({ kind, tools: approvalTools.filter((t) => t.kind === kind) })) - .filter((g) => g.tools.length > 0); - const showHeaders = groups.length > 1; + 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); + } else { + next.add(tool.id); + } + } + onUpdate({ autoApprovedToolNames: [...next] }); + } + + function toggleExpanded(groupId: string) { + setExpandedGroups((prev) => { + const next = new Set(prev); + if (next.has(groupId)) { + next.delete(groupId); + } else { + next.add(groupId); + } + return next; + }); + } + + function isGroupExpanded(groupId: string): boolean { + if (searchLower) return true; + return expandedGroups.has(groupId); + } return (
@@ -400,52 +447,144 @@ export function InlineApprovalPill({ {open && !disabled && ( -
-
- - {isOverridden ? 'Session override' : 'Pattern defaults'} - - {isOverridden && ( - +
+ {/* Header: session override / pattern defaults */} +
+
+ + {isOverridden ? 'Session override' : 'Pattern defaults'} + + {isOverridden && ( + + )} +
+ + {/* Search */} + {showSearch && ( +
+
+ + setSearch(e.target.value)} + placeholder="Filter tools…" + type="text" + value={search} + /> +
+
)}
+ {/* Tool groups */}
- {groups.map((group, i) => ( -
- {showHeaders && ( -
0 ? 'pt-2' : 'pt-1'} text-[9px] font-semibold uppercase tracking-wider text-zinc-600`}> - {approvalKindLabels[group.kind]} -
- )} - {group.tools.map((tool) => { - const detail = tool.description || (tool.providerNames.length > 0 ? tool.providerNames.join(', ') : undefined); - return ( - toggleTool(tool.id)} - /> - ); - })} + {filteredGroups.map((group, groupIdx) => { + 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; + + return ( +
+ {/* Group header */} + {isBuiltin ? ( +
0 ? 'pt-2.5' : 'pt-1'} text-[9px] font-semibold uppercase tracking-wider text-zinc-600`}> + {group.label} +
+ ) : ( + + )} + + {/* Group tools */} + {expanded && group.tools.map((tool) => { + const detail = tool.description || ( + !isBuiltin && tool.providerNames.length > 1 + ? tool.providerNames.join(', ') + : undefined + ); + return ( +
+ toggleTool(tool.id)} + /> +
+ ); + })} +
+ ); + })} + + {filteredGroups.length === 0 && searchLower && ( +
+ No tools match "{search}"
- ))} + )}
)}
); } + +function GroupToggle({ + allApproved, + someApproved, + onToggle, +}: { + allApproved: boolean; + someApproved: boolean; + onToggle: (e: React.MouseEvent) => void; +}) { + return ( + + ); +} diff --git a/src/shared/domain/tooling.ts b/src/shared/domain/tooling.ts index 2d8e4ba..b01f857 100644 --- a/src/shared/domain/tooling.ts +++ b/src/shared/domain/tooling.ts @@ -265,6 +265,69 @@ export function listApprovalToolNames( return listApprovalToolDefinitions(tooling, runtimeTools).map((tool) => tool.id); } +export interface ApprovalToolGroup { + id: string; + label: string; + kind: ApprovalToolKind; + tools: ApprovalToolDefinition[]; +} + +const approvalToolGroupKindOrder: ApprovalToolKind[] = ['builtin', 'mcp', 'lsp', 'mixed']; + +export function groupApprovalToolsByProvider( + tools: ReadonlyArray, + tooling: WorkspaceToolingSettings, +): ApprovalToolGroup[] { + const serverNames = new Map(tooling.mcpServers.map((s) => [s.id, s.name])); + const profileNames = new Map(tooling.lspProfiles.map((p) => [p.id, p.name])); + const groups = new Map(); + + for (const tool of tools) { + const groupKey = resolveApprovalToolGroupKey(tool, serverNames, profileNames); + let group = groups.get(groupKey.id); + if (!group) { + group = { id: groupKey.id, label: groupKey.label, kind: groupKey.kind, tools: [] }; + groups.set(groupKey.id, group); + } + group.tools.push(tool); + } + + return [...groups.values()].sort((a, b) => { + const kindDiff = approvalToolGroupKindOrder.indexOf(a.kind) - approvalToolGroupKindOrder.indexOf(b.kind); + if (kindDiff !== 0) return kindDiff; + return a.label.localeCompare(b.label); + }); +} + +function resolveApprovalToolGroupKey( + tool: ApprovalToolDefinition, + serverNames: ReadonlyMap, + profileNames: ReadonlyMap, +): { id: string; label: string; kind: ApprovalToolKind } { + if (tool.kind === 'builtin') { + return { id: 'builtin', label: 'Built-in', kind: 'builtin' }; + } + + const primaryProviderId = tool.providerIds[0]; + if (tool.kind === 'mcp' && primaryProviderId) { + return { + id: `mcp:${primaryProviderId}`, + label: serverNames.get(primaryProviderId) ?? tool.providerNames[0] ?? primaryProviderId, + kind: 'mcp', + }; + } + + if (tool.kind === 'lsp' && primaryProviderId) { + return { + id: `lsp:${primaryProviderId}`, + label: profileNames.get(primaryProviderId) ?? tool.providerNames[0] ?? primaryProviderId, + kind: 'lsp', + }; + } + + return { id: 'other', label: 'Other', kind: 'mixed' }; +} + export function validateMcpServerDefinition(server: McpServerDefinition): string | undefined { if (!server.name.trim()) { return 'MCP server name is required.'; diff --git a/tests/shared/tooling.test.ts b/tests/shared/tooling.test.ts index c81181f..bd25da5 100644 --- a/tests/shared/tooling.test.ts +++ b/tests/shared/tooling.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from 'bun:test'; import { + groupApprovalToolsByProvider, listApprovalToolDefinitions, normalizeWorkspaceSettings, resolveProjectToolingSettings, @@ -10,6 +11,7 @@ import { validateMcpServerDefinition, type LspProfileDefinition, type McpServerDefinition, + type WorkspaceToolingSettings, } from '@shared/domain/tooling'; const TIMESTAMP = '2026-03-23T00:00:00.000Z'; @@ -378,3 +380,96 @@ describe('tooling settings helpers', () => { }); }); }); + +describe('groupApprovalToolsByProvider', () => { + const TIMESTAMP = '2026-03-28T00:00:00.000Z'; + + function makeTooling( + mcpServers: McpServerDefinition[] = [], + lspProfiles: LspProfileDefinition[] = [], + ): WorkspaceToolingSettings { + return { mcpServers, lspProfiles }; + } + + function makeMcpServer(id: string, name: string, tools: string[]): McpServerDefinition { + return { id, name, transport: 'local', command: 'node', args: [], tools, createdAt: TIMESTAMP, updatedAt: TIMESTAMP }; + } + + function makeLspProfile(id: string, name: string): LspProfileDefinition { + return { id, name, command: 'lsp-server', args: ['--stdio'], languageId: 'typescript', fileExtensions: ['.ts'], createdAt: TIMESTAMP, updatedAt: TIMESTAMP }; + } + + test('groups MCP tools by server', () => { + const tooling = makeTooling([ + makeMcpServer('git', 'Git MCP', ['git.status', 'git.diff']), + makeMcpServer('fs', 'Filesystem', ['fs.read', 'fs.write']), + ]); + const tools = listApprovalToolDefinitions(tooling); + const groups = groupApprovalToolsByProvider(tools, tooling); + + const mcpGroups = groups.filter((g) => g.kind === 'mcp'); + expect(mcpGroups.length).toBe(2); + + const fsGroup = mcpGroups.find((g) => g.label === 'Filesystem'); + expect(fsGroup).toBeDefined(); + expect(fsGroup!.tools.map((t) => t.id).sort()).toEqual(['fs.read', 'fs.write']); + + const gitGroup = mcpGroups.find((g) => g.label === 'Git MCP'); + expect(gitGroup).toBeDefined(); + expect(gitGroup!.tools.map((t) => t.id).sort()).toEqual(['git.diff', 'git.status']); + }); + + test('groups LSP tools by profile', () => { + const tooling = makeTooling([], [makeLspProfile('ts', 'TypeScript')]); + const tools = listApprovalToolDefinitions(tooling); + const groups = groupApprovalToolsByProvider(tools, tooling); + + const lspGroups = groups.filter((g) => g.kind === 'lsp'); + expect(lspGroups.length).toBe(1); + expect(lspGroups[0].label).toBe('TypeScript'); + expect(lspGroups[0].tools.length).toBe(5); + }); + + test('keeps builtins in one group', () => { + const tooling = makeTooling(); + const tools = listApprovalToolDefinitions(tooling); + const groups = groupApprovalToolsByProvider(tools, tooling); + + const builtinGroups = groups.filter((g) => g.kind === 'builtin'); + expect(builtinGroups.length).toBe(1); + expect(builtinGroups[0].label).toBe('Built-in'); + expect(builtinGroups[0].tools.length).toBe(5); + }); + + test('multi-provider tools go into first provider group', () => { + const tooling = makeTooling([ + makeMcpServer('git-a', 'Git A', ['git.status']), + makeMcpServer('git-b', 'Git B', ['git.status']), + ]); + 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('Git A'); + expect(mcpGroups[0].tools[0].providerNames).toEqual(['Git A', 'Git B']); + }); + + test('sorts builtin first, then MCP by name, then LSP by name', () => { + const tooling = makeTooling( + [makeMcpServer('z', 'Zebra', ['z.tool']), makeMcpServer('a', 'Alpha', ['a.tool'])], + [makeLspProfile('ts', 'TypeScript')], + ); + const tools = listApprovalToolDefinitions(tooling); + const groups = groupApprovalToolsByProvider(tools, tooling); + + const kindOrder = groups.map((g) => g.kind); + expect(kindOrder[0]).toBe('builtin'); + const mcpIdx = kindOrder.indexOf('mcp'); + const lspIdx = kindOrder.indexOf('lsp'); + expect(mcpIdx).toBeLessThan(lspIdx); + + const mcpLabels = groups.filter((g) => g.kind === 'mcp').map((g) => g.label); + expect(mcpLabels).toEqual(['Alpha', 'Zebra']); + }); +});