diff --git a/server-common/src/main/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandler.java b/server-common/src/main/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandler.java index bdb540000..7c97cc003 100644 --- a/server-common/src/main/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandler.java +++ b/server-common/src/main/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandler.java @@ -715,7 +715,8 @@ private Task doCancelTask(CancelTaskParams params, ServerCallContext context) th @Override @SuppressWarnings("NullAway") - public EventKind onMessageSend(MessageSendParams params, ServerCallContext context) throws A2AError { + public EventKind onMessageSend(MessageSendParams rawParams, ServerCallContext context) throws A2AError { + MessageSendParams params = normalizeBlankIds(rawParams); LOGGER.debug("onMessageSend - task: {}; context {}", params.message().taskId(), params.message().contextId()); String msgTaskId = params.message().taskId(); if (msgTaskId != null) { @@ -927,7 +928,8 @@ public EventKind onMessageSend(MessageSendParams params, ServerCallContext conte @Override @SuppressWarnings("NullAway") public Flow.Publisher onMessageSendStream( - MessageSendParams params, ServerCallContext context) throws A2AError { + MessageSendParams rawParams, ServerCallContext context) throws A2AError { + MessageSendParams params = normalizeBlankIds(rawParams); LOGGER.debug("onMessageSendStream START - task: {}; context: {}; runningAgents: {}", params.message().taskId(), params.message().contextId(), runningAgents.size()); String msgTaskId = params.message().taskId(); @@ -1352,6 +1354,38 @@ private CompletableFuture cleanupProducer(@Nullable CompletableFuture + * Proto3 scalar fields have no wire-level presence, so a client that never set these + * fields and a client that explicitly set them to {@code ""} are indistinguishable once + * the message crosses a transport that always serializes them (see {@code emptyToNull} in + * {@code A2ACommonFieldMapper} for the equivalent normalization on the gRPC mapping path). + * Without this, a blank {@code taskId} is mistaken for a reference to an existing task + * with id {@code ""} instead of a request to start a new task. + */ + private static MessageSendParams normalizeBlankIds(MessageSendParams params) { + Message message = params.message(); + String taskId = message.taskId(); + String contextId = message.contextId(); + boolean blankTaskId = taskId != null && taskId.isEmpty(); + boolean blankContextId = contextId != null && contextId.isEmpty(); + if (!blankTaskId && !blankContextId) { + return params; + } + + Message normalizedMessage = Message.builder(message) + .taskId(blankTaskId ? null : taskId) + .contextId(blankContextId ? null : contextId) + .build(); + return MessageSendParams.builder() + .message(normalizedMessage) + .configuration(params.configuration()) + .metadata(params.metadata()) + .tenant(params.tenant()) + .build(); + } + @SuppressWarnings("NullAway") // shouldAddPushInfo guarantees pushConfigStore != null private MessageSendSetup initMessageSend(MessageSendParams params, ServerCallContext context) throws A2AError { Task task = authorizeTaskAccess(params, context); @@ -1406,7 +1440,7 @@ private AgentExecutor resolveAgentExecutor(@Nullable String tenant) { @Override public void authorizeTaskAccess(@Nullable String requestedTaskId, ServerCallContext context, TaskOperation operation) throws A2AError { - if (requestedTaskId == null) { + if (requestedTaskId == null || requestedTaskId.isEmpty()) { return; } enforceRead(context, requestedTaskId, operation); diff --git a/server-common/src/test/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandlerTest.java b/server-common/src/test/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandlerTest.java index fa9bf6814..53b25b1a1 100644 --- a/server-common/src/test/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandlerTest.java +++ b/server-common/src/test/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandlerTest.java @@ -845,6 +845,115 @@ void testSendMessageStream_WithNonExistentTaskId_ThrowsTaskNotFoundError() { "Expected TaskNotFoundError when onMessageSendStream references a non-existent taskId"); } + /** + * A blank (empty-string) taskId/contextId must be treated the same as absent (null). + *

+ * Proto3 scalar fields have no wire-level presence, so a client that never set + * taskId/contextId and one that serialized them as "" are indistinguishable once the + * message crosses a JsonFormat-based transport (see {@code JSONRPCUtils}'s use of + * {@code alwaysPrintFieldsWithNoPresence()}). Before this fix, a blank taskId was + * mistaken for a reference to an existing task with id "", causing a spurious + * TaskNotFoundError instead of starting a new task. + */ + @Test + void testSendMessage_WithBlankTaskId_TreatedAsNewTask() throws Exception { + CountDownLatch agentCompleted = new CountDownLatch(1); + agentExecutorExecute = (context, emitter) -> { + emitter.complete(); + agentCompleted.countDown(); + }; + + Message message = Message.builder() + .messageId("msg-blank-task-id") + .role(Message.Role.ROLE_USER) + .taskId("") + .contextId("") + .parts(new TextPart("hello")) + .build(); + + MessageSendParams params = MessageSendParams.builder() + .message(message) + .configuration(DEFAULT_CONFIG) + .build(); + + EventKind result = requestHandler.onMessageSend(params, NULL_CONTEXT); + + assertInstanceOf(Task.class, result, "A blank taskId should start a new task, not fail lookup"); + Task task = (Task) result; + assertNotNull(task.id()); + assertFalse(task.id().isEmpty(), "A newly generated taskId must not be blank"); + assertNotNull(task.contextId()); + assertFalse(task.contextId().isEmpty(), "A newly generated contextId must not be blank"); + + assertTrue(agentCompleted.await(5, TimeUnit.SECONDS), "Agent should have been invoked for a new task"); + assertNull(taskStore.get(""), "No task should ever be stored under the empty-string id"); + } + + /** + * Streaming counterpart of {@link #testSendMessage_WithBlankTaskId_TreatedAsNewTask()}. + */ + @Test + void testSendMessageStream_WithBlankTaskId_TreatedAsNewTask() throws Exception { + CountDownLatch agentCompleted = new CountDownLatch(1); + agentExecutorExecute = (context, emitter) -> { + emitter.complete(); + agentCompleted.countDown(); + }; + + Message message = Message.builder() + .messageId("msg-stream-blank-task-id") + .role(Message.Role.ROLE_USER) + .taskId("") + .contextId("") + .parts(new TextPart("hello")) + .build(); + + MessageSendParams params = MessageSendParams.builder() + .message(message) + .configuration(DEFAULT_CONFIG) + .build(); + + CountDownLatch streamDone = new CountDownLatch(1); + AtomicReference errorRef = new AtomicReference<>(); + AtomicReference taskIdRef = new AtomicReference<>(); + + Flow.Publisher publisher = + requestHandler.onMessageSendStream(params, contextWithVersion("1.0")); + publisher.subscribe(new Flow.Subscriber<>() { + @Override + public void onSubscribe(Flow.Subscription s) { + s.request(Long.MAX_VALUE); + } + + @Override + public void onNext(StreamingEventKind item) { + if (item instanceof Task t) { + taskIdRef.set(t.id()); + } else if (item instanceof TaskStatusUpdateEvent e) { + taskIdRef.set(e.taskId()); + } + } + + @Override + public void onError(Throwable t) { + errorRef.set(t); + streamDone.countDown(); + } + + @Override + public void onComplete() { + streamDone.countDown(); + } + }); + + assertTrue(streamDone.await(5, TimeUnit.SECONDS), "Stream should complete"); + assertNull(errorRef.get(), "A blank taskId should start a new task, not fail lookup: " + errorRef.get()); + assertTrue(agentCompleted.await(5, TimeUnit.SECONDS), "Agent should have been invoked for a new task"); + assertNotNull(taskIdRef.get()); + assertFalse(taskIdRef.get().isEmpty(), "A newly generated taskId must not be blank"); + assertNull(taskStore.get(""), "No task should ever be stored under the empty-string id"); + } + /** * Verification for Codex adversarial review finding: * When a follow-up message includes taskId but omits contextId, diff --git a/spec/src/main/java/org/a2aproject/sdk/spec/Message.java b/spec/src/main/java/org/a2aproject/sdk/spec/Message.java index 06ff5cbef..2ee068d57 100644 --- a/spec/src/main/java/org/a2aproject/sdk/spec/Message.java +++ b/spec/src/main/java/org/a2aproject/sdk/spec/Message.java @@ -225,10 +225,10 @@ public Builder messageId(String messageId) { /** * Sets the conversation context identifier. * - * @param contextId the context identifier (optional) + * @param contextId the context identifier, or {@code null} to clear it (optional) * @return this builder for method chaining */ - public Builder contextId(String contextId) { + public Builder contextId(@Nullable String contextId) { this.contextId = contextId; return this; } @@ -236,10 +236,10 @@ public Builder contextId(String contextId) { /** * Sets the task identifier this message is associated with. * - * @param taskId the task identifier (optional) + * @param taskId the task identifier, or {@code null} to clear it (optional) * @return this builder for method chaining */ - public Builder taskId(String taskId) { + public Builder taskId(@Nullable String taskId) { this.taskId = taskId; return this; }