Skip to content

Commit e9ebb83

Browse files
desmarestCopilot
desmarest
and
Copilot
committed
W365 sample: address PR #305 review feedback
Substantive bug fixes (Copilot reviewer): - AgentMetrics.InvokeObservedAgentOperation: now properly async, awaits func(), tracks success based on exceptions, and only finalizes the Activity after the operation completes. - AspNetExtensions.OnMessageReceived: case-insensitive Bearer scheme check; wrap JwtSecurityToken parsing in try/catch so malformed/opaque tokens produce 401 instead of 500; remove redundant null-conditional after IsNullOrEmpty guard; drop the undisposed HttpClient passed to ConfigurationManager (use the overload without it); refactor audience validation to LINQ Where. - A365OtelWrapper.ResolveTenantAndAgentId: null-check turnContext up front; use string.IsNullOrEmpty(agentId) instead of the no-op '?? Guid.Empty.ToString()'; narrow generic catch around observability registration. - ComputerUseOrchestrator: guard conversationId prefix length (avoid ArgumentOutOfRangeException on short conv ids); URL-encode _oneDriveUserId in Graph paths so UPNs produce valid URLs; thread CancellationToken through Upload/Share OneDrive flows; narrow generic catches to filtered (HttpRequestException/JsonException/FormatException) with OperationCanceledException re-thrown. - onStatusUpdate callback: change from Action<string> to Func<string, Task> and await it at all call sites in MyAgent and orchestrator — exceptions no longer get swallowed and ConfigureAwait(false) on an unawaited Task is no longer a no-op. Style cleanups (github-code-quality bot): - MyAgent.WelcomeMessageAsync and ExtractW365ToolListError: replace implicit-filter foreach with explicit LINQ Where/OfType/FirstOrDefault. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent fe469b4 commit e9ebb83

5 files changed

Lines changed: 115 additions & 75 deletions

File tree

