Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Pull request overview

Copilot reviewed 105 out of 105 changed files in this pull request and generated 2 comments.

shenyu-registry-api/src/main/java/org/apache/shenyu/registry/api/config/RegisterConfig.java:1

• equals is unsafe: it casts without an instanceof/class check and dereferences getRegisterType()/getServerLists() which can be null (default constructor sets them null), causing NullPointerException. Additionally, hashCode() depends on Properties iteration order, which may differ between two Properties instances that are equals() by content—this can violate the equals/hashCode contract. Make equals null-safe (Objects.equals), add type checks (this == obj, obj instanceof RegisterConfig), and compute hashCode in an order-independent way for props (e.g., sort entries by key before mixing into the hash, or hash a normalized map/entry set).
  shenyu-register-client-beat/src/main/java/org/apache/shenyu/register/client/beat/HeartbeatListener.java:1
• Two issues here can break heartbeat reporting in production: (1) Caffeine CacheLoader must not return null—returning null can trigger InvalidCacheLoadException and prevent caching/refresh semantics from working as intended. Represent absence explicitly (e.g., cache Optional<String> or cache a sentinel like empty string) and handle it in sendHeartbeat. (2) Throwing a RuntimeException from the scheduled task can cancel future executions of scheduleAtFixedRate, stopping all heartbeats after one full-cycle failure; instead, log and continue so the next period retries.
  shenyu-register-client-beat/src/main/java/org/apache/shenyu/register/client/beat/HeartbeatListener.java:1
• Two issues here can break heartbeat reporting in production: (1) Caffeine CacheLoader must not return null—returning null can trigger InvalidCacheLoadException and prevent caching/refresh semantics from working as intended. Represent absence explicitly (e.g., cache Optional<String> or cache a sentinel like empty string) and handle it in sendHeartbeat. (2) Throwing a RuntimeException from the scheduled task can cancel future executions of scheduleAtFixedRate, stopping all heartbeats after one full-cycle failure; instead, log and continue so the next period retries.
  shenyu-spring-boot-starter-client/shenyu-spring-boot-starter-client-springmvc/src/test/java/org/apache/shenyu/springboot/starter/client/springmvc/ShenyuSpringMvcClientConfigurationTest.java:1
• There are two @BeforeEach methods both assigning applicationContextRunner. JUnit will run both before each test (order is not something to rely on), so the configuration used by each test becomes fragile/flaky and the first setup is effectively overwritten. Consider using separate ApplicationContextRunner instances inside each test, or switch to @Nested test classes with distinct @BeforeEach setups.
  shenyu-client-mcp/shenyu-client-mcp-common/src/main/java/org/apache/shenyu/client/mcp/generator/McpToolsRegisterDTOGenerator.java:1
• parameters can be null when the OpenAPI operation has no parameters array. parameterFormatting(parameters) will then throw a NullPointerException. Guard by defaulting to an empty JsonArray when parameters is null (and consider similar guards for paths, path, and method lookups if the input can be incomplete).
  shenyu-registry-api/src/main/java/org/apache/shenyu/registry/api/entity/InstanceEntity.java:1
• toString() returns a type label URIRegisterDTO{...} which doesn't match this class (InstanceEntity). This makes logs/debugging misleading. Update the prefix to InstanceEntity{...} (and consider including status/weight/uri if those are meaningful for diagnosis).
  shenyu-registry-api/src/main/java/org/apache/shenyu/registry/api/path/InstancePathConstants.java:1
• DOT_SEPARATOR is declared but never used within this class (the full file is shown in the diff). Consider removing it to reduce dead code and avoid confusion.
  shenyu-spring-boot-starter-client/shenyu-spring-boot-starter-client-tars/src/test/java/org/apache/shenyu/springboot/starter/client/tars/ShenyuTarsClientConfigurationTest.java:1
• The static Mockito mock is manually closed at the end of the test. If an assertion or context initialization throws, close() is skipped and the static mock can leak into other tests. Use try-with-resources (try (MockedStatic<RegisterUtils> mocked = mockStatic(...)) { ... }) to guarantee cleanup.