diff --git a/extras/task-store-database-jpa/src/main/java/io/a2a/extras/taskstore/database/jpa/JpaDatabaseTaskStore.java b/extras/task-store-database-jpa/src/main/java/io/a2a/extras/taskstore/database/jpa/JpaDatabaseTaskStore.java index 49dad1728..a4a1f4f66 100644 --- a/extras/task-store-database-jpa/src/main/java/io/a2a/extras/taskstore/database/jpa/JpaDatabaseTaskStore.java +++ b/extras/task-store-database-jpa/src/main/java/io/a2a/extras/taskstore/database/jpa/JpaDatabaseTaskStore.java @@ -362,7 +362,10 @@ public ListTasksResult list(ListTasksParams params) { private Task transformTask(Task task, int historyLength, boolean includeArtifacts) { // Limit history if needed (keep most recent N messages) List history = task.history(); - if (historyLength > 0 && history != null && history.size() > historyLength) { + if (historyLength == 0) { + // When historyLength is 0, return empty history + history = List.of(); + } else if (historyLength > 0 && history != null && history.size() > historyLength) { history = history.subList(history.size() - historyLength, history.size()); } diff --git a/server-common/src/main/java/io/a2a/server/tasks/InMemoryTaskStore.java b/server-common/src/main/java/io/a2a/server/tasks/InMemoryTaskStore.java index d2d9362ca..d9fb956df 100644 --- a/server-common/src/main/java/io/a2a/server/tasks/InMemoryTaskStore.java +++ b/server-common/src/main/java/io/a2a/server/tasks/InMemoryTaskStore.java @@ -144,7 +144,10 @@ public ListTasksResult list(ListTasksParams params) { private Task transformTask(Task task, int historyLength, boolean includeArtifacts) { // Limit history if needed (keep most recent N messages) List history = task.history(); - if (historyLength > 0 && history != null && history.size() > historyLength) { + if (historyLength == 0) { + // When historyLength is 0, return empty history + history = List.of(); + } else if (historyLength > 0 && history != null && history.size() > historyLength) { history = history.subList(history.size() - historyLength, history.size()); } diff --git a/spec-grpc/src/main/java/io/a2a/grpc/mapper/A2ACommonFieldMapper.java b/spec-grpc/src/main/java/io/a2a/grpc/mapper/A2ACommonFieldMapper.java index c3f031e97..507ccc552 100644 --- a/spec-grpc/src/main/java/io/a2a/grpc/mapper/A2ACommonFieldMapper.java +++ b/spec-grpc/src/main/java/io/a2a/grpc/mapper/A2ACommonFieldMapper.java @@ -285,7 +285,7 @@ default Map metadataFromProto(Struct struct) { */ @Named("zeroToNull") default Integer zeroToNull(int value) { - return value > 0 ? value : null; + return value != 0 ? value : null; } /** @@ -337,13 +337,20 @@ default long instantToMillis(Instant instant) { * Converts protobuf milliseconds-since-epoch (int64) to domain Instant. *

* Returns null if input is 0 (protobuf default for unset field). + * Throws InvalidParamsError for negative values (invalid timestamps). * Use this with {@code @Mapping(qualifiedByName = "millisToInstant")}. * * @param millis milliseconds since epoch * @return domain Instant, or null if millis is 0 + * @throws InvalidParamsError if millis is negative */ @Named("millisToInstant") default Instant millisToInstant(long millis) { + if (millis < 0L) { + throw new InvalidParamsError(null, + "lastUpdatedAfter must be a valid timestamp (non-negative milliseconds since epoch), got: " + millis, + null); + } return millis > 0L ? Instant.ofEpochMilli(millis) : null; } @@ -351,21 +358,28 @@ default Instant millisToInstant(long millis) { // Enum Conversions (handling UNSPECIFIED/UNKNOWN) // ======================================================================== /** - * Converts protobuf TaskState to domain TaskState, treating UNSPECIFIED/UNKNOWN as null. + * Converts protobuf TaskState to domain TaskState, treating UNSPECIFIED as null. *

- * Protobuf enums default to UNSPECIFIED (0 value) when unset. The domain may also have - * UNKNOWN for unparseable values. Both should map to null for optional fields. + * Protobuf enums default to UNSPECIFIED (0 value) when unset, which maps to null for optional fields. + * However, UNRECOGNIZED (invalid enum values from JSON) throws InvalidParamsError for proper validation. * Use this with {@code @Mapping(qualifiedByName = "taskStateOrNull")}. * * @param state the protobuf TaskState - * @return domain TaskState or null if UNSPECIFIED/UNKNOWN + * @return domain TaskState or null if UNSPECIFIED + * @throws InvalidParamsError if state is UNRECOGNIZED (invalid enum value) */ @Named("taskStateOrNull") default io.a2a.spec.TaskState taskStateOrNull(io.a2a.grpc.TaskState state) { if (state == null || state == io.a2a.grpc.TaskState.TASK_STATE_UNSPECIFIED) { return null; } + // Reject invalid enum values (e.g., "INVALID_STATUS" from JSON) + if (state == io.a2a.grpc.TaskState.UNRECOGNIZED) { + throw new InvalidParamsError(null, + "Invalid task state value. Must be one of: SUBMITTED, WORKING, INPUT_REQUIRED, AUTH_REQUIRED, COMPLETED, CANCELED, FAILED, REJECTED", + null); + } io.a2a.spec.TaskState result = TaskStateMapper.INSTANCE.fromProto(state); - return result == io.a2a.spec.TaskState.UNKNOWN ? null : result; + return result; } } diff --git a/spec-grpc/src/main/java/io/a2a/grpc/utils/JSONRPCUtils.java b/spec-grpc/src/main/java/io/a2a/grpc/utils/JSONRPCUtils.java index ad46a8f38..590c8b53d 100644 --- a/spec-grpc/src/main/java/io/a2a/grpc/utils/JSONRPCUtils.java +++ b/spec-grpc/src/main/java/io/a2a/grpc/utils/JSONRPCUtils.java @@ -467,6 +467,10 @@ private static JsonProcessingException convertProtoBufExceptionToJsonProcessingE if (message.contains(prefix)) { return new InvalidParamsJsonMappingException(ERROR_MESSAGE.formatted(message.substring(message.indexOf(prefix) + prefix.length())), id); } + prefix = "Invalid enum value:"; + if (message.contains(prefix)) { + return new InvalidParamsJsonMappingException(ERROR_MESSAGE.formatted(message.substring(message.indexOf(prefix) + prefix.length())), id); + } // Try to extract specific error details using regex patterns Matcher matcher = EXTRACT_WRONG_TYPE.matcher(message); @@ -548,7 +552,7 @@ public static String toJsonRPCRequest(@Nullable String requestId, String method, output.name("method").value(method); } if (payload != null) { - String resultValue = JsonFormat.printer().omittingInsignificantWhitespace().print(payload); + String resultValue = JsonFormat.printer().includingDefaultValueFields().omittingInsignificantWhitespace().print(payload); output.name("params").jsonValue(resultValue); } output.endObject(); @@ -571,7 +575,7 @@ public static String toJsonRPCResultResponse(Object requestId, com.google.protob output.name("id").value(number.longValue()); } } - String resultValue = JsonFormat.printer().omittingInsignificantWhitespace().print(builder); + String resultValue = JsonFormat.printer().includingDefaultValueFields().omittingInsignificantWhitespace().print(builder); output.name("result").jsonValue(resultValue); output.endObject(); return result.toString(); diff --git a/spec/src/main/java/io/a2a/spec/ListTasksParams.java b/spec/src/main/java/io/a2a/spec/ListTasksParams.java index 6813dcf45..81fc0eb08 100644 --- a/spec/src/main/java/io/a2a/spec/ListTasksParams.java +++ b/spec/src/main/java/io/a2a/spec/ListTasksParams.java @@ -29,7 +29,7 @@ public record ListTasksParams( ) { /** * Compact constructor for validation. - * Validates that the tenant parameter is not null. + * Validates that the tenant parameter is not null and parameters are within valid ranges. * * @param contextId filter by context ID * @param status filter by task status @@ -39,9 +39,22 @@ public record ListTasksParams( * @param lastUpdatedAfter filter by last update timestamp * @param includeArtifacts whether to include artifacts * @param tenant the tenant identifier + * @throws InvalidParamsError if pageSize or historyLength are out of valid range */ public ListTasksParams { Assert.checkNotNullParam("tenant", tenant); + + // Validate pageSize (1-100) + if (pageSize != null && (pageSize < 1 || pageSize > 100)) { + throw new InvalidParamsError(null, + "pageSize must be between 1 and 100, got: " + pageSize, null); + } + + // Validate historyLength (>= 0) + if (historyLength != null && historyLength < 0) { + throw new InvalidParamsError(null, + "historyLength must be non-negative, got: " + historyLength, null); + } } /** * Default constructor for listing all tasks. diff --git a/transport/grpc/src/test/java/io/a2a/transport/grpc/handler/GrpcHandlerTest.java b/transport/grpc/src/test/java/io/a2a/transport/grpc/handler/GrpcHandlerTest.java index 77a2c955f..a3ce7ca2a 100644 --- a/transport/grpc/src/test/java/io/a2a/transport/grpc/handler/GrpcHandlerTest.java +++ b/transport/grpc/src/test/java/io/a2a/transport/grpc/handler/GrpcHandlerTest.java @@ -21,6 +21,8 @@ import io.a2a.grpc.GetTaskRequest; import io.a2a.grpc.ListTaskPushNotificationConfigRequest; import io.a2a.grpc.ListTaskPushNotificationConfigResponse; +import io.a2a.grpc.ListTasksRequest; +import io.a2a.grpc.ListTasksResponse; import io.a2a.grpc.Message; import io.a2a.grpc.Part; import io.a2a.grpc.PushNotificationConfig; @@ -1209,6 +1211,53 @@ private void assertGrpcError(StreamRecorder streamRecorder, Status.Code e Assertions.assertTrue(streamRecorder.getValues().isEmpty()); } + @Test + public void testListTasksNegativeTimestampReturnsInvalidArgument() { + TestGrpcHandler handler = new TestGrpcHandler(CARD, requestHandler, internalExecutor); + StreamRecorder responseObserver = StreamRecorder.create(); + + // Negative timestamp should trigger validation error + ListTasksRequest request = ListTasksRequest.newBuilder() + .setLastUpdatedAfter(-1L) + .setTenant("") + .build(); + + handler.listTasks(request, responseObserver); + + // Should return error with INVALID_ARGUMENT status + Assertions.assertTrue(responseObserver.getError() != null); + StatusRuntimeException error = (StatusRuntimeException) responseObserver.getError(); + assertEquals(Status.INVALID_ARGUMENT.getCode(), error.getStatus().getCode()); + } + + @Test + public void testListTasksEmptyResultIncludesAllFields() throws Exception { + TestGrpcHandler handler = new TestGrpcHandler(CARD, requestHandler, internalExecutor); + StreamRecorder responseObserver = StreamRecorder.create(); + + // Query for a context that doesn't exist - should return empty result + ListTasksRequest request = ListTasksRequest.newBuilder() + .setContextId("nonexistent-context-id") + .setTenant("") + .build(); + + handler.listTasks(request, responseObserver); + + // Should succeed with empty result + Assertions.assertNull(responseObserver.getError()); + List responses = responseObserver.getValues(); + assertEquals(1, responses.size()); + + ListTasksResponse response = responses.get(0); + // Verify all fields are present (even if empty/default) + Assertions.assertNotNull(response.getTasksList(), "tasks field should not be null"); + assertEquals(0, response.getTasksList().size(), "tasks should be empty list"); + assertEquals(0, response.getTotalSize(), "totalSize should be 0"); + assertEquals(0, response.getPageSize(), "pageSize should be 0"); + // nextPageToken will be empty string for empty results (protobuf default) + Assertions.assertNotNull(response.getNextPageToken()); + } + private static class TestGrpcHandler extends GrpcHandler { private final AgentCard card; private final RequestHandler handler; diff --git a/transport/jsonrpc/src/test/java/io/a2a/transport/jsonrpc/handler/JSONRPCHandlerTest.java b/transport/jsonrpc/src/test/java/io/a2a/transport/jsonrpc/handler/JSONRPCHandlerTest.java index a55233adb..3436a34b1 100644 --- a/transport/jsonrpc/src/test/java/io/a2a/transport/jsonrpc/handler/JSONRPCHandlerTest.java +++ b/transport/jsonrpc/src/test/java/io/a2a/transport/jsonrpc/handler/JSONRPCHandlerTest.java @@ -30,12 +30,15 @@ import io.a2a.jsonrpc.common.wrappers.GetTaskResponse; import io.a2a.jsonrpc.common.wrappers.ListTaskPushNotificationConfigRequest; import io.a2a.jsonrpc.common.wrappers.ListTaskPushNotificationConfigResponse; +import io.a2a.jsonrpc.common.wrappers.ListTasksResult; import io.a2a.jsonrpc.common.wrappers.SendMessageRequest; import io.a2a.jsonrpc.common.wrappers.SendMessageResponse; import io.a2a.jsonrpc.common.wrappers.SendStreamingMessageRequest; import io.a2a.jsonrpc.common.wrappers.SendStreamingMessageResponse; import io.a2a.jsonrpc.common.wrappers.SetTaskPushNotificationConfigRequest; import io.a2a.jsonrpc.common.wrappers.SetTaskPushNotificationConfigResponse; +import io.a2a.jsonrpc.common.wrappers.ListTasksRequest; +import io.a2a.jsonrpc.common.wrappers.ListTasksResponse; import io.a2a.jsonrpc.common.wrappers.SubscribeToTaskRequest; import io.a2a.server.ServerCallContext; import io.a2a.server.auth.UnauthenticatedUser; @@ -56,7 +59,9 @@ import io.a2a.spec.Event; import io.a2a.spec.GetTaskPushNotificationConfigParams; import io.a2a.spec.InternalError; +import io.a2a.spec.InvalidParamsError; import io.a2a.spec.InvalidRequestError; +import io.a2a.spec.ListTasksParams; import io.a2a.spec.ListTaskPushNotificationConfigParams; import io.a2a.spec.Message; import io.a2a.spec.MessageSendParams; @@ -1959,4 +1964,27 @@ public void testNoVersionDefaultsToCurrentVersionSuccess() { assertNull(response.getError()); Assertions.assertSame(message, response.getResult()); } + + @Test + public void testListTasksEmptyResultIncludesAllFields() { + JSONRPCHandler handler = new JSONRPCHandler(CARD, requestHandler, internalExecutor); + + // Query for a context that doesn't exist - should return empty result + ListTasksParams params = ListTasksParams.builder() + .contextId("nonexistent-context-id") + .tenant("") + .build(); + ListTasksRequest request = new ListTasksRequest("1", params); + ListTasksResponse response = handler.onListTasks(request, callContext); + + // Should return success with all fields present (not null) + assertNull(response.getError()); + ListTasksResult result = response.getResult(); + Assertions.assertNotNull(result); + Assertions.assertNotNull(result.tasks(), "tasks field should not be null"); + assertEquals(0, result.tasks().size(), "tasks should be empty list"); + assertEquals(0, result.totalSize(), "totalSize should be 0"); + assertEquals(0, result.pageSize(), "pageSize should be 0"); + // nextPageToken can be null for empty results + } } diff --git a/transport/rest/src/main/java/io/a2a/transport/rest/handler/RestHandler.java b/transport/rest/src/main/java/io/a2a/transport/rest/handler/RestHandler.java index 87e57f415..d0a229cbc 100644 --- a/transport/rest/src/main/java/io/a2a/transport/rest/handler/RestHandler.java +++ b/transport/rest/src/main/java/io/a2a/transport/rest/handler/RestHandler.java @@ -219,21 +219,41 @@ public HTTPRestResponse listTasks(@Nullable String contextId, @Nullable String s paramsBuilder.contextId(contextId); } if (status != null) { + TaskState taskState = null; + + // Try JSON format first (e.g., "working", "completed") try { - paramsBuilder.status(TaskState.fromString(status)); + taskState = TaskState.fromString(status); } catch (IllegalArgumentException e) { - try { - paramsBuilder.status(TaskState.valueOf(status)); - } catch (IllegalArgumentException valueOfError) { - String validStates = Arrays.stream(TaskState.values()) - .map(TaskState::asString) - .collect(Collectors.joining(", ")); - Map errorData = new HashMap<>(); - errorData.put("parameter", "status"); - errorData.put("reason", "Must be one of: " + validStates); - throw new InvalidParamsError(null, "Invalid params", errorData); + // Try protobuf enum format (e.g., "TASK_STATE_WORKING") + if (status.startsWith("TASK_STATE_")) { + String enumName = status.substring("TASK_STATE_".length()); + try { + taskState = TaskState.valueOf(enumName); + } catch (IllegalArgumentException protoError) { + // Fall through to error handling below + } + } else { + // Try enum constant name directly (e.g., "WORKING") + try { + taskState = TaskState.valueOf(status); + } catch (IllegalArgumentException valueOfError) { + // Fall through to error handling below + } } } + + if (taskState == null) { + String validStates = Arrays.stream(TaskState.values()) + .map(TaskState::asString) + .collect(Collectors.joining(", ")); + Map errorData = new HashMap<>(); + errorData.put("parameter", "status"); + errorData.put("reason", "Must be one of: " + validStates); + throw new InvalidParamsError(null, "Invalid params", errorData); + } + + paramsBuilder.status(taskState); } if (pageSize != null) { paramsBuilder.pageSize(pageSize); @@ -247,12 +267,25 @@ public HTTPRestResponse listTasks(@Nullable String contextId, @Nullable String s paramsBuilder.tenant(tenant); if (lastUpdatedAfter != null) { try { - paramsBuilder.lastUpdatedAfter(Instant.parse(lastUpdatedAfter)); - } catch (DateTimeParseException e) { - Map errorData = new HashMap<>(); - errorData.put("parameter", "lastUpdatedAfter"); - errorData.put("reason", "Must be valid ISO-8601 timestamp"); - throw new InvalidParamsError(null, "Invalid params", errorData); + // Try parsing as Unix milliseconds first (integer) + long millis = Long.parseLong(lastUpdatedAfter); + if (millis < 0L) { + Map errorData = new HashMap<>(); + errorData.put("parameter", "lastUpdatedAfter"); + errorData.put("reason", "Must be a non-negative timestamp value, got: " + millis); + throw new InvalidParamsError(null, "Invalid params", errorData); + } + paramsBuilder.lastUpdatedAfter(Instant.ofEpochMilli(millis)); + } catch (NumberFormatException nfe) { + // Fall back to ISO-8601 format + try { + paramsBuilder.lastUpdatedAfter(Instant.parse(lastUpdatedAfter)); + } catch (DateTimeParseException e) { + Map errorData = new HashMap<>(); + errorData.put("parameter", "lastUpdatedAfter"); + errorData.put("reason", "Must be valid Unix milliseconds or ISO-8601 timestamp"); + throw new InvalidParamsError(null, "Invalid params", errorData); + } } } if (includeArtifacts != null) { @@ -338,7 +371,8 @@ private void validate(String json) { private HTTPRestResponse createSuccessResponse(int statusCode, com.google.protobuf.Message.Builder builder) { try { - String jsonBody = JsonFormat.printer().print(builder); + // Include default value fields to ensure empty arrays, zeros, etc. are present in JSON + String jsonBody = JsonFormat.printer().includingDefaultValueFields().print(builder); return new HTTPRestResponse(statusCode, "application/json", jsonBody); } catch (InvalidProtocolBufferException e) { return createErrorResponse(new InternalError("Failed to serialize response: " + e.getMessage())); diff --git a/transport/rest/src/test/java/io/a2a/transport/rest/handler/RestHandlerTest.java b/transport/rest/src/test/java/io/a2a/transport/rest/handler/RestHandlerTest.java index 1ed1492e1..a2e6ef6e9 100644 --- a/transport/rest/src/test/java/io/a2a/transport/rest/handler/RestHandlerTest.java +++ b/transport/rest/src/test/java/io/a2a/transport/rest/handler/RestHandlerTest.java @@ -879,4 +879,82 @@ public void testNoVersionDefaultsToCurrentVersionSuccess() { Assertions.assertEquals("application/json", response.getContentType()); Assertions.assertNotNull(response.getBody()); } + + @Test + public void testListTasksNegativeTimestampReturns422() { + RestHandler handler = new RestHandler(CARD, requestHandler, internalExecutor); + + // Negative timestamp should return 422 (Invalid params) + RestHandler.HTTPRestResponse response = handler.listTasks(null, null, null, null, + null, "-1", null, "", callContext); + + Assertions.assertEquals(422, response.getStatusCode()); + Assertions.assertEquals("application/json", response.getContentType()); + Assertions.assertTrue(response.getBody().contains("InvalidParamsError")); + } + + @Test + public void testListTasksUnixMillisecondsTimestamp() { + RestHandler handler = new RestHandler(CARD, requestHandler, internalExecutor); + taskStore.save(MINIMAL_TASK); + + // Unix milliseconds timestamp should be accepted + String timestampMillis = String.valueOf(System.currentTimeMillis() - 10000); + RestHandler.HTTPRestResponse response = handler.listTasks(null, null, null, null, + null, timestampMillis, null, "", callContext); + + Assertions.assertEquals(200, response.getStatusCode()); + Assertions.assertEquals("application/json", response.getContentType()); + Assertions.assertTrue(response.getBody().contains("tasks")); + } + + @Test + public void testListTasksProtobufEnumStatus() { + RestHandler handler = new RestHandler(CARD, requestHandler, internalExecutor); + taskStore.save(MINIMAL_TASK); + + // Protobuf enum format (TASK_STATE_SUBMITTED) should be accepted + RestHandler.HTTPRestResponse response = handler.listTasks(null, "TASK_STATE_SUBMITTED", null, null, + null, null, null, "", callContext); + + Assertions.assertEquals(200, response.getStatusCode()); + Assertions.assertEquals("application/json", response.getContentType()); + Assertions.assertTrue(response.getBody().contains(MINIMAL_TASK.id())); + } + + @Test + public void testListTasksEnumConstantStatus() { + RestHandler handler = new RestHandler(CARD, requestHandler, internalExecutor); + taskStore.save(MINIMAL_TASK); + + // Enum constant format (SUBMITTED) should be accepted + RestHandler.HTTPRestResponse response = handler.listTasks(null, "SUBMITTED", null, null, + null, null, null, "", callContext); + + Assertions.assertEquals(200, response.getStatusCode()); + Assertions.assertEquals("application/json", response.getContentType()); + Assertions.assertTrue(response.getBody().contains(MINIMAL_TASK.id())); + } + + @Test + public void testListTasksEmptyResultIncludesAllFields() { + RestHandler handler = new RestHandler(CARD, requestHandler, internalExecutor); + + // Query for a context that doesn't exist - should return empty result with all fields + RestHandler.HTTPRestResponse response = handler.listTasks("nonexistent-context-id", null, null, null, + null, null, null, "", callContext); + + Assertions.assertEquals(200, response.getStatusCode()); + Assertions.assertEquals("application/json", response.getContentType()); + + String body = response.getBody(); + // Verify all required fields are present (not missing) + Assertions.assertTrue(body.contains("\"tasks\""), "Response should contain tasks field"); + Assertions.assertTrue(body.contains("\"totalSize\""), "Response should contain totalSize field"); + Assertions.assertTrue(body.contains("\"pageSize\""), "Response should contain pageSize field"); + Assertions.assertTrue(body.contains("\"nextPageToken\""), "Response should contain nextPageToken field"); + // Verify empty array, not null + Assertions.assertTrue(body.contains("\"tasks\":[]") || body.contains("\"tasks\": []"), + "tasks should be empty array"); + } }