.NET: Set AgentResponse.CreatedAt from the A2A task status timestamp - #7865
.NET: Set AgentResponse.CreatedAt from the A2A task status timestamp#7865Charles (anneheartrecord) wants to merge 1 commit into
Conversation
A2AAgent left CreatedAt unset on every response it built, so consumers saw a null creation time for A2A agents while the same property is populated by CopilotStudioAgent and GitHubCopilotAgent. The value was already reachable: AgentTaskStatus carries a Timestamp recording when the status was produced, and the surrounding code already reads Status.State from the same object. Set CreatedAt from Status.Timestamp in the three converters whose input carries one - the AgentTask response and update paths, and the task status update event path. The Message and TaskArtifactUpdateEvent converters are left alone because neither type exposes a timestamp.
There was a problem hiding this comment.
Pull request overview
Populates A2A response timestamps from task status metadata.
Changes:
- Maps task status timestamps onto three task-backed response paths.
- Adds regression coverage for task response conversion.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
A2AAgent.cs |
Maps status timestamps to CreatedAt. |
A2AAgentTests.cs |
Tests non-streaming and fallback-streaming mappings. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| // Arrange - an explicit timestamp distinct from "now", so the assertion cannot pass | ||
| // by accident against AgentTaskStatus.Timestamp's DateTimeOffset.UtcNow default. |
| { | ||
| AgentId = this.Id, | ||
| ResponseId = statusUpdateEvent.TaskId, | ||
| CreatedAt = statusUpdateEvent.Status.Timestamp, |
Luis Rodriguez (luisangelrod)
left a comment
There was a problem hiding this comment.
Reviewed the full change at 600df790, including all six response converters and the existing streaming status-event test path. The three new assignments are correctly limited to A2A inputs that carry a status timestamp, and leaving the message and artifact paths unset is appropriate because those inputs do not expose one.
I ran the complete Microsoft.Agents.AI.A2A.UnitTests project on net10.0: 146 passed, 0 failed, 0 skipped. I found no additional correctness issue beyond the two existing inline threads.
I agree that the stale AgentTaskStatus/UtcNow comment should be corrected and that the already-existing TaskStatusUpdateEvent test should assert CreatedAt before approval, since that gives direct coverage for the third changed converter with minimal added setup. I will recheck after those are addressed.
Generic disclosure: this review was prepared with AI assistance and verified against the full diff, source, and local test results.
| { | ||
| AgentId = this.Id, | ||
| ResponseId = task.Id, | ||
| CreatedAt = task.Status.Timestamp, |
There was a problem hiding this comment.
I’m not sure TaskStatus.Timestamp is the right value for AgentResponse.CreatedAt. It records when the current task status was set, not when the response was created.
OpenAI keeps these concepts separate:
- created_at - when the
Responseobject was created. - completed_at - when generation completed.
An A2A TaskStatus.Timestamp is closer to a status-transition timestamp, or possibly completed_at, than OpenAI’s created_at.
Clients that want the status timestamp can access the original task through RawRepresentation and copy it to CreatedAt themselves. I think we should leave CreatedAt null unless A2A provides an actual response creation time.
There was a problem hiding this comment.
That distinction makes sense. Status.Timestamp records when the task entered its current state, so for a completed task it's closer to completed_at than created_at, and AgentResponse has no completed-at slot to put it in. Agreed that leaving CreatedAt null beats teaching clients a wrong meaning, with RawRepresentation as the escape hatch.
One converter I think is different: ConvertToAgentResponseUpdate(TaskStatusUpdateEvent). There the update we emit is the status change itself, so the event's timestamp is when that update came into existence — same shape as #6791 mapping activity.Timestamp for Copilot Studio. I can trim the PR down to just that converter and drop the two task-level mappings, or close it entirely if you'd rather keep CreatedAt untouched across the board. Which do you prefer?
There was a problem hiding this comment.
Thanks, that distinction makes sense for a live status event. My concern is that CreatedAt would be patchy across the stream because it would be populated for only one event type.
A typical task stream contains updates in this sequence:
-
Task- current task state;CreatedAtwould be unset because the task status timestamp is not the response creation time. -
TaskStatusUpdateEvent- live status change;CreatedAtwould, as you suggested, be populated from the status timestamp. -
TaskArtifactUpdateEvent- artifact or artifact chunk;CreatedAtwould be unset because the event carries no timestamp. -
TaskStatusUpdateEvent- task completion;CreatedAtwould be populated from the status timestamp.
Given that, I think it may be clearer and more consistent to leave CreatedAt unset across these A2A update types rather than have it depend on the event representation.
|
Consistency across the stream is the stronger argument here — a CreatedAt that only ever shows up on status updates isn't something callers can rely on. Closing this one, and thanks for taking the time to think it through. |
What
A2AAgentnever setsAgentResponse.CreatedAt/AgentResponseUpdate.CreatedAt, so every response it produces reports a null creation time.CopilotStudioAgentandGitHubCopilotAgentboth populate it, so the metadata surface is inconsistent across agent implementations.This sets
CreatedAtfrom the A2A task status timestamp in the three converters whose input actually carries one.Why the value is available
AgentTaskStatusin the A2A SDK declares:The surrounding code already reads
Status.Statefrom the same object on adjacent lines, so nothing new is fetched or plumbed through — the timestamp is one property away at every site that now uses it.Scope: three of six converters
A2AAgenthas six response converters. Only three can be fixed; the other three are a data limitation rather than an oversight, so I left them alone:CreatedAt?ConvertToAgentResponse(AgentTask)task.StatusConvertToAgentResponseUpdate(AgentTask)task.StatusConvertToAgentResponseUpdate(TaskStatusUpdateEvent)statusUpdateEvent.StatusConvertToAgentResponse(Message)MessageAgentMessageexposes no timestampConvertToAgentResponseUpdate(Message)MessageConvertToAgentResponseUpdate(TaskArtifactUpdateEvent)Artifact/Append/LastChunkBehaviour change worth calling out
Consumers that previously saw
CreatedAt == nullfrom an A2A agent will now see a value on the task-backed paths. That is the intent, but it is observable, so flagging it rather than burying it. The message-backed paths still return null, which is accurate — there is no timestamp to report there.Tests
Two regression tests in
A2AAgentTests, covering the non-streaming and streaming task paths. Both use an explicit timestamp distinct from "now", so they cannot pass by accident againstAgentTaskStatus.Timestamp'sDateTimeOffset.UtcNowdefault.Verified red-before-green: with the source change reverted and the tests kept, both fail with
Assert.Equal() Failure: Values differ; with the change applied,Microsoft.Agents.AI.A2A.UnitTestsis 90/90 onnet10.0.dotnet format --verify-no-changesis clean on both changed projects.Not covered by a test: the
TaskStatusUpdateEventpath. Driving a status-update event stream through the existing stub handler was more setup than the one-line change seemed to warrant, so I would rather say so than imply coverage I do not have. Happy to add it if you would prefer the path tested before merge.One question
#6790citedA2AAgentandGitHubCopilotAgentas the reference for a complete metadata surface whileCopilotStudioAgentwas the outlier. After #6791 that has inverted —CopilotStudioAgentnow sets all four ofCreatedAt/FinishReason/RawRepresentation/AdditionalProperties, and the two references have gaps of their own. This PR closes theA2AAgentone.GitHubCopilotAgentlooks like it may be missingFinishReasonandAdditionalPropertieson its update path — worth a separate pass, or would you rather have the remaining gaps swept in one PR? Happy to do either; I would just rather ask than guess at the scope you want.