From 0edab1b054720d05f7b9dd00532952662854811c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BA=A6=E7=9B=8F?= Date: Tue, 7 Apr 2026 10:19:02 +0800 Subject: [PATCH 1/3] fix: sanitize tool call arguments JSON on interrupted streaming to prevent 400 errors When streaming tool call responses are interrupted, the partially accumulated arguments (incomplete JSON) were saved to memory as-is. On subsequent requests, this malformed JSON was sent to the model API as tool call arguments, causing DashScope/OpenAI to reject with 400. Fixes #1147 Change-Id: I23701960b5d4fa0962f872fb48a6252b13c3dc0b Co-developed-by: Cursor --- .../accumulator/ToolCallsAccumulator.java | 28 +++++++++- .../dashscope/DashScopeToolsHelper.java | 47 +++++++++++----- .../openai/OpenAIMessageConverter.java | 53 +++++++++++++------ 3 files changed, 97 insertions(+), 31 deletions(-) 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..d92355784 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 @@ -86,6 +86,7 @@ void merge(ToolUseBlock block) { ToolUseBlock build() { Map finalArgs = new HashMap<>(args); String rawContentStr = this.rawContent.toString(); + boolean rawContentParsed = false; // If no parsed arguments but has raw JSON content, try to parse if (finalArgs.isEmpty() && rawContentStr.length() > 0) { @@ -95,21 +96,46 @@ ToolUseBlock build() { JsonUtils.getJsonCodec().fromJson(rawContentStr, Map.class); if (parsed != null) { finalArgs.putAll(parsed); + rawContentParsed = true; } } catch (Exception ignored) { // Parsing failed, keep empty args } } + // Use raw content only if it was successfully parsed or args were + // already populated (meaning the content matches the parsed input). + // Otherwise fall back to "{}" to avoid sending malformed JSON + // (e.g. when streaming was interrupted mid-arguments). + String contentStr; + if (rawContentStr.isEmpty()) { + contentStr = "{}"; + } else if (!finalArgs.isEmpty() || rawContentParsed) { + contentStr = rawContentStr; + } else if (isValidJson(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(); } + private boolean isValidJson(String str) { + try { + JsonUtils.getJsonCodec().fromJson(str, Object.class); + return true; + } catch (Exception e) { + return false; + } + } + private boolean isPlaceholder(String name) { // Common placeholder names return "__fragment__".equals(name) 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..ec97010d2 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 = resolveArgsJson(toolUse); DashScopeFunction function = DashScopeFunction.of(toolUse.getName(), argsJson); DashScopeToolCall toolCall = @@ -332,4 +320,37 @@ public Map mergeAdditionalQueryParams( return result.isEmpty() ? null : result; } + + /** + * Resolve the arguments JSON string from a ToolUseBlock, with validation + * to prevent sending malformed JSON (e.g. from interrupted streaming). + */ + private String resolveArgsJson(ToolUseBlock toolUse) { + String content = toolUse.getContent(); + if (content != null && !content.isEmpty()) { + if (isValidJson(content)) { + return content; + } + log.warn( + "Invalid JSON in tool call content for '{}', falling back to input" + + " serialization", + toolUse.getName()); + } + + try { + return JsonUtils.getJsonCodec().toJson(toolUse.getInput()); + } catch (Exception e) { + log.warn("Failed to serialize tool call arguments: {}", e.getMessage()); + return "{}"; + } + } + + private boolean isValidJson(String str) { + try { + JsonUtils.getJsonCodec().fromJson(str, Object.class); + return true; + } catch (Exception e) { + return false; + } + } } 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..54420286e 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 = resolveArgsJson(toolUse); // Add thought signature if present in metadata (required for Gemini) String signature = null; @@ -492,4 +476,39 @@ private void applyCacheControlFromMetadata(Msg msg, OpenAIMessage result) { result.setCacheControl(OpenAIBaseFormatter.getEphemeralCacheControl()); } } + + /** + * Resolve the arguments JSON string from a ToolUseBlock, with validation + * to prevent sending malformed JSON (e.g. from interrupted streaming). + */ + private String resolveArgsJson(ToolUseBlock toolUse) { + String content = toolUse.getContent(); + if (content != null && !content.isEmpty()) { + if (isValidJson(content)) { + return content; + } + log.warn( + "Invalid JSON in tool call content for '{}', falling back to input" + + " serialization", + toolUse.getName()); + } + + try { + return 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); + return "{}"; + } + } + + private boolean isValidJson(String str) { + try { + JsonUtils.getJsonCodec().fromJson(str, Object.class); + return true; + } catch (Exception e) { + return false; + } + } } From 2b24184eb2e0234d0ae34d58a3c4af592d1acbcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BA=A6=E7=9B=8F?= Date: Tue, 7 Apr 2026 13:39:18 +0800 Subject: [PATCH 2/3] refactor: extract shared JSON validation to JsonUtils, strengthen validation, add tests Address PR review feedback: - Extract duplicated resolveArgsJson/isValidJson into JsonUtils as shared isValidJsonObject() and resolveToolCallArgsJson() methods - Strengthen validation to reject non-object JSON (arrays, null, strings) since tool call arguments must be JSON objects - Fix edge case in ToolCallsAccumulator where finalArgs being non-empty didn't guarantee rawContent was valid JSON - Add comprehensive unit tests for all affected components Change-Id: I05640215b1de0a802434750a94804dd18e01605e Co-developed-by: Cursor --- .../accumulator/ToolCallsAccumulator.java | 20 +-- .../dashscope/DashScopeToolsHelper.java | 35 +---- .../openai/OpenAIMessageConverter.java | 37 +---- .../io/agentscope/core/util/JsonUtils.java | 67 +++++++++ .../accumulator/ToolCallsAccumulatorTest.java | 89 ++++++++++++ ...DashScopeToolsHelperComprehensiveTest.java | 40 +++++ .../openai/OpenAIMessageConverterTest.java | 48 ++++++ .../agentscope/core/util/JsonUtilsTest.java | 137 ++++++++++++++++++ 8 files changed, 386 insertions(+), 87 deletions(-) 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 d92355784..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 @@ -86,7 +86,6 @@ void merge(ToolUseBlock block) { ToolUseBlock build() { Map finalArgs = new HashMap<>(args); String rawContentStr = this.rawContent.toString(); - boolean rawContentParsed = false; // If no parsed arguments but has raw JSON content, try to parse if (finalArgs.isEmpty() && rawContentStr.length() > 0) { @@ -96,23 +95,19 @@ ToolUseBlock build() { JsonUtils.getJsonCodec().fromJson(rawContentStr, Map.class); if (parsed != null) { finalArgs.putAll(parsed); - rawContentParsed = true; } } catch (Exception ignored) { // Parsing failed, keep empty args } } - // Use raw content only if it was successfully parsed or args were - // already populated (meaning the content matches the parsed input). - // Otherwise fall back to "{}" to avoid sending malformed JSON + // 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 (!finalArgs.isEmpty() || rawContentParsed) { - contentStr = rawContentStr; - } else if (isValidJson(rawContentStr)) { + } else if (JsonUtils.isValidJsonObject(rawContentStr)) { contentStr = rawContentStr; } else { contentStr = "{}"; @@ -127,15 +122,6 @@ ToolUseBlock build() { .build(); } - private boolean isValidJson(String str) { - try { - JsonUtils.getJsonCodec().fromJson(str, Object.class); - return true; - } catch (Exception e) { - return false; - } - } - private boolean isPlaceholder(String name) { // Common placeholder names return "__fragment__".equals(name) 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 ec97010d2..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,7 +236,7 @@ public List convertToolCalls(List toolBlocks) { continue; } - String argsJson = resolveArgsJson(toolUse); + String argsJson = JsonUtils.resolveToolCallArgsJson(toolUse); DashScopeFunction function = DashScopeFunction.of(toolUse.getName(), argsJson); DashScopeToolCall toolCall = @@ -320,37 +320,4 @@ public Map mergeAdditionalQueryParams( return result.isEmpty() ? null : result; } - - /** - * Resolve the arguments JSON string from a ToolUseBlock, with validation - * to prevent sending malformed JSON (e.g. from interrupted streaming). - */ - private String resolveArgsJson(ToolUseBlock toolUse) { - String content = toolUse.getContent(); - if (content != null && !content.isEmpty()) { - if (isValidJson(content)) { - return content; - } - log.warn( - "Invalid JSON in tool call content for '{}', falling back to input" - + " serialization", - toolUse.getName()); - } - - try { - return JsonUtils.getJsonCodec().toJson(toolUse.getInput()); - } catch (Exception e) { - log.warn("Failed to serialize tool call arguments: {}", e.getMessage()); - return "{}"; - } - } - - private boolean isValidJson(String str) { - try { - JsonUtils.getJsonCodec().fromJson(str, Object.class); - return true; - } catch (Exception e) { - return false; - } - } } 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 54420286e..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,7 +330,7 @@ private OpenAIMessage convertAssistantMessage(Msg msg) { continue; } - String argsJson = resolveArgsJson(toolUse); + String argsJson = JsonUtils.resolveToolCallArgsJson(toolUse); // Add thought signature if present in metadata (required for Gemini) String signature = null; @@ -476,39 +476,4 @@ private void applyCacheControlFromMetadata(Msg msg, OpenAIMessage result) { result.setCacheControl(OpenAIBaseFormatter.getEphemeralCacheControl()); } } - - /** - * Resolve the arguments JSON string from a ToolUseBlock, with validation - * to prevent sending malformed JSON (e.g. from interrupted streaming). - */ - private String resolveArgsJson(ToolUseBlock toolUse) { - String content = toolUse.getContent(); - if (content != null && !content.isEmpty()) { - if (isValidJson(content)) { - return content; - } - log.warn( - "Invalid JSON in tool call content for '{}', falling back to input" - + " serialization", - toolUse.getName()); - } - - try { - return 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); - return "{}"; - } - } - - private boolean isValidJson(String str) { - try { - JsonUtils.getJsonCodec().fromJson(str, Object.class); - return true; - } catch (Exception e) { - return false; - } - } } 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..d8e74c912 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 { + codec.fromJson(str, Map.class); + return true; + } 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. + *
  3. Serialize {@link ToolUseBlock#getInput()} via {@link JsonCodec#toJson}
  4. + *
  5. Fall back to {@code "{}"}
  6. + *
+ * + *

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 From 1e3668f3329bfc898e5576fe8cb9b9e1eb40e8ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BA=A6=E7=9B=8F?= Date: Tue, 7 Apr 2026 14:02:04 +0800 Subject: [PATCH 3/3] fix: handle Jackson parsing "null" as null Map in isValidJsonObject Jackson's fromJson("null", Map.class) returns null instead of throwing, so isValidJsonObject("null") incorrectly returned true. Added null check on the parsed result to properly reject JSON null values. Change-Id: I411e6305aa90b06eb3bc906745a66b13fa133e66 Co-developed-by: Cursor --- .../src/main/java/io/agentscope/core/util/JsonUtils.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 d8e74c912..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 @@ -106,8 +106,8 @@ public static boolean isValidJsonObject(String str) { return false; } try { - codec.fromJson(str, Map.class); - return true; + Map parsed = codec.fromJson(str, Map.class); + return parsed != null; } catch (Exception e) { return false; }