.NET: [BREAKING] Prevent nulls in AIAgent.Id property

[SergeyMenshykh]

This PR refactors the AIAgent.Id property to prevent it from returning nulls and, as a result, breaking consumer code. With the existing code, it's possible for a derivative to override the Id property and return null. The refactored code prevents this by disallowing the AIAgent.Id property from being overridden and falling back to a Guid value if a derivative returns null.

Contributes to: #2542

[Copilot]

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

[rogerbarreto]

I have some second thoughts about how bad is always generating an Guid for the agent.

I would suggest either:

1. Having Id null by default and that is only set when the agent has indeed a server-side identifier. Where the caller is the sole responsible to indexing/identifying it's agents for further handling.

2. Do what we already did with AgentThread (Remove Id from AIAgent) where the ConversationId was removed from the abstraction recently and added only to the specialized ChatClientAgentThread not-null only when there's a representation server-side.

3. Keeping Id as always a GUID and introducing ServerId to a specialized type, so Assistant / Foundry Agents V1/V2 would have both Id as a guid and ServerId in the ChatClientAgent specialization

4. Revert AgentThread.ConversationId removal and bring it back again to be consistent with Agent.Id.

[SergeyMenshykh]

1. Having Id null by default and that is only set when the agent has indeed a server-side identifier. Where the caller is the sole responsible to indexing/identifying it's agents for further handling.

We don't want it to be nullable because many calling sites would need to be updated to var agentId = agent.Id ?? Guid.NewGuid().ToString("N") to handle the possibility of the id being null. There's also a possibility that someone will write code like this: var agentId = agent.Id! to get rid of a warning when working with an agent with server-side identifier, and it will work until they change the agent implementation to one that does not have a server-side identifier. The overall agreement at the API review meeting was that the easier we can make it for developers to get it right, the better.

2. Do what we already did with AgentThread (Remove Id from AIAgent) where the ConversationId was removed from the abstraction recently and added only to the specialized ChatClientAgentThread not-null only when there's a representation server-side.

In this case, consumers won't be able to access the id via a variable of the AIAgent type, which will complicate writing custom decorators and may require casting the AIAgent type variable to a specific agent type to access the id property.

3. Keeping Id as always a GUID and introducing ServerId to a specialized type, so Assistant / Foundry Agents V1/V2 would have both Id as a guid and ServerId in the ChatClientAgent specialization

Is the idea to introduce ChatClientAgent specialization for Assistant / Foundry Agents V1/V2 that will have the ServerId in addition to the Id property from AIAgent class?

Note: The ChatClientAgent is sealed and thus can't be inherited.

4. Revert AgentThread.ConversationId removal and bring it back again to be consistent with Agent.Id.

Agent.Id and AgentThread are consistent today in a way that abstracts the consumers from the agent implementation details. This allows consumers to work with agents polymorphically, without dealing with those details. Adding an AgentThread.ConversationId property to the abstractions, which is only relevant to a few agents, does not seem right.