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>
2026-07-24 21:48:36 +02:00 · 2026-03-24 20:20:56 +01:00
parent 130b114906
commit b4330ec01d
2 changed files with 199 additions and 142 deletions
@@ -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 (
    <div className="flex h-full flex-col">
@@ -281,7 +282,7 @@ export function ActivityPanel({
        </div>

        {/* ── Tools section ────────────────────────────────── */}
-        <div>
+        <div className="mb-4">
          <SectionHeader>
            <Server className="size-3" />
            <span>Tools</span>
@@ -341,75 +342,71 @@ export function ActivityPanel({
        </div>

        {/* ── Auto-approval overrides section ──────────────── */}
-        <div className="mb-4">
-          <SectionHeader>
-            <ShieldCheck className="size-3" />
-            <span>Auto-Approval</span>
-            {approvalDisabled && (
-              <span className="ml-auto text-[9px] font-medium normal-case tracking-normal text-zinc-600">
-                {projectIsScratchpad ? 'Scratchpad' : 'Running'}
+        {hasToolCallApproval && !projectIsScratchpad && (
+          <div className="mb-4">
+            <SectionHeader>
+              <ShieldCheck className="size-3" />
+              <span>Auto-Approval</span>
+              <span className="rounded-full bg-zinc-800 px-1.5 py-0.5 text-[9px] tabular-nums text-zinc-500">
+                {effectiveAutoApproved.size}/{approvalTools.length}
              </span>
-            )}
-          </SectionHeader>
+              {approvalDisabled && (
+                <span className="ml-auto text-[9px] font-medium normal-case tracking-normal text-zinc-600">
+                  Running
+                </span>
+              )}
+            </SectionHeader>

-          <div className="rounded-lg border border-zinc-800 bg-zinc-900/40 px-3 py-2.5">
-            {projectIsScratchpad ? (
-              <p className="text-[11px] leading-relaxed text-zinc-600">
-                Tool auto-approval does not apply to scratchpad sessions.
-              </p>
-            ) : approvalTools.length === 0 ? (
-              <p className="text-[11px] leading-relaxed text-zinc-600">
-                No approval-capable runtime tools are currently available.
-              </p>
-            ) : (
-              <>
-                {/* Override state badge + reset action */}
-                <div className="mb-2 flex items-center gap-2">
-                  <span className={`rounded-full px-2 py-0.5 text-[9px] font-semibold uppercase tracking-wider ${
-                    isOverridden
-                      ? 'bg-amber-500/15 text-amber-400'
-                      : 'bg-zinc-800 text-zinc-500'
-                  }`}>
-                    {isOverridden ? 'Custom for this session' : 'Inheriting pattern defaults'}
-                  </span>
-                  {isOverridden && (
-                    <button
-                      className="flex items-center gap-1 rounded-full px-2 py-0.5 text-[9px] font-medium text-zinc-500 transition hover:bg-zinc-800 hover:text-zinc-300 disabled:cursor-not-allowed disabled:opacity-50"
-                      disabled={approvalDisabled}
-                      onClick={() => onUpdateSessionApprovalSettings({})}
-                      type="button"
-                    >
-                      <RotateCcw className="size-2.5" />
-                      Reset to pattern
-                    </button>
-                  )}
-                </div>
+            <div className="rounded-lg border border-zinc-800 bg-zinc-900/40 px-3 py-2.5">
+              {approvalTools.length === 0 ? (
+                <p className="text-[11px] leading-relaxed text-zinc-600">
+                  No tools available yet. Connect MCP servers or wait for runtime capabilities to load.
+                </p>
+              ) : (
+                <>
+                  {/* Override state badge + reset action */}
+                  <div className="mb-2 flex items-center gap-2">
+                    <span className={`rounded-full px-2 py-0.5 text-[9px] font-semibold uppercase tracking-wider ${
+                      isOverridden
+                        ? 'bg-amber-500/15 text-amber-400'
+                        : 'bg-zinc-800 text-zinc-500'
+                    }`}>
+                      {isOverridden ? 'Session override' : 'Using pattern defaults'}
+                    </span>
+                    {isOverridden && (
+                      <button
+                        className="flex items-center gap-1 rounded-full px-2 py-0.5 text-[9px] font-medium text-zinc-500 transition hover:bg-zinc-800 hover:text-zinc-300 disabled:cursor-not-allowed disabled:opacity-50"
+                        disabled={approvalDisabled}
+                        onClick={() => onUpdateSessionApprovalSettings({})}
+                        type="button"
+                      >
+                        <RotateCcw className="size-2.5" />
+                        Reset
+                      </button>
+                    )}
+                  </div>

-                <div className="space-y-0.5">
-                  {approvalTools.map((tool) => (
-                    <ApprovalOverrideRow
-                      disabled={approvalDisabled}
-                      enabled={effectiveAutoApproved.has(tool.id)}
-                      key={tool.id}
-                      onToggle={() => {
-                        const next = new Set(effectiveAutoApproved);
-                        if (next.has(tool.id)) {
-                          next.delete(tool.id);
-                        } else {
-                          next.add(tool.id);
-                        }
-                        onUpdateSessionApprovalSettings({
-                          autoApprovedToolNames: [...next],
-                        });
-                      }}
-                      tool={tool}
-                    />
-                  ))}
-                </div>
-              </>
-            )}
+                  <ApprovalOverrideGroupedList
+                    approvalDisabled={approvalDisabled}
+                    effectiveAutoApproved={effectiveAutoApproved}
+                    onToggle={(toolId) => {
+                      const next = new Set(effectiveAutoApproved);
+                      if (next.has(toolId)) {
+                        next.delete(toolId);
+                      } else {
+                        next.add(toolId);
+                      }
+                      onUpdateSessionApprovalSettings({
+                        autoApprovedToolNames: [...next],
+                      });
+                    }}
+                    tools={approvalTools}
+                  />
+                </>
+              )}
+            </div>
          </div>
-        </div>
+        )}
      </div>
    </div>
  );
@@ -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<ApprovalToolKind, string> = {
+  builtin: 'Built-in',
+  mcp: 'MCP Servers',
+  lsp: 'Language Servers',
+  mixed: 'Other',
+};
+
+function ApprovalOverrideGroupedList({
+  tools,
+  effectiveAutoApproved,
+  approvalDisabled,
+  onToggle,
+}: {
+  tools: ApprovalToolDefinition[];
+  effectiveAutoApproved: Set<string>;
+  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 (
+    <div>
+      {groups.map((group, i) => (
+        <div key={group.kind}>
+          {showHeaders && (
+            <div className={`text-[9px] font-semibold uppercase tracking-wider text-zinc-600 ${i > 0 ? 'mt-2' : ''} mb-1`}>
+              {approvalKindLabels[group.kind]}
+            </div>
+          )}
+          {group.tools.map((tool) => (
+            <ApprovalOverrideRow
+              disabled={approvalDisabled}
+              enabled={effectiveAutoApproved.has(tool.id)}
+              key={tool.id}
+              onToggle={() => onToggle(tool.id)}
+              tool={tool}
+            />
+          ))}
+        </div>
+      ))}
+    </div>
+  );
+}
+
 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 (
    <button
      className={`flex w-full items-center gap-2 rounded-md px-2 py-1.5 text-left transition ${
@@ -500,19 +541,9 @@ function ApprovalOverrideRow({
      onClick={onToggle}
      type="button"
    >
-      <ShieldCheck className={`size-3 shrink-0 ${enabled ? 'text-indigo-400' : 'text-zinc-600'}`} />
      <div className="min-w-0 flex-1">
-        <div className="flex items-center gap-1.5">
-          <span className="truncate text-[12px] font-medium text-zinc-300">{tool.label}</span>
-          <span className="shrink-0 rounded-full bg-zinc-800 px-1.5 py-0.5 text-[8px] font-semibold uppercase tracking-wider text-zinc-500">
-            {kindBadge}
-          </span>
-        </div>
-        {(tool.description || tool.providerNames.length > 0) && (
-          <div className="truncate text-[10px] text-zinc-600">
-            {tool.description ?? tool.providerNames.join(', ')}
-          </div>
-        )}
+        <span className="truncate text-[12px] font-medium text-zinc-300">{tool.label}</span>
+        {detail && <div className="truncate text-[10px] text-zinc-600">{detail}</div>}
      </div>
      <ToggleSwitch enabled={enabled} />
    </button>
@@ -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({
            </div>
          </section>

-          {/* Tool auto-approval defaults */}
-          <section className="space-y-4">
-            <h4 className="text-[12px] font-semibold uppercase tracking-wider text-zinc-500">
-              Tool Auto-Approval Defaults
-            </h4>
+          {/* Tool auto-approval defaults — only relevant when tool-call approval is on */}
+          {isCheckpointEnabled('tool-call') && (
+            <section className="space-y-4">
+              <h4 className="text-[12px] font-semibold uppercase tracking-wider text-zinc-500">
+                Tool Auto-Approval Defaults
+              </h4>

-            <p className="text-[11px] leading-relaxed text-zinc-600">
-              When tool-call approval is enabled, these tools will be auto-approved without manual review.
-              Sessions can override these defaults from the Activity panel.
-            </p>
+              <p className="text-[11px] leading-relaxed text-zinc-600">
+                Tools marked as auto-approved will skip manual review.
+                Sessions can override these defaults from the Activity panel.
+              </p>

-            <div className="rounded-xl border border-zinc-800 bg-zinc-900/50 px-4 py-3">
-              {approvalTools.length === 0 ? (
-                <p className="py-2 text-center text-[11px] text-zinc-600">
-                  No approval-capable runtime tools are currently available.
-                </p>
-              ) : (
-                <div className="space-y-0.5">
-                  {approvalTools.map((tool) => (
-                    <ToolApprovalToggleRow
-                      enabled={autoApprovedSet.has(tool.id)}
-                      key={tool.id}
-                      onToggle={() => toggleToolAutoApproval(tool.id)}
-                      tool={tool}
-                    />
-                  ))}
-                </div>
-              )}
-            </div>
-          </section>
+              <div className="rounded-xl border border-zinc-800 bg-zinc-900/50 px-4 py-3">
+                {approvalTools.length === 0 ? (
+                  <p className="py-2 text-center text-[11px] text-zinc-600">
+                    No tools available yet. Connect MCP servers or wait for runtime capabilities to load.
+                  </p>
+                ) : (
+                  <ToolApprovalGroupedList
+                    autoApprovedSet={autoApprovedSet}
+                    onToggle={toggleToolAutoApproval}
+                    tools={approvalTools}
+                  />
+                )}
+              </div>
+            </section>
+          )}
        </div>
      </div>
    </div>
@@ -595,21 +593,19 @@ export function PatternEditor({

 /* ── Toggle switch ─────────────────────────────────────────── */

-function ToggleSwitch({ enabled, onToggle }: { enabled: boolean; onToggle: () => void }) {
+function ToggleSwitch({ enabled }: { enabled: boolean }) {
  return (
-    <button
+    <span
      className={`relative inline-flex h-[18px] w-[32px] shrink-0 items-center rounded-full transition-colors ${
        enabled ? 'bg-indigo-500' : 'bg-zinc-700'
      }`}
-      onClick={onToggle}
-      type="button"
    >
      <span
        className={`inline-block size-[14px] rounded-full bg-white shadow-sm transition-transform ${
          enabled ? 'translate-x-[16px]' : 'translate-x-[2px]'
        }`}
      />
-    </button>
+    </span>
  );
 }

@@ -646,14 +642,14 @@ function ApprovalCheckpointRow({

  return (
    <div className="rounded-xl border border-zinc-800 bg-zinc-900/50 p-4">
-      <div className="flex items-center gap-3">
+      <button className="flex w-full items-center gap-3 text-left" onClick={() => onToggle(!enabled)} type="button">
        <ShieldCheck className={`size-4 shrink-0 ${enabled ? 'text-indigo-400' : 'text-zinc-600'}`} />
        <div className="min-w-0 flex-1">
          <span className="text-[12px] font-medium text-zinc-200">{label}</span>
          <p className="text-[11px] text-zinc-500">{description}</p>
        </div>
-        <ToggleSwitch enabled={enabled} onToggle={() => onToggle(!enabled)} />
-      </div>
+        <ToggleSwitch enabled={enabled} />
+      </button>

      {/* 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<ApprovalToolKind, string> = {
+  builtin: 'Built-in',
+  mcp: 'MCP Servers',
+  lsp: 'Language Servers',
+  mixed: 'Other',
+};
+
+function ToolApprovalGroupedList({
+  tools,
+  autoApprovedSet,
+  onToggle,
+}: {
+  tools: ApprovalToolDefinition[];
+  autoApprovedSet: Set<string>;
+  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 (
+    <div>
+      {groups.map((group, i) => (
+        <div key={group.kind}>
+          {showHeaders && (
+            <div className={`text-[9px] font-semibold uppercase tracking-wider text-zinc-600 ${i > 0 ? 'mt-3' : ''} mb-1`}>
+              {approvalKindLabels[group.kind]}
+            </div>
+          )}
+          {group.tools.map((tool) => (
+            <ToolApprovalToggleRow
+              enabled={autoApprovedSet.has(tool.id)}
+              key={tool.id}
+              onToggle={() => onToggle(tool.id)}
+              tool={tool}
+            />
+          ))}
+        </div>
+      ))}
+    </div>
+  );
+}

 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 (
    <button
      className="flex w-full items-center gap-2.5 rounded-lg px-2 py-1.5 text-left transition hover:bg-zinc-800/60"
@@ -736,19 +771,10 @@ function ToolApprovalToggleRow({
      type="button"
    >
      <div className="min-w-0 flex-1">
-        <div className="flex items-center gap-2">
-          <span className="truncate text-[12px] font-medium text-zinc-300">{tool.label}</span>
-          <span className="shrink-0 rounded-full bg-zinc-800 px-1.5 py-0.5 text-[8px] font-semibold uppercase tracking-wider text-zinc-500">
-            {kindBadge}
-          </span>
-        </div>
-        {(tool.description || tool.providerNames.length > 0) && (
-          <div className="truncate text-[10px] text-zinc-600">
-            {tool.description ?? tool.providerNames.join(', ')}
-          </div>
-        )}
+        <span className="truncate text-[12px] font-medium text-zinc-300">{tool.label}</span>
+        {detail && <div className="truncate text-[10px] text-zinc-600">{detail}</div>}
      </div>
-      <ToggleSwitch enabled={enabled} onToggle={onToggle} />
+      <ToggleSwitch enabled={enabled} />
    </button>
  );
 }