Fix PR comments

This commit is contained in:
Peter Ibekwe
2026-06-15 17:42:12 -07:00
Unverified
parent 48f137c1ef
commit 2c6c17054e
4 changed files with 19 additions and 20 deletions
@@ -171,7 +171,7 @@ internal sealed class InvokeFunctionToolExecutor(
this.Logger.LogWarning(
"Approval response '{RequestId}' did not match any pending invocation on '{ActionId}'.",
approval.RequestId, this.Id);
await this.AssignErrorAsync(context, "Function invocation was not approved by user.").ConfigureAwait(false);
await this.AssignErrorAsync(context, "No pending approval matched the response.").ConfigureAwait(false);
}
else if (!approval.Approved)
{
@@ -184,11 +184,10 @@ internal sealed class InvokeFunctionToolExecutor(
}
else
{
// Snapshot was consumed by a concurrent delivery; surface the not-approved error.
this.Logger.LogWarning(
"Approval response '{RequestId}' had no remaining pending snapshot on '{ActionId}'.",
approval.RequestId, this.Id);
await this.AssignErrorAsync(context, "Function invocation was not approved by user.").ConfigureAwait(false);
await this.AssignErrorAsync(context, "No pending approval matched the response.").ConfigureAwait(false);
}
}
}
@@ -149,14 +149,15 @@ internal sealed class InvokeMcpToolExecutor(
.OfType<ToolApprovalResponseContent>()
.FirstOrDefault(r => this._approvalSnapshots.ContainsKey(r.RequestId));
if (approvalResponse?.Approved != true)
if (approvalResponse is null)
{
// Rejected, no matching pending, or unknown request id: surface a not-approved
// error and drop any matched snapshot.
if (approvalResponse is not null)
{
this._approvalSnapshots.TryRemove(approvalResponse.RequestId, out _);
}
await this.AssignErrorAsync(context, "No pending approval matched the response.").ConfigureAwait(false);
return;
}
if (!approvalResponse.Approved)
{
this._approvalSnapshots.TryRemove(approvalResponse.RequestId, out _);
await this.AssignErrorAsync(context, "MCP tool invocation was not approved by user.").ConfigureAwait(false);
return;
}
@@ -165,7 +166,7 @@ internal sealed class InvokeMcpToolExecutor(
// Headers are re-evaluated (they may contain auth secrets not persisted to state).
if (!this._approvalSnapshots.TryRemove(approvalResponse.RequestId, out ApprovalSnapshot? snapshot))
{
await this.AssignErrorAsync(context, "MCP tool invocation was not approved by user.").ConfigureAwait(false);
await this.AssignErrorAsync(context, "No pending approval matched the response.").ConfigureAwait(false);
return;
}
@@ -905,7 +905,7 @@ public sealed class InvokeFunctionToolExecutorTest(ITestOutputHelper output) : W
i.Method.Name == nameof(IWorkflowContext.QueueStateUpdateAsync)
&& i.Arguments.Count >= 2
&& i.Arguments[1] is StringValue sv
&& sv.Value.Contains("not approved by user"));
&& sv.Value.Contains("No pending approval"));
}
/// <summary>
@@ -1003,13 +1003,13 @@ public sealed class InvokeFunctionToolExecutorTest(ITestOutputHelper output) : W
// Act
await action.CaptureResponseAsync(mockContext.Object, response, CancellationToken.None);
// Assert - the valid approval drove invocation; no not-approved error was assigned.
// Assert - the valid approval drove invocation; no error was assigned.
Assert.True(functionWasInvoked);
Assert.DoesNotContain(mockContext.Invocations, i =>
i.Method.Name == nameof(IWorkflowContext.QueueStateUpdateAsync)
&& i.Arguments.Count >= 2
&& i.Arguments[1] is StringValue sv
&& sv.Value.Contains("not approved by user"));
&& sv.Value.StartsWith("Error:", StringComparison.Ordinal));
}
/// <summary>
@@ -1050,12 +1050,12 @@ public sealed class InvokeFunctionToolExecutorTest(ITestOutputHelper output) : W
// Assert - the registered AIFunction was invoked exactly once.
Assert.Equal(1, invocationCount);
// The second delivery surfaced the not-approved error path.
// The second delivery surfaced the no-pending-approval error.
Assert.Contains(mockContext.Invocations, i =>
i.Method.Name == nameof(IWorkflowContext.QueueStateUpdateAsync)
&& i.Arguments.Count >= 2
&& i.Arguments[1] is StringValue sv
&& sv.Value.Contains("not approved by user"));
&& sv.Value.Contains("No pending approval"));
}
/// <summary>
@@ -1090,7 +1090,7 @@ public sealed class InvokeFunctionToolExecutorTest(ITestOutputHelper output) : W
await action.CaptureResponseAsync(mockContext.Object, response, CancellationToken.None);
// Assert - the host-computed result was assigned to Output.Result and no
// not-approved error was emitted.
// error was emitted.
Assert.Contains(mockContext.Invocations, i =>
i.Method.Name == nameof(IWorkflowContext.QueueStateUpdateAsync)
&& i.Arguments.Count >= 2
@@ -1100,7 +1100,7 @@ public sealed class InvokeFunctionToolExecutorTest(ITestOutputHelper output) : W
i.Method.Name == nameof(IWorkflowContext.QueueStateUpdateAsync)
&& i.Arguments.Count >= 2
&& i.Arguments[1] is StringValue sv
&& sv.Value.Contains("not approved by user"));
&& sv.Value.StartsWith("Error:", StringComparison.Ordinal));
}
/// <summary>
@@ -1585,8 +1585,7 @@ public sealed class InvokeMcpToolExecutorTest(ITestOutputHelper output) : Workfl
/// <summary>
/// Builds an approval response paired to the request id stamped on the emitted
/// <c>MCPToolApprovalRequestContent</c>. Mirrors the framework's symmetric
/// content-id rewriting at the envelope boundary.
/// <c>ToolApprovalRequestContent</c>.
/// </summary>
private static ExternalInputResponse CreateApprovalResponseFor(IReadOnlyList<ExternalInputRequest> emittedRequests, bool approved)
{