fix: Correctly set Append flag in TaskArtifactUpdateEvent and add UpdateArtifactAsync usage examples

[Copilot]

Fixes the Append property in TaskArtifactUpdateEvent to accurately reflect what actually occurred during artifact updates, rather than simply echoing the user's request. Additionally adds comprehensive usage examples for the UpdateArtifactAsync method in the samples.

Problem

When calling UpdateArtifactAsync(taskId, artifact, append: true) on a task with no existing artifacts, the system would:

1. Create a new artifact (correct behavior)
2. Set Append = true in the event (incorrect - misleading)

This was confusing because the event indicated that appending occurred when actually a new artifact was created. Additionally, the UpdateArtifactAsync method lacked clear usage examples in the samples.

Solution

The Append property in TaskArtifactUpdateEvent now reflects the actual operation performed:

• Append = true when parts were successfully appended to an existing artifact
• Append = false when a new artifact was created (regardless of what the user requested)

Enhanced the EchoAgentWithTasks sample to demonstrate proper UpdateArtifactAsync usage patterns alongside the existing ReturnArtifactAsync usage.

Changes

• Modified UpdateArtifactAsync to always set Append = false when creating new artifacts
• Added comprehensive test coverage for the edge case where append=true is requested but no artifacts exist
• Enhanced EchoAgentWithTasks sample with three usage patterns:
  • ReturnArtifactAsync for simple, complete responses (default behavior)
  • UpdateArtifactAsync with streaming for chunked responses (use stream: <text>)
  • UpdateArtifactAsync with append for building artifacts incrementally (use append: <text>)
• Fixed inconsistent indentation in A2A.csproj

This ensures that event consumers receive accurate information about what actually happened, while providing clear examples of how to use both artifact methods effectively.

Fixes #21.

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[brandonh-msft]

@copilot try again

[Copilot]

@copilot try again

Fixed the Append flag logic in TaskArtifactUpdateEvent. The issue was that when append=true was requested but no existing artifacts existed, the event still showed Append=true even though we actually created a new artifact.

Now the Append property correctly reflects what actually happened:

• Append=true when parts were appended to an existing artifact
• Append=false when a new artifact was created (regardless of user request)

Added a specific test case to cover this scenario. All 190 tests are passing.

Commit: bf9ba4e

[Blackhex]

@copilot This method is not used yet (except in tests). Show me how it is supposed to be used on agent implementations in samples/ folder. This should play nice with the current ReturnArtifactAsync method implementation so observe how it is being used in samples/

[Copilot]

Enhanced the EchoAgentWithTasks sample to demonstrate how UpdateArtifactAsync should be used in agent implementations. The sample now shows three patterns:

1. Simple responses: ReturnArtifactAsync for complete, single artifacts (default behavior)
2. Streaming responses: UpdateArtifactAsync with chunked data (use stream: <text> command)
3. Incremental building: UpdateArtifactAsync with append functionality (use append: <text> command)

The key difference from ReturnArtifactAsync is that UpdateArtifactAsync supports:

• append parameter to add to existing artifacts vs creating new ones
• lastChunk parameter for streaming scenarios
• More granular control over artifact construction

This demonstrates the proper usage patterns alongside the existing ReturnArtifactAsync usage in the samples. Commit: 8a8af29

[hoverwars]

Hello @brandonh-msft. Why are you taking the last artifact from the task ?
Is this a limitation from the A2A protocol, meaning we can only append to the last artifact from the task ?
If this is not a limitation, you could use the artifact object received from the method params and look for a corresponding artifactId ?

[gijswalraven]

Matching by artifactId is the correct approach here, not "last artifact".

The Artifact model already has ArtifactId as a required field. A task can have multiple artifacts concurrently — if an agent is streaming two separate outputs (say, a text artifact and a code artifact), using Last() would incorrectly append parts to whichever artifact was added most recently, regardless of which stream the chunk belongs to.

The fix should be:

var existing = task.Artifacts?.FirstOrDefault(a => a.ArtifactId == artifact.ArtifactId);
if (existing is not null)
{
    existing.Parts.AddRange(artifact.Parts);
}
else
{
    task.Artifacts ??= [];
    task.Artifacts.Add(artifact);
}

This way each artifact chunk is correctly routed to the artifact it belongs to, regardless of insertion order. The Append flag on the emitted event can then accurately reflect true when an existing artifact was found and updated, and false when a new one was created — which aligns with what Copilot already implemented for the event side.

[cdavernas]

Any news on this? It's IMHO an absolute must.

[marcinbelczewski]

Hi all,

I'm interested in using the UpdateArtifactAsync functionality in my project. I noticed this PR has been open for a while and there's an unanswered design question from @hoverwars about whether appending should target artifacts by artifactId rather than always appending to the last artifact.

Is there any planned work to move this forward? If it would be helpful, I'd be happy to contribute - either by addressing the design feedback on this PR or by submitting a fresh implementation that incorporates the suggestions.

Thanks for all the work on a2a-dotnet!

[gijswalraven]

Design decision: match by artifactId, not by last artifact

Picking up on @hoverwars' unanswered review question and @marcinbelczewski's offer to help — here's a concrete answer.

Why artifactId and not Last()

