Skip to content

Commit 989b6eb

Browse files
.NET: [BREAKING] Prevent nulls in AIAgent property (#2719)
* prevent nulls in AIAgent property * address feedback
1 parent 3481914 commit 989b6eb

File tree

13 files changed

+56
-33
lines changed

13 files changed

+56
-33
lines changed

dotnet/src/Microsoft.Agents.AI.A2A/A2AAgent.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ public override async IAsyncEnumerable<AgentRunResponseUpdate> RunStreamingAsync
198198
}
199199

200200
/// <inheritdoc/>
201-
public override string Id => this._id ?? base.Id;
201+
protected override string? IdCore => this._id;
202202

203203
/// <inheritdoc/>
204204
public override string? Name => this._name ?? base.Name;

dotnet/src/Microsoft.Agents.AI.Abstractions/AIAgent.cs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,6 @@ namespace Microsoft.Agents.AI;
2222
[DebuggerDisplay("{DisplayName,nq}")]
2323
public abstract class AIAgent
2424
{
25-
/// <summary>Default ID of this agent instance.</summary>
26-
private readonly string _id = Guid.NewGuid().ToString("N");
27-
2825
/// <summary>
2926
/// Gets the unique identifier for this agent instance.
3027
/// </summary>
@@ -37,7 +34,19 @@ public abstract class AIAgent
3734
/// agent instances in multi-agent scenarios. They should remain stable for the lifetime
3835
/// of the agent instance.
3936
/// </remarks>
40-
public virtual string Id => this._id;
37+
public string Id { get => this.IdCore ?? field; } = Guid.NewGuid().ToString("N");
38+
39+
/// <summary>
40+
/// Gets a custom identifier for the agent, which can be overridden by derived classes.
41+
/// </summary>
42+
/// <value>
43+
/// A string representing the agent's identifier, or <see langword="null"/> if the default ID should be used.
44+
/// </value>
45+
/// <remarks>
46+
/// Derived classes can override this property to provide a custom identifier.
47+
/// When <see langword="null"/> is returned, the <see cref="Id"/> property will use the default randomly-generated identifier.
48+
/// </remarks>
49+
protected virtual string? IdCore => null;
4150

4251
/// <summary>
4352
/// Gets the human-readable name of the agent.
@@ -61,7 +70,7 @@ public abstract class AIAgent
6170
/// This property provides a guaranteed non-null string suitable for display in user interfaces,
6271
/// logs, or other contexts where a readable identifier is needed.
6372
/// </remarks>
64-
public virtual string DisplayName => this.Name ?? this.Id ?? this._id; // final fallback to _id in case Id override returns null
73+
public virtual string DisplayName => this.Name ?? this.Id;
6574

6675
/// <summary>
6776
/// Gets a description of the agent's purpose, capabilities, or behavior.

dotnet/src/Microsoft.Agents.AI.Abstractions/DelegatingAIAgent.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ protected DelegatingAIAgent(AIAgent innerAgent)
5454
protected AIAgent InnerAgent { get; }
5555

5656
/// <inheritdoc />
57-
public override string Id => this.InnerAgent.Id;
57+
protected override string? IdCore => this.InnerAgent.Id;
5858

5959
/// <inheritdoc />
6060
public override string? Name => this.InnerAgent.Name;

dotnet/src/Microsoft.Agents.AI.DurableTask/EntityAgentWrapper.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ internal sealed class EntityAgentWrapper(
1919
private readonly IServiceProvider? _entityScopedServices = entityScopedServices;
2020

2121
// The ID of the agent is always the entity ID.
22-
public override string Id => this._entityContext.Id.ToString();
22+
protected override string? IdCore => this._entityContext.Id.ToString();
2323

2424
public override async Task<AgentRunResponse> RunAsync(
2525
IEnumerable<ChatMessage> messages,

dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowHostAgent.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public WorkflowHostAgent(Workflow workflow, string? id = null, string? name = nu
3939
this._describeTask = this._workflow.DescribeProtocolAsync().AsTask();
4040
}
4141

42-
public override string Id => this._id ?? base.Id;
42+
protected override string? IdCore => this._id;
4343
public override string? Name { get; }
4444
public override string? Description { get; }
4545

dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ public ChatClientAgent(IChatClient chatClient, ChatClientAgentOptions? options,
121121
public IChatClient ChatClient { get; }
122122

123123
/// <inheritdoc/>
124-
public override string Id => this._agentOptions?.Id ?? base.Id;
124+
protected override string? IdCore => this._agentOptions?.Id;
125125

126126
/// <inheritdoc/>
127127
public override string? Name => this._agentOptions?.Name;

dotnet/tests/Microsoft.Agents.AI.Abstractions.UnitTests/AIAgentTests.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,13 +214,31 @@ public async Task InvokeStreamingWithSingleMessageCallsMockedInvokeWithMessageIn
214214
[Fact]
215215
public void ValidateAgentIDIsIdempotent()
216216
{
217+
// Arrange
217218
var agent = new MockAgent();
218219

220+
// Act
219221
string id = agent.Id;
222+
223+
// Assert
220224
Assert.NotNull(id);
221225
Assert.Equal(id, agent.Id);
222226
}
223227

228+
[Fact]
229+
public void ValidateAgentIDCanBeProvidedByDerivedAgentClass()
230+
{
231+
// Arrange
232+
var agent = new MockAgent(id: "test-agent-id");
233+
234+
// Act
235+
string id = agent.Id;
236+
237+
// Assert
238+
Assert.NotNull(id);
239+
Assert.Equal("test-agent-id", id);
240+
}
241+
224242
#region GetService Method Tests
225243

226244
/// <summary>
@@ -344,6 +362,13 @@ public abstract class TestAgentThread : AgentThread;
344362

345363
private sealed class MockAgent : AIAgent
346364
{
365+
public MockAgent(string? id = null)
366+
{
367+
this.IdCore = id;
368+
}
369+
370+
protected override string? IdCore { get; }
371+
347372
public override AgentThread GetNewThread()
348373
=> throw new NotImplementedException();
349374

dotnet/tests/Microsoft.Agents.AI.Abstractions.UnitTests/DelegatingAIAgentTests.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Threading.Tasks;
77
using Microsoft.Extensions.AI;
88
using Moq;
9+
using Moq.Protected;
910

1011
namespace Microsoft.Agents.AI.Abstractions.UnitTests;
1112

@@ -31,7 +32,7 @@ public DelegatingAIAgentTests()
3132
this._testThread = new TestAgentThread();
3233

3334
// Setup inner agent mock
34-
this._innerAgentMock.Setup(x => x.Id).Returns("test-agent-id");
35+
this._innerAgentMock.Protected().SetupGet<string>("IdCore").Returns("test-agent-id");
3536
this._innerAgentMock.Setup(x => x.Name).Returns("Test Agent");
3637
this._innerAgentMock.Setup(x => x.Description).Returns("Test Description");
3738
this._innerAgentMock.Setup(x => x.GetNewThread()).Returns(this._testThread);
@@ -93,7 +94,7 @@ public void Id_DelegatesToInnerAgent()
9394

9495
// Assert
9596
Assert.Equal("test-agent-id", id);
96-
this._innerAgentMock.Verify(x => x.Id, Times.Once);
97+
this._innerAgentMock.Protected().VerifyGet<string>("IdCore", Times.Once());
9798
}
9899

99100
/// <summary>

dotnet/tests/Microsoft.Agents.AI.Hosting.AGUI.AspNetCore.IntegrationTests/BasicStreamingTests.cs

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -276,15 +276,9 @@ public async ValueTask DisposeAsync()
276276
[SuppressMessage("Performance", "CA1812:Avoid uninstantiated internal classes", Justification = "Instantiated via dependency injection")]
277277
internal sealed class FakeChatClientAgent : AIAgent
278278
{
279-
public FakeChatClientAgent()
280-
{
281-
this.Id = "fake-agent";
282-
this.Description = "A fake agent for testing";
283-
}
284-
285-
public override string Id { get; }
279+
protected override string? IdCore => "fake-agent";
286280

287-
public override string? Description { get; }
281+
public override string? Description => "A fake agent for testing";
288282

289283
public override AgentThread GetNewThread()
290284
{
@@ -350,15 +344,9 @@ public FakeInMemoryAgentThread(JsonElement serializedThread, JsonSerializerOptio
350344
[SuppressMessage("Performance", "CA1812:Avoid uninstantiated internal classes", Justification = "Instantiated via dependency injection")]
351345
internal sealed class FakeMultiMessageAgent : AIAgent
352346
{
353-
public FakeMultiMessageAgent()
354-
{
355-
this.Id = "fake-multi-message-agent";
356-
this.Description = "A fake agent that sends multiple messages for testing";
357-
}
358-
359-
public override string Id { get; }
347+
protected override string? IdCore => "fake-multi-message-agent";
360348

361-
public override string? Description { get; }
349+
public override string? Description => "A fake agent that sends multiple messages for testing";
362350

363351
public override AgentThread GetNewThread()
364352
{

dotnet/tests/Microsoft.Agents.AI.Hosting.AGUI.AspNetCore.UnitTests/AGUIEndpointRouteBuilderExtensionsTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ private static List<JsonElement> ParseSseEvents(string responseContent)
421421

422422
private sealed class MultiResponseAgent : AIAgent
423423
{
424-
public override string Id => "multi-response-agent";
424+
protected override string? IdCore => "multi-response-agent";
425425

426426
public override string? Description => "Agent that produces multiple text chunks";
427427

@@ -510,7 +510,7 @@ public TestInMemoryAgentThread(JsonElement serializedThreadState, JsonSerializer
510510

511511
private sealed class TestAgent : AIAgent
512512
{
513-
public override string Id => "test-agent";
513+
protected override string? IdCore => "test-agent";
514514

515515
public override string? Description => "Test agent";
516516

0 commit comments

Comments
 (0)