Skip to content

Commit 10d1bf6

Browse files
paulrutterclaude
andauthored
fix(server): treat blank taskId/contextId as absent in message/send (#1172)
Proto3 scalar fields have no wire-level presence, so a message that never set taskId/contextId and one that explicitly serialized them as "" are indistinguishable once they cross a transport that always serializes non-optional fields. DefaultRequestHandler only checked for null, so a blank taskId was mistaken for a reference to an existing task with id "" instead of a request to start a new task, causing a spurious TaskNotFoundError (or a raw backend error, depending on the TaskStore implementation) for legitimate new-task requests. Normalize blank taskId/contextId to null once, at the top of onMessageSend/onMessageSendStream, mirroring the emptyToNull convention already used on the gRPC mapping path (A2ACommonFieldMapper). Also harden the public authorizeTaskAccess(String, ...) overload, which is called directly by transport handlers with a raw id. Message.Builder's taskId()/contextId() setters are annotated @nullable to match the record's own nullable fields, since normalization needs to pass null through them. This fixes #1171 Claude-Session: https://claude.ai/code/session_01MKiygCWwiHF1q8R5MgWx18 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 82dce70 commit 10d1bf6

3 files changed

Lines changed: 150 additions & 7 deletions

File tree

‎server-common/src/main/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandler.java‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -715,7 +715,8 @@ private Task doCancelTask(CancelTaskParams params, ServerCallContext context) th
715715

716716
@Override
717717
@SuppressWarnings("NullAway")
718-
public EventKind onMessageSend(MessageSendParams params, ServerCallContext context) throws A2AError {
718+
public EventKind onMessageSend(MessageSendParams rawParams, ServerCallContext context) throws A2AError {
719+
MessageSendParams params = normalizeBlankIds(rawParams);
719720
LOGGER.debug("onMessageSend - task: {}; context {}", params.message().taskId(), params.message().contextId());
720721
String msgTaskId = params.message().taskId();
721722
if (msgTaskId != null) {
@@ -927,7 +928,8 @@ public EventKind onMessageSend(MessageSendParams params, ServerCallContext conte
927928
@Override
928929
@SuppressWarnings("NullAway")
929930
public Flow.Publisher<StreamingEventKind> onMessageSendStream(
930-
MessageSendParams params, ServerCallContext context) throws A2AError {
931+
MessageSendParams rawParams, ServerCallContext context) throws A2AError {
932+
MessageSendParams params = normalizeBlankIds(rawParams);
931933
LOGGER.debug("onMessageSendStream START - task: {}; context: {}; runningAgents: {}",
932934
params.message().taskId(), params.message().contextId(), runningAgents.size());
933935
String msgTaskId = params.message().taskId();
@@ -1352,6 +1354,38 @@ private CompletableFuture<Void> cleanupProducer(@Nullable CompletableFuture<Void
13521354
});
13531355
}
13541356

1357+
/**
1358+
* Treats a blank {@code taskId}/{@code contextId} on the message the same as absent.
1359+
* <p>
1360+
* Proto3 scalar fields have no wire-level presence, so a client that never set these
1361+
* fields and a client that explicitly set them to {@code ""} are indistinguishable once
1362+
* the message crosses a transport that always serializes them (see {@code emptyToNull} in
1363+
* {@code A2ACommonFieldMapper} for the equivalent normalization on the gRPC mapping path).
1364+
* Without this, a blank {@code taskId} is mistaken for a reference to an existing task
1365+
* with id {@code ""} instead of a request to start a new task.
1366+
*/
1367+
private static MessageSendParams normalizeBlankIds(MessageSendParams params) {
1368+
Message message = params.message();
1369+
String taskId = message.taskId();
1370+
String contextId = message.contextId();
1371+
boolean blankTaskId = taskId != null && taskId.isEmpty();
1372+
boolean blankContextId = contextId != null && contextId.isEmpty();
1373+
if (!blankTaskId && !blankContextId) {
1374+
return params;
1375+
}
1376+
1377+
Message normalizedMessage = Message.builder(message)
1378+
.taskId(blankTaskId ? null : taskId)
1379+
.contextId(blankContextId ? null : contextId)
1380+
.build();
1381+
return MessageSendParams.builder()
1382+
.message(normalizedMessage)
1383+
.configuration(params.configuration())
1384+
.metadata(params.metadata())
1385+
.tenant(params.tenant())
1386+
.build();
1387+
}
1388+
13551389
@SuppressWarnings("NullAway") // shouldAddPushInfo guarantees pushConfigStore != null
13561390
private MessageSendSetup initMessageSend(MessageSendParams params, ServerCallContext context) throws A2AError {
13571391
Task task = authorizeTaskAccess(params, context);
@@ -1406,7 +1440,7 @@ private AgentExecutor resolveAgentExecutor(@Nullable String tenant) {
14061440

14071441
@Override
14081442
public void authorizeTaskAccess(@Nullable String requestedTaskId, ServerCallContext context, TaskOperation operation) throws A2AError {
1409-
if (requestedTaskId == null) {
1443+
if (requestedTaskId == null || requestedTaskId.isEmpty()) {
14101444
return;
14111445
}
14121446
enforceRead(context, requestedTaskId, operation);

‎server-common/src/test/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandlerTest.java‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,115 @@ void testSendMessageStream_WithNonExistentTaskId_ThrowsTaskNotFoundError() {
845845
"Expected TaskNotFoundError when onMessageSendStream references a non-existent taskId");
846846
}
847847

848+
/**
849+
* A blank (empty-string) taskId/contextId must be treated the same as absent (null).
850+
* <p>
851+
* Proto3 scalar fields have no wire-level presence, so a client that never set
852+
* taskId/contextId and one that serialized them as "" are indistinguishable once the
853+
* message crosses a JsonFormat-based transport (see {@code JSONRPCUtils}'s use of
854+
* {@code alwaysPrintFieldsWithNoPresence()}). Before this fix, a blank taskId was
855+
* mistaken for a reference to an existing task with id "", causing a spurious
856+
* TaskNotFoundError instead of starting a new task.
857+
*/
858+
@Test
859+
void testSendMessage_WithBlankTaskId_TreatedAsNewTask() throws Exception {
860+
CountDownLatch agentCompleted = new CountDownLatch(1);
861+
agentExecutorExecute = (context, emitter) -> {
862+
emitter.complete();
863+
agentCompleted.countDown();
864+
};
865+
866+
Message message = Message.builder()
867+
.messageId("msg-blank-task-id")
868+
.role(Message.Role.ROLE_USER)
869+
.taskId("")
870+
.contextId("")
871+
.parts(new TextPart("hello"))
872+
.build();
873+
874+
MessageSendParams params = MessageSendParams.builder()
875+
.message(message)
876+
.configuration(DEFAULT_CONFIG)
877+
.build();
878+
879+
EventKind result = requestHandler.onMessageSend(params, NULL_CONTEXT);
880+
881+
assertInstanceOf(Task.class, result, "A blank taskId should start a new task, not fail lookup");
882+
Task task = (Task) result;
883+
assertNotNull(task.id());
884+
assertFalse(task.id().isEmpty(), "A newly generated taskId must not be blank");
885+
assertNotNull(task.contextId());
886+
assertFalse(task.contextId().isEmpty(), "A newly generated contextId must not be blank");
887+
888+
assertTrue(agentCompleted.await(5, TimeUnit.SECONDS), "Agent should have been invoked for a new task");
889+
assertNull(taskStore.get(""), "No task should ever be stored under the empty-string id");
890+
}
891+
892+
/**
893+
* Streaming counterpart of {@link #testSendMessage_WithBlankTaskId_TreatedAsNewTask()}.
894+
*/
895+
@Test
896+
void testSendMessageStream_WithBlankTaskId_TreatedAsNewTask() throws Exception {
897+
CountDownLatch agentCompleted = new CountDownLatch(1);
898+
agentExecutorExecute = (context, emitter) -> {
899+
emitter.complete();
900+
agentCompleted.countDown();
901+
};
902+
903+
Message message = Message.builder()
904+
.messageId("msg-stream-blank-task-id")
905+
.role(Message.Role.ROLE_USER)
906+
.taskId("")
907+
.contextId("")
908+
.parts(new TextPart("hello"))
909+
.build();
910+
911+
MessageSendParams params = MessageSendParams.builder()
912+
.message(message)
913+
.configuration(DEFAULT_CONFIG)
914+
.build();
915+
916+
CountDownLatch streamDone = new CountDownLatch(1);
917+
AtomicReference<Throwable> errorRef = new AtomicReference<>();
918+
AtomicReference<String> taskIdRef = new AtomicReference<>();
919+
920+
Flow.Publisher<StreamingEventKind> publisher =
921+
requestHandler.onMessageSendStream(params, contextWithVersion("1.0"));
922+
publisher.subscribe(new Flow.Subscriber<>() {
923+
@Override
924+
public void onSubscribe(Flow.Subscription s) {
925+
s.request(Long.MAX_VALUE);
926+
}
927+
928+
@Override
929+
public void onNext(StreamingEventKind item) {
930+
if (item instanceof Task t) {
931+
taskIdRef.set(t.id());
932+
} else if (item instanceof TaskStatusUpdateEvent e) {
933+
taskIdRef.set(e.taskId());
934+
}
935+
}
936+
937+
@Override
938+
public void onError(Throwable t) {
939+
errorRef.set(t);
940+
streamDone.countDown();
941+
}
942+
943+
@Override
944+
public void onComplete() {
945+
streamDone.countDown();
946+
}
947+
});
948+
949+
assertTrue(streamDone.await(5, TimeUnit.SECONDS), "Stream should complete");
950+
assertNull(errorRef.get(), "A blank taskId should start a new task, not fail lookup: " + errorRef.get());
951+
assertTrue(agentCompleted.await(5, TimeUnit.SECONDS), "Agent should have been invoked for a new task");
952+
assertNotNull(taskIdRef.get());
953+
assertFalse(taskIdRef.get().isEmpty(), "A newly generated taskId must not be blank");
954+
assertNull(taskStore.get(""), "No task should ever be stored under the empty-string id");
955+
}
956+
848957
/**
849958
* Verification for Codex adversarial review finding:
850959
* When a follow-up message includes taskId but omits contextId,

‎spec/src/main/java/org/a2aproject/sdk/spec/Message.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,21 +225,21 @@ public Builder messageId(String messageId) {
225225
/**
226226
* Sets the conversation context identifier.
227227
*
228-
* @param contextId the context identifier (optional)
228+
* @param contextId the context identifier, or {@code null} to clear it (optional)
229229
* @return this builder for method chaining
230230
*/
231-
public Builder contextId(String contextId) {
231+
public Builder contextId(@Nullable String contextId) {
232232
this.contextId = contextId;
233233
return this;
234234
}
235235

236236
/**
237237
* Sets the task identifier this message is associated with.
238238
*
239-
* @param taskId the task identifier (optional)
239+
* @param taskId the task identifier, or {@code null} to clear it (optional)
240240
* @return this builder for method chaining
241241
*/
242-
public Builder taskId(String taskId) {
242+
public Builder taskId(@Nullable String taskId) {
243243
this.taskId = taskId;
244244
return this;
245245
}

0 commit comments

Comments
 (0)