From 3ec69d990b108f39581e53273c43feef25a6e097 Mon Sep 17 00:00:00 2001 From: David Kaya Date: Fri, 27 Mar 2026 21:20:14 +0100 Subject: [PATCH] feat: enable 'always approve for session' for runtime tools Runtime tool permission requests (read, write, shell, store_memory) now resolve stable approval names via fallback in the sidecar approval coordinator, enabling the 'Always approve' button and session-level auto-approval for built-in tools. Backend: - Add fallback tool names for PermissionRequestRead (read), PermissionRequestWrite (write), PermissionRequestShell (shell), and PermissionRequestMemory (store_memory) - Add autoApprovedToolName parameter to RequiresToolCallApproval for permission-kind-based session approval matching - Track tool.execution_start events in ToolNamesByCallId for supplementary exact-name resolution Frontend: - Use approval.toolName ?? approval.permissionKind as fallback key in resolveSessionApproval and ApprovalBanner - Add shell, read, write permission-kind entries to builtin approval tool definitions so pruning preserves session overrides Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Services/CopilotApprovalCoordinator.cs | 45 ++++++++++-- .../Services/CopilotTurnExecutionState.cs | 5 ++ .../CopilotTurnExecutionStateTests.cs | 25 +++++++ .../CopilotWorkflowRunnerTests.cs | 70 ++++++++++++++++++- src/main/AryxAppService.ts | 7 +- .../components/chat/ApprovalBanner.tsx | 7 +- src/shared/domain/tooling.ts | 8 +++ tests/shared/approval.test.ts | 31 ++++++++ tests/shared/tooling.test.ts | 16 ++++- 9 files changed, 199 insertions(+), 15 deletions(-) diff --git a/sidecar/src/Aryx.AgentHost/Services/CopilotApprovalCoordinator.cs b/sidecar/src/Aryx.AgentHost/Services/CopilotApprovalCoordinator.cs index 0c8d948..b2b1315 100644 --- a/sidecar/src/Aryx.AgentHost/Services/CopilotApprovalCoordinator.cs +++ b/sidecar/src/Aryx.AgentHost/Services/CopilotApprovalCoordinator.cs @@ -9,6 +9,7 @@ internal sealed class CopilotApprovalCoordinator private const string ApprovedDecision = "approved"; private const string RejectedDecision = "rejected"; private const string ToolCallApprovalKind = "tool-call"; + private const string StoreMemoryToolName = "store_memory"; private const string WebFetchToolName = "web_fetch"; private const string ShellPermissionKind = "shell"; private const string WritePermissionKind = "write"; @@ -49,7 +50,8 @@ internal sealed class CopilotApprovalCoordinator CancellationToken cancellationToken) { string? toolName = ResolveApprovalToolName(request, toolNamesByCallId); - if (!RequiresToolCallApproval(command.Pattern.ApprovalPolicy, agent.Id, toolName)) + string? autoApprovedToolName = ResolveAutoApprovedToolName(request); + if (!RequiresToolCallApproval(command.Pattern.ApprovalPolicy, agent.Id, toolName, autoApprovedToolName)) { return CreateApprovalResult(PermissionRequestResultKind.Approved); } @@ -216,7 +218,8 @@ internal sealed class CopilotApprovalCoordinator internal static bool RequiresToolCallApproval( ApprovalPolicyDto? approvalPolicy, string agentId, - string? toolName) + string? toolName, + string? autoApprovedToolName = null) { if (approvalPolicy?.Rules is null || approvalPolicy.Rules.Count == 0) { @@ -228,9 +231,13 @@ internal sealed class CopilotApprovalCoordinator return false; } - return string.IsNullOrWhiteSpace(toolName) - || !approvalPolicy.AutoApprovedToolNames.Any(candidate => - string.Equals(candidate, toolName, StringComparison.OrdinalIgnoreCase)); + IReadOnlyList autoApprovedToolNames = approvalPolicy.AutoApprovedToolNames; + if (autoApprovedToolNames.Count == 0) + { + return true; + } + + return !MatchesAutoApprovedTool(autoApprovedToolNames, toolName, autoApprovedToolName); } internal static bool TryGetApprovalToolName( @@ -293,6 +300,11 @@ internal sealed class CopilotApprovalCoordinator ?? GetFallbackToolName(request); } + private static string? ResolveAutoApprovedToolName(PermissionRequest request) + { + return GetFallbackToolName(request); + } + private static string? GetDirectToolName(PermissionRequest request) { return request switch @@ -344,10 +356,33 @@ internal sealed class CopilotApprovalCoordinator return request switch { PermissionRequestUrl => WebFetchToolName, + PermissionRequestShell => ShellPermissionKind, + PermissionRequestWrite => WritePermissionKind, + PermissionRequestRead => ReadPermissionKind, + PermissionRequestMemory => StoreMemoryToolName, _ => null, }; } + private static bool MatchesAutoApprovedTool( + IReadOnlyList autoApprovedToolNames, + string? toolName, + string? autoApprovedToolName) + { + return MatchesAutoApprovedToolName(autoApprovedToolNames, toolName) + || MatchesAutoApprovedToolName(autoApprovedToolNames, autoApprovedToolName); + } + + private static bool MatchesAutoApprovedToolName( + IReadOnlyList autoApprovedToolNames, + string? toolName) + { + string? normalizedToolName = NormalizeOptionalString(toolName); + return normalizedToolName is not null + && autoApprovedToolNames.Any(candidate => + string.Equals(candidate, normalizedToolName, StringComparison.OrdinalIgnoreCase)); + } + private PendingApprovalRequest GetPendingApproval(string approvalId) { if (_pendingApprovals.TryGetValue(approvalId, out PendingApprovalRequest? pending)) diff --git a/sidecar/src/Aryx.AgentHost/Services/CopilotTurnExecutionState.cs b/sidecar/src/Aryx.AgentHost/Services/CopilotTurnExecutionState.cs index 2d3e0b7..d2d2ed0 100644 --- a/sidecar/src/Aryx.AgentHost/Services/CopilotTurnExecutionState.cs +++ b/sidecar/src/Aryx.AgentHost/Services/CopilotTurnExecutionState.cs @@ -77,6 +77,11 @@ internal sealed class CopilotTurnExecutionState RecordObservedAgentForMessage(agent, assistantMessage.Data!.MessageId); QueueThinkingIfNeeded(agent); break; + case ToolExecutionStartEvent toolExecutionStart + when !string.IsNullOrWhiteSpace(toolExecutionStart.Data?.ToolCallId) + && !string.IsNullOrWhiteSpace(toolExecutionStart.Data?.ToolName): + ToolNamesByCallId[toolExecutionStart.Data.ToolCallId.Trim()] = toolExecutionStart.Data.ToolName.Trim(); + break; case AssistantReasoningDeltaEvent: ActiveAgent = agent; QueueThinkingIfNeeded(agent); diff --git a/sidecar/tests/Aryx.AgentHost.Tests/CopilotTurnExecutionStateTests.cs b/sidecar/tests/Aryx.AgentHost.Tests/CopilotTurnExecutionStateTests.cs index a6e6641..cb74dba 100644 --- a/sidecar/tests/Aryx.AgentHost.Tests/CopilotTurnExecutionStateTests.cs +++ b/sidecar/tests/Aryx.AgentHost.Tests/CopilotTurnExecutionStateTests.cs @@ -58,6 +58,31 @@ public sealed class CopilotTurnExecutionStateTests Assert.Equal("agent-1", observedAgent.AgentId); } + [Fact] + public void ObserveSessionEvent_ToolExecutionStart_TracksToolNameByCallId() + { + RunTurnCommandDto command = CreateCommand(); + CopilotTurnExecutionState state = new(command); + + state.ObserveSessionEvent( + command.Pattern.Agents[0], + SessionEvent.FromJson( + """ + { + "type": "tool.execution_start", + "data": { + "toolCallId": "tool-call-1", + "toolName": "view" + }, + "id": "33333333-3333-3333-3333-333333333333", + "timestamp": "2026-03-27T00:00:00Z" + } + """)); + + Assert.True(state.ToolNamesByCallId.TryGetValue("tool-call-1", out string? toolName)); + Assert.Equal("view", toolName); + } + [Fact] public async Task EmitThinkingIfNeeded_DoesNotDuplicateQueuedThinkingActivity() { diff --git a/sidecar/tests/Aryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs b/sidecar/tests/Aryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs index b4ad8e0..3427ce4 100644 --- a/sidecar/tests/Aryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs +++ b/sidecar/tests/Aryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs @@ -766,7 +766,29 @@ public sealed class CopilotWorkflowRunnerTests } [Fact] - public void TryGetApprovalToolName_ReadsMcpCustomAndHookRequests() + public void RequiresToolCallApproval_HonorsRuntimeApprovalAliases() + { + ApprovalPolicyDto policy = new() + { + Rules = + [ + new ApprovalCheckpointRuleDto + { + Kind = "tool-call", + AgentIds = ["agent-1"], + }, + ], + AutoApprovedToolNames = ["read", "store_memory"], + }; + + Assert.False(CopilotApprovalCoordinator.RequiresToolCallApproval(policy, "agent-1", "view", "read")); + Assert.False(CopilotApprovalCoordinator.RequiresToolCallApproval(policy, "agent-1", "remember_fact", "store_memory")); + Assert.True(CopilotApprovalCoordinator.RequiresToolCallApproval(policy, "agent-1", "write_file", "write")); + Assert.True(CopilotApprovalCoordinator.RequiresToolCallApproval(policy, "agent-1", "git.status")); + } + + [Fact] + public void TryGetApprovalToolName_ResolvesDirectNamesAndRuntimeFallbacks() { Assert.True( CopilotApprovalCoordinator.TryGetApprovalToolName( @@ -804,7 +826,7 @@ public sealed class CopilotWorkflowRunnerTests out string? hookToolName)); Assert.Equal("web_fetch", hookToolName); - Assert.False( + Assert.True( CopilotApprovalCoordinator.TryGetApprovalToolName( new PermissionRequestShell { @@ -818,7 +840,49 @@ public sealed class CopilotWorkflowRunnerTests CanOfferSessionApproval = false, }, out string? shellToolName)); - Assert.Null(shellToolName); + Assert.Equal("shell", shellToolName); + } + + [Fact] + public void TryGetApprovalToolName_FallsBackToRuntimeApprovalAliasesWhenLookupMissing() + { + Assert.True( + CopilotApprovalCoordinator.TryGetApprovalToolName( + new PermissionRequestRead + { + Kind = "read", + ToolCallId = "tool-call-read", + Intention = "Inspect a file", + Path = "README.md", + }, + out string? readToolName)); + Assert.Equal("read", readToolName); + + Assert.True( + CopilotApprovalCoordinator.TryGetApprovalToolName( + new PermissionRequestWrite + { + Kind = "write", + ToolCallId = "tool-call-write", + Intention = "Update a file", + FileName = "README.md", + Diff = "@@ -1 +1 @@", + }, + out string? writeToolName)); + Assert.Equal("write", writeToolName); + + Assert.True( + CopilotApprovalCoordinator.TryGetApprovalToolName( + new PermissionRequestMemory + { + Kind = "memory", + ToolCallId = "tool-call-memory", + Subject = "repo conventions", + Fact = "Use Bun for script execution.", + Citations = "package.json", + }, + out string? memoryToolName)); + Assert.Equal("store_memory", memoryToolName); } [Fact] diff --git a/src/main/AryxAppService.ts b/src/main/AryxAppService.ts index 4171705..d33d8e9 100644 --- a/src/main/AryxAppService.ts +++ b/src/main/AryxAppService.ts @@ -729,10 +729,11 @@ export class AryxAppService extends EventEmitter { this.setSessionPendingApprovalState(session, dequeuePendingApprovalState(session, approvalId)); session.updatedAt = resolvedAt; - if (decision === 'approved' && alwaysApprove && approval.toolName) { + const approvalKey = approval.toolName ?? approval.permissionKind; + if (decision === 'approved' && alwaysApprove && approvalKey) { const existing = session.approvalSettings?.autoApprovedToolNames ?? []; - if (!existing.includes(approval.toolName)) { - session.approvalSettings = { autoApprovedToolNames: [...existing, approval.toolName] }; + if (!existing.includes(approvalKey)) { + session.approvalSettings = { autoApprovedToolNames: [...existing, approvalKey] }; } } diff --git a/src/renderer/components/chat/ApprovalBanner.tsx b/src/renderer/components/chat/ApprovalBanner.tsx index 5c0629b..1fd9afe 100644 --- a/src/renderer/components/chat/ApprovalBanner.tsx +++ b/src/renderer/components/chat/ApprovalBanner.tsx @@ -23,7 +23,8 @@ 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 canAlwaysApprove = approval.kind === 'tool-call' && !!approval.toolName; + const approvalToolKey = approval.toolName ?? approval.permissionKind; + const canAlwaysApprove = approval.kind === 'tool-call' && !!approvalToolKey; return (
@@ -90,11 +91,11 @@ export function ApprovalBanner({ {canAlwaysApprove && (