diff --git a/agentscope-core/src/main/java/io/agentscope/core/tool/ToolSchemaGenerator.java b/agentscope-core/src/main/java/io/agentscope/core/tool/ToolSchemaGenerator.java index 0cd3feafb..f77257880 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/tool/ToolSchemaGenerator.java +++ b/agentscope-core/src/main/java/io/agentscope/core/tool/ToolSchemaGenerator.java @@ -22,6 +22,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; /** @@ -91,15 +92,25 @@ Map generateParameterSchema(Method method, Set excludePa return schema; } - // TODO: putAll silently overwrites when different parameters define the same def key - // (e.g. different classes with the same simple name under PLAIN_DEFINITION_KEYS). - // Add value-equality check and fail-fast on true conflicts in a follow-up. @SuppressWarnings("unchecked") private void hoistDefs( Map paramSchema, String key, Map target) { Object raw = paramSchema.remove(key); if (raw instanceof Map defs && !defs.isEmpty()) { - target.putAll((Map) defs); + Map normalizedDefs = (Map) defs; + for (Map.Entry entry : normalizedDefs.entrySet()) { + String defKey = entry.getKey(); + Object incomingDef = entry.getValue(); + Object existingDef = target.get(defKey); + if (existingDef == null) { + target.put(defKey, incomingDef); + continue; + } + if (!Objects.equals(existingDef, incomingDef)) { + throw new IllegalStateException( + "Conflicting schema definition found for key: " + defKey); + } + } } } diff --git a/agentscope-core/src/test/java/io/agentscope/core/tool/ToolSchemaGeneratorTest.java b/agentscope-core/src/test/java/io/agentscope/core/tool/ToolSchemaGeneratorTest.java new file mode 100644 index 000000000..9a7b7cb9c --- /dev/null +++ b/agentscope-core/src/test/java/io/agentscope/core/tool/ToolSchemaGeneratorTest.java @@ -0,0 +1,86 @@ +/* + * Copyright 2024-2026 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.agentscope.core.tool; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +@Tag("unit") +@DisplayName("ToolSchemaGenerator Tests") +class ToolSchemaGeneratorTest { + + private final ToolSchemaGenerator generator = new ToolSchemaGenerator(); + + @Test + @DisplayName("Should throw when hoisted defs contain conflicting definition for same key") + void testHoistDefsConflict() throws Exception { + Method hoistDefsMethod = + ToolSchemaGenerator.class.getDeclaredMethod( + "hoistDefs", Map.class, String.class, Map.class); + hoistDefsMethod.setAccessible(true); + + Map existingDef = + Map.of("type", "object", "properties", Map.of("value", Map.of("type", "string"))); + Map conflictDef = + Map.of("type", "object", "properties", Map.of("value", Map.of("type", "integer"))); + + Map target = new HashMap<>(); + target.put("Material", existingDef); + Map paramSchema = new HashMap<>(); + paramSchema.put("$defs", Map.of("Material", conflictDef)); + + InvocationTargetException exception = + assertThrows( + InvocationTargetException.class, + () -> hoistDefsMethod.invoke(generator, paramSchema, "$defs", target)); + assertInstanceOf(IllegalStateException.class, exception.getCause()); + assertTrue(exception.getCause().getMessage().contains("Material")); + } + + @Test + @DisplayName("Should allow hoisted defs when same key has equivalent definition") + void testHoistDefsEquivalent() throws Exception { + Method hoistDefsMethod = + ToolSchemaGenerator.class.getDeclaredMethod( + "hoistDefs", Map.class, String.class, Map.class); + hoistDefsMethod.setAccessible(true); + + Map definition = + Map.of("type", "object", "properties", Map.of("value", Map.of("type", "string"))); + + Map target = new HashMap<>(); + target.put("Material", definition); + Map paramSchema = new HashMap<>(); + paramSchema.put("$defs", Map.of("Material", definition)); + + assertDoesNotThrow(() -> hoistDefsMethod.invoke(generator, paramSchema, "$defs", target)); + assertEquals(1, target.size()); + assertEquals(definition, target.get("Material")); + assertFalse(paramSchema.containsKey("$defs")); + } +} diff --git a/agentscope-extensions/agentscope-extensions-a2a/agentscope-extensions-a2a-server/src/main/java/io/agentscope/core/a2a/server/transport/DeploymentProperties.java b/agentscope-extensions/agentscope-extensions-a2a/agentscope-extensions-a2a-server/src/main/java/io/agentscope/core/a2a/server/transport/DeploymentProperties.java index 6251af125..0cc4ec618 100644 --- a/agentscope-extensions/agentscope-extensions-a2a/agentscope-extensions-a2a-server/src/main/java/io/agentscope/core/a2a/server/transport/DeploymentProperties.java +++ b/agentscope-extensions/agentscope-extensions-a2a/agentscope-extensions-a2a-server/src/main/java/io/agentscope/core/a2a/server/transport/DeploymentProperties.java @@ -30,6 +30,7 @@ public record DeploymentProperties(String host, int port) { private static final Logger log = LoggerFactory.getLogger(DeploymentProperties.class); + private static final String DEFAULT_HOST = "localhost"; public static class Builder { @@ -55,7 +56,12 @@ public DeploymentProperties build() { try { host = NetworkUtils.getLocalIpAddress(); log.info("Local IP address: {}", host); - } catch (SocketException ignored) { + } catch (SocketException exception) { + host = DEFAULT_HOST; + log.warn( + "Failed to resolve local IP address, fallback to default host: {}", + DEFAULT_HOST, + exception); } } if (null == port) { diff --git a/agentscope-extensions/agentscope-extensions-a2a/agentscope-extensions-a2a-server/src/test/java/io/agentscope/core/a2a/server/transport/DeploymentPropertiesTest.java b/agentscope-extensions/agentscope-extensions-a2a/agentscope-extensions-a2a-server/src/test/java/io/agentscope/core/a2a/server/transport/DeploymentPropertiesTest.java index a5e86dbce..60e3cadb6 100644 --- a/agentscope-extensions/agentscope-extensions-a2a/agentscope-extensions-a2a-server/src/test/java/io/agentscope/core/a2a/server/transport/DeploymentPropertiesTest.java +++ b/agentscope-extensions/agentscope-extensions-a2a/agentscope-extensions-a2a-server/src/test/java/io/agentscope/core/a2a/server/transport/DeploymentPropertiesTest.java @@ -18,7 +18,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -109,7 +108,7 @@ void testUseLocalhostWhenGetLocalIpFails() throws SocketException { new DeploymentProperties.Builder().port(port).build(); assertNotNull(deploymentProperties); - assertNull(deploymentProperties.host()); // host will be null since we couldn't get it + assertEquals("localhost", deploymentProperties.host()); assertEquals(port, deploymentProperties.port()); }