diff --git a/agentscope-core/src/main/java/io/agentscope/core/agent/accumulator/ToolCallsAccumulator.java b/agentscope-core/src/main/java/io/agentscope/core/agent/accumulator/ToolCallsAccumulator.java index 289e02382..75aa2cc90 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/agent/accumulator/ToolCallsAccumulator.java +++ b/agentscope-core/src/main/java/io/agentscope/core/agent/accumulator/ToolCallsAccumulator.java @@ -101,11 +101,23 @@ ToolUseBlock build() { } } + // Always validate rawContent is a legal JSON object before using it + // as content. This prevents persisting malformed JSON fragments + // (e.g. when streaming was interrupted mid-arguments). + String contentStr; + if (rawContentStr.isEmpty()) { + contentStr = "{}"; + } else if (JsonUtils.isValidJsonObject(rawContentStr)) { + contentStr = rawContentStr; + } else { + contentStr = "{}"; + } + return ToolUseBlock.builder() .id(toolId != null ? toolId : generateId()) .name(name) .input(finalArgs) - .content(rawContentStr.isEmpty() ? "{}" : rawContentStr) + .content(contentStr) .metadata(metadata.isEmpty() ? null : metadata) .build(); } diff --git a/agentscope-core/src/main/java/io/agentscope/core/formatter/dashscope/DashScopeToolsHelper.java b/agentscope-core/src/main/java/io/agentscope/core/formatter/dashscope/DashScopeToolsHelper.java index 7a5a9eb57..a39fe29df 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/formatter/dashscope/DashScopeToolsHelper.java +++ b/agentscope-core/src/main/java/io/agentscope/core/formatter/dashscope/DashScopeToolsHelper.java @@ -236,19 +236,7 @@ public List convertToolCalls(List toolBlocks) { continue; } - // Prioritize using content field (raw arguments string), fallback to input map - // serialization - String argsJson; - if (toolUse.getContent() != null && !toolUse.getContent().isEmpty()) { - argsJson = toolUse.getContent(); - } else { - try { - argsJson = JsonUtils.getJsonCodec().toJson(toolUse.getInput()); - } catch (Exception e) { - log.warn("Failed to serialize tool call arguments: {}", e.getMessage()); - argsJson = "{}"; - } - } + String argsJson = JsonUtils.resolveToolCallArgsJson(toolUse); DashScopeFunction function = DashScopeFunction.of(toolUse.getName(), argsJson); DashScopeToolCall toolCall = diff --git a/agentscope-core/src/main/java/io/agentscope/core/formatter/openai/OpenAIMessageConverter.java b/agentscope-core/src/main/java/io/agentscope/core/formatter/openai/OpenAIMessageConverter.java index 01a4c4f3d..44d7b6d16 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/formatter/openai/OpenAIMessageConverter.java +++ b/agentscope-core/src/main/java/io/agentscope/core/formatter/openai/OpenAIMessageConverter.java @@ -330,23 +330,7 @@ private OpenAIMessage convertAssistantMessage(Msg msg) { continue; } - // Prioritize using content field (raw arguments string), fallback to input map - // serialization - String argsJson; - if (toolUse.getContent() != null && !toolUse.getContent().isEmpty()) { - argsJson = toolUse.getContent(); - } else { - try { - argsJson = JsonUtils.getJsonCodec().toJson(toolUse.getInput()); - } catch (Exception e) { - String errorMsg = - e.getMessage() != null - ? e.getMessage() - : e.getClass().getSimpleName(); - log.warn("Failed to serialize tool call arguments: {}", errorMsg); - argsJson = "{}"; - } - } + String argsJson = JsonUtils.resolveToolCallArgsJson(toolUse); // Add thought signature if present in metadata (required for Gemini) String signature = null; diff --git a/agentscope-core/src/main/java/io/agentscope/core/util/JsonUtils.java b/agentscope-core/src/main/java/io/agentscope/core/util/JsonUtils.java index 2e1f90fef..ab50d25b4 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/util/JsonUtils.java +++ b/agentscope-core/src/main/java/io/agentscope/core/util/JsonUtils.java @@ -16,6 +16,11 @@ package io.agentscope.core.util; +import io.agentscope.core.message.ToolUseBlock; +import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** * Utility class for accessing the global {@link JsonCodec} instance. * @@ -43,6 +48,8 @@ */ public final class JsonUtils { + private static final Logger log = LoggerFactory.getLogger(JsonUtils.class); + private static volatile JsonCodec codec = new JacksonJsonCodec(); private JsonUtils() { @@ -82,4 +89,64 @@ public static void setJsonCodec(JsonCodec jsonCodec) { public static void resetToDefault() { codec = new JacksonJsonCodec(); } + + /** + * Check whether the given string is a valid JSON object (i.e. starts with '{' and + * can be parsed into a {@link Map}). + * + *

