mirror of
https://github.com/davidkaya/aryx.git
synced 2026-07-24 05:28:46 +02:00
feat: category-based approval for runtime tools
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>
This commit is contained in:
@@ -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 =
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -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" />
|
||||
|
||||
@@ -93,6 +93,7 @@ export interface ResolveApprovalCommand {
|
||||
requestId: string;
|
||||
approvalId: string;
|
||||
decision: ApprovalDecision;
|
||||
alwaysApprove: boolean;
|
||||
}
|
||||
|
||||
export interface ResolveUserInputCommand {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
Reference in New Issue
Block a user