From b4330ec01db573324e7ea4df405e88d6da81a140 Mon Sep 17 00:00:00 2001 From: David Kaya Date: Tue, 24 Mar 2026 20:20:56 +0100 Subject: [PATCH] fix: improve approval auto-approval UX - fix double-toggle bug in PatternEditor (nested buttons caused clicks to cancel out) - only show tool auto-approval section when tool-call checkpoint is enabled - group tools by kind (built-in, MCP, LSP) with sub-headers when mixed - remove redundant per-row shield icons and kind badges from ActivityPanel - fix missing bottom margin on Tools section in ActivityPanel - shorten and clarify status badges and empty-state messages Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/renderer/components/ActivityPanel.tsx | 203 +++++++++++++--------- src/renderer/components/PatternEditor.tsx | 138 +++++++++------ 2 files changed, 199 insertions(+), 142 deletions(-) diff --git a/src/renderer/components/ActivityPanel.tsx b/src/renderer/components/ActivityPanel.tsx index 6451cc3..eb2f9e6 100644 --- a/src/renderer/components/ActivityPanel.tsx +++ b/src/renderer/components/ActivityPanel.tsx @@ -23,7 +23,7 @@ import type { SessionToolingSelection, WorkspaceToolingSettings, } from '@shared/domain/tooling'; -import { listApprovalToolDefinitions, type ApprovalToolDefinition } from '@shared/domain/tooling'; +import { listApprovalToolDefinitions, type ApprovalToolDefinition, type ApprovalToolKind } from '@shared/domain/tooling'; import type { SessionApprovalSettings } from '@shared/domain/approval'; import { ProviderIcon } from './ProviderIcons'; @@ -211,6 +211,7 @@ export function ActivityPanel({ const approvalDisabled = isBusy || projectIsScratchpad; const accent = modeAccent[pattern.mode] ?? modeAccent.single; const hasTools = mcpServers.length > 0 || lspProfiles.length > 0; + const hasToolCallApproval = pattern.approvalPolicy?.rules.some((r) => r.kind === 'tool-call') ?? false; return (
@@ -281,7 +282,7 @@ export function ActivityPanel({
{/* ── Tools section ────────────────────────────────── */} -
+
Tools @@ -341,75 +342,71 @@ export function ActivityPanel({
{/* ── Auto-approval overrides section ──────────────── */} -
- - - Auto-Approval - {approvalDisabled && ( - - {projectIsScratchpad ? 'Scratchpad' : 'Running'} + {hasToolCallApproval && !projectIsScratchpad && ( +
+ + + Auto-Approval + + {effectiveAutoApproved.size}/{approvalTools.length} - )} - + {approvalDisabled && ( + + Running + + )} + -
- {projectIsScratchpad ? ( -

- Tool auto-approval does not apply to scratchpad sessions. -

- ) : approvalTools.length === 0 ? ( -

- No approval-capable runtime tools are currently available. -

- ) : ( - <> - {/* Override state badge + reset action */} -
- - {isOverridden ? 'Custom for this session' : 'Inheriting pattern defaults'} - - {isOverridden && ( - - )} -
+
+ {approvalTools.length === 0 ? ( +

+ No tools available yet. Connect MCP servers or wait for runtime capabilities to load. +

+ ) : ( + <> + {/* Override state badge + reset action */} +
+ + {isOverridden ? 'Session override' : 'Using pattern defaults'} + + {isOverridden && ( + + )} +
-
- {approvalTools.map((tool) => ( - { - const next = new Set(effectiveAutoApproved); - if (next.has(tool.id)) { - next.delete(tool.id); - } else { - next.add(tool.id); - } - onUpdateSessionApprovalSettings({ - autoApprovedToolNames: [...next], - }); - }} - tool={tool} - /> - ))} -
- - )} + { + const next = new Set(effectiveAutoApproved); + if (next.has(toolId)) { + next.delete(toolId); + } else { + next.add(toolId); + } + onUpdateSessionApprovalSettings({ + autoApprovedToolNames: [...next], + }); + }} + tools={approvalTools} + /> + + )} +
-
+ )}
); @@ -473,6 +470,56 @@ function toggleId(current: string[], id: string): string[] { : [...current, id]; } +/* ── Approval override grouped list ─────────────────────────── */ + +const approvalKindOrder: ApprovalToolKind[] = ['builtin', 'mcp', 'lsp', 'mixed']; +const approvalKindLabels: Record = { + builtin: 'Built-in', + mcp: 'MCP Servers', + lsp: 'Language Servers', + mixed: 'Other', +}; + +function ApprovalOverrideGroupedList({ + tools, + effectiveAutoApproved, + approvalDisabled, + onToggle, +}: { + tools: ApprovalToolDefinition[]; + effectiveAutoApproved: Set; + approvalDisabled: boolean; + onToggle: (toolId: string) => void; +}) { + const groups = approvalKindOrder + .map((kind) => ({ kind, tools: tools.filter((t) => t.kind === kind) })) + .filter((g) => g.tools.length > 0); + const showHeaders = groups.length > 1; + + return ( +
+ {groups.map((group, i) => ( +
+ {showHeaders && ( +
0 ? 'mt-2' : ''} mb-1`}> + {approvalKindLabels[group.kind]} +
+ )} + {group.tools.map((tool) => ( + onToggle(tool.id)} + tool={tool} + /> + ))} +
+ ))} +
+ ); +} + function ApprovalOverrideRow({ tool, enabled, @@ -484,13 +531,7 @@ function ApprovalOverrideRow({ disabled: boolean; onToggle: () => void; }) { - const kindBadge = tool.kind === 'builtin' - ? 'Built-in' - : tool.kind === 'lsp' - ? 'LSP' - : tool.kind === 'mcp' - ? 'MCP' - : 'Mixed'; + const detail = tool.description || (tool.providerNames.length > 0 ? tool.providerNames.join(', ') : undefined); return ( diff --git a/src/renderer/components/PatternEditor.tsx b/src/renderer/components/PatternEditor.tsx index 0c0447d..5dfbb96 100644 --- a/src/renderer/components/PatternEditor.tsx +++ b/src/renderer/components/PatternEditor.tsx @@ -32,6 +32,7 @@ import { import { listApprovalToolDefinitions, type ApprovalToolDefinition, + type ApprovalToolKind, type RuntimeToolDefinition, type WorkspaceToolingSettings, } from '@shared/domain/tooling'; @@ -557,36 +558,33 @@ export function PatternEditor({ - {/* Tool auto-approval defaults */} -
-

- Tool Auto-Approval Defaults -

+ {/* Tool auto-approval defaults — only relevant when tool-call approval is on */} + {isCheckpointEnabled('tool-call') && ( +
+

+ Tool Auto-Approval Defaults +

-

- When tool-call approval is enabled, these tools will be auto-approved without manual review. - Sessions can override these defaults from the Activity panel. -

+

+ Tools marked as auto-approved will skip manual review. + Sessions can override these defaults from the Activity panel. +

-
- {approvalTools.length === 0 ? ( -

- No approval-capable runtime tools are currently available. -

- ) : ( -
- {approvalTools.map((tool) => ( - toggleToolAutoApproval(tool.id)} - tool={tool} - /> - ))} -
- )} -
-
+
+ {approvalTools.length === 0 ? ( +

+ No tools available yet. Connect MCP servers or wait for runtime capabilities to load. +

+ ) : ( + + )} +
+
+ )} @@ -595,21 +593,19 @@ export function PatternEditor({ /* ── Toggle switch ─────────────────────────────────────────── */ -function ToggleSwitch({ enabled, onToggle }: { enabled: boolean; onToggle: () => void }) { +function ToggleSwitch({ enabled }: { enabled: boolean }) { return ( - + ); } @@ -646,14 +642,14 @@ function ApprovalCheckpointRow({ return (
-
+
+ + {/* Agent scope selector */} {enabled && agents.length > 1 && ( @@ -711,7 +707,52 @@ function ApprovalCheckpointRow({ ); } -/* ── Tool auto-approval toggle row ─────────────────────────── */ +/* ── Tool auto-approval grouped list ───────────────────────── */ + +const approvalKindOrder: ApprovalToolKind[] = ['builtin', 'mcp', 'lsp', 'mixed']; +const approvalKindLabels: Record = { + builtin: 'Built-in', + mcp: 'MCP Servers', + lsp: 'Language Servers', + mixed: 'Other', +}; + +function ToolApprovalGroupedList({ + tools, + autoApprovedSet, + onToggle, +}: { + tools: ApprovalToolDefinition[]; + autoApprovedSet: Set; + onToggle: (toolId: string) => void; +}) { + const groups = approvalKindOrder + .map((kind) => ({ kind, tools: tools.filter((t) => t.kind === kind) })) + .filter((g) => g.tools.length > 0); + const showHeaders = groups.length > 1; + + return ( +
+ {groups.map((group, i) => ( +
+ {showHeaders && ( +
0 ? 'mt-3' : ''} mb-1`}> + {approvalKindLabels[group.kind]} +
+ )} + {group.tools.map((tool) => ( + onToggle(tool.id)} + tool={tool} + /> + ))} +
+ ))} +
+ ); +} function ToolApprovalToggleRow({ tool, @@ -722,13 +763,7 @@ function ToolApprovalToggleRow({ enabled: boolean; onToggle: () => void; }) { - const kindBadge = tool.kind === 'builtin' - ? 'Built-in' - : tool.kind === 'lsp' - ? 'LSP' - : tool.kind === 'mcp' - ? 'MCP' - : 'Mixed'; + const detail = tool.description || (tool.providerNames.length > 0 ? tool.providerNames.join(', ') : undefined); return ( ); }