The current implementation appends to task.Artifacts[^1] (the last artifact). This is incorrect for any scenario with more than one concurrent artifact stream. A task can legitimately have multiple artifacts being built up in parallel — for example, a code artifact and an explanation artifact streamed simultaneously. Using Last() would corrupt whichever artifact happened to be inserted most recently, regardless of which stream the incoming chunk belongs to.

The Artifact model already has ArtifactId as a [JsonRequired] field, making it the natural key. The correct lookup is:

var existing = task.Artifacts?.FirstOrDefault(a => a.ArtifactId == artifact.ArtifactId);
if (existing is not null)
{
    existing.Parts.AddRange(artifact.Parts);
}
else
{
    task.Artifacts ??= [];
    task.Artifacts.Add(artifact);
}

The Append flag on the emitted TaskArtifactUpdateEvent can then accurately reflect what actually happened — true when an existing artifact was found and updated, false when a new one was created — which aligns with the rest of Copilot's implementation in this PR.

Recommendation

The rest of this PR (the new UpdateArtifactAsync method, the event flag correction, the sample updates, the tests) looks solid. The only change needed is swapping task.Artifacts[^1] for the FirstOrDefault by artifactId approach above, and updating the guard condition from task.Artifacts.Count > 0 to checking whether the lookup returned a match.

@brandonh-msft — would you prefer to update this PR with that change, or close it and have someone submit a clean PR? @marcinbelczewski offered to contribute — that could be a good path forward if the current PR isn't being actively maintained. Either way, this is a one-function change once the design question is settled.

cc @hoverwars @darrelmiller @cdavernas

[gijswalraven]

Code review

Critical

task.Artifacts[^1] — wrong lookup key (already flagged by @hoverwars)

src/A2A/Server/TaskManager.cs ~line 493

The implementation indexes into the last artifact unconditionally. This is incorrect when a task has multiple artifacts — chunks for artifact A would corrupt artifact B if artifact B happens to be last. The fix is to match by ArtifactId:

var existing = task.Artifacts.FirstOrDefault(a => a.ArtifactId == artifact.ArtifactId);

---

High

Append event sends the full merged artifact, not the chunk

src/A2A/Server/TaskManager.cs — the append branch

When append = true, the event emits lastArtifact (the already-merged artifact with all accumulated parts). A client that correctly handles Append = true — by appending the event's parts to its local copy — will duplicate every previously received part on every subsequent chunk. The event should carry artifact (the incoming chunk), not lastArtifact:

// Wrong: sends accumulated artifact
var taskUpdateEvent = new TaskArtifactUpdateEvent
{
    Artifact = lastArtifact,  // full merged content
    Append = true,
    ...
};

// Correct: sends only the new chunk
var taskUpdateEvent = new TaskArtifactUpdateEvent
{
    Artifact = artifact,      // just the new parts
    Append = true,
    ...
};

The server-side accumulation in task.Artifacts is fine; the issue is only on the event payload.

---

Medium

Code duplication in the two branches

The enumerator lookup and TaskArtifactUpdateEvent construction are copy-pasted between the if (append && ...) and else branches. The only differences are Append = true/false and which artifact goes into the event. Extracting to a single dispatch block at the end would make future changes safer:

bool actuallyAppended = existing is not null;
var eventArtifact = actuallyAppended ? artifact : artifact; // same in both cases post-fix

// single event dispatch
var taskUpdateEvent = new TaskArtifactUpdateEvent
{
    TaskId = task.Id,
    Artifact = eventArtifact,
    Append = actuallyAppended,
    LastChunk = lastChunk
};

XML doc still says "last artifact"

src/A2A/Server/ITaskManager.cs — UpdateArtifactAsync remarks

When append is true, the artifact's parts are added to the last artifact in the task's collection.

This will need updating once the artifactId-based lookup is in place.

---

Low

append parameter is bool, but TaskArtifactUpdateEvent.Append is bool?

Minor inconsistency. The model allows null (meaning "not set"), but the method forces callers to choose true or false. Not a bug, but it's worth aligning — or at least being intentional about it.

Missing test: multiple concurrent artifacts

There's no test covering the key scenario that motivates the artifactId lookup: a task with two different artifacts being streamed in parallel. Once the lookup is fixed, a test like:

// Task has artifact A (initial chunk) and artifact B (initial chunk)
// Append chunk to artifact A — verify artifact B is not modified

would lock in the correct behaviour and prevent regression.

Missing cancellation test for UpdateArtifactAsync

Every other method has a ShouldThrowOperationCanceledException_WhenCancellationTokenIsCanceled test. UpdateArtifactAsync doesn't.

---

Summary

The structure of this PR is good — splitting UpdateArtifactAsync from ReturnArtifactAsync is the right call, and the Append flag correction on the event is correct in principle. Two concrete changes are needed before this can merge:

1. Replace task.Artifacts[^1] with FirstOrDefault(a => a.ArtifactId == artifact.ArtifactId)
2. Send artifact (the chunk) in the event, not lastArtifact (the accumulated artifact)

Everything else above is either documentation or test hygiene.

[darrelmiller]

Oops. I missed this PR. I implemented streaming artifacts in this PR #280 I believe my implementation addresses the two issues that @gijswalraven raised.

[rokonec]

This will be somehow implemented in v1