Skip to content

Commit 248435a

Browse files
westey-mglorious-beard
authored andcommitted
.Net: Fix bug where invoking with no message throws for some agents. (microsoft#11217)
### Motivation and Context We were relying on the thread being created when we added messages to it the first time, but in some cases no messages will be added, since the agent could be invoked with no messages. ### Description Fix bug where invoking with no message throws for some agents. ### Contribution Checklist <!-- Before submitting this PR, please make sure: --> - [ ] The code builds clean without any errors or warnings - [ ] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations - [ ] All unit tests pass, and I have added new tests where possible - [ ] I didn't break anyone 😄
1 parent c108c06 commit 248435a

File tree

5 files changed

+53
-1
lines changed

5 files changed

+53
-1
lines changed

dotnet/src/Agents/Abstractions/Agent.cs

+6
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,12 @@ protected virtual async Task<TThreadType> EnsureThreadExistsWithMessagesAsync<TT
198198
throw new KernelException($"{this.GetType().Name} currently only supports agent threads of type {nameof(TThreadType)}.");
199199
}
200200

201+
// We have to explicitly call create here to ensure that the thread is created
202+
// before we invoke using the thread. While threads will be created when
203+
// notified of new messages, some agents support invoking without a message,
204+
// and in that case no messages will be sent in the next step.
205+
await thread.CreateAsync(cancellationToken).ConfigureAwait(false);
206+
201207
// Notify the thread that new messages are available.
202208
foreach (var message in messages)
203209
{

dotnet/src/Agents/Abstractions/AgentThread.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public abstract class AgentThread
3232
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to monitor for cancellation requests. The default is <see cref="CancellationToken.None"/>.</param>
3333
/// <returns>A task that completes when the thread has been created.</returns>
3434
/// <exception cref="InvalidOperationException">The thread has been deleted.</exception>
35-
protected virtual async Task CreateAsync(CancellationToken cancellationToken = default)
35+
protected internal virtual async Task CreateAsync(CancellationToken cancellationToken = default)
3636
{
3737
if (this.IsDeleted)
3838
{

dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/InvokeConformance/BedrockAgentInvokeTests.cs

+8
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Copyright (c) Microsoft. All rights reserved.
22

3+
using System;
34
using System.Linq;
45
using System.Threading.Tasks;
56
using Microsoft.SemanticKernel;
@@ -40,6 +41,13 @@ public override Task InvokeWithoutThreadCreatesThreadAsync()
4041
return base.InvokeWithoutThreadCreatesThreadAsync();
4142
}
4243

44+
[Fact(Skip = "This test is for manual verification.")]
45+
public override Task InvokeWithoutMessageCreatesThreadAsync()
46+
{
47+
// The Bedrock agent does not support invoking without a message.
48+
return Assert.ThrowsAsync<InvalidOperationException>(async () => await base.InvokeWithoutThreadCreatesThreadAsync());
49+
}
50+
4351
[Fact(Skip = "This test is for manual verification.")]
4452
public override Task MultiStepInvokeWithPluginAndArgOverridesAsync()
4553
{

dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/InvokeConformance/InvokeTests.cs

+19
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,25 @@ public virtual async Task InvokeWithoutThreadCreatesThreadAsync()
6161
await this.Fixture.DeleteThread(firstResult.Thread);
6262
}
6363

64+
[RetryFact(3, 5000)]
65+
public virtual async Task InvokeWithoutMessageCreatesThreadAsync()
66+
{
67+
// Arrange
68+
var agent = this.Fixture.Agent;
69+
70+
// Act
71+
var asyncResults = agent.InvokeAsync([]);
72+
var results = await asyncResults.ToListAsync();
73+
74+
// Assert
75+
Assert.Single(results);
76+
var firstResult = results.First();
77+
Assert.NotNull(firstResult.Thread);
78+
79+
// Cleanup
80+
await this.Fixture.DeleteThread(firstResult.Thread);
81+
}
82+
6483
[RetryFact(3, 5000)]
6584
public virtual async Task ConversationMaintainsHistoryAsync()
6685
{

dotnet/src/IntegrationTests/Agents/CommonInterfaceConformance/InvokeStreamingConformance/InvokeStreamingTests.cs

+19
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,25 @@ public virtual async Task InvokeStreamingAsyncWithoutThreadCreatesThreadAsync()
6262
await this.Fixture.DeleteThread(firstResult.Thread);
6363
}
6464

65+
[RetryFact(3, 10_000)]
66+
public virtual async Task InvokeStreamingAsyncWithoutMessageCreatesThreadAsync()
67+
{
68+
// Arrange
69+
var agent = this.Fixture.Agent;
70+
71+
// Act
72+
var asyncResults = agent.InvokeStreamingAsync([]);
73+
var results = await asyncResults.ToListAsync();
74+
75+
// Assert
76+
var firstResult = results.First();
77+
var resultString = string.Join(string.Empty, results.Select(x => x.Message.Content));
78+
Assert.NotNull(firstResult.Thread);
79+
80+
// Cleanup
81+
await this.Fixture.DeleteThread(firstResult.Thread);
82+
}
83+
6584
[RetryFact(3, 10_000)]
6685
public virtual async Task ConversationMaintainsHistoryAsync()
6786
{

0 commit comments

Comments
 (0)