Skip to content

fix: Add missing extensions to Artifact and Message #409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kabir merged 1 commit into from
Oct 29, 2025
Merged

fix: Add missing extensions to Artifact and Message #409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...core/src/test/java/io/a2a/extras/queuemanager/replicated/core/EventSerializationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public void testTaskStatusUpdateEventSerialization() throws JsonProcessingExcept
public void testTaskArtifactUpdateEventSerialization() throws JsonProcessingException {
// Create a TaskArtifactUpdateEvent
List<Part<?>> parts = List.of(new TextPart("Test artifact content"));
Artifact artifact = new Artifact("test-artifact-123", "Test Artifact", "Test description", parts, null);
Artifact artifact = new Artifact("test-artifact-123", "Test Artifact", "Test description", parts, null, null);
TaskArtifactUpdateEvent originalEvent = new TaskArtifactUpdateEvent.Builder()
.taskId("test-task-xyz")
.contextId("test-context-uvw")
Expand Down
1 change: 1 addition & 0 deletions server-common/src/main/java/io/a2a/server/util/ArtifactUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public static Artifact newArtifact(String name, List<Part<?>> parts, String desc
name,
description,
parts,
null,
null
);
}
Expand Down
6 changes: 4 additions & 2 deletions spec-grpc/src/main/java/io/a2a/grpc/utils/ProtoUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,8 @@ public static Message message(io.a2a.grpc.MessageOrBuilder message) {
message.getContextId().isEmpty() ? null : message.getContextId(),
message.getTaskId().isEmpty() ? null : message.getTaskId(),
null, // referenceTaskIds is not in grpc message
struct(message.getMetadata())
struct(message.getMetadata()),
message.getExtensionsList().isEmpty() ? null : message.getExtensionsList()
);
}

Expand Down Expand Up @@ -906,7 +907,8 @@ private static Artifact artifact(io.a2a.grpc.ArtifactOrBuilder artifact) {
artifact.getName(),
artifact.getDescription(),
artifact.getPartsList().stream().map(item -> part(item)).collect(Collectors.toList()),
struct(artifact.getMetadata())
struct(artifact.getMetadata()),
artifact.getExtensionsList().isEmpty() ? null : artifact.getExtensionsList()
);
}

