From 689b3352204318b9e99983f949d5bcd1d6fcc0ea Mon Sep 17 00:00:00 2001
From: David Kaya <david@kaya.sk>
Date: Fri, 27 Mar 2026 21:36:27 +0100
Subject: [PATCH] feat: category-based approval for runtime tools
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Replace 20+ individual runtime tool toggles with 5 permission categories
(read, write, shell, web_fetch, store_memory) matching how Copilot CLI,
Cline, and other coding agents handle approvals.

- Add resolveApprovalToolKey() to map permission kinds to category IDs
- Simplify fallbackRuntimeApprovalTools to 5 categories
- Thread alwaysApprove through PendingApprovalHandle → sidecar resolve
- Update ApprovalBanner to use category labels for Always Approve button
- Update ResolveApprovalCommand contract with alwaysApprove field
- Update tests for category-based model

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 src/main/AryxAppService.ts                    | 13 ++---
 src/main/sidecar/sidecarProcess.ts            |  3 +-
 .../components/chat/ApprovalBanner.tsx        |  9 ++--
 src/shared/contracts/sidecar.ts               |  1 +
 src/shared/domain/approval.ts                 | 22 ++++++++
 src/shared/domain/tooling.ts                  | 45 ++++------------
 tests/shared/approval.test.ts                 | 21 ++++++++
 tests/shared/tooling.test.ts                  | 51 +++++++++----------
 8 files changed, 93 insertions(+), 72 deletions(-)

