diff --git a/sidecar/src/Aryx.AgentHost/Contracts/ProtocolModels.cs b/sidecar/src/Aryx.AgentHost/Contracts/ProtocolModels.cs index 82ae5eb..e727ebe 100644 --- a/sidecar/src/Aryx.AgentHost/Contracts/ProtocolModels.cs +++ b/sidecar/src/Aryx.AgentHost/Contracts/ProtocolModels.cs @@ -678,6 +678,7 @@ public sealed class ApprovalRequestedEventDto : SidecarEventDto public string? AgentName { get; init; } public string? ToolName { get; init; } public string? PermissionKind { get; init; } + public string? ApprovalToolKey { get; init; } public string Title { get; init; } = string.Empty; public string? Detail { get; init; } public PermissionDetailDto? PermissionDetail { get; init; } diff --git a/sidecar/src/Aryx.AgentHost/Services/Providers/Copilot/CopilotApprovalCoordinator.cs b/sidecar/src/Aryx.AgentHost/Services/Providers/Copilot/CopilotApprovalCoordinator.cs index 293094d..3125892 100644 --- a/sidecar/src/Aryx.AgentHost/Services/Providers/Copilot/CopilotApprovalCoordinator.cs +++ b/sidecar/src/Aryx.AgentHost/Services/Providers/Copilot/CopilotApprovalCoordinator.cs @@ -21,22 +21,33 @@ internal sealed class CopilotApprovalCoordinator private const string HookPermissionKind = "hook"; private const string ToolCallingActivityType = "tool-calling"; - private static readonly Dictionary HookToolCategories = new(StringComparer.OrdinalIgnoreCase) + private static readonly Dictionary HookToolApprovals = new(StringComparer.OrdinalIgnoreCase) { - ["view"] = ReadPermissionKind, - ["glob"] = ReadPermissionKind, - ["grep"] = ReadPermissionKind, - ["lsp"] = ReadPermissionKind, - ["edit"] = WritePermissionKind, - ["create"] = WritePermissionKind, - ["powershell"] = ShellPermissionKind, - ["read_powershell"] = ShellPermissionKind, - ["write_powershell"] = ShellPermissionKind, - ["stop_powershell"] = ShellPermissionKind, - ["list_powershell"] = ShellPermissionKind, - ["web_fetch"] = UrlPermissionKind, - ["web_search"] = UrlPermissionKind, - ["store_memory"] = MemoryPermissionKind, + ["view"] = new(ReadPermissionKind, ReadPermissionKind), + ["show_file"] = new(ReadPermissionKind, ReadPermissionKind), + ["read_file"] = new(ReadPermissionKind, ReadPermissionKind), + ["glob"] = new(ReadPermissionKind, ReadPermissionKind), + ["grep"] = new(ReadPermissionKind, ReadPermissionKind), + ["rg"] = new(ReadPermissionKind, ReadPermissionKind), + ["lsp"] = new(ReadPermissionKind, ReadPermissionKind), + ["edit"] = new(WritePermissionKind, WritePermissionKind), + ["create"] = new(WritePermissionKind, WritePermissionKind), + ["write_file"] = new(WritePermissionKind, WritePermissionKind), + ["apply_patch"] = new(WritePermissionKind, WritePermissionKind), + ["bash"] = new(ShellPermissionKind, ShellPermissionKind), + ["read_bash"] = new(ShellPermissionKind, ShellPermissionKind), + ["write_bash"] = new(ShellPermissionKind, ShellPermissionKind), + ["stop_bash"] = new(ShellPermissionKind, ShellPermissionKind), + ["list_bash"] = new(ShellPermissionKind, ShellPermissionKind), + ["powershell"] = new(ShellPermissionKind, ShellPermissionKind), + ["read_powershell"] = new(ShellPermissionKind, ShellPermissionKind), + ["write_powershell"] = new(ShellPermissionKind, ShellPermissionKind), + ["stop_powershell"] = new(ShellPermissionKind, ShellPermissionKind), + ["list_powershell"] = new(ShellPermissionKind, ShellPermissionKind), + ["web_fetch"] = new(UrlPermissionKind, WebFetchToolName), + ["web_search"] = new(UrlPermissionKind, WebFetchToolName), + ["store_memory"] = new(MemoryPermissionKind, StoreMemoryToolName), + ["remember_fact"] = new(MemoryPermissionKind, StoreMemoryToolName), }; private readonly ConcurrentDictionary _pendingApprovals = new(StringComparer.Ordinal); @@ -96,19 +107,22 @@ internal sealed class CopilotApprovalCoordinator Func onApproval, CancellationToken cancellationToken) { - string? toolName = ResolveApprovalToolName(request, toolCalls); - string? autoApprovedToolName = ResolveAutoApprovedToolName(request); - string? mcpServerApprovalKey = ResolveMcpServerApprovalKey(request, command.Tooling?.McpServers); - string? approvalCacheKey = ResolveApprovalCacheKey(toolName, autoApprovedToolName); + ResolvedApprovalContext approval = ResolveApprovalContext(request, toolCalls, command.Tooling?.McpServers); + string? approvalCacheKey = ResolveApprovalCacheKey(approval.ToolName, approval.ApprovalToolKey); - AgentActivityEventDto? fileChangeActivity = BuildToolCallFileChangeActivity(command, agent, request, toolName); + AgentActivityEventDto? fileChangeActivity = BuildToolCallFileChangeActivity(command, agent, request, approval.ToolName); if (fileChangeActivity is not null && onActivity is not null) { await onActivity(fileChangeActivity).ConfigureAwait(false); } if (IsToolApprovedForRequest(command.RequestId, approvalCacheKey) - || !RequiresToolCallApproval(command.Workflow.Settings.ApprovalPolicy, agent.GetAgentId(), toolName, autoApprovedToolName, mcpServerApprovalKey)) + || !RequiresToolCallApproval( + command.Workflow.Settings.ApprovalPolicy, + agent.GetAgentId(), + approval.ToolName, + approval.ApprovalToolKey, + approval.McpServerApprovalKey)) { return CreateApprovalResult(PermissionRequestResultKind.Approved); } @@ -127,7 +141,7 @@ internal sealed class CopilotApprovalCoordinator request, invocation, pending.ApprovalId, - toolName)) + approval.ToolName)) .ConfigureAwait(false); using CancellationTokenRegistration registration = cancellationToken.Register( @@ -155,21 +169,22 @@ internal sealed class CopilotApprovalCoordinator string approvalId, string? toolName) { - string permissionKind = ResolvePermissionKind(request, command.Tooling?.McpServers); + ResolvedApprovalContext approval = ResolveApprovalContext(request, toolName, command.Tooling?.McpServers); string agentId = agent.GetAgentId(); string agentName = agent.GetAgentName(); string? sessionId = NormalizeOptionalString(invocation.SessionId); - string? normalizedToolName = NormalizeOptionalString(toolName); + string? normalizedToolName = approval.ToolName; + string? displayToolName = normalizedToolName ?? NormalizeOptionalString(approval.ApprovalToolKey); string? requestedUrl = request is PermissionRequestUrl urlRequest ? NormalizeOptionalString(urlRequest.Url) : null; - string title = normalizedToolName is null - ? $"Approve {permissionKind}" - : $"Approve {normalizedToolName}"; - string detail = normalizedToolName is null - ? $"{agentName} requested {permissionKind} permission" - : $"{agentName} requested {permissionKind} permission for tool \"{normalizedToolName}\""; + string title = displayToolName is null + ? $"Approve {approval.PermissionKind}" + : $"Approve {displayToolName}"; + string detail = displayToolName is null + ? $"{agentName} requested {approval.PermissionKind} permission" + : $"{agentName} requested {approval.PermissionKind} permission for tool \"{displayToolName}\""; if (requestedUrl is not null) { @@ -195,7 +210,8 @@ internal sealed class CopilotApprovalCoordinator AgentId = NormalizeOptionalString(agentId), AgentName = NormalizeOptionalString(agentName), ToolName = normalizedToolName, - PermissionKind = permissionKind, + PermissionKind = approval.PermissionKind, + ApprovalToolKey = NormalizeOptionalString(approval.ApprovalToolKey), Title = title, Detail = detail, PermissionDetail = BuildPermissionDetail(request, command.Tooling?.McpServers), @@ -314,7 +330,7 @@ internal sealed class CopilotApprovalCoordinator ApprovalPolicyDto? approvalPolicy, string agentId, string? toolName, - string? autoApprovedToolName = null, + string? approvalToolKey = null, string? mcpServerApprovalKey = null) { if (approvalPolicy?.Rules is null || approvalPolicy.Rules.Count == 0) @@ -333,7 +349,7 @@ internal sealed class CopilotApprovalCoordinator return true; } - return !MatchesAutoApprovedTool(autoApprovedToolNames, toolName, autoApprovedToolName) + return !MatchesAutoApprovedTool(autoApprovedToolNames, toolName, approvalToolKey) && !MatchesAutoApprovedToolName(autoApprovedToolNames, mcpServerApprovalKey); } @@ -411,9 +427,67 @@ internal sealed class CopilotApprovalCoordinator ?? GetFallbackToolName(request); } - private static string? ResolveAutoApprovedToolName(PermissionRequest request) + private static ResolvedApprovalContext ResolveApprovalContext( + PermissionRequest request, + ToolCallRegistry? toolCalls, + IReadOnlyList? configuredMcpServers) { - return GetFallbackToolName(request); + return ResolveApprovalContext( + request, + ResolveApprovalToolName(request, toolCalls), + configuredMcpServers); + } + + private static ResolvedApprovalContext ResolveApprovalContext( + PermissionRequest request, + string? toolName, + IReadOnlyList? configuredMcpServers) + { + string? normalizedToolName = NormalizeOptionalString(toolName); + return request switch + { + PermissionRequestShell => new( + normalizedToolName, + ShellPermissionKind, + ShellPermissionKind, + null), + PermissionRequestWrite => new( + normalizedToolName, + WritePermissionKind, + WritePermissionKind, + null), + PermissionRequestRead => new( + normalizedToolName, + ReadPermissionKind, + ReadPermissionKind, + null), + PermissionRequestUrl => new( + normalizedToolName, + UrlPermissionKind, + WebFetchToolName, + null), + PermissionRequestMemory => new( + normalizedToolName, + MemoryPermissionKind, + StoreMemoryToolName, + null), + PermissionRequestMcp => new( + normalizedToolName, + McpPermissionKind, + normalizedToolName, + ResolveMcpServerApprovalKey(request, configuredMcpServers)), + PermissionRequestCustomTool => new( + normalizedToolName, + CustomToolPermissionKind, + normalizedToolName, + null), + PermissionRequestHook => ResolveHookApprovalContext(normalizedToolName, configuredMcpServers), + _ => new( + normalizedToolName, + NormalizeOptionalString(request.Kind) ?? "tool access", + normalizedToolName, + ResolveMcpServerApprovalKey(request, configuredMcpServers)), + }; } private const string McpServerApprovalPrefix = "mcp_server:"; @@ -522,44 +596,56 @@ internal sealed class CopilotApprovalCoordinator PermissionRequestWrite => WritePermissionKind, PermissionRequestRead => ReadPermissionKind, PermissionRequestMemory => StoreMemoryToolName, - PermissionRequestHook hook => ResolveHookToolCategory(hook.ToolName), + PermissionRequestHook hook => ResolveHookApprovalToolKey(hook.ToolName), _ => null, }; } internal static string? ResolveHookToolCategory(string? toolName) { - string? normalized = NormalizeOptionalString(toolName); - if (normalized is null) - { - return null; - } - - return HookToolCategories.TryGetValue(normalized, out string? category) ? category : null; + return TryResolveHookToolApproval(toolName, out HookToolApprovalMapping? mapping) && mapping is not null + ? mapping.PermissionKind + : null; } - private static string ResolvePermissionKind( - PermissionRequest request, + internal static string? ResolveHookApprovalToolKey(string? toolName) + { + return TryResolveHookToolApproval(toolName, out HookToolApprovalMapping? mapping) && mapping is not null + ? mapping.ApprovalToolKey + : null; + } + + internal static ResolvedApprovalContext ResolveHookApprovalContext( + string? toolName, IReadOnlyList? configuredMcpServers) { - string permissionKind = string.IsNullOrWhiteSpace(request.Kind) - ? "tool access" - : request.Kind.Trim(); - - if (request is not PermissionRequestHook hook) + string? normalizedToolName = NormalizeOptionalString(toolName); + if (normalizedToolName is null) { - return permissionKind; + return new ResolvedApprovalContext(null, HookPermissionKind, null, null); } - string? resolvedCategory = ResolveHookToolCategory(hook.ToolName); - if (resolvedCategory is not null) + if (TryResolveHookToolApproval(normalizedToolName, out HookToolApprovalMapping? mapping) && mapping is not null) { - return resolvedCategory; + return new ResolvedApprovalContext( + normalizedToolName, + mapping.PermissionKind, + mapping.ApprovalToolKey, + null); } - return ResolveHookMcpServerName(hook.ToolName, configuredMcpServers) is not null - ? McpPermissionKind - : permissionKind; + string? mcpServerApprovalKey = ResolveHookMcpServerApprovalKey(normalizedToolName, configuredMcpServers); + return mcpServerApprovalKey is not null + ? new ResolvedApprovalContext( + normalizedToolName, + McpPermissionKind, + normalizedToolName, + mcpServerApprovalKey) + : new ResolvedApprovalContext( + normalizedToolName, + HookPermissionKind, + normalizedToolName, + null); } private static PermissionDetailDto BuildHookPermissionDetail( @@ -619,13 +705,29 @@ internal sealed class CopilotApprovalCoordinator : strippedToolName; } + private static bool TryResolveHookToolApproval( + string? toolName, + out HookToolApprovalMapping? mapping) + { + string? normalizedToolName = NormalizeOptionalString(toolName); + if (normalizedToolName is not null + && HookToolApprovals.TryGetValue(normalizedToolName, out HookToolApprovalMapping? resolvedMapping)) + { + mapping = resolvedMapping; + return true; + } + + mapping = null; + return false; + } + private static bool MatchesAutoApprovedTool( IReadOnlyList autoApprovedToolNames, string? toolName, - string? autoApprovedToolName) + string? approvalToolKey) { return MatchesAutoApprovedToolName(autoApprovedToolNames, toolName) - || MatchesAutoApprovedToolName(autoApprovedToolNames, autoApprovedToolName); + || MatchesAutoApprovedToolName(autoApprovedToolNames, approvalToolKey); } private static bool MatchesAutoApprovedToolName( @@ -720,6 +822,16 @@ internal sealed class CopilotApprovalCoordinator return normalized.Count > 0 ? normalized : null; } + internal sealed record ResolvedApprovalContext( + string? ToolName, + string PermissionKind, + string? ApprovalToolKey, + string? McpServerApprovalKey); + + private sealed record HookToolApprovalMapping( + string PermissionKind, + string ApprovalToolKey); + private sealed record PendingApprovalRequest( string RequestId, string SessionId, diff --git a/sidecar/src/Aryx.AgentHost/Services/Providers/Copilot/CopilotSessionHooks.cs b/sidecar/src/Aryx.AgentHost/Services/Providers/Copilot/CopilotSessionHooks.cs index 6886186..266ce3f 100644 --- a/sidecar/src/Aryx.AgentHost/Services/Providers/Copilot/CopilotSessionHooks.cs +++ b/sidecar/src/Aryx.AgentHost/Services/Providers/Copilot/CopilotSessionHooks.cs @@ -248,17 +248,16 @@ internal static class CopilotSessionHooks }; } - string? autoApprovedToolName = CopilotApprovalCoordinator.ResolveHookToolCategory(toolName) ?? toolName; - string? mcpServerApprovalKey = CopilotApprovalCoordinator.ResolveHookMcpServerApprovalKey( + CopilotApprovalCoordinator.ResolvedApprovalContext approval = CopilotApprovalCoordinator.ResolveHookApprovalContext( toolName, command.Tooling?.McpServers); bool requiresApproval = CopilotApprovalCoordinator.RequiresToolCallApproval( command.Workflow.Settings.ApprovalPolicy, agentDefinition.GetAgentId(), - toolName, - autoApprovedToolName, - mcpServerApprovalKey); + approval.ToolName, + approval.ApprovalToolKey, + approval.McpServerApprovalKey); return new PreToolUseHookOutput { diff --git a/sidecar/tests/Aryx.AgentHost.Tests/CopilotSessionHooksTests.cs b/sidecar/tests/Aryx.AgentHost.Tests/CopilotSessionHooksTests.cs index 161e27a..a5ee2ec 100644 --- a/sidecar/tests/Aryx.AgentHost.Tests/CopilotSessionHooksTests.cs +++ b/sidecar/tests/Aryx.AgentHost.Tests/CopilotSessionHooksTests.cs @@ -154,11 +154,15 @@ public sealed class CopilotSessionHooksTests [Theory] [InlineData("view", "read")] [InlineData("grep", "read")] + [InlineData("rg", "read")] [InlineData("edit", "write")] + [InlineData("apply_patch", "write")] [InlineData("powershell", "shell")] - public async Task Create_PreToolUseAutoAllowsWhenCategoryIsApproved(string toolName, string category) + [InlineData("web_search", "web_fetch")] + [InlineData("store_memory", "store_memory")] + public async Task Create_PreToolUseAutoAllowsWhenApprovalToolKeyIsApproved(string toolName, string approvalToolKey) { - RunTurnCommandDto command = CreateCommandWithAutoApprovedCategory(category); + RunTurnCommandDto command = CreateCommandWithAutoApprovedCategory(approvalToolKey); SessionHooks hooks = CopilotSessionHooks.Create(command, command.Workflow.GetAgentNodes()[0], ResolvedHookSet.Empty, new RecordingHookCommandRunner()); PreToolUseHookOutput? decision = await hooks.OnPreToolUse!( diff --git a/sidecar/tests/Aryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs b/sidecar/tests/Aryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs index 4cb3405..d39aae8 100644 --- a/sidecar/tests/Aryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs +++ b/sidecar/tests/Aryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs @@ -1487,6 +1487,28 @@ public sealed class CopilotWorkflowRunnerTests Assert.True(CopilotApprovalCoordinator.RequiresToolCallApproval(policy, "agent-1", "git.status")); } + [Fact] + public void RequiresToolCallApproval_HonorsCanonicalHookApprovalKeysForModernToolNames() + { + ApprovalPolicyDto policy = new() + { + Rules = + [ + new ApprovalCheckpointRuleDto + { + Kind = "tool-call", + AgentIds = ["agent-1"], + }, + ], + AutoApprovedToolNames = ["read", "write", "web_fetch"], + }; + + Assert.False(CopilotApprovalCoordinator.RequiresToolCallApproval(policy, "agent-1", "rg", "read")); + Assert.False(CopilotApprovalCoordinator.RequiresToolCallApproval(policy, "agent-1", "apply_patch", "write")); + Assert.False(CopilotApprovalCoordinator.RequiresToolCallApproval(policy, "agent-1", "web_search", "web_fetch")); + Assert.True(CopilotApprovalCoordinator.RequiresToolCallApproval(policy, "agent-1", "powershell", "shell")); + } + [Fact] public void RequiresToolCallApproval_HonorsMcpServerLevelApprovalKey() { @@ -1738,6 +1760,7 @@ public sealed class CopilotWorkflowRunnerTests "lsp_ts_hover"); Assert.Equal("lsp_ts_hover", approvalEvent.ToolName); + Assert.Equal("lsp_ts_hover", approvalEvent.ApprovalToolKey); Assert.Equal("Approve lsp_ts_hover", approvalEvent.Title); Assert.Contains("tool \"lsp_ts_hover\"", approvalEvent.Detail); Assert.NotNull(approvalEvent.PermissionDetail); @@ -1770,6 +1793,7 @@ public sealed class CopilotWorkflowRunnerTests "web_fetch"); Assert.Equal("web_fetch", approvalEvent.ToolName); + Assert.Equal("web_fetch", approvalEvent.ApprovalToolKey); Assert.Equal("Approve web_fetch", approvalEvent.Title); Assert.Contains("url permission", approvalEvent.Detail); Assert.Contains("tool \"web_fetch\"", approvalEvent.Detail); @@ -1992,11 +2016,21 @@ public sealed class CopilotWorkflowRunnerTests [Theory] [InlineData("view", "read")] + [InlineData("show_file", "read")] + [InlineData("read_file", "read")] [InlineData("glob", "read")] [InlineData("grep", "read")] + [InlineData("rg", "read")] [InlineData("lsp", "read")] [InlineData("edit", "write")] [InlineData("create", "write")] + [InlineData("write_file", "write")] + [InlineData("apply_patch", "write")] + [InlineData("bash", "shell")] + [InlineData("read_bash", "shell")] + [InlineData("write_bash", "shell")] + [InlineData("stop_bash", "shell")] + [InlineData("list_bash", "shell")] [InlineData("powershell", "shell")] [InlineData("read_powershell", "shell")] [InlineData("write_powershell", "shell")] @@ -2005,6 +2039,7 @@ public sealed class CopilotWorkflowRunnerTests [InlineData("web_fetch", "url")] [InlineData("web_search", "url")] [InlineData("store_memory", "memory")] + [InlineData("remember_fact", "memory")] public void ResolveHookToolCategory_ReturnsExpectedCategoryForKnownTools(string toolName, string expectedCategory) { Assert.Equal(expectedCategory, CopilotApprovalCoordinator.ResolveHookToolCategory(toolName)); @@ -2027,6 +2062,29 @@ public sealed class CopilotWorkflowRunnerTests Assert.Null(CopilotApprovalCoordinator.ResolveHookToolCategory(" ")); } + [Theory] + [InlineData("view", "read")] + [InlineData("rg", "read")] + [InlineData("apply_patch", "write")] + [InlineData("bash", "shell")] + [InlineData("web_fetch", "web_fetch")] + [InlineData("web_search", "web_fetch")] + [InlineData("store_memory", "store_memory")] + [InlineData("remember_fact", "store_memory")] + public void ResolveHookApprovalToolKey_ReturnsExpectedKeyForKnownTools(string toolName, string expectedKey) + { + Assert.Equal(expectedKey, CopilotApprovalCoordinator.ResolveHookApprovalToolKey(toolName)); + } + + [Fact] + public void ResolveHookApprovalToolKey_ReturnsNullForUnknownTools() + { + Assert.Null(CopilotApprovalCoordinator.ResolveHookApprovalToolKey("custom_tool")); + Assert.Null(CopilotApprovalCoordinator.ResolveHookApprovalToolKey(null)); + Assert.Null(CopilotApprovalCoordinator.ResolveHookApprovalToolKey("")); + Assert.Null(CopilotApprovalCoordinator.ResolveHookApprovalToolKey(" ")); + } + [Fact] public void ResolveHookMcpServerApprovalKey_PrefersLongestConfiguredServerName() { @@ -2119,6 +2177,7 @@ public sealed class CopilotWorkflowRunnerTests "view"); Assert.Equal("view", approvalEvent.ToolName); + Assert.Equal("read", approvalEvent.ApprovalToolKey); Assert.Equal("read", approvalEvent.PermissionKind); Assert.Contains("read permission", approvalEvent.Detail); } @@ -2151,6 +2210,7 @@ public sealed class CopilotWorkflowRunnerTests "icm-mcp-get_schedule"); Assert.Equal("mcp", approvalEvent.PermissionKind); + Assert.Equal("icm-mcp-get_schedule", approvalEvent.ApprovalToolKey); Assert.Contains("mcp permission", approvalEvent.Detail); Assert.NotNull(approvalEvent.PermissionDetail); Assert.Equal("mcp", approvalEvent.PermissionDetail!.Kind); @@ -2182,6 +2242,7 @@ public sealed class CopilotWorkflowRunnerTests "icm-mcp-get_schedule"); Assert.Equal("hook", approvalEvent.PermissionKind); + Assert.Equal("icm-mcp-get_schedule", approvalEvent.ApprovalToolKey); } [Fact] @@ -2214,6 +2275,7 @@ public sealed class CopilotWorkflowRunnerTests Assert.False(pending.IsCompleted); Assert.NotNull(observedApproval); + Assert.Equal("lsp_ts_definition", observedApproval!.ApprovalToolKey); await coordinator.ResolveApprovalAsync( new ResolveApprovalCommandDto @@ -2270,6 +2332,8 @@ public sealed class CopilotWorkflowRunnerTests Assert.Equal("tool-calling", observedActivity!.ActivityType); Assert.Equal("apply_patch", observedActivity.ToolName); Assert.Equal("tool-call-write-1", observedActivity.ToolCallId); + Assert.Equal("apply_patch", observedApproval!.ToolName); + Assert.Equal("write", observedApproval.ApprovalToolKey); ToolCallFileChangeDto preview = Assert.Single(observedActivity.FileChanges!); Assert.Equal("README.md", preview.Path); @@ -2388,6 +2452,7 @@ public sealed class CopilotWorkflowRunnerTests Assert.False(firstPending.IsCompleted); Assert.NotNull(firstApproval); + Assert.Equal("read", firstApproval!.ApprovalToolKey); await coordinator.ResolveApprovalAsync( new ResolveApprovalCommandDto @@ -2428,6 +2493,82 @@ public sealed class CopilotWorkflowRunnerTests Assert.Equal(PermissionRequestResultKind.Approved, secondResult.Kind); } + [Fact] + public async Task RequestApprovalAsync_AlwaysApproveCachesCanonicalWriteApprovalForModernHookTools() + { + CopilotApprovalCoordinator coordinator = new(); + ApprovalRequestedEventDto? firstApproval = null; + RunTurnCommandDto command = CreateApprovalCommand(); + + Task firstPending = coordinator.RequestApprovalAsync( + command, + command.Workflow.GetAgentNodes()[0], + new PermissionRequestWrite + { + Kind = "write", + ToolCallId = "tool-call-write-1", + Intention = "Update the README", + FileName = "README.md", + Diff = "@@ -1 +1 @@", + NewFileContents = "# Aryx\n", + }, + new PermissionInvocation + { + SessionId = "copilot-session-1", + }, + CreateToolCallRegistry(("tool-call-write-1", "apply_patch")), + approval => + { + firstApproval = approval; + return Task.CompletedTask; + }, + CancellationToken.None); + + Assert.False(firstPending.IsCompleted); + Assert.NotNull(firstApproval); + Assert.Equal("write", firstApproval!.ApprovalToolKey); + + await coordinator.ResolveApprovalAsync( + new ResolveApprovalCommandDto + { + ApprovalId = firstApproval.ApprovalId, + Decision = "approved", + AlwaysApprove = true, + }, + CancellationToken.None); + + PermissionRequestResult firstResult = await firstPending; + Assert.Equal(PermissionRequestResultKind.Approved, firstResult.Kind); + + bool sawSecondApproval = false; + PermissionRequestResult secondResult = await coordinator.RequestApprovalAsync( + command, + command.Workflow.GetAgentNodes()[0], + new PermissionRequestWrite + { + Kind = "write", + ToolCallId = "tool-call-write-2", + Intention = "Create docs draft", + FileName = "docs\\guide.md", + Diff = "@@ -0,0 +1 @@", + NewFileContents = "# Guide\n", + }, + new PermissionInvocation + { + SessionId = "copilot-session-1", + }, + CreateToolCallRegistry(("tool-call-write-2", "write_file")), + approval => + { + sawSecondApproval = true; + return Task.CompletedTask; + }, + CancellationToken.None); + + Assert.False(sawSecondApproval); + Assert.Equal(PermissionRequestResultKind.Approved, secondResult.Kind); + } + [Fact] public async Task RequestApprovalAsync_AlwaysApproveCacheDoesNotCarryAcrossTurnRequests() { diff --git a/sidecar/tests/Aryx.AgentHost.Tests/SidecarProtocolHostTests.cs b/sidecar/tests/Aryx.AgentHost.Tests/SidecarProtocolHostTests.cs index 7b85013..17aa546 100644 --- a/sidecar/tests/Aryx.AgentHost.Tests/SidecarProtocolHostTests.cs +++ b/sidecar/tests/Aryx.AgentHost.Tests/SidecarProtocolHostTests.cs @@ -412,6 +412,7 @@ public sealed class SidecarProtocolHostTests AgentId = "agent-1", AgentName = "Primary", PermissionKind = "tool access", + ApprovalToolKey = "shell", Title = "Approve tool access", PermissionDetail = new PermissionDetailDto { @@ -437,6 +438,7 @@ public sealed class SidecarProtocolHostTests Assert.Equal("turn-approval", approvalEvent.GetProperty("requestId").GetString()); Assert.Equal("approval-1", approvalEvent.GetProperty("approvalId").GetString()); Assert.Equal("tool-call", approvalEvent.GetProperty("approvalKind").GetString()); + Assert.Equal("shell", approvalEvent.GetProperty("approvalToolKey").GetString()); Assert.Equal("Approve tool access", approvalEvent.GetProperty("title").GetString()); JsonElement permissionDetail = approvalEvent.GetProperty("permissionDetail"); Assert.Equal("shell", permissionDetail.GetProperty("kind").GetString());