Expand Down
12 changes: 10 additions & 2 deletions spec/src/main/java/io/a2a/spec/Artifact.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
*/
@JsonInclude(JsonInclude.Include.NON_ABSENT)
@JsonIgnoreProperties(ignoreUnknown = true)
public record Artifact(String artifactId, String name, String description, List<Part<?>> parts, Map<String, Object> metadata) {
public record Artifact(String artifactId, String name, String description, List<Part<?>> parts, Map<String, Object> metadata,
List<String> extensions) {

public Artifact {
Assert.checkNotNullParam("artifactId", artifactId);
Expand All @@ -28,6 +29,7 @@ public static class Builder {
private String description;
private List<Part<?>> parts;
private Map<String, Object> metadata;
private List<String> extensions;

public Builder(){
}
Expand All @@ -38,6 +40,7 @@ public Builder(Artifact existingArtifact) {
description = existingArtifact.description;
parts = existingArtifact.parts;
metadata = existingArtifact.metadata;
extensions = existingArtifact.extensions;
}

public Builder artifactId(String artifactId) {
Expand Down Expand Up @@ -71,8 +74,13 @@ public Builder metadata(Map<String, Object> metadata) {
return this;
}

public Builder extensions(List<String> extensions) {
this.extensions = this.extensions = (extensions == null) ? null : List.copyOf(extensions);
return this;
}
Comment on lines 77 to 80
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To ensure the immutability of the Artifact record, it's a good practice to create a defensive copy of the extensions list. Assigning the list directly allows the Artifact's internal state to be modified from outside if a mutable list is passed to the builder. This would violate the immutability contract of a record.

I noticed that the parts(Part<?>... parts) method correctly uses List.of() to create an immutable list. This new extensions method should follow a similar robust pattern.

For consistency and robustness, you might also consider applying this to other collection-based builder methods like parts(List<Part<?>> parts) and metadata(Map<String, Object> metadata).

Suggested change
public Builder extensions(List<String> extensions) {
this.extensions = extensions;
return this;
}
public Builder extensions(List<String> extensions) {
this.extensions = (extensions == null) ? null : List.copyOf(extensions);
return this;
}

Copy link
Collaborator Author

@fjuma fjuma Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed


public Artifact build() {
return new Artifact(artifactId, name, description, parts, metadata);
return new Artifact(artifactId, name, description, parts, metadata, extensions);
}
}
}
21 changes: 17 additions & 4 deletions spec/src/main/java/io/a2a/spec/Message.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,18 @@ public final class Message implements EventKind, StreamingEventKind {
private final Map<String, Object> metadata;
private final String kind;
private final List<String> referenceTaskIds;
private final List<String> extensions;

public Message(Role role, List<Part<?>> parts, String messageId, String contextId, String taskId,
List<String> referenceTaskIds, Map<String, Object> metadata) {
this(role, parts, messageId, contextId, taskId, referenceTaskIds, metadata, MESSAGE);
List<String> referenceTaskIds, Map<String, Object> metadata, List<String> extensions) {
this(role, parts, messageId, contextId, taskId, referenceTaskIds, metadata, extensions, MESSAGE);
}

@JsonCreator
public Message(@JsonProperty("role") Role role, @JsonProperty("parts") List<Part<?>> parts,
@JsonProperty("messageId") String messageId, @JsonProperty("contextId") String contextId,
@JsonProperty("taskId") String taskId, @JsonProperty("referenceTaskIds") List<String> referenceTaskIds,
@JsonProperty("metadata") Map<String, Object> metadata,
@JsonProperty("metadata") Map<String, Object> metadata, @JsonProperty("extensions") List<String> extensions,
@JsonProperty("kind") String kind) {
Assert.checkNotNullParam("kind", kind);
Assert.checkNotNullParam("parts", parts);
Expand All @@ -63,6 +64,7 @@ public Message(@JsonProperty("role") Role role, @JsonProperty("parts") List<Part
this.taskId = taskId;
this.referenceTaskIds = referenceTaskIds;
this.metadata = metadata;
this.extensions = extensions;
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The Message class appears to be designed as mostly immutable (with final fields), but assigning the extensions list directly can break this if a mutable list is provided. This would allow the internal state of the Message object to be changed after its construction. To ensure immutability for this field, you should create a defensive copy of the list.

For completeness, you might consider applying the same pattern to other collection fields like parts, referenceTaskIds, and metadata to make the class more robustly immutable (acknowledging that contextId and taskId are mutable by design).

Suggested change
this.extensions = extensions;
this.extensions = (extensions == null) ? null : List.copyOf(extensions);

Copy link
Collaborator Author

@fjuma fjuma Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

this.kind = kind;
}

Expand Down Expand Up @@ -102,6 +104,10 @@ public List<String> getReferenceTaskIds() {
return referenceTaskIds;
}

public List<String> getExtensions() {
return extensions;
}

@Override
public String getKind() {
return kind;
Expand Down Expand Up @@ -132,6 +138,7 @@ public static class Builder {
private String taskId;
private List<String> referenceTaskIds;
private Map<String, Object> metadata;
private List<String> extensions;

public Builder() {
}
Expand All @@ -144,6 +151,7 @@ public Builder(Message message) {
taskId = message.taskId;
referenceTaskIds = message.referenceTaskIds;
metadata = message.metadata;
extensions = message.extensions;
}

public Builder role(Role role) {
Expand Down Expand Up @@ -186,9 +194,14 @@ public Builder metadata(Map<String, Object> metadata) {
return this;
}

public Builder extensions(List<String> extensions) {
this.extensions = (extensions == null) ? null : List.copyOf(extensions);
return this;
}

public Message build() {
return new Message(role, parts, messageId == null ? UUID.randomUUID().toString() : messageId,
contextId, taskId, referenceTaskIds, metadata);
contextId, taskId, referenceTaskIds, metadata, extensions);
}
}
}