Tool call {@code arguments} must be JSON objects, so plain JSON values like + * {@code null}, arrays, or strings are rejected. + * + * @param str the string to validate + * @return {@code true} if {@code str} is a non-null, parseable JSON object + */ + @SuppressWarnings("unchecked") + public static boolean isValidJsonObject(String str) { + if (str == null || str.isEmpty()) { + return false; + } + try { + Map parsed = codec.fromJson(str, Map.class); + return parsed != null; + } catch (Exception e) { + return false; + } + } + + /** + * Resolve the arguments JSON string from a {@link ToolUseBlock}, ensuring the + * result is always a valid JSON object. + * + *

Resolution order: + *

    + *
  1. Use {@link ToolUseBlock#getContent()} if it is a valid JSON object
    2. + *
  2. Serialize {@link ToolUseBlock#getInput()} via {@link JsonCodec#toJson}
    3. + *
  3. Fall back to {@code "{}"}
    4. + *
+ * + *

This prevents sending malformed JSON (e.g. from interrupted streaming) as + * tool call arguments, which would cause model APIs to reject the request. + * + * @param toolUse the tool use block + * @return a valid JSON object string representing the tool call arguments + */ + public static String resolveToolCallArgsJson(ToolUseBlock toolUse) { + String content = toolUse.getContent(); + if (content != null && !content.isEmpty()) { + if (isValidJsonObject(content)) { + return content; + } + log.warn( + "Invalid JSON in tool call content for '{}', falling back to input" + + " serialization", + toolUse.getName()); + } + + try { + return codec.toJson(toolUse.getInput()); + } catch (Exception e) { + log.warn("Failed to serialize tool call arguments: {}", e.getMessage()); + return "{}"; + } + } } diff --git a/agentscope-core/src/test/java/io/agentscope/core/agent/accumulator/ToolCallsAccumulatorTest.java b/agentscope-core/src/test/java/io/agentscope/core/agent/accumulator/ToolCallsAccumulatorTest.java index eb197d574..3e5446526 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/agent/accumulator/ToolCallsAccumulatorTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/agent/accumulator/ToolCallsAccumulatorTest.java @@ -323,4 +323,93 @@ void testMultipleParallelToolCallsWithStreamingChunks() { List allCalls = accumulator.getAllAccumulatedToolCalls(); assertEquals(2, allCalls.size()); } + + @Test + @DisplayName("Should produce valid JSON content when streaming is interrupted mid-arguments") + void testInterruptedStreamingProducesValidJsonContent() { + // Simulate streaming that gets interrupted mid-arguments: + // Model was outputting {"query": "hello wor... but got cut off + ToolUseBlock chunk1 = + ToolUseBlock.builder() + .id("call_1") + .name("search") + .content("{\"query\": \"hello wor") + .build(); + + accumulator.add(chunk1); + + List result = accumulator.buildAllToolCalls(); + assertEquals(1, result.size()); + + ToolUseBlock toolCall = result.get(0); + // Content should fall back to "{}" since the raw content is invalid JSON + assertEquals("{}", toolCall.getContent()); + // Input should be empty since parsing failed + assertTrue(toolCall.getInput().isEmpty()); + } + + @Test + @DisplayName("Should produce valid JSON content when multiple chunks are interrupted") + void testInterruptedMultiChunkStreamingProducesValidJsonContent() { + // First chunk starts the arguments + ToolUseBlock chunk1 = + ToolUseBlock.builder() + .id("call_1") + .name("get_weather") + .content("{\"city\":") + .build(); + + // Second chunk is a partial value — streaming interrupted here + ToolUseBlock chunk2 = + ToolUseBlock.builder().id("call_1").name("__fragment__").content("\"Bei").build(); + + accumulator.add(chunk1); + accumulator.add(chunk2); + + List result = accumulator.buildAllToolCalls(); + assertEquals(1, result.size()); + + ToolUseBlock toolCall = result.get(0); + assertEquals("{}", toolCall.getContent()); + assertTrue(toolCall.getInput().isEmpty()); + } + + @Test + @DisplayName("Should handle non-object JSON content like arrays or null") + void testNonObjectJsonContentFallsBackToEmpty() { + ToolUseBlock chunk = + ToolUseBlock.builder().id("call_1").name("tool").content("[1, 2, 3]").build(); + + accumulator.add(chunk); + + List result = accumulator.buildAllToolCalls(); + assertEquals(1, result.size()); + // Arrays are not valid JSON objects for tool call arguments + assertEquals("{}", result.get(0).getContent()); + } + + @Test + @DisplayName("Should preserve valid content even when input was populated via merge") + void testValidContentPreservedWithMergedInput() { + // First chunk: input populated via parsed args + Map args = new HashMap<>(); + args.put("city", "Tokyo"); + ToolUseBlock chunk1 = + ToolUseBlock.builder() + .id("call_1") + .name("weather") + .input(args) + .content("{\"city\": \"Tokyo\"}") + .build(); + + accumulator.add(chunk1); + + List result = accumulator.buildAllToolCalls(); + assertEquals(1, result.size()); + + ToolUseBlock toolCall = result.get(0); + // Valid JSON content should be preserved + assertEquals("{\"city\": \"Tokyo\"}", toolCall.getContent()); + assertEquals("Tokyo", toolCall.getInput().get("city")); + } } diff --git a/agentscope-core/src/test/java/io/agentscope/core/formatter/dashscope/DashScopeToolsHelperComprehensiveTest.java b/agentscope-core/src/test/java/io/agentscope/core/formatter/dashscope/DashScopeToolsHelperComprehensiveTest.java index 6374a7cc1..f955824b6 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/formatter/dashscope/DashScopeToolsHelperComprehensiveTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/formatter/dashscope/DashScopeToolsHelperComprehensiveTest.java @@ -316,6 +316,46 @@ void testConvertToolCallsWithEmptyInput() { assertEquals("{}", result.get(0).getFunction().getArguments()); } + @Test + void testConvertToolCallsWithInvalidJsonContent() { + // Simulate interrupted streaming: content is incomplete JSON + ToolUseBlock block = + ToolUseBlock.builder() + .id("call_broken") + .name("search") + .input(Map.of("query", "test")) + .content("{\"query\": \"hel") + .build(); + + List result = helper.convertToolCalls(List.of(block)); + + assertEquals(1, result.size()); + String argsJson = result.get(0).getFunction().getArguments(); + // Should fall back to input serialization, not the broken content + assertTrue(argsJson.contains("query")); + assertTrue(argsJson.contains("test")); + } + + @Test + void testConvertToolCallsWithNonObjectJsonContent() { + // Content is valid JSON but not an object (array) + ToolUseBlock block = + ToolUseBlock.builder() + .id("call_array") + .name("tool") + .input(Map.of("key", "value")) + .content("[1, 2, 3]") + .build(); + + List result = helper.convertToolCalls(List.of(block)); + + assertEquals(1, result.size()); + String argsJson = result.get(0).getFunction().getArguments(); + // Should fall back to input since content is not a JSON object + assertTrue(argsJson.contains("key")); + assertTrue(argsJson.contains("value")); + } + @Test void testConvertToolCallsWithComplexArgs() { Map complexArgs = new HashMap<>(); diff --git a/agentscope-core/src/test/java/io/agentscope/core/formatter/openai/OpenAIMessageConverterTest.java b/agentscope-core/src/test/java/io/agentscope/core/formatter/openai/OpenAIMessageConverterTest.java index 665fce463..6ac9fdb44 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/formatter/openai/OpenAIMessageConverterTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/formatter/openai/OpenAIMessageConverterTest.java @@ -531,6 +531,54 @@ void testToolCallFallbackToInputMapWhenContentEmpty() { assertTrue(args.contains("city")); assertTrue(args.contains("Shanghai")); } + + @Test + @DisplayName( + "Should fallback to input when content is invalid JSON (interrupted streaming)") + void testToolCallFallbackWhenContentIsInvalidJson() { + ToolUseBlock toolBlock = + ToolUseBlock.builder() + .id("call_broken") + .name("search") + .input(Map.of("query", "hello")) + .content("{\"query\": \"hel") + .build(); + + Msg msg = Msg.builder().role(MsgRole.ASSISTANT).content(List.of(toolBlock)).build(); + + OpenAIMessage result = converter.convertToMessage(msg, false); + + assertNotNull(result); + assertNotNull(result.getToolCalls()); + assertEquals(1, result.getToolCalls().size()); + String args = result.getToolCalls().get(0).getFunction().getArguments(); + // Should fall back to input serialization + assertTrue(args.contains("query")); + assertTrue(args.contains("hello")); + } + + @Test + @DisplayName("Should fallback to input when content is non-object JSON like array") + void testToolCallFallbackWhenContentIsNonObjectJson() { + ToolUseBlock toolBlock = + ToolUseBlock.builder() + .id("call_array") + .name("tool") + .input(Map.of("key", "value")) + .content("[1, 2, 3]") + .build(); + + Msg msg = Msg.builder().role(MsgRole.ASSISTANT).content(List.of(toolBlock)).build(); + + OpenAIMessage result = converter.convertToMessage(msg, false); + + assertNotNull(result); + assertNotNull(result.getToolCalls()); + assertEquals(1, result.getToolCalls().size()); + String args = result.getToolCalls().get(0).getFunction().getArguments(); + assertTrue(args.contains("key")); + assertTrue(args.contains("value")); + } } @Nested diff --git a/agentscope-core/src/test/java/io/agentscope/core/util/JsonUtilsTest.java b/agentscope-core/src/test/java/io/agentscope/core/util/JsonUtilsTest.java index 3ea0c8411..9614d46b1 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/util/JsonUtilsTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/util/JsonUtilsTest.java @@ -17,17 +17,22 @@ package io.agentscope.core.util; 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.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.fasterxml.jackson.core.type.TypeReference; +import io.agentscope.core.message.ToolUseBlock; import java.lang.reflect.Type; import java.util.Map; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; class JsonUtilsTest { @@ -116,6 +121,138 @@ void testCustomCodecIsUsed() { assertEquals("custom_json", result); } + @Nested + @DisplayName("isValidJsonObject Tests") + class IsValidJsonObjectTests { + + @Test + @DisplayName("Should accept valid JSON object") + void testValidJsonObject() { + assertTrue(JsonUtils.isValidJsonObject("{\"key\":\"value\"}")); + assertTrue(JsonUtils.isValidJsonObject("{}")); + assertTrue(JsonUtils.isValidJsonObject("{\"a\":1,\"b\":true}")); + } + + @Test + @DisplayName("Should reject incomplete JSON") + void testIncompleteJson() { + assertFalse(JsonUtils.isValidJsonObject("{\"query\":\"hel")); + assertFalse(JsonUtils.isValidJsonObject("{\"key\":")); + assertFalse(JsonUtils.isValidJsonObject("{")); + } + + @Test + @DisplayName("Should reject non-object JSON values") + void testNonObjectJsonValues() { + assertFalse(JsonUtils.isValidJsonObject("null")); + assertFalse(JsonUtils.isValidJsonObject("[1,2,3]")); + assertFalse(JsonUtils.isValidJsonObject("\"just a string\"")); + assertFalse(JsonUtils.isValidJsonObject("42")); + assertFalse(JsonUtils.isValidJsonObject("true")); + } + + @Test + @DisplayName("Should reject null and empty") + void testNullAndEmpty() { + assertFalse(JsonUtils.isValidJsonObject(null)); + assertFalse(JsonUtils.isValidJsonObject("")); + } + } + + @Nested + @DisplayName("resolveToolCallArgsJson Tests") + class ResolveToolCallArgsJsonTests { + + @Test + @DisplayName("Should return valid content when present") + void testValidContent() { + ToolUseBlock block = + ToolUseBlock.builder() + .id("call_1") + .name("tool") + .input(Map.of("fallback", "value")) + .content("{\"key\":\"value\"}") + .build(); + + assertEquals("{\"key\":\"value\"}", JsonUtils.resolveToolCallArgsJson(block)); + } + + @Test + @DisplayName("Should fall back to input when content is invalid JSON") + void testInvalidContentFallsBackToInput() { + ToolUseBlock block = + ToolUseBlock.builder() + .id("call_1") + .name("tool") + .input(Map.of("city", "Beijing")) + .content("{\"query\":\"hel") + .build(); + + String result = JsonUtils.resolveToolCallArgsJson(block); + assertTrue(result.contains("city")); + assertTrue(result.contains("Beijing")); + } + + @Test + @DisplayName("Should fall back to input when content is null") + void testNullContentFallsBackToInput() { + ToolUseBlock block = + ToolUseBlock.builder() + .id("call_1") + .name("tool") + .input(Map.of("key", "value")) + .content(null) + .build(); + + String result = JsonUtils.resolveToolCallArgsJson(block); + assertTrue(result.contains("key")); + } + + @Test + @DisplayName("Should fall back to input when content is empty") + void testEmptyContentFallsBackToInput() { + ToolUseBlock block = + ToolUseBlock.builder() + .id("call_1") + .name("tool") + .input(Map.of("key", "value")) + .content("") + .build(); + + String result = JsonUtils.resolveToolCallArgsJson(block); + assertTrue(result.contains("key")); + } + + @Test + @DisplayName("Should reject non-object JSON content like arrays") + void testNonObjectJsonContentFallsBackToInput() { + ToolUseBlock block = + ToolUseBlock.builder() + .id("call_1") + .name("tool") + .input(Map.of("key", "value")) + .content("[1,2,3]") + .build(); + + String result = JsonUtils.resolveToolCallArgsJson(block); + assertTrue(result.contains("key")); + } + + @Test + @DisplayName("Should return {} when both content and input are empty") + void testEmptyContentAndInput() { + ToolUseBlock block = + ToolUseBlock.builder() + .id("call_1") + .name("tool") + .input(Map.of()) + .content("") + .build(); + + assertEquals("{}", JsonUtils.resolveToolCallArgsJson(block)); + } + } + private static class CustomJsonCodec implements JsonCodec { @Override