diff --git a/src/main/AryxAppService.ts b/src/main/AryxAppService.ts
index d33d8e9..4822f0c 100644
--- a/src/main/AryxAppService.ts
+++ b/src/main/AryxAppService.ts
@@ -42,6 +42,7 @@ import {
   normalizeSessionApprovalSettings,
   pruneApprovalPolicyTools,
   pruneSessionApprovalSettings,
+  resolveApprovalToolKey,
   resolvePendingApproval,
   type ApprovalDecision,
   type PendingApprovalMessageRecord,
@@ -124,7 +125,7 @@ type AppServiceEvents = {
 type PendingApprovalHandle = {
   sessionId: string;
   requestId: string;
-  resolve: (decision: ApprovalDecision) => void | Promise<void>;
+  resolve: (decision: ApprovalDecision, alwaysApprove?: boolean) => void | Promise<void>;
 };
 
 type PendingUserInputHandle = {
@@ -635,8 +636,8 @@ export class AryxAppService extends EventEmitter<AppServiceEvents> {
           await this.applyAgentActivity(workspace, session.id, requestId, event);
         },
         async (event) => {
-          await this.handleApprovalRequested(workspace, session.id, requestId, event, (decision) =>
-            this.sidecar.resolveApproval(event.approvalId, decision));
+          await this.handleApprovalRequested(workspace, session.id, requestId, event, (decision, alwaysApprove) =>
+            this.sidecar.resolveApproval(event.approvalId, decision, alwaysApprove));
         },
         async (event) => {
           await this.handleUserInputRequested(workspace, session.id, requestId, event, (answer, wasFreeform) =>
@@ -729,7 +730,7 @@ export class AryxAppService extends EventEmitter<AppServiceEvents> {
     this.setSessionPendingApprovalState(session, dequeuePendingApprovalState(session, approvalId));
     session.updatedAt = resolvedAt;
 
-    const approvalKey = approval.toolName ?? approval.permissionKind;
+    const approvalKey = resolveApprovalToolKey(approval.toolName, approval.permissionKind);
     if (decision === 'approved' && alwaysApprove && approvalKey) {
       const existing = session.approvalSettings?.autoApprovedToolNames ?? [];
       if (!existing.includes(approvalKey)) {
@@ -748,7 +749,7 @@ export class AryxAppService extends EventEmitter<AppServiceEvents> {
     this.pendingApprovalHandles.delete(approvalId);
 
     try {
-      await Promise.resolve(handle.resolve(decision));
+      await Promise.resolve(handle.resolve(decision, alwaysApprove));
     } catch (error) {
       const failedAt = nowIso();
       this.rejectPendingApprovals(
@@ -1396,7 +1397,7 @@ export class AryxAppService extends EventEmitter<AppServiceEvents> {
     sessionId: string,
     requestId: string,
     approval: ApprovalRequestedEvent | PendingApprovalRecord,
-    resolve: (decision: ApprovalDecision) => void | Promise<void>,
+    resolve: (decision: ApprovalDecision, alwaysApprove?: boolean) => void | Promise<void>,
   ): Promise<void> {
     const session = this.requireSession(workspace, sessionId);
     const pendingApproval =
diff --git a/src/main/sidecar/sidecarProcess.ts b/src/main/sidecar/sidecarProcess.ts
index 9257c9a..be795b7 100644
--- a/src/main/sidecar/sidecarProcess.ts
+++ b/src/main/sidecar/sidecarProcess.ts
@@ -120,12 +120,13 @@ export class SidecarClient {
     });
   }
 
-  async resolveApproval(approvalId: string, decision: ApprovalDecision): Promise<void> {
+  async resolveApproval(approvalId: string, decision: ApprovalDecision, alwaysApprove?: boolean): Promise<void> {
     return this.dispatch<void>({
       type: 'resolve-approval',
       requestId: `approval-${Date.now()}`,
       approvalId,
       decision,
+      alwaysApprove: alwaysApprove ?? false,
     });
   }
 
diff --git a/src/renderer/components/chat/ApprovalBanner.tsx b/src/renderer/components/chat/ApprovalBanner.tsx
index 1fd9afe..8d16b14 100644
--- a/src/renderer/components/chat/ApprovalBanner.tsx
+++ b/src/renderer/components/chat/ApprovalBanner.tsx
@@ -3,7 +3,9 @@ import { Bot, Check, ChevronDown, Loader2, ShieldAlert, ShieldBan, ShieldCheck,
 
 import { MarkdownContent } from '@renderer/components/MarkdownContent';
 import { permissionDetailSummary, PermissionDetailView } from '@renderer/components/chat/PermissionDetailView';
+import { resolveApprovalToolKey } from '@shared/domain/approval';
 import type { ApprovalDecision, PendingApprovalRecord } from '@shared/domain/approval';
+import { resolveToolLabel } from '@shared/domain/tooling';
 
 /* ── ApprovalBanner ────────────────────────────────────────── */
 
@@ -23,8 +25,9 @@ export function ApprovalBanner({
   const kindLabel = approval.kind === 'final-response' ? 'Final response review' : 'Tool call approval';
   const hasMessages = approval.messages && approval.messages.length > 0;
   const showPosition = position !== undefined && total !== undefined && total > 1;
-  const approvalToolKey = approval.toolName ?? approval.permissionKind;
+  const approvalToolKey = resolveApprovalToolKey(approval.toolName, approval.permissionKind);
   const canAlwaysApprove = approval.kind === 'tool-call' && !!approvalToolKey;
+  const approvalToolLabel = approvalToolKey ? resolveToolLabel(approvalToolKey) : undefined;
 
   return (
     <div className="rounded-xl border border-amber-500/30 bg-amber-500/5 px-4 py-3" role="alert">
@@ -91,11 +94,11 @@ export function ApprovalBanner({
         </button>
         {canAlwaysApprove && (
           <button
-            aria-label={`Always approve ${approvalToolKey}`}
+            aria-label={`Always approve ${approvalToolLabel}`}
             className="inline-flex items-center gap-1.5 rounded-lg bg-emerald-600/20 px-3.5 py-1.5 text-[12px] font-medium text-emerald-300 transition hover:bg-emerald-600/30 disabled:cursor-not-allowed disabled:opacity-50"
             disabled={isResolving}
             onClick={() => onResolve('approved', true)}
-            title={`Auto-approve "${approvalToolKey}" for the rest of this session`}
+            title={`Auto-approve "${approvalToolLabel}" for the rest of this session`}
             type="button"
           >
             <ShieldBan className="size-3" />
diff --git a/src/shared/contracts/sidecar.ts b/src/shared/contracts/sidecar.ts
index b64cde7..f15e598 100644
--- a/src/shared/contracts/sidecar.ts
+++ b/src/shared/contracts/sidecar.ts
@@ -93,6 +93,7 @@ export interface ResolveApprovalCommand {
   requestId: string;
   approvalId: string;
   decision: ApprovalDecision;
+  alwaysApprove: boolean;
 }
 
 export interface ResolveUserInputCommand {
diff --git a/src/shared/domain/approval.ts b/src/shared/domain/approval.ts
index ea14eef..a3d2c20 100644
--- a/src/shared/domain/approval.ts
+++ b/src/shared/domain/approval.ts
@@ -193,6 +193,28 @@ export function approvalPolicyRequiresCheckpoint(
   return rule.agentIds.includes(normalizedAgentId);
 }
 
+const runtimePermissionKinds: ReadonlySet<string> = new Set(['read', 'write', 'shell', 'memory', 'url']);
+
+/**
+ * Resolves the canonical approval key for a pending approval.
+ *
+ * Runtime tools always use their permission-kind category (`read`, `write`, `shell`)
+ * mapped to the corresponding approval-tool id. MCP/custom/hook tools use their
+ * specific tool name.
+ */
+export function resolveApprovalToolKey(
+  toolName: string | undefined,
+  permissionKind: string | undefined,
+): string | undefined {
+  if (permissionKind && runtimePermissionKinds.has(permissionKind)) {
+    if (permissionKind === 'memory') return 'store_memory';
+    if (permissionKind === 'url') return 'web_fetch';
+    return permissionKind;
+  }
+
+  return toolName;
+}
+
 export function approvalPolicyAutoApprovesTool(
   policy: ApprovalPolicy | undefined,
   toolName?: string,
diff --git a/src/shared/domain/tooling.ts b/src/shared/domain/tooling.ts
index 2fe6588..2d8e4ba 100644
--- a/src/shared/domain/tooling.ts
+++ b/src/shared/domain/tooling.ts
@@ -137,42 +137,16 @@ export function resolveToolLabel(toolId: string): string {
   return builtinToolLabels.get(toolId) ?? toolId;
 }
 
-// Fallback runtime tools used before sidecar capabilities are loaded or when the
-// CLI cannot report its built-in tool catalog dynamically.
+// Fallback approval tool categories for runtime tools. The approval UI groups
+// built-in tools by permission category (read, write, shell, web, memory) rather
+// than listing 20+ individual tools, matching other coding agents (Copilot CLI,
+// Cline, Roo Code, etc.).
 const fallbackRuntimeApprovalTools: ReadonlyArray<RuntimeToolDefinition> = [
-  // Permission-kind approval categories (used by sidecar for runtime tool session approval)
-  { id: 'shell', label: 'Shell commands' },
   { id: 'read', label: 'Read files' },
   { id: 'write', label: 'Write files' },
-  // Shell tools (bash variants — SDK reports these on all platforms)
-  { id: 'bash', label: 'Execute shell commands' },
-  { id: 'read_bash', label: 'Read shell output' },
-  { id: 'write_bash', label: 'Write shell input' },
-  { id: 'stop_bash', label: 'Stop shell session' },
-  { id: 'list_bash', label: 'List shell sessions' },
-  // File tools
-  { id: 'view', label: 'View files' },
-  { id: 'create', label: 'Create files' },
-  { id: 'edit', label: 'Edit files' },
-  { id: 'apply_patch', label: 'Apply patch' },
-  // Search tools
-  { id: 'grep', label: 'Search file contents' },
-  { id: 'glob', label: 'Find files by pattern' },
-  // Agent tools
-  { id: 'task', label: 'Run sub-agent' },
-  { id: 'read_agent', label: 'Read agent results' },
-  { id: 'list_agents', label: 'List agents' },
-  // Web tools
-  { id: 'web_fetch', label: 'Fetch web content' },
-  { id: 'web_search', label: 'Search the web' },
-  // Other tools
-  { id: 'skill', label: 'Invoke skill' },
-  { id: 'show_file', label: 'Show file' },
-  { id: 'fetch_copilot_cli_documentation', label: 'Fetch CLI docs' },
-  { id: 'update_todo', label: 'Update checklist' },
+  { id: 'shell', label: 'Shell commands' },
+  { id: 'web_fetch', label: 'Web access' },
   { id: 'store_memory', label: 'Store memory' },
-  { id: 'sql', label: 'Query session data' },
-  { id: 'lsp', label: 'Language server' },
 ];
 
 export function createWorkspaceSettings(): WorkspaceSettings {
@@ -238,12 +212,13 @@ export function normalizeSessionToolingSelection(
 
 export function listApprovalToolDefinitions(
   tooling: WorkspaceToolingSettings,
-  runtimeTools: ReadonlyArray<RuntimeToolDefinition> = fallbackRuntimeApprovalTools,
+  _runtimeTools: ReadonlyArray<RuntimeToolDefinition> = fallbackRuntimeApprovalTools,
 ): ApprovalToolDefinition[] {
   const toolsById = new Map<string, ApprovalToolDefinition>();
-  const runtimeApprovalTools = runtimeTools.length > 0 ? runtimeTools : fallbackRuntimeApprovalTools;
 
-  for (const tool of runtimeApprovalTools) {
+  // Always use category-level approval for built-in runtime tools regardless of
+  // what the sidecar reports as individual tools.
+  for (const tool of fallbackRuntimeApprovalTools) {
     registerApprovalTool(toolsById, {
       id: tool.id,
       label: resolveToolLabel(tool.id),
diff --git a/tests/shared/approval.test.ts b/tests/shared/approval.test.ts
index b73919e..31bb53d 100644
--- a/tests/shared/approval.test.ts
+++ b/tests/shared/approval.test.ts
@@ -9,6 +9,7 @@ import {
   normalizePendingApprovalState,
   normalizeSessionApprovalSettings,
   pruneSessionApprovalSettings,
+  resolveApprovalToolKey,
   dequeuePendingApprovalState,
   enqueuePendingApprovalState,
   listPendingApprovals,
@@ -240,4 +241,24 @@ describe('approval helpers', () => {
     expect(approvalPolicyRequiresToolCallApproval(effective, 'agent-1', 'shell')).toBe(false);
     expect(approvalPolicyRequiresToolCallApproval(effective, 'agent-1', 'write')).toBe(true);
   });
+
+  test('resolveApprovalToolKey returns permission category for runtime tools', () => {
+    expect(resolveApprovalToolKey('view', 'read')).toBe('read');
+    expect(resolveApprovalToolKey('edit', 'write')).toBe('write');
+    expect(resolveApprovalToolKey('bash', 'shell')).toBe('shell');
+    expect(resolveApprovalToolKey('web_fetch', 'url')).toBe('web_fetch');
+    expect(resolveApprovalToolKey('store_memory', 'memory')).toBe('store_memory');
+  });
+
+  test('resolveApprovalToolKey returns tool name for non-runtime tools', () => {
+    expect(resolveApprovalToolKey('git.status', 'mcp')).toBe('git.status');
+    expect(resolveApprovalToolKey('lsp_ts_hover', 'custom-tool')).toBe('lsp_ts_hover');
+    expect(resolveApprovalToolKey('web_fetch', 'hook')).toBe('web_fetch');
+    expect(resolveApprovalToolKey('some_tool', undefined)).toBe('some_tool');
+  });
+
+  test('resolveApprovalToolKey returns undefined when both are absent', () => {
+    expect(resolveApprovalToolKey(undefined, undefined)).toBeUndefined();
+    expect(resolveApprovalToolKey(undefined, 'mcp')).toBeUndefined();
+  });
 });
diff --git a/tests/shared/tooling.test.ts b/tests/shared/tooling.test.ts
index 26aa9b5..c81181f 100644
--- a/tests/shared/tooling.test.ts
+++ b/tests/shared/tooling.test.ts
@@ -228,7 +228,7 @@ describe('tooling settings helpers', () => {
     });
   });
 
-  test('prefers dynamically reported runtime tools over the fallback builtin catalog', () => {
+  test('always uses category-level builtins regardless of dynamically reported runtime tools', () => {
     const tools = listApprovalToolDefinitions(
       { mcpServers: [], lspProfiles: [] },
       [
@@ -240,15 +240,15 @@ describe('tooling settings helpers', () => {
       ],
     );
 
-    expect(tools).toContainEqual({
-      id: 'fetch',
-      label: 'fetch',
-      description: 'Dynamic runtime tool from Copilot CLI.',
-      kind: 'builtin',
-      providerIds: ['builtin:fetch'],
-      providerNames: ['Built-in'],
-    });
-    expect(tools.some((tool) => tool.id === 'web_fetch')).toBe(false);
+    // Category-level builtins must always be present
+    expect(tools.some((tool) => tool.id === 'read')).toBe(true);
+    expect(tools.some((tool) => tool.id === 'write')).toBe(true);
+    expect(tools.some((tool) => tool.id === 'shell')).toBe(true);
+    expect(tools.some((tool) => tool.id === 'web_fetch')).toBe(true);
+    expect(tools.some((tool) => tool.id === 'store_memory')).toBe(true);
+
+    // Dynamic individual tools do NOT appear in the approval list
+    expect(tools.some((tool) => tool.id === 'fetch')).toBe(false);
   });
 
   test('resolves human-readable labels from the tool metadata registry', () => {
@@ -268,36 +268,33 @@ describe('tooling settings helpers', () => {
     expect(resolveToolLabel('')).toBe('');
   });
 
-  test('fallback catalog includes all standard non-internal tools with friendly labels', () => {
+  test('fallback catalog uses 5 permission-category entries for built-in tools', () => {
     const tools = listApprovalToolDefinitions({ mcpServers: [], lspProfiles: [] });
     const builtinTools = tools.filter((t) => t.kind === 'builtin');
 
-    expect(builtinTools.length).toBeGreaterThanOrEqual(23);
-    expect(builtinTools.some((t) => t.id === 'bash')).toBe(true);
-    expect(builtinTools.some((t) => t.id === 'web_fetch')).toBe(true);
-    expect(builtinTools.some((t) => t.id === 'task')).toBe(true);
-    expect(builtinTools.some((t) => t.id === 'store_memory')).toBe(true);
+    expect(builtinTools.length).toBe(5);
 
-    // Permission-kind approval categories should be included
-    expect(builtinTools.some((t) => t.id === 'shell')).toBe(true);
+    // Permission-kind approval categories
     expect(builtinTools.some((t) => t.id === 'read')).toBe(true);
     expect(builtinTools.some((t) => t.id === 'write')).toBe(true);
+    expect(builtinTools.some((t) => t.id === 'shell')).toBe(true);
+    expect(builtinTools.some((t) => t.id === 'web_fetch')).toBe(true);
+    expect(builtinTools.some((t) => t.id === 'store_memory')).toBe(true);
 
     // Labels should be human-readable, not raw IDs
-    const bashTool = builtinTools.find((t) => t.id === 'bash');
-    expect(bashTool?.label).toBe('Execute shell commands');
-    const shellTool = builtinTools.find((t) => t.id === 'shell');
-    expect(shellTool?.label).toBe('Shell commands');
     const readTool = builtinTools.find((t) => t.id === 'read');
     expect(readTool?.label).toBe('Read files');
     const writeTool = builtinTools.find((t) => t.id === 'write');
     expect(writeTool?.label).toBe('Write files');
+    const shellTool = builtinTools.find((t) => t.id === 'shell');
+    expect(shellTool?.label).toBe('Shell commands');
 
-    // Internal tools should not appear in the fallback
-    expect(builtinTools.some((t) => t.id === 'ask_user')).toBe(false);
-    expect(builtinTools.some((t) => t.id === 'report_intent')).toBe(false);
-    expect(builtinTools.some((t) => t.id === 'task_complete')).toBe(false);
-    expect(builtinTools.some((t) => t.id === 'exit_plan_mode')).toBe(false);
+    // Individual tools should NOT appear in the approval list
+    expect(builtinTools.some((t) => t.id === 'bash')).toBe(false);
+    expect(builtinTools.some((t) => t.id === 'view')).toBe(false);
+    expect(builtinTools.some((t) => t.id === 'edit')).toBe(false);
+    expect(builtinTools.some((t) => t.id === 'grep')).toBe(false);
+    expect(builtinTools.some((t) => t.id === 'task')).toBe(false);
   });
 
   test('resolves workspace and project tooling with accepted discovered MCP servers', () => {