diff --git a/sidecar/src/Eryx.AgentHost/Services/CopilotWorkflowRunner.cs b/sidecar/src/Eryx.AgentHost/Services/CopilotWorkflowRunner.cs index d96568d..31af9e4 100644 --- a/sidecar/src/Eryx.AgentHost/Services/CopilotWorkflowRunner.cs +++ b/sidecar/src/Eryx.AgentHost/Services/CopilotWorkflowRunner.cs @@ -43,6 +43,7 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner throw new InvalidOperationException(validationError.Message); } + ConcurrentDictionary toolNamesByCallId = new(StringComparer.Ordinal); await using AgentBundle bundle = await AgentBundle.CreateAsync( command, (agent, request, invocation) => RequestApprovalAsync( @@ -50,6 +51,7 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner agent, request, invocation, + toolNamesByCallId, onApproval, cancellationToken), cancellationToken); @@ -85,7 +87,8 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner AgentActivityEventDto? activity = TryCreateActivityFromRequest( command, requestInfo, - activeAgent); + activeAgent, + toolNamesByCallId); if (activity is not null) { @@ -239,10 +242,11 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner PatternAgentDefinitionDto agent, PermissionRequest request, PermissionInvocation invocation, + IReadOnlyDictionary toolNamesByCallId, Func onApproval, CancellationToken cancellationToken) { - TryGetApprovalToolName(request, out string? toolName); + TryGetApprovalToolName(request, toolNamesByCallId, out string? toolName); if (!RequiresToolCallApproval(command.Pattern.ApprovalPolicy, agent.Id, toolName)) { @@ -294,7 +298,8 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner private static AgentActivityEventDto? TryCreateActivityFromRequest( RunTurnCommandDto command, RequestInfoEvent requestInfo, - AgentIdentity? activeAgent) + AgentIdentity? activeAgent, + ConcurrentDictionary toolNamesByCallId) { if (TryGetHandoffTarget(command.Pattern, requestInfo, out AgentIdentity handoffAgent)) { @@ -305,11 +310,17 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner sourceAgent: activeAgent); } - if (!activeAgent.HasValue || !TryGetToolName(requestInfo, out string toolName)) + if (!activeAgent.HasValue + || !TryGetToolRequestInfo(requestInfo, out string toolName, out string? toolCallId)) { return null; } + if (!string.IsNullOrWhiteSpace(toolCallId)) + { + toolNamesByCallId[toolCallId] = toolName; + } + return CreateActivityEvent( command, activityType: "tool-calling", @@ -353,11 +364,15 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner return !string.IsNullOrWhiteSpace(agent.AgentName); } - private static bool TryGetToolName(RequestInfoEvent requestInfo, out string toolName) + private static bool TryGetToolRequestInfo( + RequestInfoEvent requestInfo, + out string toolName, + out string? toolCallId) { if (TryReadPortableValue(requestInfo.Request.Data, FunctionCallContentType, out object? functionCall)) { toolName = GetStringProperty(functionCall, "Name") ?? "function"; + toolCallId = NormalizeOptionalString(GetStringProperty(functionCall, "CallId")); return true; } @@ -366,22 +381,26 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner toolName = GetStringProperty(mcpToolCall, "ToolName") ?? GetStringProperty(mcpToolCall, "ServerName") ?? string.Empty; + toolCallId = NormalizeOptionalString(GetStringProperty(mcpToolCall, "CallId")); return !string.IsNullOrWhiteSpace(toolName); } - if (TryReadPortableValue(requestInfo.Request.Data, CodeInterpreterToolCallContentType, out _)) + if (TryReadPortableValue(requestInfo.Request.Data, CodeInterpreterToolCallContentType, out object? codeInterpreterToolCall)) { toolName = "code interpreter"; + toolCallId = NormalizeOptionalString(GetStringProperty(codeInterpreterToolCall, "CallId")); return true; } if (TryReadPortableValue(requestInfo.Request.Data, ImageGenerationToolCallContentType, out _)) { toolName = "image generation"; + toolCallId = null; return true; } toolName = string.Empty; + toolCallId = null; return false; } @@ -489,13 +508,36 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner string.Equals(candidate, toolName, StringComparison.OrdinalIgnoreCase)); } - internal static bool TryGetApprovalToolName(PermissionRequest request, out string? toolName) + internal static bool TryGetApprovalToolName( + PermissionRequest request, + IReadOnlyDictionary? toolNamesByCallId, + out string? toolName) { toolName = request switch { PermissionRequestMcp mcp when !string.IsNullOrWhiteSpace(mcp.ToolName) => mcp.ToolName.Trim(), PermissionRequestCustomTool customTool when !string.IsNullOrWhiteSpace(customTool.ToolName) => customTool.ToolName.Trim(), PermissionRequestHook hook when !string.IsNullOrWhiteSpace(hook.ToolName) => hook.ToolName.Trim(), + _ => null, + }; + + if (!string.IsNullOrWhiteSpace(toolName)) + { + return true; + } + + string? toolCallId = NormalizeOptionalString(GetStringProperty(request, "ToolCallId")); + if (toolCallId is not null + && toolNamesByCallId is not null + && toolNamesByCallId.TryGetValue(toolCallId, out string? resolvedToolName) + && !string.IsNullOrWhiteSpace(resolvedToolName)) + { + toolName = resolvedToolName.Trim(); + return true; + } + + toolName = request switch + { PermissionRequestUrl => "web_fetch", _ => null, }; @@ -503,6 +545,9 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner return !string.IsNullOrWhiteSpace(toolName); } + internal static bool TryGetApprovalToolName(PermissionRequest request, out string? toolName) + => TryGetApprovalToolName(request, toolNamesByCallId: null, out toolName); + private static string CreateApprovalRequestId() { return $"approval-{Guid.NewGuid():N}"; @@ -530,6 +575,11 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner return instance?.GetType().GetProperty(propertyName)?.GetValue(instance) as string; } + private static string? NormalizeOptionalString(string? value) + { + return string.IsNullOrWhiteSpace(value) ? null : value.Trim(); + } + private static StreamingSegment GetOrCreateSegment(List segments, string messageId, string authorName) { StreamingSegment? existing = segments.LastOrDefault(segment => segment.MessageId == messageId); diff --git a/sidecar/tests/Eryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs b/sidecar/tests/Eryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs index 31726cb..21ad95f 100644 --- a/sidecar/tests/Eryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs +++ b/sidecar/tests/Eryx.AgentHost.Tests/CopilotWorkflowRunnerTests.cs @@ -258,7 +258,7 @@ public sealed class CopilotWorkflowRunnerTests } [Fact] - public void TryGetApprovalToolName_ReadsMcpCustomHookAndUrlRequests() + public void TryGetApprovalToolName_ReadsMcpCustomAndHookRequests() { Assert.True( CopilotWorkflowRunner.TryGetApprovalToolName( @@ -296,18 +296,6 @@ public sealed class CopilotWorkflowRunnerTests out string? hookToolName)); Assert.Equal("web_fetch", hookToolName); - Assert.True( - CopilotWorkflowRunner.TryGetApprovalToolName( - new PermissionRequestUrl - { - Kind = "url", - ToolCallId = "tool-call-1", - Intention = "Fetch the requested page", - Url = "https://example.com/docs", - }, - out string? urlToolName)); - Assert.Equal("web_fetch", urlToolName); - Assert.False( CopilotWorkflowRunner.TryGetApprovalToolName( new PermissionRequestShell @@ -325,6 +313,77 @@ public sealed class CopilotWorkflowRunnerTests Assert.Null(shellToolName); } + [Fact] + public void TryGetApprovalToolName_UsesToolCallLookupForPermissionCategoriesWithoutDirectToolNames() + { + Dictionary toolNamesByCallId = new(StringComparer.Ordinal) + { + ["tool-call-url"] = "web_fetch", + ["tool-call-shell"] = "shell", + ["tool-call-read"] = "view", + }; + + Assert.True( + CopilotWorkflowRunner.TryGetApprovalToolName( + new PermissionRequestUrl + { + Kind = "url", + ToolCallId = "tool-call-url", + Intention = "Fetch the requested page", + Url = "https://example.com/docs", + }, + toolNamesByCallId, + out string? urlToolName)); + Assert.Equal("web_fetch", urlToolName); + + Assert.True( + CopilotWorkflowRunner.TryGetApprovalToolName( + new PermissionRequestShell + { + Kind = "shell", + ToolCallId = "tool-call-shell", + FullCommandText = "curl https://example.com/docs", + Intention = "Fetch documentation with curl", + Commands = [], + PossiblePaths = [], + PossibleUrls = [], + HasWriteFileRedirection = false, + CanOfferSessionApproval = false, + }, + toolNamesByCallId, + out string? shellToolName)); + Assert.Equal("shell", shellToolName); + + Assert.True( + CopilotWorkflowRunner.TryGetApprovalToolName( + new PermissionRequestRead + { + Kind = "read", + ToolCallId = "tool-call-read", + Intention = "Inspect a file", + Path = "README.md", + }, + toolNamesByCallId, + out string? readToolName)); + Assert.Equal("view", readToolName); + } + + [Fact] + public void TryGetApprovalToolName_FallsBackToWebFetchForUncorrelatedUrlRequests() + { + Assert.True( + CopilotWorkflowRunner.TryGetApprovalToolName( + new PermissionRequestUrl + { + Kind = "url", + ToolCallId = "tool-call-1", + Intention = "Fetch the requested page", + Url = "https://example.com/docs", + }, + out string? urlToolName)); + Assert.Equal("web_fetch", urlToolName); + } + [Fact] public void BuildPermissionApprovalEvent_IncludesToolContextWhenKnown() {