Skip to content

fix: Don't autogenerate message id (0.3 TCK has a test for a null one) #899

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
ehsavoie merged 1 commit into from
May 29, 2026
+9 −11
Merged

fix: Don't autogenerate message id (0.3 TCK has a test for a null one) #899

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
9 changes: 3 additions & 6 deletions ...t-0.3/spec-grpc/src/main/java/org/a2aproject/sdk/compat03/grpc/utils/ProtoUtils_v0_3.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.a2aproject.sdk.compat03.spec.GetTaskPushNotificationConfigParams_v0_3;
import org.a2aproject.sdk.compat03.spec.HTTPAuthSecurityScheme_v0_3;
import org.a2aproject.sdk.compat03.spec.ImplicitOAuthFlow_v0_3;
import org.a2aproject.sdk.compat03.spec.InvalidParamsError_v0_3;
import org.a2aproject.sdk.compat03.spec.InvalidRequestError_v0_3;
import org.a2aproject.sdk.compat03.spec.ListTaskPushNotificationConfigParams_v0_3;
import org.a2aproject.sdk.compat03.spec.Message_v0_3;
Expand Down Expand Up @@ -151,7 +150,9 @@ public static org.a2aproject.sdk.compat03.grpc.Task task(Task_v0_3 task) {

public static org.a2aproject.sdk.compat03.grpc.Message message(Message_v0_3 message) {
org.a2aproject.sdk.compat03.grpc.Message.Builder builder = org.a2aproject.sdk.compat03.grpc.Message.newBuilder();
builder.setMessageId(message.messageId());
if (message.messageId() != null) {
builder.setMessageId(message.messageId());
}
Comment thread
kabir marked this conversation as resolved.
if (message.contextId() != null) {
builder.setContextId(message.contextId());
}
Expand Down Expand Up @@ -863,10 +864,6 @@ public static Task_v0_3 task(org.a2aproject.sdk.compat03.grpc.TaskOrBuilder task
}

public static Message_v0_3 message(org.a2aproject.sdk.compat03.grpc.MessageOrBuilder message) {
if (message.getMessageId().isEmpty()) {
throw new InvalidParamsError_v0_3();
}

return new Message_v0_3(
role(message.getRole()),
message.getContentList().stream().map(item -> part(item)).collect(Collectors.toList()),
Expand Down
9 changes: 4 additions & 5 deletions compat-0.3/spec/src/main/java/org/a2aproject/sdk/compat03/spec/Message_v0_3.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.a2aproject.sdk.compat03.spec;


import java.util.List;
import java.util.Map;
import java.util.UUID;
Expand All @@ -14,7 +13,7 @@
public record Message_v0_3(
Role role,
List<Part_v0_3<?>> parts,
String messageId,
@Nullable String messageId,
@Nullable String contextId,
@Nullable String taskId,
@Nullable List<String> referenceTaskIds,
Expand All @@ -25,7 +24,7 @@ public record Message_v0_3(

public static final String KIND = "message";

public Message_v0_3(Role role, List<Part_v0_3<?>> parts, String messageId, @Nullable String contextId,
public Message_v0_3(Role role, List<Part_v0_3<?>> parts, @Nullable String messageId, @Nullable String contextId,
@Nullable String taskId, @Nullable List<String> referenceTaskIds,
@Nullable Map<String, Object> metadata, @Nullable List<String> extensions, String kind) {
Assert.checkNotNullParam("role", role);
Expand All @@ -35,7 +34,7 @@ public Message_v0_3(Role role, List<Part_v0_3<?>> parts, String messageId, @Null
throw new IllegalArgumentException("Parts cannot be empty");
}
this.parts = List.copyOf(parts);
this.messageId = messageId == null ? UUID.randomUUID().toString() : messageId;
this.messageId = messageId;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 28, 2026

Choose a reason for hiding this comment

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

medium

Although the record constructor has been updated to allow and preserve a null messageId (instead of autogenerating a UUID), the Message_v0_3.Builder.build() method (around line 146) still autogenerates a UUID if messageId is null:

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

This makes the builder inconsistent with the record constructor and prevents clients from using the builder to construct a Message_v0_3 with a null messageId. Consider updating the builder to also allow null messageId without autogenerating a UUID.

Copy link
Copy Markdown
Collaborator Author

@kabir kabir May 28, 2026

Choose a reason for hiding this comment

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

@gemini-code-assist The record constructor is for internal serialization where a null ID is needed. This is needed since the TCK has a test for null message IDs and we need to be able to to pass this test.

The Builder is more client/user API, and the thinking is to steer them towards having to provide a messageId

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 28, 2026

Choose a reason for hiding this comment

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

Thank you for the clarification, kabir. That makes sense given the different requirements for internal serialization versus the public-facing builder API. I understand the design choice to enforce explicit messageId provision for clients.

this.kind = kind != null ? kind : KIND;
if (!this.kind.equals(KIND)) {
throw new IllegalArgumentException("Invalid Message");
Expand All @@ -47,7 +46,7 @@ public Message_v0_3(Role role, List<Part_v0_3<?>> parts, String messageId, @Null
this.extensions = extensions != null ? List.copyOf(extensions) : null;
}
Comment thread
kabir marked this conversation as resolved.

public Message_v0_3(Role role, List<Part_v0_3<?>> parts, String messageId, @Nullable String contextId,
public Message_v0_3(Role role, List<Part_v0_3<?>> parts, @Nullable String messageId, @Nullable String contextId,
@Nullable String taskId, @Nullable List<String> referenceTaskIds,
@Nullable Map<String, Object> metadata, @Nullable List<String> extensions) {
this(role, parts, messageId, contextId, taskId, referenceTaskIds, metadata, extensions, KIND);
Expand Down
2 changes: 2 additions & 0 deletions ...-0.3/spec/src/test/java/org/a2aproject/sdk/compat03/spec/TaskSerialization_v0_3_Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,7 @@ void testDeserializeTaskWithHistoryFromJson() throws JsonProcessingException_v0_
},
"history": [
{
"messageId": "msg-001",
"role": "user",
"parts": [
{
Expand All @@ -608,6 +609,7 @@ void testDeserializeTaskWithHistoryFromJson() throws JsonProcessingException_v0_
]
},
{
"messageId": "msg-002",
"role": "agent",
"parts": [
{
Expand Down