mirror of
https://github.com/davidkaya/aryx.git
synced 2026-08-04 02:48:44 +02:00
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>
This commit is contained in:
@@ -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<string> 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<string> autoApprovedToolNames,
|
||||
string? toolName,
|
||||
string? autoApprovedToolName)
|
||||
{
|
||||
return MatchesAutoApprovedToolName(autoApprovedToolNames, toolName)
|
||||
|| MatchesAutoApprovedToolName(autoApprovedToolNames, autoApprovedToolName);
|
||||
}
|
||||
|
||||
private static bool MatchesAutoApprovedToolName(
|
||||
IReadOnlyList<string> 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))
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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()
|
||||
{
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -729,10 +729,11 @@ export class AryxAppService extends EventEmitter<AppServiceEvents> {
|
||||
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] };
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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 (
|
||||
<div className="rounded-xl border border-amber-500/30 bg-amber-500/5 px-4 py-3" role="alert">
|
||||
@@ -90,11 +91,11 @@ export function ApprovalBanner({
|
||||
</button>
|
||||
{canAlwaysApprove && (
|
||||
<button
|
||||
aria-label={`Always approve ${approval.toolName}`}
|
||||
aria-label={`Always approve ${approvalToolKey}`}
|
||||
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 "${approval.toolName}" for the rest of this session`}
|
||||
title={`Auto-approve "${approvalToolKey}" for the rest of this session`}
|
||||
type="button"
|
||||
>
|
||||
<ShieldBan className="size-3" />
|
||||
|
||||
@@ -92,6 +92,10 @@ const lspApprovalOperations = [
|
||||
// Human-readable labels for built-in runtime tools.
|
||||
// Both bash and powershell variants are included for forward compatibility.
|
||||
const builtinToolLabels: ReadonlyMap<string, string> = new Map([
|
||||
// Permission-kind approval categories (used by sidecar for runtime tool session approval)
|
||||
['shell', 'Shell commands'],
|
||||
['read', 'Read files'],
|
||||
['write', 'Write files'],
|
||||
// Shell tools
|
||||
['bash', 'Execute shell commands'],
|
||||
['powershell', 'Execute shell commands'],
|
||||
@@ -136,6 +140,10 @@ export function resolveToolLabel(toolId: string): string {
|
||||
// Fallback runtime tools used before sidecar capabilities are loaded or when the
|
||||
// CLI cannot report its built-in tool catalog dynamically.
|
||||
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' },
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
normalizePendingApproval,
|
||||
normalizePendingApprovalState,
|
||||
normalizeSessionApprovalSettings,
|
||||
pruneSessionApprovalSettings,
|
||||
dequeuePendingApprovalState,
|
||||
enqueuePendingApprovalState,
|
||||
listPendingApprovals,
|
||||
@@ -209,4 +210,34 @@ describe('approval helpers', () => {
|
||||
pendingApprovalQueue: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
test('prune preserves permission-kind approval entries when they are known tools', () => {
|
||||
const settings = normalizeSessionApprovalSettings({
|
||||
autoApprovedToolNames: ['read', 'write', 'shell', 'git.status', 'unknown_tool'],
|
||||
});
|
||||
const knownToolNames = ['read', 'write', 'shell', 'git.status', 'bash', 'view'];
|
||||
|
||||
const pruned = pruneSessionApprovalSettings(settings, knownToolNames);
|
||||
expect(pruned?.autoApprovedToolNames).toEqual(['read', 'write', 'shell', 'git.status']);
|
||||
});
|
||||
|
||||
test('session approval with permission-kind entries overrides pattern defaults', () => {
|
||||
const effective = resolveEffectiveApprovalPolicy(
|
||||
{
|
||||
rules: [{ kind: 'tool-call' }],
|
||||
autoApprovedToolNames: ['git.status'],
|
||||
},
|
||||
normalizeSessionApprovalSettings({
|
||||
autoApprovedToolNames: ['read', 'shell'],
|
||||
}),
|
||||
);
|
||||
|
||||
expect(effective).toEqual({
|
||||
rules: [{ kind: 'tool-call' }],
|
||||
autoApprovedToolNames: ['read', 'shell'],
|
||||
});
|
||||
expect(approvalPolicyRequiresToolCallApproval(effective, 'agent-1', 'read')).toBe(false);
|
||||
expect(approvalPolicyRequiresToolCallApproval(effective, 'agent-1', 'shell')).toBe(false);
|
||||
expect(approvalPolicyRequiresToolCallApproval(effective, 'agent-1', 'write')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -258,6 +258,9 @@ describe('tooling settings helpers', () => {
|
||||
expect(resolveToolLabel('fetch_copilot_cli_documentation')).toBe('Fetch CLI docs');
|
||||
expect(resolveToolLabel('glob')).toBe('Find files by pattern');
|
||||
expect(resolveToolLabel('lsp')).toBe('Language server');
|
||||
expect(resolveToolLabel('shell')).toBe('Shell commands');
|
||||
expect(resolveToolLabel('read')).toBe('Read files');
|
||||
expect(resolveToolLabel('write')).toBe('Write files');
|
||||
});
|
||||
|
||||
test('passes through unknown tool IDs as labels', () => {
|
||||
@@ -269,15 +272,26 @@ describe('tooling settings helpers', () => {
|
||||
const tools = listApprovalToolDefinitions({ mcpServers: [], lspProfiles: [] });
|
||||
const builtinTools = tools.filter((t) => t.kind === 'builtin');
|
||||
|
||||
expect(builtinTools.length).toBeGreaterThanOrEqual(20);
|
||||
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);
|
||||
|
||||
// Permission-kind approval categories should be included
|
||||
expect(builtinTools.some((t) => t.id === 'shell')).toBe(true);
|
||||
expect(builtinTools.some((t) => t.id === 'read')).toBe(true);
|
||||
expect(builtinTools.some((t) => t.id === 'write')).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');
|
||||
|
||||
// Internal tools should not appear in the fallback
|
||||
expect(builtinTools.some((t) => t.id === 'ask_user')).toBe(false);
|
||||
|
||||
Reference in New Issue
Block a user