‎dotnet/w365-computer-use/sample-agent/Agent/MyAgent.cs‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -152,12 +152,11 @@ await AgentMetrics.InvokeObservedAgentOperation(
152152
turnContext,
153153
async () =>
154154
{
155-
foreach (ChannelAccount member in turnContext.Activity.MembersAdded)
155+
var recipientId = turnContext.Activity.Recipient.Id;
156+
var newMembers = turnContext.Activity.MembersAdded.Where(m => m.Id != recipientId);
157+
foreach (var member in newMembers)
156158
{
157-
if (member.Id != turnContext.Activity.Recipient.Id)
158-
{
159-
await turnContext.SendActivityAsync(AgentWelcomeMessage);
160-
}
159+
await turnContext.SendActivityAsync(AgentWelcomeMessage);
161160
}
162161
});
163162
}
@@ -245,7 +244,7 @@ await A365OtelWrapper.InvokeObservedAgentOperation(
245244
additionalTools: nonCuaAdditionalTools,
246245
mcpClient: null,
247246
graphAccessToken: null,
248-
onStatusUpdate: status => turnContext.StreamingResponse.QueueInformativeUpdateAsync(status).ConfigureAwait(false),
247+
onStatusUpdate: status => turnContext.StreamingResponse.QueueInformativeUpdateAsync(status),
249248
onCuaStarting: null,
250249
onFolderLinkReady: null,
251250
includeCuaTool: false,
@@ -310,7 +309,7 @@ await A365OtelWrapper.InvokeObservedAgentOperation(
310309
additionalTools: additionalTools,
311310
mcpClient: mcpClient,
312311
graphAccessToken: graphToken,
313-
onStatusUpdate: status => turnContext.StreamingResponse.QueueInformativeUpdateAsync(status).ConfigureAwait(false),
312+
onStatusUpdate: status => turnContext.StreamingResponse.QueueInformativeUpdateAsync(status),
314313
onCuaStarting: async (isNewSession) =>
315314
{
316315
if (isNewSession)
@@ -507,19 +506,18 @@ private static bool IsDevelopment()
507506
return null;
508507
}
509508

510-
foreach (var tool in additionalTools)
509+
var errorTool = additionalTools
510+
.OfType<AIFunction>()
511+
.FirstOrDefault(fn => string.Equals(fn.Name, "Error", StringComparison.OrdinalIgnoreCase));
512+
if (errorTool == null)
511513
{
512-
if (tool is not AIFunction fn) continue;
513-
if (!string.Equals(fn.Name, "Error", StringComparison.OrdinalIgnoreCase)) continue;
514-
515-
var description = fn.Description ?? string.Empty;
516-
var extracted = ExtractQuotedField(description, "ExceptionMessage=")
517-
?? ExtractQuotedField(description, "Message=")
518-
?? (string.IsNullOrWhiteSpace(description) ? null : description);
519-
return extracted;
514+
return null;
520515
}
521516

522-
return null;
517+
var description = errorTool.Description ?? string.Empty;
518+
return ExtractQuotedField(description, "ExceptionMessage=")
519+
?? ExtractQuotedField(description, "Message=")
520+
?? (string.IsNullOrWhiteSpace(description) ? null : description);
523521
}
524522

525523
private static string? ExtractQuotedField(string source, string fieldPrefix)

‎dotnet/w365-computer-use/sample-agent/AspNetExtensions.cs‎

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,12 @@ public static void AddAgentAspNetAuthentication(this IServiceCollection services
4040
throw new ArgumentException($"{nameof(TokenValidationOptions)}:Audiences requires at least one ClientId");
4141
}
4242

43-
foreach (var audience in validationOptions.Audiences)
43+
var invalidAudiences = validationOptions.Audiences
44+
.Where(audience => !Guid.TryParse(audience, out _))
45+
.ToList();
46+
if (invalidAudiences.Count > 0)
4447
{
45-
if (!Guid.TryParse(audience, out _))
46-
{
47-
throw new ArgumentException($"{nameof(TokenValidationOptions)}:Audiences values must be a GUID");
48-
}
48+
throw new ArgumentException($"{nameof(TokenValidationOptions)}:Audiences values must be a GUID");
4949
}
5050

5151
if (validationOptions.ValidIssuers == null || validationOptions.ValidIssuers.Count == 0)
@@ -104,33 +104,42 @@ public static void AddAgentAspNetAuthentication(this IServiceCollection services
104104

105105
options.Events = new JwtBearerEvents
106106
{
107-
OnMessageReceived = async context =>
107+
OnMessageReceived = context =>
108108
{
109109
string authorizationHeader = context.Request.Headers.Authorization.ToString();
110110

111111
if (string.IsNullOrEmpty(authorizationHeader))
112112
{
113113
context.Options.TokenValidationParameters.ConfigurationManager ??= options.ConfigurationManager as BaseConfigurationManager;
114-
await Task.CompletedTask.ConfigureAwait(false);
115-
return;
114+
return Task.CompletedTask;
116115
}
117116

118-
string[] parts = authorizationHeader?.Split(' ')!;
119-
if (parts.Length != 2 || parts[0] != "Bearer")
117+
string[] parts = authorizationHeader.Split(' ');
118+
if (parts.Length != 2 || !string.Equals(parts[0], "Bearer", StringComparison.OrdinalIgnoreCase))
120119
{
121120
context.Options.TokenValidationParameters.ConfigurationManager ??= options.ConfigurationManager as BaseConfigurationManager;
122-
await Task.CompletedTask.ConfigureAwait(false);
123-
return;
121+
return Task.CompletedTask;
124122
}
125123

126-
JwtSecurityToken token = new(parts[1]);
127-
string issuer = token.Claims.FirstOrDefault(claim => claim.Type == AuthenticationConstants.IssuerClaim)?.Value!;
124+
string? issuer = null;
125+
try
126+
{
127+
JwtSecurityToken token = new(parts[1]);
128+
issuer = token.Claims.FirstOrDefault(claim => claim.Type == AuthenticationConstants.IssuerClaim)?.Value;
129+
}
130+
catch (ArgumentException)
131+
{
132+
// Malformed / opaque token — fall back to default configuration so the JwtBearer
133+
// handler emits a 401 instead of a 500.
134+
context.Options.TokenValidationParameters.ConfigurationManager ??= options.ConfigurationManager as BaseConfigurationManager;
135+
return Task.CompletedTask;
136+
}
128137

129138
if (validationOptions.AzureBotServiceTokenHandling && AuthenticationConstants.BotFrameworkTokenIssuer.Equals(issuer))
130139
{
131140
context.Options.TokenValidationParameters.ConfigurationManager = _openIdMetadataCache.GetOrAdd(validationOptions.AzureBotServiceOpenIdMetadataUrl, key =>
132141
{
133-
return new ConfigurationManager<OpenIdConnectConfiguration>(validationOptions.AzureBotServiceOpenIdMetadataUrl, new OpenIdConnectConfigurationRetriever(), new HttpClient())
142+
return new ConfigurationManager<OpenIdConnectConfiguration>(validationOptions.AzureBotServiceOpenIdMetadataUrl, new OpenIdConnectConfigurationRetriever())
134143
{
135144
AutomaticRefreshInterval = openIdMetadataRefresh
136145
};
@@ -140,14 +149,14 @@ public static void AddAgentAspNetAuthentication(this IServiceCollection services
140149
{
141150
context.Options.TokenValidationParameters.ConfigurationManager = _openIdMetadataCache.GetOrAdd(validationOptions.OpenIdMetadataUrl, key =>
142151
{
143-
return new ConfigurationManager<OpenIdConnectConfiguration>(validationOptions.OpenIdMetadataUrl, new OpenIdConnectConfigurationRetriever(), new HttpClient())
152+
return new ConfigurationManager<OpenIdConnectConfiguration>(validationOptions.OpenIdMetadataUrl, new OpenIdConnectConfigurationRetriever())
144153
{
145154
AutomaticRefreshInterval = openIdMetadataRefresh
146155
};
147156
});
148157
}
149158

150-
await Task.CompletedTask.ConfigureAwait(false);
159+
return Task.CompletedTask;
151160
},
152161
OnTokenValidated = context => Task.CompletedTask,
153162
OnForbidden = context => Task.CompletedTask,

0 commit comments

Comments
 (0)