mirror of
https://github.com/davidkaya/aryx.git
synced 2026-08-08 20:58:45 +02:00
feat: cache same-turn approval selections
Add an AlwaysApprove flag to resolve-approval commands and cache approved tool keys for the current request so repeated runtime permissions are auto-approved immediately within the same turn. The cache is cleared when the turn finishes to avoid leaking across future requests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -176,6 +176,7 @@ public sealed class ResolveApprovalCommandDto : SidecarCommandEnvelope
|
|||||||
{
|
{
|
||||||
public string ApprovalId { get; init; } = string.Empty;
|
public string ApprovalId { get; init; } = string.Empty;
|
||||||
public string Decision { get; init; } = string.Empty;
|
public string Decision { get; init; } = string.Empty;
|
||||||
|
public bool AlwaysApprove { get; init; }
|
||||||
}
|
}
|
||||||
|
|
||||||
public sealed class ResolveUserInputCommandDto : SidecarCommandEnvelope
|
public sealed class ResolveUserInputCommandDto : SidecarCommandEnvelope
|
||||||
|
|||||||
@@ -21,6 +21,7 @@ internal sealed class CopilotApprovalCoordinator
|
|||||||
private const string HookPermissionKind = "hook";
|
private const string HookPermissionKind = "hook";
|
||||||
|
|
||||||
private readonly ConcurrentDictionary<string, PendingApprovalRequest> _pendingApprovals = new(StringComparer.Ordinal);
|
private readonly ConcurrentDictionary<string, PendingApprovalRequest> _pendingApprovals = new(StringComparer.Ordinal);
|
||||||
|
private readonly ConcurrentDictionary<string, ConcurrentDictionary<string, byte>> _requestApprovedTools = new(StringComparer.Ordinal);
|
||||||
|
|
||||||
public Task ResolveApprovalAsync(
|
public Task ResolveApprovalAsync(
|
||||||
ResolveApprovalCommandDto command,
|
ResolveApprovalCommandDto command,
|
||||||
@@ -37,6 +38,11 @@ internal sealed class CopilotApprovalCoordinator
|
|||||||
throw new InvalidOperationException($"Approval \"{approvalId}\" is no longer pending.");
|
throw new InvalidOperationException($"Approval \"{approvalId}\" is no longer pending.");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (decision == PermissionRequestResultKind.Approved && command.AlwaysApprove)
|
||||||
|
{
|
||||||
|
CacheApprovedToolForRequest(pending.RequestId, pending.ApprovalCacheKey);
|
||||||
|
}
|
||||||
|
|
||||||
return Task.CompletedTask;
|
return Task.CompletedTask;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -51,12 +57,14 @@ internal sealed class CopilotApprovalCoordinator
|
|||||||
{
|
{
|
||||||
string? toolName = ResolveApprovalToolName(request, toolNamesByCallId);
|
string? toolName = ResolveApprovalToolName(request, toolNamesByCallId);
|
||||||
string? autoApprovedToolName = ResolveAutoApprovedToolName(request);
|
string? autoApprovedToolName = ResolveAutoApprovedToolName(request);
|
||||||
if (!RequiresToolCallApproval(command.Pattern.ApprovalPolicy, agent.Id, toolName, autoApprovedToolName))
|
string? approvalCacheKey = ResolveApprovalCacheKey(toolName, autoApprovedToolName);
|
||||||
|
if (IsToolApprovedForRequest(command.RequestId, approvalCacheKey)
|
||||||
|
|| !RequiresToolCallApproval(command.Pattern.ApprovalPolicy, agent.Id, toolName, autoApprovedToolName))
|
||||||
{
|
{
|
||||||
return CreateApprovalResult(PermissionRequestResultKind.Approved);
|
return CreateApprovalResult(PermissionRequestResultKind.Approved);
|
||||||
}
|
}
|
||||||
|
|
||||||
PendingApprovalRequest pending = CreatePendingApproval(command);
|
PendingApprovalRequest pending = CreatePendingApproval(command, approvalCacheKey);
|
||||||
if (!_pendingApprovals.TryAdd(pending.ApprovalId, pending))
|
if (!_pendingApprovals.TryAdd(pending.ApprovalId, pending))
|
||||||
{
|
{
|
||||||
throw new InvalidOperationException($"Approval \"{pending.ApprovalId}\" is already pending.");
|
throw new InvalidOperationException($"Approval \"{pending.ApprovalId}\" is already pending.");
|
||||||
@@ -252,6 +260,17 @@ internal sealed class CopilotApprovalCoordinator
|
|||||||
internal static bool TryGetApprovalToolName(PermissionRequest request, out string? toolName)
|
internal static bool TryGetApprovalToolName(PermissionRequest request, out string? toolName)
|
||||||
=> TryGetApprovalToolName(request, toolNamesByCallId: null, out toolName);
|
=> TryGetApprovalToolName(request, toolNamesByCallId: null, out toolName);
|
||||||
|
|
||||||
|
internal void ClearRequestApprovals(string requestId)
|
||||||
|
{
|
||||||
|
string? normalizedRequestId = NormalizeOptionalString(requestId);
|
||||||
|
if (normalizedRequestId is null)
|
||||||
|
{
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
_requestApprovedTools.TryRemove(normalizedRequestId, out _);
|
||||||
|
}
|
||||||
|
|
||||||
private static bool HasMatchingToolCallCheckpoint(
|
private static bool HasMatchingToolCallCheckpoint(
|
||||||
IReadOnlyList<ApprovalCheckpointRuleDto> rules,
|
IReadOnlyList<ApprovalCheckpointRuleDto> rules,
|
||||||
string agentId)
|
string agentId)
|
||||||
@@ -274,12 +293,15 @@ internal sealed class CopilotApprovalCoordinator
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
private static PendingApprovalRequest CreatePendingApproval(RunTurnCommandDto command)
|
private static PendingApprovalRequest CreatePendingApproval(
|
||||||
|
RunTurnCommandDto command,
|
||||||
|
string? approvalCacheKey)
|
||||||
{
|
{
|
||||||
return new PendingApprovalRequest(
|
return new PendingApprovalRequest(
|
||||||
command.RequestId,
|
command.RequestId,
|
||||||
command.SessionId,
|
command.SessionId,
|
||||||
CreateApprovalRequestId(),
|
CreateApprovalRequestId(),
|
||||||
|
NormalizeOptionalString(approvalCacheKey),
|
||||||
new TaskCompletionSource<PermissionRequestResultKind>(TaskCreationOptions.RunContinuationsAsynchronously));
|
new TaskCompletionSource<PermissionRequestResultKind>(TaskCreationOptions.RunContinuationsAsynchronously));
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -305,6 +327,14 @@ internal sealed class CopilotApprovalCoordinator
|
|||||||
return GetFallbackToolName(request);
|
return GetFallbackToolName(request);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static string? ResolveApprovalCacheKey(
|
||||||
|
string? toolName,
|
||||||
|
string? autoApprovedToolName)
|
||||||
|
{
|
||||||
|
return NormalizeOptionalString(autoApprovedToolName)
|
||||||
|
?? NormalizeOptionalString(toolName);
|
||||||
|
}
|
||||||
|
|
||||||
private static string? GetDirectToolName(PermissionRequest request)
|
private static string? GetDirectToolName(PermissionRequest request)
|
||||||
{
|
{
|
||||||
return request switch
|
return request switch
|
||||||
@@ -383,6 +413,34 @@ internal sealed class CopilotApprovalCoordinator
|
|||||||
string.Equals(candidate, normalizedToolName, StringComparison.OrdinalIgnoreCase));
|
string.Equals(candidate, normalizedToolName, StringComparison.OrdinalIgnoreCase));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private bool IsToolApprovedForRequest(string requestId, string? approvalCacheKey)
|
||||||
|
{
|
||||||
|
string? normalizedRequestId = NormalizeOptionalString(requestId);
|
||||||
|
string? normalizedApprovalCacheKey = NormalizeOptionalString(approvalCacheKey);
|
||||||
|
if (normalizedRequestId is null || normalizedApprovalCacheKey is null)
|
||||||
|
{
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
return _requestApprovedTools.TryGetValue(normalizedRequestId, out ConcurrentDictionary<string, byte>? approvedTools)
|
||||||
|
&& approvedTools.ContainsKey(normalizedApprovalCacheKey);
|
||||||
|
}
|
||||||
|
|
||||||
|
private void CacheApprovedToolForRequest(string requestId, string? approvalCacheKey)
|
||||||
|
{
|
||||||
|
string? normalizedRequestId = NormalizeOptionalString(requestId);
|
||||||
|
string? normalizedApprovalCacheKey = NormalizeOptionalString(approvalCacheKey);
|
||||||
|
if (normalizedRequestId is null || normalizedApprovalCacheKey is null)
|
||||||
|
{
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
ConcurrentDictionary<string, byte> approvedTools = _requestApprovedTools.GetOrAdd(
|
||||||
|
normalizedRequestId,
|
||||||
|
static _ => new ConcurrentDictionary<string, byte>(StringComparer.OrdinalIgnoreCase));
|
||||||
|
approvedTools.TryAdd(normalizedApprovalCacheKey, 0);
|
||||||
|
}
|
||||||
|
|
||||||
private PendingApprovalRequest GetPendingApproval(string approvalId)
|
private PendingApprovalRequest GetPendingApproval(string approvalId)
|
||||||
{
|
{
|
||||||
if (_pendingApprovals.TryGetValue(approvalId, out PendingApprovalRequest? pending))
|
if (_pendingApprovals.TryGetValue(approvalId, out PendingApprovalRequest? pending))
|
||||||
@@ -436,5 +494,6 @@ internal sealed class CopilotApprovalCoordinator
|
|||||||
string RequestId,
|
string RequestId,
|
||||||
string SessionId,
|
string SessionId,
|
||||||
string ApprovalId,
|
string ApprovalId,
|
||||||
|
string? ApprovalCacheKey,
|
||||||
TaskCompletionSource<PermissionRequestResultKind> Decision);
|
TaskCompletionSource<PermissionRequestResultKind> Decision);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -111,6 +111,10 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner
|
|||||||
await onExitPlanMode(exitPlanModeEvent).ConfigureAwait(false);
|
await onExitPlanMode(exitPlanModeEvent).ConfigureAwait(false);
|
||||||
return state.FinalizeCompletedMessages();
|
return state.FinalizeCompletedMessages();
|
||||||
}
|
}
|
||||||
|
finally
|
||||||
|
{
|
||||||
|
_approvalCoordinator.ClearRequestApprovals(command.RequestId);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private static async Task EmitPendingActivityEventsAsync(
|
private static async Task EmitPendingActivityEventsAsync(
|
||||||
|
|||||||
@@ -1314,6 +1314,170 @@ public sealed class CopilotWorkflowRunnerTests
|
|||||||
Assert.Equal(PermissionRequestResultKind.Approved, result.Kind);
|
Assert.Equal(PermissionRequestResultKind.Approved, result.Kind);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task RequestApprovalAsync_AlwaysApproveCachesRuntimeApprovalForCurrentTurn()
|
||||||
|
{
|
||||||
|
CopilotApprovalCoordinator coordinator = new();
|
||||||
|
ApprovalRequestedEventDto? firstApproval = null;
|
||||||
|
RunTurnCommandDto command = CreateApprovalCommand();
|
||||||
|
|
||||||
|
Task<PermissionRequestResult> firstPending = coordinator.RequestApprovalAsync(
|
||||||
|
command,
|
||||||
|
command.Pattern.Agents[0],
|
||||||
|
new PermissionRequestRead
|
||||||
|
{
|
||||||
|
Kind = "read",
|
||||||
|
ToolCallId = "tool-call-read-1",
|
||||||
|
Intention = "Inspect README guidance",
|
||||||
|
Path = "README.md",
|
||||||
|
},
|
||||||
|
new PermissionInvocation
|
||||||
|
{
|
||||||
|
SessionId = "copilot-session-1",
|
||||||
|
},
|
||||||
|
new Dictionary<string, string>(StringComparer.Ordinal)
|
||||||
|
{
|
||||||
|
["tool-call-read-1"] = "view",
|
||||||
|
},
|
||||||
|
approval =>
|
||||||
|
{
|
||||||
|
firstApproval = approval;
|
||||||
|
return Task.CompletedTask;
|
||||||
|
},
|
||||||
|
CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.False(firstPending.IsCompleted);
|
||||||
|
Assert.NotNull(firstApproval);
|
||||||
|
|
||||||
|
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.Pattern.Agents[0],
|
||||||
|
new PermissionRequestRead
|
||||||
|
{
|
||||||
|
Kind = "read",
|
||||||
|
ToolCallId = "tool-call-read-2",
|
||||||
|
Intention = "Inspect docs guidance",
|
||||||
|
Path = "docs\\guide.md",
|
||||||
|
},
|
||||||
|
new PermissionInvocation
|
||||||
|
{
|
||||||
|
SessionId = "copilot-session-1",
|
||||||
|
},
|
||||||
|
new Dictionary<string, string>(StringComparer.Ordinal)
|
||||||
|
{
|
||||||
|
["tool-call-read-2"] = "grep",
|
||||||
|
},
|
||||||
|
approval =>
|
||||||
|
{
|
||||||
|
sawSecondApproval = true;
|
||||||
|
return Task.CompletedTask;
|
||||||
|
},
|
||||||
|
CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.False(sawSecondApproval);
|
||||||
|
Assert.Equal(PermissionRequestResultKind.Approved, secondResult.Kind);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task RequestApprovalAsync_AlwaysApproveCacheDoesNotCarryAcrossTurnRequests()
|
||||||
|
{
|
||||||
|
CopilotApprovalCoordinator coordinator = new();
|
||||||
|
ApprovalRequestedEventDto? firstApproval = null;
|
||||||
|
RunTurnCommandDto firstCommand = CreateApprovalCommand();
|
||||||
|
|
||||||
|
Task<PermissionRequestResult> firstPending = coordinator.RequestApprovalAsync(
|
||||||
|
firstCommand,
|
||||||
|
firstCommand.Pattern.Agents[0],
|
||||||
|
new PermissionRequestRead
|
||||||
|
{
|
||||||
|
Kind = "read",
|
||||||
|
ToolCallId = "tool-call-read-1",
|
||||||
|
Intention = "Inspect README guidance",
|
||||||
|
Path = "README.md",
|
||||||
|
},
|
||||||
|
new PermissionInvocation
|
||||||
|
{
|
||||||
|
SessionId = "copilot-session-1",
|
||||||
|
},
|
||||||
|
new Dictionary<string, string>(StringComparer.Ordinal)
|
||||||
|
{
|
||||||
|
["tool-call-read-1"] = "view",
|
||||||
|
},
|
||||||
|
approval =>
|
||||||
|
{
|
||||||
|
firstApproval = approval;
|
||||||
|
return Task.CompletedTask;
|
||||||
|
},
|
||||||
|
CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.NotNull(firstApproval);
|
||||||
|
|
||||||
|
await coordinator.ResolveApprovalAsync(
|
||||||
|
new ResolveApprovalCommandDto
|
||||||
|
{
|
||||||
|
ApprovalId = firstApproval!.ApprovalId,
|
||||||
|
Decision = "approved",
|
||||||
|
AlwaysApprove = true,
|
||||||
|
},
|
||||||
|
CancellationToken.None);
|
||||||
|
|
||||||
|
await firstPending;
|
||||||
|
|
||||||
|
ApprovalRequestedEventDto? secondApproval = null;
|
||||||
|
RunTurnCommandDto secondCommand = CreateApprovalCommand(requestId: "turn-2");
|
||||||
|
Task<PermissionRequestResult> secondPending = coordinator.RequestApprovalAsync(
|
||||||
|
secondCommand,
|
||||||
|
secondCommand.Pattern.Agents[0],
|
||||||
|
new PermissionRequestRead
|
||||||
|
{
|
||||||
|
Kind = "read",
|
||||||
|
ToolCallId = "tool-call-read-2",
|
||||||
|
Intention = "Inspect docs guidance",
|
||||||
|
Path = "docs\\guide.md",
|
||||||
|
},
|
||||||
|
new PermissionInvocation
|
||||||
|
{
|
||||||
|
SessionId = "copilot-session-1",
|
||||||
|
},
|
||||||
|
new Dictionary<string, string>(StringComparer.Ordinal)
|
||||||
|
{
|
||||||
|
["tool-call-read-2"] = "grep",
|
||||||
|
},
|
||||||
|
approval =>
|
||||||
|
{
|
||||||
|
secondApproval = approval;
|
||||||
|
return Task.CompletedTask;
|
||||||
|
},
|
||||||
|
CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.False(secondPending.IsCompleted);
|
||||||
|
Assert.NotNull(secondApproval);
|
||||||
|
|
||||||
|
await coordinator.ResolveApprovalAsync(
|
||||||
|
new ResolveApprovalCommandDto
|
||||||
|
{
|
||||||
|
ApprovalId = secondApproval!.ApprovalId,
|
||||||
|
Decision = "approved",
|
||||||
|
},
|
||||||
|
CancellationToken.None);
|
||||||
|
|
||||||
|
PermissionRequestResult secondResult = await secondPending;
|
||||||
|
Assert.Equal(PermissionRequestResultKind.Approved, secondResult.Kind);
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task ResolveApprovalAsync_RejectsUnknownApprovalIds()
|
public async Task ResolveApprovalAsync_RejectsUnknownApprovalIds()
|
||||||
{
|
{
|
||||||
@@ -1390,11 +1554,11 @@ public sealed class CopilotWorkflowRunnerTests
|
|||||||
null!);
|
null!);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static RunTurnCommandDto CreateApprovalCommand()
|
private static RunTurnCommandDto CreateApprovalCommand(string requestId = "turn-1")
|
||||||
{
|
{
|
||||||
return new RunTurnCommandDto
|
return new RunTurnCommandDto
|
||||||
{
|
{
|
||||||
RequestId = "turn-1",
|
RequestId = requestId,
|
||||||
SessionId = "session-1",
|
SessionId = "session-1",
|
||||||
Pattern = new PatternDefinitionDto
|
Pattern = new PatternDefinitionDto
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -643,6 +643,7 @@ public sealed class SidecarProtocolHostTests
|
|||||||
RequestId = "approval-command-1",
|
RequestId = "approval-command-1",
|
||||||
ApprovalId = "approval-1",
|
ApprovalId = "approval-1",
|
||||||
Decision = "approved",
|
Decision = "approved",
|
||||||
|
AlwaysApprove = true,
|
||||||
},
|
},
|
||||||
host);
|
host);
|
||||||
|
|
||||||
@@ -651,6 +652,7 @@ public sealed class SidecarProtocolHostTests
|
|||||||
Assert.Equal("approval-command-1", completionEvent.GetProperty("requestId").GetString());
|
Assert.Equal("approval-command-1", completionEvent.GetProperty("requestId").GetString());
|
||||||
Assert.Equal("approval-1", captured?.ApprovalId);
|
Assert.Equal("approval-1", captured?.ApprovalId);
|
||||||
Assert.Equal("approved", captured?.Decision);
|
Assert.Equal("approved", captured?.Decision);
|
||||||
|
Assert.True(captured?.AlwaysApprove ?? false);
|
||||||
}
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|||||||
Reference in New Issue
Block a user